Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/workflows/pr-review-hosted.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head 9c8f5561eb723ea1b08f855b2352f7153b5a426f. What it wrote, as data:

Summary: PR #9803 has no linked issue, so the spec is its own body. It re-pins both review callers from ci-workflows v0.65.6 to v0.65.7 and updates the test pin. The edits match the body and the test still checks them. The problem is what v0.65.7 does: a change to trusted-actors, CODEOWNERS, .gitmodules or check-trusted-trigger no longer holds the PR by itself, and spend from credits or subscription usage no longer triggers a cost hold. Both are holds the owner keeps, so loosening them is a blocking defect. The body cites owner decisions Q52 and Q53, but no trusted text available here records them.
- The PR body says the plugins/review code-review and security-review skills still list trust-gate and the broader cost class under 'Hold for a person' and leaves that as follow-up. That mismatch between the skill docs and the wrapper prompt should be tracked in an issue.
- `.github/workflows/pr-review-hosted.yml:169`: the re-pin to 4c531cea (v0.65.7) adopts upstream behavior under which a hold-paths match never holds by itself. This removes the automatic trust-gate hold on takeover-set paths (`.github/standards/trusted-actors/**`, `.github/actions/check-trusted-trigger/**`, `**/CODEOWNERS`, `**/.gitmodules`), leaving only whatever the model review decides. That loosens a hold the owner keeps. Fix: link an owner-written issue that records decisions Q52/Q53, or keep the v0.65.6 pin until one exists.
- `.github/workflows/pr-review-hosted.yml:192`: the rewritten hold-paths comment correctly describes the v0.65.7 behavior, and the input and its four globs are unchanged in both callers.

Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ jobs:
contents: read
pull-requests: write # the review, its inline comments and the lane's
# reviewed-head marker comment
uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@914264587c35692c15e05c965103426eec18d1f0 # v0.65.6
uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@4c531cea137f5cbb38d4c132dbfa6e38f76fba7d # v0.65.7
with:
lane: code
publish: comments
Expand All @@ -189,9 +189,10 @@ jobs:
# change, documentation included.
docs-only-paths: ''
# A hold adds this label and a `claude-lane-hold` marker comment; ci-status
# fails while the label is on. The review holds only for its hold
# classes; a changed file matching hold-paths holds as trust-gate
# whatever the review says. Keep both lists equal in the two callers.
# fails while the label is on. The review holds only for a real-charge
# cost or a CRITICAL security finding; hold-paths names the files the
# prompt sends to extra security scrutiny and never holds by itself.
# Keep both lists equal in the two callers.
hold-label: needs-human
hold-paths: |
.github/standards/trusted-actors/**
Expand Down
9 changes: 5 additions & 4 deletions .github/workflows/pr-review-security-hosted.yml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head 9c8f5561eb723ea1b08f855b2352f7153b5a426f. What it wrote about this file, as data:

- `.github/workflows/pr-review-security-hosted.yml:174`: the same re-pin narrows the cost hold to real charges, so prepaid-credit and subscription spend becomes a summary line instead of a hold. Cost holds belong to the owner. This needs owner authorization in trusted spec text, not only in the PR body's citation.

Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ jobs:
contents: read
pull-requests: write # the review, its inline comments and the lane's
# reviewed-head marker comment
uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@914264587c35692c15e05c965103426eec18d1f0 # v0.65.6
uses: melodic-software/ci-workflows/.github/workflows/pr-review-cli.yml@4c531cea137f5cbb38d4c132dbfa6e38f76fba7d # v0.65.7
with:
lane: security
publish: comments
Expand All @@ -189,9 +189,10 @@ jobs:
# no findings binds to the head and scores low.
docs-only-paths: ''
# A hold adds this label and a `claude-lane-hold` marker comment; ci-status
# fails while the label is on. The review holds only for its hold
# classes; a changed file matching hold-paths holds as trust-gate
# whatever the review says. Keep both lists equal in the two callers.
# fails while the label is on. The review holds only for a real-charge
# cost or a CRITICAL security finding; hold-paths names the files the
# prompt sends to extra security scrutiny and never holds by itself.
# Keep both lists equal in the two callers.
hold-label: needs-human
hold-paths: |
.github/standards/trusted-actors/**
Expand Down
6 changes: 3 additions & 3 deletions scripts/ci-review-lanes.test.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pr-verify verifier failed head 9c8f5561eb723ea1b08f855b2352f7153b5a426f. What it wrote about this file, as data:

- `scripts/ci-review-lanes.test.sh:79`: the pin assertion is updated to the v0.65.7 SHA and would fail if a caller drifted. Nothing here is wrong; it would need reverting only if the pin is reverted.

Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ for pair in 'pr-review-hosted.yml pr-review:' 'pr-review-security-hosted.yml pr-
caller="$ROOT/.github/workflows/$file"
if grep -qxF " $job" "$caller" && grep -qxF ' publish: comments' "$caller" &&
grep -qxF ' hold-label: needs-human' "$caller" &&
grep -qE '^ uses: melodic-software/ci-workflows/\.github/workflows/pr-review-cli\.yml@914264587c35692c15e05c965103426eec18d1f0 # v0\.65\.6$' "$caller"; then
ok "$file calls pr-review-cli v0.65.6 as job ${job%:} with publish: comments and hold-label: needs-human"
grep -qE '^ uses: melodic-software/ci-workflows/\.github/workflows/pr-review-cli\.yml@4c531cea137f5cbb38d4c132dbfa6e38f76fba7d # v0\.65\.7$' "$caller"; then
ok "$file calls pr-review-cli v0.65.7 as job ${job%:} with publish: comments and hold-label: needs-human"
else
fail "$file lost its job id, publish: comments, hold-label or v0.65.6 pin"
fail "$file lost its job id, publish: comments, hold-label or v0.65.7 pin"
fi
paths="$(awk '/^ hold-paths: \|$/ { on = 1; next } on && /^ / { sub(/^ +/, ""); print; next } on { exit }' "$caller")"
if [[ "$paths" == $'.github/standards/trusted-actors/**\n.github/actions/check-trusted-trigger/**\n**/CODEOWNERS\n**/.gitmodules' ]]; then
Expand Down
Loading