Skip to content

Conversation

@goruha
Copy link
Member

@goruha goruha commented Sep 6, 2025

what

  • Fix demo-stacks commands in screengrab script
  • Fix screengrab workflow
  • Add screengrabs environment
  • Prevent vhs workflow from canceling running

why

  • Fix CI workflow run
  • Allow to trigger workflows on screengrabs update PR creation
  • Use the GitHub app to authenticate and create a PR
  • To avoid a race condition for vhs workflows on set no-release label

@goruha goruha requested a review from a team as a code owner September 6, 2025 19:24
@github-actions github-actions bot added the size/xs Extra small size PR label Sep 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

Warning

Rate limit exceeded

@goruha has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e4dc75f and f192aa7.

📒 Files selected for processing (1)
  • .github/settings.yml (1 hunks)
📝 Walkthrough

Walkthrough

Fixed a CLI help-flag spacing in a demo screengrab and updated the Screengrabs CI workflow to set an environment, inject ATMOS_PAGER, add a GitHub App token step, and use that token for PR creation with signed commits and templated messages.

Changes

Cohort / File(s) Summary
Demo screengrabs
demo/screengrabs/demo-stacks.txt
Inserted missing space in CLI invocation: changed atmos packer validate--help to atmos packer validate --help.
Screengrabs CI workflow
.github/workflows/screengrabs.yaml
Added environment: demo to the build job; set ATMOS_PAGER: "false" in the build step; added uses: actions/create-github-app-token@v1 (id: github-app) to produce a GitHub App token from vars.BOT_GITHUB_APP_ID and secrets.BOT_GITHUB_APP_PRIVATE_KEY; updated PR creation to use steps.github-app.outputs.token, enabled sign-commits: true, added templated commit-message and body, set base: main, and removed labels: "no-release".
VHS workflow concurrency
.github/workflows/vhs.yaml
Changed concurrency: cancel-in-progress from true to false for prepare and vhs jobs (allow overlapping runs).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Prepare as prepare job
  participant Build as build job
  participant GHApp as create-github-app-token
  participant CreatePR as create-or-update-pull-request

  Note over Prepare: workflow starts / determines version
  Prepare->>Build: needs.prepare.outputs.version
  Note over Build: run make build-all install\n(environment: demo, ATMOS_PAGER="false")
  Build->>GHApp: request app token (app-id + private-key)
  GHApp-->>Build: outputs.token
  Build->>CreatePR: create PR using outputs.token\n(sign-commits, templated commit/message/body, base=main)
  CreatePR-->>Build: PR created/updated
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • osterman
  • milldr

Pre-merge checks (2 passed, 1 warning)

❌ Failed Checks (1 warning)
Check Name Status Explanation Resolution
Description Check ⚠️ Warning The current description includes only a brief “what” and “why” for the screengrab script fix but lacks specific details on the file modifications, workflow updates like the screengrabs CI changes and vhs concurrency settings, and any testing or impact, leaving reviewers without sufficient context. Please expand the pull request description to summarize each file change (demo-stacks.txt fix, screengrabs.yaml updates, and vhs.yaml concurrency adjustments), explain the rationale and any testing performed, and consider adding a pull request template to the repository to ensure structured and consistent future PR descriptions.
✅ Passed Checks (2 passed)
Check Name Status Explanation
Title Check ✅ Passed The title concisely and clearly summarizes the primary change by stating the fix for the demo-stacks command and aligns with the main objective without unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Pre-merge checks (2 passed, 1 inconclusive)

❌ Failed Checks (1 inconclusive)
Check Name Status Explanation Resolution
Description Check ❓ Inconclusive The description outlines high-level “what” and “why” sections but omits critical details such as the new GitHub App token step, the ATMOS_PAGER environment variable, commit‐message updates, and concurrency changes, making it difficult for reviewers to grasp the full scope Enhance the description with specifics for each workflow update and add a .github/PULL_REQUEST_TEMPLATE.md file to standardize and enforce detailed PR descriptions moving forward
✅ Passed Checks (2 passed)
Check Name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Title Check ✅ Passed The title succinctly captures the primary intent of the pull request by focusing on fixing the screengrab workflows, which aligns with the changes made to both the screengrabs script and its associated CI workflows
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch screengrab-fix

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
demo/screengrabs/demo-stacks.txt (1)

1-63: Refine the --help grep check to target only demo scripts
Limit the CI check to your shell scripts (e.g. -g 'scripts/demo-stacks/*.sh') so it won’t flag screengrab markdown (like the slug in demo/screengrabs/README.md). Adjust your example to:

rg -nP '\S--help\b' -g 'scripts/demo-stacks/*.sh'
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f2781b5 and e9ff33b.

📒 Files selected for processing (1)
  • demo/screengrabs/demo-stacks.txt (1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/atmos/manifest/1.0.json : Update /pkg/datafetcher/schema/atmos/manifest/1.0.json when adding Atmos configuration options.
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.

Applied to files:

  • demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.

Applied to files:

  • demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.

Applied to files:

  • demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/stacks/stack-config/1.0.json : Update /pkg/datafetcher/schema/stacks/stack-config/1.0.json when adding Atmos configuration options.

Applied to files:

  • demo/screengrabs/demo-stacks.txt
📚 Learning: 2025-09-05T14:40:05.107Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T14:40:05.107Z
Learning: Applies to /pkg/datafetcher/schema/atmos/manifest/1.0.json : Update /pkg/datafetcher/schema/atmos/manifest/1.0.json when adding Atmos configuration options.

Applied to files:

  • demo/screengrabs/demo-stacks.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: build
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Analyze (go)
  • GitHub Check: Lint (golangci)
  • GitHub Check: Summary
🔇 Additional comments (1)
demo/screengrabs/demo-stacks.txt (1)

6-6: LGTM — correct flag forwarding

Inserting the space ensures --help is passed to validate as intended. Matches prior guidance that help should work without stack context.

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 6, 2025
@codecov
Copy link

codecov bot commented Sep 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.95%. Comparing base (4cdbe27) to head (f192aa7).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1453   +/-   ##
=======================================
  Coverage   55.94%   55.95%           
=======================================
  Files         274      274           
  Lines       28936    28936           
=======================================
+ Hits        16189    16191    +2     
+ Misses      10955    10951    -4     
- Partials     1792     1794    +2     
Flag Coverage Δ
unittests 55.95% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify
Copy link

mergify bot commented Sep 6, 2025

Important

Cloud Posse Engineering Team Review Required

This 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 #pr-reviews channel.

@mergify mergify bot added the needs-cloudposse Needs Cloud Posse assistance label Sep 6, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.github/workflows/screengrabs.yaml (1)

71-72: Good call disabling paging in CI; consider job-level env + verify accepted values.

  • Optional: move ATMOS_PAGER: "false" to the build job’s env: so all sub-steps/scripts inherit it.
  • Please confirm Atmos recognizes ATMOS_PAGER="false" (vs e.g. 0, "", or PAGER=cat).

Proposed diff:

   build:
     needs: [prepare]
     runs-on: ubuntu-latest
+    env:
+      ATMOS_PAGER: "false"
     steps:
@@
-      - name: Run make build-all install
-        run: |
-          make -C demo/screengrabs build-all install
-          git add -A
-          git status
-        env:
-          ATMOS_PAGER: "false"
+      - name: Run make build-all install
+        run: |
+          make -C demo/screengrabs build-all install
+          git add -A
+          git status
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e9ff33b and 63f5715.

📒 Files selected for processing (1)
  • .github/workflows/screengrabs.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: build
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Lint (golangci)
  • GitHub Check: build
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Analyze (go)
  • GitHub Check: Lint (golangci)
  • GitHub Check: Summary

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 6, 2025
osterman
osterman previously approved these changes Sep 7, 2025
@osterman osterman added the no-release Do not create a new release (wait for additional code changes) label Sep 7, 2025
@goruha goruha dismissed stale reviews from osterman and coderabbitai[bot] via 77a23d0 September 7, 2025 17:21
@github-actions github-actions bot added size/s Small size PR and removed size/xs Extra small size PR labels Sep 7, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
.github/workflows/screengrabs.yaml (2)

38-38: Confirm the 'demo' environment exists and won’t require approvals.

Good to scope the job, but if the environment has protection rules (reviewers/wait timer), this workflow may hang on PRs or forks. If it’s just for grouping, consider dropping environment or using an unprotected one.


72-74: Nice: disabling the pager; consider setting non-interactive env at the job level.

To stabilize outputs across all steps, move ATMOS_PAGER to the job env and add common CI flags.

Apply within this step:

-        env:
-          ATMOS_PAGER: "false"

Add to the build job (outside this hunk):

jobs:
  build:
    env:
      ATMOS_PAGER: "false"
      CI: "true"
      TF_INPUT: "0"
      TF_IN_AUTOMATION: "1"
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 63f5715 and 77a23d0.

📒 Files selected for processing (1)
  • .github/workflows/screengrabs.yaml (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Ensure consistency between CLI help text and website documentation
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: build
  • GitHub Check: Build (macos-latest, macos)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Analyze (go)
  • GitHub Check: Lint (golangci)
  • GitHub Check: Summary
🔇 Additional comments (2)
.github/workflows/screengrabs.yaml (2)

75-79: App token step looks good; verify installation and permissions.

Inputs app-id/private-key are correct and the token output is what you want. Ensure the GitHub App is installed on this repo/org with at least contents: write and pull_requests: write. If runs happen in forks, you may need to set owner: explicitly. Token auto-revocation after the job is expected behavior. (github.com)


84-84: Good switch to App token; commit signing is supported.

Using the App token here enables sign-commits: true to produce verified bot-signed commits. Repo-level permissions are already set (contents, pull-requests write). (github.com)

coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 7, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 7, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/screengrabs.yaml (1)

81-95: Gate PR creation on forks to prevent failures.

Fork PRs can’t create branches in the base repo; skip this step for forks to keep CI green.

-      - name: Create or update PR
+      - name: Create or update PR
+        if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }}
         uses: peter-evans/create-pull-request@v7
         with:
           token: ${{ steps.github-app.outputs.token }}
           branch: "chore/update-build-screengrabs-for-${{ needs.prepare.outputs.version }}"
           title: "Update screengrabs for ${{ needs.prepare.outputs.version }}"
           delete-branch: true
           sign-commits: true
           commit-message: |
             chore: update screengrabs for ${{ needs.prepare.outputs.version }}
           body: |
             This PR updates the screengrabs for Atmos version ${{ needs.prepare.outputs.version }}.
           base: main
-          labels: "no-release"          
+          labels: "no-release"
♻️ Duplicate comments (1)
.github/workflows/screengrabs.yaml (1)

75-79: Upgrade to create-github-app-token v2; add least-privilege + fork guard.

Same ask as prior review; pin to v2, scope permissions, and skip on forks to avoid secretless failures.

-      - uses: actions/create-github-app-token@v1
+      - uses: actions/create-github-app-token@v2
         id: github-app
+        if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.head.repo.fork }}
         with:
           app-id: ${{ vars.BOT_GITHUB_APP_ID }}
           private-key: ${{ secrets.BOT_GITHUB_APP_PRIVATE_KEY }}
+          permission-contents: write
+          permission-pull-requests: write
🧹 Nitpick comments (2)
.github/workflows/vhs.yaml (2)

17-17: Overlapping runs now allowed; confirm this is intentional.

Setting cancel-in-progress: false can queue redundant runs on new commits for the same PR. Consider reverting to true to auto-cancel stale builds.

-      cancel-in-progress: false
+      cancel-in-progress: true

64-64: Matrix runs won’t cancel on newer commits.

This can produce duplicate gif commits and summary noise. Prefer cancel-in-progress: true for PR-triggered jobs.

-      cancel-in-progress: false
+      cancel-in-progress: true
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0fce8ef and e4dc75f.

📒 Files selected for processing (2)
  • .github/workflows/screengrabs.yaml (3 hunks)
  • .github/workflows/vhs.yaml (2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/screengrabs.yaml

[error] 94-94: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (2)
.github/workflows/screengrabs.yaml (2)

38-38: Verify environment exists.

Ensure the “demo” environment is defined (and approvals/secrets configured) or this job will fail.


72-74: Nice: disabling paging for non-interactive run.

ATMOS_PAGER=false avoids hung steps in CI. LGTM.

@goruha goruha changed the title Fix demo-stacks command Fix screengrab workflows Sep 8, 2025
coderabbitai[bot]
coderabbitai bot previously approved these changes Sep 8, 2025
osterman
osterman previously approved these changes Sep 8, 2025
@goruha goruha merged commit 5876172 into main Sep 8, 2025
53 checks passed
@goruha goruha deleted the screengrab-fix branch September 8, 2025 21:48
@mergify mergify bot removed the needs-cloudposse Needs Cloud Posse assistance label Sep 8, 2025
@github-actions
Copy link

These changes were released in v1.190.0-test.1.

osterman pushed a commit that referenced this pull request Sep 23, 2025
* Fix demo-stacks command

* Fix workflow

* Use github app for repos

* Use github app for repos

* Use github app for repos

* Use github app for repos

* Update screengrabs.yaml

* Change cancel-in-progress to false in vhs.yaml

* Add 'no-release' label to screengrabs PR

* Fix screengrabs workflow

* Update website-preview-deploy.yml

* Change environment from demo to screengrabs

* Change environment from 'screengrabs' to 'demo'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-release Do not create a new release (wait for additional code changes) size/s Small size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants