-
Notifications
You must be signed in to change notification settings - Fork 61
t1422: Cap quality-debt PR blast radius at 5 files to prevent conflict cascades #3956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -580,6 +580,57 @@ QUALITY_DEBT_MAX=$(( MAX_WORKERS * 30 / 100 )) | |||||||||||||||||
|
|
||||||||||||||||||
| If `QUALITY_DEBT_CURRENT >= QUALITY_DEBT_MAX`, do not dispatch more quality-debt issues this cycle. | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Quality-debt PR blast radius cap (t1422) | ||||||||||||||||||
|
|
||||||||||||||||||
| Quality-debt PRs that touch many files conflict with every other PR in flight. When multiple large-batch quality-debt PRs are created concurrently, they cascade into merge conflicts — each merge moves main, invalidating the next PR's base. This was observed in March 2026: 19 of 30 open PRs were conflicting, with individual PRs touching up to 69 files. | ||||||||||||||||||
|
|
||||||||||||||||||
| **Rule: quality-debt PRs must touch at most 5 files.** This is a hard cap enforced by the worker (see `full-loop.md` "Quality-debt blast radius cap"). The pulse enforces it at dispatch time by scoping issue descriptions: | ||||||||||||||||||
|
|
||||||||||||||||||
| 1. **Per-file issues preferred.** When creating quality-debt issues (via `quality-feedback-helper.sh`, code-simplifier, or manual filing), create one issue per file or per tightly-coupled file group (max 5 files). An issue titled "Fix shellcheck violations in dispatch.sh" will produce a 1-file PR that conflicts with nothing. An issue titled "Fix shellcheck violations across 20 scripts" will produce a 20-file PR that conflicts with everything. | ||||||||||||||||||
|
|
||||||||||||||||||
| 2. **File-level dedup before dispatch.** Before dispatching a quality-debt worker, check whether any open PR already touches the same files. If overlap exists, skip the issue this cycle — the existing PR must merge first. | ||||||||||||||||||
|
|
||||||||||||||||||
| ```bash | ||||||||||||||||||
| # Get files that would be touched by this issue (from issue body or title) | ||||||||||||||||||
| # Then check open PRs for overlap | ||||||||||||||||||
| OPEN_PR_FILES=$(gh pr list --repo <slug> --state open --json number,files \ | ||||||||||||||||||
| --jq '[.[].files[].path] | unique | .[]' 2>/dev/null) | ||||||||||||||||||
|
|
||||||||||||||||||
| # If the issue mentions specific files, check for overlap | ||||||||||||||||||
| # This is a judgment call — read the issue body for file paths | ||||||||||||||||||
| # If overlap is found, skip: "Skipping quality-debt #NNN — files overlap with open PR #MMM" | ||||||||||||||||||
| ``` | ||||||||||||||||||
|
|
||||||||||||||||||
| 3. **Serial merge for quality-debt.** Do not dispatch a second quality-debt worker for the same repo while a quality-debt PR is open and mergeable. Wait for the first to merge, then dispatch the next. This prevents the conflict cascade at the source. Feature PRs are unaffected — they touch different files by nature. | ||||||||||||||||||
|
|
||||||||||||||||||
| ```bash | ||||||||||||||||||
| # Check for open quality-debt PRs in this repo | ||||||||||||||||||
| OPEN_DEBT_PRS=$(gh pr list --repo <slug> --state open \ | ||||||||||||||||||
| --json number,title,labels \ | ||||||||||||||||||
| --jq '[.[] | select(.labels[]?.name == "quality-debt" or (.title | test("quality.debt|fix:.*batch|fix:.*harden"; "i")))] | length' \ | ||||||||||||||||||
| 2>/dev/null || echo 0) | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to the previous comment, using This violates the repository's general rule: 'In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.' Removing
Suggested change
References
|
||||||||||||||||||
|
|
||||||||||||||||||
| # If there's already an open quality-debt PR, skip dispatching more | ||||||||||||||||||
| if [[ "$OPEN_DEBT_PRS" -gt 0 ]]; then | ||||||||||||||||||
| echo "Skipping quality-debt dispatch — $OPEN_DEBT_PRS quality-debt PR(s) already open for <slug>" | ||||||||||||||||||
| # Focus on merging the existing PR instead | ||||||||||||||||||
| fi | ||||||||||||||||||
| ``` | ||||||||||||||||||
|
|
||||||||||||||||||
| **Why 5 files?** A 5-file PR has a ~10% chance of conflicting with another random 5-file PR in a 200-file repo. A 50-file PR has a ~95% chance. The conflict probability scales quadratically with file count — small PRs are exponentially safer. | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Stale quality-debt PR cleanup | ||||||||||||||||||
|
|
||||||||||||||||||
| When the pulse detects quality-debt PRs that have been `CONFLICTING` for 24+ hours, close them with a comment explaining they'll be superseded by smaller, atomic PRs: | ||||||||||||||||||
|
|
||||||||||||||||||
| ```bash | ||||||||||||||||||
| # For each conflicting quality-debt PR older than 24 hours: | ||||||||||||||||||
| gh pr close <number> --repo <slug> \ | ||||||||||||||||||
| -c "Closing — this PR has merge conflicts and touches too many files (blast radius issue, see t1422). The underlying fixes will be re-created as smaller PRs (max 5 files each) to prevent conflict cascades." | ||||||||||||||||||
| ``` | ||||||||||||||||||
|
|
||||||||||||||||||
| After closing, ensure the corresponding issues are relabelled `status:available` so they re-enter the dispatch queue. The next dispatch cycle will create properly-scoped PRs. | ||||||||||||||||||
|
|
||||||||||||||||||
| ### Simplification-debt concurrency cap (10%) | ||||||||||||||||||
|
|
||||||||||||||||||
| Issues labelled `simplification-debt` (created by `/code-simplifier` analysis, approved by a human) represent maintainability improvements that preserve all functionality and knowledge. These are the lowest-priority automated work -- post-deployment nice-to-haves. | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
2>/dev/nullhere suppresses all error messages from theghandjqcommands. This can hide important issues like authentication failures, network problems, or incorrect repository slugs, leading to silent failures that are difficult to debug.This practice violates a general rule for this repository: 'Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.'
Please remove the error suppression. The
gh pr listcommand will gracefully handle cases with no open PRs by returning an empty list, whichjqwill process without error.References