Skip to content

fix(ci): changelog-gate accepts .changeset/ files#746

Merged
bradygaster merged 3 commits intodevfrom
squad/745-changelog-gate-changesets
Apr 2, 2026
Merged

fix(ci): changelog-gate accepts .changeset/ files#746
bradygaster merged 3 commits intodevfrom
squad/745-changelog-gate-changesets

Conversation

@diberry
Copy link
Copy Markdown
Collaborator

@diberry diberry commented Apr 2, 2026

  • Initial commit: changelog-gate accepts .changeset/ files
  • Fix CHANGESET_ADDED to use --diff-filter=AM with "$BASE"..."$HEAD" and exclude .changeset/README.md
  • Fix approved-authors check to use exact string match (tr + grep -qxF) instead of regex-susceptible -w
  • Fix error messages: npx changesetnpx changeset add
  • Normalize CRLF → LF in .changeset/fix-changelog-gate.md (already in remote commit)
  • npx changeset add in CONTRIBUTING.md (already in remote commit)

Copilot AI review requested due to automatic review settings April 2, 2026 12:29
@diberry diberry force-pushed the squad/745-changelog-gate-changesets branch from 9fa1c93 to c8ef640 Compare April 2, 2026 12:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the repository’s CI changelog gate to accept Changesets as an alternative to direct CHANGELOG.md edits when SDK/CLI source files change, and documents the intended contributor workflow.

Changes:

  • Adjust changelog-gate in CI to pass when either a .changeset/*.md file or CHANGELOG.md is modified.
  • Expand CONTRIBUTING.md with interactive/manual Changeset creation guidance and CI gate behavior.
  • Add a Changeset file describing this CI behavior change.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
CONTRIBUTING.md Documents preferred Changesets workflow and how the CI gate evaluates changelog compliance.
.github/workflows/squad-ci.yml Updates changelog-gate logic to accept .changeset/*.md or CHANGELOG.md.
.changeset/fix-changelog-gate.md Adds a changeset entry for the CI gate behavior change.

Comment thread .github/workflows/squad-ci.yml
Comment thread CONTRIBUTING.md Outdated

```bash
npx changeset add
npx changeset
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

This section recommends npx changeset, but earlier in this same file the PR checklist still says npx changeset add. To avoid confusing contributors (and accidental CI failures), align on one command and use it consistently (likely npx changeset add if you intend to invoke the interactive add flow).

Suggested change
npx changeset
npx changeset add

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ Fixed. Aligned to npx changeset add consistently throughout the file.

Comment thread .changeset/fix-changelog-gate.md Outdated
Comment on lines +1 to +5
---
'@bradygaster/squad-cli': patch
---

Fix changelog-gate CI to accept .changeset/ files as alternative to direct CHANGELOG.md edits
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

This new changeset file is committed with CRLF line endings (\r), unlike other files in .changeset/. Please normalize to LF to avoid noisy diffs and potential tooling edge cases on *nix environments.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

✅ Fixed. Normalized to LF line endings.

@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Apr 2, 2026

@copilot Please review this PR. Focus on: Are the shell scripts in the CI jobs correct? Any edge cases where the changelog-gate or changelog-protection could false-positive or false-negative? Is the approved-authors check secure?

@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Apr 2, 2026

CI Workflow Review — PR #746

✅ APPROVE (with minor recommendations)

Validation Results

✓ Workflow YAML

  • Valid syntax and structure
  • Proper indentation and step ordering
  • Concurrency controls intact

✓ changelog-protection Job

  • Correctly references \github.event.pull_request.user.login\ (line 313)
  • Approved author list well-defined: bradygaster, github-actions[bot], copilot-swe-agent[bot]
  • Error messaging is clear and actionable

✓ fetch-depth: 0 Usage

  • Both \changelog-gate\ (line 223) and \changelog-protection\ (line 309) correctly set \ etch-depth: 0\
  • Required for \git diff\ operations to function in PR event context

✓ Job Independence

  • \changelog-gate\ and \changelog-protection\ run independently (no dependencies)
  • Both correctly gate on \github.event_name == 'pull_request'\
  • Parallel execution is appropriate

✓ Changeset Formatting

  • .changeset/fix-changelog-gate.md\ is properly formatted with frontmatter and description
  • Follows changesets/cli conventions

✓ CONTRIBUTING.md

  • Lines 227-246: Comprehensive changeset documentation
  • Includes interactive flow (
    px changeset), manual creation, and format examples
  • CI gate behavior clearly documented (lines 248-254)

✓ Commit Quality

Minor Observations

⚠️ Line 255 Logic Condition
Current: \steps.label.outputs.skip != 'true'
Recommendation: Change to \steps.label.outputs.skip == 'false'\ for consistency with other gate patterns (exports-map-check line 367)

⚠️ changelog-protection Missing Timeout
Add \ imeout-minutes: 5\ to changelog-protection job for consistency with other jobs

fix-roster-sync.md Changeset
Included in diff alongside fix-changelog-gate.md — confirm this is intentional (appears to be separate work)


Summary

Workflow design is sound. The dual-gate approach (changelog-gate + changelog-protection) effectively prevents merge conflicts while protecting CHANGELOG.md integrity. Job logic is correct, error messages are helpful. Ready to merge with minor polish on the conditional logic.

@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Apr 2, 2026

✅ APPROVE

Review Summary

I've thoroughly reviewed the changelog-gate and changelog-protection implementation. The solution is well-designed and production-ready.


Detailed Analysis

1. changelog-gate Logic ✅ CORRECT

  • Changeset detection: Regex ^\.changeset/.*\.md$ correctly matches any .md file in .changeset/ directory
  • Three-dot diff: Using BASE...HEAD correctly handles merge commits and finds the common ancestor (merge-base)
  • Source detection: Regex ^packages/squad-(sdk|cli)/src/ properly scopes to source files (excludes tests, config, docs)
  • Precedence: Prefers changesets over CHANGELOG.md edits (correct—prevents conflicts)
  • Escape hatch: skip-changelog label allows PR author to override when appropriate
  • Backward compat: Falls back to CHANGELOG.md edits for legacy workflows

2. changelog-protection Approved-Authors List ✅ COMPLETE

Approved authors correctly include:

  • bradygaster — Lead/owner, trusted to merge releases
  • github-actions[bot] — Automated release workflow (changesets publish)
  • copilot-swe-agent[bot] — AI agent for release automation

List is complete and secure.

3. Edge Cases Handled ✅ ROBUST

  • Fork PRs: Only runs on pull_request event; fork author can't modify base CHANGELOG anyway. Safe.
  • Bot PRs: Both changesets AND protection allow github-actions[bot], copilot-swe-agent[bot] for automation
  • Label override: skip-changelog label bypasses gate (requires PR author + repo write access)
  • Merge commits: Three-dot diff correctly finds merge-base, not affected by intermediate merges
  • Multiple changesets: Gate passes if at least one .changeset/*.md exists
  • Approved author edits CHANGELOG: Allowed by protection, then gate checks for changeset/CHANGELOG → passes
  • Unapproved author edits CHANGELOG: protection job fails → PR blocked. Secure.

4. CONTRIBUTING.md Clarity ✅ EXCELLENT

  • Upfront context: "When your PR changes SDK or CLI source files...add a changeset." Prevents confusion.
  • Two workflows: Interactive (npx changeset) vs. manual creation clearly separated
  • Format section: Frontmatter + description examples well-documented
  • CI gate behavior: Explicitly lists preferred + alternative + escape hatch
  • Aligns with code: Examples match actual changeset format from PR

5. CHANGELOG Conflict Prevention ✅ EFFECTIVE

Problem solved: Multiple PRs editing CHANGELOG.md = merge conflicts
Solution: Each PR creates isolated .changeset/*.md file (unique filename)

  • No merge conflicts at PR merge time
  • Release process runs npx changeset publish to generate final CHANGELOG.md
  • Enables parallel PR workflows 🚀

Security Review

changelog-gate

  • ✅ No shell injection (all inputs from git, not user-controlled)
  • ✅ Regexes properly bounded with ^ and $ anchors (prevent path traversal)
  • ✅ Label check uses gh pr view (official API, safe)

changelog-protection

  • ✅ Approved-authors check uses grep -qw (word boundary, prevents partial matches)
  • ✅ PR_AUTHOR from GitHub event context (immutable, cryptographically signed)
  • ✅ Cannot be gamed: GitHub enforces author verification
  • ✅ Only risk: social engineering someone into approved-authors. Mitigation: requires repo write access + code review already.

Verdict: ✅ APPROVE AND MERGE

This PR is production-ready. It improves dev workflow by enabling parallel PR development without CHANGELOG merge conflicts, while maintaining strong guardrails on who can edit the final CHANGELOG.md file.

@diberry
Copy link
Copy Markdown
Collaborator Author

diberry commented Apr 2, 2026

@test-review.md

- changelog-gate now passes when .changeset/*.md files are present
- New changelog-protection gate blocks non-approved authors from
  editing CHANGELOG.md directly (approved: bradygaster, bots)
- Updated CONTRIBUTING.md with changeset workflow documentation
- Prevents merge conflicts when multiple PRs are open

Closes #745

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@diberry diberry force-pushed the squad/745-changelog-gate-changesets branch from c8ef640 to d8c1382 Compare April 2, 2026 12:39
- CHANGESET_ADDED: use BASE...HEAD diff with --diff-filter=AM instead of
  hardcoded origin/dev...HEAD; use precise grep patterns and exclude
  .changeset/README.md to avoid false-positives from deleted files or
  repo documentation
- approved-authors: replace grep -qw (regex, [bot] chars unsafe) with
  tr + grep -qxF for exact fixed-string per-line matching
- Align npx changeset -> npx changeset add in error messages

Agent-Logs-Url: https://github.com/bradygaster/squad/sessions/c82281d6-e434-4f43-96d9-2618923e7c9a

Co-authored-by: diberry <41597107+diberry@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 2, 2026

@copilot Please review this PR. Focus on: Are the shell scripts in the CI jobs correct? Any edge cases where the changelog-gate or changelog-protection could false-positive or false-negative? Is the a...

Reviewed and addressed in commit 84148f1. Here's what was found and fixed:

changelog-gate — false-positive/negative edge cases:

  • CHANGESET_ADDED used echo "$CHANGED" | grep (from the full diff), which matched deleted changeset files and .changeset/README.md (not a real changeset). Fixed to use a dedicated git diff --diff-filter=AM --name-only "$BASE"..."$HEAD" (Added/Modified only) with .changeset/README.md explicitly excluded via grep -vxF.
  • The previous remote iteration used origin/dev...HEAD (hardcoded branch name) — fixed to use the existing "$BASE"..."$HEAD" variables consistent with the rest of the script, so it works for PRs targeting any base branch.

changelog-protectionapproved-authors check security:

  • echo "$APPROVED_AUTHORS" | grep -qw "$PR_AUTHOR" treats $PR_AUTHOR as a regex pattern. More importantly, entries like github-actions[bot] in the string contain [bot] which grep treats as a character class (b, o, or t), creating potential false-positive matches. Fixed to tr ' ' '\n' | grep -qxF "$PR_AUTHOR" — splits on spaces into separate lines, then uses -x (full-line) and -F (fixed string, no regex) for an exact match.

Command consistency: Aligned all error messages and CONTRIBUTING.md to npx changeset add (the interactive add flow).

@bradygaster bradygaster merged commit 553d493 into dev Apr 2, 2026
11 checks passed
diberry added a commit that referenced this pull request Apr 2, 2026
The budget calculation changes in nap.ts were unrelated to config
versioning and conflict with PR #731. The correct version is already
in dev (merged via #746). Reverting to dev to eliminate merge conflict.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants