fix(skills): check open PRs for in-flight ADR numbers

[ralphbean]

Summary

• The renumber-adr skill now checks ADR numbers from all open PRs targeting the same base branch, not just the target branch itself. This prevents two PRs from independently picking the same number.
• Extracted the gh pr list + diff pipeline into a helper script (skills/renumber-adr/scripts/inflight-adr-numbers.sh) since the pipeline is non-trivial.
• Fixed step 2's early exit — it no longer stops if the target branch is clear, since open PRs may still claim the same number.

Test plan

• Ran inflight-adr-numbers.sh main — found 7 in-flight ADR numbers across open PRs (0024, 0038, 0044, 0057, 0065, 0066, 0068)
• shellcheck passed via pre-commit hooks

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:50 PM UTC · Completed 2:03 PM UTC
Commit: 448ff7b · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Check open PRs for in-flight ADR number collisions in renumber-adr skill

✨ Enhancement 📝 Documentation 🕐 10-20 Minutes

AI Description

• Check ADR number collisions against target branch and open PRs on the same base.
• Add a helper script to collect in-flight ADR numbers via the GitHub CLI.
• Update the skill flow to keep scanning even when target branch is clear.

Diagram

graph TD
  A["renumber-adr skill"] --> B["Target branch ADRs"] --> E[("Taken ADR numbers")]
  A --> C["Current branch ADRs"] --> E
  A --> D["inflight-adr-numbers.sh"] --> F["gh CLI"] --> G["Open PR diffs"] --> E
  E --> H["Collision check & renumber"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use GraphQL (single request) to fetch PR file paths

• ➕ Fewer gh invocations; faster and less rate-limit sensitive
• ➕ More deterministic than diffing each PR sequentially
• ➖ More complex implementation and harder to maintain/debug
• ➖ Requires GraphQL query plumbing and pagination handling

2. Move collision detection into CI (central gate) instead of a local skill

• ➕ Enforces uniqueness regardless of whether contributors run the skill
• ➕ Produces a single source of truth in the merge pipeline
• ➖ Longer feedback loop for authors (CI-only)
• ➖ Still needs a robust implementation to inspect open PRs safely

Recommendation: The current approach (a small helper script wrapping gh CLI) is the right tradeoff for simplicity and immediate value, given the expected small number of open PRs. If performance/rate limits become an issue, consider the GraphQL-based approach to reduce N+1 diff calls.

Files changed (2) +51 / -9

Enhancement (1) +20 / -0

inflight-adr-numbers.shAdd helper script to list in-flight ADR numbers from open PRs +20/-0

Add helper script to list in-flight ADR numbers from open PRs

• Introduces a Bash script that lists open PRs targeting a base branch, extracts ADR filenames from each PR diff, and outputs sorted, deduplicated four-digit ADR numbers. The script defaults to base branch "main" and produces no output when none are found.

skills/renumber-adr/scripts/inflight-adr-numbers.sh

Documentation (1) +31 / -9

SKILL.mdDocument open-PR ADR collision checks and revised workflow +31/-9

Document open-PR ADR collision checks and revised workflow

• Updates the skill overview and step-by-step process to treat ADR numbers introduced by other open PRs as taken. Removes the early-exit behavior after a clean target-branch check and documents the helper script usage.

skills/renumber-adr/SKILL.md

[github-actions]

Site preview

Preview: https://5b3c2bf1-site.fullsend-ai.workers.dev

Commit: 82f4cc654bf06aa63bcbb79d6ee5628b49bedbca

[qodo-code-review]

Code Review by Qodo

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

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Empty match exits nonzero ✓ Resolved 🐞 Bug ☼ Reliability

Description

inflight-adr-numbers.sh claims it always exits 0, but with set -euo pipefail the pipeline exits
non-zero when grep finds no matches (e.g., no open PRs or PRs without ADR files). This turns the
expected “no output means none” case into a hard failure for callers following SKILL.md.

Code

skills/renumber-adr/scripts/inflight-adr-numbers.sh[R8-20]

+# Exit code is always 0 (no output means no in-flight ADR numbers found).
+
+set -euo pipefail
+
+base="${1:-main}"
+
+gh pr list --base "$base" --state open --json number --jq '.[].number' \
+  | while read -r pr; do
+      gh pr diff "$pr" --name-only 2>/dev/null
+    done \
+  | grep '^docs/ADRs/[0-9]\{4\}-' \
+  | sed 's|docs/ADRs/\([0-9]\{4\}\)-.*|\1|' \
+  | sort -u

Relevance

⭐⭐⭐ High

Team previously accepted fix for set -euo pipefail + grep no-match causing failure; used || true
workaround.

PR-#390

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The script explicitly states it “Exit code is always 0”, but set -euo pipefail plus a pipeline
containing grep will exit non-zero when grep returns 1 for no matches. The skill documentation
also states that “no output” means no in-flight ADR numbers, which requires the script to succeed
with empty output.

skills/renumber-adr/scripts/inflight-adr-numbers.sh[8-20]
skills/renumber-adr/SKILL.md[61-82]

Agent prompt

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

## Issue description
`skills/renumber-adr/scripts/inflight-adr-numbers.sh` documents that it always exits 0, but `set -euo pipefail` combined with a piped `grep` causes exit code 1 when there are no matches (a normal “no in-flight ADRs” case).

## Issue Context
`SKILL.md` instructs that if the script produces no output, there are no in-flight ADR numbers; this implies success with empty output.

## Fix Focus Areas
- skills/renumber-adr/scripts/inflight-adr-numbers.sh[8-20]

## Suggested change
Ensure the script returns success on empty match by neutralizing `grep`’s exit-1-on-no-match behavior (and optionally other pipeline failures if you want to keep the “always 0” contract), e.g.:
- append `|| true` after the `grep` stage or at the end of the whole pipeline
- or replace with `grep ... || :`
- or restructure to conditionally run `grep`/post-processing only when there is input

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

---

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review — comment

PR: #3610 — fix(skills): check open PRs for in-flight ADR numbers
Scope: 2 files changed (62 additions, 9 deletions) — skills/renumber-adr/SKILL.md, skills/renumber-adr/scripts/inflight-adr-numbers.sh
Re-review: Prior review on 448ff7b raised 6 findings (3 medium, 1 medium, 2 low). All have been addressed in the follow-up commit 82f4cc6. Nice work.

Prior findings — resolved

#	Prior finding	Status
1	Self-collision — script includes current PR (medium)	✅ Fixed: exclude-pr parameter added
2	Exit-code contract — grep no-match exits non-zero (medium)	✅ Fixed: { grep ... || true; }
3	set -e terminates on gh pr diff failure (medium)	✅ Fixed: || true after gh pr diff
4	PR title uses feat instead of fix (medium)	✅ Fixed: title changed to fix(skills)
5	Reports modifications, not just additions (low)	✅ Addressed: documented as intentionally conservative
6	Missing # Requires: header (low)	✅ Fixed: header added

New findings

1. gh pr list default limit of 30 may miss open PRs · medium

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 22

gh pr list defaults to returning at most 30 results. If the repository has more than 30 open PRs targeting the base branch, PRs beyond the first 30 are silently ignored — their ADR numbers won't be detected. The failure mode is silent: the script exits 0 and simply omits data.

Practical impact is bounded — before this PR, no open PRs were checked at all, so this is strictly additive. But the fix is trivial and would make the feature robust as the repo scales.

Remediation: Add -L 999 (or a suitably large value) to the gh pr list command:

gh pr list --base "$base" --state open -L 999 --json number --jq '.[].number'

---

2. Unguarded gh pr list failure breaks exit-code contract · medium

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 22

With set -euo pipefail, if gh pr list fails (network error, auth failure, rate limit), the pipeline exits non-zero and the script terminates with a non-zero exit code. The script header (line 9) promises "Exit code is always 0" and the SKILL.md tells callers to treat empty output as "no in-flight numbers" — but a gh pr list failure violates both promises.

The gh pr diff call (line 25) was correctly guarded with || true in the prior review round, but gh pr list was not.

Remediation: Either guard with || true to match the stated contract, or update the contract to document that non-zero exit indicates a GitHub API failure:

gh pr list --base "$base" --state open -L 999 --json number --jq '.[].number' 2>/dev/null || true

---

3. Frontmatter description not updated · low

File: skills/renumber-adr/SKILL.md · lines 3-6

The frontmatter description still reads "conflict with ADRs already on the PR's target branch" without mentioning open PRs. The overview (lines 13-15) was updated, but the frontmatter was not, creating a minor inconsistency between what the skill advertises and what it does.

Remediation: Update the frontmatter:

description: >-
  Check whether ADR numbers in the current branch conflict with ADRs on
  the PR's target branch or in other open PRs, and renumber if needed.

---

4. writing-adrs skill has overlapping manual open-PR check · low

File: skills/writing-adrs/SKILL.md · line 125-127

The writing-adrs skill instructs agents to manually "scan open pull requests for new docs/ADRs/NNNN-*.md files so you do not collide with in-flight ADRs" when picking an ADR number. This is complementary (proactive during authoring vs. reactive pre-merge verification), but could be streamlined to reference the new inflight-adr-numbers.sh helper for consistency.

No action required — noting for awareness.

---

Security

No security findings. All shell variables are properly double-quoted, inputs flow only to gh CLI flags or [ test comparisons (neither interprets shell metacharacters), and output is filtered to strictly four-digit numeric strings via grep and sed.

Architecture & conventions

The script follows all established conventions: shebang (#!/usr/bin/env bash), set -euo pipefail, kebab-case naming, detailed header comments with Usage/Requires sections, defensive || true for optional pipeline stages. The skill integration pattern (script in skills/*/scripts/, invoked via bash in SKILL.md) matches existing skills (e2e-health, merge-queue, topissues).

PR title fix(skills) is correct per COMMITS.md — this corrects a defect where concurrent ADR collisions were missed.

---

Labels: PR modifies skill definitions and adds a helper script under skills/renumber-adr/

Previous run

Review — request-changes

PR: #3610 — feat(skills): check open PRs for in-flight ADR numbers
Scope: 2 files changed (51 additions, 9 deletions) — skills/renumber-adr/SKILL.md, skills/renumber-adr/scripts/inflight-adr-numbers.sh

Summary

This PR extends the renumber-adr skill to check ADR numbers from other open PRs, preventing concurrent number collisions. It adds a helper script (inflight-adr-numbers.sh) and restructures step 3 to incorporate in-flight ADR numbers from three sources. The approach is sound — checking open PRs is the right fix for concurrent ADR collision. However, the helper script has three functional bugs (self-inclusion, exit-code contract violation, and early loop termination) that together make it unreliable, and the PR title uses the wrong conventional commit prefix.

---

Findings

1. Script includes the current PR in its output — false self-collisions on every invocation · medium

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 14

gh pr list --base "$base" --state open returns all open PRs targeting the base branch, including the current PR. The SKILL.md correctly describes source 3 as "ADR files introduced by other open PRs," but the script has no mechanism to exclude the current PR.

When the skill runs on a PR that adds ADR 0069:

• Source 3 (script output) includes 0069 from the current PR
• The collision check finds 0069 in the "taken" set
• The skill renumbers 0069 unnecessarily

This triggers on every invocation, causing unnecessary renumbering, extra commits, and ADR number inflation.

Remediation: Accept a second argument for the current PR number and exclude it. For example:

current_pr="${2:-}"
gh pr list --base "$base" --state open --json number --jq '.[].number' \
  | while read -r pr; do
      [ "$pr" = "$current_pr" ] && continue
      gh pr diff "$pr" --name-only 2>/dev/null || true
    done \
  ...

Update SKILL.md step 3 to pass the current PR number.

---

2. Exit-code contract violation — grep no-match exits non-zero · medium

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 17

The script header documents: "Exit code is always 0 (no output means no in-flight ADR numbers found)." This is false. With set -euo pipefail, when grep on line 17 matches nothing (no open PR touches an ADR file), it returns exit code 1. Because pipefail is set, this propagates as the pipeline's exit status. Because set -e is set, the script terminates with exit code 1.

The SKILL.md tells the caller "If it produces no output, there are no in-flight ADR numbers" — but the caller also receives a non-zero exit code, which could cause the agent to interpret the invocation as a failure.

Remediation: Append || true to the grep stage:

  | grep '^docs/ADRs/[0-9]\{4\}-' || true \

Or use { grep ... || true; } to ensure the pipeline always succeeds when there are no matches.

---

3. set -e terminates while loop on transient gh pr diff failure · medium

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 16

If gh pr diff "$pr" --name-only fails for any PR (closed between list and diff, rate-limited, permissions error), the 2>/dev/null only suppresses stderr — the non-zero exit code still propagates. Under set -e, this terminates the while-loop subshell immediately, silently skipping all remaining PRs. The script produces incomplete results with no indication of the failure.

Remediation: Add || true after gh pr diff:

gh pr diff "$pr" --name-only 2>/dev/null || true

---

4. PR title uses feat — should be fix per COMMITS.md · medium

File: skills/renumber-adr/SKILL.md

Per COMMITS.md, feat is reserved for user-facing features. This change improves an internal agent capability (the renumber-adr skill's collision detection). The COMMITS.md guidance states: "Tightening internal heuristics, adjusting prompts, or tuning agent behavior that users don't directly control → refactor or fix depending on whether it corrects a defect."

The PR description says "prevents two PRs from independently picking the same number" and "Fixed step 2's early exit" — this corrects a defect where collisions were missed.

Remediation: Change PR title from feat(skills): ... to fix(skills): ....

---

5. gh pr diff --name-only reports modifications, not just additions · low

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh · line 16

gh pr diff --name-only lists all changed files regardless of change type (added, modified, deleted). If another open PR merely edits an existing ADR (e.g., fixing a typo in 0042-...), that number appears in the "in-flight" set. Since existing ADR numbers are already captured by source 1 (target branch files), this adds false positives. The behavior is conservative (overly strict, not permissive), so impact is limited to unnecessary renumbering in edge cases.

Remediation: Consider filtering to added-only files by parsing the full diff for +++ b/docs/ADRs/ where --- a/ is /dev/null, or document this as intentionally conservative behavior.

---

6. Missing # Requires: header · low

File: skills/renumber-adr/scripts/inflight-adr-numbers.sh

Existing scripts in skills/*/scripts/ consistently document dependencies with a # Requires: ... line. This script depends on gh CLI (authenticated) but does not document it.

Remediation: Add # Requires: gh CLI (authenticated). to the header comments.

---

Labels: PR modifies skill definitions under skills/renumber-adr/

[fullsend-ai-review]

🤖 Finished Review · ❌ Failure · Started 2:48 PM UTC · Completed 2:59 PM UTC
Commit: 82f4cc6 · View workflow run →

Superseded by updated review

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:27 PM UTC · Completed 7:34 PM UTC
Commit: 82f4cc6 · View workflow run →

[fullsend-ai-retro]

PR #3610 added an open-PR collision check to the renumber-adr skill. The review agent performed well: it found 6 legitimate bugs on the initial commit (3 medium-severity correctness issues), all fixed by the author in a follow-up commit. On re-review, it found 2 more medium-severity issues but submitted with COMMENT verdict — the PR merged without addressing them. A notable workflow gap: human reviewer approved the PR 2 minutes before the review agent completed its first analysis, approving code with 3 material bugs. The review agent again omitted a protected-path finding for the skills/ directory (additional evidence for #3710). The re-review run showed Failure status despite posting valid findings (additional evidence for #3548). Qodo and the fullsend review agent independently found the same grep exit-code bug, with fullsend providing significantly deeper coverage overall.

Proposals filed

• Evidence for #2940 and #3710: re-review used COMMENT verdict for medium findings (merged unfixed) and review omitted protected-path finding for skills/ on PR #3610 (in fullsend-ai/fullsend)
• Remaining unfixed bugs in inflight-adr-numbers.sh: gh pr list pagination limit and unguarded failure (in fullsend-ai/fullsend)