Repository navigation
feat(pro): GitHub merge-queue support for describe affected --upload - #2395
Conversation
Allow `atmos describe affected --upload` under GITHUB_EVENT_NAME=merge_group, resolve the diff base from event.merge_group.base_sha, and document the full settings.pro event schema (pull_request, release, drift_detection, merge_group) in the configuration reference. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds GitHub merge queue (merge_group) support: CLI uploads accept merge_group, ResolveBase extracts base_sha/head_sha/base_ref with fallbacks, upload validation uses ErrUploadRequiresSupportedEvent, and settings.pro.merge_group is preserved in upload payloads. ChangesGitHub Merge Queue Support
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/exec/describe_affected.go`:
- Around line 442-445: The error message constructed in the fmt.Errorf call
inside describe_affected.go (around the block that references args.CIEventType
and errUtils.ErrUploadRequiresSupportedEvent) omits "pull_request_target" even
though it is accepted elsewhere; update the error string to list "pull_request",
"pull_request_target", and "merge_group" as the supported events so the message
matches the accepted event set and the subsequent WithHint texts.
In `@pkg/ci/providers/github/base.go`:
- Around line 331-338: The current fallback only uses environment
GITHUB_BASE_REF; update the post-resolution logic so that if targetBranch is
empty but the webhook payload contains merge_group.base_ref, you set
res.TargetBranch from that payload. Specifically, after calling
resolveFromBaseRef(eventMergeGroup) check if targetBranch == "" and if
eventMergeGroup.MergeGroup.BaseRef (merge_group.base_ref) is non-empty, then
assign res.TargetBranch = eventMergeGroup.MergeGroup.BaseRef; keep the existing
HeadSHA override logic (res.HeadSHA = headSHA) intact.
In `@website/docs/cli/configuration/settings/pro.mdx`:
- Line 353: Replace the literal placeholder "X.Y.Z" with the actual released
Atmos CLI version in both occurrences in the prose: the Note block that begins
"**Prerequisites.** Full merge-queue support requires **Atmos CLI ≥ X.Y.Z**" and
the later migration note that also contains "X.Y.Z"; remove or update the inline
{/* TODO: ... */} comment so both occurrences are replaced (or guarded)
atomically when the release tag is available, ensuring the rendered docs no
longer show the placeholder.
In `@website/src/data/roadmap.js`:
- Line 376: The roadmap entry object with label 'GitHub merge queue support for
affected uploads' in website/src/data/roadmap.js is missing the required pr
field; add pr: 2395 to that object (the same object that has keys label, status,
quarter, changelog, docs, description, benefits) so the milestone follows the
guideline requiring a PR reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ba3b5bb-b172-4455-b190-22c18d4b9ba4
📒 Files selected for processing (9)
errors/errors.gointernal/exec/describe_affected.gointernal/exec/describe_affected_test.gointernal/exec/describe_affected_upload_test.gopkg/ci/providers/github/base.gopkg/ci/providers/github/base_test.gowebsite/blog/2026-05-07-pro-merge-queue-support.mdxwebsite/docs/cli/configuration/settings/pro.mdxwebsite/src/data/roadmap.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2395 +/- ##
==========================================
+ Coverage 78.12% 78.16% +0.03%
==========================================
Files 1098 1098
Lines 103630 103674 +44
==========================================
+ Hits 80966 81039 +73
+ Misses 18213 18182 -31
- Partials 4451 4453 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Include pull_request_target in upload-event error message - Promote merge_group.base_ref payload value when GITHUB_BASE_REF is empty - Replace X.Y.Z placeholder with concrete CLI version (1.218.0) - Add missing pr reference to merge-queue roadmap milestone Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
6732def
…o-config Workflow filenames in settings.pro are arbitrary, so Atmos Pro cannot tell from a filename alone whether a workflow plans or applies. Recommending the pull_request.synchronize fallback as "zero-config" can silently dispatch the wrong workflow on a merge-queue synthetic commit. Reframe the merge-queue docs and blog post around configuring a merge_group.checks_requested.workflows block explicitly. Keep the pull_request.synchronize backstop documented as transitional behavior so existing customers do not regress, but no longer present it as the recommended path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7425213
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/blog/2026-05-07-pro-merge-queue-support.mdx (1)
14-14: 💤 Low valueConsider using typographic quotation marks.
The straight quotes around "Atmos Pro" could be replaced with typographic quotes for better typography.
✨ Proposed typography improvement
-Until now, the CLI rejected `merge_group` events at the `--upload` boundary, and Atmos Pro had no way to correlate the synthetic merge-queue commits with stack-affected results. Required "Atmos Pro" checks would stay in **Expected — Waiting for status to be reported** until a delayed reconciler swept them. +Until now, the CLI rejected `merge_group` events at the `--upload` boundary, and Atmos Pro had no way to correlate the synthetic merge-queue commits with stack-affected results. Required "Atmos Pro" checks would stay in **Expected — Waiting for status to be reported** until a delayed reconciler swept them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/blog/2026-05-07-pro-merge-queue-support.mdx` at line 14, Replace the straight double quotes around the phrase "Atmos Pro" in the blog copy with typographic (curly) quotation marks; locate the sentence containing 'Required "Atmos Pro" checks would stay in **Expected — Waiting for status to be reported**' and change the straight quotes around Atmos Pro to opening and closing typographic quotes so it reads with proper typesetting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/blog/2026-05-07-pro-merge-queue-support.mdx`:
- Line 14: Replace the straight double quotes around the phrase "Atmos Pro" in
the blog copy with typographic (curly) quotation marks; locate the sentence
containing 'Required "Atmos Pro" checks would stay in **Expected — Waiting for
status to be reported**' and change the straight quotes around Atmos Pro to
opening and closing typographic quotes so it reads with proper typesetting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c17c15f4-db46-4933-b096-5d414a7c3636
📒 Files selected for processing (2)
website/blog/2026-05-07-pro-merge-queue-support.mdxwebsite/docs/cli/configuration/settings/pro.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- website/docs/cli/configuration/settings/pro.mdx
|
Warning Release Documentation RequiredThis PR is labeled
|
|
These changes were released in v1.218.0-rc.2. |
TestSetDescribeAffectedFlagValueInCliArgs_BaseResolution's "CI auto-detect when enabled and no explicit base" subtest cleared GITHUB_ACTIONS/CI but not GITHUB_EVENT_PATH. When this test suite runs locally or under a regular pull_request CI trigger, no merge_group event file is present, so resolveMergeGroupBase falls back to the simulated GITHUB_BASE_REF the subtest sets, and the test passes. But when this test actually runs inside a real merge_group-triggered workflow (i.e. a PR going through the GitHub merge queue), the runner's real GITHUB_EVENT_PATH points at a genuine merge_group event payload with a real base_sha -- and resolveMergeGroupBase correctly prefers that real payload data over GITHUB_BASE_REF, per its documented fallback order. The subtest never accounted for this, so it fails with an empty Ref field specifically -- and only -- in real merge-queue runs, not locally or in regular PR CI. Reproduced locally by pointing GITHUB_EVENT_PATH at a synthetic merge_group payload; fixed by explicitly clearing GITHUB_EVENT_PATH so the subtest is hermetic regardless of the ambient CI context it happens to execute in. This was pre-existing on main (from PR #2395) and surfaced when PR #2903 hit the merge queue for the first time; not a regression from this branch's own changes, but fixed here directly per repo convention for CI-blocking issues found on an open PR's branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ai): close DX gaps found in atmos ai field test Fixes 16 findings from a hands-on field test of `atmos ai` commands: skill install/uninstall: - Enforce the compatibility.atmos version gate for bundled and multi-skill Git package installs, not just single-skill Git clones - Reject unrecognized --client values instead of silently no-op'ing (extends pkg/flags WithValidValues to string-slice flags generically) - Warn when --path is combined with --client/--scope/--global, since --path skips auto-distribution and those flags are otherwise ignored - Give the registry-corruption error an actionable hint via the error-builder pattern - Show a skill's minimum required Atmos version in `skill list --detailed` and flag when an installed skill has a newer catalog version available - Fix `agent-skills/skills/atmos-ai/SKILL.md` doc drift (never documented `skill install`) and remove a phantom `info` subcommand from --help - Add local-path/file:// support to skill source parsing and the downloader - Document --scope/--global precedence ask/exec/sessions/MCP: - Make `exec`/`ask --session` actually persist and resume conversations (previously a complete no-op despite being a documented flag) - Resolve a session's Model from the constructed AI client instead of a raw config lookup, fixing sessions export/import for the default zero-config claude-code provider path - Apply --mcp server filtering for CLI providers too (was silently ignored, all configured servers were always passed through) - Reject invalid --format values instead of silently falling back to text Two intentional behavior changes: - `ai exec` can now return exit code 2 for a genuine infrastructure-level tool failure (e.g. an unregistered tool) without waiting on the 25-iteration tool-call loop to exhaust - `sessions clean --older-than 0d` now deletes all sessions immediately, distinguished from the flag not being passed at all (which still defaults to 30 days); negative durations are now a hard parse error instead of silently falling back to the default Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 7 Dependabot alerts in website deps Bumps pnpm overrides for two transitive website dependencies to their patched versions: - js-yaml 3.15.0 -> 3.15.1, 4.3.0 -> 4.3.1 (GHSA-5p4m-2wfm-xmqj, quadratic CPU consumption in !!omap resolution, high severity, alerts cloudposse#268/cloudposse#269) - mermaid 11.16.0 -> 11.16.1 (GHSA-rhh3-jpg6-66xh, GHSA-c4c3-pg64-4m4v, GHSA-6x64-9x62-f2gx, GHSA-3rrr-jr9j-h3q3, GHSA-2v8p-3f2j-5mp7, alerts cloudposse#263-cloudposse#267) Both are within-major-version patches (not blocked by dependabot.yml's major-bump ignore policy). No CodeQL alerts were open. Verified with a full `pnpm run build` in website/ — succeeds, no new broken links. NOTICE is unchanged (no license-set change from these bumps). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): fix flaky CI assertion in session export warning test TestManager_ExportSession_WarnsOnUnimportableCheckpoint asserted on the raw ui.Warning() output, including ANSI styling. Under CI=true (the real GitHub Actions env, confirmed by reproducing locally with CI=true) the formatter emits the message across two adjacent styled runs, which split the literal substring "not be re-importable" the test was checking for -- the visible text was identical, only the styling boundary differed from a local run. Passed locally, failed in CI. Strip ANSI codes and collapse whitespace before asserting on captured UI output, so the test checks content, not rendering byte-layout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(ai): add `atmos ai skill update` to refresh outdated bundled skills Closes the gap from finding cloudposse#11 of the field-test fix pass: bundled skills are static copies made at install time, so upgrading the atmos binary alone never refreshed a skill you'd already installed, even when the new release shipped improved skill content. `atmos ai skill list --detailed` already surfaced an "update available" hint; nothing acted on it. `atmos ai skill update [name]` compares each installed bundled skill's recorded version against the catalog embedded in the running binary and reinstalls only the ones that are actually outdated: - With no <name>, updates every installed bundled skill that has a newer version available (a single confirmation, not one per skill). Skills already current are left untouched and reported separately, so it's safe to run repeatedly. - An outdated skill is reinstalled the same way `install <name> --force` would install it, reusing all the same client-distribution/scope flags and logic (--client, --all-clients, --scope, --global, --path). - Skills installed from GitHub aren't covered yet (no cheap way to check their upstream version without a fetch); update returns a clear error pointing at `install <source> --force` as the manual path for those. Both `atmos ai skill list`'s "update available" indicator and the new `update` command now share one `marketplace.SkillVersionOutdated` helper instead of duplicating the comparison, so they can never disagree. Docs, blog post, and roadmap updated; the `atmos ai skill --help` screengrab regenerated (--filter'd to just that one cast). Relabeling this PR patch -> minor since this is new user-visible functionality, which requires both per this repo's release-doc policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(fixes): add fix records for this session's four PR changes Per the fix-log skill: one record each for the 16-finding atmos ai field-test fix pass, the website Dependabot remediation, the flaky CI test-assertion fix, and the new atmos ai skill update command. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(flags): validate env-sourced flag values, not just CLI-set ones ValidateFlagValues only checked cmd.Flags().Changed(), so an invalid value supplied purely via a bound environment variable (e.g. ATMOS_AI_SKILL_CLIENT=bogus) silently bypassed validation -- a skill install/update would report success while distributing to zero clients. Now validates whenever Viper reports the value as actually set (CLI or env), not just CLI-changed. Found via a field-test pass on the atmos ai skill update / pkg/flags changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): surface batch failures in `atmos ai skill update`/install UpdateAllBundled/InstallAllBundled's batch path logged a warning per failed skill but always returned nil, so a real per-skill failure (e.g. a reinstall that can't remove the existing install) was reported as overall success with exit code 0 -- the only signal was an easily-missed log line, invisible entirely at logs.level: Error. Adds a distinct outcomeRejected for expected compatibility/structure skips (already covered by an existing test), keeping it separate from genuine outcomeFailed so only real failures propagate an error. Found via a field-test pass on the atmos ai skill update changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): resolve anonymous chat session Model via client.GetModel() The anonymous-session branch (no --session given) still called getModelFromConfig -- a raw config lookup with no default fallback -- instead of client.GetModel(), unlike the named-session branch fixed in a prior commit. A zero-config claude-code chat with no --session got a blank Model on its session record, breaking re-import (which requires a non-empty Model). Found via a field-test pass on the atmos ai session DX fixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): warn when --session is ignored because sessions are disabled prepareSession returned (nil, nil) identically for "no --session given" and "--session given but ai.sessions.enabled is false", so a user could run `exec --session foo` across multiple invocations believing conversation context was being carried over, with nothing persisted and no indication why -- only discoverable indirectly via `sessions list` erroring "sessions are not enabled". Found via a field-test pass on the atmos ai session DX fixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ai): fix concurrent session storage access failing with SQLITE_BUSY Two processes opening the same new session database at once (e.g. concurrent `atmos ai exec --session <same-name>` invocations) could crash with SQLITE_BUSY: PRAGMA journal_mode = WAL briefly needs exclusive access to switch modes, and busy_timeout was set after it in the pragma list, so a connection that lost the race had no busy timeout in effect yet. Reorders busy_timeout first and adds a bounded retry loop around pragma execution, since the pure-Go sqlite driver (modernc.org/sqlite) doesn't consistently honor busy_timeout for that specific one-time mode switch. Found via a field-test pass on the atmos ai session DX fixes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 5 Dependabot alerts (go-git, dompurify, nanoid) - github.com/go-git/go-git/v5: 5.19.1 -> 5.19.2, fixing a symlink traversal in worktree operations (GHSA-hc8v-wwc9-vgxm, high) and a path traversal in reference name handling (GHSA-qgq7-7hm3-q39j, medium). Alerts cloudposse#270, cloudposse#271. - dompurify (website, transitive via pnpm override): 3.4.12 -> 3.4.13, fixing an IN_PLACE sanitization XSS via detached subtree hook removal (GHSA-55q2-fjhq-7xh7, medium). Alert cloudposse#272. - nanoid (website, transitive via pnpm override, both the direct override target and postcss's own dependency): 3.3.15/3.3.16 -> 3.3.18, fixing two infinite-loop DoS issues with negative/zero size (GHSA-28wg-ghj8-5hjv, GHSA-2v37-7h3g-55p8, high). Alerts cloudposse#273, cloudposse#274. Not fixed: cloudposse#275/cloudposse#276 (npm image-size <= 2.0.2, high) -- no patched version exists yet upstream (first_patched_version is null on both advisories); nothing to bump to. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): fix flaky merge-queue failure in describe-affected base test TestSetDescribeAffectedFlagValueInCliArgs_BaseResolution's "CI auto-detect when enabled and no explicit base" subtest cleared GITHUB_ACTIONS/CI but not GITHUB_EVENT_PATH. When this test suite runs locally or under a regular pull_request CI trigger, no merge_group event file is present, so resolveMergeGroupBase falls back to the simulated GITHUB_BASE_REF the subtest sets, and the test passes. But when this test actually runs inside a real merge_group-triggered workflow (i.e. a PR going through the GitHub merge queue), the runner's real GITHUB_EVENT_PATH points at a genuine merge_group event payload with a real base_sha -- and resolveMergeGroupBase correctly prefers that real payload data over GITHUB_BASE_REF, per its documented fallback order. The subtest never accounted for this, so it fails with an empty Ref field specifically -- and only -- in real merge-queue runs, not locally or in regular PR CI. Reproduced locally by pointing GITHUB_EVENT_PATH at a synthetic merge_group payload; fixed by explicitly clearing GITHUB_EVENT_PATH so the subtest is hermetic regardless of the ambient CI context it happens to execute in. This was pre-existing on main (from PR cloudposse#2395) and surfaced when PR cloudposse#2903 hit the merge queue for the first time; not a regression from this branch's own changes, but fixed here directly per repo convention for CI-blocking issues found on an open PR's branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
what
atmos describe affected --uploadunderGITHUB_EVENT_NAME=merge_group(previously gated topull_requestonly).event.merge_group.base_sha, populateHeadSHAfromevent.merge_group.head_sha, and derive the target branch fromevent.merge_group.base_ref— with graceful fallback toGITHUB_BASE_REFwhen no event payload is available.settings.proevent schema in the configuration reference (pull_request,release,drift_detection) and add a new Merge Queue Support section documenting the optionalsettings.pro.merge_group.checks_requested.workflowsblock, the prerequisites, the resolution order, and the migration note.ErrUploadRequiresPullRequestEvent→ErrUploadRequiresSupportedEventand update the error message/hints to mentionpull_request,pull_request_target, andmerge_group.settings.pro.{pull_request,release,drift_detection,merge_group}throughStripAffectedForUpload.why
check_suite.requestedwebhooks and creates check runs on the synthetic SHAs GitHub produces when a PR enters a merge queue (gh-readonly-queue/<base>/pr-<N>-<sha>). For the check to conclude correctly when stacks are affected, the CLI must allow--uploadundermerge_groupand resolve the diff base against the target-branch commit the synthetic merge was built on top of.settings.proper-event schema was previously undocumented on atmos.tools — customers were copying shapes from test fixtures. Backfilling the full reference (and addingmerge_groupalongside it) addresses both gaps in one PR.settings.prois opaquemap[string]anyend-to-end, so no struct changes are required formerge_groupto round-trip; the new test locks that contract in to prevent a future struct-tightening from silently dropping the new block.references
/cli/configuration/settings/pro#merge-queue-supportcheck_suite.requestedforgh-readonly-queue/...head branches.Note
The Merge Queue Support section in
pro.mdx(and the migration note in the blog post) contain anAtmos CLI ≥ X.Y.Zplaceholder. Replace with the actual released version once this PR is tagged.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation