Skip to content

fix: suppression 门工件移出被审树(pr.diff 标记行重复计数根除,.github #87) - #49

Merged
randypanding merged 1 commit into
mainfrom
fix/v1.5.10-artifacts-out
Aug 21, 2026
Merged

fix: suppression 门工件移出被审树(pr.diff 标记行重复计数根除,.github #87)#49
randypanding merged 1 commit into
mainfrom
fix/v1.5.10-artifacts-out

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

v1.5.9 复测(Use-up-Plan PR#61):净增 5 正确判定,但总量=10——pr.diff 工件留在工作区,其中 5 个标记行被树扫描重复计入。工件改写 RUNNER_TEMP。基线无需上调(真实 main 计数 0)。C1:.github/,ADR-0036 修订域。

Copilot AI lite review requested due to automatic review settings August 21, 2026 07:24
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 185e2b9d-87b8-4950-8d6e-c48334f25e8b

📥 Commits

Reviewing files that changed from the base of the PR and between 1cae5c8 and 0ef078d.

📒 Files selected for processing (1)
  • .github/workflows/suppression-gate.yml

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix suppression gate double-count by writing PR artifacts to RUNNER_TEMP

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Write PR diff/title/body artifacts to RUNNER_TEMP to avoid workspace tree-scan double counting.
• Keep fail-closed behavior when PR diff is empty.
• Pass updated artifact paths into suppression-budget.sh invocation.
Diagram

graph TD
  A["GitHub Actions job"] --> B["Fetch PR diff/title/body (gh api)"] --> C["Artifacts in RUNNER_TEMP"] --> D["suppression-budget.sh"] --> E["Tree scan (.)"] --> F["Budget decision"]
  D --> C
  E --> D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Ignore generated artifacts in tree scan
  • ➕ Keeps artifacts in workspace for easier debugging/artifact upload
  • ➕ Centralizes the rule in the scanning script (less workflow-specific)
  • ➖ Requires changes to suppression-budget.sh to support excludes/ignore patterns
  • ➖ Higher risk of missing other future generated files without good defaults
2. Pipe diff/title/body via stdin or env (no files)
  • ➕ Eliminates file lifecycle/cleanup concerns entirely
  • ➕ Avoids any chance of filesystem-based double counting
  • ➖ May require more invasive script interface changes
  • ➖ Harder to debug locally compared to inspecting files

Recommendation: Current approach (write PR artifacts to $RUNNER_TEMP and pass those paths) is the lowest-risk, smallest-scope fix for the double-count issue because it removes generated inputs from the scanned tree without changing the suppression-budget script behavior.

Files changed (1) +9 / -7

Bug fix (1) +9 / -7
suppression-gate.ymlWrite PR diff/title/body into RUNNER_TEMP and update script args +9/-7

Write PR diff/title/body into RUNNER_TEMP and update script args

• Moves generation of pr.diff, pr-title.txt, and pr-body.txt from the workspace root to $RUNNER_TEMP. Updates suppression-budget.sh invocation to consume the new paths, preventing tree scanning from double-counting markers contained in generated artifacts while preserving the fail-closed empty-diff check.

.github/workflows/suppression-gate.yml

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the suppression-gate workflow to write PR diff/title/body artifacts into RUNNER_TEMP instead of the workspace, preventing tree-scan double counting of suppression marker lines (per the v1.5.9 retest context described in the PR).

Changes:

  • Redirect PR diff/title/body fetch outputs to $RUNNER_TEMP to keep the workspace scan tree clean.
  • Update suppression-budget.sh invocation to consume those $RUNNER_TEMP paths.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

gh api "repos/$REPO/pulls/$PR_NUM" -H "Accept: application/vnd.github.diff" > "$RUNNER_TEMP/pr.diff"
test -s "$RUNNER_TEMP/pr.diff" || { echo "::error::suppression-budget: PR diff 拉取为空(fail-closed)"; exit 1; }
printf '%s' "$PR_TITLE" > "$RUNNER_TEMP/pr-title.txt"
gh api "repos/$REPO/pulls/$PR_NUM" --jq '.body // ""' > "$RUNNER_TEMP/pr-body.txt
@randypanding
randypanding merged commit 5906d93 into main Aug 21, 2026
13 checks passed
@randypanding
randypanding deleted the fix/v1.5.10-artifacts-out branch August 21, 2026 07:26
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Unterminated quote breaks gate 🐞 Bug ≡ Correctness
Description
In the “取 PR diff / 标题 / 正文” step, the redirect target for pr-body.txt is missing a closing quote,
causing a bash parse error and failing the workflow before suppression-budget.sh can run.
Code

.github/workflows/suppression-gate.yml[111]

+          gh api "repos/$REPO/pulls/$PR_NUM" --jq '.body // ""' > "$RUNNER_TEMP/pr-body.txt
Relevance

●●● Strong

Deterministic shell syntax bug (unterminated quote) breaks workflow; trivial fixes like this are
accepted.

PR-#8

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow currently contains an unterminated double-quoted string on the output path. The
suppression-budget script validates that --pr-body is a readable file, but the workflow will fail
earlier due to the bash syntax error, preventing creation/usage of the body file.

.github/workflows/suppression-gate.yml[104-112]
scripts/suppression-budget.sh[53-58]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The workflow step that fetches the PR body writes to `"$RUNNER_TEMP/pr-body.txt` without a closing quote, which causes bash to error (unterminated string) and stops the job.

### Issue Context
This is inside a `run: |` block executed with `set -euo pipefail`, so the step will fail immediately and the downstream `suppression-budget.sh` invocation (which expects `--pr-body` to point to a real file) will never execute.

### Fix Focus Areas
- .github/workflows/suppression-gate.yml[106-112]

### Proposed fix
Add the missing trailing `"`:
```bash
gh api "repos/$REPO/pulls/$PR_NUM" --jq '.body // ""' > "$RUNNER_TEMP/pr-body.txt"
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This is a localized CI workflow behavior change affecting suppression-gate artifact paths and validation; it has meaningful correctness and fail-closed implications, but not enough independent logic for extended review.

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

gh api "repos/$REPO/pulls/$PR_NUM" -H "Accept: application/vnd.github.diff" > "$RUNNER_TEMP/pr.diff"
test -s "$RUNNER_TEMP/pr.diff" || { echo "::error::suppression-budget: PR diff 拉取为空(fail-closed)"; exit 1; }
printf '%s' "$PR_TITLE" > "$RUNNER_TEMP/pr-title.txt"
gh api "repos/$REPO/pulls/$PR_NUM" --jq '.body // ""' > "$RUNNER_TEMP/pr-body.txt

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Unterminated quote breaks gate 🐞 Bug ≡ Correctness

In the “取 PR diff / 标题 / 正文” step, the redirect target for pr-body.txt is missing a closing quote,
causing a bash parse error and failing the workflow before suppression-budget.sh can run.
Agent Prompt
### Issue description
The workflow step that fetches the PR body writes to `"$RUNNER_TEMP/pr-body.txt` without a closing quote, which causes bash to error (unterminated string) and stops the job.

### Issue Context
This is inside a `run: |` block executed with `set -euo pipefail`, so the step will fail immediately and the downstream `suppression-budget.sh` invocation (which expects `--pr-body` to point to a real file) will never execute.

### Fix Focus Areas
- .github/workflows/suppression-gate.yml[106-112]

### Proposed fix
Add the missing trailing `"`:
```bash
gh api "repos/$REPO/pulls/$PR_NUM" --jq '.body // ""' > "$RUNNER_TEMP/pr-body.txt"
```

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants