Skip to content

AWS extension fix#114

Merged
ejfine merged 10 commits intomainfrom
aws-fix
Dec 1, 2025
Merged

AWS extension fix#114
ejfine merged 10 commits intomainfrom
aws-fix

Conversation

@ejfine
Copy link
Contributor

@ejfine ejfine commented Dec 1, 2025

Why is this change necessary?

AWS extension is causing issues

How does this change address the issue?

disable installing it

What side effects does this change have?

N/A

How is this change tested?

Downstream repos

Other

made some fixes to the windows helper script

Summary by CodeRabbit

  • Chores

    • Updated AWS CLI and pnpm versions in development environment
    • Refined .gitignore patterns for macOS system files
  • Bug Fixes

    • Improved error handling during file processing
  • New Features

    • Added pull request metadata output to workflows
  • Refactor

    • Streamlined deployment synchronization logic
    • Enhanced template conditionals for flexible configuration

✏️ Tip: You can customize this high-level summary in your review settings.

@ejfine ejfine self-assigned this Dec 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Walkthrough

This PR encompasses configuration updates, version bumps, and script refactoring across development infrastructure. Changes include AWS CLI feature version increments, Windows host helper script optimization using rsync, pnpm version update to 10.24.0, template logic adjustments for conditional feature inclusion, and addition of a new GitHub workflow output.

Changes

Cohort / File(s) Summary
Devcontainer AWS Configuration
.devcontainer/devcontainer.json, template/.devcontainer/devcontainer.json.jinja-base
AWS CLI feature version bumped from 1.1.1 to 1.1.2 (CLI version 2.27.14 → 2.32.6); AWS Toolkit extension explicitly excluded via "-AmazonWebServices.aws-toolkit-vscode" entry; template version adds conditional logic gating aws-cli feature and Python feature configuration based on is_child_of_copier_base_template check
Development Utilities
.devcontainer/windows-host-helper.sh, src/hash_git_files.py
windows-host-helper.sh refactored to use rsync-based merge strategy replacing rm-rf cleanup; error handling simplified and bash requirement enforced; hash_git_files.py improved with targeted IsADirectoryError exception handling and Pythonic reversed loop implementation
Dependency Versions
extensions/context.py
pnpm_version updated from "10.23.0" to "10.24.0"
GitHub Workflows
template/.github/workflows/get-values.yaml.jinja-base, template/.github/workflows/pre-commit.yaml.jinja-base
New workflow_call output pr-short-num added to get-values workflow; pre-commit workflow whitespace normalization applied to "Run pre-commit" step
Git Configuration
.gitignore
DS_Store ignore patterns consolidated from \\*.DS_Store and .DS_Store to single recursive pattern **/.DS_Store

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • windows-host-helper.sh: Refactoring introduces meaningful logic changes with rsync-based merge strategy and elevated-privilege cleanup that warrants verification of behavior preservation
  • template/.devcontainer/devcontainer.json.jinja-base: New conditional template logic around aws-cli feature and extension inclusion requires validation of template context handling across different scenarios

Possibly related PRs

  • Claude code feature #99: Modifies devcontainer feature configuration for aws-cli and VS Code extension handling in the same template files
  • Readme update #72: Updates version variables in extensions/context.py context hook with similar structural modifications
  • Update Lint config #70: Modifies both extensions/context.py and devcontainer configuration files for version updates and context hash synchronization

Pre-merge checks

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'AWS extension fix' is vague and doesn't convey meaningful specifics about the changeset, which includes devcontainer updates, AWS CLI version bumps, shell script modifications, and template changes. Consider a more descriptive title such as 'Disable AWS Toolkit extension and update devcontainer dependencies' to better reflect the main changes.
Description check ❓ Inconclusive The description follows the required template structure but provides minimal detail: 'AWS extension is causing issues' and 'disable installing it' lack specificity, the testing claim references only downstream repos without explaining validation, and the mention of Windows helper script fixes is unexplained. Expand descriptions with specific details: explain what issues the extension causes, clarify which downstream repos were tested and how, and document the Windows helper script changes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ea647a0 and d009c10.

📒 Files selected for processing (8)
  • .devcontainer/devcontainer.json (3 hunks)
  • .devcontainer/windows-host-helper.sh (2 hunks)
  • .gitignore (1 hunks)
  • extensions/context.py (1 hunks)
  • src/hash_git_files.py (1 hunks)
  • template/.devcontainer/devcontainer.json.jinja-base (2 hunks)
  • template/.github/workflows/get-values.yaml.jinja-base (1 hunks)
  • template/.github/workflows/pre-commit.yaml.jinja-base (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/hash_git_files.py (3)
.github/workflows/hash_git_files.py (1)
  • find_devcontainer_hash_line (75-88)
template/.github/workflows/hash_git_files.py (1)
  • find_devcontainer_hash_line (75-88)
template/template/.github/workflows/hash_git_files.py (1)
  • find_devcontainer_hash_line (75-88)
🔇 Additional comments (10)
template/.github/workflows/pre-commit.yaml.jinja-base (1)

72-72: LGTM!

Whitespace normalization with no functional impact.

.gitignore (1)

80-80: LGTM!

The **/.DS_Store pattern is more comprehensive and covers .DS_Store files at any directory depth, consolidating the previous patterns.

src/hash_git_files.py (2)

68-70: LGTM!

Targeted IsADirectoryError handling is cleaner than catching generic exceptions. The comment appropriately documents the Windows symlink edge case.


77-77: LGTM!

Using reversed(range(len(lines))) is more Pythonic and consistent with the same function in other copies of this file (as shown in the relevant code snippets).

template/.github/workflows/get-values.yaml.jinja-base (1)

12-14: LGTM!

The new pr-short-num output is properly wired through the workflow call outputs → job outputs → step outputs chain. The description clearly documents its intended use for fixed-width naming.

.devcontainer/windows-host-helper.sh (1)

40-55: LGTM!

The rsync-based approach is a significant improvement over manual file operations. The excludes for volume mount directories (node_modules, .pnpm-store, .venv) and the informative log messages are well-considered.

.devcontainer/devcontainer.json (2)

6-9: LGTM!

AWS CLI feature and version bumps are reasonable updates.


22-22: LGTM!

The - prefix correctly excludes the AWS Toolkit extension from automatic installation. The comment documents the rationale well, aligning with the PR objective to address AWS extension issues.

template/.devcontainer/devcontainer.json.jinja-base (2)

5-10: LGTM!

The conditional gating of the AWS CLI feature based on is_child_of_copier_base_template is well-structured. This ensures child templates can manage AWS configuration independently while non-child templates get the feature by default.


27-28: LGTM!

Appropriately gating the AWS Toolkit extension exclusion with the same condition as the AWS CLI feature ensures consistency—the exclusion only appears when the feature that auto-installs the extension is present.

Comment on lines +30 to +31
sudo rm -rf "./$repoName" || true
sudo rm -rf "./$repoName/*.md"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Line 31 is redundant and potentially problematic.

If line 30 succeeds, the directory "./$repoName" no longer exists, making the glob "./$repoName/*.md" on line 31 meaningless. With set -e enabled and depending on shell behavior, the non-matching glob could cause an error.

Remove the redundant line:

 sudo rm -rf "./$repoName" || true
-sudo rm -rf "./$repoName/*.md"
📝 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.

Suggested change
sudo rm -rf "./$repoName" || true
sudo rm -rf "./$repoName/*.md"
sudo rm -rf "./$repoName" || true
🤖 Prompt for AI Agents
.devcontainer/windows-host-helper.sh around lines 30 to 31: remove the redundant
and potentially problematic second rm command (sudo rm -rf "./$repoName/*.md")
and keep only the directory removal (sudo rm -rf "./$repoName"); if you want to
explicitly remove markdown files without risking glob errors, replace the line
with a safe find command or a nullglob-guarded removal, but simplest fix is to
delete line 31.

context["copier_template_extensions_version"] = "0.3.3"
#######
context["pnpm_version"] = "10.23.0"
context["pnpm_version"] = "10.24.0"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

pnpm latest version December 2025

💡 Result:

As of December 1, 2025:

  • Latest stable pnpm: v10.22.0 (released Nov 12, 2025). [1]
  • pnpm 11 is available as pre-release/alpha/dev builds (not yet stable). [2]

Sources:
[1] pnpm GitHub releases.
[2] pnpm package listings / dev builds.


Version 10.24.0 appears to be ahead of the latest stable pnpm release. As of December 2025, the latest stable version is 10.22.0 (released November 12, 2025). Verify that 10.24.0 is a valid, available release or use the correct latest stable version (10.22.0).

🤖 Prompt for AI Agents
In extensions/context.py around line 20, the pnpm_version is set to "10.24.0"
which does not match the latest stable pnpm release; update this to the correct
stable version or validate availability. Replace "10.24.0" with the verified
stable version "10.22.0" (or fetch the current stable release dynamically) and
ensure any tests or docs referencing pnpm_version are updated to the same value.

@ejfine ejfine merged commit a726694 into main Dec 1, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant