fix(infra): add CMD to Dockerfile and fix credentials mount path in containers-rebuild #28

Merged
code-lead merged 1 commit from dev/27 into main 2026-04-17 21:25:58 +00:00
Collaborator

Summary

Two integration bugs that would each stop the first container from running:

  • Bug 1 — Dockerfile CMD: The image had no CMD, so docker run -d <image> created the container and immediately exited. docker exec fails on a stopped container. Added CMD ["sleep", "infinity"] to keep the container alive.

  • Bug 2 — Credentials mount path: just containers-rebuild had two path mismatches:

    • Host side: was hardcoding ~/.config/claude-hooks/credentials/$name/.credentials.json; now reads agents.$name.container.credentials_host_path from agents.json with the same default as defaultHostCredentialsPath() in container.ts (~/.config/claude-hooks/claude-credentials/.credentials.json)
    • Container side: was mounting to /root/.claude/.credentials.json (wrong user, wrong path); now mounts to /home/claude/.config/claude-code/.credentials.json which matches CONTAINER_CREDENTIALS_TARGET in container.ts

Also:

  • Added just containers-smoke <name> recipe for round-trip verification (bun --version + bun /opt/claude-code/cli.js --version)
  • Added tests pinning exact CONTAINER_CREDENTIALS_TARGET value and defaultHostCredentialsPath() to catch future justfile/container.ts drift
  • Added README first-time pilot section covering credentials provisioning, local image build, container creation, smoke test, enabling container mode, and service restart

Closes #27

## Summary Two integration bugs that would each stop the first container from running: - **Bug 1 — Dockerfile CMD**: The image had no `CMD`, so `docker run -d <image>` created the container and immediately exited. `docker exec` fails on a stopped container. Added `CMD ["sleep", "infinity"]` to keep the container alive. - **Bug 2 — Credentials mount path**: `just containers-rebuild` had two path mismatches: - **Host side**: was hardcoding `~/.config/claude-hooks/credentials/$name/.credentials.json`; now reads `agents.$name.container.credentials_host_path` from `agents.json` with the same default as `defaultHostCredentialsPath()` in `container.ts` (`~/.config/claude-hooks/claude-credentials/.credentials.json`) - **Container side**: was mounting to `/root/.claude/.credentials.json` (wrong user, wrong path); now mounts to `/home/claude/.config/claude-code/.credentials.json` which matches `CONTAINER_CREDENTIALS_TARGET` in `container.ts` Also: - Added `just containers-smoke <name>` recipe for round-trip verification (`bun --version` + `bun /opt/claude-code/cli.js --version`) - Added tests pinning exact `CONTAINER_CREDENTIALS_TARGET` value and `defaultHostCredentialsPath()` to catch future justfile/`container.ts` drift - Added README first-time pilot section covering credentials provisioning, local image build, container creation, smoke test, enabling container mode, and service restart Closes #27
fix(infra): add CMD to Dockerfile and fix credentials mount path in containers-rebuild
All checks were successful
qa / qa (pull_request) Successful in 49s
qa / dockerfile (pull_request) Successful in 8s
7a9a275ce0
Bug 1: Dockerfile had no CMD, causing the container to exit immediately
after `docker run -d`. Add `CMD ["sleep", "infinity"]` so the container
stays alive for `docker exec` dispatches.

Bug 2: `just containers-rebuild` mounted credentials to the wrong paths
on both sides of the bind:
- Host side: was using `credentials/$name/.credentials.json`; now reads
  `agents.$name.container.credentials_host_path` from agents.json with
  the same fallback as `defaultHostCredentialsPath()` in container.ts
  (`~/.config/claude-hooks/claude-credentials/.credentials.json`).
- Container side: was `/root/.claude/.credentials.json`; now
  `/home/claude/.config/claude-code/.credentials.json` to match
  `CONTAINER_CREDENTIALS_TARGET` in container.ts.

Also adds:
- `just containers-smoke <name>` recipe for round-trip verification
- Tests pinning the exact CONTAINER_CREDENTIALS_TARGET value and
  defaultHostCredentialsPath to catch future justfile/container.ts drift
- README first-time pilot section documenting the full bring-up procedure

Closes #27

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dev requested review from reviewer 2026-04-17 21:24:24 +00:00
reviewer approved these changes 2026-04-17 21:25:29 +00:00
Dismissed
reviewer left a comment

All acceptance criteria met, CI green (run #1522, 58s).

Bug 1 — Dockerfile CMD
CMD ["sleep", "infinity"] correctly appended. Container will now stay alive after docker run -d, allowing docker exec to work.

Bug 2a — Container-side mount path
/root/.claude/.credentials.json/home/claude/.config/claude-code/.credentials.json. Matches CONTAINER_CREDENTIALS_TARGET exactly. The justfile comment pointing to the constant is a nice touch.

Bug 2b — Host-side credentials path
jq -r ... '// empty' + bash ${creds_file:-$default_creds} is the correct pattern: // empty suppresses null output so the shell fallback fires cleanly. Default path matches defaultHostCredentialsPath() character-for-character.

Tests
Both new tests pin concrete string values rather than just structural checks — exactly the right call for a cross-boundary contract like a justfile mount. Future constant drift will immediately produce a legible failure with the fix in the error message.

Smoke recipe
just containers-smoke <name> matches the AC's required invocation. The two-step check (bun runtime + CLI binary) is exactly what's needed to validate the image is well-formed before enabling container mode.

README
Step-by-step pilot guide is complete and covers all required steps: credentials provisioning, local image build, container creation, smoke test, agents.json flag, and service restart.

All acceptance criteria met, CI green (run #1522, 58s). **Bug 1 — Dockerfile CMD** ✅ `CMD ["sleep", "infinity"]` correctly appended. Container will now stay alive after `docker run -d`, allowing `docker exec` to work. **Bug 2a — Container-side mount path** ✅ `/root/.claude/.credentials.json` → `/home/claude/.config/claude-code/.credentials.json`. Matches `CONTAINER_CREDENTIALS_TARGET` exactly. The justfile comment pointing to the constant is a nice touch. **Bug 2b — Host-side credentials path** ✅ `jq -r ... '// empty'` + bash `${creds_file:-$default_creds}` is the correct pattern: `// empty` suppresses `null` output so the shell fallback fires cleanly. Default path matches `defaultHostCredentialsPath()` character-for-character. **Tests** ✅ Both new tests pin concrete string values rather than just structural checks — exactly the right call for a cross-boundary contract like a justfile mount. Future constant drift will immediately produce a legible failure with the fix in the error message. **Smoke recipe** ✅ `just containers-smoke <name>` matches the AC's required invocation. The two-step check (bun runtime + CLI binary) is exactly what's needed to validate the image is well-formed before enabling container mode. **README** ✅ Step-by-step pilot guide is complete and covers all required steps: credentials provisioning, local image build, container creation, smoke test, `agents.json` flag, and service restart.
code-lead deleted branch dev/27 2026-04-17 21:25:58 +00:00
reviewer approved these changes 2026-04-17 21:40:00 +00:00
reviewer left a comment

Review: fix(infra): container integration fixes

CI: green — run #1522, success, 58s on head 7a9a275.

Acceptance criteria check (issue #27)

Criterion Status
Dockerfile ends with a long-running CMD CMD ["sleep", "infinity"] added at end of Dockerfile
containers-rebuild mounts creds to exact CONTAINER_CREDENTIALS_TARGET path Changed from /root/.claude/.credentials.json/home/claude/.config/claude-code/.credentials.json:ro
Recipe reads host path from agents.json with same fallback as defaultHostCredentialsPath() jq reads agents.$n.container.credentials_host_path // empty, fallback is ~/.config/claude-hooks/claude-credentials/.credentials.json — matches container.ts
container.test.ts pins recipe/container-path agreement Two new tests: exact value of CONTAINER_CREDENTIALS_TARGET + defaultHostCredentialsPath() return value
Smoke recipe or documented command for round-trip verification just containers-smoke NAME added, runs bun --version + bun /opt/claude-code/cli.js --version
README first-time pilot section Covers credentials provisioning, local build, containers-rebuild, smoke test, agents.json opt-in, service restart

Code quality notes

  • The jq fallback pattern (// empty + bash ${creds_file:-$default}) is idiomatic and correct.
  • The comment # Target path must match CONTAINER_CREDENTIALS_TARGET in src/container.ts in the justfile is exactly the right cross-reference to leave for maintainers.
  • The test descriptions are clear about which side to fix on drift, which makes the failure message self-documenting.

No issues found. All criteria met, code is correct and safe.

## Review: fix(infra): container integration fixes **CI**: ✅ green — run #1522, success, 58s on head `7a9a275`. ### Acceptance criteria check (issue #27) | Criterion | Status | |---|---| | Dockerfile ends with a long-running CMD | ✅ `CMD ["sleep", "infinity"]` added at end of Dockerfile | | `containers-rebuild` mounts creds to exact `CONTAINER_CREDENTIALS_TARGET` path | ✅ Changed from `/root/.claude/.credentials.json` → `/home/claude/.config/claude-code/.credentials.json:ro` | | Recipe reads host path from `agents.json` with same fallback as `defaultHostCredentialsPath()` | ✅ `jq` reads `agents.$n.container.credentials_host_path // empty`, fallback is `~/.config/claude-hooks/claude-credentials/.credentials.json` — matches `container.ts` | | `container.test.ts` pins recipe/container-path agreement | ✅ Two new tests: exact value of `CONTAINER_CREDENTIALS_TARGET` + `defaultHostCredentialsPath()` return value | | Smoke recipe or documented command for round-trip verification | ✅ `just containers-smoke NAME` added, runs `bun --version` + `bun /opt/claude-code/cli.js --version` | | README first-time pilot section | ✅ Covers credentials provisioning, local build, `containers-rebuild`, smoke test, `agents.json` opt-in, service restart | ### Code quality notes - The `jq` fallback pattern (`// empty` + bash `${creds_file:-$default}`) is idiomatic and correct. - The comment `# Target path must match CONTAINER_CREDENTIALS_TARGET in src/container.ts` in the justfile is exactly the right cross-reference to leave for maintainers. - The test descriptions are clear about *which side* to fix on drift, which makes the failure message self-documenting. No issues found. All criteria met, code is correct and safe.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!28
No description provided.