Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughUpdates code review instructions and frontend testing guidance, bumps Pulumi-related template context versions, flips a composite-action input default to install Pulumi CLI by default, and adds explicit GitHub Actions permissions for repository contents and a job-level write permission. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 43-44: The CSS selector example for the Reka UI dialog is split
across lines causing Markdown/rendering and copy-paste errors; edit AGENTS.md at
the sentence mentioning "a Reka UI dialog" and replace the broken multi-line
selector with a single inline code span for the combined selector (the dialog
selector should be one contiguous token rather than split), e.g. the combined
[role="dialog"][data-state="open"], so all further queries run on that found
element.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 00c1fb86-e636-44a1-aeed-b0184e1bbadc
📒 Files selected for processing (6)
.coderabbit.yamlAGENTS.mdextensions/context.pytemplate/.github/actions/install_deps/action.yml.jinja-basetemplate/.github/workflows/ci.yaml.jinja-basetemplate/template/.github/workflows/ci.yaml.jinja
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s agent/reviewer guidance, tightens GitHub Actions CI permissions to be explicit, and adjusts template defaults/versions to better match expected downstream behavior.
Changes:
- Explicitly grants
contents: readat the workflow level and adds/clarifies job-level permissions for CI. - Changes
install_depsaction default to install the Pulumi CLI unless explicitly skipped. - Bumps minimum Pulumi-related version constraints and refines reviewer/agent instructions (AGENTS + CodeRabbit config).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| template/template/.github/workflows/ci.yaml.jinja | Adds explicit contents: read at workflow level and job permission tweaks (including Dependabot update flow). |
| template/.github/workflows/ci.yaml.jinja-base | Adds explicit contents: read at workflow level to avoid implicit none when permissions are set. |
| template/.github/actions/install_deps/action.yml.jinja-base | Changes Pulumi CLI skip flag default to false so Pulumi CLI installs unless opted out. |
| extensions/context.py | Updates Pulumi/Pulumi-provider minimum version constraints used in template context. |
| AGENTS.md | Expands testing guidance and adds a Frontend Testing section. |
| .coderabbit.yaml | Adds a path-specific instruction to avoid comments about _commit in .copier-answers.yml. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Prefer using random values in tests rather than arbitrary ones (e.g. the faker library, uuids, random.randint) when possible. For enums, pick randomly rather than hardcoding one value. | ||
| - Avoid loops in tests — assert each item explicitly so failures pinpoint the exact element. When verifying a condition across all items in a collection, collect the violations into a list and assert it's empty (e.g., assert [x for x in items if bad_condition(x)] == []). | ||
| - Key `data-testid` selectors off unique IDs (e.g. UUIDs), not human-readable names which may collide or change. | ||
| - When asserting a mock or spy was called with specific arguments, always constrain as tightly as possible. In order of preference: (1) assert called exactly once with those args (`assert_called_once_with` in Python, `toHaveBeenCalledExactlyOnceWith` in Vitest/Jest); (2) if multiple calls are expected, assert the total call count and use a positional or last-call assertion (`nthCalledWith`, `lastCalledWith` / `assert_has_calls` with `call_args_list[n]`); (3) plain "called with at any point" (`toHaveBeenCalledWith`, `assert_called_with`) is a last resort only when neither the call count nor the call order can reasonably be constrained. |
There was a problem hiding this comment.
toHaveBeenCalledExactlyOnceWith is not a standard matcher in Jest (and isn’t available in many Vitest setups unless an additional matcher library is installed). To keep this guidance broadly correct, consider recommending toHaveBeenCalledTimes(1) + toHaveBeenCalledWith(...) or toHaveBeenNthCalledWith(1, ...) / toHaveBeenLastCalledWith(...) instead, unless this template explicitly includes a matcher package that provides toHaveBeenCalledExactlyOnceWith.
Suggested change
| - When asserting a mock or spy was called with specific arguments, always constrain as tightly as possible. In order of preference: (1) assert called exactly once with those args (`assert_called_once_with` in Python, `toHaveBeenCalledExactlyOnceWith` in Vitest/Jest); (2) if multiple calls are expected, assert the total call count and use a positional or last-call assertion (`nthCalledWith`, `lastCalledWith` / `assert_has_calls` with `call_args_list[n]`); (3) plain "called with at any point" (`toHaveBeenCalledWith`, `assert_called_with`) is a last resort only when neither the call count nor the call order can reasonably be constrained. | |
| - When asserting a mock or spy was called with specific arguments, always constrain as tightly as possible. In order of preference: (1) assert called exactly once with those args (`assert_called_once_with` in Python, `toHaveBeenCalledTimes(1)` together with `toHaveBeenCalledWith(...)` or `toHaveBeenNthCalledWith(1, ...)` / `toHaveBeenLastCalledWith(...)` in Vitest/Jest); (2) if multiple calls are expected, assert the total call count and use a positional or last-call assertion (`nthCalledWith`, `lastCalledWith` / `assert_has_calls` with `call_args_list[n]`); (3) plain "called with at any point" (`toHaveBeenCalledWith`, `assert_called_with`) is a last resort only when neither the call count nor the call order can reasonably be constrained. |
idonaldson
approved these changes
Mar 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why is this change necessary?
Some more rules. Some CI permission tweaks to be more explicit
What side effects does this change have?
Switching the default value for the install_deps action would be a breaking change, but that whole thing was just introduced yesterday and I had been assuming it was defaulted to false when I wrote all the code using it anyway, so it should be fine.
How is this change tested?
Downstream aws-central-infra repos
Summary by CodeRabbit
Chores