From c3fb7878fdd57692bd9ec357fcba3d1b01fa1b0b Mon Sep 17 00:00:00 2001 From: glennfaison Date: Tue, 10 Nov 2026 09:38:04 +0000 Subject: [PATCH] fix(protocol): prs_for_issue and issue_for_pr work on a real gh (#89) They asked gh pr list and gh pr view for authorAssociation, which is not a PR field in any gh release, so both failed with 'Unknown JSON field'. They now read the REST pull request, which has author_association and user.login, and apply the same trust rule through trusted_jq. issue_for_pr reads GitHub's own issue links with gh 2.72 or newer and still reads the description without it. Co-authored-by: Claude --- .changeset/pr-author-association.md | 5 +++ README.md | 2 +- skills/meta/agent-workflow-protocol/SKILL.md | 2 +- .../agent-workflow-protocol/scripts/lib.sh | 36 +++++++++++-------- tests/lib.test.sh | 29 +++++++++++++-- 5 files changed, 55 insertions(+), 19 deletions(-) create mode 100644 .changeset/pr-author-association.md diff --git a/.changeset/pr-author-association.md b/.changeset/pr-author-association.md new file mode 100644 index 0000000..f247299 --- /dev/null +++ b/.changeset/pr-author-association.md @@ -0,0 +1,5 @@ +--- +"github-agent-workflow": patch +--- + +`prs_for_issue` and `issue_for_pr` work on a real `gh`. They asked `gh pr list` and `gh pr view` for `authorAssociation`, which `gh` rejects as a PR field (`Unknown JSON field`), so neither could find the PR that belongs to an issue. Both now read the REST pull request, which has the author's association, and apply the same trust rule as before. `issue_for_pr` reads GitHub's own issue links with `gh` 2.72 or newer, and without that it still reads the description. It now prints the issue numbers in numeric order. diff --git a/README.md b/README.md index c3d5a82..1f32fc6 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ Building blocks the pipeline calls. Each is useful on its own and does no GitHub An agent that supports skills (Claude Code, Kilo Code, Codex, …), or one that can read a `SKILL.md` and follow it. -- `gh` (authenticated, repository write access), `jq`, and Node.js for the shell helpers. Runners with no shell work too: the protocol tells the agent to do the same operations with the tools it has and to log what it cannot do. +- `gh` (authenticated, repository write access), `jq`, and Node.js for the shell helpers. `gh` 2.72 or newer also reads the issues GitHub itself links to a PR (its Development sidebar). An older `gh` works and reads only the PR description. Runners with no shell work too: the protocol tells the agent to do the same operations with the tools it has and to log what it cannot do. - Two skills from [mattpocock/skills](https://github.com/mattpocock/skills): `tdd` and `code-review` (a repository's own code-review skill satisfies the latter). The plugin declares this dependency for Claude Code; everywhere else, install it yourself. See [external-skills.json](./external-skills.json). ## Install diff --git a/skills/meta/agent-workflow-protocol/SKILL.md b/skills/meta/agent-workflow-protocol/SKILL.md index 201daa6..cce01aa 100644 --- a/skills/meta/agent-workflow-protocol/SKILL.md +++ b/skills/meta/agent-workflow-protocol/SKILL.md @@ -200,7 +200,7 @@ Hidden HTML comments make state readable without parsing prose. Append them as t |---|---| | `markers N` | every trusted agent marker on N, in order | | `prs_for_issue N` | every PR for issue N, one ` ` per line. Matches a trusted `agent:implements` marker, a `/N-` branch, or `Closes\|Fixes\|Resolves #N` in the body | -| `issue_for_pr N` | the issue numbers PR N implements | +| `issue_for_pr N` | the issue numbers PR N implements, in numeric order. Reads a trusted `agent:implements` marker and `Closes\|Fixes\|Resolves #N` in the description, and, with `gh` 2.72 or newer, the issues GitHub links to the PR | | `review_rounds N`, `shepherd_attempts N` | the two counters | | `fail_item N issue\|pr STEP REASON` | comment the failure on N with the `agent:failure` marker | | `failures N STEP` | failed runs of STEP on N since the last escalation | diff --git a/skills/meta/agent-workflow-protocol/scripts/lib.sh b/skills/meta/agent-workflow-protocol/scripts/lib.sh index c8d51f8..82ef8c4 100644 --- a/skills/meta/agent-workflow-protocol/scripts/lib.sh +++ b/skills/meta/agent-workflow-protocol/scripts/lib.sh @@ -125,11 +125,6 @@ trusted_jq() { printf '((.author_association | IN("OWNER","MEMBER","COLLABORATOR")) or (.user.login | IN(%s)))' "$(_trusted_logins)" } -# Same predicate over one `gh ... --json author,authorAssociation` object. -_trusted_json_jq() { - printf '((.authorAssociation | IN("OWNER","MEMBER","COLLABORATOR")) or (.author.login | IN(%s)))' "$(_trusted_logins)" -} - # ---- Leases ---------------------------------------------------------------------- has_label() { gh "$2" view "$1" --json labels --jq '.labels[].name' | grep -qx -- "$3"; } @@ -299,22 +294,33 @@ ready_sha() { # Implementing must not push a branch that fails this pattern. prs_for_issue() { - gh pr list --state all --limit 200 --json number,state,title,body,headRefName,author,authorAssociation \ - | jq -r --arg n "$1" " - .[] | $(_trusted_json_jq) as \$ok + local prs page + # The REST pull request has `author_association` and `user.login`, which `trusted_jq` reads. `gh pr list --json` has no + # author association, and asking it for one is an error. The 200 newest PRs, as before. + prs=$(for page in 1 2; do gh api "repos/{owner}/{repo}/pulls?state=all&per_page=100&page=$page" || exit 1; done) || return 1 + printf '%s\n' "$prs" | jq -r --arg n "$1" " + .[] | $(trusted_jq) as \$ok | select( ((\$ok) and ((.body // \"\") | contains(\"\"))) - or (.headRefName | test(\"^[a-z]+/\" + \$n + \"-\")) + or ((.head.ref // \"\") | test(\"^[a-z]+/\" + \$n + \"-\")) or ((.body // \"\") | test(\"(?i)(closes|fixes|resolves)\\\\s+#\" + \$n + \"\\\\b\")) or ((.title // \"\") | test(\"(?i)(closes|fixes|resolves|implements)\\\\s+(?:[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+)?#\" + \$n + \"\\\\b\")) ) - | \"\\(.number) \\(.state)\"" + | \"\\(.number) \\(if .merged_at then \"MERGED\" elif .state == \"open\" then \"OPEN\" else \"CLOSED\" end)\"" } issue_for_pr() { - gh pr view "$1" --json body,closingIssuesReferences,author,authorAssociation --jq " - $(_trusted_json_jq) as \$ok - | ([ (.body // \"\") | scan(\"agent:implements issue=([0-9]+)\") | .[0] | select(\$ok) ] - + [ (.body // \"\") | scan(\"(?i)(?:closes|fixes|resolves)\\\\s+#([0-9]+)\") | .[0] ] - + [ .closingIssuesReferences[].number | tostring ]) | unique | .[]" + local n=$1 pr refs + pr=$(gh api "repos/{owner}/{repo}/pulls/$n") || return 1 + # GitHub's own links (closing keywords, the Development sidebar). `gh` has this field from 2.72. With an older `gh` it is + # skipped, and the body is still read for `Closes #N` and for a trusted marker. + if ! refs=$(gh pr view "$n" --json closingIssuesReferences --jq '.closingIssuesReferences[].number' 2>&1); then + case $refs in *"Unknown JSON field"*) refs="" ;; *) printf '%s\n' "$refs" >&2; return 1 ;; esac + fi + { printf '%s\n' "$pr" | jq -r " + $(trusted_jq) as \$ok + | ([ (.body // \"\") | scan(\"agent:implements issue=([0-9]+)\") | .[0] | select(\$ok) ] + + [ (.body // \"\") | scan(\"(?i)(?:closes|fixes|resolves)\\\\s+#([0-9]+)\") | .[0] ]) | .[]" + printf '%s\n' "$refs" + } | sed '/^$/d' | sort -un } # ---- Client guidance ----------------------------------------------------------------- diff --git a/tests/lib.test.sh b/tests/lib.test.sh index 378175b..1dc8ec0 100644 --- a/tests/lib.test.sh +++ b/tests/lib.test.sh @@ -21,7 +21,11 @@ echo '[]' > "$STATE/prs.json"; echo '[]' > "$STATE/issues.json" cat > "$WORK/bin/gh" <<'STUB' #!/usr/bin/env bash cmd=$1; shift +# gh has no author association on a pull request: `--json authorAssociation` is an error for pr list and pr view. +if [ "$cmd" = pr ] && [[ "$*" == *authorAssociation* ]]; then echo 'Unknown JSON field: "authorAssociation"' >&2; exit 1; fi if [ "$cmd" = pr ] && [ "${1:-}" = list ]; then cat "$STATE/prs.json"; exit 0; fi +# prs.json holds [{number,state:OPEN|MERGED|CLOSED,title,body,headRefName,author:{login},authorAssociation,closing:[numbers]}]. A REST pull request has other names. +restpr='{number,title,body,state:(if .state=="OPEN" then "open" else "closed" end),merged_at:(if .state=="MERGED" then "2026-01-01T00:00:00Z" else null end),head:{ref:.headRefName},user:{login:.author.login},author_association:.authorAssociation}' jqf() { local f=""; while [ $# -gt 0 ]; do [ "$1" = --jq ] && f=$2; shift; done; jq -r "${f:-.}"; } # Issues live in $STATE/issues.json: [{number,title,state,labels:[names],createdAt,updatedAt,body}]. # Unlike a stub that returns a fixed list, this one makes a lookup that finds nothing actually find @@ -67,6 +71,11 @@ case "$cmd" in */contents/*) f="$STATE/contents/${path#*/contents/}"; f="${f%%\?*}"; [ -f "$f" ] && { cat "$f"; exit 0; }; exit 1 ;; */pulls/*/files) n=$(echo "$path" | sed -n 's#.*/pulls/\([0-9]*\)/files#\1#p') [ -f "$STATE/files_$n.json" ] || exit 1; jqf "$@" < "$STATE/files_$n.json"; exit 0 ;; + */pulls\?*) page=${path##*page=} # 100 to a page, as the REST API pages + jq -c --argjson p "$page" ".[($page-1)*100:$page*100] | map($restpr)" "$STATE/prs.json" | jqf "$@"; exit 0 ;; + */pulls/[0-9]*) n=${path##*/pulls/}; out=$(jq -c --argjson n "$n" ".[]|select(.number==\$n)|$restpr" "$STATE/prs.json") + [ -n "$out" ] || { echo "HTTP 404: Not Found" >&2; exit 1; } + printf '%s\n' "$out" | jqf "$@"; exit 0 ;; esac n=$(echo "$1" | sed -n 's#.*/issues/\([0-9]*\)/comments#\1#p') { cat "$STATE/comments_$n.json" 2>/dev/null || echo '[]'; } | jqf "$@" ;; @@ -81,7 +90,8 @@ case "$cmd" in elif [ "$cmd" = pr ] && [[ "$*" == *baseRefName* ]]; then echo '{"baseRefName":"main"}' | jqf "$@" elif [ "$cmd" = pr ] && [[ " $* " == *closingIssuesReferences* ]]; then - jq --argjson n "$n" '.[]|select(.number==$n)|{body,authorAssociation,author,closingIssuesReferences:(.closing//[])|map({number:.})}' "$STATE/prs.json" | jqf "$@" + [ -n "${STUB_OLD_GH:-}" ] && { echo 'Unknown JSON field: "closingIssuesReferences"' >&2; exit 1; } # gh before 2.72 + jq --argjson n "$n" '.[]|select(.number==$n)|{closingIssuesReferences:(.closing//[])|map({number:.})}' "$STATE/prs.json" | jqf "$@" else jq -Rn '{labels:[inputs|select(length>0)|{name:.}]}' "$lf" | jqf "$@"; fi ;; close) while [ $# -gt 0 ]; do case $1 in --reason) set_issue "$n" closeReason "$2"; shift ;; esac; shift; done set_issue "$n" state closed ;; @@ -356,7 +366,10 @@ cat > "$STATE/prs.json" <<'J' {"number":52,"state":"CLOSED","body":"nothing","headRefName":"feature/9-bar","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, {"number":53,"state":"OPEN","body":"Closes #70.","headRefName":"feature/x","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, {"number":54,"state":"OPEN","title":"Fixes some/repo#80: thing","body":"nothing","headRefName":"x","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, - {"number":55,"state":"OPEN","title":"Implements #81","body":"nothing","headRefName":"x","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}] + {"number":55,"state":"OPEN","title":"Implements #81","body":"nothing","headRefName":"x","closing":[],"authorAssociation":"OWNER","author":{"login":"maintainer"}}, + {"number":56,"state":"OPEN","body":"","headRefName":"x","closing":[],"authorAssociation":"MEMBER","author":{"login":"dev"}}, + {"number":57,"state":"OPEN","body":"nothing","headRefName":"y","closing":[14],"authorAssociation":"COLLABORATOR","author":{"login":"dev"}}, + {"number":58,"state":"MERGED","body":"Closes #15\n","headRefName":"z","closing":[15,3],"authorAssociation":"OWNER","author":{"login":"maintainer"}}] J check "link by marker+branch" "50 OPEN" "$(prs_for_issue 7)" check "link by Closes text" "51 MERGED" "$(prs_for_issue 8)" @@ -366,6 +379,18 @@ check "link by title (owner/repo#N)" "54 OPEN" "$(prs_for_issue 80)" check "link by title (Implements #N)" "55 OPEN" "$(prs_for_issue 81)" check "issue_for_pr (marker)" 7 "$(issue_for_pr 50)" check "issue_for_pr (Closes)" 8 "$(issue_for_pr 51)" +check "a MEMBER's marker is a link" "56 OPEN" "$(prs_for_issue 13)" +check "issue_for_pr reads GitHub's own links" 14 "$(issue_for_pr 57)" +check "issue_for_pr joins the body and GitHub's links, once each, in numeric order" "3 15 16" "$(issue_for_pr 58 | tr '\n' ' ' | sed 's/ $//')" +check "an old gh without closingIssuesReferences still reads the body" "8" "$(STUB_OLD_GH=1 issue_for_pr 51)" +issue_for_pr 999 >/dev/null 2>&1; check "issue_for_pr fails for a PR that is not there" 1 $? +gh pr list --json number,authorAssociation >/dev/null 2>&1; check "gh has no authorAssociation on pr list (the stub refuses it as gh does)" 1 $? +gh pr view 50 --json body,authorAssociation >/dev/null 2>&1; check "gh has no authorAssociation on pr view either" 1 $? +jq -n '[range(1;231) | {number:(1000+.),state:"CLOSED",body:"",headRefName:"x",closing:[],authorAssociation:"NONE",author:{login:"z"}}] + | .[119] |= (.body="" | .authorAssociation="OWNER") + | .[219] |= (.body="" | .authorAssociation="OWNER")' > "$STATE/prs.json" +check "a PR on the second page of PRs is found" "1120 CLOSED" "$(prs_for_issue 20)" +check "a PR past the 200 newest is not (the cap is as it was)" "" "$(prs_for_issue 21)" echo "Trust" cat > "$STATE/prs.json" <<'J'