fix(verify-pr): self-exclude own workflow check-runs from CI Status - #307
Conversation
The head-SHA check-run prefetch enumerated every check-run on the commit, including verify-pr's own workflow (`fullsend verify-pr`) runs — the in-progress dispatch and any superseded prior attempt. Those are non-terminal/failed at evaluation time, so correctness.md Check 1a mapped them to pending/failed and forced a permanent self-referential CI Status WARN/FAIL: verify-pr could never report CI Status = PASS (TC-6332). Add the CI-Status analogue of Step 1's `running-workflow-name` self- exclusion. pre-verify-pr.sh enumerates this workflow's own check-run names for the head SHA — across all of its runs at that SHA, so superseded attempts are covered — and passes them to a pure, unit-testable Python helper `filter_own_check_runs`, which drops them before they reach `github.check_runs`. Exclusion is by workflow NAME, not run ID. Name gathering is best-effort: a token lacking actions:read degrades to no self-exclusion (prior behavior) with a warning rather than aborting. Fixes TC-6332. Assisted-by: Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reviewer's GuideThe PR prevents verify-pr from evaluating its own workflow check-runs as substantive CI results by collecting all matching job names on the head SHA in the trusted shell prefetch, filtering them in a pure Python transform helper, and documenting and testing the behavior, including graceful fallback when GitHub enumeration is unavailable. Sequence diagram for verify-pr self-excluding workflow checkssequenceDiagram
participant Shell as pre-verify-pr.sh
participant GitHub as GitHub API
participant Transform as pre_verify_pr.py
participant Bundle as CI Status input
participant Correctness as correctness.md
Shell->>GitHub: gh api check-runs for head SHA
Shell->>GitHub: gh api workflow runs and jobs for head SHA
GitHub-->>Shell: Own job names across all workflow runs
Shell->>Transform: --own-check-names-file
Transform->>Transform: _read_own_check_names
Transform->>Transform: filter_own_check_runs
Transform-->>Bundle: Bundle without verify-pr check-runs
Bundle->>Correctness: Evaluate substantive CI checks
Correctness-->>Correctness: CI Status can reach PASS
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="plugins/sdlc-workflow/skills/verify-pr/correctness.md" line_range="46-51" />
<code_context>
**Sandbox mode** (CI Status input provided): do **not** run `gh` — read the
pre-fetched check-run outcomes from the CI Status input (each entry has `name`,
-`status`, `conclusion`, `details_url`). Map each entry to a status:
+`status`, `conclusion`, `details_url`). These are already **self-excluded** on the
+trusted runner: verify-pr's own workflow (`fullsend verify-pr`) check-runs — the
+in-progress dispatch and any superseded prior attempt — are removed before the
+bundle is written (the CI-Status analogue of Step 1's `running-workflow-name`
+self-exclusion), so the sandbox never evaluates verify-pr's own runs and a PR
+whose substantive checks all pass can reach CI Status = PASS. Map each entry to a
+status:
</code_context>
<issue_to_address>
**issue:** The sandbox guidance unconditionally says verify-pr's own check-runs are already removed and that the sandbox never evaluates them, but the shell explicitly falls back to an empty names file when enumeration fails, and `filter_own_check_runs` then preserves all check-runs. In that documented fallback case, the original self-referential WARN/FAIL behavior remains while the correctness instructions incorrectly imply it cannot occur.
**Triggers:** When the runner token lacks `actions:read` or own-run enumeration encounters an API failure.
**Suggested fix:** Document the best-effort fallback and instruct the evaluator to account for an input that can still contain verify-pr's own runs, or make self-exclusion a required successful prefetch.
```suggestion
`status`, `conclusion`, `details_url`). The trusted runner best-effort
**self-excludes** verify-pr's own workflow (`fullsend verify-pr`) check-runs — the
in-progress dispatch and any superseded prior attempt — before writing the bundle
(the CI-Status analogue of Step 1's `running-workflow-name` self-exclusion). If
check-run enumeration fails, the fallback may leave verify-pr's own runs in the
CI Status input; account for that possibility when evaluating each entry. Map each entry to a
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and a name-based exclusion changes the CI verification policy for every PR and could hide a real failing check, causing false PASS results with no inherent signal that the check was omitted. Reverting prevents future false statuses but does not undo verification results already produced while the exclusion was active.
Blocking findings: plugins/sdlc-workflow/skills/verify-pr/correctness.md:51
The real fullsend-verify-pr.yml (pull_request_target + ok-to-test, TC-6331) lived only on the verify-pr-fullsend feature branch; main still carried the workflow_dispatch placeholder. GitHub registers and dispatches pull_request_target from the workflow file on the DEFAULT branch (unlike pull_request, which is read from the PR base branch), so the trigger was never active: since TC-6331 merged, no verify-pr run fired for any PR — fork or same-repo (e.g. PR #307 got zero dispatch). Replace the placeholder on main with the real workflow so the event registers. A `branches: [verify-pr-fullsend]` filter scopes the rollout to PRs targeting the feature branch, so verify-pr does not yet review PRs into main; the base-branch copy still executes in trusted context. Widen/remove the filter when the feature graduates to main. Assisted-by: Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 Finished Verify Pr · ✅ Success · Started 3:28 PM UTC · Completed 3:41 PM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $5.14 |
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review — Classified as code change request — this is the same finding as the inline comment on correctness.md:51 (the best-effort self-exclusion is documented as unconditional). Sub-task TC-6349 created to address it. Noted the accompanying Sourcery assessment that a name-based exclusion changes CI verification policy for every PR and could mask a real failing check — a human reviewer should weigh that before merging. |
Verification Report for TC-6343 (commit 654cf01)
Overall: FAILOverall FAIL is driven by Commit Traceability: the sole commit cites the underlying bug TC-6332 but not the task TC-6343 — recommend amending/adding a commit that references TC-6343. Two informational WARNs: (1) Review Feedback — sourcery-ai flagged that correctness.md's new sandbox guidance states verify-pr's own check-runs are 'already self-excluded' unconditionally, while pre-verify-pr.sh degrades to a no-op when own-run enumeration fails (e.g. a token lacking actions:read), so the self-referential WARN/FAIL can still occur; a review-feedback sub-task was created to document the fallback. (2) CI Status — the only non-passing check is verify-pr's own in-progress harness run (the very self-referential artifact this PR fixes), still present because the runner's prefetch has not yet applied this PR's fix; all substantive checks pass. Acceptance criteria are fully met, the change is additive and well-tested, and no secrets were introduced. This report is informational — a human reviewer decides whether to merge. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
Reword correctness.md Check 1a sandbox-mode note: the own-check-run self-exclusion is best-effort (needs actions:read) and degrades to a no-op with a WARNING when enumeration fails, so the CI Status input can still contain verify-pr's own runs. Instruct the evaluator to map each entry on its merits and note own-run-only non-passing entries in the evidence rather than treating the PR as blocked. Implements TC-6349 Assisted-by: Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 Finished Verify Pr · ✅ Success · Started 4:25 PM UTC · Completed 4:36 PM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $5.20 |
Verification Report for TC-6343 (commit da053ba)
Overall: FAILOverall: FAIL — driven solely by Commit Traceability: neither of the PR's two commits references the parent task TC-6343. Commit All substantive verification passed: scope is contained to the 4 task-listed files, no secrets, and all 5 acceptance criteria are met — the reproducer test ( CI Status is WARN, not FAIL: the only non-passing entry is verify-pr's own The one review finding (sourcery-ai: the correctness.md self-exclusion note was stated unconditionally despite the best-effort fallback) was already classified as a code change request, tracked as sub-task TC-6349, and addressed by commit This skill does not merge the PR. A human reviewer should decide on merge — and consider adding a This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
TC-6343 Option A self-pin: move the base URL commit and re-lock so the verify-pr CI run fetches the fixed pre-verify-pr.sh (CI-Status own-workflow self-exclusion) and the reworded correctness.md Check 1a from this PR's own fix commits, rather than the stale fa3f4b7 pin. - .fullsend/harness/verify-pr.yaml: base URL commit fa3f4b7 -> da053ba (root harness/verify-pr.yaml is byte-identical, sha256 unchanged) - .fullsend/lock.yaml: fullsend lock regenerates all 11 child pins at da053ba; pre-verify-pr.sh, pre_verify_pr.py, and correctness.md hashes now reflect the TC-6343 / TC-6349 fixes Merge via merge commit (squash disabled) so da053ba stays reachable and the pin keeps resolving after merge. Assisted-by: Claude Code Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🤖 Finished Verify Pr · ✅ Success · Started 5:01 PM UTC · Completed 5:10 PM UTC Commit: Runtime: claude · Model: claude-opus-4-8 · Effort: high · Cost: $4.50 |
Verification Report for TC-6343 (commit 02f271b)
Overall: WARNOverall: WARN - no FAIL, but several WARN checks a human should weigh before merging:
Sourcery also noted (informationally) that name-based CI self-exclusion changes verification policy for every PR and could mask a genuinely failing check - a human reviewer should weigh that trade-off. This skill does not merge the PR or transition the issue. This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
1 participant
Implements TC-6343. Fixes TC-6332. Also implements TC-6349 (doc-accuracy follow-up).
Problem
The head-SHA check-run prefetch enumerated every check-run on the commit — including verify-pr's own workflow (
fullsend verify-pr) runs (the in-progress dispatch and any superseded prior attempt). Those own runs are non-terminal (or a superseded attempt failed) at evaluation time, socorrectness.mdCheck 1a mapped them to pending/failed, draggingCI Statusto a permanent self-referential WARN/FAIL. verify-pr could essentially never reportCI Status = PASS.Fix
The CI-Status analogue of Step 1's
running-workflow-name: wait-for-checksself-exclusion:pre-verify-pr.shenumerates this workflow's own check-run names for the head SHA — across all of its runs at that SHA, so superseded attempts are covered too — and passes them to the transform. Best-effort: a token lackingactions:readdegrades to no self-exclusion (prior behavior) with a warning rather than aborting.pre_verify_pr.pygains a pure, unit-testable helperfilter_own_check_runs(check_runs, own_names)(plus_read_own_check_names), applied in_github_from_dirbefore the bundle reachesgithub.check_runs. Exclusion is by workflow name, not run ID.correctness.mdCheck 1a documents the self-exclusion as best-effort (TC-6349): it degrades to a no-op with aWARNINGwhen own-run enumeration fails (e.g. missingactions:read), so the evaluator is told the CI Status input may still contain verify-pr's own runs and must map each entry on its merits.Verification
python3 -m pytest plugins/sdlc-workflow/scripts/test_pre_verify_pr.py -q— 90 passed (new reproducer, filter unit tests, preservation,_read_own_check_names, two CLI round-trips, shell regression guard).uvx skillsaw— 0 errors.claude plugin validate plugins/sdlc-workflow— passed.🤖 Generated with Claude Code
Summary by Sourcery
Exclude verify-pr's own workflow check-runs from CI Status evaluation so successful substantive checks can report PASS.
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: