Skip to content

fix: resolve code review findings and improve documentation consistency #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
keito4 merged 1 commit into from
Dec 30, 2025
Merged

fix: resolve code review findings and improve documentation consistency #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Lint → Test → Build → SCA → Deploy:Stg → E2E → Deploy:Prod
## 5. 環境作成

基本的にdevcontainerを使用する。
またベースはghcr.io/keito4/config-base:1.0.13を使用する。
またベースはghcr.io/keito4/config-base:1.13.1を使用する。

## 6. デプロイ

Expand Down
2 changes: 2 additions & 0 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ RUN apt-get update && apt-get install -y \
wget \
xz-utils \
shellcheck \
python3 \
python3-pip \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& NODE_VERSION=v22.14.0 \
Expand Down
13 changes: 2 additions & 11 deletions .devcontainer/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ On container startup, the following configurations are automatically applied:

### Development Tools Included

- **Languages**: Node.js 20, npm, various language runtimes
- **Languages**: Node.js 22.14.0, npm, various language runtimes
- **Version Control**: Git with advanced configuration and hooks
- **Container Tools**: Docker, docker-compose
- **Cloud Tools**: AWS CLI, Terraform, kubectl
Expand All @@ -57,16 +57,7 @@ On container startup, the following configurations are automatically applied:

### Known Issues

#### Node.js Version Compatibility

**Current Status**: The container currently uses Node.js v20.x, but some dependencies require newer versions:

- `semantic-release` (v25.0.2) requires Node.js ^22.14.0 || >= 24.10.0
- This may cause warnings during `npm install` and potentially affect semantic release functionality

**Workaround**: The current setup continues to work with warnings. Consider upgrading to Node.js 22+ when moving to production releases.

**Resolution**: Future updates should upgrade the base Node.js version in `Dockerfile` (line 19) and ensure CI/CD workflows use compatible versions.
No known issues at this time. The container uses Node.js v22.14.0, which is compatible with all dependencies including `semantic-release` (v25.0.2).

### Claude Code Integration

Expand Down
212 changes: 110 additions & 102 deletions .devcontainer/VERSIONING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,141 +1,149 @@
# DevContainer Semantic Versioning

This document explains how to implement semantic versioning for the devcontainer image releases.
This document explains how semantic versioning is implemented for the devcontainer image releases.

## Current Issue
## Overview

Currently, all devcontainer images are tagged as `latest` only. This makes it difficult to:
The devcontainer image uses **automated semantic versioning** powered by `semantic-release`. Versions are determined automatically based on conventional commit messages, eliminating the need for manual version tagging.

- Track which version of the devcontainer you're using
- Roll back to previous versions if needed
- Understand what changes were made between versions
## How It Works

## Proposed Solution
### 1. Automatic Versioning with semantic-release

### 1. Use Git Tags for Versioning
When you push to the `main` branch, the GitHub Actions workflow (`.github/workflows/docker-image.yml`) automatically:

Create git tags following semantic versioning (semver) format:
1. Analyzes commit messages since the last release
2. Determines the next version based on commit types:
- `feat:` → **Minor** version bump (1.0.0 → 1.1.0)
- `fix:`, `perf:` → **Patch** version bump (1.0.0 → 1.0.1)
- `BREAKING CHANGE:` in footer → **Major** version bump (1.0.0 → 2.0.0)
- `docs:`, `style:`, `refactor:`, `test:`, `chore:` → No version bump
3. Builds and pushes Docker images with both version tag and `latest`
4. Creates a GitHub release with auto-generated release notes

- `v1.0.0` - Major version for breaking changes
- `v1.1.0` - Minor version for new features
- `v1.0.1` - Patch version for bug fixes
### 2. Conventional Commits

### 2. Version Script
Your commit messages must follow the conventional commits format:

Use the provided `script/version.sh` to easily create version tags:
```
<type>[optional scope]: <description>

```bash
# Bump patch version (1.0.0 -> 1.0.1)
./script/version.sh --type patch
[optional body]

# Bump minor version (1.0.0 -> 1.1.0)
./script/version.sh --type minor
[optional footer(s)]
```
Comment on lines +28 to +34

@coderabbitai coderabbitai Bot Dec 30, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add language identifier to the fenced code block.

The code block showing the conventional commits format is missing a language identifier. This helps with syntax highlighting and meets markdown formatting standards.

🔎 Proposed fix 
-```
+```text
 <type>[optional scope]: <description>
 
 [optional body]
 
 [optional footer(s)]
-```
+```

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

28-28: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents 
.devcontainer/VERSIONING.md around lines 28 to 34: the fenced code block showing
the conventional commit template lacks a language identifier; update the opening
backticks to include "text" (i.e., ```text) so the block is syntax-highlighted
and compliant with markdown standards, leaving the block content unchanged and
keeping the closing backticks as-is.


# Bump major version (1.0.0 -> 2.0.0)
./script/version.sh --type major
**Examples:**

```bash
feat: add Python support to DevContainer
fix: resolve hookify plugin import error
perf: optimize Docker build cache
docs: update versioning documentation

# Preview next version without creating tag
./script/version.sh --dry-run
# Breaking change (major version)
feat!: migrate to Node.js 22

BREAKING CHANGE: Node.js 20 is no longer supported
```

### 3. GitHub Workflow Modifications
### 3. Manual Release (Optional)

The `.github/workflows/docker-image.yml` workflow needs to be updated to:
If you need to create a manual release, use the GitHub Actions workflow dispatch:

1. **Trigger on tags**: Add tag push trigger
2. **Extract version from git tag**: Use the git tag as the Docker image tag
3. **Build multiple tags**: Create both versioned tag and latest
1. Go to Actions → "Build and Release DevContainer Image"
2. Click "Run workflow"
3. Select release mode:
- `auto` (default): Use semantic-release
- `patch`: Force a patch release
- `minor`: Force a minor release
- `major`: Force a major release
- `custom`: Specify a custom version

#### Required Changes to `.github/workflows/docker-image.yml`:
## Workflow Trigger

**Add tag trigger:**
The workflow is triggered by:

```yaml
on:
push:
branches: [main]
tags: ['v*'] # Trigger on version tags
paths:
- '.devcontainer/**'
- 'features/**'
- '.github/workflows/devcontainer-image.yml'
```
- **Automatic**: Push to `main` branch
- **Manual**: workflow_dispatch (Actions UI)

**Add version extraction step:**

```yaml
- name: Extract version
id: version
run: |
if [[ $GITHUB_REF == refs/tags/* ]]; then
VERSION=${GITHUB_REF#refs/tags/}
else
VERSION=latest
fi
echo "version=$VERSION" >> $GITHUB_OUTPUT
echo "Version: $VERSION"
```
**Note**: Unlike the previous proposal, tag pushes do NOT trigger builds. Versions are created automatically by semantic-release.

**Update devcontainer build step:**

```yaml
- name: Pre-build Dev Container image
uses: devcontainers/ci@v0.3
with:
imageName: ghcr.io/${{ github.repository_owner }}/config-base
imageTag: ${{ steps.version.outputs.version }}
platforms: linux/amd64,linux/arm64
push: always
runCmd: echo done
```
## Image Tags

**Add latest tag for version releases:**
Each release creates two tags:

```yaml
- name: Tag as latest (for version tags)
if: startsWith(github.ref, 'refs/tags/v')
run: |
docker tag ghcr.io/${{ github.repository_owner }}/config-base:${{ steps.version.outputs.version }} \
ghcr.io/${{ github.repository_owner }}/config-base:latest
docker push ghcr.io/${{ github.repository_owner }}/config-base:latest
- `ghcr.io/keito4/config-base:{version}` (e.g., `1.13.1`)
- `ghcr.io/keito4/config-base:latest`

### Usage Example

**Pin to a specific version (recommended for production):**

```json
{
"image": "ghcr.io/keito4/config-base:1.13.1"
}
```

## Usage Workflow

1. **Make changes** to devcontainer configuration
2. **Test changes** locally
3. **Create version tag**:
```bash
./script/version.sh --type patch
git push origin v1.0.1
```
4. **GitHub Actions** will automatically build and push the tagged image
5. **Users can reference** specific versions:
```json
{
"image": "ghcr.io/keito4/config-base:v1.0.1"
}
```
**Use latest (for development):**

```json
{
"image": "ghcr.io/keito4/config-base:latest"
}
```

## Benefits

- **Zero manual versioning**: Versions are determined automatically
- **Consistent changelog**: Release notes generated from commit messages
- **Version tracking**: Know exactly which version you're using
- **Rollback capability**: Can easily go back to previous versions
- **Change history**: Git tags provide clear version history
- **Change history**: GitHub releases provide clear version history
- **Stability**: Production environments can pin to specific versions
- **Development**: Latest tag still available for development

## Migration Plan
## Configuration

The semantic-release configuration is in `.releaserc.json`:

```json
{
"branches": ["main"],
"plugins": [
"@semantic-release/commit-analyzer",
"@semantic-release/release-notes-generator",
"@semantic-release/github"
]
}
```

1. ✅ Create versioning script (`script/version.sh`)
2. ⚠️ Update GitHub workflow (requires manual modification due to permissions)
3. Create initial version tag (e.g., `v1.0.0`)
4. Test the new versioning workflow
5. Update documentation
## Commit Type Reference

| Type | Version Bump | Description |
| ------------------------------ | ------------ | ----------------------- |
| `feat:` | **Minor** | New feature |
| `fix:` | **Patch** | Bug fix |
| `perf:` | **Patch** | Performance improvement |
| `feat!:` or `BREAKING CHANGE:` | **Major** | Breaking change |
| `docs:` | None | Documentation only |
| `style:` | None | Code style (formatting) |
| `refactor:` | None | Code refactoring |
| `test:` | None | Adding tests |
| `chore:` | None | Maintenance tasks |

## Migration Status

- ✅ semantic-release configured and working
- ✅ GitHub workflow automated
- ✅ Conventional commits enforced via commitlint
- ✅ Multi-platform image builds (amd64/arm64)
- ✅ Docker layer caching enabled
- ✅ Automated release notes generation

## Notes

- The version script follows semantic versioning principles
- Git tags trigger the Docker image builds automatically
- Both versioned and `latest` tags are maintained
- The workflow cannot be automatically updated due to GitHub security restrictions
- The workflow skips release if no release-triggering commits are found
- Both automatic (semantic-release) and manual releases are supported
- All releases are published to GitHub Container Registry (ghcr.io)
- Release notes are automatically generated from commit history
11 changes: 9 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Install shellcheck
run: sudo apt-get update && sudo apt-get install -y shellcheck
- name: Install shellcheck and bats
run: sudo apt-get update && sudo apt-get install -y shellcheck bats

- name: Run linter
run: npm run lint
Expand All @@ -34,6 +34,13 @@ jobs:
- name: Run tests with coverage
run: npm run test:coverage

- name: Run Bats integration tests
run: |
for test_file in test/integration/*.bats; do
echo "Running $test_file"
bats "$test_file" || echo "::warning::Test $test_file failed (non-blocking)"
done
Comment on lines +37 to +42

@coderabbitai coderabbitai Bot Dec 30, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Non-blocking test failures may allow broken code to merge.

The integration tests emit warnings on failure but don't block the CI pipeline. This contradicts quality gate principles and may allow regressions to pass through.

Consider making the tests blocking:

🔎 Proposed fix to make integration tests blocking 
-      - name: Run Bats integration tests
-        run: |
-          for test_file in test/integration/*.bats; do
-            echo "Running $test_file"
-            bats "$test_file" || echo "::warning::Test $test_file failed (non-blocking)"
-          done
+      - name: Run Bats integration tests
+        run: |
+          EXIT_CODE=0
+          for test_file in test/integration/*.bats; do
+            echo "Running $test_file"
+            bats "$test_file" || EXIT_CODE=1
+          done
+          exit $EXIT_CODE

If non-blocking behavior is intentional during the rollout phase, consider adding a TODO comment with a timeline to make these tests blocking.

As per coding guidelines, CI pipelines should validate code quality with tests as blocker-level gates.

📝 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
- name: Run Bats integration tests
run: |
for test_file in test/integration/*.bats; do
echo "Running $test_file"
bats "$test_file" || echo "::warning::Test $test_file failed (non-blocking)"
done
- name: Run Bats integration tests
run: |
EXIT_CODE=0
for test_file in test/integration/*.bats; do
echo "Running $test_file"
bats "$test_file" || EXIT_CODE=1
done
exit $EXIT_CODE
🤖 Prompt for AI Agents 
.github/workflows/ci.yml lines 37-42: the integration test loop swallows
failures by echoing warnings, making test failures non-blocking; change the loop
to exit with a non-zero status on any failing bats test so CI fails (e.g., stop
catching the failure or collect exit codes and exit 1 if any failed), or if
non-blocking is intentional add a TODO comment explaining rollout and a deadline
and document the rationale.


- name: Upload coverage reports
if: always()
uses: codecov/codecov-action@v4
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/docker-image.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ jobs:
if: steps.release.outputs.skip_release != 'true'
working-directory: ${{ github.workspace }}
run: |
docker run --rm -v "$PWD":/workspace -w /workspace ghcr.io/${{ github.repository_owner }}/config-base:latest bash -lc 'brew --version; terraform --version; jq --version > devcontainer-info.txt'
docker run --rm -v "$PWD":/workspace -w /workspace ghcr.io/${{ github.repository_owner }}/config-base:latest bash -lc '{ brew --version; terraform --version; jq --version; } > devcontainer-info.txt'

- uses: actions/upload-artifact@v4
if: steps.release.outputs.skip_release != 'true'
Expand Down
8 changes: 1 addition & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: version-patch version-minor version-major version-dry-run credentials clean-credentials list-credentials brew-leaves brew-categorized brew-generate brew-deps brew-uses claude-setup claude-sync claude-plugins
.PHONY: version-patch version-minor version-major version-dry-run credentials clean-credentials list-credentials brew-leaves brew-categorized brew-generate brew-deps brew-uses claude-setup

# Semantic versioning for devcontainer
version-patch:
Expand Down Expand Up @@ -47,9 +47,3 @@ brew-uses: ## Show packages that depend on a specific package
# Claude Code setup
claude-setup: ## Setup Claude Code (sync settings + install plugins)
@./script/setup-claude.sh

claude-sync: ## Sync Claude Code settings only (no plugin install)
@./script/setup-claude.sh --sync-only

claude-plugins: ## Install Claude Code plugins only
@./script/setup-claude.sh --plugins-only
13 changes: 7 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ The `.claude/` directory contains Claude Code configuration that is partially ve
- `agents/` - Specialized agent configurations
- `hooks/` - Event-driven automation scripts
- `plugins/config.json` - Custom plugin repository configuration
- `plugins/known_marketplaces.json` - List of plugin marketplaces
- `plugins/known_marketplaces.json.template` - Template for plugin marketplace configuration (generates `known_marketplaces.json` locally)
- `CLAUDE.md` - Global development standards and guidelines

### Local-Only Files (Git-Ignored)
Expand All @@ -146,7 +146,7 @@ Claude Code設定は`export.sh`と`import.sh`スクリプトで自動的に同
- `commands/` - カスタムスラッシュコマンド
- `agents/` - 専用エージェント設定
- `hooks/` - イベント駆動の自動化スクリプト
- `plugins/config.json`, `plugins/known_marketplaces.json` - プラグイン設定
- `plugins/config.json`, `plugins/known_marketplaces.json.template` - プラグイン設定（テンプレート）
- `CLAUDE.md` - 開発標準とガイドライン

**同期されない設定（ローカル専用）**
Expand All @@ -162,9 +162,10 @@ Claude Code設定は`export.sh`と`import.sh`スクリプトで自動的に同

Plugin configuration is managed through two layers:

1. **Marketplace Configuration** (version-controlled in `plugins/known_marketplaces.json`)
1. **Marketplace Configuration** (template in `plugins/known_marketplaces.json.template`, generated as `known_marketplaces.json` locally)
- Defines which plugin marketplaces to use
- Shared across all team members
- Template is shared across all team members
- Generated file is local-only (not version-controlled)
- Examples: official Anthropic plugins, community repositories

2. **Plugin Activation** (local-only in `settings.local.json`)
Expand Down Expand Up @@ -412,7 +413,7 @@ This repository includes comprehensive GitHub Actions workflows and development

#### GitHub Actions Workflows

- **CI Pipeline** (`.github/workflows/ci.yml`): Automated testing, linting, and quality checks (uses Node.js 20)
- **CI Pipeline** (`.github/workflows/ci.yml`): Automated testing, linting, and quality checks (uses Node.js 22)
- **Claude Code Integration** (`.github/workflows/claude.yml`): AI-assisted code review and issue management
- **Docker Image Build** (`.github/workflows/docker-image.yml`): Containerized build and deployment pipeline
- **Library Auto-Update** (`.github/workflows/update-libraries.yml`): Scheduled Codex/Claude tooling refresh that raises a PR when `npm run update:libs` produces changes
Expand Down Expand Up @@ -583,7 +584,7 @@ Releases are automatically created when changes are pushed to the main branch.

#### Compatibility Notes

**Node.js Version Requirements**: The current semantic-release (v25.0.2) requires Node.js ^22.14.0 || >= 24.10.0, but the repository currently uses Node.js v20.x in development containers and CI. This produces warnings but continues to function. Consider upgrading Node.js versions for full compatibility.
**Node.js Version**: The repository uses Node.js v22.14.0 in development containers and CI, which is compatible with semantic-release (v25.0.2) requirements (^22.14.0 || >= 24.10.0).

### AI-Assisted Development Workflows

Expand Down
Loading