Adds devcontainer config files#11
Merged
Merged
Conversation
Add Python packages for Ansible environment.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a devcontainer image/config, Ansible provisioning and dependency lists, CI workflows for devcontainer and Copilot setup, and updates pre-commit and VS Code tool settings. ChangesDevelopment Environment Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Removed the check-json hook from pre-commit configuration.
Updated versions for ansible-lint and markdownlint in pre-commit config.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The GitHub Actions workflows reference non-existent or future action versions (e.g.,
actions/checkout@v6,docker/login-action@v4.2.0,actions/setup-python@v6.2.0); pin these to currently available major versions to avoid workflow resolution failures. - In
devcontainer-ci.yml, theenvvalues use theinputscontext even for non-workflow_calltriggers; sinceinputsis only defined for reusable workflows, consider splitting the env logic or adding conditional defaults so push/PR triggers do not fail expression evaluation. - The devcontainer provisioning playbook assumes
pipxis present before runningpipx install ansible; it would be more robust to install or validatepipxfirst (e.g., viaaptor a bootstrap task) to prevent failures on base images that do not ship it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitHub Actions workflows reference non-existent or future action versions (e.g., `actions/checkout@v6`, `docker/login-action@v4.2.0`, `actions/setup-python@v6.2.0`); pin these to currently available major versions to avoid workflow resolution failures.
- In `devcontainer-ci.yml`, the `env` values use the `inputs` context even for non-`workflow_call` triggers; since `inputs` is only defined for reusable workflows, consider splitting the env logic or adding conditional defaults so push/PR triggers do not fail expression evaluation.
- The devcontainer provisioning playbook assumes `pipx` is present before running `pipx install ansible`; it would be more robust to install or validate `pipx` first (e.g., via `apt` or a bootstrap task) to prevent failures on base images that do not ship it.
## Individual Comments
### Comment 1
<location path=".github/workflows/devcontainer-ci.yml" line_range="45-54" />
<code_context>
+# GitHub Copilot cloud agent customization.
+# <https://docs.github.com/en/copilot/how-tos/use-copilot-agents/cloud-agent/customize-the-agent-environment>
+name: Copilot Setup Steps
+env:
+ # Pin external repositories to specific SHAs for reproducibility.
+ COGNI_AI_AGENTS_REF: main
</code_context>
<issue_to_address>
**issue (bug_risk):** Referencing `inputs.*` in top-level `env` will fail for non-`workflow_call` events.
Using the `inputs` context in `REQUIRED_COMMANDS` and `REQUIRED_PYTHON_PACKAGES` at the workflow `env` level will cause `push`, `pull_request`, and `schedule` runs to fail with `Unrecognized named-value: 'inputs'`, since `inputs` is only defined for `workflow_call`. Move this logic into a job-level `env` behind `if: github.event_name == 'workflow_call'`, or ensure `inputs` are only referenced inside a `workflow_call`-only job.
</issue_to_address>
### Comment 2
<location path=".github/workflows/devcontainer-ci.yml" line_range="78-79" />
<code_context>
+ packages: write # Enables push to GHCR
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout repository
+ uses: actions/checkout@v6
+ with:
+ fetch-depth: 0
</code_context>
<issue_to_address>
**issue (bug_risk):** Actions are pinned to non-existent major versions, which will cause runtime failures.
`actions/checkout@v6` (and `actions/setup-python@v6.2.0` in the other workflow) are invalid refs and will prevent the workflow from running. Please pin to existing releases, e.g. `actions/checkout@v4` and `actions/setup-python@v5`, optionally with a specific minor/patch or SHA for reproducibility.
</issue_to_address>
### Comment 3
<location path=".github/workflows/copilot-setup-steps.yml" line_range="27" />
<code_context>
+ permissions:
+ contents: read
+ steps:
+ - uses: actions/checkout@v6
+ - name: Clone agents
+ run: |
</code_context>
<issue_to_address>
**issue (bug_risk):** `actions/*` versions here are also pinned to non-existent majors.
These `actions/*` versions (e.g. `actions/checkout@v6`, `actions/setup-python@v6.2.0`, possibly `actions/cache@v5.0.5`) do not exist and will cause the workflow to fail. Please update them to currently published majors (for example `actions/checkout@v4`, `actions/setup-python@v5`, `actions/cache@v4`) and optionally add tighter pinning if deterministic behavior is required.
</issue_to_address>
### Comment 4
<location path=".github/workflows/devcontainer-ci.yml" line_range="99-113" />
<code_context>
+ # Remove /. pattern to handle repos like "org/.github" -> "org/github"
+ SAFE_REPO_NAME="${REPO_LOWER//\/\./\/}"
+ echo "name=ghcr.io/${SAFE_REPO_NAME}/devcontainer" >> "$GITHUB_OUTPUT"
+ - name: Check cache image existence
+ id: cache_check
+ continue-on-error: true
+ run: |
+ IMAGE_NAME="${{ steps.image.outputs.name }}"
+ echo "Checking if cache image exists: ${IMAGE_NAME}:latest"
+ if docker pull "${IMAGE_NAME}:latest" 2>/dev/null; then
+ echo "✓ Cache image found: ${IMAGE_NAME}:latest"
+ echo "exists=true" >> "$GITHUB_OUTPUT"
+ else
+ echo "⚠ Cache image not found: ${IMAGE_NAME}:latest"
+ echo "⚠ Build will proceed without cache (first build or image expired)"
+ echo "exists=false" >> "$GITHUB_OUTPUT"
+ fi
+ - name: Build and test dev container
</code_context>
<issue_to_address>
**suggestion:** The cache existence check sets an output that is never used.
`cache_check` sets an `exists` output that isn’t referenced later (the build step always uses `cacheFrom: ${{ steps.image.outputs.name }}`). If you don’t plan to branch on this, consider removing the step to cut noise and runtime. If you do want conditional cache usage, wire `steps.cache_check.outputs.exists` into a condition or into how `cacheFrom` is set.
```suggestion
- name: Build and test dev container
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+45
to
+54
| env: | ||
| # Keep the commands and packages in lexicographical order. | ||
| REQUIRED_COMMANDS: >- | ||
| ${{ inputs.required_commands != '' && inputs.required_commands || 'actionlint | ||
| ansible | ||
| docker | ||
| gh | ||
| make | ||
| node | ||
| npm |
There was a problem hiding this comment.
issue (bug_risk): Referencing inputs.* in top-level env will fail for non-workflow_call events.
Using the inputs context in REQUIRED_COMMANDS and REQUIRED_PYTHON_PACKAGES at the workflow env level will cause push, pull_request, and schedule runs to fail with Unrecognized named-value: 'inputs', since inputs is only defined for workflow_call. Move this logic into a job-level env behind if: github.event_name == 'workflow_call', or ensure inputs are only referenced inside a workflow_call-only job.
Comment on lines
+78
to
+79
| - name: Checkout repository | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
issue (bug_risk): Actions are pinned to non-existent major versions, which will cause runtime failures.
actions/checkout@v6 (and actions/setup-python@v6.2.0 in the other workflow) are invalid refs and will prevent the workflow from running. Please pin to existing releases, e.g. actions/checkout@v4 and actions/setup-python@v5, optionally with a specific minor/patch or SHA for reproducibility.
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@v6 |
There was a problem hiding this comment.
issue (bug_risk): actions/* versions here are also pinned to non-existent majors.
These actions/* versions (e.g. actions/checkout@v6, actions/setup-python@v6.2.0, possibly actions/cache@v5.0.5) do not exist and will cause the workflow to fail. Please update them to currently published majors (for example actions/checkout@v4, actions/setup-python@v5, actions/cache@v4) and optionally add tighter pinning if deterministic behavior is required.
Comment on lines
+99
to
+113
| - name: Check cache image existence | ||
| id: cache_check | ||
| continue-on-error: true | ||
| run: | | ||
| IMAGE_NAME="${{ steps.image.outputs.name }}" | ||
| echo "Checking if cache image exists: ${IMAGE_NAME}:latest" | ||
| if docker pull "${IMAGE_NAME}:latest" 2>/dev/null; then | ||
| echo "✓ Cache image found: ${IMAGE_NAME}:latest" | ||
| echo "exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "⚠ Cache image not found: ${IMAGE_NAME}:latest" | ||
| echo "⚠ Build will proceed without cache (first build or image expired)" | ||
| echo "exists=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
| - name: Build and test dev container |
There was a problem hiding this comment.
suggestion: The cache existence check sets an output that is never used.
cache_check sets an exists output that isn’t referenced later (the build step always uses cacheFrom: ${{ steps.image.outputs.name }}). If you don’t plan to branch on this, consider removing the step to cut noise and runtime. If you do want conditional cache usage, wire steps.cache_check.outputs.exists into a condition or into how cacheFrom is set.
Suggested change
| - name: Check cache image existence | |
| id: cache_check | |
| continue-on-error: true | |
| run: | | |
| IMAGE_NAME="${{ steps.image.outputs.name }}" | |
| echo "Checking if cache image exists: ${IMAGE_NAME}:latest" | |
| if docker pull "${IMAGE_NAME}:latest" 2>/dev/null; then | |
| echo "✓ Cache image found: ${IMAGE_NAME}:latest" | |
| echo "exists=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "⚠ Cache image not found: ${IMAGE_NAME}:latest" | |
| echo "⚠ Build will proceed without cache (first build or image expired)" | |
| echo "exists=false" >> "$GITHUB_OUTPUT" | |
| fi | |
| - name: Build and test dev container | |
| - name: Build and test dev container |
This was referenced May 29, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Add a standardized devcontainer environment with provisioning automation and CI validation, plus supporting Copilot agent setup and development tooling updates.
New Features:
Enhancements:
CI:
Summary by CodeRabbit