wrapper-guard: an arm may only claim the span its wrapper actually covers
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Round eight replaced an absence-driven allow with an endpoint inventory, and
review found the inventory answered the wrong question. It recorded which
wrapper TOUCHES an endpoint, when the sound question is whether the wrapper
SPANS it. A PATCH to a numbered milestone blocked with "use milestone-close.sh".
That wrapper takes only -t <title> and sends state=closed on both the gh and tea
paths, so it cannot express a title, description or due-date edit. The block was
correct and the advice was not — the same remediation-accuracy defect as the
round-seven subresource arm, one step quieter, because a wrapper was rounded up
from owning a slice to owning the endpoint.
The treatment already existed two arms away: /pulls/{n} named pr-close.sh for
state and said in the message that a PR title/body edit is a real wrapper gap.
So this was a consistency failure rather than a missing idea, which is why the
fix is not just the reported arm. Auditing every arm for span against the flags
each script accepts found a second bad one that review had not reached:
/pulls/{n}/requested_reviewers was mapped to pr-review.sh, and pr-review.sh
takes -a <action> -c <comment> and files a verdict. Nothing in this tree adds a
requested reviewer, so that arm was advertising a wrapper that cannot make the
call. It is unowned and now flows through, like a comment edit.
Changes:
- /milestones/{n} keeps blocking, and the message states that milestone-close.sh
owns the close only while title/description/due-date is a wrapper gap.
- /pulls/{n}/requested_reviewers becomes residue, above the reviews arm so it
cannot be refused with "use pr-review.sh".
- /issues/{n} now also names issue-assign.sh, which owns the assignee field;
issue-edit.sh has no assignee flag, so the old advice was short by one wrapper
for a PATCH that sets one.
- The map comment carries a span column, so a future arm has to state what its
wrapper covers rather than imply all of it.
Fixtures assert the span language, not just the wrapper name: the milestone edit
must say it owns the close only, and a PATCH setting an assignee must name
issue-assign.sh. Negative-controlled — reverting each of the three behaviours
fails that fixture and only that fixture.
92/92 (was 89), locally and in ci-base. shellcheck clean at warning+. The
18-command ordinary sweep blocks the same three round-six flips and nothing new.
Gates: fixtures 92/92 in ci-base, shellcheck clean at warning+.
This commit is contained in:
Hermes Agent
@@ -170,7 +170,16 @@ FIXTURES="$TMP/fixtures.tsv"
|
||||
printf '2\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/labels"}}\ta PR is an issue where labels live, so the issue wrapper owns them\tissue-edit.sh\n'
|
||||
printf '2\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1"}}\tPR state is pr-close.sh, and the gap in that arm is stated\tpr-close.sh\n'
|
||||
printf '2\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/milestones/4"}}\ta milestone state change is milestone-close.sh, not the create wrapper\tmilestone-close.sh\n'
|
||||
# SPAN. A wrapper that owns a slice of an endpoint must not be advertised as
|
||||
# owning the endpoint. milestone-close.sh takes only -t <title> and sends
|
||||
# state=closed, so a title/description/due-date edit is a gap and the message
|
||||
# has to say so — round eight named the wrapper and stopped there.
|
||||
printf '2\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/milestones/1"}}\ta milestone edit blocks, but the advice states the close-only span\towns the CLOSE only\n'
|
||||
printf '2\t{"tool_input":{"command":"gh api -X PATCH repos/a/b/issues/1 -f assignee=u"}}\tissue-edit.sh cannot set an assignee, so the message names the one that can\tissue-assign.sh owns the assignee\n'
|
||||
# The residue: still genuinely owned by nothing, and still flowing through.
|
||||
# Requesting a reviewer is not submitting one; pr-review.sh files verdicts and
|
||||
# nothing in the tree adds a requested reviewer.
|
||||
printf '0\t{"tool_input":{"command":"gh api -X POST repos/a/b/pulls/1/requested_reviewers -f reviewers[]=u"}}\tno wrapper requests a reviewer, so it is not refused with pr-review.sh\n'
|
||||
printf '0\t{"tool_input":{"command":"curl -X PATCH -d @b https://git.example.invalid/api/v1/repos/a/b/issues/comments/5"}}\tediting a comment has no wrapper; only creating one does\n'
|
||||
printf '0\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/issues/1/stopwatch/start"}}\tno wrapper owns a stopwatch, and none is invented for it\n'
|
||||
printf '0\t{"tool_input":{"command":"gh api -X POST repos/a/b/issues/1/times -f time=60"}}\tnor time tracking\n'
|
||||
|
||||
@@ -195,17 +195,30 @@ if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then
|
||||
# ONLY because a wrapper in this directory owns that call. It is an
|
||||
# inventory, not a model — read off `ls tools/git/*.sh` and the flags each
|
||||
# script accepts, so it can be re-derived and checked rather than believed.
|
||||
# Second column is what the wrapper SPANS; a partial span must be stated in
|
||||
# the block message, never rounded up to ownership of the whole endpoint.
|
||||
#
|
||||
# /pulls/{n}/reviews, /requested_reviewers pr-review.sh
|
||||
# /pulls/{n}/merge pr-merge.sh
|
||||
# /issues/{n}/comments issue-comment.sh (create)
|
||||
# /issues|pulls/{n}/assignees issue-assign.sh
|
||||
# /issues|pulls/{n}/labels issue-edit.sh, issue-assign.sh
|
||||
# /milestones/{n} milestone-close.sh
|
||||
# /issues/{n} issue-edit.sh; issue-close.sh,
|
||||
# issue-reopen.sh for state
|
||||
# /pulls/{n} pr-close.sh (state only)
|
||||
# /pulls, /issues, /milestones the create wrappers
|
||||
# /pulls/{n}/reviews pr-review.sh full
|
||||
# /pulls/{n}/merge pr-merge.sh full (-m method, -d)
|
||||
# /issues/{n}/comments issue-comment.sh create only (POST)
|
||||
# /issues|pulls/{n}/assignees issue-assign.sh full (-a, -r)
|
||||
# /issues|pulls/{n}/labels issue-edit.sh sets the whole list;
|
||||
# issue-assign.sh -l same
|
||||
# /milestones/{n} milestone-close.sh CLOSE ONLY — title,
|
||||
# description, due date
|
||||
# are a wrapper gap
|
||||
# /issues/{n} issue-edit.sh title/body/labels/
|
||||
# milestone; close/reopen
|
||||
# for state; assignee is
|
||||
# issue-assign.sh
|
||||
# /pulls/{n} pr-close.sh state only — title and
|
||||
# body are a wrapper gap
|
||||
# /pulls, /issues, the create wrappers full
|
||||
# /milestones
|
||||
#
|
||||
# Owned by nothing, so they flow through: /issues/comments/{id} (a comment
|
||||
# EDIT), /pulls/{n}/requested_reviewers, and the residue caught by
|
||||
# subtraction below.
|
||||
#
|
||||
# Round seven had a single arm allowing EVERY path under a numbered issue or
|
||||
# PR, on the reasoning that no wrapper owned any of them. Review showed that
|
||||
@@ -222,10 +235,25 @@ if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then
|
||||
# override is a guard that is off. So the fix is precision in BOTH
|
||||
# directions: every arm names the wrapper that actually owns the call, and
|
||||
# anything genuinely unowned still flows through (below).
|
||||
#
|
||||
# Round eight got the inventory right and the SPAN wrong, which review caught
|
||||
# on /milestones/{n}: milestone-close.sh takes only -t <title> and hardcodes
|
||||
# state=closed, so it cannot express a title, description or due-date edit,
|
||||
# and naming it there told an agent to use a wrapper that cannot make the
|
||||
# call. "Which wrapper touches this endpoint" is the wrong question; "does
|
||||
# the wrapper SPAN this endpoint" is the right one. Where a wrapper owns only
|
||||
# a slice, `alsoown` must say which slice and name the rest as a gap — the
|
||||
# treatment /pulls/{n} already had, and that two other arms did not, so this
|
||||
# was a consistency failure rather than a missing idea. Auditing every arm
|
||||
# for span (not just the reported one) is what found requested_reviewers.
|
||||
endpoint=""; wrapper=""; alsoown=""
|
||||
case "$CMD" in
|
||||
*"/pulls/"*"/reviews"*|*"/pulls/"*"/requested_reviewers"*)
|
||||
endpoint="pull-request review"; wrapper="pr-review.sh" ;;
|
||||
# Requesting a reviewer is not submitting one. pr-review.sh takes
|
||||
# -a <action> -c <comment> and files a verdict; nothing in the tree adds a
|
||||
# requested reviewer. Unowned, so it flows through — placed above the
|
||||
# reviews arm so it cannot be refused with "use pr-review.sh".
|
||||
*"/pulls/"*"/requested_reviewers"*) : ;;
|
||||
*"/pulls/"*"/reviews"*) endpoint="pull-request review"; wrapper="pr-review.sh" ;;
|
||||
*"/pulls/"*"/merge"*) endpoint="pull-request merge"; wrapper="pr-merge.sh" ;;
|
||||
# A comment EDIT/DELETE lives at /issues/comments/{id} — a sibling of the
|
||||
# numbered issue, not a child of it. issue-comment.sh only creates, so
|
||||
@@ -238,12 +266,17 @@ if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then
|
||||
*"/issues/"*"/labels"*|*"/pulls/"*"/labels"*)
|
||||
endpoint="issue label"; wrapper="issue-edit.sh"
|
||||
alsoown="issue-assign.sh -l sets labels too (and the milestone)." ;;
|
||||
*"/milestones/"[0-9]*) endpoint="milestone state"; wrapper="milestone-close.sh" ;;
|
||||
*"/milestones/"[0-9]*) endpoint="milestone"; wrapper="milestone-close.sh"
|
||||
alsoown="milestone-close.sh owns the CLOSE only — it takes -t <title>
|
||||
and sends state=closed. A milestone's title, description or due date is a real
|
||||
wrapper gap: no tool in this tree edits them, and the override exists for it." ;;
|
||||
# The numbered object itself. These two arms are the fuzzy ones — they
|
||||
# match a number and then anything — so they are refined immediately
|
||||
# below rather than trusted as written.
|
||||
*"/issues/"[0-9]*) endpoint="issue edit"; wrapper="issue-edit.sh"
|
||||
alsoown="issue-close.sh and issue-reopen.sh own the state change." ;;
|
||||
alsoown="issue-close.sh and issue-reopen.sh own the state change, and
|
||||
issue-assign.sh owns the assignee, labels and milestone fields at this same
|
||||
number — issue-edit.sh does not set an assignee." ;;
|
||||
*"/pulls/"[0-9]*) endpoint="pull-request edit"; wrapper="pr-close.sh"
|
||||
alsoown="pr-close.sh owns state=closed. A PR's labels, assignee and
|
||||
milestone are the ISSUE object on both providers, so issue-edit.sh and
|
||||
|
||||
Reference in New Issue
Block a user