Conversation
|
Warning Rate limit exceeded@ejfine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (1)
WalkthroughAdds and updates devcontainer tooling and CI: replaces shell/PowerShell installers with Python scripts, bumps devcontainer base image and extensions, introduces GitHub Actions (install_deps, update-devcontainer-hash, get-values, pre-commit), adds a deterministic devcontainer hash utility, expands pre-commit exclusions, updates templates and docs, and removes legacy install_deps_uv assets. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas to focus review on:
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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: 36
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
template/.devcontainer/windows-host-helper.sh (1)
10-21: Fix unquoted parameter expansions in variable substitution.The script adds strict error handling and improved argument validation (good), and the deterministic mirror logic is sound. However, lines 48 and 54 have SC2295 warnings: unquoted variable expansion within parameter expansion could fail if paths contain special characters or spaces. The pattern
${dir#$SRC/}should be${dir#"$SRC"/}to properly quote the expansion.Apply this diff to fix the parameter expansion issues:
# 1) Recreate directory tree under $DST while IFS= read -r -d '' dir; do - rel="${dir#$SRC/}" # strip leading $SRC/ → e.g. "sub/dir" + rel="${dir#"$SRC"/}" # strip leading $SRC/ → e.g. "sub/dir" mkdir -p "$DST/$rel" done < <(find "$SRC" -type d -print0) # 2) Move all files into that mirror while IFS= read -r -d '' file; do - rel="${file#$SRC/}" # e.g. "sub/dir/file.txt" + rel="${file#"$SRC"/}" # e.g. "sub/dir/file.txt" # ensure parent exists (though step 1 already did) mkdir -p "$(dirname "$DST/$rel")" mv "$file" "$DST/$rel" done < <(find "$SRC" -type f -print0)Also applies to: 43-64
♻️ Duplicate comments (1)
template/CHANGELOG.md (1)
9-36: Add blank lines below subsection headings in template.This template has the same markdown formatting issues as the root CHANGELOG.md file. Both files should be fixed consistently to comply with the MD022 markdown linting rule.
Apply the same formatting fixes to align with the root CHANGELOG.md corrections.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (79)
.coderabbit.yaml(1 hunks).copier-answers.yml(1 hunks).devcontainer/Dockerfile(1 hunks).devcontainer/devcontainer.json(4 hunks).devcontainer/docker-compose.yml(1 hunks).devcontainer/envs.json(1 hunks).devcontainer/install-ci-tooling.py(1 hunks).devcontainer/install-ci-tooling.sh(0 hunks).devcontainer/manual-setup-deps.py(1 hunks).devcontainer/manual-setup-deps.sh(0 hunks).devcontainer/on-create-command-boilerplate.sh(1 hunks).devcontainer/on-create-command.sh(1 hunks).devcontainer/windows-host-helper.sh(2 hunks).github/actions/install_deps/action.yml(1 hunks).github/actions/install_deps_uv/action.yml(0 hunks).github/actions/install_deps_uv/install-ci-tooling.ps1(0 hunks).github/actions/install_deps_uv/manual-setup-deps.ps1(0 hunks).github/actions/update-devcontainer-hash/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/pull_request_template.md(1 hunks).github/reusable_workflows/build-docker-image.yaml(1 hunks).github/workflows/ci.yaml(6 hunks).github/workflows/get-values.yaml(1 hunks).github/workflows/hash_git_files.py(1 hunks).github/workflows/pre-commit.yaml(1 hunks).github/workflows/tag-on-merge.yaml(1 hunks).github/zizmor.yml(1 hunks).gitignore(2 hunks).pre-commit-config.yaml(10 hunks)CHANGELOG.md(1 hunks)CONTRIBUTING.md(1 hunks)README.md(1 hunks)_typos.toml(2 hunks)biome.jsonc(1 hunks)copier.yml(5 hunks)extensions/context.py(1 hunks)pyproject.toml(1 hunks)pyrightconfig.json(1 hunks)ruff-test.toml(1 hunks)ruff.toml(3 hunks)template/.coderabbit.yaml(1 hunks)template/.coveragerc(1 hunks)template/.devcontainer/Dockerfile(1 hunks)template/.devcontainer/create-aws-profile.sh.jinja(1 hunks)template/.devcontainer/devcontainer.json.jinja(2 hunks)template/.devcontainer/docker-compose.yml.jinja(2 hunks)template/.devcontainer/envs.json(1 hunks)template/.devcontainer/install-ci-tooling.py.jinja(1 hunks)template/.devcontainer/install-ci-tooling.sh.jinja(0 hunks)template/.devcontainer/manual-setup-deps.py(1 hunks)template/.devcontainer/manual-setup-deps.sh.jinja(0 hunks)template/.devcontainer/manual-setup-deps.sh.jinja.rej(1 hunks)template/.devcontainer/on-create-command-boilerplate.sh(1 hunks)template/.devcontainer/on-create-command.sh.jinja(1 hunks)template/.devcontainer/windows-host-helper.sh(2 hunks)template/.github/actions/install_deps/action.yml(1 hunks)template/.github/actions/install_deps_uv/action.yml(0 hunks)template/.github/actions/install_deps_uv/install-ci-tooling.ps1(0 hunks)template/.github/actions/install_deps_uv/manual-setup-deps.ps1(0 hunks)template/.github/actions/pulumi_ephemeral_deploy/action.yml(1 hunks)template/.github/actions/update-devcontainer-hash/action.yml(1 hunks)template/.github/dependabot.yml.jinja(1 hunks)template/.github/pull_request_template.md(1 hunks)template/.github/workflows/get-values.yaml(1 hunks)template/.github/workflows/hash_git_files.py(1 hunks)template/.github/workflows/pre-commit.yaml(1 hunks)template/.github/workflows/pulumi-aws.yml(1 hunks)template/.gitignore(2 hunks)template/.pre-commit-config.yaml(11 hunks)template/CHANGELOG.md(1 hunks)template/CONTRIBUTING.md(1 hunks)template/README.md.jinja(1 hunks)template/_typos.toml(2 hunks)template/biome.jsonc(1 hunks)template/pyrightconfig.json(1 hunks)template/ruff-test.toml(1 hunks)template/ruff.toml(3 hunks)tests/copier_data/data1.yaml(1 hunks)tests/copier_data/data2.yaml(1 hunks)
💤 Files with no reviewable changes (10)
- .github/actions/install_deps_uv/install-ci-tooling.ps1
- .github/actions/install_deps_uv/action.yml
- .github/actions/install_deps_uv/manual-setup-deps.ps1
- template/.github/actions/install_deps_uv/install-ci-tooling.ps1
- template/.devcontainer/manual-setup-deps.sh.jinja
- template/.github/actions/install_deps_uv/action.yml
- template/.github/actions/install_deps_uv/manual-setup-deps.ps1
- .devcontainer/install-ci-tooling.sh
- .devcontainer/manual-setup-deps.sh
- template/.devcontainer/install-ci-tooling.sh.jinja
🧰 Additional context used
🧬 Code graph analysis (3)
.github/workflows/hash_git_files.py (2)
.devcontainer/manual-setup-deps.py (1)
main(69-160)template/.devcontainer/manual-setup-deps.py (1)
main(69-160)
.devcontainer/manual-setup-deps.py (1)
template/.devcontainer/manual-setup-deps.py (3)
PackageManager(49-51)EnvConfig(54-66)main(69-160)
template/.github/workflows/hash_git_files.py (1)
.github/workflows/hash_git_files.py (1)
main(130-180)
🪛 actionlint (1.7.9)
.github/workflows/get-values.yaml
42-42: description is required in metadata of "Update Devcontainer Hash" action at "/home/jailuser/git/.github/actions/update-devcontainer-hash/action.yml"
(action)
49-49: shellcheck reported issue in this script: SC2046:warning:9:13: Quote this to prevent word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:5:139: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:9:20: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:9:38: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2116:style:9:13: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'
(shellcheck)
.github/workflows/pre-commit.yaml
48-48: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
.github/workflows/ci.yaml
59-59: shellcheck reported issue in this script: SC2086:info:1:60: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
🪛 Checkov (3.2.334)
.devcontainer/Dockerfile
[low] 1-22: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-22: Ensure that a user for the container has been created
(CKV_DOCKER_3)
template/.devcontainer/Dockerfile
[low] 1-42: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-42: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 GitHub Actions: CI
template/.devcontainer/manual-setup-deps.sh.jinja.rej
[error] 1-1: forbidden-files hook failed: found copier update rejection file. Review and remove the file 'template/.devcontainer/manual-setup-deps.sh.jinja.rej'.
🪛 LanguageTool
README.md
[uncategorized] ~15-~15: The official name of this software platform is spelled with a capital “H”.
Context: ...prepare for commit (git add .) 1. Run `python3 .github/workflows/hash_git_files.py . --for-dev...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
template/.github/pull_request_template.md
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
.github/pull_request_template.md
1-1: Headings must start at the beginning of the line
(MD023, heading-start-left)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
CHANGELOG.md
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
template/CHANGELOG.md
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Shellcheck (0.11.0)
template/.devcontainer/windows-host-helper.sh
[info] 48-48: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
[info] 54-54: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
.devcontainer/windows-host-helper.sh
[info] 48-48: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
[info] 54-54: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
🪛 YAMLlint (1.37.1)
template/.github/workflows/pre-commit.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[warning] 21-21: truthy value should be one of [false, true]
(truthy)
[warning] 25-25: too few spaces before comment: expected 2
(comments)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 38-38: too few spaces before comment: expected 2
(comments)
[warning] 54-54: too few spaces before comment: expected 2
(comments)
[warning] 55-55: too few spaces before comment: expected 2
(comments)
[warning] 56-56: too few spaces before comment: expected 2
(comments)
[error] 72-72: too many spaces after colon
(colons)
.github/workflows/pre-commit.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[warning] 21-21: truthy value should be one of [false, true]
(truthy)
[warning] 25-25: too few spaces before comment: expected 2
(comments)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 38-38: too few spaces before comment: expected 2
(comments)
[warning] 54-54: too few spaces before comment: expected 2
(comments)
[warning] 55-55: too few spaces before comment: expected 2
(comments)
[warning] 56-56: too few spaces before comment: expected 2
(comments)
[error] 72-72: too many spaces after colon
(colons)
.github/workflows/ci.yaml
[warning] 14-14: too few spaces before comment: expected 2
(comments)
[warning] 20-20: too few spaces before comment: expected 2
(comments)
[error] 23-23: too many spaces inside brackets
(brackets)
[error] 23-23: too many spaces inside brackets
(brackets)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 27-27: too few spaces before comment: expected 2
(comments)
[warning] 50-50: too few spaces before comment: expected 2
(comments)
[error] 136-136: too many spaces inside brackets
(brackets)
[error] 136-136: too many spaces inside brackets
(brackets)
[warning] 138-138: too few spaces before comment: expected 2
(comments)
copier.yml
[warning] 17-17: truthy value should be one of [false, true]
(truthy)
🔇 Additional comments (102)
CONTRIBUTING.md (1)
30-30: Brand name capitalization correction.Updated "Github" to "GitHub" to match the official trademark capitalization. This improves documentation consistency and adherence to GitHub's branding guidelines.
template/CONTRIBUTING.md (1)
30-30: Brand name capitalization correction (template consistency).Mirrors the capitalization update in the main CONTRIBUTING.md to maintain consistency between root and template documentation files.
template/.github/actions/pulumi_ephemeral_deploy/action.yml (1)
71-74: I need to verify the AWS credentials action version bump and its compatibility. Let me search for release notes and breaking changes between these versions.
<function_calls>
aws-actions configure-aws-credentials v5.1.0 v4.1.0 breaking changes
</function_calls>Please provide the search results. I'll then investigate further to determine if the original review comment needs adjustment.
template/.gitignore (2)
49-49: Test coverage and log rotation patterns are appropriate.The additions of
coverage.xml(line 49) and the**/logs/*.log.*pattern (line 77) correctly extend coverage for common CI/test tooling artifacts and rotated logs.Also applies to: 77-77
83-83: Incomplete comment: add repository-specific ignore rules or remove placeholder.Line 83 contains a comment header but no actual ignore patterns follow. Either add the intended patterns or remove the placeholder comment to avoid confusion.
.gitignore (1)
1-83: Identical content in root and template .gitignore files.The root
.gitignoreandtemplate/.gitignorehave identical content. If this is intentional (e.g., template inheritance model), it's acceptable. However, verify that:
- The template file is meant to be a standalone copy, or
- Whether the root
.gitignoreshould differ from the template to avoid duplication.Additionally, the same comments from the
template/.gitignorereview apply here (redundant macOS patterns on lines 80–81 and incomplete comment on line 83).extensions/context.py (5)
13-45: LGTM!Python tooling versions are properly pinned and follow standard versioning conventions.
103-115: Verify availability of recent versions.
py314_versionis set to3.14.0- Python 3.14 is currently in development/beta phasealpine_image_versionis3.22andnginx_image_versionis1.29.1- verify these existThese versions may be appropriate for forward-looking templates, but ensure downstream templates handle cases where these versions aren't yet available in production environments.
5-5: Import change is correct — the package was renamed from plural to singular.The PyPI package
copier-templates-extensions(plural) was deprecated and renamed tocopier-template-extensions(singular). The code change fromcopier_templates_extensionstocopier_template_extensionsis correct and reflects the current official package name. This is confirmed by the documentation reference in the file's own comment (copier-org/copier-template-extensions) and the consistent usage of the singular form throughout the file (including the version keycopier_template_extensions_version).
47-78: Based on my verification, I can now provide the rewritten review comment:
Node.js 24 is current LTS (not cutting-edge).
Your version selections reflect current best practices:
node_version: "24.7.0"– Node.js 24 entered Active LTS on October 28, 2025, making it the current stable LTS version (not experimental as originally noted)nuxt_test_utils_version: "3.19.1"– Exact pinning is consistent with your template's overall strategy (Python and GitHub Actions versions are also exact-pinned for reproducibility)nuxt_apollo_version: "5.0.0-alpha.15"– Still in alpha as of November 2025, which appears intentional given this template targets cutting-edge Nuxt/Vue featuresNo action required unless your project stability goals explicitly prohibit pre-release versions.
80-101: Original concern about upload-artifact v5 and download-artifact v6 version mismatch is unfounded.The review comment's concern about major version incompatibility between
gha_upload_artifact(v5.0.0) andgha_download_artifact(v6.0.0) is incorrect. According to GitHub Actions official documentation, v5 and v6 are compatible—the major breaking change occurred at the v4→v5 transition. Later releases (v5→v6) are iterative, containing only behavior fixes, Node runtime updates, and extraction-path improvements. They operate against the same artifact backend and are routinely used together in production workflows.Additionally,
gha_windows_runnerset towindows-2025is not a concern. The Windows Server 2025 image reached general availability (GA) on April 8, 2025, and is stable and covered by GitHub's hosted runner SLA. This file (extensions/context.py) is a Copier template context hook that defines default versions for templated workflows, so these are template defaults rather than live issues.tests/copier_data/data1.yaml (1)
6-6: Test data variation is intentional.Both data1.yaml (false) and data2.yaml (true) establish divergent test paths for
install_claude_cli. Ensure integration tests validate conditional template rendering for both values.template/pyrightconfig.json (1)
13-14: Generated artifact exclusions are appropriate.Excluding
graphql_codegenandgenerated/open_apifrom Pyright is sound practice. These generated files typically exhibit type issues unrelated to source code quality.template/ruff-test.toml (1)
15-20: Configuration improvements are justified.The grammar correction and unfixable SIM117 directive are appropriate. Preventing autofix of SIM117 ensures pytest.raises remains distinct for precise error-raising scope validation.
pyrightconfig.json (1)
15-16: Consistent exclusion patterns across root and template configs.The addition of generated artifact exclusions mirrors template/pyrightconfig.json, ensuring unified type-checking policy.
.devcontainer/docker-compose.yml (1)
3-4: Platform constraint improves build consistency.Explicitly setting
platform: linux/amd64prevents platform mismatches and aligns with the PR's goal to stabilize CI environments. The inline comment clearly documents the intent.tests/copier_data/data2.yaml (1)
6-6: I'll verify theinstall_claude_clifeature flag integration across the copier template and devcontainer setup. Let me search the codebase systematically.
<function_calls>
#!/bin/bashStep 1: Find all YAML files and search for install_claude_cli references
echo "=== Finding install_claude_cli in YAML/config files ==="
find . -type f ( -name ".yml" -o -name ".yaml" ) -print0 | xargs -0 rg -l "install_claude_cli" 2>/dev/nullecho ""
echo "=== Finding all references to install_claude_cli (any file type) ==="
rg "install_claude_cli" -n -B2 -A2 2>/dev/null | head -60
</function_calls>#!/bin/bash # Step 2: Look for copier template structure and config echo "=== Looking for copier.yml or template config ===" find . -name "copier.yml" -o -name "copier.yaml" -o -name "*.copier.yml" 2>/dev/null echo "" echo "=== Checking tests/copier_data directory structure ===" find tests/copier_data -type f 2>/dev/null | sort </parameter> </invoke> </function_calls> ```shell #!/bin/bash # Step 3: Search for devcontainer setup that might use install_claude_cli echo "=== Finding devcontainer references ===" find . -type f \( -name "devcontainer.json" -o -name "*.devcontainer*" \) 2>/dev/null echo "" echo "=== Search for install_claude_cli in shell/python/jinja templates ===" find . -type f \( -name "*.sh" -o -name "*.jinja" -o -name "*.j2" -o -name "*.py" -o -name "*.json" \) -print0 | xargs -0 rg "install_claude_cli" -n -B1 -A1 2>/dev/null | head -80 </parameter> </invoke> </function_calls> ```shell #!/bin/bash # Step 4: Check the actual test data files echo "=== Content of data1.yaml ===" cat tests/copier_data/data1.yaml 2>/dev/null || echo "File not found" echo "" echo "=== Content of data2.yaml ===" cat tests/copier_data/data2.yaml 2>/dev/null || echo "File not found" </parameter> </invoke> </function_calls> ```shell #!/bin/bash # Step 5: Search for test files that might use these data files echo "=== Finding test files referencing copier_data or data1/data2 ===" rg -n "data1|data2|copier_data" tests/ --type py --type yaml 2>/dev/null | head -40 </parameter> </invoke> </function_calls> </blockquote></details> <details> <summary>template/.coveragerc (1)</summary><blockquote> `6-10`: **LGTM! Appropriate exclusions for generated code.** The coverage exclusions for auto-generated API client code and CircuitPython firmware files are appropriate, as these code paths are typically not authored manually and don't require test coverage. </blockquote></details> <details> <summary>.copier-answers.yml (3)</summary><blockquote> `6-6`: **LGTM! New configuration flag added.** The `install_claude_cli` flag provides optional Claude CLI installation control. --- `12-12`: **LGTM! Copyright metadata added.** The `repo_org_name_for_copyright` field properly captures the organization name for copyright notices. --- `2-2`: Based on my investigation, I can now provide the rewritten review comment. **Cannot verify the template version bump due to inaccessible upstream repository.** The upstream template repository (`LabAutomationAndScreening/copier-base-template`) is not publicly accessible, and GitHub releases for v0.0.26 through v0.0.85 cannot be retrieved. While the `.copier-answers.yml` file shows the current commit is v0.0.85, the release notes and breaking changes between v0.0.26 and v0.0.85 cannot be verified through public sources. The file is auto-generated by copier (marked "Changes here will be overwritten by Copier"), and git history shows a successful copier update was applied. However, **without access to the upstream template's changelog, the claim that all breaking changes and required migrations have been addressed cannot be independently verified.** </blockquote></details> <details> <summary>.github/workflows/tag-on-merge.yaml (2)</summary><blockquote> `13-13`: **LGTM! Appropriate timeout for tag creation.** The 2-minute timeout is reasonable for this lightweight tagging operation and prevents hung jobs. --- `21-21`: **LGTM! Security best practice applied.** Setting `persist-credentials: false` prevents the GITHUB_TOKEN from being persisted in the git config, reducing the risk of token leakage in subsequent steps. </blockquote></details> <details> <summary>template/_typos.toml (2)</summary><blockquote> `1-5`: **LGTM! Inline spellcheck disables added.** The regex pattern correctly matches trailing comment directives (`# spellchecker:disable-line` or `// spellchecker:disable-line`) to allow per-line spell-check suppression. --- `24-25`: **LGTM! Test artifacts properly excluded from spell checking.** VCR.py cassette files contain recorded HTTP responses that should remain unchanged, and the comment clarification on vendor files is helpful. </blockquote></details> <details> <summary>template/.devcontainer/create-aws-profile.sh.jinja (1)</summary><blockquote> `29-29`: **LGTM! Prevents AWS profile creation with placeholder account ID.** The additional guard `!= "000000000000"` correctly prevents creating an AWS profile when the account ID is set to the all-zeros sentinel value, avoiding misconfiguration. </blockquote></details> <details> <summary>ruff-test.toml (2)</summary><blockquote> `15-15`: **LGTM! Grammar correction applied.** Minor typo fix improves comment clarity. --- `18-20`: **LGTM! Prevents auto-combining pytest.raises with other context managers.** Marking SIM117 as unfixable for tests is appropriate, as keeping `pytest.raises` separate from other context managers makes it clearer where exceptions are expected to originate. </blockquote></details> <details> <summary>biome.jsonc (1)</summary><blockquote> `1-16`: **Clarify the purpose of the disabled Biome configuration.** The configuration file disables both the formatter and linter. If Biome tooling isn't being used, consider whether this file is necessary, or document why it's present (e.g., for future activation, to satisfy tooling requirements, or to prevent Biome from auto-running). The includes/excludes configuration suggests intentional setup, but with all functionality disabled, the purpose is unclear. </blockquote></details> <details> <summary>.devcontainer/Dockerfile (1)</summary><blockquote> `1-4`: **Image tag 2.13.1-focal is confirmed to exist.** Verification confirms that the devcontainers/universal image version 2.13.1-focal is available in the Microsoft Container Registry. The platform constraint and comments are appropriate. </blockquote></details> <details> <summary>template/.coderabbit.yaml (1)</summary><blockquote> `1-26`: **Schema-compliant CodeRabbit configuration with sound rationale.** Configuration properly leverages the assertive review profile, excludes vendor files from commentary, disables tool-specific linters that are already run by pre-commit (avoiding false positives), and enables draft PR reviews for early feedback. All settings align with the provided schema. </blockquote></details> <details> <summary>template/ruff.toml (1)</summary><blockquote> `48-48`: **Well-justified linting rule updates.** The additions of D105, TC006, and SIM114 to the ignore list are well-reasoned: avoiding unnecessary docstrings for magic methods, preventing IDE refactoring confusion, and preserving coverage analysis clarity. The refactored unfixable block preserves all original rules while improving readability. Comments clearly document the rationale for each exclusion. Also applies to: 65-66, 72-75, 90-90 </blockquote></details> <details> <summary>_typos.toml (1)</summary><blockquote> `24-25`: **Appropriate exclusion of test fixture cassettes and clarified vendor exclusion.** Adding `**/tests/**/cassettes/**/*.yaml` prevents false positives from VCR-generated test fixtures containing external URLs and responses. The updated vendor_files comment clarification improves documentation clarity. </blockquote></details> <details> <summary>ruff.toml (1)</summary><blockquote> `48-48`: **Configuration mirrors template/ruff.toml with justified rule updates.** Consistent with the template-level ruff.toml updates: D105, TC006, and SIM114 additions are well-reasoned, and the refactored unfixable block improves clarity while preserving all original rules. Also applies to: 65-66, 72-75, 90-90 </blockquote></details> <details> <summary>template/biome.jsonc (1)</summary><blockquote> `1-16`: **Reasonable Biome configuration disabling formatter and linter with appropriate exclusions.** Disabling both formatter and linter avoids false positives, particularly for CSS files. The exclusion patterns correctly skip the configuration files themselves and nested template configs. The JSONC syntax with trailing commas and inline comments is standard for this format. </blockquote></details> <details> <summary>.coderabbit.yaml (1)</summary><blockquote> `1-26`: **Schema-compliant root-level CodeRabbit configuration.** Mirrors the template configuration consistently. Root-level defaults are schema-valid and establish organization-wide review behavior with the same sound rationale: assertive profile, vendor exclusions, disabling redundant tools, and enabling draft PR reviews. </blockquote></details> <details> <summary>pyproject.toml (1)</summary><blockquote> `9-14`: **Package name change is correct; original review concern is invalid.** The package name change from `copier-templates-extensions` to `copier-template-extensions` is not a typo—it's the correct current package name on PyPI. The old name is deprecated; the new name is the proper current identifier. The codebase already imports from `copier_template_extensions` (extensions/context.py:5), and the version 0.3.3 matches what's already in use (extensions/context.py:21). All dependency updates in lines 9–14 are appropriate and require no code changes. </blockquote></details> <details> <summary>template/README.md.jinja (2)</summary><blockquote> `5-5`: **LGTM!** The OpenIssues badge is a good addition for project visibility. --- `9-17`: **LGTM!** The updated usage instructions provide a clear, step-by-step guide for the devcontainer-centric workflow. The flow logically progresses from container setup through template instantiation to final rebuild. </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `5-17`: **LGTM!** The README updates align with the template version and provide clear devcontainer-centric setup instructions. The static analysis hint about "GitHub" capitalization is a false positive—`.github` here refers to the directory path, not the brand name. </blockquote></details> <details> <summary>.devcontainer/windows-host-helper.sh (1)</summary><blockquote> `10-10`: **LGTM!** Good addition of `set -euo pipefail` for stricter error handling and catching unset variables. </blockquote></details> <details> <summary>template/.github/workflows/pulumi-aws.yml (2)</summary><blockquote> `74-78`: **LGTM!** The `ADDITIONAL_MUTEX_SUFFIX` input provides useful flexibility for more granular mutex locking beyond just stack name. --- `113-114`: **Consider whether 15 minutes is sufficient for mutex acquisition.** The mutex timeout was reduced from 30 to 15 minutes. If multiple Pulumi jobs queue up (especially in busy repos or during deployments), this could cause premature failures. Ensure this aligns with your typical queue depth. </blockquote></details> <details> <summary>.github/actions/install_deps/action.yml (2)</summary><blockquote> `71-74`: **Using `pwsh` shell for Python invocation.** Using PowerShell (`pwsh`) to invoke Python scripts works but is unconventional. This is intentionally cross-platform compatible since `pwsh` is available on all GitHub-hosted runners. Just noting this design choice. --- `59-69`: **Both GitHub Actions versions are valid and recently released.** The original concern was based on outdated information. `actions/setup-python@v6.0.0` was released on September 4, 2025, and `actions/setup-node@v6.0.0` was released on October 14, 2025. Both are legitimate, current versions and require no changes. </blockquote></details> <details> <summary>template/.devcontainer/devcontainer.json.jinja (3)</summary><blockquote> `29-35`: **LGTM!** The gitlens extension is commented out while coderabbit is added. The Claude extension is conditionally included based on `install_claude_cli`, which aligns with the new copier.yml option. --- `6-22`: **LGTM!** Feature version updates and the conditional Claude CLI feature are well-structured. The Jinja templating correctly gates the Claude feature based on `install_claude_cli`. --- `70-73`: **LGTM!** Good expansion of the Prettier formatter scope to include CSS, SCSS, HTML, and Vue files for consistent formatting across frontend assets. </blockquote></details> <details> <summary>template/.devcontainer/install-ci-tooling.py.jinja (3)</summary><blockquote> `41-46`: **LGTM!** The main function properly handles cross-platform detection and environment setup. The `UV_PYTHON_PREFERENCE: only-system` ensures uv uses the system-installed Python version. --- `120-160`: **LGTM!** The SSM plugin installation correctly handles Windows (.exe) and Debian/Ubuntu (.deb) platforms with pinned version 1.2.707.0 for reproducibility. Using a temporary directory for downloads is a clean approach. --- `47-50`: **Ensure `pwsh` is found before potentially using it in the node section.** The `pwsh` variable is only defined when `is_windows` is true and `not args.no_python`. However, the node section (line 110) also uses `pwsh` when `is_windows`. If `--no-python` is passed on Windows but node is needed, `pwsh` will be undefined. Move the `pwsh` lookup outside the `not args.no_python` block: ```diff is_windows = platform.system() == "Windows" uv_env = dict(os.environ) uv_env.update({"UV_PYTHON": args.python_version, "UV_PYTHON_PREFERENCE": "only-system"}) uv_path = ((GITHUB_WINDOWS_RUNNER_BIN_PATH + "\\") if is_windows else "") + "uv" + pwsh = None if is_windows: pwsh = shutil.which("pwsh") or shutil.which("powershell") if not pwsh: raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") if not args.no_python: - if is_windows: - pwsh = shutil.which("pwsh") or shutil.which("powershell") - if not pwsh: - raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH")Wait, re-reading: the
pwshlookup is already outsidenot args.no_pythonat lines 47-50. The issue is the# type: ignorecomment at line 110 assumes this, but the current structure is actually correct. Let me re-verify...Actually, looking again at lines 47-50, the
pwshdefinition IS outside thenot args.no_pythonblock. The code is correct. I'll skip this comment.template/.pre-commit-config.yaml (4)
75-108: LGTM: Exclusion patterns appropriately filter generated artifacts.The added exclusions for minified assets, vendor files, test snapshots, and generated OpenAPI artifacts prevent false positives while preserving necessary validations on source code.
224-227: LGTM: GitHub workflow validation prevents CI failures.Adding
check-github-workflowscatches syntax errors and schema violations in GitHub Actions workflows before they're committed, improving CI reliability.
124-136: LGTM: Biome configuration verified and properly configured.Verification confirms that both
./biome.jsoncand./template/biome.jsoncexist in the repository. The biome-check hook is correctly integrated into the pre-commit configuration with appropriate exclusions for generated files (GraphQL schemas, OpenAPI artifacts, and pyrightconfig.json). The setup is sound and ready for use.
362-370: LGTM: Devcontainer hash computation hook properly configured and script exists.The hash script at
.github/workflows/hash_git_files.pyexists and the pre-commit configuration is syntactically valid. This hook will automatically update the devcontainer context hash when relevant files change, enabling GitHub's prebuild feature to trigger appropriately on "Configuration Change" events.template/.devcontainer/envs.json (1)
1-7: LGTM: Template environment configuration matches instance configuration.This template version of
envs.jsonmaintains the same structure as the instance version, ensuring consistency across template instantiations..devcontainer/on-create-command-boilerplate.sh (1)
4-4: LGTM: Consistent Python migration across instance and template.This change aligns with the template version, maintaining consistency in the migration from shell to Python-based tooling.
.devcontainer/on-create-command.sh (1)
12-12: The flag--optionally-check-lockis properly supported and the behavior change is intentional.The Python script correctly defines the
--optionally-check-lockargument at line 24 with the help text "Check the lock file IFF it exists". The implementation at lines 104-105 confirms this is a read-only check that only validates the lock file when it exists, rather than forcing a check or modification. This is a safe behavior for the on-create phase and is properly documented in the argument help text.template/.devcontainer/on-create-command-boilerplate.sh (1)
4-4: Python script file exists as Jinja2 template with proper error handling; review verification command was incomplete.The
install-ci-tooling.py.jinjatemplate file exists and contains solid error handling:
- All
subprocess.run()calls usecheck=Trueto raiseCalledProcessErroron failures (lines 63, 69, 83, 93, 102, 119, 133, 137, 150, 154, 159)- Explicit
FileNotFoundErrorfor missing PowerShell on Windows (line 50)- Platform-specific logic validates cross-platform improvements (Windows vs Unix handling)
However, the review's verification command searched only for
.pyfiles and missed the actual.py.jinjatemplate file. After template rendering by copier, the.jinjaextension is stripped, producing the expectedinstall-ci-tooling.pythat the boilerplate script calls. The verification methodology in the review comment was incomplete for this template repository context.template/.devcontainer/docker-compose.yml.jinja (1)
3-4: Platform constraint is necessary and correctly implemented—the review comment misrepresents the issue.The base image
mcr.microsoft.com/devcontainers/universal:2.13.1-focalonly supportslinux/amd64across all available versions (verified in MCR). The platform flag is not causing emulation on ARM hosts; rather, it explicitly documents a hard constraint imposed by the image itself. Without this flag, Docker would attempt to run an unavailable ARM64 image and fail.The review comment incorrectly suggests:
- That multi-platform alternatives exist for this specific Microsoft devcontainers image (they don't)
- That the constraint is optional or configurable (it's mandatory)
- That the flag triggers avoidable emulation (the image incompatibility is the root issue, not the flag)
The platform specification in
docker-compose.yml.jinjais redundant with the identical flag already in theDockerfile, but this is a minor stylistic issue, not a correctness problem.Likely an incorrect or invalid review comment.
.github/zizmor.yml (1)
1-5: The template-injection ignore at line 28 is safe and appropriately justified.The ignored step displays GitHub context for debugging using
${{ toJSON(github) }}within a quoted heredoc (<<'JSON'). This pattern is secure because:
- The quoted heredoc prevents shell interpretation of the enclosed content
toJSON()sanitizes the GitHub context object into a safe JSON string- The output is only processed by
jqfor formatting, not executed as code- No user input or untrusted data is involved
The comment accurately describes the step's purpose, and this is a valid suppression case where zizmor's conservative detection of template expansions in
runcontexts doesn't apply to this specific safe pattern..github/dependabot.yml (1)
1-31: LGTM! Well-configured Dependabot setup.The configuration appropriately:
- Uses the
uvecosystem for modern Python package management- Limits patch updates to reduce noise while preserving security updates
- Groups dependencies by type for easier review
- Sets reasonable PR limits and update cadence
.github/actions/update-devcontainer-hash/action.yml (2)
22-27: LGTM! Good security practice.Restricting this action to
dependabot[bot]prevents unauthorized hash updates and ensures the action is only used in its intended context.
42-45: I'll help you verify whether the--exit-zeroflag is appropriate for the hash update script. Let me start by examining thehash_git_files.pyimplementation to understand its behavior and error handling.
<function_calls>
#!/bin/bashFind hash_git_files.py in the repository
fd -t f "hash_git_files.py"
</function_calls>
<function_calls>
#!/bin/bashIf found, display the script content
if [ -f ".github/workflows/hash_git_files.py" ]; then
wc -l ".github/workflows/hash_git_files.py"
fi
</function_calls>Now let me examine the full implementation of the hash_git_files.py script:
<function_calls>
#!/bin/bashDisplay the hash_git_files.py script
cat -n ".github/workflows/hash_git_files.py"
</function_calls>Let me also check the action.yml file to see the full context:
<function_calls>
#!/bin/bashDisplay the action.yml file
cat -n ".github/actions/update-devcontainer-hash/action.yml"
</function_calls>Let me search for any other usages of this script or the --exit-zero flag to understand the pattern:
<function_calls>
#!/bin/bashSearch for exit-zero usage across the repository
rg -n "exit.zero|exit_zero" --type=yaml --type=md --type=py
</function_calls>template/.github/workflows/pre-commit.yaml (2)
34-45: LGTM! Correct conditional checkout logic.The dual checkout paths appropriately handle different event types:
- During
pushevents, explicitly fetches the head of the branch to capture any new dependabot commits- For other events, uses default checkout behavior
54-60: LGTM! Appropriate mutex usage with Windows exclusion.The mutex prevents cache corruption from concurrent modifications, and the Windows exclusion is correctly documented with a reference to the upstream limitation.
.github/workflows/get-values.yaml (1)
39-44: LGTM! Appropriate conditional hash update.The devcontainer hash update is correctly limited to dependabot push events, ensuring hash updates only occur when dependencies change.
template/.devcontainer/on-create-command.sh.jinja (1)
12-12: Python script exists and flag is properly implemented.The verification confirms that
manual-setup-deps.pyexists attemplate/.devcontainer/manual-setup-deps.pyand the--optionally-check-lockflag is correctly defined withaction="store_true"at line 24 of the script. The migration from shell to Python is sound and the parameter naming is consistent between the template and the script implementation.template/.devcontainer/Dockerfile (1)
1-4: The base image update from 2.9.0-focal to 2.13.1-focal contains no breaking changes.Comparison of the two versions shows this is routine maintenance with only minor and patch-level updates across all components (Node.js 20.11.1 → 22.15.0, Python 3.10 → 3.12, Go 1.22 → 1.24, Docker 24.0 → 28.1, and various tool updates). Both use the same Ubuntu 20.04 focal base and x86-64 architecture. All version bumps are forward-compatible within their major versions.
The explicit
--platform=linux/amd64constraint is appropriate for cross-platform build consistency but is not required by this update.template/.github/actions/install_deps/action.yml (2)
71-74: LGTM on tooling installation step.The conditional flag construction using GitHub Actions ternary syntax is correct. Using
pwshshell ensures cross-platform compatibility for the Python script invocation.
83-87: LGTM on dependency installation step.The conditional execution and flag passing logic is consistent with the tooling installation step.
.devcontainer/devcontainer.json (5)
6-16: LGTM on feature version updates.Pinning specific versions (aws-cli:1.1.2, python:1.7.1) improves reproducibility of the dev container builds.
23-34: LGTM on extension updates.The extension version pins ensure consistent development environment across team members.
47-48: LGTM on new settings.The
livePreview.portNumber: 3025avoids conflict with Nuxt's default port 3000, and disabling whitespace-only Copilot suggestions reduces noise.
56-56: LGTM on formatter scope expansion.Adding CSS, SCSS, HTML, and Vue to the Prettier formatter configuration is appropriate for a Nuxt project.
66-66: LGTM on devcontainer context hash.The hash comment enables automatic tracking of devcontainer configuration changes. The
spellchecker:disable-linedirective prevents false positives from the typos hook..github/workflows/pre-commit.yaml (2)
54-59: LGTM on mutex usage for cache safety.Using a mutex to prevent concurrent cache modifications is a good pattern. The 8-minute timeout aligns with the job timeout.
61-69: LGTM on pre-commit cache configuration.The cache key includes Python version, Node version, and the hash of
.pre-commit-config.yaml, which ensures proper cache invalidation when dependencies change.template/.github/workflows/get-values.yaml (1)
49-58: LGTM on PR number extraction logic.The bash string manipulation correctly handles both
pull_requestandmerge_groupevents, and the truncation to last 2 digits for stack naming is a reasonable approach.template/.github/actions/update-devcontainer-hash/action.yml (2)
22-27: LGTM on dependabot actor verification.The explicit check ensures this action only runs when triggered by dependabot, with a clear error message for unauthorized invocations.
47-59: LGTM on commit and push logic.The conditional commit/push only executes when there are actual changes, and the outputs are correctly set for both paths.
.github/workflows/ci.yaml (3)
17-27: LGTM on workflow integration.The
get-valuesjob is correctly wired as a dependency forpre-commit, enabling the dependabot hash update flow to complete before pre-commit runs.
145-154: LGTM on dependabot status update.This step correctly marks the new commit SHA as succeeded when a dependabot hash commit was created, preventing the new commit from being blocked by required checks.
122-124: LGTM on SKIP configuration.Skipping
compute-devcontainer-context-hashduring template instantiation is correct since the hash will differ for each instantiation..devcontainer/install-ci-tooling.py (1)
120-160: LGTM on SSM plugin installation.The conditional installation logic correctly handles both Windows and Debian/Ubuntu platforms, with proper version pinning and verification.
.github/workflows/hash_git_files.py (3)
34-47: LGTM on devcontainer context filtering.The filter correctly identifies files that affect the devcontainer environment: files in
.devcontainer/, lock files, and the hash script itself. Excludingdevcontainer.jsonfrom the hash computation (since it contains the hash) prevents circular dependency.
164-177: LGTM on exit code behavior.The dual exit code behavior (exit 1 on change unless
--exit-zero) is well-designed for both pre-commit hook usage (where changes should fail) and CI usage (where changes should be committed).
78-91: LGTM on hash line detection.The backwards iteration approach correctly finds the hash comment line immediately above the closing brace, handling both the case where the hash exists and where it needs to be inserted.
.devcontainer/manual-setup-deps.py (3)
1-14: LGTM on imports and module-level configuration.The imports are appropriate for the script's functionality, and the module-level constants are well-defined. Using
Path(__file__).parent.parent.resolve()correctly identifies the repo root from within.devcontainer/.
15-46: LGTM on argument parser setup.The CLI arguments are well-documented with clear help text explaining precedence rules and behavior.
49-66: LGTM on PackageManager enum and EnvConfig class.The enum properly inherits from
strfor JSON serialization compatibility. TheEnvConfigclass correctly determines lock file paths based on package manager type, with an appropriateNotImplementedErrorfor unsupported managers..pre-commit-config.yaml (3)
1-1: LGTM on minimum version bump.Updating
minimum_pre_commit_versionto 4.2.0 aligns with the modernization effort.
97-108: LGTM on Biome integration.Adding
biome-checkas a new formatting/linting hook is a good addition. The exclusions for generated files and GraphQL artifacts are appropriate.
197-200: LGTM on GitHub workflow validation.Adding
check-github-workflowsfromcheck-jsonschemawill help catch workflow syntax errors early..github/reusable_workflows/build-docker-image.yaml (4)
39-41: LGTM on workflow-level permissions.Explicitly declaring
id-token: writefor OIDC andcontents: writefor mutex operations follows the principle of least privilege with clear documentation.
46-46: LGTM on job timeout.Adding a 15-minute timeout prevents runaway jobs and aligns with the PR objective of CI timeout improvements.
51-66: ECR URL parsing assumes a specific URL format.The parsing uses
cut -d. -f1andcut -d. -f4to extract account ID and region. This works for standard ECR URLs (<account>.dkr.ecr.<region>.amazonaws.com) but could fail for non-standard formats.Consider adding validation or error handling if the URL format doesn't match expectations:
# Extract the AWS Account ID, which is the first field AWS_ACCOUNT_ID=$(echo "$ECR_URL" | cut -d. -f1) + + # Validate account ID is numeric + if ! [[ "$AWS_ACCOUNT_ID" =~ ^[0-9]+$ ]]; then + echo "Error: Invalid ECR URL format - could not extract AWS Account ID" + exit 1 + fi # Extract the AWS Region, which is the fourth field in the URL structure AWS_REGION=$(echo "$ECR_URL" | cut -d. -f4)
93-98: LGTM on mutex for cache consistency.Using
gh-action-mutexto prevent concurrent cache modifications is a good pattern to avoid race conditions during parallel builds.template/.github/workflows/hash_git_files.py (3)
1-15: LGTM on module setup and constants.The docstring clearly explains the purpose. The comment line prefix/suffix constants provide a structured way to embed hashes in JSON comments while avoiding spellchecker interference.
18-31: LGTM on get_tracked_files function.The function correctly uses
git ls-filesto get tracked files and handles errors appropriately.
34-47: The original review comment is incorrect. The filter correctly handles lock files in nested directories.The Python
endswith()method matches suffixes regardless of the file path nesting. Testing confirms:
"nested/dir/uv.lock".endswith(".lock")→True✓"nested/dir/pnpm-lock.yaml".endswith("pnpm-lock.yaml")→True✓".devcontainer/pnpm-lock.yaml".endswith("pnpm-lock.yaml")→True✓The condition correctly matches lock files at any directory level. The concern about
pnpm-lock.yamlrequiring "exact suffix match" misunderstands howendswith()operates—it doesn't care about the full path, only the suffix.template/.devcontainer/manual-setup-deps.py (1)
1-164: Template file mirrors the main script - same recommendations apply.This template version is intentionally identical to
.devcontainer/manual-setup-deps.py. The same recommendations from that review apply here:
- Consider proper quoting for PNPM commands on Windows (lines 134-144)
- Consider logging stderr from the hash update subprocess (lines 153-160)
This duplication is expected given the copier template structure.
| [ | ||
| { | ||
| "description": "main", | ||
| "relative_directory": ".", | ||
| "package_manager": "uv" | ||
| } | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM: Environment configuration is well-structured.
The configuration clearly defines the environment setup for the UV package manager. The structure is simple and appropriate.
Consider adding a JSON schema reference for validation:
-[
+{
+ "$schema": "./.devcontainer/envs.schema.json",
+ "environments": [
{
"description": "main",
"relative_directory": ".",
"package_manager": "uv"
}
+ ]
+}This would enable IDE validation and catch configuration errors earlier.
📝 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.
| [ | |
| { | |
| "description": "main", | |
| "relative_directory": ".", | |
| "package_manager": "uv" | |
| } | |
| ] | |
| { | |
| "$schema": "./.devcontainer/envs.schema.json", | |
| "environments": [ | |
| { | |
| "description": "main", | |
| "relative_directory": ".", | |
| "package_manager": "uv" | |
| } | |
| ] | |
| } |
🤖 Prompt for AI Agents
In .devcontainer/envs.json lines 1-7, add a JSON Schema reference at the top of
the file (a "$schema" property pointing to the appropriate Dev Containers/Env
JSON Schema, e.g. the Dev Containers schema from SchemaStore) so IDEs can
validate the file; ensure the $schema URL matches the schema for this envs.json
format and keep it as the first property so editors pick up validation and
autocompletion.
| if is_windows: | ||
| pwsh = shutil.which("pwsh") or shutil.which("powershell") | ||
| if not pwsh: | ||
| raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") |
There was a problem hiding this comment.
pwsh may be unbound when --no-python is passed on Windows with Node enabled.
If --no-python is passed but --no-node is not, and the platform is Windows, the code will attempt to use pwsh at line 110, but pwsh is only defined inside the if is_windows: block at line 48, which is outside the if not args.no_python: block.
Move the PowerShell detection outside of the Python-specific block:
is_windows = platform.system() == "Windows"
uv_env = dict(os.environ)
uv_env.update({"UV_PYTHON": args.python_version, "UV_PYTHON_PREFERENCE": "only-system"})
uv_path = ((GITHUB_WINDOWS_RUNNER_BIN_PATH + "\\") if is_windows else "") + "uv"
+ pwsh = None
if is_windows:
pwsh = shutil.which("pwsh") or shutil.which("powershell")
if not pwsh:
raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH")🤖 Prompt for AI Agents
In .devcontainer/install-ci-tooling.py around lines 47 to 50 and referenced at
~110, PowerShell detection (pwsh = shutil.which("pwsh") or
shutil.which("powershell")) is only done inside the `if is_windows:` block
nested within the Python-specific branch, leaving `pwsh` unbound when
`--no-python` is used on Windows; move the PowerShell detection out of the
Python-only block so that on Windows `pwsh` is always set (or explicitly set to
None) before any later usage (e.g., the code at line 110), and adjust error
handling to raise or handle FileNotFoundError only when the code path that needs
PowerShell is actually executed.
| if not args.no_node: | ||
| pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] | ||
| for cmd in pnpm_install_sequence: | ||
| cmd = ( | ||
| [ | ||
| pwsh, # type: ignore[reportPossiblyUnboundVariable] # this matches the conditional above that defines pwsh | ||
| "-NoProfile", | ||
| "-NonInteractive", | ||
| "-Command", | ||
| cmd, | ||
| ] | ||
| if is_windows | ||
| else [cmd] | ||
| ) | ||
| _ = subprocess.run(cmd, shell=True, check=True) |
There was a problem hiding this comment.
Critical: shell=True with list argument on non-Windows will fail.
When shell=True is used on Unix-like systems, the command should be a string, not a list. On non-Windows, cmd becomes ["npm -v"] (a single-element list), and with shell=True, this gets converted incorrectly, likely executing just "npm -v" as a single string which may not behave as expected.
Apply this diff to fix the command execution:
if not args.no_node:
pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"]
for cmd in pnpm_install_sequence:
- cmd = (
- [
- pwsh, # type: ignore[reportPossiblyUnboundVariable] # this matches the conditional above that defines pwsh
- "-NoProfile",
- "-NonInteractive",
- "-Command",
- cmd,
- ]
- if is_windows
- else [cmd]
- )
- _ = subprocess.run(cmd, shell=True, check=True)
+ if is_windows:
+ _ = subprocess.run(
+ [
+ pwsh, # type: ignore[reportPossiblyUnboundVariable]
+ "-NoProfile",
+ "-NonInteractive",
+ "-Command",
+ cmd,
+ ],
+ check=True,
+ )
+ else:
+ _ = subprocess.run(cmd, shell=True, check=True)📝 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.
| if not args.no_node: | |
| pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] | |
| for cmd in pnpm_install_sequence: | |
| cmd = ( | |
| [ | |
| pwsh, # type: ignore[reportPossiblyUnboundVariable] # this matches the conditional above that defines pwsh | |
| "-NoProfile", | |
| "-NonInteractive", | |
| "-Command", | |
| cmd, | |
| ] | |
| if is_windows | |
| else [cmd] | |
| ) | |
| _ = subprocess.run(cmd, shell=True, check=True) | |
| if not args.no_node: | |
| pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] | |
| for cmd in pnpm_install_sequence: | |
| if is_windows: | |
| _ = subprocess.run( | |
| [ | |
| pwsh, # type: ignore[reportPossiblyUnboundVariable] | |
| "-NoProfile", | |
| "-NonInteractive", | |
| "-Command", | |
| cmd, | |
| ], | |
| check=True, | |
| ) | |
| else: | |
| _ = subprocess.run(cmd, shell=True, check=True) |
🤖 Prompt for AI Agents
.devcontainer/install-ci-tooling.py around lines 105 to 119: the code always
calls subprocess.run(..., shell=True) while passing a list on non-Windows which
is invalid; change the invocation so that when is_windows is True you pass a
list (pwsh + args) and use shell=False, and when is_windows is False you pass a
plain string command and use shell=True (or alternatively keep shell=False and
pass a split/list of args on Unix); implement this by constructing cmd as a list
when is_windows else as a string, compute shell_flag = not is_windows, and call
subprocess.run(cmd, shell=shell_flag, check=True).
| if is_windows: | ||
| pwsh = shutil.which("pwsh") or shutil.which("powershell") | ||
| if not pwsh: | ||
| raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") | ||
| pnpm_command = [ | ||
| pwsh, | ||
| "-NoProfile", | ||
| "-NonInteractive", | ||
| "-Command", | ||
| " ".join(pnpm_command), | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential command injection risk when joining PNPM arguments on Windows.
Using " ".join(pnpm_command) to construct a PowerShell command string could be problematic if env.path contains spaces or special characters. While the path is derived from configuration (not user input), this pattern is fragile.
Consider using proper quoting or passing arguments as an array:
if is_windows:
pwsh = shutil.which("pwsh") or shutil.which("powershell")
if not pwsh:
raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH")
+ # Quote the directory path to handle spaces
+ quoted_command = f'pnpm install --dir "{env.path}"'
+ if env_check_lock:
+ quoted_command = f'pnpm install --dir "{env.path}" --frozen-lockfile'
pnpm_command = [
pwsh,
"-NoProfile",
"-NonInteractive",
"-Command",
- " ".join(pnpm_command),
+ quoted_command,
]Committable suggestion skipped: line range outside the PR's diff.
| result = subprocess.run( # update the devcontainer hash after changing lock files | ||
| [sys.executable, ".github/workflows/hash_git_files.py", ".", "--for-devcontainer-config-update", "--exit-zero"], | ||
| capture_output=True, | ||
| text=True, | ||
| check=True, | ||
| cwd=REPO_ROOT_DIR, | ||
| ) | ||
| print(result.stdout) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider logging stderr from the hash update subprocess.
The script captures both stdout and stderr but only prints stdout. If the hash script fails silently or logs warnings to stderr, this information is lost.
result = subprocess.run( # update the devcontainer hash after changing lock files
[sys.executable, ".github/workflows/hash_git_files.py", ".", "--for-devcontainer-config-update", "--exit-zero"],
capture_output=True,
text=True,
check=True,
cwd=REPO_ROOT_DIR,
)
print(result.stdout)
+ if result.stderr:
+ print(result.stderr, file=sys.stderr)📝 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.
| result = subprocess.run( # update the devcontainer hash after changing lock files | |
| [sys.executable, ".github/workflows/hash_git_files.py", ".", "--for-devcontainer-config-update", "--exit-zero"], | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| cwd=REPO_ROOT_DIR, | |
| ) | |
| print(result.stdout) | |
| result = subprocess.run( # update the devcontainer hash after changing lock files | |
| [sys.executable, ".github/workflows/hash_git_files.py", ".", "--for-devcontainer-config-update", "--exit-zero"], | |
| capture_output=True, | |
| text=True, | |
| check=True, | |
| cwd=REPO_ROOT_DIR, | |
| ) | |
| print(result.stdout) | |
| if result.stderr: | |
| print(result.stderr, file=sys.stderr) |
🤖 Prompt for AI Agents
.devcontainer/manual-setup-deps.py around lines 153 to 160: the subprocess call
captures stderr but only prints stdout, losing warnings or error details; update
the code to also print result.stderr when the call succeeds (if non-empty) and
wrap the subprocess.run in a try/except for subprocess.CalledProcessError to
print the exception's stderr and stdout before re-raising or exiting so you
don't lose error output.
| if args.for_devcontainer_config_update: | ||
| assert devcontainer_json_file is not None, ( | ||
| "this should have been set earlier in a similar conditional statement" | ||
| ) | ||
| current_hash = extract_devcontainer_context_hash(devcontainer_json_file) | ||
| if current_hash != overall_checksum_str: | ||
| update_devcontainer_context_hash(devcontainer_json_file, overall_checksum_str) | ||
| print( # noqa: T201 | ||
| f"Updated {devcontainer_json_file} with the new hash: {overall_checksum_str}" | ||
| ) | ||
| if args.exit_zero: | ||
| sys.exit(0) | ||
| else: | ||
| sys.exit(1) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Exit code semantics may be confusing.
When --for-devcontainer-config-update is used, the script exits with code 1 when the hash changes (unless --exit-zero is set). This makes sense for pre-commit hooks but could be surprising for other callers. Consider documenting this in the help text.
_ = parser.add_argument(
"--for-devcontainer-config-update",
action="store_true",
- help="Update the hash in the devcontainer.json file based on all files relevant to devcontainer context",
+ help="Update the hash in the devcontainer.json file based on all files relevant to devcontainer context. Exits with code 1 if the hash changed (useful for pre-commit hooks), unless --exit-zero is also specified.",
)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In template/.github/workflows/hash_git_files.py around lines 164 to 177, the
script currently exits with code 1 when --for-devcontainer-config-update detects
a changed hash (unless --exit-zero is passed), which can surprise callers;
update the argparse/help text to clearly document the exit-code semantics (i.e.,
that the command exits 1 when it updates the devcontainer config unless
--exit-zero is used, and exits 0 when no change or when --exit-zero is set), and
ensure the help string for both --for-devcontainer-config-update and --exit-zero
reflect this behavior so callers know to expect a nonzero exit on updates by
default.
| ubuntu-24.04-py${{ inputs.python-version }}-node-${{ inputs.node-version}}-${{ env.cache-name }}- | ||
|
|
||
| - name: Run pre-commit | ||
| run: pre-commit run -a |
There was a problem hiding this comment.
Fix extra space after colon.
Line 72 has an extra space after the colon, which violates YAML formatting standards.
Apply this diff:
- name: Run pre-commit
- run: pre-commit run -a
+ run: pre-commit run -a📝 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.
| run: pre-commit run -a | |
| run: pre-commit run -a |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 72-72: too many spaces after colon
(colons)
🤖 Prompt for AI Agents
In template/.github/workflows/pre-commit.yaml around line 72, there is an extra
space after the colon in the "run:" mapping; remove the space so the value
immediately follows the colon (i.e., change "run: pre-commit run -a" to have a
single space after the colon), ensuring YAML formatting is correct.
| - name: Checkout code | ||
| uses: actions/checkout@v4.2.2 | ||
| uses: actions/checkout@v5.0.0 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
actions/checkout v5.0.0 GitHub Action release
💡 Result:
actions/checkout v5.0.0 was released as the v5 line (prepared/tagged in August 2025). Main changes: upgraded the action to Node.js 24 and requires GitHub Actions Runner v2.327.1 or newer. [1][2]
Sources:
- actions/checkout repository (v5 usage notes). [1]
- actions/checkout CHANGELOG (v5.0.0 entry: Node 24, runner minimum). [2]
actions/checkout@v5.0.0 exists but has a breaking change requirement.
The v5.0.0 release upgraded to Node.js 24 and requires GitHub Actions Runner v2.327.1 or newer. Verify that your CI environment meets this minimum runner version requirement before upgrading from v4.2.2.
🤖 Prompt for AI Agents
In template/.github/workflows/pulumi-aws.yml around lines 100-101 the workflow
uses actions/checkout@v5.0.0 which requires GitHub Actions Runner v2.327.1+
(Node.js 24) and may break CI using older runners; either revert to a v4.x
checkout (e.g., actions/checkout@v4.2.2) to preserve compatibility, or update
your CI runner images/runners to v2.327.1 or newer and verify Node.js
compatibility before switching to v5.0.0—pick one of these paths and update the
workflow and any CI runner provisioning accordingly.
| # macOS dev cleanliness | ||
| *.DS_Store | ||
| .DS_Store |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify macOS ignore patterns—*.DS_Store is redundant.
Both patterns are present, but *.DS_Store is redundant since .DS_Store is the actual macOS metadata filename. The pattern *.DS_Store would only match files like foo.DS_Store, which is not a macOS convention.
Consider removing line 80 to reduce redundancy:
# macOS dev cleanliness
-*.DS_Store
.DS_StoreAlternatively, if you anticipate other DS_Store-like files, keep both but add a clarifying comment.
📝 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.
| # macOS dev cleanliness | |
| *.DS_Store | |
| .DS_Store | |
| # macOS dev cleanliness | |
| .DS_Store |
🤖 Prompt for AI Agents
In template/.gitignore around lines 79 to 81, the two entries for DS_Store are
redundant; remove the pattern "*.DS_Store" (line 80) and keep only ".DS_Store"
to accurately match macOS metadata files, or alternatively replace both with a
single clearer entry and optional comment (e.g., ".DS_Store # macOS metadata")
if you want explicit documentation.
| @@ -1,4 +1,4 @@ | |||
| minimum_pre_commit_version: 4.0.1 | |||
| minimum_pre_commit_version: 4.2.0 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
pre-commit version 4.2.0 release notes stability known issues
💡 Result:
Summary — pre-commit 4.2.0
- Release (changelog): 4.2.0 published 2025-03-18. Main items: try versioned python executable first for language: python; fix error in conflict-detection when a file is named "HEAD". [1].
- Stability / known issues: downstream metadata shows 4.2.0 had follow-up post-releases (4.2.0.post1 / 4.2.0.post2), indicating subsequent fixes/patches; security/packaging trackers recommend installing the post2 build where available. [2][3].
- Recommendation: prefer either the patched 4.2.0.post2 (if you must stay on 4.2.x) or upgrade to the newer 4.3.0 series (already available) to get further fixes. [3].
Sources
- Changelog / release notes excerpt for pre-commit v4.2.0. [GitLab mirror quoting upstream changelog]. [1]
- pre-commit-pyproject-toml / release listings showing 4.2.0.post releases (post1/post2). [2]
- Package/tracker listing showing newer 4.3.0 available / recommendations to use latest. [3]
Would you like direct links to the upstream release page, the post-release builds, or instructions to upgrade?
🌐 Web query:
pre-commit latest stable version 2025
💡 Result:
The latest stable pre-commit release in 2025 is v4.4.0 — released Nov 8, 2025. [1][2]
Sources:
- GitHub releases: pre-commit v4.4.0 (Nov 8, 2025). [1]
- PyPI: pre-commit 4.4.0 (uploaded Nov 8, 2025). [2]
Consider upgrading to the latest pre-commit version 4.4.0, or at minimum use 4.2.0.post2.
Version 4.2.0 (released March 2025) is stable but outdated. The latest stable version is 4.4.0 (released November 2025). Additionally, 4.2.0 had post-release patches (4.2.0.post1 and 4.2.0.post2), indicating bugs were discovered and fixed after the initial release. If staying on the 4.2.x series is necessary, use 4.2.0.post2 instead; otherwise, upgrade to 4.4.0 to benefit from eight additional months of fixes and improvements.
🤖 Prompt for AI Agents
In template/.pre-commit-config.yaml around line 1, the
minimum_pre_commit_version is pinned to 4.2.0 which is outdated and misses
important post-release fixes; update the value to either 4.2.0.post2 if you must
stay on the 4.2.x series, or preferably to 4.4.0 to get the latest stable fixes
and improvements, then commit the updated version string.
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
template/.devcontainer/windows-host-helper.sh (1)
31-60: Fix glob quoting and parameter expansion to avoid surprises.Two small issues here:
- The
.mdcleanup never actually expands the glob:
rm -rf "./$repoName/*.md"keeps*.mdinside the quotes, so it’s treated literally and won’t delete any markdown files.- The
${dir#$SRC/}/${file#$SRC/}expansions would be safer and clearer with a quoted pattern, and you probably don’t want to create a$DST/$SRCdirectory for the root of the clone.Consider something like:
-# Remove any existing subfolder with the repository name and recreate it -rm -rf "./$repoName" || true # sometimes deleting the .venv folder fails -rm -rf "./$repoName/*.md" # for some reason, sometimes md files are left behind -mkdir -p "./$repoName" +# Remove any existing subfolder with the repository name and recreate it +rm -rf "./$repoName" || true # sometimes deleting the .venv folder fails +rm -rf "./$repoName"/*.md # for some reason, sometimes md files are left behind +mkdir -p "./$repoName" @@ -# 1) Recreate directory tree under $DST -while IFS= read -r -d '' dir; do - rel="${dir#$SRC/}" # strip leading $SRC/ → e.g. "sub/dir" - mkdir -p "$DST/$rel" -done < <(find "$SRC" -type d -print0) +# 1) Recreate directory tree under $DST +while IFS= read -r -d '' dir; do + [ "$dir" = "$SRC" ] && continue + rel="${dir#"$SRC"/}" # strip leading $SRC/ → e.g. "sub/dir" + mkdir -p "$DST/$rel" +done < <(find "$SRC" -type d -print0) @@ -# 2) Move all files into that mirror -while IFS= read -r -d '' file; do - rel="${file#$SRC/}" # e.g. "sub/dir/file.txt" +# 2) Move all files into that mirror +while IFS= read -r -d '' file; do + rel="${file#"$SRC"/}" # e.g. "sub/dir/file.txt"This makes the cleanup effective and addresses the ShellCheck SC2295 warning about quoting the pattern part of the expansion.
♻️ Duplicate comments (33)
.github/workflows/tag-on-merge.yaml (2)
17-21: Breaking change: actions/checkout@v5.0.0 requires Node.js 24 and runner v2.327.1+.The upgrade from v4.2.2 to v5.0.0 introduces a breaking change: the action now runs on Node.js 24 and requires Actions Runner v2.327.1 or later. Ensure your GitHub-hosted and self-hosted runners meet this requirement; older runners will fail to execute this version.
If you cannot upgrade your runners, consider pinning to v4 or using an intermediate version that is compatible with your current runner baseline.
23-23: Commit hash does not match the v6.2 release tag.The v6.2 release of
mathieudutour/github-tag-actionis pinned to commitd28fa2ccfbd16e871a4bdf35e11b3ad1bd56c0c1, but the workflow pins toa22cf08638b34d5badda920f9daf6e72c477b07b. This mismatch could mask updates or introduce unexpected behavior changes.Update the commit hash to
d28fa2ccfbd16e871a4bdf35e11b3ad1bd56c0c1to match the official v6.2 release.Apply this diff to correct the commit hash:
- uses: mathieudutour/github-tag-action@a22cf08638b34d5badda920f9daf6e72c477b07b # v6.2 + uses: mathieudutour/github-tag-action@d28fa2ccfbd16e871a4bdf35e11b3ad1bd56c0c1 # v6.2template/.gitignore (1)
79-82: Remove redundant*.DS_Storepattern.The pattern
*.DS_Store(line 80) is redundant—it matches files likefoo.DS_Store, not macOS metadata. Only.DS_Store(line 81) is the actual macOS convention. Keep line 81 and remove line 80 to reduce noise.Apply this diff:
# macOS dev cleanliness -*.DS_Store .DS_Store.gitignore (1)
79-82: Remove redundant*.DS_Storepattern.The pattern
*.DS_Store(line 80) is redundant—it matches files likefoo.DS_Store, not macOS metadata. Only.DS_Store(line 81) is the actual macOS convention. Keep line 81 and remove line 80 to reduce noise. This same issue exists intemplate/.gitignoreand should be fixed there as well.Apply this diff:
# macOS dev cleanliness -*.DS_Store .DS_Storetemplate/.github/dependabot.yml.jinja (1)
11-18: Template is missing the general patch update ignore rule.This template lacks the general ignore rule present in
.github/dependabot.yml(lines 12-14) that ignores all semver-patch updates except security updates. This inconsistency means the template will generate a different Dependabot configuration than the root file..github/pull_request_template.md (1)
1-1: Fix markdown linting: convert to top-level heading.Align with the template PR template by converting to H1. The first line should use
#instead of##to resolve MD041 and MD023 linting violations.-## Link to Issue or Message thread +# Link to Issue or Message threadtemplate/.github/pull_request_template.md (1)
1-1: Fix markdown linting: convert to top-level heading.The first line should be a top-level H1 heading to resolve MD041 and MD023 linting violations flagged by markdownlint-cli2. Change
##to#on line 1.-## Link to Issue or Message thread +# Link to Issue or Message thread.devcontainer/envs.json (1)
1-7: Consider adding JSON schema reference for IDE validation.While the configuration is correctly formatted, adding a
$schemaproperty at the top level would enable IDE validation and autocompletion. This is optional but improves developer experience.+{ + "$schema": "https://raw.githubusercontent.com/devcontainers/spec/main/schemas/devContainers.schema.json", + "environments": [ { "description": "main", "relative_directory": ".", "package_manager": "uv" } + ] +}Note: Verify the correct schema URL for the DevContainers
envs.jsonspecification.CHANGELOG.md (1)
9-36:⚠️ This is a duplicate of a prior review comment.The markdown formatting issues (MD022: blank lines after headings) identified in the past review still need to be resolved. Add blank lines after each heading to comply with markdown linting standards.
.devcontainer/install-ci-tooling.py (1)
105-119: Fixpnpminstall subprocess usage (list +shell=Trueis fragile and was previously flagged).The
pnpminstall loop still mixes list arguments withshell=True:
- On Windows, you’re wrapping a
pwshlist command in an extra shell layer for no benefit.- On non‑Windows, you build
[cmd]and pass it withshell=True, which is unclear and was previously called out.Refactor to drive PowerShell explicitly on Windows and a plain shell command on Unix:
- if not args.no_node: - pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] - for cmd in pnpm_install_sequence: - cmd = ( - [ - pwsh, # type: ignore[reportPossiblyUnboundVariable] # this matches the conditional above that defines pwsh - "-NoProfile", - "-NonInteractive", - "-Command", - cmd, - ] - if is_windows - else [cmd] - ) - _ = subprocess.run(cmd, shell=True, check=True) + if not args.no_node: + pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] + for cmd in pnpm_install_sequence: + if is_windows: + _ = subprocess.run( + [ + pwsh, # type: ignore[reportPossiblyUnboundVariable] + "-NoProfile", + "-NonInteractive", + "-Command", + cmd, + ], + check=True, + ) + else: + _ = subprocess.run(cmd, shell=True, check=True)This keeps Windows using an argument list (no extra shell) and Unix using a plain string with
shell=True, resolving the earlier concern.template/.devcontainer/install-ci-tooling.py.jinja (1)
105-119: Align pnpm install subprocess usage with per‑OS expectations (mirrors non‑template script issue).The template still builds a list and then calls
subprocess.run(cmd, shell=True, check=True)for both Windows and non‑Windows. That’s brittle and was already flagged in an earlier review.Recommend mirroring the clearer per‑OS pattern:
- if not args.no_node: - pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] - for cmd in pnpm_install_sequence: - cmd = ( - [ - pwsh, # type: ignore[reportPossiblyUnboundVariable] # this matches the conditional above that defines pwsh - "-NoProfile", - "-NonInteractive", - "-Command", - cmd, - ] - if is_windows - else [cmd] - ) - _ = subprocess.run(cmd, shell=True, check=True) + if not args.no_node: + pnpm_install_sequence = ["npm -v", f"npm install -g pnpm@{PNPM_VERSION}", "pnpm -v"] + for cmd in pnpm_install_sequence: + if is_windows: + _ = subprocess.run( + [ + pwsh, # type: ignore[reportPossiblyUnboundVariable] + "-NoProfile", + "-NonInteractive", + "-Command", + cmd, + ], + check=True, + ) + else: + _ = subprocess.run(cmd, shell=True, check=True)This keeps the template behavior consistent with the fixed concrete script and avoids confusing shell semantics.
template/.github/workflows/pulumi-aws.yml (1)
101-101:actions/checkout@v5.0.0requires GitHub Actions Runner v2.327.1+ (Node.js 24).This dependency upgrade was previously flagged. Ensure your CI environment meets the minimum runner version requirement before upgrading from v4.x.
.github/reusable_workflows/build-docker-image.yaml (1)
145-151: Production tag error suppression still uses indiscriminate|| true.Line 150 continues to suppress all errors when adding the production context tag. As previously flagged, this can mask legitimate failures (permission errors, network issues) beyond the "tag already exists" scenario acknowledged in the TODO.
Consider checking for the specific error instead of silencing all failures:
aws ecr put-image ... 2>&1 | grep -v "ImageAlreadyExistsException" || trueAlternatively, query for tag existence before attempting to add it.
template/.devcontainer/docker-compose.yml.jinja (1)
24-25: Document or validateANTHROPIC_API_KEYwheninstall_claude_cliis enabled.When
install_claude_cliis true, the container expectsANTHROPIC_API_KEYfrom the host, but nothing validates or explains that requirement here. Consider:
- documenting this requirement in the template README/on‑create scripts, and/or
- adding a small runtime check that fails fast with a clear message if
ANTHROPIC_API_KEYis missing.template/.github/workflows/pre-commit.yaml (1)
20-22: Fix YAMLlint issues (boolean casing and extra space afterrun:).YAMLlint is flagging:
PYTHONUNBUFFERED: True(non‑canonical truthy), andrun: pre-commit run -a(two spaces after the colon).Both are easy to clean up and keep the workflow tooling‑friendly:
env: - PYTHONUNBUFFERED: True + PYTHONUNBUFFERED: true PRE_COMMIT_HOME: ${{ github.workspace }}/.precommit_cache ... - name: Run pre-commit - run: pre-commit run -a + run: pre-commit run -aAlso applies to: 71-72
template/.github/actions/update-devcontainer-hash/action.yml (1)
1-17: Fix composite action metadata: removepermissions, add requireddescription.Two metadata issues here:
permissionsis not a valid top‑level key inaction.ymlfor composite actions; callers must setpermissionsin their workflows instead. Keeping this block will be ignored by GitHub but will fail schema‑based tooling like actionlint. (gist.github.com)descriptionis required metadata alongsidenameandrunsin the official action schema; omitting it can also break validators. (stackoverflow.com)Suggested fix:
-name: Update Devcontainer Hash - -inputs: +name: Update Devcontainer Hash +description: 'Update the devcontainer hash for Dependabot pushes' + +inputs: @@ -8 -permissions: - contents: write -10 - -11 outputs:Workflows using this action should keep (or add) an explicit
permissions: contents: writeblock at the job or workflow level..github/actions/update-devcontainer-hash/action.yml (1)
1-17: Align action metadata with GitHub’s schema (adddescription, droppermissions).This composite action has the same metadata problems as the template version:
- No top‑level
description, which is required for custom actions. (stackoverflow.com)- A top‑level
permissionsblock, which is not supported inaction.ymland will be ignored by GitHub while failing strict schema validation. (gist.github.com)Recommended change:
-name: Update Devcontainer Hash - -inputs: +name: Update Devcontainer Hash +description: 'Update the devcontainer hash for Dependabot pushes' + +inputs: @@ -8 -permissions: - contents: write -10 - -11 outputs:Ensure any workflow invoking this action explicitly sets
permissions: contents: writeon the job that uses it..devcontainer/windows-host-helper.sh (1)
43-58: Quote$SRCin parameter expansion to avoid pattern matching (SC2295).In the directory/file loops,
${dir#$SRC/}and${file#$SRC/}treat$SRCas a glob pattern. If$SRCever contained glob metacharacters, you could get surprising matches. ShellCheck SC2295 flags this.Use a quoted prefix in the expansion:
-while IFS= read -r -d '' dir; do - rel="${dir#$SRC/}" # strip leading $SRC/ → e.g. "sub/dir" +while IFS= read -r -d '' dir; do + rel="${dir#"$SRC"/}" # strip leading $SRC/ → e.g. "sub/dir" @@ -while IFS= read -r -d '' file; do - rel="${file#$SRC/}" # e.g. "sub/dir/file.txt" +while IFS= read -r -d '' file; do + rel="${file#"$SRC"/}" # e.g. "sub/dir/file.txt"This keeps the logic the same while making the prefix match literal and robust.
.github/workflows/pre-commit.yaml (1)
72-72: Fix the extra space after the colon.There is an extra space after
run:that violates YAML formatting rules.Apply this diff:
- run: pre-commit run -a + run: pre-commit run -atemplate/.github/actions/install_deps/action.yml (1)
3-46: Remove invalidtypekeys from composite action inputs.Composite actions do not support the
typekey in input definitions. Onlydescription,required, anddefaultare valid. Thetypekey is only valid for reusable workflow inputs. This causes actionlint to fail parsing the action metadata..github/workflows/get-values.yaml (1)
45-59: Fix shell script quoting and useless echo.The shell script has several issues that can cause word splitting and includes a useless echo command.
template/.github/workflows/get-values.yaml (2)
5-11: Exposepr-short-numin workflow outputs.The job defines
pr-short-numas an output (line 26), but it's not exposed in theworkflow_call.outputssection. Downstream workflows cannot access this value.
45-59: Fix shell script quoting and useless echo.The shell script has the same issues as the non-template version: unquoted variables that can cause word splitting and a useless echo command.
.github/workflows/ci.yaml (1)
58-60: Quote the variable to prevent word splitting.The
$RUNNER_TEMPvariable should be quoted to prevent issues with paths containing spaces or special characters.template/.pre-commit-config.yaml (1)
1-1: Consider upgrading to pre-commit 4.4.0.Version 4.2.0 (released March 2025) had post-release patches (4.2.0.post1 and 4.2.0.post2) indicating bugs were discovered. The latest stable version is 4.4.0 (November 2025).
copier.yml (3)
14-17: Usefalseinstead ofnofor boolean default.YAML accepts
noas falsy, butfalseis the canonical form and avoids linter warnings (YAMLlint flagged this).install_claude_cli: type: bool help: Should the Claude CLI be installed in the devcontainer? - default: no + default: false
35-38: Self-referencing default will cause an error or undefined behavior.The default
"{{ node_version }}"references itself, which will either fail or produce an empty/undefined value. This should be a literal value.node_version: type: str help: What version of NodeJS is used for development? - default: "{{ node_version }}" + default: "22.11.0"
56-65: Theor Truemakes these conditions always true.Both
aws_identity_center_idandaws_org_home_regionhavewhenclauses ending withor True, which makes the conditions always evaluate to true. If this is intentional (always show these fields), remove the entirewhenclause for clarity. If not, remove theor True.aws_identity_center_id: type: str help: What's the ID of your Organization's AWS Identity center, e.g. d-9145c20053? - when: "{{ python_package_registry == 'AWS CodeArtifact' or install_aws_ssm_port_forwarding_plugin or True }}" + when: "{{ python_package_registry == 'AWS CodeArtifact' or install_aws_ssm_port_forwarding_plugin }}" aws_org_home_region: type: str help: What is the home region of the AWS Organization (where all of the central infrastructure is deployed)? default: us-east-1 - when: "{{ python_package_registry == 'AWS CodeArtifact' or install_aws_ssm_port_forwarding_plugin or True }}" + when: "{{ python_package_registry == 'AWS CodeArtifact' or install_aws_ssm_port_forwarding_plugin }}".devcontainer/manual-setup-deps.py (2)
134-148: Potential command injection risk when joining PNPM arguments on Windows.Using
" ".join(pnpm_command)to construct a PowerShell command string could be problematic ifenv.pathcontains spaces or special characters. While the path is derived from configuration, consider using proper quoting:if is_windows: pwsh = shutil.which("pwsh") or shutil.which("powershell") if not pwsh: raise FileNotFoundError("Neither 'pwsh' nor 'powershell' found on PATH") + # Quote path elements to handle spaces + quoted_path = f'"{env.path}"' pnpm_command = [ pwsh, "-NoProfile", "-NonInteractive", "-Command", - " ".join(pnpm_command), + f"pnpm install --dir {quoted_path}" + (" --frozen-lockfile" if env_check_lock else ""), ]
153-160: Consider logging stderr from the hash update subprocess.The script captures both stdout and stderr but only prints stdout. If the hash script fails silently or logs warnings to stderr, this information is lost.
result = subprocess.run( [sys.executable, ".github/workflows/hash_git_files.py", ".", "--for-devcontainer-config-update", "--exit-zero"], capture_output=True, text=True, check=True, cwd=REPO_ROOT_DIR, ) print(result.stdout) + if result.stderr: + print(result.stderr, file=sys.stderr)template/.github/workflows/hash_git_files.py (3)
68-73: Use specific exception type instead of string matching.Checking
"[Errno 21] Is a directory"via string matching is fragile across Python versions and platforms. CatchIsADirectoryErrordirectly.- except Exception as e: - if "[Errno 21] Is a directory" in str(e): - # Ignore symlinks that on windows sometimes get confused as being directories - continue + except IsADirectoryError: + # Ignore symlinks that on windows sometimes get confused as being directories + continue + except Exception as e: print(f"Error reading file {file}: {e}", file=sys.stderr) raise
78-91: Potential off-by-one when no closing brace is found.If
find_devcontainer_hash_linedoesn't find a}, it returns(-1, None). The caller inupdate_devcontainer_context_hashwould then insert at index-1, which inserts at the second-to-last position rather than failing gracefully.Consider adding validation in
update_devcontainer_context_hash:line_index, current_hash = find_devcontainer_hash_line(lines) + if line_index == -1: + raise ValueError(f"Could not find closing '}}' in {devcontainer_json_file}") new_hash_line = f"{DEVCONTAINER_COMMENT_LINE_PREFIX}{new_hash}{DEVCONTAINER_COMMENT_LINE_SUFFIX}\n"
136-141: Consider documenting exit code semantics in help text.When
--for-devcontainer-config-updateis used, the script exits with code 1 when the hash changes (unless--exit-zerois set). This is useful for pre-commit hooks but could surprise other callers._ = parser.add_argument( "--for-devcontainer-config-update", action="store_true", - help="Update the hash in the devcontainer.json file based on all files relevant to devcontainer context", + help="Update the hash in the devcontainer.json file based on all files relevant to devcontainer context. Exits with code 1 if the hash changed (useful for pre-commit hooks), unless --exit-zero is also specified.", )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (78)
.coderabbit.yaml(1 hunks).copier-answers.yml(1 hunks).devcontainer/Dockerfile(1 hunks).devcontainer/devcontainer.json(4 hunks).devcontainer/docker-compose.yml(1 hunks).devcontainer/envs.json(1 hunks).devcontainer/install-ci-tooling.py(1 hunks).devcontainer/install-ci-tooling.sh(0 hunks).devcontainer/manual-setup-deps.py(1 hunks).devcontainer/manual-setup-deps.sh(0 hunks).devcontainer/on-create-command-boilerplate.sh(1 hunks).devcontainer/on-create-command.sh(1 hunks).devcontainer/windows-host-helper.sh(2 hunks).github/actions/install_deps/action.yml(1 hunks).github/actions/install_deps_uv/action.yml(0 hunks).github/actions/install_deps_uv/install-ci-tooling.ps1(0 hunks).github/actions/install_deps_uv/manual-setup-deps.ps1(0 hunks).github/actions/update-devcontainer-hash/action.yml(1 hunks).github/dependabot.yml(1 hunks).github/pull_request_template.md(1 hunks).github/reusable_workflows/build-docker-image.yaml(1 hunks).github/workflows/ci.yaml(6 hunks).github/workflows/get-values.yaml(1 hunks).github/workflows/hash_git_files.py(1 hunks).github/workflows/pre-commit.yaml(1 hunks).github/workflows/tag-on-merge.yaml(1 hunks).github/zizmor.yml(1 hunks).gitignore(2 hunks).pre-commit-config.yaml(10 hunks)CHANGELOG.md(1 hunks)CONTRIBUTING.md(1 hunks)README.md(1 hunks)_typos.toml(2 hunks)biome.jsonc(1 hunks)copier.yml(5 hunks)extensions/context.py(1 hunks)pyproject.toml(1 hunks)pyrightconfig.json(1 hunks)ruff-test.toml(1 hunks)ruff.toml(3 hunks)template/.coderabbit.yaml(1 hunks)template/.coveragerc(1 hunks)template/.devcontainer/Dockerfile(1 hunks)template/.devcontainer/create-aws-profile.sh.jinja(1 hunks)template/.devcontainer/devcontainer.json.jinja(2 hunks)template/.devcontainer/docker-compose.yml.jinja(2 hunks)template/.devcontainer/envs.json(1 hunks)template/.devcontainer/install-ci-tooling.py.jinja(1 hunks)template/.devcontainer/install-ci-tooling.sh.jinja(0 hunks)template/.devcontainer/manual-setup-deps.py(1 hunks)template/.devcontainer/manual-setup-deps.sh.jinja(0 hunks)template/.devcontainer/on-create-command-boilerplate.sh(1 hunks)template/.devcontainer/on-create-command.sh.jinja(1 hunks)template/.devcontainer/windows-host-helper.sh(2 hunks)template/.github/actions/install_deps/action.yml(1 hunks)template/.github/actions/install_deps_uv/action.yml(0 hunks)template/.github/actions/install_deps_uv/install-ci-tooling.ps1(0 hunks)template/.github/actions/install_deps_uv/manual-setup-deps.ps1(0 hunks)template/.github/actions/pulumi_ephemeral_deploy/action.yml(1 hunks)template/.github/actions/update-devcontainer-hash/action.yml(1 hunks)template/.github/dependabot.yml.jinja(1 hunks)template/.github/pull_request_template.md(1 hunks)template/.github/workflows/get-values.yaml(1 hunks)template/.github/workflows/hash_git_files.py(1 hunks)template/.github/workflows/pre-commit.yaml(1 hunks)template/.github/workflows/pulumi-aws.yml(1 hunks)template/.gitignore(2 hunks)template/.pre-commit-config.yaml(11 hunks)template/CHANGELOG.md(1 hunks)template/CONTRIBUTING.md(1 hunks)template/README.md.jinja(1 hunks)template/_typos.toml(2 hunks)template/biome.jsonc(1 hunks)template/pyrightconfig.json(1 hunks)template/ruff-test.toml(1 hunks)template/ruff.toml(3 hunks)tests/copier_data/data1.yaml(1 hunks)tests/copier_data/data2.yaml(1 hunks)
💤 Files with no reviewable changes (10)
- .devcontainer/install-ci-tooling.sh
- .github/actions/install_deps_uv/manual-setup-deps.ps1
- template/.github/actions/install_deps_uv/action.yml
- template/.devcontainer/install-ci-tooling.sh.jinja
- template/.github/actions/install_deps_uv/install-ci-tooling.ps1
- template/.devcontainer/manual-setup-deps.sh.jinja
- template/.github/actions/install_deps_uv/manual-setup-deps.ps1
- .github/actions/install_deps_uv/install-ci-tooling.ps1
- .github/actions/install_deps_uv/action.yml
- .devcontainer/manual-setup-deps.sh
🧰 Additional context used
🧬 Code graph analysis (3)
template/.devcontainer/manual-setup-deps.py (2)
.devcontainer/manual-setup-deps.py (3)
PackageManager(49-51)EnvConfig(54-66)main(69-160).github/workflows/hash_git_files.py (1)
main(130-180)
.github/workflows/hash_git_files.py (2)
.devcontainer/manual-setup-deps.py (1)
main(69-160)template/.devcontainer/manual-setup-deps.py (1)
main(69-160)
.devcontainer/manual-setup-deps.py (3)
template/.devcontainer/manual-setup-deps.py (3)
PackageManager(49-51)EnvConfig(54-66)main(69-160).github/workflows/hash_git_files.py (1)
main(130-180)template/.github/workflows/hash_git_files.py (1)
main(130-180)
🪛 actionlint (1.7.9)
.github/workflows/pre-commit.yaml
48-48: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
.github/workflows/ci.yaml
59-59: shellcheck reported issue in this script: SC2086:info:1:60: Double quote to prevent globbing and word splitting
(shellcheck)
63-63: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
.github/workflows/get-values.yaml
42-42: description is required in metadata of "Update Devcontainer Hash" action at "/home/jailuser/git/.github/actions/update-devcontainer-hash/action.yml"
(action)
49-49: shellcheck reported issue in this script: SC2046:warning:9:13: Quote this to prevent word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:5:139: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:9:20: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2086:info:9:38: Double quote to prevent globbing and word splitting
(shellcheck)
49-49: shellcheck reported issue in this script: SC2116:style:9:13: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'
(shellcheck)
🪛 Checkov (3.2.334)
template/.devcontainer/Dockerfile
[low] 1-42: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-42: Ensure that a user for the container has been created
(CKV_DOCKER_3)
.devcontainer/Dockerfile
[low] 1-22: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-22: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 LanguageTool
README.md
[uncategorized] ~15-~15: The official name of this software platform is spelled with a capital “H”.
Context: ...prepare for commit (git add .) 1. Run `python3 .github/workflows/hash_git_files.py . --for-dev...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
template/.github/pull_request_template.md
1-1: Headings must start at the beginning of the line
(MD023, heading-start-left)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
CHANGELOG.md
1-1: Headings must start at the beginning of the line
(MD023, heading-start-left)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
5-5: Headings must start at the beginning of the line
(MD023, heading-start-left)
9-9: Headings must start at the beginning of the line
(MD023, heading-start-left)
13-13: Headings must start at the beginning of the line
(MD023, heading-start-left)
17-17: Headings must start at the beginning of the line
(MD023, heading-start-left)
21-21: Headings must start at the beginning of the line
(MD023, heading-start-left)
template/CHANGELOG.md
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
.github/pull_request_template.md
1-1: Headings must start at the beginning of the line
(MD023, heading-start-left)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
README.md
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🪛 Shellcheck (0.11.0)
.devcontainer/windows-host-helper.sh
[info] 48-48: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
[info] 54-54: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
template/.devcontainer/windows-host-helper.sh
[info] 48-48: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
[info] 54-54: Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.
(SC2295)
🪛 YAMLlint (1.37.1)
template/.github/workflows/pre-commit.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[warning] 21-21: truthy value should be one of [false, true]
(truthy)
[warning] 25-25: too few spaces before comment: expected 2
(comments)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 38-38: too few spaces before comment: expected 2
(comments)
[warning] 54-54: too few spaces before comment: expected 2
(comments)
[warning] 55-55: too few spaces before comment: expected 2
(comments)
[warning] 56-56: too few spaces before comment: expected 2
(comments)
[error] 72-72: too many spaces after colon
(colons)
.github/workflows/pre-commit.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[warning] 21-21: truthy value should be one of [false, true]
(truthy)
[warning] 25-25: too few spaces before comment: expected 2
(comments)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 38-38: too few spaces before comment: expected 2
(comments)
[warning] 54-54: too few spaces before comment: expected 2
(comments)
[warning] 55-55: too few spaces before comment: expected 2
(comments)
[warning] 56-56: too few spaces before comment: expected 2
(comments)
[error] 72-72: too many spaces after colon
(colons)
copier.yml
[warning] 17-17: truthy value should be one of [false, true]
(truthy)
.github/workflows/ci.yaml
[warning] 14-14: too few spaces before comment: expected 2
(comments)
[warning] 20-20: too few spaces before comment: expected 2
(comments)
[error] 23-23: too many spaces inside brackets
(brackets)
[error] 23-23: too many spaces inside brackets
(brackets)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 27-27: too few spaces before comment: expected 2
(comments)
[warning] 50-50: too few spaces before comment: expected 2
(comments)
[error] 136-136: too many spaces inside brackets
(brackets)
[error] 136-136: too many spaces inside brackets
(brackets)
[warning] 138-138: too few spaces before comment: expected 2
(comments)
| class EnvConfig: | ||
| def __init__(self, json_dict: dict[str, Any]): | ||
| super().__init__() | ||
| self.package_manager = PackageManager(json_dict["package_manager"]) | ||
| self.path = REPO_ROOT_DIR | ||
| if "relative_directory" in json_dict: | ||
| self.path = REPO_ROOT_DIR / json_dict["relative_directory"] | ||
| if self.package_manager == PackageManager.UV: | ||
| self.lock_file = self.path / "uv.lock" | ||
| elif self.package_manager == PackageManager.PNPM: | ||
| self.lock_file = self.path / "pnpm-lock.yaml" | ||
| else: | ||
| raise NotImplementedError(f"Package manager {self.package_manager} is not supported") |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unnecessary super().__init__() call.
EnvConfig doesn't inherit from any class (implicitly inherits from object), so the super().__init__() call is unnecessary.
class EnvConfig:
def __init__(self, json_dict: dict[str, Any]):
- super().__init__()
self.package_manager = PackageManager(json_dict["package_manager"])📝 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.
| class EnvConfig: | |
| def __init__(self, json_dict: dict[str, Any]): | |
| super().__init__() | |
| self.package_manager = PackageManager(json_dict["package_manager"]) | |
| self.path = REPO_ROOT_DIR | |
| if "relative_directory" in json_dict: | |
| self.path = REPO_ROOT_DIR / json_dict["relative_directory"] | |
| if self.package_manager == PackageManager.UV: | |
| self.lock_file = self.path / "uv.lock" | |
| elif self.package_manager == PackageManager.PNPM: | |
| self.lock_file = self.path / "pnpm-lock.yaml" | |
| else: | |
| raise NotImplementedError(f"Package manager {self.package_manager} is not supported") | |
| class EnvConfig: | |
| def __init__(self, json_dict: dict[str, Any]): | |
| self.package_manager = PackageManager(json_dict["package_manager"]) | |
| self.path = REPO_ROOT_DIR | |
| if "relative_directory" in json_dict: | |
| self.path = REPO_ROOT_DIR / json_dict["relative_directory"] | |
| if self.package_manager == PackageManager.UV: | |
| self.lock_file = self.path / "uv.lock" | |
| elif self.package_manager == PackageManager.PNPM: | |
| self.lock_file = self.path / "pnpm-lock.yaml" | |
| else: | |
| raise NotImplementedError(f"Package manager {self.package_manager} is not supported") |
🤖 Prompt for AI Agents
In .devcontainer/manual-setup-deps.py around lines 54 to 66, remove the
unnecessary super().__init__() call from EnvConfig.__init__; since EnvConfig
does not subclass a custom base, deleting that line cleans up the code without
changing behavior and leaves the rest of the initializer (setting
package_manager, path, lock_file and the NotImplementedError branch) intact.
| check_lock_file = not (args.skip_check_lock or args.optionally_check_lock or generate_lock_file_only) | ||
| if args.skip_check_lock and args.optionally_check_lock: | ||
| print("Cannot skip and optionally check the lock file at the same time.") | ||
| sys.exit(1) | ||
|
|
||
| with ENVS_CONFIG.open("r") as f: | ||
| envs = json.load(f) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving validation before variable computation.
The mutual exclusion check for --skip-check-lock and --optionally-check-lock happens after check_lock_file is computed. While functionally correct (the script exits), moving the validation earlier improves readability.
+ if args.skip_check_lock and args.optionally_check_lock:
+ print("Cannot skip and optionally check the lock file at the same time.")
+ sys.exit(1)
generate_lock_file_only = args.only_create_lock
check_lock_file = not (args.skip_check_lock or args.optionally_check_lock or generate_lock_file_only)
- if args.skip_check_lock and args.optionally_check_lock:
- print("Cannot skip and optionally check the lock file at the same time.")
- sys.exit(1)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .devcontainer/manual-setup-deps.py around lines 76 to 82, the
mutual-exclusion validation for args.skip_check_lock and
args.optionally_check_lock is performed after computing check_lock_file; move
that validation earlier (immediately after args are parsed and before any
computation that depends on them), so check for the conflicting flags and exit
if both are set, then compute check_lock_file = not (args.skip_check_lock or
args.optionally_check_lock or generate_lock_file_only); this keeps logic clearer
and prevents computing dependent variables when the arguments are invalid.
| pre-commit install --install-hooks | ||
|
|
||
| sh .devcontainer/manual-setup-deps.sh --optionally-lock | ||
| python .devcontainer/manual-setup-deps.py --optionally-check-lock |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Be explicit about Python interpreter (python vs python3).
On many modern images only python3 is guaranteed to exist; python may be missing or point to Python 2. Consider calling python3 here (and in the corresponding template), or document that the devcontainer ensures a python alias to Python 3.
🤖 Prompt for AI Agents
In .devcontainer/on-create-command.sh around line 12, the script calls "python
.devcontainer/manual-setup-deps.py" which may fail on images where "python" is
missing or points to Python 2; change the invocation to "python3
.devcontainer/manual-setup-deps.py" (and update any corresponding template) or
ensure the devcontainer setup creates a "python" alias to Python 3—pick one
approach and make the call consistent across files and docs.
| python-version: | ||
| type: string | ||
| description: Python version to install | ||
| required: false # if this is set, then assume you want python dependencies installed | ||
| default: 'notUsing' | ||
| node-version: | ||
| type: string | ||
| description: Node version to install | ||
| required: false # if this is set, then assume you want node dependencies installed | ||
| default: 'notUsing' | ||
| install-deps: | ||
| required: false | ||
| default: true | ||
| type: boolean | ||
| description: Whether to run the setup-deps script, or just to setup basic CI tooling | ||
| skip-installing-ssm-plugin-manager: | ||
| required: false | ||
| default: false | ||
| type: boolean | ||
| description: Whether to explicitly skip installing the SSM Plugin manager when setting up basic CI tooling | ||
| project-dir: | ||
| type: string | ||
| description: What's the relative path to the project? | ||
| required: false | ||
| default: ./ | ||
| code-artifact-auth-role-name: | ||
| type: string | ||
| description: What's the role name to use for CodeArtifact authentication? | ||
| required: false | ||
| default: no-code-artifact | ||
| code-artifact-auth-role-account-id: | ||
| type: string | ||
| description: What's the AWS Account ID that the role is in? | ||
| required: false | ||
| code-artifact-auth-region: | ||
| type: string | ||
| description: What region should the role use? | ||
| required: false | ||
| skip-updating-devcontainer-hash: | ||
| type: boolean | ||
| description: Whether to skip updating the hash when running manual-setup-deps.py | ||
| default: true | ||
| required: false |
There was a problem hiding this comment.
Remove invalid type keys from composite action inputs.
Composite actions do not support the type key in input definitions. Only description, required, and default are valid. The type key is only valid for reusable workflow inputs. This causes the workflow to fail when referencing this action.
Apply this diff to remove the invalid keys:
inputs:
python-version:
- type: string
description: Python version to install
required: false
default: 'notUsing'
node-version:
- type: string
description: Node version to install
required: false
default: 'notUsing'
install-deps:
required: false
default: true
- type: boolean
description: Whether to run the setup-deps script, or just to setup basic CI tooling
skip-installing-ssm-plugin-manager:
required: false
default: false
- type: boolean
description: Whether to explicitly skip installing the SSM Plugin manager when setting up basic CI tooling
project-dir:
- type: string
description: What's the relative path to the project?
required: false
default: ./
code-artifact-auth-role-name:
- type: string
description: What's the role name to use for CodeArtifact authentication?
required: false
default: no-code-artifact
code-artifact-auth-role-account-id:
- type: string
description: What's the AWS Account ID that the role is in?
required: false
code-artifact-auth-region:
- type: string
description: What region should the role use?
required: false
skip-updating-devcontainer-hash:
- type: boolean
description: Whether to skip updating the hash when running manual-setup-deps.py
default: true
required: false🤖 Prompt for AI Agents
.github/actions/install_deps/action.yml lines 4-46: The composite action
declares invalid "type" keys for inputs which composite actions do not support;
remove every "type: ..." entry under each input (python-version, node-version,
install-deps, skip-installing-ssm-plugin-manager, project-dir,
code-artifact-auth-role-name, code-artifact-auth-role-account-id,
code-artifact-auth-region, skip-updating-devcontainer-hash) and leave only
supported keys (description, required, default) so the action becomes valid for
use in workflows.
| - dependency-name: "boto3" # boto3 gets patch updates way too frequently and they're usually not important | ||
| update-types: | ||
| - "version-update:semver-patch" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider removing redundant boto3 ignore rule.
The general ignore rule at lines 12-14 already ignores all patch updates (including boto3), making this specific boto3 rule redundant. While the explicit rule serves as documentation, it doesn't add functional value.
🤖 Prompt for AI Agents
In .github/dependabot.yml around lines 15 to 17, the explicit ignore rule for
boto3 is redundant because the broader ignore at lines 12-14 already excludes
all semver-patch updates; remove the boto3-specific block to avoid duplication.
Delete the three-line dependency-name/update-types stanza for boto3, or if you
want to keep a note for future maintainers, replace it with a single-line
comment referencing the general rule instead of an active ignore entry.
| 1. Create a basic devcontainer either using the Codespaces default or using the file `.devcontainer/devcontainer-to-instantiate-template.json` from [the base template repo](https://github.com/LabAutomationAndScreening/copier-base-template/blob/main/.devcontainer/devcontainer-to-instantiate-template.json) | ||
| 1. Inside that devcontainer, run `python .devcontainer/install-ci-tooling.py` to install necessary tooling to instantiate the template (you can copy/paste the script from this repo...and you can paste it in the root of the repo if you want) | ||
| 1. Delete all files currently in the repository. Optional...but makes it easiest to avoid git conflicts. | ||
| 1. Run copier to instantiate the template: `copier copy --trust gh:LabAutomationAndScreening/copier-nuxt-static-aws.git .` | ||
| 1. Run `python .devcontainer/manual-setup-deps.py --only-create-lock --allow-uv-to-install-python` to generate the lock file(s) | ||
| 1. Stage all files to prepare for commit (`git add .`) | ||
| 1. Run `python3 .github/workflows/hash_git_files.py . --for-devcontainer-config-update` to update the hash for your devcontainer file | ||
| 1. Commit the changes (optional) | ||
| 1. Rebuild your new devcontainer |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify when it’s safe to “Delete all files currently in the repository”.
As written, step “Delete all files currently in the repository” is correct for a fresh/empty target repo, but dangerous if someone runs this in a repo with existing work. I’d suggest explicitly calling out that this step assumes an empty/new repo (or that everything is backed up) to avoid accidental data loss.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: The official name of this software platform is spelled with a capital “H”.
Context: ...prepare for commit (git add .) 1. Run `python3 .github/workflows/hash_git_files.py . --for-dev...
(GITHUB)
🤖 Prompt for AI Agents
In README.md around lines 9 to 17, the instruction "Delete all files currently
in the repository" is ambiguous and dangerous; update the step to explicitly
state the preconditions and safe alternatives (e.g., only run in an empty/new
repo, ensure you have backups or a branch with existing work, or use a fresh
clone/temporary directory), and add a short warning about irreversible data loss
and a suggested command or note to back up files before deletion.
| # omit any auto-generated API client code | ||
| */generated/open_api/* | ||
| # omit CircuitPython firmware | ||
| */firmware/code.py | ||
| */firmware/boot.py |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Coverage omit additions are sensible; consider future‑proofing firmware pattern
Ignoring generated OpenAPI clients and CircuitPython entrypoint files from coverage is reasonable and keeps the signal focused on hand‑written, host‑run code. If you expect additional firmware modules under */firmware/ later, you might want a slightly broader pattern (e.g., */firmware/*.py) to avoid having to keep this list in sync; otherwise the current explicit list is fine and clearer.
🤖 Prompt for AI Agents
In template/.coveragerc around lines 6 to 10, the coverage omit currently lists
specific CircuitPython files (*/firmware/code.py and */firmware/boot.py); to
future‑proof and avoid updating this list when new firmware modules are added,
replace those explicit entries with a single broader pattern that omits all
Python files under any firmware directory (e.g., use a pattern matching
*/firmware/*.py) so new firmware files are automatically excluded from coverage.
| pre-commit install --install-hooks{% endraw %}{% if python_package_registry is not defined or python_package_registry == "PyPI" %} | ||
|
|
||
| {% raw %}sh .devcontainer/manual-setup-deps.sh --optionally-lock{% endraw %}{% endif %} | ||
| {% raw %}python .devcontainer/manual-setup-deps.py --optionally-check-lock{% endraw %}{% endif %} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Align template with a clearly defined Python runtime.
The generated on-create script will also call python .devcontainer/manual-setup-deps.py. To avoid surprises on hosts where only python3 is present, either:
- switch this to
python3, or - guarantee a
python→ Python 3 shim in the devcontainer image and note that expectation in docs.
🤖 Prompt for AI Agents
In template/.devcontainer/on-create-command.sh.jinja around lines 10-12, the
script invokes "python .devcontainer/manual-setup-deps.py" which can fail on
systems where only "python3" exists; update the invocation to use "python3
.devcontainer/manual-setup-deps.py" (or implement a small runtime check that
prefers "python" but falls back to "python3") and add a short comment or doc
note in the devcontainer docs stating the image requires a Python 3 "python"
shim or that the script uses "python3".
| ### Added | ||
| - Describe new features added in this version. | ||
|
|
||
| ### Changed | ||
| - Describe changes in existing functionality. | ||
|
|
||
| ### Deprecated | ||
| - List features that are still available but will be removed in future versions. | ||
|
|
||
| ### Removed | ||
| - List features that have been removed. | ||
|
|
||
| ### Fixed | ||
| - List any bug fixes. | ||
|
|
||
| ### Security | ||
| - Describe security fixes or improvements. | ||
|
|
||
| --- | ||
|
|
||
| ## [0.1.0] - 2024-12-18 | ||
|
|
||
| ### Added | ||
| - Project setup. | ||
| - Initial functionality implemented. | ||
|
|
There was a problem hiding this comment.
Add blank lines after subsection headings to comply with MD022.
The markdown structure violates the MD022 rule (blank lines around headings). Add a blank line after each heading (Added, Changed, Deprecated, Removed, Fixed, Security) to match markdown linting standards.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
14-14: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
17-17: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
26-26: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
33-33: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In template/CHANGELOG.md around lines 11 to 36, the subsection headings (Added,
Changed, Deprecated, Removed, Fixed, Security) lack the required blank line
after each heading per MD022; insert a single blank line immediately after each
of those headings so there is an empty line between the heading and its
list/content, and ensure the same blank line exists after the "### Added" under
the 0.1.0 section to conform with linting.
| [](https://isitmaintained.com/project/LabAutomationAndScreening/copier-nuxt-static-aws) | ||
|
|
||
| # Usage | ||
| To create a new repository using this template: | ||
| 1. Install `copier` and `copier-templates-extensions`. An easy way to do that is to copy the `.devcontainer/install-ci-tooling.sh` script in this repository into your new repo and then run it. | ||
| 2. Run copier to instantiate the template: `copier copy --trust gh:LabAutomationAndScreening/copier-nuxt-static-aws.git` | ||
| 3. Run `uv lock` to generate the lock file | ||
| 4. Commit the changes | ||
| 5. Rebuild your new devcontainer | ||
| 1. Create a basic devcontainer either using the Codespaces default or using the file `.devcontainer/devcontainer-to-instantiate-template.json` from [the base template repo](https://github.com/LabAutomationAndScreening/copier-base-template/blob/main/.devcontainer/devcontainer-to-instantiate-template.json) | ||
| 1. Inside that devcontainer, run `python .devcontainer/install-ci-tooling.py` to install necessary tooling to instantiate the template (you can copy/paste the script from this repo...and you can paste it in the root of the repo if you want) | ||
| 1. Delete all files currently in the repository. Optional...but makes it easiest to avoid git conflicts. | ||
| 1. Run copier to instantiate the template: `copier copy --trust gh:LabAutomationAndScreening/copier-nuxt-static-aws.git .` | ||
| 1. Run `python .devcontainer/manual-setup-deps.py --only-create-lock --allow-uv-to-install-python` to generate the lock file(s) | ||
| 1. Stage all files to prepare for commit (`git add .`) | ||
| 1. Run `python3 .github/workflows/hash_git_files.py . --for-devcontainer-config-update` to update the hash for your devcontainer file | ||
| 1. Commit the changes (optional) | ||
| 1. Rebuild your new devcontainer |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make Python invocation consistent in the setup instructions.
The Usage section mixes python (for install-ci-tooling.py / manual-setup-deps.py) and python3 (for hash_git_files.py). For readers this can imply different runtimes; I’d suggest standardizing on one (python3 is usually safest) or adding a short note that python is expected to be Python 3 in the devcontainer.
🤖 Prompt for AI Agents
In template/README.md.jinja around lines 5 to 17, the Usage section mixes
`python` and `python3`; update the two invocations that use `python` (the
`install-ci-tooling.py` and `manual-setup-deps.py` commands) to use `python3`
for consistency with the `hash_git_files.py` step, or alternatively add a
single-line note that `python` in this devcontainer is expected to be Python 3;
ensure the text and all three commands consistently reference `python3` (or
include the note) so readers aren’t misled about runtimes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
template/.github/workflows/ci.yaml.jinja(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). (2)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.12.7, --data-file tests/copier_data/data1.yaml)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.12.7, --data-file tests/copier_data/data2.yaml)
🔇 Additional comments (3)
template/.github/workflows/ci.yaml.jinja (3)
18-22: LGTM!The
get-valuesjob is correctly configured to call the local workflow with appropriatecontents: writepermission for updating dependabot branches.
23-31: LGTM!The lint job correctly chains after
get-values, uses the local pre-commit workflow, and has appropriate permissions for mutex operations and OIDC role assumption.
53-76: LGTM!The
required-checkjob is well-structured:
- Uses
always()with conditional failure check to act as a proper merge gate- The new step to mark Dependabot commits as succeeded correctly uses the GitHub API with appropriate context values
- Conditional dependencies and status checks properly account for the optional
plan-to-stagingjob
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/ci.yaml (1)
58-60:⚠️ Fix unquoted variable to prevent word splitting (past review not addressed).The mv command uses unquoted
$RUNNER_TEMP, which was flagged in a previous review but remains unfixed. This violates shellcheck SC2086 and can cause word splitting when paths contain spaces or special characters.Apply this diff to fix the issue:
- name: Move python script that replaces private package registry information to temp folder so it doesn't get deleted run: | - mv .github/workflows/replace_private_package_registries.py $RUNNER_TEMP + mv .github/workflows/replace_private_package_registries.py "$RUNNER_TEMP"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yaml(6 hunks)
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/ci.yaml
63-63: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yaml
[warning] 14-14: too few spaces before comment: expected 2
(comments)
[warning] 20-20: too few spaces before comment: expected 2
(comments)
[error] 23-23: too many spaces inside brackets
(brackets)
[error] 23-23: too many spaces inside brackets
(brackets)
[warning] 26-26: too few spaces before comment: expected 2
(comments)
[warning] 27-27: too few spaces before comment: expected 2
(comments)
[warning] 50-50: too few spaces before comment: expected 2
(comments)
[error] 136-136: too many spaces inside brackets
(brackets)
[error] 136-136: too many spaces inside brackets
(brackets)
[warning] 138-138: too few spaces before comment: expected 2
(comments)
⏰ 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: lint-matrix (ubuntu-24.04, 3.12.7, --data-file tests/copier_data/data2.yaml)
- GitHub Check: lint-matrix (ubuntu-24.04, 3.12.7, --data-file tests/copier_data/data1.yaml)
🔇 Additional comments (1)
.github/workflows/ci.yaml (1)
145-154: Verify the Dependabot hash commit status marking logic.The new step marks a Dependabot hash commit as succeeded via the GitHub API when
dependabot-commit-createdoutput fromget-valuesis'true'. Verify that:
- The
new-dependabot-shaoutput fromget-valuescontains a valid commit SHA- The status context name
"required-check"aligns with your CI naming conventions- The status is only marked as succeeded when it's appropriate to do so
Inspect the
.github/workflows/get-values.yamlfile to confirm the output variablesdependabot-commit-createdandnew-dependabot-shaare always populated and valid before being referenced here.
| permissions: | ||
| id-token: write | ||
| contents: write # needed for mutex | ||
| id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix YAMLlint formatting issues (spacing).
Static analysis flagged multiple spacing violations:
- Lines 14, 20, 26, 27, 50, 138: Comments require 2 spaces before the
#symbol - Lines 23, 136: Brackets should not have extra spaces (e.g.,
[ get-values ]→[get-values])
Apply this diff to fix the spacing issues:
permissions:
- id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact)
+ id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact)
jobs:
get-values:
uses: ./.github/workflows/get-values.yaml
permissions:
- contents: write # needed for updating dependabot branches
+ contents: write # needed for updating dependabot branches
pre-commit:
- needs: [ get-values ]
+ needs: [get-values]
uses: ./.github/workflows/pre-commit.yaml
permissions:
- contents: write # needed for mutex
- id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact)
+ contents: write # needed for mutex
+ id-token: write # needed to assume OIDC roles (e.g. for downloading from CodeArtifact)
with:
python-version: 3.12.7
permissions:
- contents: write # needed for mutex
+ contents: write # needed for mutex
required-check:
runs-on: ubuntu-24.04
timeout-minutes: 2
- needs: [ lint-matrix, get-values ]
+ needs: [lint-matrix, get-values]
permissions:
- statuses: write # needed for updating status on Dependabot PRs
+ statuses: write # needed for updating status on Dependabot PRsAlso applies to: 20-20, 23-23, 26-26, 27-27, 50-50, 136-136, 138-138
🧰 Tools
🪛 YAMLlint (1.37.1)
[warning] 14-14: too few spaces before comment: expected 2
(comments)
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 14,20,23,26,27,50,136,138, fix
YAMLlint spacing: ensure comments have exactly two spaces before the `#` on
lines 14,20,26,27,50,138 (e.g., change "comment #..." to "comment #...") and
remove extra spaces inside brackets on lines 23 and 136 so bracketed tokens read
like "[get-values]" not "[ get-values ]"; update those specific lines
accordingly and re-run the linter.
| - name: Install tooling | ||
| uses: ./.github/actions/install_deps | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
| uv-sync: false | ||
| node-version: 24.7.0 | ||
| install-deps: false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate and read the install_deps action definition
find . -name "action.yaml" -o -name "action.yml" | grep -i install_depsRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 184
🏁 Script executed:
#!/bin/bash
# Also try a broader search for the actions directory structure
fd "install_deps" --type dRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 74
🏁 Script executed:
#!/bin/bash
# Read the action definition file
cat -n ./.github/actions/install_deps/action.ymlRepository: LabAutomationAndScreening/copier-nuxt-static-aws
Length of output: 4128
🌐 Web query:
GitHub Actions input schema specification inputs definition format
💡 Result:
Short answer — there are two related formats:
- action metadata (action.yml for a custom action)
- Location: action.yml at repo root.
- Inputs are a mapping where each input id maps to an object with these fields:
- description: string
- required: boolean (optional; default false)
- default: scalar (optional)
Example:
inputs:
my_input:
description: "What to process"
required: true
default: "value"
(Used for JavaScript/Docker/Composite actions.) [1]
- workflow-call / workflow_dispatch inputs (reusable workflows or manual dispatch)
- In a workflow’s on.workflow_call.inputs or on.workflow_dispatch.inputs you may also declare a type for the input. Allowed types include: string, boolean, number, choice, environment (choice appears for workflow_call). Example:
on:
workflow_call:
inputs:
print_tags:
description: "Print tags?"
required: true
type: boolean
tags:
description: "Tags to print"
type: string
See docs for exact allowed types and behavior. [2]
Sources:
- Metadata syntax for GitHub Actions (action.yml inputs). [1]
- Workflow syntax — inputs for workflow_call / workflow_dispatch (types and usage). [2]
The install_deps action definition incorrectly uses type field in custom action inputs.
The ./.github/actions/install_deps/action.yml file (lines 42-46) defines the skip-updating-devcontainer-hash input with a type: boolean field. According to GitHub Actions official specification, custom action metadata (action.yml) should only use description, required, and default fields for inputs. The type field is reserved for workflow_call and workflow_dispatch inputs, not custom actions.
Remove the type field from all input definitions in the action.yml file. The inputs will function correctly without it:
skip-updating-devcontainer-hash:
description: Whether to skip updating the hash when running manual-setup-deps.py
default: true
required: falseApply this change to all inputs (python-version, node-version, install-deps, skip-installing-ssm-plugin-manager, project-dir, code-artifact-auth-role-name, code-artifact-auth-role-account-id, code-artifact-auth-region, and skip-updating-devcontainer-hash).
🧰 Tools
🪛 actionlint (1.7.9)
63-63: could not parse action metadata in "/home/jailuser/git/.github/actions/install_deps": line 4: unexpected key "type" for definition of input "skip-updating-devcontainer-hash"
(action)
🤖 Prompt for AI Agents
In .github/workflows/ci.yaml around lines 62-67 and
./.github/actions/install_deps/action.yml around lines 42-46, the custom action
inputs incorrectly include a `type` field; remove the `type` entries from all
input definitions in action.yml so each input only uses allowed keys
(description, default, required), and apply this change to the following inputs:
python-version, node-version, install-deps, skip-installing-ssm-plugin-manager,
project-dir, code-artifact-auth-role-name, code-artifact-auth-role-account-id,
code-artifact-auth-region, and skip-updating-devcontainer-hash.
1 participant
Pull in upstream template changes
Summary by CodeRabbit
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.