Conversation
📝 WalkthroughWalkthroughThis PR updates CI/devcontainer/tooling versions, adds copier template options (pull_from_ecr, has_backend), introduces a check-skip-duplicates composite action and Pulumi preview PR-commenting, and syncs corresponding template and test data changes. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.devcontainer/devcontainer.json (1)
6-11: 🧹 Nitpick | 🔵 Trivial
"optimize": truewill significantly increase devcontainer build time.
optimizemeans "Optimize Python for performance when compiled (slow)" — it enables PGO (Profile-Guided Optimization) when building the Python binary from source. This is a compile-time build-time optimization and is entirely unrelated to Python's runtime-Oflag, so there's no conflict with the CodeRabbit review instructions. However, this option is noted to add significant time to the build process. Ensure this trade-off (faster Python runtime vs. slower devcontainer build) is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/devcontainer.json around lines 6 - 11, The devcontainer currently enables slow compile-time Python PGO via the feature setting "ghcr.io/devcontainers/features/python:1.8.0" with "optimize": true; change "optimize" to false (or add a comment/PR note documenting the intentional trade-off) so builds aren’t significantly lengthened unless PGO is explicitly desired — update the "optimize" property in the devcontainer.json entry for the Python feature (or add documentation in the PR) to reflect the chosen behavior.template/.devcontainer/Dockerfile (1)
20-31:⚠️ Potential issue | 🟡 Minor
VENV_PATHis overwritten — thegraphql_lambdapath is lost from the container's environment.
VENV_PATHis set to${GRAPHQL_LAMBDA_PROJECT_PATH}/.venvon Line 21, then immediately overwritten by${INFRA_PROJECT_PATH}/.venvon Line 28. The directories for both venvs are created correctly, but any tool or script that reads theVENV_PATHenvironment variable at runtime will only see the infrastructure path. If nothing outside this Dockerfile usesVENV_PATHto locate the graphql_lambda venv, this is harmless but still misleading. If any devcontainer lifecycle script or IDE tooling uses it, the graphql_lambda venv will be unreachable by name.Consider using distinct environment variable names (e.g.,
GRAPHQL_LAMBDA_VENV_PATHandINFRA_VENV_PATH) to avoid the overwrite.✏️ Proposed fix
ENV GRAPHQL_LAMBDA_PROJECT_PATH=/workspaces/${REPO_NAME}/graphql_lambda -ENV VENV_PATH=${GRAPHQL_LAMBDA_PROJECT_PATH}/.venv +ENV GRAPHQL_LAMBDA_VENV_PATH=${GRAPHQL_LAMBDA_PROJECT_PATH}/.venv RUN mkdir -p /workspaces && \ - mkdir -p ${VENV_PATH} && \ - chmod -R 777 /workspaces ${GRAPHQL_LAMBDA_PROJECT_PATH} ${VENV_PATH} && \ - chgrp -R 0 /workspaces ${GRAPHQL_LAMBDA_PROJECT_PATH} ${VENV_PATH} + mkdir -p ${GRAPHQL_LAMBDA_VENV_PATH} && \ + chmod -R 777 /workspaces ${GRAPHQL_LAMBDA_PROJECT_PATH} ${GRAPHQL_LAMBDA_VENV_PATH} && \ + chgrp -R 0 /workspaces ${GRAPHQL_LAMBDA_PROJECT_PATH} ${GRAPHQL_LAMBDA_VENV_PATH} ENV INFRA_PROJECT_PATH=/workspaces/${REPO_NAME}/infrastructure -ENV VENV_PATH=${INFRA_PROJECT_PATH}/.venv +ENV INFRA_VENV_PATH=${INFRA_PROJECT_PATH}/.venv RUN mkdir -p ${INFRA_VENV_PATH} && \ - chmod -R 777 ${INFRA_PROJECT_PATH} ${VENV_PATH} && \ - chgrp -R 0 ${INFRA_PROJECT_PATH} ${VENV_PATH} + chmod -R 777 ${INFRA_PROJECT_PATH} ${INFRA_VENV_PATH} && \ + chgrp -R 0 ${INFRA_PROJECT_PATH} ${INFRA_VENV_PATH}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/Dockerfile` around lines 20 - 31, The Dockerfile overwrites VENV_PATH (first set from GRAPHQL_LAMBDA_PROJECT_PATH then reset from INFRA_PROJECT_PATH); change to two distinct env vars (e.g., GRAPHQL_LAMBDA_VENV_PATH and INFRA_VENV_PATH) and update the mkdir/chmod/chgrp commands to use those new names so both virtualenv paths are preserved in the container environment; update the ENV lines that currently set VENV_PATH and any subsequent RUNs referencing VENV_PATH to use GRAPHQL_LAMBDA_VENV_PATH and INFRA_VENV_PATH respectively.template/.github/workflows/ci.yaml.jinja (1)
128-150:⚠️ Potential issue | 🟡 Minor
check-skip-duplicatefailure silently passes the required check.
check-skip-duplicateis listed inworkflow-summary'sneedsbut is absent from the bash success-pattern check. If the job fails (e.g., transientgh pr listAPI error) on apull_requestevent:
- Every downstream job (
lint,build-app-frontend,pulumi-prod) is skipped (theirif:condition resolves tofalsewhen the dependency fails).- The success check sees
skippedfor all of them — which matches^(skipped|success)$.Mark the required-check as succeededthen fires for the PR, letting it merge with zero CI validation.Adding
check-skip-duplicateto the bash check closes the gap:🐛 Proposed fix
if [[ ! "${{ needs.get-values.result }}" =~ $success_pattern ]] || [[ ! "${{ needs.build-app-frontend.result }}" =~ $success_pattern ]] ||{% if use_staging_environment %} [[ ! "${{ needs.plan-to-staging.result }}" =~ $success_pattern ]] ||{% endif %} - [[ ! "${{ needs.pulumi-prod.result }}" =~ $success_pattern ]]; then + [[ ! "${{ needs.pulumi-prod.result }}" =~ $success_pattern ]] || + [[ ! "${{ needs.check-skip-duplicate.result }}" =~ $success_pattern ]]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.github/workflows/ci.yaml.jinja` around lines 128 - 150, The workflow-summary job's bash "fail if prior job failure" check omits the check-skip-duplicate job so a failing check can be hidden; update the conditional inside the "fail if prior job failure" step to also test [[ ! "${{ needs.check-skip-duplicate.result }}" =~ $success_pattern ]] (i.e., include check-skip-duplicate alongside needs.get-values, needs.build-app-frontend, needs.pulumi-prod and the optional needs.plan-to-staging) so the script will fail when check-skip-duplicate is not skipped or successful.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/Dockerfile:
- Around line 8-10: The Dockerfile RUN that fetches the Yarn GPG key uses "curl
-sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee
/etc/apt/keyrings/yarn-archive-keyring.gpg" and needs to abort on HTTP errors;
update the curl invocation in that RUN command to include the --fail (or -f)
flag so non-2xx responses cause curl to exit non-zero and prevent writing
invalid data to the keyring, keeping the rest of the pipeline (gpg --dearmor |
tee ...) unchanged.
In @.github/pull_request_template.md:
- Around line 1-21: The PR template contains unnecessary trailing spaces at the
ends of the heading lines (e.g., the headings "## Link to Issue or Message
thread", "## Why is this change necessary?", "## How does this change address
the issue?", "## What side effects does this change have?", "## How is this
change tested?", and "## Other"); remove the trailing whitespace from each of
these heading lines so they are clean Markdown headings (no content change),
save the file, and commit the updated .github/pull_request_template.md.
In @.github/workflows/pre-commit.yaml:
- Line 62: Update the GitHub Actions step that currently references "uses:
actions/cache@v5.0.2" to "uses: actions/cache@v5.0.3" so the workflow picks up
the `@actions/cache` v5.0.5 security fix; locate the line containing uses:
actions/cache@v5.0.2 in the workflow and change the version pin to v5.0.3.
In @.pre-commit-config.yaml:
- Line 45: The frozen comment for the typos hook is incomplete—update the inline
comment after the rev (rev: e1f6f6eaedd8587fa3c76ec20e7cbaa8f7132b2d) to the
full patch semver used by crate-ci/typos (e.g., replace "# frozen: v1" with the
concrete tag like "# frozen: v1.28.2"); you can obtain the exact tag by running
pre-commit autoupdate --freeze or by looking up the release for that rev, then
update the comment next to the rev entry in .pre-commit-config.yaml.
In `@copier.yml`:
- Around line 36-40: The boolean default for the copier variable pull_from_ecr
is using the non-standard YAML literal "no"; update the default to the YAML 1.2
boolean keyword false (i.e., change the default value for pull_from_ecr to
false) so YAMLlint stops flagging it and the file conforms to YAML 1.2 boolean
keywords.
- Around line 66-72: The `when` expressions for aws_identity_center_id and
aws_org_home_region in copier.yml currently include `or True`, which
short-circuits and makes those prompts always show; remove the `or True` from
the `when` expressions (or if the intent is to always prompt, delete the entire
`when` lines for each of aws_identity_center_id and aws_org_home_region) so the
conditional logic for python_package_registry,
install_aws_ssm_port_forwarding_plugin, and pull_from_ecr is respected.
- Around line 1-6: The file copier.yml has too many consecutive blank lines at
the top (lines between the leading comment and the upstream questions block);
open copier.yml and remove extra blank lines so there are no more than two
consecutive blank lines between the initial comment and the questions block
(i.e., collapse lines 2–6 down to up to two blank lines), then save the file so
YAML linter passes.
- Around line 141-150: The sed invocation in the _tasks command block that
updates ruff.toml uses GNU-only syntax (sed -i -E) and breaks on macOS; update
the command in copier.yml to be portable by either (a) adding a runtime check
for sed flavor and using sed -i -E for GNU sed but sed -i '' -E for BSD sed, or
(b) replace the sed call with a POSIX-friendly alternative such as perl -i -pe
to perform the same regex replacement on ruff.toml; ensure the change targets
the sed invocation that constructs target-version="$py_tag" so macOS and Linux
both succeed.
In `@template/.coderabbit.yaml`:
- Around line 8-9: Update the instruction string that currently says
"super().init()" to use the correct Python dunder initializer
"super().__init__()" so the guidance is technically accurate; locate the YAML
entry containing the path "**/*.py" and replace the incorrect "super().init()"
text in its instructions value with "super().__init__()".
In `@template/.devcontainer/Dockerfile`:
- Around line 8-10: Update the RUN line that removes the old yarn list and pipes
curl into gpg/tee so that curl fails on HTTP errors: add curl's --fail (and
optionally --show-error) flag and ensure the pipeline will stop on failure (rely
on existing pipefail or explicitly set it). Specifically modify the command that
invokes curl | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg so
that curl uses --fail (curl --fail --silent --show-error ...) before piping to
gpg, ensuring an HTTP error won't write invalid data to the
yarn-archive-keyring.gpg file.
In `@template/.github/actions/pulumi_ephemeral_deploy/action.yml`:
- Around line 95-100: The comment body currently embeds the full Pulumi preview
via steps.pulumi-run.outputs.stdout which can exceed GitHub's 65,536-character
comment limit and cause truncation/failure; update the action's message
generation (the message: block) to safely handle long output by truncating
stdout to a fixed size or limiting to the first N lines and/or providing a
fallback like "(no output captured)", and include a link to the full run logs;
locate the message: template that references ${{ steps.pulumi-run.outputs.stdout
}} and replace it with a trimmed/line-limited expression or conditional that
ensures outputs longer than the limit are truncated and a reference URL is
provided.
- Line 83: The inline ratchet comment on the uses line ("uses:
mathiasvr/command-output@34408ea3d0528273faff3d9e201761ae96106cd0 #
ratchet:mathiasvr/command-output@v2.0.0") needs an extra space before the "#" to
satisfy YAMLlint; update the uses line in action.yml (the line containing
mathiasvr/command-output@34408ea3d0528273faff3d9e201761ae96106cd0) by adding a
second space before the inline "# ratchet:" comment.
In `@template/.github/pull_request_template.md`:
- Line 1: The PR template currently begins with the H2 heading "## Link to Issue
or Message thread" which triggers markdownlint MD041; fix by either changing
that line to a top-level H1 (prepend a single #) so the file has a first-line
heading, or add a markdownlint disable comment at the very top (<!--
markdownlint-disable MD041 -->) to suppress the rule for this file, or
alternatively add a `.markdownlint.json` entry to disable MD041 for PR
templates; update the template accordingly so linting no longer flags the
first-line heading.
In `@template/.github/workflows/pulumi-aws.yml`:
- Line 107: The workflow currently grants pull-requests: write at the
reusable-workflow level which applies to all caller jobs even when
SHOW_PREVIEW_COMMENT_ON_PR is false; to fix this either move the permission
declaration from the top-level to only the job/step that posts the PR comment
(the Preview job/step that checks SHOW_PREVIEW_COMMENT_ON_PR) so callers only
grant write when the comment will be posted, or if you must keep it at the
workflow level, add a clear README note in the template explaining that
pull-requests: write is always granted to callers and cannot be conditionalized
via workflow_call, referencing the SHOW_PREVIEW_COMMENT_ON_PR variable and the
Preview step name so maintainers understand the limitation.
- Line 127: The workflow currently references actions/download-artifact@v7.0.0
which requires self-hosted runners be at least Actions Runner version 2.327.1;
either update your self-hosted runners to >=2.327.1 or change the action to a
version compatible with older runners (for example use
actions/download-artifact@v3) so the workflow doesn't break; locate the line
containing "uses: actions/download-artifact@v7.0.0" and either replace the
version tag or add a comment/upgrade step ensuring runner version >=2.327.1.
In `@tests/copier_data/data1.yaml`:
- Around line 8-13: Reduce the consecutive blank lines between the YAML keys
'pull_from_ecr' and 'node_version' to at most two (preferably one) to satisfy
YAMLlint; edit the block containing the 'pull_from_ecr' and 'node_version'
entries and remove the extra empty lines so there are no more than two
consecutive blank lines.
In `@tests/copier_data/data2.yaml`:
- Around line 8-13: Remove the excessive blank lines between the YAML keys
pull_from_ecr and node_version in the data2.yaml test fixture so there are no
more than two consecutive blank lines; open the block containing pull_from_ecr
and node_version, delete the extra empty lines (leaving at most two) so the YAML
linter error for "too many consecutive blank lines" is resolved.
---
Outside diff comments:
In @.devcontainer/devcontainer.json:
- Around line 6-11: The devcontainer currently enables slow compile-time Python
PGO via the feature setting "ghcr.io/devcontainers/features/python:1.8.0" with
"optimize": true; change "optimize" to false (or add a comment/PR note
documenting the intentional trade-off) so builds aren’t significantly lengthened
unless PGO is explicitly desired — update the "optimize" property in the
devcontainer.json entry for the Python feature (or add documentation in the PR)
to reflect the chosen behavior.
In `@template/.devcontainer/Dockerfile`:
- Around line 20-31: The Dockerfile overwrites VENV_PATH (first set from
GRAPHQL_LAMBDA_PROJECT_PATH then reset from INFRA_PROJECT_PATH); change to two
distinct env vars (e.g., GRAPHQL_LAMBDA_VENV_PATH and INFRA_VENV_PATH) and
update the mkdir/chmod/chgrp commands to use those new names so both virtualenv
paths are preserved in the container environment; update the ENV lines that
currently set VENV_PATH and any subsequent RUNs referencing VENV_PATH to use
GRAPHQL_LAMBDA_VENV_PATH and INFRA_VENV_PATH respectively.
In `@template/.github/workflows/ci.yaml.jinja`:
- Around line 128-150: The workflow-summary job's bash "fail if prior job
failure" check omits the check-skip-duplicate job so a failing check can be
hidden; update the conditional inside the "fail if prior job failure" step to
also test [[ ! "${{ needs.check-skip-duplicate.result }}" =~ $success_pattern ]]
(i.e., include check-skip-duplicate alongside needs.get-values,
needs.build-app-frontend, needs.pulumi-prod and the optional
needs.plan-to-staging) so the script will fail when check-skip-duplicate is not
skipped or successful.
---
Duplicate comments:
In @.coderabbit.yaml:
- Around line 8-9: The instruction string in the .coderabbit.yaml diff contains
a typo "super().init()" which should be the proper Python constructor call;
update that literal in the YAML (the quoted instructions value) to
"super().__init__()" so the text matches the correct method name and calling
convention used elsewhere.
In `@template/.github/workflows/pre-commit.yaml`:
- Line 62: Update the GitHub Actions step that references the actions/cache
action: replace the pinned version token "actions/cache@v5.0.2" with the patched
release "actions/cache@v5.0.3" so the workflow uses the security-fixed version;
locate the line containing the literal "uses: actions/cache@v5.0.2" in the
workflow and change it to "uses: actions/cache@v5.0.3".
| # temporary hack until yarn updates its GPG key | ||
| RUN rm /etc/apt/sources.list.d/yarn.list || true && \ | ||
| curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null |
There was a problem hiding this comment.
Same curl without --fail concern as in template/.devcontainer/Dockerfile.
The same fix applies here: add -f to curl -sS so that HTTP error responses abort the build rather than writing garbage to the GPG keyring.
🛡️ Proposed fix
- curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null
+ curl -sSf https://dl.yarnpkg.com/debian/pubkey.gpg | gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg > /dev/null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/Dockerfile around lines 8 - 10, The Dockerfile RUN that
fetches the Yarn GPG key uses "curl -sS https://dl.yarnpkg.com/debian/pubkey.gpg
| gpg --dearmor | tee /etc/apt/keyrings/yarn-archive-keyring.gpg" and needs to
abort on HTTP errors; update the curl invocation in that RUN command to include
the --fail (or -f) flag so non-2xx responses cause curl to exit non-zero and
prevent writing invalid data to the keyring, keeping the rest of the pipeline
(gpg --dearmor | tee ...) unchanged.
| ## Link to Issue or Message thread | ||
|
|
||
|
|
||
|
|
||
| ## Why is this change necessary? | ||
| ## Why is this change necessary? | ||
|
|
||
|
|
||
|
|
||
| ## How does this change address the issue? | ||
| ## How does this change address the issue? | ||
|
|
||
|
|
||
|
|
||
| ## What side effects does this change have? | ||
| ## What side effects does this change have? | ||
|
|
||
|
|
||
|
|
||
| ## How is this change tested? | ||
| ## How is this change tested? | ||
|
|
||
|
|
||
|
|
||
| ## Other | ||
| ## Other |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Trailing spaces on heading lines are harmless but unnecessary.
CommonMark/GitHub strips trailing spaces from heading lines, so there is no rendered difference. The markdownlint MD041 warning (first line must be H1) is a false positive for PR templates that intentionally use H2 as the top-level heading — safe to ignore.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/pull_request_template.md around lines 1 - 21, The PR template
contains unnecessary trailing spaces at the ends of the heading lines (e.g., the
headings "## Link to Issue or Message thread", "## Why is this change
necessary?", "## How does this change address the issue?", "## What side effects
does this change have?", "## How is this change tested?", and "## Other");
remove the trailing whitespace from each of these heading lines so they are
clean Markdown headings (no content change), save the file, and commit the
updated .github/pull_request_template.md.
|
|
||
| - name: Cache Pre-commit hooks | ||
| uses: actions/cache@v4.3.0 | ||
| uses: actions/cache@v5.0.2 |
There was a problem hiding this comment.
Upgrade actions/cache to v5.0.3 to pick up a security fix.
v5.0.3 bumps @actions/cache to v5.0.5, which resolves a security advisory (actions/cache/security/dependabot/33). Since v5.0.3 is already available, upgrading the pin from v5.0.2 is low-risk and recommended.
🔒 Proposed fix
- uses: actions/cache@v5.0.2
+ uses: actions/cache@v5.0.3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uses: actions/cache@v5.0.2 | |
| uses: actions/cache@v5.0.3 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/pre-commit.yaml at line 62, Update the GitHub Actions step
that currently references "uses: actions/cache@v5.0.2" to "uses:
actions/cache@v5.0.3" so the workflow picks up the `@actions/cache` v5.0.5
security fix; locate the line containing uses: actions/cache@v5.0.2 in the
workflow and change the version pin to v5.0.3.
| # Reformatting (should generally come before any file format or other checks, because reformatting can change things) | ||
| - repo: https://github.com/crate-ci/typos | ||
| rev: 802d5794ff9cf7b15610c47eca99cd1ab757d8d4 # frozen: v1 | ||
| rev: e1f6f6eaedd8587fa3c76ec20e7cbaa8f7132b2d # frozen: v1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Incomplete frozen: comment for typos.
All other frozen comments carry a precise semver tag (e.g., v3.8.1, v0.15.2). Line 45 only says # frozen: v1, which offers no version context. crate-ci/typos uses patch-level tags (e.g., v1.28.2). Running pre-commit autoupdate --freeze likely resolved the floating v1 tag instead of the concrete release tag.
✏️ Suggested fix
- rev: e1f6f6eaedd8587fa3c76ec20e7cbaa8f7132b2d # frozen: v1
+ rev: e1f6f6eaedd8587fa3c76ec20e7cbaa8f7132b2d # frozen: v1.x.y ← replace with the actual semver🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.pre-commit-config.yaml at line 45, The frozen comment for the typos hook is
incomplete—update the inline comment after the rev (rev:
e1f6f6eaedd8587fa3c76ec20e7cbaa8f7132b2d) to the full patch semver used by
crate-ci/typos (e.g., replace "# frozen: v1" with the concrete tag like "#
frozen: v1.28.2"); you can obtain the exact tag by running pre-commit autoupdate
--freeze or by looking up the release for that rev, then update the comment next
to the rev entry in .pre-commit-config.yaml.
| # Questions specific to this template | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Fix YAMLlint error: too many consecutive blank lines.
Lines 2–6 contain 5 consecutive blank lines between the comment and the upstream questions block; YAML lint requires ≤ 2.
🔧 Proposed fix
# Questions specific to this template
-
-
-
-
-
# Questions managed by upstream template📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Questions specific to this template | |
| # Questions specific to this template |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 6-6: too many blank lines (5 > 2)
(empty-lines)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@copier.yml` around lines 1 - 6, The file copier.yml has too many consecutive
blank lines at the top (lines between the leading comment and the upstream
questions block); open copier.yml and remove extra blank lines so there are no
more than two consecutive blank lines between the initial comment and the
questions block (i.e., collapse lines 2–6 down to up to two blank lines), then
save the file so YAML linter passes.
| @@ -1,21 +1,21 @@ | |||
| ## Link to Issue or Message thread | |||
| ## Link to Issue or Message thread | |||
There was a problem hiding this comment.
MD041: Consider adding a top-level heading or suppressing the lint rule.
The file starts with an ## (H2) heading, triggering markdownlint rule MD041 (first-line-heading). If this is intentional for PR templates (where the PR title itself acts as the document title), add a markdownlint disable comment or a .markdownlint.json config to suppress MD041 for this path.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/pull_request_template.md` at line 1, The PR template
currently begins with the H2 heading "## Link to Issue or Message thread" which
triggers markdownlint MD041; fix by either changing that line to a top-level H1
(prepend a single #) so the file has a first-line heading, or add a markdownlint
disable comment at the very top (<!-- markdownlint-disable MD041 -->) to
suppress the rule for this file, or alternatively add a `.markdownlint.json`
entry to disable MD041 for PR templates; update the template accordingly so
linting no longer flags the first-line heading.
| permissions: | ||
| id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact) | ||
| contents: write # needed for mutex | ||
| pull-requests: write # needed to post the preview on the PR as a comment |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
pull-requests: write applies to all jobs in this reusable workflow, not just the Preview step.
Since permissions can't be conditionalized on SHOW_PREVIEW_COMMENT_ON_PR in a workflow_call, this is a known GitHub Actions limitation and the current structure is the pragmatic solution. Just note that callers that set SHOW_PREVIEW_COMMENT_ON_PR: false still grant pull-requests: write to their entire pulumi job run.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/workflows/pulumi-aws.yml` at line 107, The workflow
currently grants pull-requests: write at the reusable-workflow level which
applies to all caller jobs even when SHOW_PREVIEW_COMMENT_ON_PR is false; to fix
this either move the permission declaration from the top-level to only the
job/step that posts the PR comment (the Preview job/step that checks
SHOW_PREVIEW_COMMENT_ON_PR) so callers only grant write when the comment will be
posted, or if you must keep it at the workflow level, add a clear README note in
the template explaining that pull-requests: write is always granted to callers
and cannot be conditionalized via workflow_call, referencing the
SHOW_PREVIEW_COMMENT_ON_PR variable and the Preview step name so maintainers
understand the limitation.
|
|
||
| - name: Download Artifact | ||
| uses: actions/download-artifact@v6.0.0 | ||
| uses: actions/download-artifact@v7.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the actual file and the context around line 127
cat -n template/.github/workflows/pulumi-aws.yml | sed -n '120,135p'Repository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 1039
🌐 Web query:
actions/download-artifact v7 node.js version runner requirements
💡 Result:
actions/download-artifact@v7:
- Node.js runtime: runs on Node.js 24 (
runs.using: node24). [1] - Minimum GitHub Actions runner version: 2.327.1 (update self-hosted runners before upgrading). [1]
Note: download-artifact@v4+ is not supported on GitHub Enterprise Server (GHES) (GHES users are directed to v3 / v3-node20). [1]
Sources:
[1] actions/download-artifact README (v7 “What’s new”) https://github.com/actions/download-artifact
actions/download-artifact@v7.0.0 requires minimum Actions runner version 2.327.1 for self-hosted environments.
actions/download-artifact@v7 runs on Node.js 24 and requires a minimum Actions Runner version of 2.327.1. If you are using self-hosted runners, ensure they are updated before upgrading. GitHub-hosted runners are always up to date.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/workflows/pulumi-aws.yml` at line 127, The workflow
currently references actions/download-artifact@v7.0.0 which requires self-hosted
runners be at least Actions Runner version 2.327.1; either update your
self-hosted runners to >=2.327.1 or change the action to a version compatible
with older runners (for example use actions/download-artifact@v3) so the
workflow doesn't break; locate the line containing "uses:
actions/download-artifact@v7.0.0" and either replace the version tag or add a
comment/upgrade step ensuring runner version >=2.327.1.
| pull_from_ecr: false | ||
|
|
||
|
|
||
|
|
||
|
|
||
| node_version: 22.13.0 | ||
| node_version: 24.11.1 |
There was a problem hiding this comment.
Fix YAMLlint error: too many consecutive blank lines (4 > 2).
Lines 9–12 contain 4 consecutive blank lines between pull_from_ecr and node_version.
🔧 Proposed fix
pull_from_ecr: false
-
-
-
-
+
node_version: 24.11.1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pull_from_ecr: false | |
| node_version: 22.13.0 | |
| node_version: 24.11.1 | |
| pull_from_ecr: false | |
| node_version: 24.11.1 |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 12-12: too many blank lines (4 > 2)
(empty-lines)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/copier_data/data1.yaml` around lines 8 - 13, Reduce the consecutive
blank lines between the YAML keys 'pull_from_ecr' and 'node_version' to at most
two (preferably one) to satisfy YAMLlint; edit the block containing the
'pull_from_ecr' and 'node_version' entries and remove the extra empty lines so
there are no more than two consecutive blank lines.
| pull_from_ecr: true | ||
|
|
||
|
|
||
|
|
||
|
|
||
| node_version: 22.14.0 | ||
| node_version: 24.11.1 |
There was a problem hiding this comment.
Fix YAMLlint error: too many consecutive blank lines (4 > 2).
Same issue as data1.yaml: lines 9–12 contain 4 consecutive blank lines between pull_from_ecr and node_version.
🔧 Proposed fix
pull_from_ecr: true
-
-
-
-
+
node_version: 24.11.1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pull_from_ecr: true | |
| node_version: 22.14.0 | |
| node_version: 24.11.1 | |
| pull_from_ecr: true | |
| node_version: 24.11.1 |
🧰 Tools
🪛 YAMLlint (1.38.0)
[error] 12-12: too many blank lines (4 > 2)
(empty-lines)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/copier_data/data2.yaml` around lines 8 - 13, Remove the excessive blank
lines between the YAML keys pull_from_ecr and node_version in the data2.yaml
test fixture so there are no more than two consecutive blank lines; open the
block containing pull_from_ecr and node_version, delete the extra empty lines
(leaving at most two) so the YAML linter error for "too many consecutive blank
lines" is resolved.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@template/.github/workflows/ci.yaml.jinja`:
- Around line 36-37: Rename the workflow job named "check-skip-duplicate" to
"check-skip-duplicates" so it matches the composite action directory
"./.github/actions/check-skip-duplicates", and then update every workflow
reference that depends on it (any needs: check-skip-duplicate and any
needs.<job>.outputs usages) to use the new "check-skip-duplicates" identifier;
ensure the job id in the job block and all downstream needs/outputs keys are
changed consistently (search for the string "check-skip-duplicate" and replace
with "check-skip-duplicates").
- Around line 141-153: The "fail if prior job failure" run step currently treats
skipped as acceptable via success_pattern and therefore lets a failing
check-skip-duplicate silently pass; update that step so
needs.check-skip-duplicate.result is only accepted when it equals "success" (not
"skipped") — e.g. add an explicit condition or separate test for
needs.check-skip-duplicate.result == "success" before the combined regex checks
(modify the success_pattern logic or add an extra if clause) so
check-skip-duplicate failures cannot be masked while keeping the other needs.*
results matching ^(skipped|success)$.
- Around line 155-164: Update the step that posts the "required-check" commit
status so it runs for both pull_request and merge_group events and calls the
repository statuses endpoint directly: change the if condition to include
github.event_name == 'merge_group' (e.g., github.event_name == 'pull_request' ||
github.event_name == 'merge_group') and replace the use of
github.event.pull_request.statuses_url with the statuses API path using the
commit SHA (use github.api_url or the REST path /repos/${{ github.repository
}}/statuses/${{ github.sha }}), keeping the same GH_TOKEN, context
"required-check", description and target_url so the status is created for merge
queue runs as well.
| - id: check | ||
| uses: ./.github/actions/check-skip-duplicates |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Job name and action path are inconsistent (check-skip-duplicate vs check-skip-duplicates).
The job is named check-skip-duplicate (no trailing s) but the composite action lives at ./.github/actions/check-skip-duplicates (with s). Not a functional issue, but aligning them avoids confusion.
🔧 Suggested rename of the job to match the action path
- check-skip-duplicate:
+ check-skip-duplicates:
runs-on: ...
...
outputs:
- should-run: ${{ steps.check.outputs.should-run }}
+ should-run: ${{ steps.check.outputs.should-run }}Then update all needs and needs.<job>.outputs references accordingly:
- - check-skip-duplicate
+ - check-skip-duplicates- if: needs.check-skip-duplicate.outputs.should-run == 'true'
+ if: needs.check-skip-duplicates.outputs.should-run == 'true'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/workflows/ci.yaml.jinja` around lines 36 - 37, Rename the
workflow job named "check-skip-duplicate" to "check-skip-duplicates" so it
matches the composite action directory
"./.github/actions/check-skip-duplicates", and then update every workflow
reference that depends on it (any needs: check-skip-duplicate and any
needs.<job>.outputs usages) to use the new "check-skip-duplicates" identifier;
ensure the job id in the job block and all downstream needs/outputs keys are
changed consistently (search for the string "check-skip-duplicate" and replace
with "check-skip-duplicates").
| - name: fail if prior job failure | ||
| if: >- | ||
| needs.get-values.result != 'success' || | ||
| needs.build-app-frontend.result != 'success' ||{% endraw %}{% if use_staging_environment %}{% raw %} | ||
| needs.plan-to-staging.result != 'success' ||{% endraw %}{% endif %}{% raw %} | ||
| (needs.pulumi-prod.result != 'success' && needs.pulumi-prod.result != 'skipped') | ||
| run: | | ||
| exit 1 | ||
| success_pattern="^(skipped|success)$" # these are the possibilities: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts#needs-context | ||
|
|
||
| if [[ ! "${{ needs.get-values.result }}" =~ $success_pattern ]] || | ||
| [[ ! "${{ needs.lint.result }}" =~ $success_pattern ]] || | ||
| [[ ! "${{ needs.build-app-frontend.result }}" =~ $success_pattern ]] ||{% endraw %}{% if use_staging_environment %}{% raw %} | ||
| [[ ! "${{ needs.plan-to-staging.result }}" =~ $success_pattern ]] ||{% endraw %}{% endif %}{% raw %} | ||
| [[ ! "${{ needs.pulumi-prod.result }}" =~ $success_pattern ]]; then | ||
| echo "❌ One or more jobs did not finish with skipped or success" | ||
| exit 1 | ||
| fi | ||
| echo "✅ All jobs finished with skipped or success" |
There was a problem hiding this comment.
check-skip-duplicate failure silently green-lights the gate.
check-skip-duplicate has no if: condition, so it always runs and its result should never be skipped. If it fails (e.g. GitHub API error, misconfigured action), all downstream jobs evaluate needs.check-skip-duplicate.outputs.should-run == 'true' as false and are skipped. If a job fails or is skipped, all jobs that need it are skipped unless the jobs use a conditional expression that causes the job to continue — so every gated job shows skipped. The bash script matches all of them against ^(skipped|success)$, exits 0, and the required-check status is posted as success. CI passes with zero real work done.
The fix is to add a success-only guard for check-skip-duplicate.result (it should never be skipped, so allowing that state here would mask real failures):
🐛 Proposed fix
if [[ ! "${{ needs.get-values.result }}" =~ $success_pattern ]] ||
+ [[ "${{ needs.check-skip-duplicate.result }}" != "success" ]] ||
[[ ! "${{ needs.lint.result }}" =~ $success_pattern ]] ||🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/workflows/ci.yaml.jinja` around lines 141 - 153, The "fail
if prior job failure" run step currently treats skipped as acceptable via
success_pattern and therefore lets a failing check-skip-duplicate silently pass;
update that step so needs.check-skip-duplicate.result is only accepted when it
equals "success" (not "skipped") — e.g. add an explicit condition or separate
test for needs.check-skip-duplicate.result == "success" before the combined
regex checks (modify the success_pattern logic or add an extra if clause) so
check-skip-duplicate failures cannot be masked while keeping the other needs.*
results matching ^(skipped|success)$.
| - name: Mark the required-check as succeeded so the PR can be merged | ||
| if: ${{ github.event_name == 'pull_request' }} | ||
| env: | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| gh api \ | ||
| -X POST -H "Accept: application/vnd.github.v3+json" \ | ||
| "${{ github.event.pull_request.statuses_url }}" \ | ||
| -f state=success -f context="required-check" -f description="✅ All required checks passed in the job triggered by pull_request" \ | ||
| -f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n template/.github/workflows/ci.yaml.jinja | sed -n '155,164p'Repository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 760
🌐 Web query:
GitHub Actions merge_group event context github.event.pull_request availability
💡 Result:
In a workflow run triggered by merge_group, github.event.pull_request is not available (it’s only present for pull_request / pull_request_target-type events). For example, github.head_ref is explicitly documented as only available for pull_request or pull_request_target, not merge_group, which is a separate event. [3]
For merge queue runs you must read from the merge-group payload instead:
- Use
github.event.merge_group(the event payload’s top-level object for this trigger). [2] - The payload includes
head_shafor the merge-group commit you should test/report statuses against (commonly used asgithub.event.merge_group.head_sha). [1]
If you need PR metadata (number, labels, etc.), GitHub’s merge_group payload does not include full pull_request details today, and people have requested it as product feedback—so you generally need to look it up via the API based on available merge-group fields. [4]
Sources: [1] GitHub Changelog, [2] “Webhook events and payloads” (merge_group), [3] “Contexts” reference, [4] GitHub Community discussion.
🌐 Web query:
GitHub API POST /repos/{owner}/{repo}/statuses/{sha} endpoint documentation
💡 Result:
Official GitHub REST API documentation for POST /repos/{owner}/{repo}/statuses/{sha} (“Create a commit status”) is under REST API endpoints for commit statuses. [1]
Key details from the docs:
- Path:
POST /repos/{owner}/{repo}/statuses/{sha}[1] - Purpose: Create a status (
error|failure|pending|success) on a specific commit SHA. [1] - Body fields:
state(required),target_url,description,context(defaults todefault). [1] - Auth/permissions: requires push access; fine-grained tokens need Commit statuses: write permission. [1]
- Notes: limit of 1000 statuses per
shaandcontextper repo. [1]
Example request (from the docs):
curl -L -X POST \
-H "Accept: application/vnd.github+json" \
-H "Authorization: Bearer <YOUR-TOKEN>" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/repos/OWNER/REPO/statuses/SHA \
-d '{"state":"success","target_url":"https://example.com/build/status","description":"The build succeeded!","context":"continuous-integration/jenkins"}'
``` [1]
**Sources**
- [1] GitHub Docs – REST API endpoints for commit statuses (includes “Create a commit status”)
---
🌐 Web query:
`GitHub Actions github.sha variable merge_group event commit SHA value`
💡 Result:
For a workflow triggered by the **`merge_group`** event (merge queue):
- **`${{ github.sha }}` / `GITHUB_SHA` = the SHA of the *merge group* (the synthetic/speculative commit that represents the queued PR(s) merged onto the base branch)**.** GitHub explicitly documents `GITHUB_SHA` for `merge_group` as “SHA of the merge group.” [1]
- In the webhook/event payload, the merge queue tells you to run checks against the merge group’s **`merge_group.head_sha`** (“perform checks on the head SHA and report status back”). This is the commit SHA the merge queue wants validated/reported on. [2][3]
So, for `merge_group`, treat **`github.sha` as the merge-group commit SHA to test and report status on** (not an individual PR branch HEAD). If you need the base commit used to form the merge group, it’s in **`${{ github.event.merge_group.base_sha }}`** (payload field). [2]
**Sources**: GitHub Actions event trigger docs (`merge_group`) [1], GitHub webhook payload docs (`merge_group`) [2], GitHub changelog announcement (merge_group + `head_sha`) [3].
---
</details>
**`required-check` commit status is not posted for `merge_group` runs.**
The step is guarded by `if: ${{ github.event_name == 'pull_request' }}`, so it is skipped when the workflow is triggered by `merge_group`. If your branch protection rule references `required-check` as a required commit status for the merge queue, the merge queue will never receive a `success` status for this context and merges will be permanently blocked.
Extend the condition to include `merge_group` events and use the statuses API endpoint directly (since `github.event.pull_request.statuses_url` is unavailable in merge_group context):
<details>
<summary>Suggested fix</summary>
```diff
- if: ${{ github.event_name == 'pull_request' }}
+ if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }}
env:
GH_TOKEN: ${{ github.token }}
run: |
gh api \
-X POST -H "Accept: application/vnd.github.v3+json" \
- "${{ github.event.pull_request.statuses_url }}" \
+ "/repos/${{ github.repository }}/statuses/${{ github.sha }}" \
-f state=success -f context="required-check" \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Mark the required-check as succeeded so the PR can be merged | |
| if: ${{ github.event_name == 'pull_request' }} | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| gh api \ | |
| -X POST -H "Accept: application/vnd.github.v3+json" \ | |
| "${{ github.event.pull_request.statuses_url }}" \ | |
| -f state=success -f context="required-check" -f description="✅ All required checks passed in the job triggered by pull_request" \ | |
| -f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" | |
| - name: Mark the required-check as succeeded so the PR can be merged | |
| if: ${{ github.event_name == 'pull_request' || github.event_name == 'merge_group' }} | |
| env: | |
| GH_TOKEN: ${{ github.token }} | |
| run: | | |
| gh api \ | |
| -X POST -H "Accept: application/vnd.github.v3+json" \ | |
| "/repos/${{ github.repository }}/statuses/${{ github.sha }}" \ | |
| -f state=success -f context="required-check" -f description="✅ All required checks passed in the job triggered by pull_request" \ | |
| -f target_url="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.github/workflows/ci.yaml.jinja` around lines 155 - 164, Update the
step that posts the "required-check" commit status so it runs for both
pull_request and merge_group events and calls the repository statuses endpoint
directly: change the if condition to include github.event_name == 'merge_group'
(e.g., github.event_name == 'pull_request' || github.event_name ==
'merge_group') and replace the use of github.event.pull_request.statuses_url
with the statuses API path using the commit SHA (use github.api_url or the REST
path /repos/${{ github.repository }}/statuses/${{ github.sha }}), keeping the
same GH_TOKEN, context "required-check", description and target_url so the
status is created for merge queue runs as well.
1 participant
Why is this change necessary?
Pull in upstream changes to deal with yarn
Also set up the workflow duplicate skip process
How does this change address the issue?
Pulls in upstream changes and updates CI
What side effects does this change have?
N/A
How is this change tested?
https://github.com/ejfine/rytermedia-com
Summary by CodeRabbit
New Features
Chores