Refresh bench tooling and Claude profiles#2
Conversation
|
@codex review |
Reviewer's GuideThis PR refreshes the Flutter bench tooling by bumping the documented/default Flutter SDK version to 3.44.4 across guides, templates, and scripts, and adds a best-effort VS Code server extension update helper script that integrates with devcontainer lifecycle, while also touching devcontainer and Docker layering to keep the bench aligned with its parent workBenches setup. Sequence diagram for ensure-latest-vscode-extension devcontainer helpersequenceDiagram
actor Developer
participant Devcontainer as devcontainer_lifecycle
participant Script as ensure_latest_vscode_extension_sh
participant CodeCLI as code_cli
participant DevcontainerJSON as devcontainer_json
participant ExtensionsCache as extensionsCache
participant ExtensionsDir as extensions_dir
Developer->>Devcontainer: Attach to devcontainer
Devcontainer->>Script: ensure-latest-vscode-extension.sh --background
Script->>Script: parse flags (--background, --list, --worker)
Script->>CodeCLI: find_code_cli()
alt code_cli found
Script->>DevcontainerJSON: load_devcontainer_extensions()
DevcontainerJSON-->>Script: extension IDs
Script->>Script: spawn --worker via nohup
Script->>ExtensionsCache: remove cached entries for each extension
Script->>CodeCLI: code --install-extension \<id\> --force
CodeCLI-->>ExtensionsDir: write updated extension directory
ExtensionsDir-->>Script: latest installed version name
else code_cli not found
Script-->>Devcontainer: VS Code Server CLI not found, skipping updates
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 1 issue, and left some high level feedback:
- Consider parsing
devcontainer.jsonwith a JSON-aware tool (e.g.,jq) instead ofawk/sedso the extension list extraction is more robust to formatting or comments changes in the file. - The
ensure-latest-vscode-extension.shscript assumes the VS Code server layout under${HOME}/.vscode-server; if you expect variations (e.g., different server roots or insiders), it might be worth making the base path configurable via an env var. - When running with
--background, failures in the worker process are only visible in the log file; consider printing that log path again on error or adding a brief summary to help users discover update issues more easily.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider parsing `devcontainer.json` with a JSON-aware tool (e.g., `jq`) instead of `awk`/`sed` so the extension list extraction is more robust to formatting or comments changes in the file.
- The `ensure-latest-vscode-extension.sh` script assumes the VS Code server layout under `${HOME}/.vscode-server`; if you expect variations (e.g., different server roots or insiders), it might be worth making the base path configurable via an env var.
- When running with `--background`, failures in the worker process are only visible in the log file; consider printing that log path again on error or adding a brief summary to help users discover update issues more easily.
## Individual Comments
### Comment 1
<location path="scripts/ensure-latest-vscode-extension.sh" line_range="19-34" />
<code_context>
+BACKGROUND=false
+LIST_ONLY=false
+
+case "${1:-}" in
+ --background)
+ BACKGROUND=true
+ shift
+ ;;
+ --worker)
+ shift
+ ;;
+ --list)
+ LIST_ONLY=true
+ shift
+ ;;
+esac
+
+find_code_cli() {
</code_context>
<issue_to_address>
**suggestion:** Option parsing only handles the first argument, which can lead to confusing behavior when combining flags or passing unknown options.
Because the `case "${1:-}" in` block only inspects and shifts once, any subsequent flags (e.g. `--background --list`, `--help`) are treated as extension IDs. This makes combined options behave unpredictably. Please refactor to a loop-based option parser (e.g. `while [[ $# -gt 0 ]]; do case "$1" in ...`) that consumes all recognized flags and clearly separates options from extension IDs, and either rejects or ignores unknown options.
```suggestion
BACKGROUND=false
LIST_ONLY=false
while [[ $# -gt 0 ]]; do
case "$1" in
--background)
BACKGROUND=true
shift
;;
--worker)
# Deprecated/ignored; retained for compatibility
shift
;;
--list)
LIST_ONLY=true
shift
;;
--)
shift
break
;;
-*)
echo "Unknown option: $1" >&2
exit 1
;;
*)
# First non-option argument; treat remaining as extension IDs
break
;;
esac
done
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| BACKGROUND=false | ||
| LIST_ONLY=false | ||
|
|
||
| case "${1:-}" in | ||
| --background) | ||
| BACKGROUND=true | ||
| shift | ||
| ;; | ||
| --worker) | ||
| shift | ||
| ;; | ||
| --list) | ||
| LIST_ONLY=true | ||
| shift | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
suggestion: Option parsing only handles the first argument, which can lead to confusing behavior when combining flags or passing unknown options.
Because the case "${1:-}" in block only inspects and shifts once, any subsequent flags (e.g. --background --list, --help) are treated as extension IDs. This makes combined options behave unpredictably. Please refactor to a loop-based option parser (e.g. while [[ $# -gt 0 ]]; do case "$1" in ...) that consumes all recognized flags and clearly separates options from extension IDs, and either rejects or ignores unknown options.
| BACKGROUND=false | |
| LIST_ONLY=false | |
| case "${1:-}" in | |
| --background) | |
| BACKGROUND=true | |
| shift | |
| ;; | |
| --worker) | |
| shift | |
| ;; | |
| --list) | |
| LIST_ONLY=true | |
| shift | |
| ;; | |
| esac | |
| BACKGROUND=false | |
| LIST_ONLY=false | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --background) | |
| BACKGROUND=true | |
| shift | |
| ;; | |
| --worker) | |
| # Deprecated/ignored; retained for compatibility | |
| shift | |
| ;; | |
| --list) | |
| LIST_ONLY=true | |
| shift | |
| ;; | |
| --) | |
| shift | |
| break | |
| ;; | |
| -*) | |
| echo "Unknown option: $1" >&2 | |
| exit 1 | |
| ;; | |
| *) | |
| # First non-option argument; treat remaining as extension IDs | |
| break | |
| ;; | |
| esac | |
| done |
There was a problem hiding this comment.
Pull request overview
This PR refreshes the FlutterBench toolchain to align with the synchronized workBenches parent, primarily by updating Flutter version references, improving bench image robustness, and wiring in portable Claude profile mounting plus a VS Code extension refresh hook.
Changes:
- Bump documented/configured Flutter version references to 3.44.4 across templates and docs.
- Update the Layer 2 bench image (
Dockerfile.layer2) with refreshed Android/Flutter install flows and timeouts/caching. - Add a
postAttachCommandhook plus a new script to best-effort refresh VS Code remote extensions, and mount.claude-profilesinto the container.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
vscode-tasks-snippets.md |
Updates embedded Flutter version snippet to 3.44.4. |
docs/vscode-tasks-snippets.md |
Mirrors the Flutter version snippet update in docs. |
templates/flutter-devcontainer-template/scripts/README.md |
Updates displayed Flutter version in example output. |
templates/flutter-devcontainer-template/README.md |
Updates Flutter version examples and stated tool versions. |
template/README.md |
Keeps the template README aligned with updated Flutter version references. |
scripts/new-flutter-project.sh |
Updates the echoed default Flutter version for generated projects. |
scripts/ensure-latest-vscode-extension.sh |
New script to force reinstall/update remote VS Code extensions (best-effort, optionally background). |
README.md |
Updates the sample .env Flutter version value. |
env-file-docker-compose-guide.md |
Updates hard-coded/example Flutter version values throughout the guide. |
docs/template-configuration-guide.md |
Updates Flutter version examples in configuration guidance. |
docs/env-file-docker-compose-guide.md |
Mirrors the env-file guide’s version updates under /docs. |
docs/CONTAINER_COMPARISON.md |
Updates the stated Flutter SDK version in the comparison doc. |
docs/centralized-configuration-guide.md |
Updates Flutter version examples in centralized configuration docs. |
Dockerfile.layer2 |
Refreshes Android cmdline tools, changes Android SDK package installation defaults/flow, and updates Flutter install behavior. |
CONTAINER_COMPARISON.md |
Updates the stated Flutter SDK version in the root comparison doc. |
.devcontainer/devcontainer.json |
Adds post-attach extension updater hook, enables extension auto-update settings, and mounts .claude-profiles. |
| { | ||
| "name": "devBenches / flutterBench", | ||
| "initializeCommand": "bash ${localWorkspaceFolder}/../scripts/ensure-sonarqube-mcp.sh && bash ${localWorkspaceFolder}/scripts/ensure-images.sh", | ||
| "postAttachCommand": "bash /workspace/projects/workBenches/devBenches/flutterBench/scripts/ensure-latest-vscode-extension.sh --background", |
| #!/usr/bin/env bash | ||
| # Best-effort update for VS Code remote extensions. |
| # Layer 2: Flutter Bench Image | ||
| # Extends Layer 1 (dev-bench-base) with Flutter-specific tools | ||
| # Includes: Flutter SDK, Android SDK, Dart tools, mobile dev tools |
| archive="/var/cache/android-sdk-downloads/platform-tools_r${ANDROID_PLATFORM_TOOLS_VERSION}-linux.zip"; \ | ||
| for attempt in 1 2 3 4 5; do \ | ||
| curl -fL --retry 2 --connect-timeout 30 --max-time 900 -C - \ | ||
| -o "$archive" \ | ||
| "https://dl.google.com/android/repository/platform-tools_r${ANDROID_PLATFORM_TOOLS_VERSION}-linux.zip" \ | ||
| && break; \ | ||
| sleep 5; \ | ||
| done; \ | ||
| unzip -q -o "$archive" -d /opt/android-sdk; \ | ||
| chmod -R 775 /opt/android-sdk |
| ARG INSTALL_ANDROID_SDK_PACKAGES=false | ||
| ARG ANDROID_SDK_PACKAGE_INSTALL_TIMEOUT=600 | ||
| RUN if [ "$INSTALL_ANDROID_SDK_PACKAGES" = "true" ]; then \ |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faad77fceb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| unzip -q -o "$archive" -d /opt/android-sdk; \ | ||
| chmod -R 775 /opt/android-sdk | ||
|
|
||
| ARG INSTALL_ANDROID_SDK_PACKAGES=false |
There was a problem hiding this comment.
With this default set to false, the normal devcontainer build path (scripts/ensure-images.sh → scripts/rebuild-stack.sh → scripts/build-layer2.sh) never passes INSTALL_ANDROID_SDK_PACKAGES, so only platform-tools are installed and the packages that were previously always present (emulator, ndk;27.0.12077973, build-tools, and platforms) are skipped. Any FlutterBench workflow that launches an Android emulator or relies on a preinstalled NDK/build-tools now fails or downloads tools at runtime despite the bench docs still advertising a full Android SDK; either make the default true for the bench image or have the build script pass the opt-in explicitly.
Useful? React with 👍 / 👎.
|
|
||
| ### Development Tools (Lightweight) | ||
| - ✅ **Flutter SDK 3.24.0** (stable channel only) | ||
| - ✅ **Flutter SDK 3.44.4** (stable channel only) |
There was a problem hiding this comment.
This line now advertises 3.44.4 for project containers, but the template it documents still pins 3.24.0 in templates/flutter-devcontainer-template/.env.example, .devcontainer/docker-compose.yml, and .devcontainer/Dockerfile (checked with rg --hidden FLUTTER_VERSION). Users following the README without overriding FLUTTER_VERSION will build 3.24.0 rather than the documented SDK, which is especially confusing for projects depending on the newer Dart/Flutter version.
Useful? React with 👍 / 👎.
| { | ||
| "name": "devBenches / flutterBench", | ||
| "initializeCommand": "bash ${localWorkspaceFolder}/../scripts/ensure-sonarqube-mcp.sh && bash ${localWorkspaceFolder}/scripts/ensure-images.sh", | ||
| "postAttachCommand": "bash /workspace/projects/workBenches/devBenches/flutterBench/scripts/ensure-latest-vscode-extension.sh --background", |
There was a problem hiding this comment.
When the bench is opened from any host path other than $HOME/projects/workBenches/devBenches/flutterBench, this points at a non-existent script inside the container, so the extension refresh never starts and VS Code reports the lifecycle command failure. The rest of this devcontainer already uses ${localWorkspaceFolder} on the host side, but this new container-side hook hard-codes one checkout layout; use the mounted workspace/repo location instead of the fixed /workspace/projects/workBenches/... path.
Useful? React with 👍 / 👎.
| "source=${localEnv:HOME}/.agents,target=/home/${localEnv:USER}/.agents,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude,target=/home/${localEnv:USER}/.claude,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude.json,target=/home/${localEnv:USER}/.claude.json,type=bind,consistency=cached", | ||
| "source=${localEnv:HOME}/.claude-profiles,target=/home/${localEnv:USER}/.claude-profiles,type=bind,consistency=cached", |
There was a problem hiding this comment.
For users who do not have ~/.claude-profiles on the host, this new bind mount prevents the devcontainer from starting because Docker --mount bind semantics error when the source path is absent. Since these portable profiles are optional credentials, create the directory during initialization or avoid adding the mount unless the source exists.
Useful? React with 👍 / 👎.
Summary
Validation
git diff --checkbash -ndocker buildx build --checkSummary by Sourcery
Align bench tooling and documentation with the latest Flutter SDK and VS Code extension workflow.
New Features:
Documentation:
Chores: