diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index d42cc12c..accbb444 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -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 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' diff --git a/packages/mosaic/framework/tools/git/wrapper-guard.sh b/packages/mosaic/framework/tools/git/wrapper-guard.sh index cc745ac6..4881fc6b 100755 --- a/packages/mosaic/framework/tools/git/wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/wrapper-guard.sh @@ -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