[wip] Additional devcontainer improvements#3519
Open
stbenjam wants to merge 8 commits into
Open
Conversation
Configures the openshift-eng/ai-helpers marketplace and installs the golang and typescript-lsp plugins during container setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Use the hosted Atlassian MCP server instead of the standalone CLI binary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bind-mount ~/.claude/.credentials.json (rw) so MCP OAuth tokens persist across container rebuilds. The init script ensures the file exists on the host before the container starts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Install golang plugin from ai-helpers marketplace (not claude-plugins-official) - Add anthropics/claude-plugins-official marketplace for typescript-lsp - Update .claude/settings.json with enabled plugins and marketplace config Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add .mcp.json so Claude Code discovers the sippy-dev MCP server - Register sippy-dev MCP server in post-create.sh for devcontainer - Add make devcontainer-up and devcontainer-claude targets - Skip go/npm dep check for devcontainer targets Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
stbenjam
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
Contributor
WalkthroughThis PR adds Claude Code development environment integration to the devcontainer by mounting credentials, initializing configuration files, and registering Claude plugins and MCP servers (Playwright, marketplace plugins, sippy-dev) alongside build tooling targets. ChangesClaude Code Development Environment Setup
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 17✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Makefile (1)
81-82: ⚡ Quick winAdd a container state check or document the prerequisite.
The target assumes the
sippy-devcontainer is running. If it's not,podman execwill fail with a potentially unclear error message.🛡️ Recommended fix to add container state check
devcontainer-claude: + `@podman` inspect sippy-dev --format '{{.State.Running}}' 2>/dev/null | grep -q true || \ + (echo "ERROR: sippy-dev container is not running. Run 'make devcontainer-up' first." && exit 1) podman exec -it -w /workspace sippy-dev claude $(CLAUDE_ARGS)Alternatively, document in a comment that
devcontainer-upmust be run first:+# Requires devcontainer-up to be run first devcontainer-claude: podman exec -it -w /workspace sippy-dev claude $(CLAUDE_ARGS)🤖 Prompt for 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. In `@Makefile` around lines 81 - 82, The devcontainer-claude Makefile target assumes the sippy-dev container is running; add a runtime check or document the prerequisite. Update the devcontainer-claude target to first verify container state (e.g., check podman/containers for a running sippy-dev) and fail with a clear message advising to run devcontainer-up if not running, or simply add a comment above the target stating that devcontainer-up must be executed before devcontainer-claude; reference the Makefile target name devcontainer-claude and the podman exec invocation to locate where to add the check or comment.
🤖 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/devcontainer.json:
- Line 12: Replace the bind mount of the single credentials file with a bind
mount of the parent .claude directory so inode-swapping updates won’t break the
container (i.e., change the mount entry that currently binds the specific
credentials.json to bind the .claude directory instead, keeping the target
directory under the container user). Also update the init script
(init-services.sh) to create the .claude directory if missing rather than
creating the credentials file so the container can accept file replacements
performed by the Claude CLI.
In @.devcontainer/post-create.sh:
- Around line 20-27: post-create.sh runs multiple "claude" CLI commands without
verifying the CLI exists; add a defensive check at the top of the script (before
the claude mcp / plugin commands) that uses command availability (e.g., command
-v claude or which claude) and, if missing, prints a clear error message and
exits non-zero so the configuration commands (the lines invoking claude mcp add,
claude plugin marketplace add, claude plugin install, etc.) are not executed;
ensure the check is present before the block that starts with "echo '==>
Configuring Claude Code plugins...'" so all referenced invocations are
protected.
---
Nitpick comments:
In `@Makefile`:
- Around line 81-82: The devcontainer-claude Makefile target assumes the
sippy-dev container is running; add a runtime check or document the
prerequisite. Update the devcontainer-claude target to first verify container
state (e.g., check podman/containers for a running sippy-dev) and fail with a
clear message advising to run devcontainer-up if not running, or simply add a
comment above the target stating that devcontainer-up must be executed before
devcontainer-claude; reference the Makefile target name devcontainer-claude and
the podman exec invocation to locate where to add the check or comment.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: cd5e254b-654d-45c1-9b7c-9ecc8800a8c8
⛔ Files ignored due to path filters (1)
.claude/settings.jsonis excluded by!.claude/**
📒 Files selected for processing (5)
.devcontainer/devcontainer.json.devcontainer/init-services.sh.devcontainer/post-create.sh.gitignoreMakefile
| "mounts": [ | ||
| "source=${localEnv:HOME}/.config/gcloud,target=/home/vscode/.config/gcloud,type=bind,readonly" | ||
| "source=${localEnv:HOME}/.config/gcloud,target=/home/vscode/.config/gcloud,type=bind,readonly", | ||
| "source=${localEnv:HOME}/.claude/.credentials.json,target=/home/vscode/.claude/.credentials.json,type=bind" |
Contributor
There was a problem hiding this comment.
Consider mounting the .claude directory instead of the specific credentials file.
File-level bind mounts are fragile when the application performs atomic file updates (write to temporary file + rename). If the Claude CLI updates credentials by replacing the file rather than modifying it in-place, the bind mount will break and the container will still see the old inode.
Mounting the parent directory is more robust:
📁 Recommended fix to mount the directory
- "source=${localEnv:HOME}/.claude/.credentials.json,target=/home/vscode/.claude/.credentials.json,type=bind"
+ "source=${localEnv:HOME}/.claude,target=/home/vscode/.claude,type=bind"This also requires updating .devcontainer/init-services.sh accordingly (it currently creates the file, but would only need to create the directory).
🤖 Prompt for 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.
In @.devcontainer/devcontainer.json at line 12, Replace the bind mount of the
single credentials file with a bind mount of the parent .claude directory so
inode-swapping updates won’t break the container (i.e., change the mount entry
that currently binds the specific credentials.json to bind the .claude directory
instead, keeping the target directory under the container user). Also update the
init script (init-services.sh) to create the .claude directory if missing rather
than creating the credentials file so the container can accept file replacements
performed by the Claude CLI.
Contributor
|
Scheduling required tests: |
Contributor
|
@stbenjam: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
openshift-ci
Bot
added
the
needs-rebase
Contributor
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
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
2 participants
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 CodeRabbit
devcontainer-upanddevcontainer-claudemake targets for simplified devcontainer management and command execution.