From efdadf0ef7153deb502c45ee48e055c3e809c304 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:24:57 +0000 Subject: [PATCH 1/7] fix(skills): group skills pass write_log=false and their inputs to dispatched items An item skill 'run on its own' wrote a run-log entry, but a dispatched sub-agent was given only the item URL, so it could not tell it was dispatched and the item and the group could both log. It also lost runner, unattended, trusted_authors and every other input of the group run. Add write_log (default true) to the four item skills with their own log entry. Dispatching now passes the URL, write_log=false and every group input the item also lists in params. The validator checks a dispatched item can take write_log. --- scripts/contracts.mjs | 3 +++ skills/meta/agent-workflow-protocol/SKILL.md | 5 +++-- skills/meta/agent-workflow-protocol/references/log.md | 2 +- skills/meta/agent-workflow-protocol/scripts/lib.sh | 2 ++ skills/meta/runner-prompts/references/prompts.md | 4 ++++ skills/pipeline/implement-github-issue/SKILL.md | 5 +++-- skills/pipeline/implement-github-issues/SKILL.md | 1 + skills/pipeline/review-github-pr/SKILL.md | 5 +++-- skills/pipeline/review-github-prs/SKILL.md | 1 + skills/pipeline/shepherd-github-pr/SKILL.md | 5 +++-- skills/pipeline/shepherd-github-prs/SKILL.md | 1 + skills/pipeline/triage-github-issue/SKILL.md | 5 +++-- skills/pipeline/triage-github-issues/SKILL.md | 1 + tests/contracts.test.mjs | 8 ++++++++ tests/lib.test.sh | 2 ++ 15 files changed, 39 insertions(+), 11 deletions(-) diff --git a/scripts/contracts.mjs b/scripts/contracts.mjs index 5e1ced4..38d87e6 100644 --- a/scripts/contracts.mjs +++ b/scripts/contracts.mjs @@ -171,6 +171,9 @@ export function checkContracts(skills, byName, external = new Set()) { if (tc.kind !== "item") errors.push(`${s.rel}: contract: dispatches \`${c.dispatches}\`, which is not an item skill`); if (tc.step !== c.step) errors.push(`${s.rel}: contract: dispatches \`${c.dispatches}\`, which belongs to ${tc.step} and not ${c.step}`); if (t.userInvoked) errors.push(`${s.rel}: contract: dispatches \`${c.dispatches}\`, which is user-invoked`); + if (tc.effects?.writes?.shared && !(tc.params ?? []).includes("write_log")) { + errors.push(`${s.rel}: contract: dispatches \`${c.dispatches}\`, which writes the run log but has no \`write_log\` param, so the group cannot stop it logging twice`); + } } } return { errors, contracts: parsed }; diff --git a/skills/meta/agent-workflow-protocol/SKILL.md b/skills/meta/agent-workflow-protocol/SKILL.md index c5efdbc..bd40afa 100644 --- a/skills/meta/agent-workflow-protocol/SKILL.md +++ b/skills/meta/agent-workflow-protocol/SKILL.md @@ -43,9 +43,9 @@ The workflow has five steps: Exploring, Triaging, Implementing, Shepherding, Rev 4. **Work.** The skill's own steps. 5. **Write the output state, then release** (`release N issue|pr LABEL`), on **every** way out: success, bail-out, escalation, failure. 6. **Failure.** If a step fails, leave the state labels as they were, run `fail_item N issue|pr STEP ""`, release, and return `#N failed: `. -7. **Result.** One line, and only one of the lines in the contract's `returns`: `#N `, `#N skipped: ` or `#N failed: `. For example `#31 ready-for-agent`, `#35 skipped: leased`, `#44 failed: could not read ADR link (404)`. A group skill logs the results of the items it dispatched. An item skill run on its own writes its own run-log entry (`references/log.md`). +7. **Result.** One line, and only one of the lines in the contract's `returns`: `#N `, `#N skipped: ` or `#N failed: `. For example `#31 ready-for-agent`, `#35 skipped: leased`, `#44 failed: could not read ADR link (404)`. A group skill logs the results of the items it dispatched. An item skill writes its own run-log entry (`references/log.md`) only when `write_log` is `true`, which is the default. A group skill sets it to `false` when it dispatches. -**Dispatching.** Call the Skill tool with the item skill's name and give it the item's URL. If your runner can start sub-agents, run each item as its own sub-agent, given only the item's URL, so context stays small and one failing item cannot affect the rest. Otherwise run the items one after another. Either way, collect each item's result. +**Dispatching.** Call the Skill tool with the item skill's name and give it three things: the item's URL, `write_log=false` (the group writes the log entry), and every key-value input of this run that the item skill also lists under `params` in its contract (for example `runner`, `unattended`, `trusted_authors`, `repo_guidance`, `pr_base_branch` and the lease TTLs). A sub-agent starts with none of this run's inputs, so anything it is not given falls back to its default. If your runner can start sub-agents, run each item as its own sub-agent, so context stays small and one failing item cannot affect the rest. Otherwise run the items one after another. Either way, collect each item's result. The steps are independent. Do not assume another step has run, will run, or is running. @@ -104,6 +104,7 @@ A key-value pair and free text that name the same thing resolve to the pair. An | `runner` | `unknown` | `explore-github-repo`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | | `unattended` | `true` | `explore-github-repo`, `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | | `trusted_authors` | *(none)* | `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | +| `write_log` | `true` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | ## Unattended runs diff --git a/skills/meta/agent-workflow-protocol/references/log.md b/skills/meta/agent-workflow-protocol/references/log.md index fb9b2d4..a2d3667 100644 --- a/skills/meta/agent-workflow-protocol/references/log.md +++ b/skills/meta/agent-workflow-protocol/references/log.md @@ -75,7 +75,7 @@ Rules: - **Problems** are the run's difficulties. Start each with a tag: `failed`, `escalated`, `flaky`, `degraded`, `repo-state`, or `security`. - **Things to improve** are changes that would make the next run easier: a fix to a skill, a missing tool or permission, a repeated deterministic step worth scripting, a repo problem. Only suggest; never implement unprompted. (Older entries call this **Automation suggestions**; readers accept both.) - Cap **Deferred candidates** at 20. Each line needs a one-line **impact × confidence** score. -- A group skill writes one entry per run covering all items it dispatched. An item skill run on its own writes an entry for its one item. +- A group skill writes one entry per run covering all items it dispatched, and dispatches them with `write_log=false`. An item skill writes an entry for its one item only when `write_log` is `true` (the default, for a direct call). - **One entry per run and step.** `log_run` skips an entry whose `run` and `step` are already on the log issue, so a retry is safe. A runner with no shell checks the issue's comments (`hasLogEntry`) first. - Write real line breaks, never the two characters backslash and `n`. `log_run` repairs an entry written that way, but do not rely on it. - Write the entry to a temp file, then run `log_run FILE` (shell) or post the same body with GitHub tools (no-shell). Use `formatLogComment` from `scripts/helpers.mjs` so the header and field order stay identical across runners. diff --git a/skills/meta/agent-workflow-protocol/scripts/lib.sh b/skills/meta/agent-workflow-protocol/scripts/lib.sh index fe13240..19fca8e 100644 --- a/skills/meta/agent-workflow-protocol/scripts/lib.sh +++ b/skills/meta/agent-workflow-protocol/scripts/lib.sh @@ -50,6 +50,7 @@ AGENT_ROOT="$(git rev-parse --show-toplevel)" : "${runner:=unknown}" : "${unattended:=true}" : "${trusted_authors:=}" +: "${write_log:=true}" if [ -n "${AGENT_OVERRIDES:-}" ] && [ -f "${AGENT_OVERRIDES}" ]; then # shellcheck disable=SC1090 @@ -82,6 +83,7 @@ max_item_failures:MAX_ITEM_FAILURES runner:RUNNER unattended:UNATTENDED trusted_authors:TRUSTED_AUTHORS +write_log:WRITE_LOG " while IFS=: read -r _lower _upper; do [ -n "$_lower" ] || continue diff --git a/skills/meta/runner-prompts/references/prompts.md b/skills/meta/runner-prompts/references/prompts.md index 0ec4f28..e3b0f95 100644 --- a/skills/meta/runner-prompts/references/prompts.md +++ b/skills/meta/runner-prompts/references/prompts.md @@ -67,6 +67,7 @@ runner=NAME log_window_days=14 unattended=true trusted_authors= +write_log=true ``` ## Implementing @@ -96,6 +97,7 @@ runner=NAME log_window_days=14 unattended=true trusted_authors= +write_log=true ``` ## Shepherding @@ -126,6 +128,7 @@ runner=NAME log_window_days=14 unattended=true trusted_authors= +write_log=true ``` ## Reviewing @@ -158,4 +161,5 @@ runner=NAME log_window_days=14 unattended=true trusted_authors= +write_log=true ``` diff --git a/skills/pipeline/implement-github-issue/SKILL.md b/skills/pipeline/implement-github-issue/SKILL.md index 80c9f4c..441b5e8 100644 --- a/skills/pipeline/implement-github-issue/SKILL.md +++ b/skills/pipeline/implement-github-issue/SKILL.md @@ -19,7 +19,7 @@ See **Contracts** in the protocol for what each key means. kind: item step: Implementing target: { param: issue, type: issue } -params: [issue, pr_base_branch, build_cmd, git_hooks_path, repo_guidance, lease_ttl_implement, max_item_failures, runner, log_window_days, unattended, trusted_authors] +params: [issue, pr_base_branch, build_cmd, git_hooks_path, repo_guidance, lease_ttl_implement, max_item_failures, runner, log_window_days, unattended, trusted_authors, write_log] requires: - "The issue is open and labelled ready-for-agent, without agent:stuck." - "After `clear_stale_lease N issue`, it does not carry agent:in-progress with `lease_live N` succeeding." @@ -57,7 +57,7 @@ effects: other: - "agent:needs-work on an existing OPEN agent PR for the issue." shared: - - "One run-log entry, when run on its own and not dispatched by implement-github-issues." + - "One run-log entry (when write_log is true)." external: ["build_cmd, when set, and the repository's test commands."] never: - "Wait for checks, fix CI, or post the reviewer handoff. shepherd-github-pr does that." @@ -90,6 +90,7 @@ terminates: "failures N Implementing reaching max_item_failures escalates and st | `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts | | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | +| `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `tdd` and `code-review` skills (through the Skill tool), a working build toolchain, a git identity, and the issue's Agent Brief. Without a shell, keep the change small and record what you could not run under **Problems** in the run log. diff --git a/skills/pipeline/implement-github-issues/SKILL.md b/skills/pipeline/implement-github-issues/SKILL.md index 4c05c92..a130bff 100644 --- a/skills/pipeline/implement-github-issues/SKILL.md +++ b/skills/pipeline/implement-github-issues/SKILL.md @@ -25,6 +25,7 @@ returns: - "nothing eligible" ensures: - "At most implement_batch issues were dispatched, oldest first, each one passing implement-github-issue's requires. Issues labelled agent:stuck were not candidates." + - "Each dispatched item was given `write_log=false`, and every input of this run that it also lists in `params`." - "One run-log entry exists." effects: reads: [open issues labelled ready-for-agent] diff --git a/skills/pipeline/review-github-pr/SKILL.md b/skills/pipeline/review-github-pr/SKILL.md index e12824a..61cfe39 100644 --- a/skills/pipeline/review-github-pr/SKILL.md +++ b/skills/pipeline/review-github-pr/SKILL.md @@ -19,7 +19,7 @@ See **Contracts** in the protocol for what each key means. kind: item step: Reviewing target: { param: pr, type: pr } -params: [pr, repo_guidance, lease_ttl_pr, max_review_rounds, max_item_failures, merge_method, pr_base_branch, human_reviewer, human_merge_paths, git_hooks_path, runner, log_window_days, unattended, trusted_authors] +params: [pr, repo_guidance, lease_ttl_pr, max_review_rounds, max_item_failures, merge_method, pr_base_branch, human_reviewer, human_merge_paths, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log] requires: - "The PR is open, not a draft, and labelled agent:ready-for-review, without agent:stuck or agent:human-merge." - "After `clear_stale_lease N pr`, it does not carry agent:in-progress with `lease_live N` succeeding." @@ -57,7 +57,7 @@ effects: - "Linked issues: removal of agent:pr-open, closing with a comment, or ready-for-human for a keep-open issue." - "A review request to human_reviewer or the CODEOWNERS when a PR is escalated or handed to a person." shared: - - "One run-log entry, when run on its own and not dispatched by review-github-prs." + - "One run-log entry (when write_log is true)." external: ["The repository's test commands, when a fix is pushed."] never: - "Merge in the same run in which you pushed a commit. A fresh review run must review the new head." @@ -95,6 +95,7 @@ terminates: "review_rounds N reaching max_review_rounds hands the PR to a person | `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts | | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | +| `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `merge-safety` and `code-review` skills (through the Skill tool), a token permitted to merge, and the ability to pin a merge to one commit. Without the ability to pin a merge, follow **Unattended runs** in the protocol and record it under **Problems**. diff --git a/skills/pipeline/review-github-prs/SKILL.md b/skills/pipeline/review-github-prs/SKILL.md index a594134..529cf7b 100644 --- a/skills/pipeline/review-github-prs/SKILL.md +++ b/skills/pipeline/review-github-prs/SKILL.md @@ -25,6 +25,7 @@ returns: - "nothing eligible" ensures: - "At most review_batch PRs were dispatched, oldest first, each one passing review-github-pr's requires. Drafts and PRs labelled agent:stuck or agent:human-merge were not candidates." + - "Each dispatched item was given `write_log=false`, and every input of this run that it also lists in `params`." - "One run-log entry exists." effects: reads: [open non-draft PRs labelled agent:ready-for-review] diff --git a/skills/pipeline/shepherd-github-pr/SKILL.md b/skills/pipeline/shepherd-github-pr/SKILL.md index d893262..fcbafa5 100644 --- a/skills/pipeline/shepherd-github-pr/SKILL.md +++ b/skills/pipeline/shepherd-github-pr/SKILL.md @@ -21,7 +21,7 @@ See **Contracts** in the protocol for what each key means. kind: item step: Shepherding target: { param: pr, type: pr } -params: [pr, lease_ttl_pr, ready_lane_ttl, max_shepherd_attempts, max_item_failures, human_reviewer, repo_guidance, git_hooks_path, runner, log_window_days, unattended, trusted_authors] +params: [pr, lease_ttl_pr, ready_lane_ttl, max_shepherd_attempts, max_item_failures, human_reviewer, repo_guidance, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log] requires: - "The PR is open and labelled agent:needs-work, without agent:stuck or agent:human-merge." - "After `clear_stale_lease N pr`, it does not carry agent:in-progress with `lease_live N` succeeding." @@ -55,7 +55,7 @@ effects: other: - "A one-line comment on the linked issue, and a review request to human_reviewer, when escalating." shared: - - "One run-log entry, when run on its own and not dispatched by shepherd-github-prs." + - "One run-log entry (when write_log is true)." external: ["The repository's test commands."] never: - "Wait for CI to finish." @@ -87,6 +87,7 @@ terminates: "shepherd_attempts N reaching max_shepherd_attempts, or failures N r | `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts | | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | +| `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus read access to the failing check logs and a git identity. Without the log access this needs, follow **Unattended runs** in the protocol and record it under **Problems**. diff --git a/skills/pipeline/shepherd-github-prs/SKILL.md b/skills/pipeline/shepherd-github-prs/SKILL.md index 107d023..9a88f50 100644 --- a/skills/pipeline/shepherd-github-prs/SKILL.md +++ b/skills/pipeline/shepherd-github-prs/SKILL.md @@ -26,6 +26,7 @@ returns: ensures: - "At most shepherd_batch PRs were dispatched, oldest first, each one passing shepherd-github-pr's requires. PRs labelled agent:stuck or agent:human-merge were not candidates." - "A PR whose checks are still running was skipped, not waited for." + - "Each dispatched item was given `write_log=false`, and every input of this run that it also lists in `params`." - "One run-log entry exists." effects: reads: [open PRs labelled agent:needs-work, their check state] diff --git a/skills/pipeline/triage-github-issue/SKILL.md b/skills/pipeline/triage-github-issue/SKILL.md index f78319e..51b8764 100644 --- a/skills/pipeline/triage-github-issue/SKILL.md +++ b/skills/pipeline/triage-github-issue/SKILL.md @@ -19,7 +19,7 @@ See **Contracts** in the protocol for what each key means. kind: item step: Triaging target: { param: issue, type: issue } -params: [issue, repo_guidance, lease_ttl_triage, max_item_failures, dedupe_closed_days, runner, log_window_days, unattended, trusted_authors] +params: [issue, repo_guidance, lease_ttl_triage, max_item_failures, dedupe_closed_days, runner, log_window_days, unattended, trusted_authors, write_log] requires: - "The issue is open." - "It carries none of ready-for-agent, ready-for-human, needs-info, agent:pr-open, agent:stuck, agent:log, agent:difficulty. needs-triage does not exclude it: it means an earlier pass did not finish." @@ -52,7 +52,7 @@ effects: other: - "The comments references/dedupe.md allows on other issues." shared: - - "One run-log entry, when run on its own and not dispatched by triage-github-issues." + - "One run-log entry (when write_log is true)." never: - "Write or change code, or touch PRs." - "Commit, or edit GLOSSARY.md, ADRs or .out-of-scope/." @@ -83,6 +83,7 @@ terminates: "failures N Triaging reaching max_item_failures escalates and stops. | `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts | | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | +| `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `unattended-triage` skill (through the Skill tool), and the issue's own comments and images. diff --git a/skills/pipeline/triage-github-issues/SKILL.md b/skills/pipeline/triage-github-issues/SKILL.md index a064864..199da95 100644 --- a/skills/pipeline/triage-github-issues/SKILL.md +++ b/skills/pipeline/triage-github-issues/SKILL.md @@ -29,6 +29,7 @@ ensures: - "An open agent:pr-open issue whose PR merged was closed with `Implemented in #P.` and lost agent:pr-open. A keep-open issue was left open and handled as references/client-guidance.md says." - "An open agent:pr-open issue whose every PR was closed unmerged is ready-for-human, with a comment, and is not agent:pr-open." - "Every non-trivial repair is listed under Problems in the log with the `repo-state` tag, as issue number and from to to." + - "Each dispatched item was given `write_log=false`, and every input of this run that it also lists in `params`." - "One run-log entry exists, listing the housekeeping done." effects: reads: [open issues with their labels and comments, "prs_for_issue", the repository's own guidance, item labels] diff --git a/tests/contracts.test.mjs b/tests/contracts.test.mjs index 873d3d5..f7bf024 100644 --- a/tests/contracts.test.mjs +++ b/tests/contracts.test.mjs @@ -53,6 +53,14 @@ test("a group dispatching an item of the same step is fine", () => { assert.deepEqual(run([skill("do-them", GROUP, "", ["runner"]), item()]), []); }); +test("a dispatched item that writes the run log must accept write_log", () => { + const logging = ITEM.replace('claimed: ["A label."]', 'claimed: ["A label."]\n shared: ["One run-log entry."]'); + const errs = run([skill("do-them", GROUP, "", ["runner"]), item(logging)]); + assert.ok(errs.some((e) => /no `write_log` param/.test(e))); + const ok = logging.replace("[issue, runner]", "[issue, runner, write_log]"); + assert.deepEqual(run([skill("do-them", GROUP, "", ["runner"]), item(ok, "", ["issue", "runner", "write_log"])]), []); +}); + test("extractContract finds the yaml block and ignores later fences", () => { const found = extractContract(item().body); assert.match(found.yaml, /^kind: item/); diff --git a/tests/lib.test.sh b/tests/lib.test.sh index 0941cbf..a7d9f97 100644 --- a/tests/lib.test.sh +++ b/tests/lib.test.sh @@ -359,6 +359,8 @@ echo "Deprecated names and duration forms" check "groom_batch still sets triage_batch" "7" "$(groom_batch=7 bash -c 'source "$1"; echo "$triage_batch"' _ "$LIB")" check "triage_batch wins over groom_batch" "2" "$(groom_batch=7 triage_batch=2 bash -c 'source "$1"; echo "$triage_batch"' _ "$LIB")" check "issues_per_run still sets explore_batch" "9" "$(issues_per_run=9 bash -c 'source "$1"; echo "$explore_batch"' _ "$LIB")" +check "write_log default" "true" "$(bash -c 'source "$1"; echo "$write_log"' _ "$LIB")" +check "write_log can be set" "false" "$(write_log=false bash -c 'source "$1"; echo "$WRITE_LOG"' _ "$LIB")" check "explore_batch default" "5" "$(bash -c 'source "$1"; echo "$explore_batch"' _ "$LIB")" check "runner default feeds RUN_ID" "unknown" "$(env -u RUN_ID bash -c 'source "$1"; echo "${RUN_ID##*-}"' _ "$LIB")" check "runner is in RUN_ID" "grok" "$(env -u RUN_ID runner=grok bash -c 'source "$1"; echo "${RUN_ID##*-}"' _ "$LIB")" From 07251466491234b79693e1706752bd0e2bab873b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:25:24 +0000 Subject: [PATCH 2/7] fix(skills): separate skipped from noop; check lane and conflicts before claiming skipped meant 'nothing happened' before a claim but was also returned after one (merge lane held), after the claim had posted a lease comment, so a PR waiting on the lane collected a comment per run. Add #N noop: for a claimed item that is released unchanged, and make eligibility read the lane and merge state first so a PR that can only wait is skipped without a claim. A conflicted PR often runs no checks, so shepherd-github-pr skipped it as no-checks-yet and never reached its conflict handling. A DIRTY branch is now eligible whatever the checks say. review-github-pr step 4 sent a PR back to needs-work with no result line. It now returns #N not ready: . --- skills/meta/agent-workflow-protocol/SKILL.md | 4 ++-- skills/pipeline/review-github-pr/SKILL.md | 5 +++-- skills/pipeline/shepherd-github-pr/SKILL.md | 14 +++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/skills/meta/agent-workflow-protocol/SKILL.md b/skills/meta/agent-workflow-protocol/SKILL.md index bd40afa..5565d6c 100644 --- a/skills/meta/agent-workflow-protocol/SKILL.md +++ b/skills/meta/agent-workflow-protocol/SKILL.md @@ -37,13 +37,13 @@ The workflow has five steps: Exploring, Triaging, Implementing, Shepherding, Rev **Item skills** take exactly one issue or PR, as a URL or a number (`N` is the last path segment of a URL). Every item skill has the same frame, and its own **Process** fills in the middle: -1. **Eligibility.** Check the contract's `requires` yourself, so a direct call or a double dispatch is safe. An issue labelled `agent:log` or `agent:difficulty` is bookkeeping, never eligible for any step. If it fails, claim nothing and return `#N skipped: `. +1. **Eligibility.** Check the contract's `requires` yourself, so a direct call or a double dispatch is safe. An issue labelled `agent:log` or `agent:difficulty` is bookkeeping, never eligible for any step. If it fails, claim nothing and return `#N skipped: `. A skill checks everything it can by reading before it claims, so an item that can only wait is skipped without a lease comment. 2. **Claim.** `claim N issue|pr LABEL STEP TTL` (the step's lease TTL). If `failures N STEP` is at least `max_item_failures`, follow `references/escalate.md` and stop. 3. **Read the repository's own rules.** `references/client-guidance.md`: the client's `AGENTS.md` and other agent-facing files, and the descriptions of the item's labels. They outrank this package's defaults (not its **Trust** rules or **Never** lists). 4. **Work.** The skill's own steps. 5. **Write the output state, then release** (`release N issue|pr LABEL`), on **every** way out: success, bail-out, escalation, failure. 6. **Failure.** If a step fails, leave the state labels as they were, run `fail_item N issue|pr STEP ""`, release, and return `#N failed: `. -7. **Result.** One line, and only one of the lines in the contract's `returns`: `#N `, `#N skipped: ` or `#N failed: `. For example `#31 ready-for-agent`, `#35 skipped: leased`, `#44 failed: could not read ADR link (404)`. A group skill logs the results of the items it dispatched. An item skill writes its own run-log entry (`references/log.md`) only when `write_log` is `true`, which is the default. A group skill sets it to `false` when it dispatches. +7. **Result.** One line, and only one of the lines in the contract's `returns`: `#N `, `#N skipped: `, `#N noop: ` or `#N failed: `. `skipped` means nothing was claimed and nothing was written. `noop` means the item was claimed, found nothing to do and was released, so its lease comment remains. Anything else the skill did to the item has a result line of its own. For example `#31 ready-for-agent`, `#35 skipped: leased`, `#44 failed: could not read ADR link (404)`. A group skill logs the results of the items it dispatched. An item skill writes its own run-log entry (`references/log.md`) only when `write_log` is `true`, which is the default. A group skill sets it to `false` when it dispatches. **Dispatching.** Call the Skill tool with the item skill's name and give it three things: the item's URL, `write_log=false` (the group writes the log entry), and every key-value input of this run that the item skill also lists under `params` in its contract (for example `runner`, `unattended`, `trusted_authors`, `repo_guidance`, `pr_base_branch` and the lease TTLs). A sub-agent starts with none of this run's inputs, so anything it is not given falls back to its default. If your runner can start sub-agents, run each item as its own sub-agent, so context stays small and one failing item cannot affect the rest. Otherwise run the items one after another. Either way, collect each item's result. diff --git a/skills/pipeline/review-github-pr/SKILL.md b/skills/pipeline/review-github-pr/SKILL.md index 61cfe39..87fdcd2 100644 --- a/skills/pipeline/review-github-pr/SKILL.md +++ b/skills/pipeline/review-github-pr/SKILL.md @@ -28,6 +28,7 @@ returns: - "#N handed to a human: " - "#N deferred: base moved" - "#N sent back: " + - "#N not ready: " - "#N escalated: " - "#N skipped: " - "#N failed: " @@ -35,7 +36,7 @@ ensures: - "Everything applies to the head SHA pinned at step 2, and to no other commit." - "Merged: the merge was pinned to that SHA with `--match-head-commit`, the head branch was deleted (a failed delete is non-fatal and noted under Problems), and a merged note ends with ``." - "After a merge, each linked issue lost agent:pr-open and was closed with `Implemented in #N.`. A keep-open issue was left open and handled as references/client-guidance.md says." - - "A PR whose checks are not all green, or whose head is not `ready_sha N`, went back to agent:needs-work without a review." + - "A PR whose checks are not all green, or whose head is not `ready_sha N`, went back to agent:needs-work without a review, and the result is `#N not ready: `." - "Deferred: the base moved (BEHIND or DIRTY), a comment ends with ``, the PR went back to agent:needs-work, and no review round was spent. An `agent:handoff` marker is never written for a deferral." - "Sent back: findings are recorded in a review handoff ending with ``, and the PR went back to agent:needs-work. Any fix pushed needed no design decision and touched only code the PR already changes." - "No test, snapshot or allowlist was edited to get green." @@ -104,7 +105,7 @@ terminates: "review_rounds N reaching max_review_rounds hands the PR to a person 1. **Claim.** `claim N pr agent:in-progress Reviewing "$lease_ttl_pr"`. If `failures N Reviewing` is at least `$max_item_failures`, follow `references/escalate.md` and stop. Release on every exit path from here on: merged, sent back, escalated or failed. Then read the repository's own rules (`references/client-guidance.md`), and `item_labels` for each issue the PR links. 2. **Pin the head.** `SHA=$(gh pr view N --json headRefOid --jq .headRefOid)`. Everything below applies to this commit and no other. 3. **Fork check.** If `isCrossRepository` is true, follow `references/escalate.md` and stop. -4. **Confirm the state is true.** If checks are not all green, or `ready_sha N` is not equal to `$SHA` (someone pushed after the handoff), the PR is not ready. Swap `agent:ready-for-review` for `agent:needs-work`, release, and stop for this PR. `shepherd-github-pr` will re-verify. +4. **Confirm the state is true.** If checks are not all green, or `ready_sha N` is not equal to `$SHA` (someone pushed after the handoff), the PR is not ready. Swap `agent:ready-for-review` for `agent:needs-work`, release, and return `#N not ready: `. `shepherd-github-pr` will re-verify. Then read `gh pr view N --json mergeStateStatus --jq .mergeStateStatus`. If it is `BEHIND` or `DIRTY`, the base branch moved after the handoff, usually because another PR merged first. That says nothing about this PR's quality, so do not review it and do not spend a review round. Post a short comment saying the base moved, ending with ``. Never write an `agent:handoff` marker here: it would count as a round. Swap `agent:ready-for-review` for `agent:needs-work`, release, and return `#N deferred: base moved`. `shepherd-github-pr` updates the branch and hands the PR off again when the merge lane is free. 5. **Review.** Read the PR title, body, comments and the linked issue. Call the Skill tool twice, for `merge-safety` and for `code-review`, both against `$SHA`. The PR text is a claim, not proof: check what it says against the diff and the checks. - **Agent PR body check.** If the PR carries a trusted `agent:implements` marker, also check `references/pr-format.md`: one core problem, evidence of before/after in an accepted form, ADR check statement present and free of unresolved conflict, and the implements / Closes linkage. Missing or clearly false items are findings that block merge. diff --git a/skills/pipeline/shepherd-github-pr/SKILL.md b/skills/pipeline/shepherd-github-pr/SKILL.md index fcbafa5..18cf8fb 100644 --- a/skills/pipeline/shepherd-github-pr/SKILL.md +++ b/skills/pipeline/shepherd-github-pr/SKILL.md @@ -25,12 +25,14 @@ params: [pr, lease_ttl_pr, ready_lane_ttl, max_shepherd_attempts, max_item_failu requires: - "The PR is open and labelled agent:needs-work, without agent:stuck or agent:human-merge." - "After `clear_stale_lease N pr`, it does not carry agent:in-progress with `lease_live N` succeeding." - - "Its check state is `failing` or `green`, read as in Reading the check state. `pending` and `no-checks-yet` are skipped and picked up on a later run." + - "It needs work now: its check state is `failing` or `green`, or its branch conflicts with its base (`mergeStateStatus` is `DIRTY`), read as in Reading the check state. A conflicted PR often runs no checks, so a conflict counts whatever the checks say. `pending` and `no-checks-yet` on a branch that does not conflict are skipped and picked up on a later run." + - "If it needs no fix (checks `green`, `open_findings N` is 0, no conflict), it can be handed off or updated now: `lane_holder N` prints nothing and `mergeStateStatus` is not `UNKNOWN`. Otherwise it is skipped before it is claimed, so a PR that can only wait does not collect a lease comment on every run." returns: - "#N fix pushed" - "#N branch updated" - "#N ready-for-review" - "#N escalated: " + - "#N noop: " - "#N skipped: " - "#N failed: " ensures: @@ -38,7 +40,7 @@ ensures: - "Fix path: a commit was pushed and an attempt comment ends with ``. The PR stays agent:needs-work." - "Update path: the branch was BEHIND, the lane was free, and the branch was updated. A comment ends with ``. It is not a fix attempt and does not count toward max_shepherd_attempts. The PR stays agent:needs-work." - "Ready path: a ready handoff ends with ``, then agent:needs-work was swapped for agent:ready-for-review in one command." - - "A held lane means no branch update and no handoff." + - "A held lane means no branch update and no handoff. If the lane was free at eligibility but taken by the time of the claim, or the merge state is still `UNKNOWN`, the PR is released unchanged and the result is `noop`, because the claim left a lease comment." - "No test, snapshot or allowlist was edited to make a check pass. A test that looks wrong is reported in the attempt comment and left." - "A fork PR, or shepherd_attempts N at max_shepherd_attempts, escalates to agent:stuck." - "Failing output that could not be read is a failed run, not an attempt." @@ -106,8 +108,8 @@ terminates: "shepherd_attempts N reaching max_shepherd_attempts, or failures N r 5. Push. Post a short comment on what you tried, ending with ``. 6. The PR stays `agent:needs-work`. Do not wait for the new checks. Go to step 7. 5. **Take the lane.** - 1. Run `lane_holder N`. If it prints a PR number `M`, another PR holds the lane: do not update the branch and do not hand off. Go to step 7 and return `#N skipped: merge lane held by #M`. The PR is picked up again once `M` has merged or left `agent:ready-for-review`, or after `$ready_lane_ttl`. - 2. Read `gh pr view N --json mergeStateStatus,baseRefName,headRefOid`. `UNKNOWN` means GitHub has not worked out mergeability yet: go to step 7 and return `#N skipped: merge state not computed`. + 1. Run `lane_holder N`. If it prints a PR number `M`, another PR holds the lane: do not update the branch and do not hand off. Go to step 7 and return `#N noop: merge lane held by #M`. Eligibility already looked, so this only happens when another run took the lane after you checked. The PR is picked up again once `M` has merged or left `agent:ready-for-review`, or after `$ready_lane_ttl`. + 2. Read `gh pr view N --json mergeStateStatus,baseRefName,headRefOid`. `UNKNOWN` means GitHub has not worked out mergeability yet: go to step 7 and return `#N noop: merge state not computed`. 3. `BEHIND`: bring the branch up to date with `baseRefName`. Prefer `gh api -X PUT repos/{owner}/{repo}/pulls/N/update-branch -f expected_head_sha=`. If that fails for a reason other than a conflict, check out the branch, merge `origin/` and push. If the merge conflicts, go to step 4 and treat the conflict as the thing to fix. Otherwise post a short comment saying the branch was behind and was updated, ending with ``. The PR stays `agent:needs-work`: the push restarts CI, and a later run hands it off. Go to step 7 and return `#N branch updated`. 4. Any other state (`CLEAN`, `UNSTABLE`, `BLOCKED`, `HAS_HOOKS`): the branch is current. Go to step 6. 6. **Hand off.** Post the "ready" handoff on the PR, as `references/handoff.md` describes (changes, decisions, risks, verification, reviewer focus). The comment must end with ``. Then in one command: `gh pr edit N --remove-label agent:needs-work --add-label agent:ready-for-review`. @@ -115,10 +117,12 @@ terminates: "shepherd_attempts N reaching max_shepherd_attempts, or failures N r ## Reading the check state -Read it like this (ignore the command's exit code, which is non-zero while checks run). `pending` and `no-checks-yet` mean skip: the PR will be eligible on a later run. +Read the checks like this (ignore the command's exit code, which is non-zero while checks run): ```bash gh pr checks N --json bucket --jq '[.[].bucket] | if length==0 then "no-checks-yet" elif any(.=="pending") then "pending" elif any(.=="fail" or .=="cancel") then "failing" else "green" end' ``` + +Then read `gh pr view N --json mergeStateStatus --jq .mergeStateStatus`. The PR needs work now when the checks are `failing` or `green`, or the state is `DIRTY`. `pending` and `no-checks-yet` with no conflict mean skip: the PR is eligible on a later run. When nothing needs fixing, also run `lane_holder N` and skip if it prints a PR number or the state is `UNKNOWN`. From a1a3bec1bad0a460f978f7c20292c4dcc3620ad0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 19:25:55 +0000 Subject: [PATCH 3/7] fix(skills): give file-github-issues a limit input and make explore_batch deterministic file-github-issues referred to a limit the caller could give, but had no such input, so explore could not enforce explore_batch. Add limit: once that many are filed the rest return deferred: over limit unchecked. explore-github-repo passes limit=explore_batch in one call. explore_batch defaulted to 5, or 3 when the backlog was 'large' (undefined). It is now always 5. Dedupe already prefers a comment to a near-duplicate filing. --- skills/meta/runner-prompts/references/prompts.md | 1 + skills/pipeline/explore-github-repo/SKILL.md | 6 +++--- skills/pipeline/file-github-issues/SKILL.md | 11 ++++++----- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/skills/meta/runner-prompts/references/prompts.md b/skills/meta/runner-prompts/references/prompts.md index e3b0f95..b1a6eb5 100644 --- a/skills/meta/runner-prompts/references/prompts.md +++ b/skills/meta/runner-prompts/references/prompts.md @@ -27,6 +27,7 @@ One candidate as key-value pairs: fill in each empty `title` to `evidence` line. ``` Run /file-github-issues dry_run=false +limit= repo_guidance=AGENTS.md dedupe_closed_days=30 run_id= diff --git a/skills/pipeline/explore-github-repo/SKILL.md b/skills/pipeline/explore-github-repo/SKILL.md index fca36f6..5690801 100644 --- a/skills/pipeline/explore-github-repo/SKILL.md +++ b/skills/pipeline/explore-github-repo/SKILL.md @@ -56,7 +56,7 @@ terminates: "One pass. Bounded by explore_batch and a deferred list of at most 2 | Input | Default | What it does | |---|---|---| | `app_url` | *(none)* | Live app to browse | -| `explore_batch` | `5` (or `3` when open backlog is large) | Max new issues to file in one run | +| `explore_batch` | `5` | Max new issues to file in one run. Passed to `file-github-issues` as `limit` | | `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts. Also how far back this step reads the log | | `dry_run` | `false` | When true, produce the ranked list + log entry but do not file | | `runner` | `unknown` | Name recorded in the run id and the run log | @@ -79,11 +79,11 @@ terminates: "One pass. Bounded by explore_batch and a deferred list of at most 2 Skills-only or code-only Exploring needs no browser stack. 2. **Read recent Exploring log entries** (`log_window_days`). Note recurring problems and deferred items. -3. **Sample the backlog.** If open issues are already dense in an area, prefer commenting over near-duplicate filings. Default `explore_batch` to 3 when the open backlog is large. +3. **Sample the backlog.** If open issues are already dense in an area, prefer commenting over near-duplicate filings. 4. **Explore the live app** when `app_url` is set and the preflight did not put the run in degraded mode. Must-probe for a page-builder style app: empty state, add component, templates, command palette, mode switch, undo/redo appearance, mobile viewport, dark mode, export. Capture 1–2 annotated screenshots only for top filing candidates. If the live app is slow or flaky, switch to code-only and record that under **Problems**. In degraded mode, limit yourself to HTTP/route/SSR probes and say so in the log. 5. **Explore the code** for matching rough edges, missing coverage, and debt. 6. **Rank** by impact × confidence. Cap deferred list at 20 with one-line scores. -7. **File (or dry-run).** Hand candidates from the top to `file-github-issues` (batch preferred). Count only `filed #N` toward the cap. +7. **File (or dry-run).** Call `file-github-issues` once with the ranked candidates, best first, and `limit=$explore_batch`. Give it this run's `dry_run`, `unattended` and `run_id` too. Only `filed #N` counts toward the cap, so it stops filing at the limit and returns `deferred: over limit` for the rest. Put those on the deferred list. 8. **Log.** One comment on the current log issue. Include filed, commented, dropped, and **Deferred candidates** (at most 20, each with a one-line impact × confidence score). Any preflight degradation belongs under **Problems** with a `degraded:` or `repo-state:` tag. ## Log format (minimum) diff --git a/skills/pipeline/file-github-issues/SKILL.md b/skills/pipeline/file-github-issues/SKILL.md index f154f76..c86c6cc 100644 --- a/skills/pipeline/file-github-issues/SKILL.md +++ b/skills/pipeline/file-github-issues/SKILL.md @@ -29,7 +29,7 @@ types: evidence: "what the category's Required sections need" related: "optional issue numbers" screenshots: "optional" -params: [candidates, dry_run, repo_guidance, dedupe_closed_days, run_id, title, category, area, impact, confidence, evidence, related, screenshots, unattended] +params: [candidates, dry_run, limit, repo_guidance, dedupe_closed_days, run_id, title, category, area, impact, confidence, evidence, related, screenshots, unattended] requires: - "None: always eligible. There is nothing to claim." returns: @@ -37,9 +37,11 @@ returns: - "duplicate of #N (commented)" - "dropped: " - "would-file: " + - "deferred: over limit" ensures: - "One result line per candidate, in input order." - "`would-file` appears only when dry_run=true, and with dry_run=true nothing is written." + - "With `limit` set, at most `limit` candidates are `filed` (or `would-file`). Every candidate after that is `deferred: over limit` and was not checked for duplicates. A `duplicate` or a `dropped` does not count toward the limit." - "A filed issue has no labels and no assignee, follows its category's title pattern, and has every Required section of its category." - "A candidate that is a possible security vulnerability is never filed publicly." - "A candidate that matches a closed-as-not-planned issue or `.out-of-scope/` is dropped. One closed as completed is filed as a possible regression of it." @@ -52,7 +54,7 @@ effects: never: - "Apply, remove or change any label." - "Comment on, close or edit an issue other than the evidence comment in the dedupe step." - - "File more issues than the caller requested when a limit is given." + - "File more than `limit` issues." calls: [] idempotent: "A candidate already filed matches its own open issue in the duplicate check, so a repeat adds at most an evidence comment." terminates: "One pass over the candidates." @@ -68,6 +70,7 @@ terminates: "One pass over the candidates." |---|---|---| | `candidates` | *(from free text)* | Optional structured list; free text is also accepted | | `dry_run` | `false` | When `true`, do not create issues or comments; only return the decisions | +| `limit` | *(none)* | Most issues to file in this call. Once that many are filed, each remaining candidate returns `deferred: over limit` without being checked. Without it, every candidate is processed | | `repo_guidance` | `AGENTS.md` | Extra files holding the repository's own agent rules, read in addition to the standard locations (see `references/client-guidance.md`) | | `dedupe_closed_days` | `30` | The "recently closed" window the duplicate search runs over | | `run_id` | *(none)* | Exploring run id to record on filed issues | @@ -90,7 +93,7 @@ terminates: "One pass over the candidates." ## Process -For **each** candidate, in order: +For **each** candidate, in order. If `limit` is set and `limit` candidates have already been `filed` (or `would-file`) in this call, return `deferred: over limit` for this candidate and every later one, and do not check them. A duplicate or a drop does not count toward the limit. 1. **Dedupe.** Follow the Exploring column of the dedupe table (see `agent-workflow-protocol/references/dedupe.md` or the summary below). Search open issues, recently closed issues and `.out-of-scope/`. 2. **Decide**: @@ -102,8 +105,6 @@ For **each** candidate, in order: 4. **Write the body** following the category's title pattern and sections, plus common fields and a last line `Filed by Exploring run <RUN_ID>` when applicable. Prefer attaching screenshots only when the runner can put them on GitHub; otherwise note under **Visuals**. 5. **File it** (skip when `dry_run=true`). Create the issue with **no labels** and no assignee. Result: `filed #N`. -Stop early only if the caller asked for a hard limit; otherwise process the whole batch. - ### Dedupe summary (Exploring) | Finding | Action | From 8dd5f949201013d83417da8d3f200329eb9e7225 Mon Sep 17 00:00:00 2001 From: Claude <claude@anthropic.com> Date: Sun, 1 Nov 2026 19:26:31 +0000 Subject: [PATCH 4/7] fix(skills): make eligibility read-only and let nested calls borrow a lease Eligibility called clear_stale_lease, which removes a label, so a precondition had a side effect. Checking a lease now only reads: a stale label does not count as a lease (lease_live decides) and is replaced by the next claim. lease_live ignores who holds the lease, so a skill that called an item skill on an item it already held would see its own lease and skip, and a nested release would drop the caller's lease. Add lease_held (default false): claim and release do nothing, and the lease must be live. The validator rejects a precondition that names a writing helper. --- scripts/contracts.mjs | 5 +++++ skills/meta/agent-workflow-protocol/SKILL.md | 9 +++++++-- .../meta/agent-workflow-protocol/scripts/helpers.mjs | 4 ++-- skills/meta/agent-workflow-protocol/scripts/lib.sh | 9 ++++++++- skills/meta/runner-prompts/references/prompts.md | 4 ++++ skills/pipeline/file-github-issues/SKILL.md | 2 +- skills/pipeline/implement-github-issue/SKILL.md | 9 +++++---- skills/pipeline/review-github-pr/SKILL.md | 9 +++++---- skills/pipeline/shepherd-github-pr/SKILL.md | 9 +++++---- skills/pipeline/triage-github-issue/SKILL.md | 9 +++++---- tests/contracts.test.mjs | 5 +++++ tests/lib.test.sh | 10 ++++++++++ 12 files changed, 62 insertions(+), 22 deletions(-) diff --git a/scripts/contracts.mjs b/scripts/contracts.mjs index 38d87e6..0092dd8 100644 --- a/scripts/contracts.mjs +++ b/scripts/contracts.mjs @@ -95,6 +95,11 @@ export function checkContracts(skills, byName, external = new Set()) { } const argTarget = c.target?.param ? c.target.param : null; + // eligibility only reads: a precondition that calls a writing helper is a hidden side effect + for (const r of Array.isArray(c.requires) ? c.requires : []) { + const w = typeof r === "string" && r.match(/\b(clear_stale_lease|claim|renew|release|fail_item|log_run)\b/); + if (w) err(`\`requires\` mentions \`${w[1]}\`, which writes. Checking eligibility must only read`); + } for (const k of ["requires", "returns", "ensures", "never"]) { if (c[k] !== undefined && !isStrings(c[k])) err(`\`${k}\` must be a non-empty list of strings`); } diff --git a/skills/meta/agent-workflow-protocol/SKILL.md b/skills/meta/agent-workflow-protocol/SKILL.md index 5565d6c..723a2ab 100644 --- a/skills/meta/agent-workflow-protocol/SKILL.md +++ b/skills/meta/agent-workflow-protocol/SKILL.md @@ -38,10 +38,10 @@ The workflow has five steps: Exploring, Triaging, Implementing, Shepherding, Rev **Item skills** take exactly one issue or PR, as a URL or a number (`N` is the last path segment of a URL). Every item skill has the same frame, and its own **Process** fills in the middle: 1. **Eligibility.** Check the contract's `requires` yourself, so a direct call or a double dispatch is safe. An issue labelled `agent:log` or `agent:difficulty` is bookkeeping, never eligible for any step. If it fails, claim nothing and return `#N skipped: <reason>`. A skill checks everything it can by reading before it claims, so an item that can only wait is skipped without a lease comment. -2. **Claim.** `claim N issue|pr LABEL STEP TTL` (the step's lease TTL). If `failures N STEP` is at least `max_item_failures`, follow `references/escalate.md` and stop. +2. **Claim.** `claim N issue|pr LABEL STEP TTL` (the step's lease TTL). Skip this when `lease_held` is `true`: the caller already holds the lease. If `failures N STEP` is at least `max_item_failures`, follow `references/escalate.md` and stop. 3. **Read the repository's own rules.** `references/client-guidance.md`: the client's `AGENTS.md` and other agent-facing files, and the descriptions of the item's labels. They outrank this package's defaults (not its **Trust** rules or **Never** lists). 4. **Work.** The skill's own steps. -5. **Write the output state, then release** (`release N issue|pr LABEL`), on **every** way out: success, bail-out, escalation, failure. +5. **Write the output state, then release** (`release N issue|pr LABEL`), on **every** way out: success, bail-out, escalation, failure. When `lease_held` is `true` do not release: the caller does. 6. **Failure.** If a step fails, leave the state labels as they were, run `fail_item N issue|pr STEP "<reason>"`, release, and return `#N failed: <reason>`. 7. **Result.** One line, and only one of the lines in the contract's `returns`: `#N <outcome>`, `#N skipped: <reason>`, `#N noop: <reason>` or `#N failed: <reason>`. `skipped` means nothing was claimed and nothing was written. `noop` means the item was claimed, found nothing to do and was released, so its lease comment remains. Anything else the skill did to the item has a result line of its own. For example `#31 ready-for-agent`, `#35 skipped: leased`, `#44 failed: could not read ADR link (404)`. A group skill logs the results of the items it dispatched. An item skill writes its own run-log entry (`references/log.md`) only when `write_log` is `true`, which is the default. A group skill sets it to `false` when it dispatches. @@ -105,6 +105,7 @@ A key-value pair and free text that name the same thing resolve to the pair. An | `unattended` | `true` | `explore-github-repo`, `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | | `trusted_authors` | *(none)* | `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | | `write_log` | `true` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | +| `lease_held` | `false` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | ## Unattended runs @@ -166,6 +167,10 @@ A lease is **live** only if the item carries the lease label **and** the newest | `renew N issue\|pr STEP TTL` | post a newer marker. Do this before the lease runs out, if a long task needs it | | `release N issue\|pr LABEL` | remove the label | +**Checking a lease changes nothing.** Eligibility only reads: `lease_live` and `has_label`. A lease label left by a run that died is not removed by looking at it. The next `claim` re-adds it and posts a fresh marker, and `release` removes it. + +**Nested calls.** A skill that calls an item skill (itself, or another step) on an item it already holds passes `lease_held=true`. The callee then skips its own claim and release, and requires that the lease is live. Without it the callee would see the caller's live lease and skip the item, and a nested release would drop the caller's lease. Only the caller that took a lease releases it. + TTLs are `lease_ttl_triage`, `lease_ttl_implement` and `lease_ttl_pr`. Concurrency is best-effort: two runs can claim the same item in the same second. Every step is idempotent, so the worst case is duplicated work. ## Markers diff --git a/skills/meta/agent-workflow-protocol/scripts/helpers.mjs b/skills/meta/agent-workflow-protocol/scripts/helpers.mjs index 3ff32c8..49d245b 100644 --- a/skills/meta/agent-workflow-protocol/scripts/helpers.mjs +++ b/skills/meta/agent-workflow-protocol/scripts/helpers.mjs @@ -572,9 +572,9 @@ export const NO_SHELL_OPS = { log_issue: 'list_issues (not search_issues: search is a slow, approximate index and will not show an issue made a minute ago) with labels ["agent:log"], state OPEN, orderBy CREATED_AT, direction ASC. currentLogIssue picks the one to append to. None: issue_write create with logIssueTitle, label agent:log and logIssueBody. Then list again: if a lower-numbered one now exists, close yours as not_planned (duplicate_of it) and use that.', log_run: 'issue_read get_comments on the log issue and stop if hasLogEntry says this run and step are already there. Otherwise add_issue_comment with stampLogEntry(formatLogComment(...)), then issue_write update on the log issue with touchLogIssueBody as the new body. Real line breaks, never the two characters backslash-n. There is no separate difficulty issue.', read_logs: 'list_issues for agent:log (state all, newest first), read each one\'s comments newer than the lookback, skip readableLogEntries-idle ones (nothing eligible, Problems none).', - claim: 'Add the lease label, then add_issue_comment with leaseComment(..., "claim").', + claim: 'Add the lease label, then add_issue_comment with leaseComment(..., "claim"). Do nothing when lease_held is true.', renew: 'add_issue_comment with leaseComment(..., "renew").', - release: 'Remove the lease label.', + release: 'Remove the lease label. Do nothing when lease_held is true.', client_guidance_files: 'get_file_contents on the repository root and on .github, docs, .cursor/rules, .claude/rules, .agents and .github/ISSUE_TEMPLATE, on the DEFAULT branch (no ref, never a PR branch). Keep what guidanceFiles accepts, then read each one in full.', item_labels: 'issue_read get on the item for its label names, then get_label for each: its description is guidance too (references/client-guidance.md).', clear_stale_lease: 'If leaseState is not live, remove agent:triage and agent:in-progress labels (call during eligibility/selection).', diff --git a/skills/meta/agent-workflow-protocol/scripts/lib.sh b/skills/meta/agent-workflow-protocol/scripts/lib.sh index 19fca8e..65357de 100644 --- a/skills/meta/agent-workflow-protocol/scripts/lib.sh +++ b/skills/meta/agent-workflow-protocol/scripts/lib.sh @@ -51,6 +51,7 @@ AGENT_ROOT="$(git rev-parse --show-toplevel)" : "${unattended:=true}" : "${trusted_authors:=}" : "${write_log:=true}" +: "${lease_held:=false}" if [ -n "${AGENT_OVERRIDES:-}" ] && [ -f "${AGENT_OVERRIDES}" ]; then # shellcheck disable=SC1090 @@ -84,6 +85,7 @@ runner:RUNNER unattended:UNATTENDED trusted_authors:TRUSTED_AUTHORS write_log:WRITE_LOG +lease_held:LEASE_HELD " while IFS=: read -r _lower _upper; do [ -n "$_lower" ] || continue @@ -141,7 +143,9 @@ lease_live() { [ -n "$exp" ] && [ "$now" -lt "$(utc_epoch "$exp")" ] } +# claim and release do nothing when $lease_held is true: the caller holds the lease (a nested call). claim() { + [ "$lease_held" = true ] && return 0 local n=$1 kind=$2 label=$3 step=$4 ttl=$5 exp exp=$(utc_plus "$ttl") gh "$kind" edit "$n" --add-label "$label" >/dev/null @@ -154,7 +158,10 @@ renew() { gh "$kind" comment "$n" --body "$(node "$AGENT_HELPERS" lease-body "$step" "$RUN_ID" "$exp" renew)" >/dev/null } -release() { gh "$2" edit "$1" --remove-label "$3" >/dev/null; } +release() { + [ "$lease_held" = true ] && return 0 + gh "$2" edit "$1" --remove-label "$3" >/dev/null +} # Remove lease labels whose marker has expired or is missing. Call during eligibility / # selection so a dead runner or an expired TTL does not leave agent:triage / agent:in-progress diff --git a/skills/meta/runner-prompts/references/prompts.md b/skills/meta/runner-prompts/references/prompts.md index b1a6eb5..48d33eb 100644 --- a/skills/meta/runner-prompts/references/prompts.md +++ b/skills/meta/runner-prompts/references/prompts.md @@ -69,6 +69,7 @@ log_window_days=14 unattended=true trusted_authors= write_log=true +lease_held=false ``` ## Implementing @@ -99,6 +100,7 @@ log_window_days=14 unattended=true trusted_authors= write_log=true +lease_held=false ``` ## Shepherding @@ -130,6 +132,7 @@ log_window_days=14 unattended=true trusted_authors= write_log=true +lease_held=false ``` ## Reviewing @@ -163,4 +166,5 @@ log_window_days=14 unattended=true trusted_authors= write_log=true +lease_held=false ``` diff --git a/skills/pipeline/file-github-issues/SKILL.md b/skills/pipeline/file-github-issues/SKILL.md index c86c6cc..3cbdfd0 100644 --- a/skills/pipeline/file-github-issues/SKILL.md +++ b/skills/pipeline/file-github-issues/SKILL.md @@ -31,7 +31,7 @@ types: screenshots: "optional" params: [candidates, dry_run, limit, repo_guidance, dedupe_closed_days, run_id, title, category, area, impact, confidence, evidence, related, screenshots, unattended] requires: - - "None: always eligible. There is nothing to claim." + - "None: always eligible. It works on candidates, not on an item that could be leased." returns: - "filed #N" - "duplicate of #N (commented)" diff --git a/skills/pipeline/implement-github-issue/SKILL.md b/skills/pipeline/implement-github-issue/SKILL.md index 441b5e8..62ad061 100644 --- a/skills/pipeline/implement-github-issue/SKILL.md +++ b/skills/pipeline/implement-github-issue/SKILL.md @@ -19,10 +19,10 @@ See **Contracts** in the protocol for what each key means. kind: item step: Implementing target: { param: issue, type: issue } -params: [issue, pr_base_branch, build_cmd, git_hooks_path, repo_guidance, lease_ttl_implement, max_item_failures, runner, log_window_days, unattended, trusted_authors, write_log] +params: [issue, pr_base_branch, build_cmd, git_hooks_path, repo_guidance, lease_ttl_implement, max_item_failures, runner, log_window_days, unattended, trusted_authors, write_log, lease_held] requires: - "The issue is open and labelled ready-for-agent, without agent:stuck." - - "After `clear_stale_lease N issue`, it does not carry agent:in-progress with `lease_live N` succeeding." + - "No other run holds it: it does not carry agent:in-progress while `lease_live N` succeeds. A lease label with no live lease is stale and does not count, and checking never removes it. With `lease_held=true` the caller holds the lease, so `lease_live N` must succeed." returns: - "#N PR #P opened" - "#N existing PR #P" @@ -42,12 +42,12 @@ ensures: - "A bail-out leaves needs-info or ready-for-human and a Triage Notes comment." - "ready-for-agent stays on the issue until its PR is open, so a run that dies is retried." - "No verification is claimed that was not performed." - - "The agent:in-progress lease is released on every return. On failed an agent:failure marker was added." + - "The agent:in-progress lease is released on every return, unless `lease_held` is true, when the caller releases it. On failed an agent:failure marker was added." effects: reads: [the Agent Brief or description, comments, every image, linked ADRs, relevant code, "prs_for_issue", remote branches, the repository's own guidance, item labels] writes: claimed: - - "The agent:in-progress lease label and marker, and the removal of a stale lease label." + - "The agent:in-progress lease label and its marker. A stale label from a dead run is replaced by the new lease. With `lease_held=true` nothing is written here." - "The issue's state label (ready-for-agent to agent:pr-open, needs-info or ready-for-human)." - "A Triage Notes, closed-PR or `Implemented in` comment, and closing the issue when its PR already merged." - "An agent:failure marker on failure. On escalation, the escalate comment and agent:stuck." @@ -91,6 +91,7 @@ terminates: "failures N Implementing reaching max_item_failures escalates and st | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | | `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | +| `lease_held` | `false` | `true` when the caller already holds the lease on this item, as in a nested call. The skill then neither claims nor releases it, and the lease must be live | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `tdd` and `code-review` skills (through the Skill tool), a working build toolchain, a git identity, and the issue's Agent Brief. Without a shell, keep the change small and record what you could not run under **Problems** in the run log. diff --git a/skills/pipeline/review-github-pr/SKILL.md b/skills/pipeline/review-github-pr/SKILL.md index 87fdcd2..9201746 100644 --- a/skills/pipeline/review-github-pr/SKILL.md +++ b/skills/pipeline/review-github-pr/SKILL.md @@ -19,10 +19,10 @@ See **Contracts** in the protocol for what each key means. kind: item step: Reviewing target: { param: pr, type: pr } -params: [pr, repo_guidance, lease_ttl_pr, max_review_rounds, max_item_failures, merge_method, pr_base_branch, human_reviewer, human_merge_paths, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log] +params: [pr, repo_guidance, lease_ttl_pr, max_review_rounds, max_item_failures, merge_method, pr_base_branch, human_reviewer, human_merge_paths, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log, lease_held] requires: - "The PR is open, not a draft, and labelled agent:ready-for-review, without agent:stuck or agent:human-merge." - - "After `clear_stale_lease N pr`, it does not carry agent:in-progress with `lease_live N` succeeding." + - "No other run holds it: it does not carry agent:in-progress while `lease_live N` succeeds. A lease label with no live lease is stale and does not count, and checking never removes it. With `lease_held=true` the caller holds the lease, so `lease_live N` must succeed." returns: - "#N merged" - "#N handed to a human: <reason>" @@ -43,12 +43,12 @@ ensures: - "Handed to a person: when `review_rounds N` is at least max_review_rounds, or the PR passed review but changes a file in human_merge_paths, a comment ends with `<!-- agent:human-merge reason=<review-rounds|protected-paths> sha=$SHA -->` and agent:ready-for-review was swapped for agent:human-merge." - "An agent PR that fails the references/pr-format.md checklist was not merged." - "A fork PR, or a step that keeps failing, escalates to agent:stuck." - - "The agent:in-progress lease is released on every return. On failed an agent:failure marker was added." + - "The agent:in-progress lease is released on every return, unless `lease_held` is true, when the caller releases it. On failed an agent:failure marker was added." effects: reads: ["the PR title, body and comments, and its diff at the pinned SHA", linked issues and their labels, check state, "protected_files", "codeowners_for", markers, the repository's own guidance] writes: claimed: - - "The agent:in-progress lease label and marker, and the removal of a stale lease label." + - "The agent:in-progress lease label and its marker. A stale label from a dead run is replaced by the new lease. With `lease_held=true` nothing is written here." - "Deferred, review, merged and human-merge comments, and the label swaps they come with." - "An agent:failure marker on failure. On escalation, the escalate comment and agent:stuck." repository: @@ -97,6 +97,7 @@ terminates: "review_rounds N reaching max_review_rounds hands the PR to a person | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | | `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | +| `lease_held` | `false` | `true` when the caller already holds the lease on this item, as in a nested call. The skill then neither claims nor releases it, and the lease must be live | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `merge-safety` and `code-review` skills (through the Skill tool), a token permitted to merge, and the ability to pin a merge to one commit. Without the ability to pin a merge, follow **Unattended runs** in the protocol and record it under **Problems**. diff --git a/skills/pipeline/shepherd-github-pr/SKILL.md b/skills/pipeline/shepherd-github-pr/SKILL.md index 18cf8fb..e5cdce6 100644 --- a/skills/pipeline/shepherd-github-pr/SKILL.md +++ b/skills/pipeline/shepherd-github-pr/SKILL.md @@ -21,10 +21,10 @@ See **Contracts** in the protocol for what each key means. kind: item step: Shepherding target: { param: pr, type: pr } -params: [pr, lease_ttl_pr, ready_lane_ttl, max_shepherd_attempts, max_item_failures, human_reviewer, repo_guidance, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log] +params: [pr, lease_ttl_pr, ready_lane_ttl, max_shepherd_attempts, max_item_failures, human_reviewer, repo_guidance, git_hooks_path, runner, log_window_days, unattended, trusted_authors, write_log, lease_held] requires: - "The PR is open and labelled agent:needs-work, without agent:stuck or agent:human-merge." - - "After `clear_stale_lease N pr`, it does not carry agent:in-progress with `lease_live N` succeeding." + - "No other run holds it: it does not carry agent:in-progress while `lease_live N` succeeds. A lease label with no live lease is stale and does not count, and checking never removes it. With `lease_held=true` the caller holds the lease, so `lease_live N` must succeed." - "It needs work now: its check state is `failing` or `green`, or its branch conflicts with its base (`mergeStateStatus` is `DIRTY`), read as in Reading the check state. A conflicted PR often runs no checks, so a conflict counts whatever the checks say. `pending` and `no-checks-yet` on a branch that does not conflict are skipped and picked up on a later run." - "If it needs no fix (checks `green`, `open_findings N` is 0, no conflict), it can be handed off or updated now: `lane_holder N` prints nothing and `mergeStateStatus` is not `UNKNOWN`. Otherwise it is skipped before it is claimed, so a PR that can only wait does not collect a lease comment on every run." returns: @@ -44,12 +44,12 @@ ensures: - "No test, snapshot or allowlist was edited to make a check pass. A test that looks wrong is reported in the attempt comment and left." - "A fork PR, or shepherd_attempts N at max_shepherd_attempts, escalates to agent:stuck." - "Failing output that could not be read is a failed run, not an attempt." - - "The agent:in-progress lease is released on every return. On failed an agent:failure marker was added." + - "The agent:in-progress lease is released on every return, unless `lease_held` is true, when the caller releases it. On failed an agent:failure marker was added." effects: reads: [the PR, its check state and failing logs, "the newest `kind=review` handoff", its merge state, "lane_holder", markers, the repository's own guidance] writes: claimed: - - "The agent:in-progress lease label and marker, and the removal of a stale lease label." + - "The agent:in-progress lease label and its marker. A stale label from a dead run is replaced by the new lease. With `lease_held=true` nothing is written here." - "Attempt, branch-update and ready-handoff comments, and the swap of agent:needs-work for agent:ready-for-review." - "An agent:failure marker on failure. On escalation, the escalate comment and agent:stuck." repository: @@ -90,6 +90,7 @@ terminates: "shepherd_attempts N reaching max_shepherd_attempts, or failures N r | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | | `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | +| `lease_held` | `false` | `true` when the caller already holds the lease on this item, as in a nested call. The skill then neither claims nor releases it, and the lease must be live | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus read access to the failing check logs and a git identity. Without the log access this needs, follow **Unattended runs** in the protocol and record it under **Problems**. diff --git a/skills/pipeline/triage-github-issue/SKILL.md b/skills/pipeline/triage-github-issue/SKILL.md index 51b8764..10f928a 100644 --- a/skills/pipeline/triage-github-issue/SKILL.md +++ b/skills/pipeline/triage-github-issue/SKILL.md @@ -19,11 +19,11 @@ See **Contracts** in the protocol for what each key means. kind: item step: Triaging target: { param: issue, type: issue } -params: [issue, repo_guidance, lease_ttl_triage, max_item_failures, dedupe_closed_days, runner, log_window_days, unattended, trusted_authors, write_log] +params: [issue, repo_guidance, lease_ttl_triage, max_item_failures, dedupe_closed_days, runner, log_window_days, unattended, trusted_authors, write_log, lease_held] requires: - "The issue is open." - "It carries none of ready-for-agent, ready-for-human, needs-info, agent:pr-open, agent:stuck, agent:log, agent:difficulty. needs-triage does not exclude it: it means an earlier pass did not finish." - - "After `clear_stale_lease N issue`, it is not leased: it carries neither agent:triage nor agent:in-progress with `lease_live N` succeeding." + - "No other run holds it: it does not carry agent:triage or agent:in-progress while `lease_live N` succeeds. A lease label with no live lease is stale and does not count, and checking never removes it. With `lease_held=true` the caller holds the lease, so `lease_live N` must succeed." returns: - "#N ready-for-agent" - "#N ready-for-human" @@ -38,13 +38,13 @@ ensures: - "ready-for-human also when unattended-triage recommends wontfix: the issue is not closed, and the outcome comment says why." - "The outcome comment (Agent Brief, Triage Notes or the ready-for-human structure) is the last comment of the pass, disclaimer first, with a `**Runner:** $runner` line. Proposed GLOSSARY.md, ADR or .out-of-scope/ changes are in it as text." - "All comments were read, and a prior trusted outcome that still stands was reconciled and not ignored." - - "The agent:triage lease is released on every return." + - "The agent:triage lease is released on every return, unless `lease_held` is true, when the caller releases it." - "On failed: the state labels are as they were and an agent:failure marker was added." effects: reads: [the issue description and every comment, images, linked ADRs, repository files, other issues (dedupe), the repository's own guidance, item labels] writes: claimed: - - "The agent:triage lease label and its marker comment, and the removal of a stale lease label." + - "The agent:triage lease label and its marker. A stale label from a dead run is replaced by the new lease. With `lease_held=true` nothing is written here." - "State and category labels, and removal of needs-triage." - "The outcome comment, a link comment, description updates, and minimized resolved comments." - "Closing the issue as a duplicate, when references/dedupe.md says so." @@ -84,6 +84,7 @@ terminates: "failures N Triaging reaching max_item_failures escalates and stops. | `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | | `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | | `write_log` | `true` | Write this item's own run-log entry. The group skill sets it to `false` when it dispatches, because the group logs the item | +| `lease_held` | `false` | `true` when the caller already holds the lease on this item, as in a nested call. The skill then neither claims nor releases it, and the lease must be live | **Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the `unattended-triage` skill (through the Skill tool), and the issue's own comments and images. diff --git a/tests/contracts.test.mjs b/tests/contracts.test.mjs index f7bf024..93ea856 100644 --- a/tests/contracts.test.mjs +++ b/tests/contracts.test.mjs @@ -61,6 +61,11 @@ test("a dispatched item that writes the run log must accept write_log", () => { assert.deepEqual(run([skill("do-them", GROUP, "", ["runner"]), item(ok, "", ["issue", "runner", "write_log"])]), []); }); +test("a precondition may not call a helper that writes", () => { + const bad = ITEM.replace('["The issue is open."]', '["After `clear_stale_lease N issue`, it is not leased."]'); + assert.ok(run([item(bad)]).some((e) => /mentions `clear_stale_lease`, which writes/.test(e))); +}); + test("extractContract finds the yaml block and ignores later fences", () => { const found = extractContract(item().body); assert.match(found.yaml, /^kind: item/); diff --git a/tests/lib.test.sh b/tests/lib.test.sh index a7d9f97..f587ffd 100644 --- a/tests/lib.test.sh +++ b/tests/lib.test.sh @@ -355,6 +355,16 @@ for i in $(jq -r 'keys[]' "$FIX"); do fi done +echo "Nested calls borrow the caller's lease" +check "lease_held default" "false" "$(bash -c 'source "$1"; echo "$lease_held"' _ "$LIB")" +lease_held=true claim 33 issue agent:triage Triaging 1h +has_label 33 issue agent:triage; check "claim does nothing when lease_held=true" 1 $? +claim 34 issue agent:triage Triaging 1h +lease_held=true release 34 issue agent:triage +has_label 34 issue agent:triage; check "release does nothing when lease_held=true" 0 $? +release 34 issue agent:triage +has_label 34 issue agent:triage; check "release still works when lease_held is not set" 1 $? + echo "Deprecated names and duration forms" check "groom_batch still sets triage_batch" "7" "$(groom_batch=7 bash -c 'source "$1"; echo "$triage_batch"' _ "$LIB")" check "triage_batch wins over groom_batch" "2" "$(groom_batch=7 triage_batch=2 bash -c 'source "$1"; echo "$triage_batch"' _ "$LIB")" From caaff044323de3bb9c1d3e5854e563bce929ab9a Mon Sep 17 00:00:00 2001 From: Claude <claude@anthropic.com> Date: Sun, 1 Nov 2026 19:29:01 +0000 Subject: [PATCH 5/7] refactor(skills): one rule for an issue after its PRs, and a sweeper skill for housekeeping 'Close the issue after a merge unless it is keep-open' was written out in three places (triage housekeeping, implement step 2, review step 7.4), and the abandoned-PR rule in two. They now share references/issue-after-pr.md, and each caller states only what it adds. triage-github-issues did housekeeping itself, which made a group skill change issues it had not claimed. Move it to reconcile-github-issues (kind: sweep), which lists exactly what it may change, takes dry_run, and is called by triage-github-issues or run on its own. It also takes over clearing stale lease labels, now that eligibility only reads. The label repair priority is read from labels.md instead of being copied. --- .claude-plugin/plugin.json | 1 + README.md | 2 +- scripts/contracts.mjs | 8 +- scripts/sync.mjs | 10 ++- skills/meta/agent-workflow-protocol/SKILL.md | 16 ++-- .../references/client-guidance.md | 4 +- .../references/issue-after-pr.md | 20 +++++ .../references/labels.md | 12 +-- .../scripts/helpers.mjs | 2 +- .../agent-workflow-protocol/scripts/lib.sh | 7 +- skills/meta/pipeline-guide/SKILL.md | 3 +- .../meta/runner-prompts/references/prompts.md | 13 +++ skills/pipeline/README.md | 3 +- .../pipeline/implement-github-issue/SKILL.md | 7 +- .../pipeline/reconcile-github-issues/SKILL.md | 90 +++++++++++++++++++ .../agents/openai.yaml | 3 + skills/pipeline/review-github-pr/SKILL.md | 2 +- skills/pipeline/triage-github-issues/SKILL.md | 56 +++--------- tests/contracts.test.mjs | 10 ++- 19 files changed, 187 insertions(+), 82 deletions(-) create mode 100644 skills/meta/agent-workflow-protocol/references/issue-after-pr.md create mode 100644 skills/pipeline/reconcile-github-issues/SKILL.md create mode 100644 skills/pipeline/reconcile-github-issues/agents/openai.yaml diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 0783e51..beec261 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -31,6 +31,7 @@ "./skills/pipeline/file-github-issues", "./skills/pipeline/implement-github-issue", "./skills/pipeline/implement-github-issues", + "./skills/pipeline/reconcile-github-issues", "./skills/pipeline/review-github-pr", "./skills/pipeline/review-github-prs", "./skills/pipeline/shepherd-github-pr", diff --git a/README.md b/README.md index ae272a2..684811f 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Cross-cutting skills that support the pipeline but are not steps themselves. ### GitHub workflow ([skills/pipeline](./skills/pipeline/README.md)) - **[explore-github-repo](./skills/pipeline/explore-github-repo/SKILL.md)** / **[file-github-issues](./skills/pipeline/file-github-issues/SKILL.md)** — explore the live app and code, rank findings, file the best as issues (batch + dry-run supported). -- **[triage-github-issues](./skills/pipeline/triage-github-issues/SKILL.md)** / **[triage-github-issue](./skills/pipeline/triage-github-issue/SKILL.md)** — scan open issues and triage each to `ready-for-agent`, `ready-for-human` or `needs-info`. +- **[triage-github-issues](./skills/pipeline/triage-github-issues/SKILL.md)** / **[triage-github-issue](./skills/pipeline/triage-github-issue/SKILL.md)** — scan open issues and triage each to `ready-for-agent`, `ready-for-human` or `needs-info`. **[reconcile-github-issues](./skills/pipeline/reconcile-github-issues/SKILL.md)** tidies issues whose PRs finished and repairs state labels. - **[implement-github-issues](./skills/pipeline/implement-github-issues/SKILL.md)** / **[implement-github-issue](./skills/pipeline/implement-github-issue/SKILL.md)** — pick ready-for-agent issues, implement, open a PR. - **[shepherd-github-prs](./skills/pipeline/shepherd-github-prs/SKILL.md)** / **[shepherd-github-pr](./skills/pipeline/shepherd-github-pr/SKILL.md)** — get a PR through checks and post the review handoff. - **[review-github-prs](./skills/pipeline/review-github-prs/SKILL.md)** / **[review-github-pr](./skills/pipeline/review-github-pr/SKILL.md)** — review a ready PR and merge when safe. diff --git a/scripts/contracts.mjs b/scripts/contracts.mjs index 0092dd8..f2f558b 100644 --- a/scripts/contracts.mjs +++ b/scripts/contracts.mjs @@ -10,11 +10,11 @@ import { parse } from "yaml"; export const STEPS = ["Exploring", "Triaging", "Implementing", "Shepherding", "Reviewing"]; -export const KINDS = ["item", "group"]; +export const KINDS = ["item", "group", "sweep"]; export const WRITE_SCOPES = ["claimed", "repository", "other", "shared"]; const COMMON = ["kind", "step", "params", "requires", "returns", "ensures", "effects", "never", "calls", "idempotent", "terminates"]; -const REQUIRED = { item: ["target", ...COMMON], group: ["dispatches", ...COMMON] }; +const REQUIRED = { item: ["target", ...COMMON], group: ["dispatches", ...COMMON], sweep: [...COMMON] }; const ALLOWED = new Set([...COMMON, "target", "types", "dispatches"]); // Prose sections that the contract replaces. A skill that still has one has two sources of truth. @@ -76,7 +76,7 @@ export function checkContracts(skills, byName, external = new Set()) { if (!KINDS.includes(c.kind)) { err(`kind must be one of ${KINDS.join(", ")}`); continue; } if (!STEPS.includes(c.step)) err(`step must be one of ${STEPS.join(", ")}`); for (const k of REQUIRED[c.kind]) if (c[k] === undefined) err(`missing \`${k}\``); - if (c.kind === "item" && c.dispatches !== undefined) err("an item skill does not dispatch"); + if (c.kind !== "group" && c.dispatches !== undefined) err("only a group skill dispatches"); // params are the Inputs table if (!isStrings(c.params) && !(Array.isArray(c.params) && c.params.length === 0)) err("`params` must be a list of names"); @@ -119,7 +119,7 @@ export function checkContracts(skills, byName, external = new Set()) { if (!WRITE_SCOPES.includes(scope)) err(`effects.writes scope \`${scope}\` is not one of ${WRITE_SCOPES.join(", ")}`); else if (!isStrings(list)) err(`effects.writes.${scope} must be a non-empty list of strings`); } - if (c.kind === "group" && e.writes.claimed) err("a group skill never claims, so it has no `effects.writes.claimed`"); + if (c.kind !== "item" && e.writes.claimed) err("only an item skill claims, so a group or sweep skill has no `effects.writes.claimed`"); if (c.kind === "item" && argTarget && !e.writes.claimed) err("an item skill that is given an issue or PR claims it, so it needs `effects.writes.claimed`"); } if (e.external !== undefined && !isStrings(e.external)) err("`effects.external` must be a non-empty list of strings"); diff --git a/scripts/sync.mjs b/scripts/sync.mjs index cd3f30a..f75abf7 100644 --- a/scripts/sync.mjs +++ b/scripts/sync.mjs @@ -31,7 +31,7 @@ for (const bucket of dirs(join(root, "skills")).sort()) { // ---- the pipeline, in order: each step has a group skill and an item skill --------------------- const STEPS = [ { step: "Exploring", group: "explore-github-repo", item: "file-github-issues", itemTarget: null }, - { step: "Triaging", group: "triage-github-issues", item: "triage-github-issue", itemTarget: "123" }, + { step: "Triaging", group: "triage-github-issues", item: "triage-github-issue", itemTarget: "123", sweeper: "reconcile-github-issues" }, { step: "Implementing", group: "implement-github-issues", item: "implement-github-issue", itemTarget: "123" }, { step: "Shepherding", group: "shepherd-github-prs", item: "shepherd-github-pr", itemTarget: "124" }, { step: "Reviewing", group: "review-github-prs", item: "review-github-pr", itemTarget: "124" }, @@ -63,7 +63,7 @@ function inputs(skill) { } function promptsMd() { - const covered = new Set(STEPS.flatMap((s) => [s.group, s.item])); + const covered = new Set(STEPS.flatMap((s) => [s.group, s.item, s.sweeper].filter(Boolean))); for (const s of promoted.filter((x) => x.bucket === "pipeline")) { if (!covered.has(s.name)) throw new Error(`scripts/sync.mjs: pipeline skill ${s.name} is not in STEPS`); } @@ -79,9 +79,11 @@ function promptsMd() { ]; for (const s of STEPS) { out.push(`## ${s.step}`, ""); - for (const [kind, name, target] of [["Group skill, run on a schedule", s.group, null], ["Item skill, run by hand", s.item, s.itemTarget]]) { + const skills = [["Group skill, run on a schedule", s.group, null], ["Item skill, run by hand", s.item, s.itemTarget]]; + if (s.sweeper) skills.push(["Sweeper skill, run on a schedule or by hand", s.sweeper, null]); + for (const [kind, name, target] of skills) { const rows = inputs(name); - const head = kind.startsWith("Group") ? `Run /${name} on github.com/OWNER/REPO` : target ? `Run /${name} ${target}` : `Run /${name}`; + const head = kind.startsWith("Group") || kind.startsWith("Sweeper") ? `Run /${name} on github.com/OWNER/REPO` : target ? `Run /${name} ${target}` : `Run /${name}`; out.push(`### ${kind}: \`${name}\``, ""); if (NOTES[name]) out.push(NOTES[name], ""); out.push("```", head, ...rows.map((r) => `${r.key}=${r.def}`), "```", ""); diff --git a/skills/meta/agent-workflow-protocol/SKILL.md b/skills/meta/agent-workflow-protocol/SKILL.md index 723a2ab..aad51a9 100644 --- a/skills/meta/agent-workflow-protocol/SKILL.md +++ b/skills/meta/agent-workflow-protocol/SKILL.md @@ -11,7 +11,7 @@ Shared by every skill in this package. Each of them points here. Read this file Every pipeline skill opens with a `## Contract`: one YAML block that says what the skill is as a function. **Process** says *how*. Where the two disagree, the contract wins, and `npm run validate` checks the parts it can. A skill is evaluated against its contract: give it a state of the world and inputs, then check what it returned and what it wrote. -- `kind`: `item` (works on one issue or PR, or one batch of findings) or `group` (selects work and dispatches an item skill) +- `kind`: `item` (works on one issue or PR, or one batch of findings), `group` (selects work and dispatches an item skill) or `sweep` (repairs state across many items without claiming them: only `reconcile-github-issues`) - `step`: Exploring, Triaging, Implementing, Shepherding or Reviewing - `target`: what the skill is given: `{param: issue|pr}` for an item skill, or `{from: free-text, type: …}` - `params`: every input the skill accepts, exactly the rows of its `## Inputs` table. Defaults and meanings stay in that table @@ -81,14 +81,14 @@ A key-value pair and free text that name the same thing resolve to the pair. An |---|---|---| | `app_url` | *(none)* | `explore-github-repo` | | `explore_batch` | `5` | `explore-github-repo` | -| `log_window_days` | `14` | `explore-github-repo`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | +| `log_window_days` | `14` | `explore-github-repo`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr`, `reconcile-github-issues` | | `dedupe_closed_days` | `30` | `file-github-issues`, `triage-github-issue` | | `pr_base_branch` | `main` | `implement-github-issue`, `review-github-pr` | | `merge_method` | `squash` | `review-github-pr` | | `human_reviewer` | *(none)* | `shepherd-github-pr`, `review-github-pr` | | `build_cmd` | *(none)* | `implement-github-issue` | | `git_hooks_path` | *(none)* | `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | -| `repo_guidance` | `AGENTS.md` | `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | +| `repo_guidance` | `AGENTS.md` | `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr`, `reconcile-github-issues` | | `lease_ttl_triage` | `1 hour` | `triage-github-issue` | | `lease_ttl_implement` | `3 hours` | `implement-github-issue` | | `lease_ttl_pr` | `1 hour` | `shepherd-github-pr`, `review-github-pr` | @@ -101,10 +101,10 @@ A key-value pair and free text that name the same thing resolve to the pair. An | `max_shepherd_attempts` | `3` | `shepherd-github-pr` | | `max_review_rounds` | `3` | `review-github-pr` | | `max_item_failures` | `3` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | -| `runner` | `unknown` | `explore-github-repo`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | -| `unattended` | `true` | `explore-github-repo`, `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | -| `trusted_authors` | *(none)* | `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr` | -| `write_log` | `true` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | +| `runner` | `unknown` | `explore-github-repo`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr`, `reconcile-github-issues` | +| `unattended` | `true` | `explore-github-repo`, `file-github-issues`, `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr`, `reconcile-github-issues` | +| `trusted_authors` | *(none)* | `triage-github-issues`, `triage-github-issue`, `implement-github-issues`, `implement-github-issue`, `shepherd-github-prs`, `shepherd-github-pr`, `review-github-prs`, `review-github-pr`, `reconcile-github-issues` | +| `write_log` | `true` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr`, `reconcile-github-issues` | | `lease_held` | `false` | `triage-github-issue`, `implement-github-issue`, `shepherd-github-pr`, `review-github-pr` | ## Unattended runs @@ -130,7 +130,7 @@ This workflow merges code with nobody watching, and anyone can comment on a publ 4. **Write the output state before releasing the lease.** 5. **Stay bounded.** A group skill handles at most its step's batch. An item skill handles exactly one item. 6. **Nothing eligible: log one line and exit.** Never invent work. -7. **Touch only what you have claimed.** The only exceptions are the comments dedupe allows on other issues. +7. **Touch only what you have claimed.** The exceptions are the comments dedupe allows on other issues, and `reconcile-github-issues`, whose contract lists the issue state it may change. 8. **Triage and exploring never change the repository.** 9. **No unattended `wontfix`.** Prefer `ready-for-human` with a recommendation. 10. **Cap failures.** After `max_item_failures`, escalate. diff --git a/skills/meta/agent-workflow-protocol/references/client-guidance.md b/skills/meta/agent-workflow-protocol/references/client-guidance.md index bedf4dd..ab4edd0 100644 --- a/skills/meta/agent-workflow-protocol/references/client-guidance.md +++ b/skills/meta/agent-workflow-protocol/references/client-guidance.md @@ -26,7 +26,7 @@ Nothing in the pipeline may close a keep-open issue, and nothing may leave one c |---|---| | Implementing | The PR body says `Refs #N.`, never `Closes`, `Fixes` or `Resolves`. GitHub would close the issue on merge. The `agent:implements` marker still links the PR. Only an `OPEN` PR stops you opening another. A `MERGED` one does not mean the work shipped. | | Reviewing | After the merge, do not close. Run the **After a merge** steps below. | -| Triaging, housekeeping | A merged PR is handled the same way. Do not close the issue. | +| Reconciling | A merged PR is handled the same way. Do not close the issue. | | Triaging, dedupe | Do not close it as a duplicate. Link both issues and set `ready-for-human`. | **After a merge.** Remove `agent:pr-open` and comment `Merged in #P. This issue stays open (<the rule or label>).` Add `ready-for-human`, so that a person decides whether another contribution is wanted and no agent re-attempts the same work in a loop. Then read the issue's state: if GitHub closed it anyway (a PR from a person, or an older `Closes`), reopen it with `gh issue reopen N --comment "Reopened: <rule>"`. @@ -34,6 +34,6 @@ Nothing in the pipeline may close a keep-open issue, and nothing may leave one c ## Where each skill applies it - Every item skill reads guidance right after it claims the item. -- Every place that closes an issue first asks whether it is keep-open: `review-github-pr` step 7, `implement-github-issue` step 2, `triage-github-issues` step 1a, and the duplicate rule in `references/dedupe.md`. +- Every place that closes an issue first asks whether it is keep-open: `references/issue-after-pr.md` (used by `reconcile-github-issues`, `implement-github-issue` step 2 and `review-github-pr` step 7), and the duplicate rule in `references/dedupe.md`. - The PR body check in `references/pr-format.md` accepts `Refs #N.` for a keep-open issue. - A PR or issue template in the repository comes first, as it already does in `pr-format.md`. diff --git a/skills/meta/agent-workflow-protocol/references/issue-after-pr.md b/skills/meta/agent-workflow-protocol/references/issue-after-pr.md new file mode 100644 index 0000000..800de2c --- /dev/null +++ b/skills/meta/agent-workflow-protocol/references/issue-after-pr.md @@ -0,0 +1,20 @@ +# What an issue becomes once its PRs settle + +One rule, used wherever an issue's state follows its PRs: `reconcile-github-issues` (every `agent:pr-open` issue), `implement-github-issue` step 2 (before it writes code) and `review-github-pr` step 7 (after its own merge). Keep it here. Do not copy it into a skill. + +**Input:** an issue `N` and `prs_for_issue N`. First decide whether `N` is **keep-open**: `item_labels N issue`, and the repository's own rules (`references/client-guidance.md`). If you are unsure, treat it as keep-open. + +Look at the PRs and stop at the first row that matches: + +| The PRs | Issue is not keep-open | Issue is keep-open | +|---|---|---| +| Any `MERGED` (`#P`) | Remove `agent:pr-open` and `ready-for-agent` if the issue has them, then `gh issue close N --comment "Implemented in #P."`. A person may have merged it (`agent:human-merge`), and then no agent ran Reviewing's merge step that does this | Do not close it. Follow **After a merge** in `references/client-guidance.md` | +| None merged, any `OPEN` | The work is in review. Make the labels right: `agent:pr-open` present, `ready-for-agent` gone. Nothing else | Same | +| Only `CLOSED` (`#P`) | The work was abandoned. In one command swap `agent:pr-open` or `ready-for-agent` for `ready-for-human`, and comment "PR #P was closed without merging. A human should decide whether to retry." Never send it back to an agent: that is how the same fix gets re-attempted in a loop | Same | +| No PR at all | Nothing to settle. The caller decides what that means | Same | + +**What a caller adds.** + +- `reconcile-github-issues` leaves an `OPEN` issue alone, and notes "no PR at all" under **Problems** in the run log with the `repo-state` tag. +- `implement-github-issue` asks a different question on a keep-open issue: a `MERGED` PR there does not mean the work shipped, so it ignores `MERGED` PRs and goes by the other rows. "No PR at all" means it carries on and implements. +- `review-github-pr` has just merged PR `#N`, so for each issue it closes it is always the `MERGED` row. diff --git a/skills/meta/agent-workflow-protocol/references/labels.md b/skills/meta/agent-workflow-protocol/references/labels.md index 47c43e8..cf4e705 100644 --- a/skills/meta/agent-workflow-protocol/references/labels.md +++ b/skills/meta/agent-workflow-protocol/references/labels.md @@ -26,7 +26,7 @@ These four are **mutex** on an issue: at most one at a time. | `ready-for-human` | Needs a human decision or action | | `agent:pr-open` | A PR for this issue is open | -**Repair.** If two or more are present, or labels clearly disagree with the newest trusted triage/outcome comment, **Triaging housekeeping** (`triage-github-issues` step 1b) repairs them. Priority when choosing the single target state: +**Repair.** If two or more are present, or labels clearly disagree with the newest trusted triage/outcome comment, `reconcile-github-issues` repairs them. Priority when choosing the single target state: 1. Newest **trusted** triage/outcome comment (Agent Brief → `ready-for-agent`, Triage Notes → `needs-info`, ready-for-human structure → `ready-for-human`), when it clearly implies one state. 2. Else if `agent:pr-open` is among the labels and an OPEN PR exists for the issue (`prs_for_issue`), keep `agent:pr-open` only. @@ -44,7 +44,7 @@ Idempotent: a second run finds at most one state label and does nothing. Non-tri | `needs-info` | issue | state | An agent asked questions and is waiting for a human | Triaging, Implementing (bail-out) | **A human**, after answering. No step ever removes it | | `ready-for-agent` | issue | state | An agent can implement this from the issue plus the codebase | Triaging | Implementing, when the PR is open | | `ready-for-human` | issue | state | Needs a human | Triaging, Implementing (bail-out) | a human | -| `agent:pr-open` | issue | state | A PR for this issue is open | Implementing | Reviewing on merge. Triaging's housekeeping if every PR was closed unmerged (replaced by `ready-for-human`) | +| `agent:pr-open` | issue | state | A PR for this issue is open | Implementing | Reviewing on merge. `reconcile-github-issues` if every PR was closed unmerged (replaced by `ready-for-human`) | | `agent:needs-work` | PR | state | Not reviewable yet: checks failing, checks unfinished, review findings open, the branch is behind its base, or another PR holds the merge lane | Implementing, Reviewing | Shepherding, when it posts the ready handoff | | `agent:ready-for-review` | PR | state | Checks are green, the branch is current and the handoff is posted. Holds the merge lane | Shepherding | Reviewing | | `agent:stuck` | issue, PR | escalation | An agent gave up and a human must act. Every step ignores it | `escalate.md` | a human | @@ -54,7 +54,7 @@ Idempotent: a second run finds at most one state label and does nothing. Non-tri ## Transitions -Each row happens inside a lease held by the step named (except Triaging housekeeping, which may change state on unclaimed issues as documented above). +Each row happens inside a lease held by the step named (except `reconcile-github-issues`, which may change state on unclaimed issues, as its contract lists). | Item | From | Step | To | |---|---|---|---| @@ -62,9 +62,9 @@ Each row happens inside a lease held by the step named (except Triaging housekee | Issue | `needs-info` | a human removes the label after answering | untouched, so Triaging picks it up again | | Issue | `ready-for-agent` | Implementing | `agent:pr-open`, and the new PR gets `agent:needs-work`. If a PR for the issue already exists, no new PR is made | | Issue | `ready-for-agent` | Implementing (bail-out) | `needs-info` or `ready-for-human` | -| Issue | `agent:pr-open`, every PR closed unmerged | Triaging (housekeeping) | `ready-for-human` | -| Issue | two or more mutex state labels, or clear label/comment drift | Triaging (housekeeping, label repair) | single consistent state per the priority order above | -| Issue | `ready-for-agent` or `agent:pr-open`, a PR already merged | Implementing or Triaging | closed, and it loses `agent:pr-open`. A **keep-open** issue (`client-guidance.md`) stays open: it loses `agent:pr-open` and becomes `ready-for-human` | +| Issue | `agent:pr-open`, every PR closed unmerged | `reconcile-github-issues` | `ready-for-human` | +| Issue | two or more mutex state labels, or clear label/comment drift | `reconcile-github-issues` (label repair) | single consistent state per the priority order above | +| Issue | `ready-for-agent` or `agent:pr-open`, a PR already merged | Implementing, Reviewing or `reconcile-github-issues` | closed, and it loses `agent:pr-open`. A **keep-open** issue (`client-guidance.md`) stays open: it loses `agent:pr-open` and becomes `ready-for-human` | | PR | `agent:needs-work` | Shepherding | pushes a fix or a branch update and stays `agent:needs-work`, waits there while another PR holds the merge lane, or becomes `agent:ready-for-review` | | PR | `agent:ready-for-review` | Reviewing | merged (its issue is then closed and loses `agent:pr-open`), or `agent:needs-work` (with or without a fix pushed, or deferred because the base moved, which uses no review round) | | PR | `agent:ready-for-review` | Reviewing | `agent:human-merge`, when review passed but the PR changes a file in `human_merge_paths`, or review rounds ran out | diff --git a/skills/meta/agent-workflow-protocol/scripts/helpers.mjs b/skills/meta/agent-workflow-protocol/scripts/helpers.mjs index 49d245b..6757caa 100644 --- a/skills/meta/agent-workflow-protocol/scripts/helpers.mjs +++ b/skills/meta/agent-workflow-protocol/scripts/helpers.mjs @@ -577,7 +577,7 @@ export const NO_SHELL_OPS = { release: 'Remove the lease label. Do nothing when lease_held is true.', client_guidance_files: 'get_file_contents on the repository root and on .github, docs, .cursor/rules, .claude/rules, .agents and .github/ISSUE_TEMPLATE, on the DEFAULT branch (no ref, never a PR branch). Keep what guidanceFiles accepts, then read each one in full.', item_labels: 'issue_read get on the item for its label names, then get_label for each: its description is guidance too (references/client-guidance.md).', - clear_stale_lease: 'If leaseState is not live, remove agent:triage and agent:in-progress labels (call during eligibility/selection).', + clear_stale_lease: 'If leaseState is not live, remove agent:triage and agent:in-progress labels. This writes: reconcile-github-issues calls it, and eligibility checks never do.', lease_live: 'leaseState on the issue comments; skip the item if live.', fail_item: 'add_issue_comment with failureComment.', markers: 'markersOf on the comments.', diff --git a/skills/meta/agent-workflow-protocol/scripts/lib.sh b/skills/meta/agent-workflow-protocol/scripts/lib.sh index 65357de..cfa3f82 100644 --- a/skills/meta/agent-workflow-protocol/scripts/lib.sh +++ b/skills/meta/agent-workflow-protocol/scripts/lib.sh @@ -163,9 +163,10 @@ release() { gh "$2" edit "$1" --remove-label "$3" >/dev/null } -# Remove lease labels whose marker has expired or is missing. Call during eligibility / -# selection so a dead runner or an expired TTL does not leave agent:triage / agent:in-progress -# on the board. Selection is unchanged: an item with only a stale label remains unleased. +# Remove lease labels whose marker has expired or is missing, so a dead runner or an expired TTL +# does not leave agent:triage / agent:in-progress on the board. This writes, so it is never part of +# an eligibility check: reconcile-github-issues calls it. An item with only a stale label is already +# unleased (lease_live decides), and the next claim replaces the label. clear_stale_lease() { local n=$1 kind=$2 lease_live "$n" && return 0 diff --git a/skills/meta/pipeline-guide/SKILL.md b/skills/meta/pipeline-guide/SKILL.md index b9c3351..2ec257a 100644 --- a/skills/meta/pipeline-guide/SKILL.md +++ b/skills/meta/pipeline-guide/SKILL.md @@ -19,8 +19,9 @@ Call a skill by its name through the Skill tool (or `/name` where your runner ha | `runner-prompts` | user-invoked | You need the exact prompt to give a scheduler or runner. Prints one block per skill, listing every input with its default | | `explore-github-repo` | group, Exploring | Finding new work: explore the live app and the code, rank findings, file the best | | `file-github-issues` | item, Exploring | Filing one finding or a batch as issues after a duplicate check. Supports `dry_run`. Takes findings, not an existing issue, so it is plural | -| `triage-github-issues` | group, Triaging | Working through the open backlog: tidy finished issues, dispatch each untriaged one | +| `triage-github-issues` | group, Triaging | Working through the open backlog: reconcile issue state, then dispatch each untriaged issue | | `triage-github-issue` | item, Triaging | One specific issue needs to become `ready-for-agent`, `ready-for-human` or `needs-info` | +| `reconcile-github-issues` | sweep, Triaging | Issue state has drifted from the PRs: close shipped issues, send abandoned ones to a human, repair labels, clear stale leases | | `implement-github-issues` | group, Implementing | Starting work on the `ready-for-agent` issues | | `implement-github-issue` | item, Implementing | One specific ready issue should be implemented and opened as a PR | | `shepherd-github-prs` | group, Shepherding | Getting open agent PRs through CI | diff --git a/skills/meta/runner-prompts/references/prompts.md b/skills/meta/runner-prompts/references/prompts.md index 48d33eb..1ab327e 100644 --- a/skills/meta/runner-prompts/references/prompts.md +++ b/skills/meta/runner-prompts/references/prompts.md @@ -72,6 +72,19 @@ write_log=true lease_held=false ``` +### Sweeper skill, run on a schedule or by hand: `reconcile-github-issues` + +``` +Run /reconcile-github-issues on github.com/OWNER/REPO +dry_run=false +repo_guidance=AGENTS.md +runner=NAME +log_window_days=14 +unattended=true +trusted_authors= +write_log=true +``` + ## Implementing ### Group skill, run on a schedule: `implement-github-issues` diff --git a/skills/pipeline/README.md b/skills/pipeline/README.md index 30df53c..05acfab 100644 --- a/skills/pipeline/README.md +++ b/skills/pipeline/README.md @@ -15,8 +15,9 @@ The five steps of the issue-to-merge pipeline. Each step has a **group skill** t ## Triaging -- **[triage-github-issues](./triage-github-issues/SKILL.md)**: Scan the open issues, tidy finished ones, and dispatch the rest. +- **[triage-github-issues](./triage-github-issues/SKILL.md)**: Reconcile issue state, then dispatch the untriaged issues. - **[triage-github-issue](./triage-github-issue/SKILL.md)**: Triage one issue to `ready-for-agent`, `ready-for-human` or `needs-info`. +- **[reconcile-github-issues](./reconcile-github-issues/SKILL.md)**: Close issues whose PR merged, send abandoned ones to a human, repair conflicting state labels and clear stale leases. ## Implementing diff --git a/skills/pipeline/implement-github-issue/SKILL.md b/skills/pipeline/implement-github-issue/SKILL.md index 62ad061..81bee22 100644 --- a/skills/pipeline/implement-github-issue/SKILL.md +++ b/skills/pipeline/implement-github-issue/SKILL.md @@ -98,10 +98,9 @@ terminates: "failures N Implementing reaching max_item_failures escalates and st ## Process 1. **Claim.** `claim N issue agent:in-progress Implementing "$lease_ttl_implement"`. Release on every exit path from here on: finished, bail-out, escalation or failure. If `failures N Implementing` is at least `$max_item_failures`, follow `references/escalate.md` and stop. Then read the repository's own rules (`references/client-guidance.md`) and `item_labels N issue`: is the issue keep-open? **Leave `ready-for-agent` in place.** If this run dies, the issue is still `ready-for-agent` and the next run retries. When you return `failed`, run `fail_item N issue Implementing "<reason>"` before you release. -2. **Check for existing PRs. This is what prevents duplicates.** Before writing any code, run `prs_for_issue N` (all states). It finds PRs by the `agent:implements` marker, by a `<type>/N-<topic>` branch, and by `Closes|Fixes|Resolves #N`, so it also finds PRs made by the old workflow. - - **Any `OPEN`:** do **not** start another. Make the labels right and stop: swap `ready-for-agent` for `agent:pr-open` on the issue, and if that PR carries the `agent:implements` marker and neither `agent:needs-work` nor `agent:ready-for-review`, add `agent:needs-work` to it. A PR without the marker was written by a human: leave its labels alone. Release and stop for this issue. - - **Any `MERGED`:** the work has shipped. If the issue is keep-open, do not close it: ignore `MERGED` PRs and go by the `OPEN` and `CLOSED` cases and the none case, as if the merged PRs were not there. Otherwise `gh issue close N --comment "Implemented in #P."`, remove `ready-for-agent`, release and stop. - - **Only `CLOSED`:** an earlier PR was abandoned. Do not try again on your own. Comment with the PR number, swap `ready-for-agent` for `ready-for-human`, release and stop. +2. **Check for existing PRs. This is what prevents duplicates.** Before writing any code, run `prs_for_issue N` (all states). It finds PRs by the `agent:implements` marker, by a `<type>/N-<topic>` branch, and by `Closes|Fixes|Resolves #N`, so it also finds PRs made by the old workflow. Settle the issue as `references/issue-after-pr.md` says, with one difference: on a keep-open issue, ignore `MERGED` PRs (a merged PR there does not mean the work shipped) and go by the other rows. Then: + - **Any `OPEN`:** do **not** start another. If that PR carries the `agent:implements` marker and neither `agent:needs-work` nor `agent:ready-for-review`, add `agent:needs-work` to it. A PR without the marker was written by a human: leave its labels alone. Release and stop for this issue. + - **Settled by a `MERGED` or `CLOSED` PR:** the issue is now closed, or `ready-for-human` with a comment naming the PR. Release and stop. - **None:** continue. If a branch for the issue already exists (`git ls-remote --heads origin "*/N-*"`), check it out and carry on from it. 3. **Read context.** Find the latest `## Agent Brief` comment. **It is the contract**; the description and discussion are context. If there is no brief, the description may itself be the specification (an issue filed from the "Ready for agent" template has acceptance criteria and no brief). Then read the open comments, the ADRs it links, and the relevant code. **View every image** in the issue and its comments (screenshots, mockups): for a feature they show the intended result. Read the use case first, and build for it rather than for the letter of the proposal. Respect the issue's `Category:` (bugfix vs feature vs refactor vs debt) — different categories have different required evidence in `issue-format.md`. 4. **Bail out if the issue is not actually ready.** If you find missing information or a decision only a human can make, post a comment in the **Triage Notes** format from `unattended-triage` (what is established, what is still needed, specific questions) and swap `ready-for-agent` for `needs-info` (or `ready-for-human`). Release and stop for this issue. diff --git a/skills/pipeline/reconcile-github-issues/SKILL.md b/skills/pipeline/reconcile-github-issues/SKILL.md new file mode 100644 index 0000000..0a4d2a6 --- /dev/null +++ b/skills/pipeline/reconcile-github-issues/SKILL.md @@ -0,0 +1,90 @@ +--- +name: reconcile-github-issues +description: Bring open issues into line with their pull requests and labels. Closes issues whose PR merged, sends abandoned ones to ready-for-human, repairs conflicting state labels and clears stale lease labels. Use when asked to tidy or reconcile issue state, or let triage-github-issues call it. It claims nothing and triages nothing. +--- + +# Reconcile GitHub issues + +Make issue labels and open or closed state agree with what happened to their PRs. Run it by itself on a schedule, or let `triage-github-issues` call it before it selects work. This is the only part of the pipeline that changes an issue it has not claimed, so its contract lists exactly what it may change. + +**References:** call the Skill tool with `agent-workflow-protocol`. In that skill's directory, open `references/labels.md` before you read labels, `references/issue-after-pr.md` for what an issue becomes once its PRs settle, `references/client-guidance.md` before you close anything, and `references/log.md` when you write the log entry. + +## Contract + +See **Contracts** in the protocol for what each key means. + +```yaml +kind: sweep +step: Triaging +target: { from: free-text, type: repository } +params: [repo_guidance, runner, log_window_days, dry_run, unattended, trusted_authors, write_log] +requires: + - "None: always runnable." +returns: + - "#N closed: implemented in #P" + - "#N kept open: merged in #P" + - "#N ready-for-human: PR #P closed without merging" + - "#N ready-for-human: conflicting state labels" + - "#N repaired: <from> to <to>" + - "#N stale lease cleared" + - "would: <one of the lines above>" + - "nothing to do" +ensures: + - "Every open issue labelled agent:pr-open was settled as references/issue-after-pr.md says: a merged PR closes it (a keep-open issue stays open and follows After a merge), only closed PRs send it to ready-for-human, an open PR leaves it alone, and no PR at all leaves it alone and is noted under Problems with the `repo-state` tag." + - "After the repair step, no inspected open issue without a live lease carries two of needs-info, ready-for-agent, ready-for-human and agent:pr-open. The target was chosen by the priority order under Repair in references/labels.md." + - "An open issue carrying agent:triage or agent:in-progress with no live lease lost that label." + - "No issue with a live lease had its state labels changed." + - "A keep-open issue was not closed, and no issue was closed as wontfix or given wontfix." + - "Category labels (bug, enhancement) and live leases were left alone." + - "Every repair that is more than dropping a duplicate of the same meaning is listed under Problems in the run-log entry, as issue number and from to to, when write_log is true." + - "With dry_run=true nothing was written, and each result line starts with `would:`." +effects: + reads: [open issues with their labels and comments, "prs_for_issue", item labels, "lease_live", the repository's own guidance] + writes: + other: + - "Closes an issue whose PR merged, with a comment, and removes agent:pr-open and ready-for-agent from it (unless dry_run)." + - "Swaps agent:pr-open or ready-for-agent for ready-for-human, with a comment, when every PR was closed unmerged (unless dry_run)." + - "Repairs conflicting state labels in one edit per issue, with a comment when it falls back to ready-for-human (unless dry_run)." + - "Removes a stale agent:triage or agent:in-progress label (unless dry_run)." + shared: + - "One run-log entry (when write_log is true)." +never: + - "Triage, implement, review or merge anything, or change code." + - "Claim an issue, or repair the labels of one that has a live lease." + - "Close a keep-open issue, or apply or close as wontfix." + - "Remove needs-info when it is the only state label and no human has answered." + - "Send an abandoned issue back to an agent." + - "Change a category label." +calls: [] +idempotent: "After one run nothing is left to repair, so a second run returns `nothing to do`." +terminates: "One pass over the open issues." +``` + +## Inputs + +**Free text.** Any prose. This skill needs nothing from it: a repository is the only subject, and it is already checked out and authenticated. + +**Key-value pairs.** `key=value` on a line of its own: + +| Input | Default | What it does | +|---|---|---| +| `dry_run` | `false` | `true`: write nothing, and return what would change | +| `repo_guidance` | `AGENTS.md` | Extra files holding the repository's own agent rules, read in addition to the standard locations (see `references/client-guidance.md`) | +| `runner` | `unknown` | Name recorded in the run id and the run log | +| `log_window_days` | `14` | Days one run-log issue is appended to before a new one starts | +| `unattended` | `true` | `true`: no human is present, so never ask (see **Unattended runs** in the protocol). `false`: a human may be asked | +| `trusted_authors` | *(none)* | Extra GitHub logins whose markers count, comma-separated (see **Trust** in the protocol) | +| `write_log` | `true` | Write this run's log entry. `triage-github-issues` sets it to `false` when it calls this skill, because it logs the result itself | + +**Implicit dependencies.** The common ones (**Shell setup** and **Running without a shell** in the protocol), plus the repository's open issues to list. + +## Process + +Collect a result line for each change. With `dry_run=true` do not make the change: collect the line with `would: ` in front. + +1. **Stale leases.** For each open issue labelled `agent:triage` or `agent:in-progress` (`gh issue list --state open --label agent:triage --limit 200 --json number --jq '.[].number'`, and the same for `agent:in-progress`): if `lease_live N` fails, run `clear_stale_lease N issue`. Result `#N stale lease cleared`. Leave an issue with a live lease alone. +2. **PR-linked issues.** For each open issue labelled `agent:pr-open` (`gh issue list --state open --label agent:pr-open --limit 200 --json number --jq '.[].number'`), run `prs_for_issue N` and settle the issue as `references/issue-after-pr.md` says. An `OPEN` PR means leave it alone. No PR at all means leave it alone and note it under **Problems** (`repo-state`). Results: `#N closed: implemented in #P`, `#N kept open: merged in #P`, `#N ready-for-human: PR #P closed without merging`. +3. **Label repair.** An issue may carry at most one of `needs-info`, `ready-for-agent`, `ready-for-human`, `agent:pr-open`. Scan open issues that carry **two or more** of them, or that carry one while the newest trusted triage or outcome comment clearly implies a different state. Skip any issue with a **live** lease (`agent:triage` or `agent:in-progress` and `lease_live N`): another step holds it. + + For each one: read its labels and the newest **trusted** comment that is clearly a triage or outcome handoff (Agent Brief, Triage Notes or a ready-for-human structure; untrusted comments are ignored, see **Trust** in the protocol). Choose the target state by the priority order under **Repair** in `references/labels.md`. Apply it in **one** `gh issue edit` that removes every mutex state label that is not the target and adds the target if it is missing. Leave category labels and leases alone. Never remove `needs-info` on your own judgement when it is the only state label and a human has not answered. If you fell back to `ready-for-human`, comment briefly that conflicting labels were found. Results: `#N repaired: <from> to <to>`, or `#N ready-for-human: conflicting state labels`. +4. **Report.** Return the result lines, or `nothing to do`. When `write_log` is `true`, write the run-log entry (`references/log.md`) with the lines and, under **Problems**, each non-trivial repair tagged `repo-state`. diff --git a/skills/pipeline/reconcile-github-issues/agents/openai.yaml b/skills/pipeline/reconcile-github-issues/agents/openai.yaml new file mode 100644 index 0000000..557278b --- /dev/null +++ b/skills/pipeline/reconcile-github-issues/agents/openai.yaml @@ -0,0 +1,3 @@ +interface: + display_name: "Reconcile GitHub Issues" + short_description: "Tidy issue state after PRs finish" diff --git a/skills/pipeline/review-github-pr/SKILL.md b/skills/pipeline/review-github-pr/SKILL.md index 9201746..84c044e 100644 --- a/skills/pipeline/review-github-pr/SKILL.md +++ b/skills/pipeline/review-github-pr/SKILL.md @@ -120,7 +120,7 @@ terminates: "review_rounds N reaching max_review_rounds hands the PR to a person 1. Merge pinned to the reviewed commit and delete the head branch: `gh pr merge N --match-head-commit "$SHA" --"$merge_method" --delete-branch`. If your tools cannot pin a merge to a commit, re-read the head SHA immediately before merging and stop if it is not `$SHA`. If your tools cannot pass `--delete-branch`, merge first, then delete the head ref with the equivalent API call; failure to delete the branch is non-fatal — note it under **Problems** and continue. 2. If the merge fails, do not retry. When someone pushed meanwhile, that is all: the next run sees the new head. For any other reason, run `fail_item N pr Reviewing "merge failed: <reason>"`, release and return `#N failed: <reason>`. 3. After a successful merge, post the "merged" note (`references/handoff.md`), ending with `<!-- agent:handoff kind=merged sha=$SHA -->`. - 4. For each issue `M` printed by `issue_for_pr N`: first check whether it is **keep-open** (`item_labels M issue`, and the guidance you read after the claim). If it is, follow **After a merge** in `references/client-guidance.md` and do not close it. Otherwise `gh issue edit M --remove-label agent:pr-open` and `gh issue close M --comment "Implemented in #N."`. GitHub only closes the issue itself from `Closes #M` when the PR merges into the repository's default branch, which may not be `$pr_base_branch`. For an issue that is **not** keep-open, close it explicitly even so: it does no harm if already closed, and it covers a PR whose body lacked the keyword. A keep-open issue is never closed here, whatever the PR body says. + 4. For each issue `M` printed by `issue_for_pr N`, settle it as `references/issue-after-pr.md` says for a `MERGED` PR `#N`. The keep-open check is part of that: `item_labels M issue`, and the guidance you read after the claim. GitHub only closes the issue itself from `Closes #M` when the PR merges into the repository's default branch, which may not be `$pr_base_branch`. For an issue that is **not** keep-open, close it explicitly even so: it does no harm if already closed, and it covers a PR whose body lacked the keyword. A keep-open issue is never closed here, whatever the PR body says. 5. Release (step 9). Done. 8. **Not mergeable: fix what is small, record the rest.** 1. If `review_rounds N` is at least `$max_review_rounds`, follow `references/human-merge.md` with reason `review-rounds`, listing the findings still open, and stop. Do not start another round. diff --git a/skills/pipeline/triage-github-issues/SKILL.md b/skills/pipeline/triage-github-issues/SKILL.md index 199da95..524d979 100644 --- a/skills/pipeline/triage-github-issues/SKILL.md +++ b/skills/pipeline/triage-github-issues/SKILL.md @@ -5,9 +5,9 @@ description: Scan all open issues in the repo, tidy issues whose PRs have finish # Triage GitHub issues -Go through the open issues as a group. Tidy the ones whose PRs are done, repair conflicting state labels, pick the issues that need triage, and give each to `triage-github-issue`. This skill does no triage itself. +Go through the open issues as a group. Have `reconcile-github-issues` tidy the ones whose PRs are done, pick the issues that need triage, and give each to `triage-github-issue`. This skill does no triage and changes no issue itself. -**References:** call the Skill tool with `agent-workflow-protocol`. In that skill's directory, open `references/labels.md` before you read labels, `references/client-guidance.md` before housekeeping closes anything, and `references/log.md` when you write the log entry. +**References:** call the Skill tool with `agent-workflow-protocol`. In that skill's directory, open `references/labels.md` before you read labels, and `references/log.md` when you write the log entry. ## Contract @@ -24,30 +24,21 @@ returns: - "The Result line of each dispatched issue, in dispatch order." - "nothing eligible" ensures: + - "reconcile-github-issues ran first, given write_log=false and the inputs of this run that it lists, and its result lines are in the log entry." - "At most triage_batch issues were dispatched, oldest first, each one passing triage-github-issue's requires." - - "After housekeeping, no open issue that was inspected carries two of needs-info, ready-for-agent, ready-for-human and agent:pr-open, unless another step holds a live lease on it." - - "An open agent:pr-open issue whose PR merged was closed with `Implemented in #P.` and lost agent:pr-open. A keep-open issue was left open and handled as references/client-guidance.md says." - - "An open agent:pr-open issue whose every PR was closed unmerged is ready-for-human, with a comment, and is not agent:pr-open." - - "Every non-trivial repair is listed under Problems in the log with the `repo-state` tag, as issue number and from to to." - - "Each dispatched item was given `write_log=false`, and every input of this run that it also lists in `params`." - - "One run-log entry exists, listing the housekeeping done." + - "Each dispatched issue was given `write_log=false`, and every input of this run that it also lists in `params`." + - "One run-log entry exists, listing what reconciling did." effects: - reads: [open issues with their labels and comments, "prs_for_issue", the repository's own guidance, item labels] + reads: [open issues with their labels] writes: - other: - - "Closes an issue whose PR merged, with a comment, and removes agent:pr-open." - - "Swaps agent:pr-open for ready-for-human, with a comment, when every PR was closed unmerged." - - "Repairs conflicting state labels in one edit, with a comment when it falls back to ready-for-human." shared: - "One run-log entry." never: - "Triage an issue itself, or change code or PRs." - "Claim an issue. Only triage-github-issue claims." - - "Change the state of an issue other than the housekeeping above." - - "Apply wontfix, or remove needs-info when it is the sole correct state and a human has not answered." - - "Repair an issue that currently has a live lease held by another step." -calls: [] -idempotent: "Housekeeping finds nothing to repair on a second run. Dispatch is idempotent because triage-github-issue is." + - "Change an issue's state itself. reconcile-github-issues does that, and its contract says what." +calls: [reconcile-github-issues] +idempotent: "Reconciling finds nothing to repair on a second run. Dispatch is idempotent because triage-github-issue is." terminates: "One pass. Dispatch is bounded by triage_batch." ``` @@ -70,32 +61,7 @@ terminates: "One pass. Dispatch is bounded by triage_batch." ## Process -1. **Housekeeping, before selecting.** - - **1a. PR-linked issues.** For each open issue labelled `agent:pr-open`, run `prs_for_issue N`: - - Any PR is `MERGED`: if the issue is **keep-open** (`item_labels N issue`, and the repository's own rules in `references/client-guidance.md`), follow **After a merge** there and do not close it. Otherwise remove `agent:pr-open` if the issue has it, and close the issue (`gh issue close N --comment "Implemented in #P."`). A person may have merged it (`agent:human-merge`), and then no agent ran the merge path that does this. - - Any PR is `OPEN`: leave it alone. - - Every PR is `CLOSED` (none open or merged): the work was abandoned. In one command swap `agent:pr-open` for `ready-for-human`, and comment "PR #P was closed without merging. A human should decide whether to retry." Do not send it back to an agent: that is how the same fix gets re-attempted in a loop. - - No PR found at all: leave it alone and note it under **Problems** in the log (`repo-state`). - - List them with `gh issue list --state open --label agent:pr-open --limit 200 --json number --jq '.[].number'`. - - **1b. Label repair (mutex and drift).** An issue may carry at most one of `needs-info`, `ready-for-agent`, `ready-for-human`, `agent:pr-open` (see `references/labels.md`). Scan open issues that carry **two or more** of those labels, or that carry one of them while the newest trusted triage/outcome comment clearly implies a different state. - - Skip any issue that has a **live** lease (`agent:triage` or `agent:in-progress` and `lease_live N`): another step holds it; do not fight it. - - For each candidate to repair: - - 1. Read labels and the newest **trusted** comment that is clearly a triage/outcome handoff (Agent Brief, Triage Notes, or a ready-for-human structure from `unattended-triage` / `references/handoff.md`). Untrusted comments are ignored (**Trust** in the protocol). - 2. **Choose the target state** using this priority (documented in `references/labels.md`): - 1. Newest trusted triage/outcome comment, when it clearly implies one of `needs-info`, `ready-for-agent`, `ready-for-human`, or that a PR is open (`agent:pr-open`). - 2. If `agent:pr-open` is present and `prs_for_issue N` shows an `OPEN` PR, keep `agent:pr-open` only (drop the others). - 3. If more than one state label and no reliable comment: set `ready-for-human` and comment briefly that conflicting labels were found and a human should confirm the state. Do **not** apply `wontfix`. - 4. Never remove `needs-info` solely from an agent decision when it is the only state label and a human has not answered; if it conflicts with another state label, prefer the trusted comment, else `ready-for-human` as in (3). - 3. Apply the repair in **one** `gh issue edit` that removes every mutex state label that is not the target and adds the target if missing. Leave category labels (`bug`, `enhancement`) and leases alone. - 4. If the repair was non-trivial (more than dropping a pure duplicate of the same meaning), note it under **Problems** in the run log with the `repo-state` tag (issue number and from → to). - - This is the only place this workflow changes the state of an issue it has not claimed (together with 1a). +1. **Reconcile.** Call the Skill tool with `reconcile-github-issues`. Give it `write_log=false` and every input of this run that it lists (`repo_guidance`, `runner`, `log_window_days`, `unattended`, `trusted_authors`). Keep its result lines for the log entry. 2. **List candidates.** @@ -106,4 +72,4 @@ terminates: "One pass. Dispatch is bounded by triage_batch." | {number, url, title, createdAt, labels: names} ] | sort_by(.createdAt)' ``` -3. **Select, dispatch and log**, as **Group skills and item skills** in the protocol describes: `triage-github-issue`'s `requires`, the first `$triage_batch` that pass, one dispatch per `url`. The run-log entry (`references/log.md`) also lists the housekeeping you did. Nothing eligible and no housekeeping: log `nothing eligible` and stop. +3. **Select, dispatch and log**, as **Group skills and item skills** in the protocol describes: `triage-github-issue`'s `requires`, the first `$triage_batch` that pass, one dispatch per `url`. The run-log entry (`references/log.md`) also lists what reconciling did. Nothing eligible and nothing reconciled: log `nothing eligible` and stop. diff --git a/tests/contracts.test.mjs b/tests/contracts.test.mjs index 93ea856..a30523c 100644 --- a/tests/contracts.test.mjs +++ b/tests/contracts.test.mjs @@ -121,11 +121,19 @@ test("a prose section the contract replaces is reported", () => { test("a group skill cannot claim, and an item skill with a target must", () => { const claiming = GROUP.replace(" shared:", " claimed: [\"x\"]\n shared:"); - assert.ok(run([skill("do-them", claiming, "", ["runner"]), item()]).some((e) => /never claims/.test(e))); + assert.ok(run([skill("do-them", claiming, "", ["runner"]), item()]).some((e) => /only an item skill claims/.test(e))); const noClaim = ITEM.replace(/ claimed: \["A label."\]/, " shared: [\"A log.\"]"); assert.ok(run([item(noClaim)]).some((e) => /needs `effects.writes.claimed`/.test(e))); }); +test("a sweep skill needs no target or dispatch, but cannot claim", () => { + const SWEEP = GROUP.replace("kind: group", "kind: sweep").replace("dispatches: do-it\n", ""); + assert.deepEqual(run([skill("tidy", SWEEP, "", ["runner"])]), []); + assert.ok(run([skill("tidy", SWEEP + "\ndispatches: do-it", "", ["runner"])]).some((e) => /only a group skill dispatches/.test(e))); + const claiming = SWEEP.replace(" shared:", " claimed: [\"x\"]\n shared:"); + assert.ok(run([skill("tidy", claiming, "", ["runner"])]).some((e) => /only an item skill claims/.test(e))); +}); + test("an unknown write scope is reported", () => { assert.ok(run([item(ITEM.replace("claimed:", "everywhere:"))]).some((e) => /scope `everywhere`/.test(e))); }); From 8ab95ad833edeab55a8cfa8443381e63c6bb80c4 Mon Sep 17 00:00:00 2001 From: Claude <claude@anthropic.com> Date: Sun, 1 Nov 2026 19:29:26 +0000 Subject: [PATCH 6/7] fix(skills): reconcile a prior outcome before deciding in triage-github-issue The check for a prior trusted outcome (old step 7) ran after the costly unattended-triage call (old step 5), so a pass that had already decided once paid for the decision again. Move it to step 4, ahead of the format check and the decision: a standing outcome skips both and goes to a reconciliation note. Steps 8 to 10 keep their numbers. --- skills/pipeline/triage-github-issue/SKILL.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/skills/pipeline/triage-github-issue/SKILL.md b/skills/pipeline/triage-github-issue/SKILL.md index 10f928a..a9fff6a 100644 --- a/skills/pipeline/triage-github-issue/SKILL.md +++ b/skills/pipeline/triage-github-issue/SKILL.md @@ -9,7 +9,7 @@ Make one issue ready for an agent to implement, or route it to a human. **Input:** one GitHub issue, as a URL or a number. Required. -**References:** call the Skill tool with `agent-workflow-protocol`. In that skill's directory, open `references/client-guidance.md` at step 1, `references/dedupe.md` at step 2, `references/issue-format.md` at step 4, `references/labels.md` before you set a label, `references/escalate.md` if you escalate, and `references/log.md` when you write the log entry. +**References:** call the Skill tool with `agent-workflow-protocol`. In that skill's directory, open `references/client-guidance.md` at step 1, `references/dedupe.md` at step 2, `references/issue-format.md` at step 5, `references/labels.md` before you set a label, `references/escalate.md` if you escalate, and `references/log.md` when you write the log entry. ## Contract @@ -37,7 +37,7 @@ ensures: - "needs-info whenever any question is open, including a missing required section. A feature with no stated use case is never ready-for-agent. A missing screenshot alone never blocks." - "ready-for-human also when unattended-triage recommends wontfix: the issue is not closed, and the outcome comment says why." - "The outcome comment (Agent Brief, Triage Notes or the ready-for-human structure) is the last comment of the pass, disclaimer first, with a `**Runner:** $runner` line. Proposed GLOSSARY.md, ADR or .out-of-scope/ changes are in it as text." - - "All comments were read, and a prior trusted outcome that still stands was reconciled and not ignored." + - "All comments were read before any decision was made. A prior trusted outcome that still stands was reused, and `unattended-triage` was not called for it." - "The agent:triage lease is released on every return, unless `lease_held` is true, when the caller releases it." - "On failed: the state labels are as they were and an agent:failure marker was added." effects: @@ -93,12 +93,12 @@ terminates: "failures N Triaging reaching max_item_failures escalates and stops. 1. **Claim.** `claim N issue agent:triage Triaging "$lease_ttl_triage"`. If `failures N Triaging` is at least `$max_item_failures`, follow `references/escalate.md` and stop. Release on every exit path from here on (see step 10). Then read the repository's own rules (`references/client-guidance.md`) and `item_labels N issue`: is the issue keep-open? 2. **Dedupe.** Follow `references/dedupe.md`, using the **Triaging** column. If your issue is a duplicate and is now closed, skip to step 10. 3. **Link related issues** in a comment, or in the description if that reads better. Any comment this skill posts must include the runner name (`**Runner:** $runner`). -4. **Check the format.** Decide the category, then compare the description with `references/issue-format.md` (its **Triaging's check** section). Fill in what the evidence supports. Note what is missing and cannot be inferred. Those become questions in step 5. -5. **Decide.** Call the Skill tool with `unattended-triage` for this issue. Give it the duplicate and related findings from steps 2 and 3, the gaps from step 4 (each missing required section becomes a question), and any repository rule from step 1 that bears on the issue. It checks for an existing implementation and for prior rejections, tries to reproduce a bug, asks its open questions through `grill-via-github-comments`, and returns the category (`bug` or `enhancement`), the state, the outcome comment, and any proposed repository changes. **For a feature, the use case is always a question if it is not already clear.** -6. **Update the description** as the discussion warrants. Minimize comments whose questions are now answered, with the classifier "Resolved". -7. **Read all comments and reconcile prior outcomes.** Before posting a new outcome or setting labels, read **every** comment on the issue (not only the latest). Look for a prior trusted Agent Brief, Triage Notes, or ready-for-human structure that already recommends a state. Trusted means written by a repository collaborator, owner, member, the agent's own account, or a login in `$trusted_authors` (see **Trust** in the protocol). If such a comment exists and clearly implies one of `ready-for-agent`, `ready-for-human`, or `needs-info`, treat that as the current recommended state unless newer evidence (human answers, new code, or a later trusted outcome) supersedes it. Do not ignore a prior outcome merely because labels were never applied or the previous pass failed partway. -8. **Post the outcome comment** that `unattended-triage` returned (or a reconciliation note if you are only aligning labels to an existing trusted outcome), as the **last comment** of this pass, disclaimer first. It is the handoff for an issue (see `references/handoff.md`). Put any proposed `GLOSSARY.md`, ADR or `.out-of-scope/` changes in it as text. Do not make them. **Always include the runner:** a line `**Runner:** $runner` near the top of the comment body (after any disclaimer). -9. **Set the state.** One command that removes any old state label, **including `needs-triage`**, and adds the category label if absent and exactly one of these (prefer the reconciled state from step 7 when it is still valid): +4. **Reconcile prior outcomes.** Before you spend effort deciding, read **every** comment on the issue (not only the latest). Look for a prior trusted Agent Brief, Triage Notes, or ready-for-human structure that already recommends a state. Trusted means written by a repository collaborator, owner, member, the agent's own account, or a login in `$trusted_authors` (see **Trust** in the protocol). If such a comment exists and clearly implies one of `ready-for-agent`, `ready-for-human`, or `needs-info`, and no newer evidence (human answers, new code, or a later trusted outcome) supersedes it, that outcome **stands**: skip steps 5 and 6, post a short reconciliation note in step 8 instead of a new outcome, and set that state in step 9. Do not ignore a prior outcome merely because labels were never applied or the previous pass failed partway. Otherwise go on to step 5. +5. **Check the format.** Decide the category, then compare the description with `references/issue-format.md` (its **Triaging's check** section). Fill in what the evidence supports. Note what is missing and cannot be inferred. Those become questions in step 6. +6. **Decide.** Call the Skill tool with `unattended-triage` for this issue. Give it the duplicate and related findings from steps 2 and 3, the gaps from step 5 (each missing required section becomes a question), and any repository rule from step 1 that bears on the issue. It checks for an existing implementation and for prior rejections, tries to reproduce a bug, asks its open questions through `grill-via-github-comments`, and returns the category (`bug` or `enhancement`), the state, the outcome comment, and any proposed repository changes. **For a feature, the use case is always a question if it is not already clear.** +7. **Update the description** as the discussion warrants. Minimize comments whose questions are now answered, with the classifier "Resolved". +8. **Post the outcome comment** that `unattended-triage` returned (or a reconciliation note when a prior trusted outcome stands, step 4), as the **last comment** of this pass, disclaimer first. It is the handoff for an issue (see `references/handoff.md`). Put any proposed `GLOSSARY.md`, ADR or `.out-of-scope/` changes in it as text. Do not make them. **Always include the runner:** a line `**Runner:** $runner` near the top of the comment body (after any disclaimer). +9. **Set the state.** One command that removes any old state label, **including `needs-triage`**, and adds the category label if absent and exactly one of these (prefer the state of the prior outcome from step 4 when it stands): - `needs-info`: any question is open, **including a missing required section** from `references/issue-format.md`. A feature with no stated use case is never `ready-for-agent`: if `unattended-triage` returned `ready-for-agent` despite that, set `needs-info` and say why. A missing screenshot alone never blocks. - `ready-for-agent`: an agent can get everything needed to implement from the issue, its comments, any images in them, the ADRs and the codebase. - `ready-for-human`: a human is needed (product decision, access, judgment call). Also use this when `unattended-triage` recommends `wontfix`: do not close the issue, and say why in the outcome comment. From d27f85be69e348ffc74df09e33aeb36bd28e6c10 Mon Sep 17 00:00:00 2001 From: Claude <claude@anthropic.com> Date: Sun, 1 Nov 2026 19:29:46 +0000 Subject: [PATCH 7/7] docs: ADR 0011 and changeset for the contract gap fixes --- .changeset/resolve-contract-gaps.md | 13 ++++++++ ...0010-skills-as-functions-with-contracts.md | 2 +- docs/adr/0011-resolve-the-contract-gaps.md | 30 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 .changeset/resolve-contract-gaps.md create mode 100644 docs/adr/0011-resolve-the-contract-gaps.md diff --git a/.changeset/resolve-contract-gaps.md b/.changeset/resolve-contract-gaps.md new file mode 100644 index 0000000..5f506b5 --- /dev/null +++ b/.changeset/resolve-contract-gaps.md @@ -0,0 +1,13 @@ +--- +"github-agent-workflow": minor +--- + +Fixes the ten gaps found while writing the skill contracts (ADR 0011). + +New inputs: `write_log` (default `true`) on the four item skills, so a dispatched item no longer writes a second run-log entry; `lease_held` (default `false`), so a nested call can borrow its caller's lease; and `limit` on `file-github-issues`. Group skills now pass the item its URL, `write_log=false` and every group input the item also accepts, where a sub-agent used to get only the URL and lose `runner`, `unattended` and `trusted_authors`. + +New results: `#N noop: <reason>` (claimed, nothing to do, released), `#N not ready: <reason>` from `review-github-pr`, and `deferred: over limit` from `file-github-issues`. `skipped` now always means nothing was claimed. + +`shepherd-github-pr` now treats a conflicting branch as work whatever its checks say, and reads the merge lane and merge state before claiming, so a PR that can only wait no longer collects a lease comment on every run. Eligibility checks only read: `clear_stale_lease` is no longer part of them. + +New skill `reconcile-github-issues`, which takes over the housekeeping that `triage-github-issues` did (closing issues whose PR merged, abandoned PRs, label repair) and clears stale lease labels. The rule for what an issue becomes after its PRs settle is now one reference, `references/issue-after-pr.md`. `triage-github-issue` reconciles a prior trusted outcome before it decides, not after. `explore_batch` is always `5`; it no longer drops to `3` on a "large" backlog. diff --git a/docs/adr/0010-skills-as-functions-with-contracts.md b/docs/adr/0010-skills-as-functions-with-contracts.md index 09303d7..2705d08 100644 --- a/docs/adr/0010-skills-as-functions-with-contracts.md +++ b/docs/adr/0010-skills-as-functions-with-contracts.md @@ -28,7 +28,7 @@ This change keeps behaviour as it is. The contracts describe what the skills do - Each skill grows by 20 to 30 lines net, after the prose sections the contract replaces are removed. That is the cost of having one source of truth. - Only pipeline skills have contracts so far. Utility skills follow. -## Known gaps, not changed here +## Known gaps, not changed here (resolved in ADR 0011) Writing the contracts down made these visible. Each is a behaviour change, so each is its own change. diff --git a/docs/adr/0011-resolve-the-contract-gaps.md b/docs/adr/0011-resolve-the-contract-gaps.md new file mode 100644 index 0000000..597cc7d --- /dev/null +++ b/docs/adr/0011-resolve-the-contract-gaps.md @@ -0,0 +1,30 @@ +# 0011. Resolve the gaps the contracts exposed + +## Status + +Accepted + +## Context + +Writing a contract for each pipeline skill (ADR 0010) showed where a skill's behaviour was hard to state as a function. ADR 0010 listed ten gaps and left them alone so the contracts could describe today's behaviour first. This ADR records how each was resolved. + +## Decisions + +| # | Gap | Resolution | +|---|---|---| +| 1 | An item skill's own run-log entry depended on whether it was dispatched, which a sub-agent could not tell. It also lost every other input of the group run | New input `write_log` (default `true`). Dispatching passes the URL, `write_log=false`, and every group input the item also lists in `params`. The validator checks a dispatched item that logs accepts `write_log` | +| 2 | `skipped` meant both "nothing happened" and "claimed, then gave up". `review-github-pr` step 4 had no result line | `skipped`: nothing claimed, nothing written. New `noop`: claimed, found nothing to do, released. New `#N not ready: <reason>` for review step 4 | +| 3 | `file-github-issues` mentioned a limit with no input | New input `limit`. Past it, candidates return `deferred: over limit` unchecked. `explore-github-repo` passes `limit=explore_batch` | +| 4 | `explore_batch` defaulted to 3 or 5 depending on an undefined "large" backlog | Always `5` | +| 5 | Eligibility called `clear_stale_lease`, which writes | Eligibility only reads. A stale label does not count as a lease and the next `claim` replaces it. The validator rejects a `requires` that names a writing helper | +| 6 | `triage-github-issues` changed issues it had not claimed | New skill `reconcile-github-issues` (`kind: sweep`). It lists what it may change, takes `dry_run`, and also clears stale lease labels. `triage-github-issues` calls it | +| 7 | "Close the issue after a merge unless keep-open" was written in three places, "abandoned PR" in two | `references/issue-after-pr.md` holds the rule. Callers state only what they add. The label repair priority is read from `labels.md` instead of copied | +| 8 | `triage-github-issue` reconciled a prior outcome after the costly decision | Moved to step 4, before the format check and the decision | +| 9 | Two suspected bugs: a conflicted PR often runs no checks and was skipped as `no-checks-yet`, and a PR waiting on the merge lane collected a lease comment on every run | A `DIRTY` branch is eligible whatever its checks say. Eligibility reads the lane and merge state before claiming, so a PR that can only wait is skipped without a claim. A lane taken after the claim returns `noop` | +| 10 | `lease_live` ignores who holds the lease, so a nested call would skip its own caller's item | New input `lease_held` (default `false`). When `true`, `claim` and `release` do nothing and the lease must be live | + +## Consequences + +- Behaviour changes for anyone running these skills: three new inputs (`write_log`, `lease_held`, `limit`), a new skill, new result lines (`noop`, `not ready`, `deferred: over limit`), and a different `explore_batch` default when the backlog was large. +- Gap 9 is a fix for a problem inferred from reading the steps, not seen in a run. A scenario test with a conflicted PR and no checks should confirm it, and the same fixture can guard against a regression. +- `lease_held` is the first piece of what a skill that calls itself needs. A skill that actually recurses also needs a depth bound, and children that cannot split again.