Migrate devcontainer to Docker Compose and update toolchain - #681
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR reworks the devcontainer setup: pins the base image, expands package installs (adds jq/tree/yq, Node.js 26, ChangesDevcontainer Configuration and Tooling Update
Sequence Diagram(s)sequenceDiagram
participant ensurereposh as ensure-repos.sh
participant kilojsonc as kilo.jsonc
participant envfile as .phlex-kilo.env
participant DockerCompose
participant phlexdev as phlex-dev service
ensurereposh->>kilojsonc: read existing config
ensurereposh->>ensurereposh: rewrite fnal-litellm baseURL to host.docker.internal
ensurereposh->>envfile: write KILO_CONFIG_CONTENT
DockerCompose->>envfile: load via env_file
DockerCompose->>phlexdev: start service with mounted volumes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.devcontainer/docker-compose.yml:
- Around line 20-21: The devcontainer compose config currently treats
${HOME}/.phlex-kilo.env as required even though ensure-repos.sh only creates it
conditionally, so update the wiring to avoid startup failure when the file is
missing. Either change ensure-repos.sh so it always creates .phlex-kilo.env
(even if empty) or adjust the env_file entry in docker-compose.yml to use an
optional/conditional path. Refer to ensure-repos.sh and the env_file setting in
docker-compose.yml when making the fix.
In @.devcontainer/Dockerfile:
- Line 107: Remove the global npm update step from the devcontainer image build
so the image does not drift against the live registry on each rebuild. Update
the Dockerfile build steps to install only the required tool, and pin that
install to an explicit `@kilocode/cli` version instead of relying on npm -g
update. Use the existing Dockerfile package-install section to make the change.
- Around line 108-109: The install flow in the Dockerfile still allows lifecycle
scripts to run because `npm config set allow-scripts=...` is only policy
metadata. Update the `npm install -g `@kilocode/cli`` step to actually block
install-time scripts, using `--ignore-scripts` or the npm allowlist mechanism
supported by this image’s npm version, and keep the change localized to the
install commands in the Dockerfile.
In @.devcontainer/ensure-repos.sh:
- Around line 192-199: The env file consumed by docker-compose is only written
inside the kilo.jsonc existence check, so .phlex-kilo.env can be missing or
stale on fresh or rerun setups. Update ensure-repos.sh so it always creates or
refreshes ${HOME}/.phlex-kilo.env before Compose starts, using the
KILO_CONFIG_CONTENT assignment path in the kilo.jsonc handling block and
ensuring the file is cleared or set to an empty value when the config is absent.
- Around line 197-198: Escape the JSONC-derived KILO_CONFIG_CONTENT before
writing the .phlex-kilo.env assignment, since ensure-repos.sh currently echoes a
single-quoted value that will break if the content contains apostrophes. Update
the logic around the KILO_CONFIG_CONTENT transformation and the echo that writes
"${HOME}/.phlex-kilo.env" so any embedded single quotes are safely escaped or
encoded before persisting the env file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4568aea2-d948-4677-8e64-99f520ae50ec
📒 Files selected for processing (6)
.devcontainer/Dockerfile.devcontainer/codespace.code-workspace.devcontainer/devcontainer.json.devcontainer/docker-compose.yml.devcontainer/ensure-repos.sh.devcontainer/post-create.sh
💤 Files with no reviewable changes (2)
- .devcontainer/codespace.code-workspace
- .devcontainer/post-create.sh
📜 Review details
⚠️ CI failures not shown inline (8)
GitHub Actions: greenc-FNAL checking YAML files / yaml-check: greenc-FNAL checking YAML files
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = "success" ]; then�[0m
�[36;1m echo "✅ YAML check passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::YAML check failed."�[0m
GitHub Actions: greenc-FNAL checking YAML files / 0_yaml-check.txt: greenc-FNAL checking YAML files
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = "success" ]; then�[0m
�[36;1m echo "✅ YAML check passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::YAML check failed."�[0m
GitHub Actions: CodeQL Analysis / 2_determine-languages.txt: Migrate devcontainer to Docker Compose and update toolchain
Conclusion: failure
##[group]Run # If detection was skipped, use all languages or the provided language-matrix
�[36;1m# If detection was skipped, use all languages or the provided language-matrix�[0m
�[36;1mif [ "${SKIP_DETECTION}" = "true" ]; then�[0m
�[36;1m if [ "${GITHUB_EVENT_NAME}" = "workflow_call" ] && [ -n "${LANGUAGE_MATRIX}" ]; then�[0m
�[36;1m # Validate that language-matrix is valid JSON�[0m
�[36;1m if ! echo "${LANGUAGE_MATRIX}" | python3 -c "import sys, json; json.load(sys.stdin)" 2>/dev/null; then�[0m
�[36;1m echo "::error::Invalid language-matrix input: must be valid JSON array"�[0m
GitHub Actions: CodeQL Analysis / determine-languages: Migrate devcontainer to Docker Compose and update toolchain
Conclusion: failure
##[group]Run # If detection was skipped, use all languages or the provided language-matrix
�[36;1m# If detection was skipped, use all languages or the provided language-matrix�[0m
�[36;1mif [ "${SKIP_DETECTION}" = "true" ]; then�[0m
�[36;1m if [ "${GITHUB_EVENT_NAME}" = "workflow_call" ] && [ -n "${LANGUAGE_MATRIX}" ]; then�[0m
�[36;1m # Validate that language-matrix is valid JSON�[0m
�[36;1m if ! echo "${LANGUAGE_MATRIX}" | python3 -c "import sys, json; json.load(sys.stdin)" 2>/dev/null; then�[0m
�[36;1m echo "::error::Invalid language-matrix input: must be valid JSON array"�[0m
GitHub Actions: greenc-FNAL checking Python code / python-check: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
�[36;1m echo "✅ Python checks passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m
GitHub Actions: greenc-FNAL checking Python code / 0_python-check.txt: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run REPO_NAME="${REPO##*/}"
�[36;1mREPO_NAME="${REPO##*/}"�[0m
�[36;1mif [ "success" = 'success' ] && [ "success" = 'success' ]; then�[0m
�[36;1m echo "✅ Python checks passed."�[0m
�[36;1melse�[0m
�[36;1m echo "::error::Python checks failed. Comment '@${REPO_NAME}bot python-fix' on the PR to attempt auto-fix."�[0m
GitHub Actions: greenc-FNAL checking Python code / 1_scripts-test.txt: greenc-FNAL checking Python code
Conclusion: failure
%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_no_alerts_returns_zero_no_comment PASSED [ 24%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_below_threshold_alert_not_reported PASSED [ 24%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_missing_sarif_exits_nonzero PASSED [ 24%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_github_output_written PASSED [ 24%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_github_output_false_when_no_alerts PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_directory_of_sarif_files PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_sarif_mode_pr_ref_produces_filtered_url PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainSarifMode::test_non_integer_pr_ref_no_filtered_url PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_new_alert PASSED [ 25%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_min_level_filtering PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_github_api_error_exits_2 PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_missing_github_repository_exits_2 PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiMode::test_api_mode_skipped_when_sarif_has_baseline PASSED [ 26%]
scripts/test/test_check_codeql_alerts.py::TestMainApiModeWithPrRef::test_api_mode_pr_ref_produces_filtered_url PASSED [ 27%]
scripts/test/test_check_codeql_alerts.py::TestMainEntrypoint::test_entrypoint_no_alerts_exits_zero PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_reads_from_file PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_reads_from_stdin PASSED [ 27%]
scripts/test/test_clang_tidy_check_summary.py::TestLoadDiagnostics::test_invalid_...
GitHub Actions: greenc-FNAL checking Python code / scripts-test: greenc-FNAL checking Python code
Conclusion: failure
##[group]Run codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f
with:
files: coverage-scripts.xml
flags: scripts
name: phlex-scripts-coverage
fail_ci_if_error: false
verbose: true
root_dir: phlex-src
***REDACTED***
disable_file_fixes: false
disable_search: false
disable_safe_directory: false
disable_telem: false
dry_run: false
git_service: github
gcov_executable: gcov
handle_no_reports_found: false
recurse_submodules: false
run_command: upload-coverage
skip_validation: false
use_legacy_upload_endpoint: false
use_oidc: false
use_pypi: false
version: latest
env:
CODECOV_***REDACTED***
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
##[endgroup]
##[group]Run missing_deps=""
�[36;1mmissing_deps=""�[0m
�[36;1m�[0m
�[36;1m# Check for always-required commands�[0m
�[36;1mfor cmd in bash git curl; do�[0m
�[36;1m if ! command -v "$cmd" >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps $cmd"�[0m
�[36;1m fi�[0m
�[36;1mdone�[0m
�[36;1m�[0m
�[36;1m# Check for gpg only if validation is not being skipped�[0m
�[36;1mif [ "$INPUT_SKIP_VALIDATION" != "true" ]; then�[0m
�[36;1m if ! command -v gpg >/dev/null 2>&1; then�[0m
�[36;1m missing_deps="$missing_deps gpg"�[0m
�[36;1m fi�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1m# Report missing required dependencies�[0m
�[36;1mif [ -n "$missing_deps" ]; then�[0m
�[36;1m echo "Error: The following required dependencies are missing:$missing_deps"�[0m
�[36;1m echo "Please install these dependencies before using this action."�[0m
�[36;1m exit 1�[0m
�[36;1mfi�[0m
�[36;1m�[0m
�[36;1mecho "All required system dependencies are available."�[0m
shell: /usr/bin/sh -e {0}
env:
CODECOV_***REDACTED***
UV_PYTHON_INSTALL_DIR: /home/runner/work/_temp/uv-python-dir
UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
INPUT_SKIP_VALIDATION: false
...
🧰 Additional context used
🪛 Checkov (3.3.2)
.devcontainer/Dockerfile
[low] 1-118: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-118: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 Hadolint (2.14.0)
.devcontainer/Dockerfile
[info] 93-93: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[info] 105-105: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
🪛 Trivy (0.69.3)
.devcontainer/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
🔇 Additional comments (1)
.devcontainer/devcontainer.json (1)
3-9: LGTM!
d3ab6d9 to
b1d1d03
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
21 fixed, 1 new since branch point (486ea00) ❌ 1 new CodeQL alert since the previous PR commit
✅ 21 CodeQL alerts resolved since the previous PR commit
❌ 1 new CodeQL alert since the branch point
✅ 21 CodeQL alerts resolved since the branch point
Review the full CodeQL report for details. |
b1d1d03 to
8fdc415
Compare
- Per #681 (comment) @coderabbitai `@kilocode/cli` is left unpinned due to active development and regular improvements.
…ate Kilo auth providers The headroom proxy was split into two independent SSH-tunnelled ports (azure :9797, ow :9798) to support optimized and passthrough backends. Update devcontainer relay logic to handle both ports separately via socat relays on host.docker.internal:<port+10000>. Kilo auth.json provider keys were updated from the single fnal-litellm to fnal-azure and fnal-ow (sharing the same upstream gateway token). git-ai-commit now tries providers in order (fnal-azure, fnal-ow, fnal-litellm) when GIT_AI_COMMIT_KILO_PROVIDER is unset, ensuring backward compatibility during migration. Documentation and tests were updated to reflect the new provider fallback behavior and relay architecture.
Switch `devcontainer.json` to use `docker-compose.yml` for service definition, volume mounts, and environment variables to improve configurability and consistency. Key changes include: - Update `Dockerfile` to use a pinned base image (`2026-06-24`), install additional utilities (`jq`, `tree`, `yq`), and upgrade Node.js to 26.x. - Replace `kiro-cli` with `@kilocode/cli` installed via npm. - Refactor `KILO_CONFIG_CONTENT` handling: `ensure-repos.sh` now generates a `.phlex-kilo.env` file on the host which is loaded by Docker Compose. This ensures the VS Code plugin receives the correct configuration (including the `host.docker.internal` relay) at startup, replacing the previous `.bashrc` modification in `post-create.sh`. - Remove Copilot-related extensions and settings from the workspace and devcontainer configuration.
… Kilo data
- Switch `phlex-kilo-data` named volume to host bind mount at
`${HOME}/.local/share/kilo:ro` in docker-compose.yml
- Remove env_file reference to `${HOME}/.phlex-kilo.env`
- Update ensure-repos.sh to create required directories with secure
permissions (0700) and add `.phlex-devcontainer-tmp`
- Add post-create.sh logic to rewrite Kilo config baseURLs for headroom
proxy relays via socat, enabling `host.docker.internal` access from
rootless Podman's pasta networking
- Per #681 (comment) @coderabbitai `@kilocode/cli` is left unpinned due to active development and regular improvements.
fdf97fb to
50b4691
Compare
Clang-Tidy Check ResultsFound 5782 issue(s); none are newly introduced by this patch. All issues by check:
See inline comments for details. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #681 +/- ##
==========================================
+ Coverage 83.70% 83.88% +0.17%
==========================================
Files 172 172
Lines 6904 7192 +288
Branches 845 845
==========================================
+ Hits 5779 6033 +254
- Misses 847 882 +35
+ Partials 278 277 -1
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Switch
devcontainer.jsonto usedocker-compose.ymlfor servicedefinition, volume mounts, and environment variables to improve
configurability and consistency.
Key changes include:
Dockerfileto use a pinned base image (2026-06-24), installadditional utilities (
jq,tree,yq), and upgrade Node.js to 26.x.kiro-cliwith@kilocode/cliinstalled via npm.KILO_CONFIG_CONTENThandling:ensure-repos.shnowgenerates a
.phlex-kilo.envfile on the host which is loaded byDocker Compose. This ensures the VS Code plugin receives the correct
configuration (including the
host.docker.internalrelay) at startup,replacing the previous
.bashrcmodification inpost-create.sh.and devcontainer configuration.
Devcontainer setup
devcontainer.json-driven setup todocker-compose.ymlfor the main service, mounts, and environment wiring.phlex-devservice with workspace/sibling repo mounts, host config mounts, named volumes, andhost.docker.internalconnectivity.Build/runtime image
phlex-dev:2026-06-24.jq,tree,yq,podman,ssh, andsocat.kiro-clito@kilocode/cli.Startup/config handling
ensure-repos.shto generate a host-side.phlex-kilo.envfile from~/.config/kilo/kilo.jsonc, rewriting the provider base URL to use the headroom relay..bashrc-based propagation ofKILO_CONFIG_CONTENTfrompost-create.sh.Workspace/devcontainer cleanup
Script/host config changes
ensure-repos.shto only ensuregh,kilo, andgnupgpaths needed by the new flow.kiro-cli.