Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
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 (6)
📝 WalkthroughWalkthroughAdds an opt-out flag and action input to skip installing the Pulumi CLI across devcontainer scripts and CI actions, tightens Bash GitHub-CLI permission patterns, updates template/copier refs, enhances agent/test guidance, and adds frontend/deploy commands to the README template. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Workflow
participant Action as install_deps Action
participant Script as manual-setup-deps.py
participant FS as Lockfile
participant Installer as install-pulumi-cli.sh
participant Pulumi as Pulumi CLI
Workflow->>Action: invoke with skip-installing-pulumi-cli flag
Action->>Script: run manual-setup-deps.py (pass flag)
Script->>FS: check selected env lockfile exists & contains "pulumi"
alt skip-install flag true
Script-->>Action: skip Pulumi install
else lockfile has "pulumi" and not skipped
Script->>Installer: execute install-pulumi-cli.sh with lockfile path
Installer->>Pulumi: install Pulumi CLI
Installer-->>Script: exit status
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 |
There was a problem hiding this comment.
Pull request overview
Pulls in upstream Copier template updates related to Pulumi CLI handling, agent guidance, and CI/devcontainer setup.
Changes:
- Add optional Pulumi CLI installation during dependency setup (with a skip flag) and propagate the flag through the
install_depscomposite action + workflows. - Update template docs/agent guidance (frontend commands, testing guidance, VCR vs mocks guidance).
- Expand Claude bash permissions to allow limited
ghread-only commands and deny PR-mutating commands.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| template/README.md.jinja | Adds frontend dev/build commands and a Pulumi deploy command. |
| template/AGENTS.md | Expands testing guidance and command-invocation guidance. |
| template/.github/workflows/pulumi-aws.yml | Skips Pulumi install in install_deps (Pulumi installed elsewhere). |
| template/.github/workflows/pre-commit.yaml | Skips Pulumi install in install_deps. |
| template/.github/workflows/ci.yaml.jinja | Skips Pulumi install in install_deps. |
| template/.github/actions/install_deps/action.yml | Adds skip-installing-pulumi-cli input and forwards it to setup script. |
| template/.devcontainer/manual-setup-deps.py | Installs Pulumi CLI on Linux when referenced in lockfile (unless skipped). |
| template/.claude/settings/permissions/bash.jsonc | Allows limited gh read operations and denies PR-mutating commands. |
| AGENTS.md | Mirrors template testing/command guidance updates. |
| .github/workflows/pre-commit.yaml | Skips Pulumi install in install_deps. |
| .github/actions/install_deps/action.yml | Adds skip-installing-pulumi-cli input and forwards it to setup script. |
| .devcontainer/manual-setup-deps.py | Adds Pulumi-install branch (currently references missing script). |
| .devcontainer/devcontainer.json | Updates devcontainer context hash. |
| .copier-answers.yml | Updates upstream template commit reference. |
| .claude/settings/permissions/bash.jsonc | Allows limited gh read operations and denies PR-mutating commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _ = subprocess.run( | ||
| ["sh", str(REPO_ROOT_DIR / ".devcontainer" / "install-pulumi-cli.sh"), str(env.lock_file)], | ||
| check=True, | ||
| ) |
There was a problem hiding this comment.
The script attempts to run .devcontainer/install-pulumi-cli.sh, but that file does not exist in the repo root .devcontainer/ directory. If uv.lock ever contains "pulumi" on Linux, this branch will fail with a missing-file error. Either add the missing script to .devcontainer/, or guard this call by checking that the script path exists (and/or point to the correct location).
| _ = subprocess.run( | |
| ["sh", str(REPO_ROOT_DIR / ".devcontainer" / "install-pulumi-cli.sh"), str(env.lock_file)], | |
| check=True, | |
| ) | |
| pulumi_install_script = REPO_ROOT_DIR / ".devcontainer" / "install-pulumi-cli.sh" | |
| if pulumi_install_script.exists(): | |
| _ = subprocess.run( | |
| ["sh", str(pulumi_install_script), str(env.lock_file)], | |
| check=True, | |
| ) | |
| else: | |
| print( | |
| f"Warning: Pulumi CLI install script not found at {pulumi_install_script}; skipping Pulumi CLI installation.", | |
| file=sys.stderr, | |
| ) |
AGENTS.md
Outdated
|
|
||
| - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default. | ||
| - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types (unit, integration, E2E...) by default. | ||
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. |
There was a problem hiding this comment.
There’s a double space in this sentence ("the expected"), which looks like a typo.
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. | |
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. |
template/AGENTS.md
Outdated
|
|
||
| - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types by default. | ||
| - Always run tests with an explicit path (e.g. uv run pytest tests/unit) — test runners discover all types (unit, integration, E2E...) by default. | ||
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. |
There was a problem hiding this comment.
There’s a double space in this sentence ("the expected"), which looks like a typo.
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. | |
| - When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function for Python (e.g. `uv run pytest path/to/test.py::test_name --no-cov`) or a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected. |
| Run a Pulumi Preview: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev` | ||
| Deploy the dev stack: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev --up` | ||
|
|
||
|
|
There was a problem hiding this comment.
This README suggests using uv --directory=./infrastructure ..., but the updated AGENTS guidance in this template says not to use uv --directory because it breaks the permission allow-list matcher. Consider updating these instructions to use an explicit cd infrastructure step (or otherwise align README guidance with AGENTS).
| Run a Pulumi Preview: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev` | |
| Deploy the dev stack: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev --up` | |
| From the repository root, change into the infrastructure directory: `cd infrastructure` | |
| Run a Pulumi Preview: `uv run python -m infrastructure.pulumi_deploy --stack=dev` | |
| Deploy the dev stack: `uv run python -m infrastructure.pulumi_deploy --stack=dev --up` |
| uses: ./.github/actions/install_deps | ||
| with: | ||
| python-version: ${{ inputs.PYTHON_VERSION }} | ||
| skip-installing-pulumi-cli: true # at the moment, we're more specifically installing the Pulumi CLI just in the target direction inside the pulumi_ephemeral_deploy action |
There was a problem hiding this comment.
Typo in the inline comment: "target direction" should be "target directory".
| skip-installing-pulumi-cli: true # at the moment, we're more specifically installing the Pulumi CLI just in the target direction inside the pulumi_ephemeral_deploy action | |
| skip-installing-pulumi-cli: true # at the moment, we're more specifically installing the Pulumi CLI just in the target directory inside the pulumi_ephemeral_deploy action |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings/permissions/bash.jsonc:
- Around line 96-103: The deny list for GitHub PR CLI subcommands currently
includes "Bash(gh pr create *)", "Bash(gh pr edit *)", "Bash(gh pr ready *)",
"Bash(gh pr review *)", "Bash(gh pr merge *)", and "Bash(gh pr close *)" but
misses other write-capable commands; add explicit deny entries for "Bash(gh pr
update-branch *)", "Bash(gh pr revert *)", "Bash(gh pr lock *)", and "Bash(gh pr
unlock *)" so all write-capable PR operations are blocked.
In `@AGENTS.md`:
- Around line 20-21: Fix the double space in the phrase "the expected state" in
the AGENTS.md test-iteration guidance: locate the sentence containing "confirm
it is in the expected state (red or green) before widening to the full suite"
and replace the double space between "the" and "expected" with a single space so
it reads "the expected state (red or green) before widening to the full suite."
In `@template/.claude/settings/permissions/bash.jsonc`:
- Around line 96-103: The deny list for mutating GitHub PR commands is
incomplete—update the permissions array that contains entries like "Bash(gh pr
create *)", "Bash(gh pr edit *)", etc., to explicitly include all remaining
write-capable `gh pr` commands (e.g., "Bash(gh pr comment *)", "Bash(gh pr
reopen *)", "Bash(gh pr update-branch *)", "Bash(gh pr draft *)", "Bash(gh pr
lock *)", "Bash(gh pr unlock *)") so that only read-only PR commands are
permitted; modify the entries in the same JSON list to add these deny patterns
to close the mutating surface while keeping the existing deny rules.
In `@template/AGENTS.md`:
- Around line 20-21: Fix the double space typo in the guidance text in
AGENTS.md: locate the sentence containing "confirm it is in the expected state"
and remove the extra space so it reads "confirm it is in the expected state"
(this is the second bullet in the template guidance block).
In `@template/README.md.jinja`:
- Line 17: Update the README to remove the deprecated --directory usage: replace
the occurrence of the command string "uv --directory=./infrastructure run python
-m infrastructure.pulumi_deploy --stack=dev --up" so it instructs users to
either run "uv run python -m infrastructure.pulumi_deploy --stack=dev --up" from
inside the infrastructure directory or invoke the tooling with the
current-process working directory set to ./infrastructure; edit the
template/README.md.jinja entry that contains that command to reflect this
no-`--directory` rule.
🪄 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: 596cb48a-f116-4a9c-9322-e7b989a2aa1a
📒 Files selected for processing (15)
.claude/settings/permissions/bash.jsonc.copier-answers.yml.devcontainer/devcontainer.json.devcontainer/manual-setup-deps.py.github/actions/install_deps/action.yml.github/workflows/pre-commit.yamlAGENTS.mdtemplate/.claude/settings/permissions/bash.jsonctemplate/.devcontainer/manual-setup-deps.pytemplate/.github/actions/install_deps/action.ymltemplate/.github/workflows/ci.yaml.jinjatemplate/.github/workflows/pre-commit.yamltemplate/.github/workflows/pulumi-aws.ymltemplate/AGENTS.mdtemplate/README.md.jinja
| // Github | ||
| // Claude should not ever interfere with the PR process, that is how we gate AI's work | ||
| "Bash(gh pr create *)", | ||
| "Bash(gh pr edit *)", | ||
| "Bash(gh pr ready *)", | ||
| "Bash(gh pr review *)", | ||
| "Bash(gh pr merge *)", | ||
| "Bash(gh pr close *)", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "bash.jsonc" -type fRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 184
🏁 Script executed:
cat -n ./.claude/settings/permissions/bash.jsonc | head -120 | tail -40Repository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 1927
🏁 Script executed:
rg "gh pr (comment|reopen|update-branch)" ./.claude/settings/permissions/bash.jsoncRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 74
🌐 Web query:
gh pr subcommands github cli write mutating operations
💡 Result:
The GitHub CLI gh pr command provides several subcommands that perform mutating operations on pull requests (i.e., those that create, update, or delete PR state or related resources on GitHub). These require write permissions to the repository. Mutating gh pr subcommands: - gh pr create: Creates a new pull request. Supports flags like --title, --body, --base, --draft, --assignee, --reviewer, --label, --milestone, --project. Aliases: gh pr new. - gh pr edit: Updates title, body, base branch, assignees, reviewers, labels, milestones, or projects for an existing PR. Flags include --title, --body, --add-assignee, --add-reviewer, --add-label, --remove-* variants. - gh pr merge: Merges a PR. Supports strategies --merge, --rebase, --squash; flags like --delete-branch, --auto. - gh pr close: Closes a PR. Flags: --comment, --delete-branch. - gh pr ready: Marks a draft PR as ready for review (or --undo to mark as draft). - gh pr update-branch: Updates the PR's head branch with latest base branch changes (merge by default, or --rebase). - gh pr revert: Creates a new PR that reverts a merged PR. Flags like --title, --body, --draft. - gh pr lock: Locks conversation on a PR. Flag: --reason. - gh pr unlock: Unlocks conversation on a PR. Non-mutating subcommands (view/query only): list, view, status, diff, checks, checkout (local git only). All subcommands support -R, --repo to target a specific repo. Use gh pr --help for full details. Examples: gh pr create --title "Fix bug" --body "Details" --reviewer user1 gh pr merge 123 --squash --delete-branch gh pr edit 123 --add-label "bug" --title "Updated title"
Citations:
- 1: https://cli.github.com/manual/gh_pr_create
- 2: https://cli.github.com/manual/gh_pr_edit
- 3: https://cli.github.com/manual/gh_pr
- 4: https://cli.github.com/manual/gh_pr_merge
- 5: https://cli.github.com/manual/gh_help_reference
- 6: https://cli.github.com/manual/gh_pr_close
- 7: https://cli.github.com/manual/gh_pr_ready
- 8: feat: implement gh
pr revertcli/cli#8826
Deny all write-capable gh pr subcommands to prevent PR process interference.
The current deny list covers create, edit, ready, review, merge, and close, but is missing other write-capable PR operations: gh pr update-branch (updates PR head branch), gh pr revert (creates a revert PR), gh pr lock, and gh pr unlock. All of these should be explicitly denied to fully close the write surface on PR operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings/permissions/bash.jsonc around lines 96 - 103, The deny list
for GitHub PR CLI subcommands currently includes "Bash(gh pr create *)",
"Bash(gh pr edit *)", "Bash(gh pr ready *)", "Bash(gh pr review *)", "Bash(gh pr
merge *)", and "Bash(gh pr close *)" but misses other write-capable commands;
add explicit deny entries for "Bash(gh pr update-branch *)", "Bash(gh pr revert
*)", "Bash(gh pr lock *)", and "Bash(gh pr unlock *)" so all write-capable PR
operations are blocked.
| // Github | ||
| // Claude should not ever interfere with the PR process, that is how we gate AI's work | ||
| "Bash(gh pr create *)", | ||
| "Bash(gh pr edit *)", | ||
| "Bash(gh pr ready *)", | ||
| "Bash(gh pr review *)", | ||
| "Bash(gh pr merge *)", | ||
| "Bash(gh pr close *)", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n template/.claude/settings/permissions/bash.jsonc | sed -n '90,110p'Repository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 913
🏁 Script executed:
# Get list of all gh pr subcommands that modify state
# Based on GitHub CLI documentation, let's identify all gh pr commands
# and classify which ones are mutating (write operations)
python3 <<'EOF'
# GitHub CLI gh pr subcommands
gh_pr_commands = {
# Read-only commands
"list": "list pull requests",
"view": "view a pull request",
"diff": "view changes in a pull request",
"status": "show status of relevant pull requests",
"checks": "show checks for a pull request",
# Mutating/Write commands
"create": "create a pull request",
"edit": "edit a pull request",
"close": "close a pull request",
"ready": "mark a pull request as ready for review",
"review": "add a review to a pull request",
"merge": "merge a pull request",
"reopen": "reopen a closed pull request",
"comment": "comment on a pull request",
"update-branch": "update a pull request branch",
"lock": "lock a pull request",
"unlock": "unlock a pull request",
"draft": "mark a pull request as a draft",
}
print("Mutating gh pr commands:")
mutating = {k: v for k, v in gh_pr_commands.items() if k in ["create", "edit", "close", "ready", "review", "merge", "reopen", "comment", "update-branch", "lock", "unlock", "draft"]}
for cmd in sorted(mutating.keys()):
print(f" - gh pr {cmd}")
EOFRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 320
🏁 Script executed:
cat -n template/.claude/settings/permissions/bash.jsonc | sed -n '85,125p'Repository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 1862
Explicitly deny all remaining mutating gh pr commands.
The policy comment states Claude should never interfere with the PR process, but the current deny list is incomplete. Missing write-capable commands include gh pr comment, gh pr reopen, gh pr update-branch, gh pr draft, gh pr lock, and gh pr unlock. If the intent is to allow only read operations (view/diff/list), all remaining mutating commands must be explicitly denied to close the write surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.claude/settings/permissions/bash.jsonc` around lines 96 - 103, The
deny list for mutating GitHub PR commands is incomplete—update the permissions
array that contains entries like "Bash(gh pr create *)", "Bash(gh pr edit *)",
etc., to explicitly include all remaining write-capable `gh pr` commands (e.g.,
"Bash(gh pr comment *)", "Bash(gh pr reopen *)", "Bash(gh pr update-branch *)",
"Bash(gh pr draft *)", "Bash(gh pr lock *)", "Bash(gh pr unlock *)") so that
only read-only PR commands are permitted; modify the entries in the same JSON
list to add these deny patterns to close the mutating surface while keeping the
existing deny rules.
|
|
||
| ## Infrastructure Deployments | ||
| Run a Pulumi Preview: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev` | ||
| Deploy the dev stack: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev --up` |
There was a problem hiding this comment.
Align deploy command with the new no-uv --directory tooling rule.
Line [17] reintroduces uv --directory=..., which conflicts with the new tooling guidance added in this PR and can create mixed instructions.
🛠️ Proposed doc fix
-Deploy the dev stack: `uv --directory=./infrastructure run python -m infrastructure.pulumi_deploy --stack=dev --up`
+From `./infrastructure`, deploy the dev stack with:
+`uv run python -m infrastructure.pulumi_deploy --stack=dev --up`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/README.md.jinja` at line 17, Update the README to remove the
deprecated --directory usage: replace the occurrence of the command string "uv
--directory=./infrastructure run python -m infrastructure.pulumi_deploy
--stack=dev --up" so it instructs users to either run "uv run python -m
infrastructure.pulumi_deploy --stack=dev --up" from inside the infrastructure
directory or invoke the tooling with the current-process working directory set
to ./infrastructure; edit the template/README.md.jinja entry that contains that
command to reflect this no-`--directory` rule.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Line 54: In AGENTS.md locate the sentence containing the misspelled word
"avaiding" (the guidance sentence starting "When running terminal commands,
execute exactly one command per tool call...") and correct "avaiding" to
"avoiding" so the sentence reads "...Use `cd` to change to the directory you
want before running the command, avoiding the need to chain." Leave the rest of
the sentence unchanged.
In `@template/AGENTS.md`:
- Line 54: Typo in AGENTS.md guidance: replace the misspelled word "avaiding"
with "avoiding" in the sentence that begins "When running terminal commands,
execute exactly one command per tool call..." — update that occurrence (search
for "avaiding the need to chain") and change it to "avoiding the need to chain"
so the guidance reads correctly.
🪄 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: 4db6fbb0-440d-4a73-a50d-c789b88cd18e
📒 Files selected for processing (3)
.copier-answers.ymlAGENTS.mdtemplate/AGENTS.md
Pull in upstream template changes
Summary by CodeRabbit
Documentation
Chores