fix(agents): enable container mode for designer + design-reviewer #67

Merged

code-lead merged 2 commits from fix/designer-container-mode into main

2026-04-18 22:51:47 +00:00

claude-desktop commented

2026-04-18 21:16:59 +00:00

Collaborator

Summary

The designer and design-reviewer agents have penpot_mcp: true, so
the Agent SDK is configured to spawn penpot-mcp as an MCP subprocess.
That binary ships only in claude-hooks:dev (baked in by #65 /
commit 41dc22b) — it is deliberately not installed on the host.

Without container: { enabled: true }, the SDK tries to spawn
penpot-mcp on the host, fails with ENOENT, and the agent's MCP
surface is missing the mcp__penpot__* namespace at runtime. Surfaced
while running the operational smoke on #62 after PR #66 landed the
unit-level wiring smoke.

Closes the last implicit AC on #56 ("container enabled with the
claude-hooks:dev image") — this flag wasn't flipped when the
agents landed in fd59eb0.

Scope

Minimal and deliberate: only designer and design-reviewer get the
flag. boss/dev/reviewer stay in host mode (their containers
exist but go unused today) — flipping those is a separate decision.

Test plan

just qa passes (234 tests).
Pre-condition: scripts/smoke-creds.sh designer design-reviewer
green — both containers have penpot-mcp + penpot_mcp import.
After merge, restart the claude-hooks service and re-toggle
area:design on #62 to run the operational smoke end-to-end.

## Summary The `designer` and `design-reviewer` agents have `penpot_mcp: true`, so the Agent SDK is configured to spawn `penpot-mcp` as an MCP subprocess. That binary ships **only** in `claude-hooks:dev` (baked in by #65 / commit `41dc22b`) — it is deliberately not installed on the host. Without `container: { enabled: true }`, the SDK tries to spawn `penpot-mcp` on the host, fails with ENOENT, and the agent's MCP surface is missing the `mcp__penpot__*` namespace at runtime. Surfaced while running the operational smoke on #62 after PR #66 landed the unit-level wiring smoke. Closes the last implicit AC on #56 *("container enabled with the `claude-hooks:dev` image")* — this flag wasn't flipped when the agents landed in `fd59eb0`. ## Scope Minimal and deliberate: only `designer` and `design-reviewer` get the flag. `boss`/`dev`/`reviewer` stay in host mode (their containers exist but go unused today) — flipping those is a separate decision. ## Test plan - [x] `just qa` passes (234 tests). - [x] Pre-condition: `scripts/smoke-creds.sh designer design-reviewer` green — both containers have `penpot-mcp` + `penpot_mcp` import. - [ ] After merge, restart the `claude-hooks` service and re-toggle `area:design` on #62 to run the operational smoke end-to-end.

claude-desktop added 1 commit

2026-04-18 21:16:59 +00:00

fix(agents): enable container mode for designer + design-reviewer

qa / qa (pull_request) Successful in 2m29s

Details

qa / dockerfile (pull_request) Successful in 7s

Details

88ee4e8bf6

The `designer` and `design-reviewer` agents carry `penpot_mcp: true`,
so the Agent SDK is configured to spawn `penpot-mcp` as an MCP
subprocess. That binary ships only in `claude-hooks:dev` (baked in by
PR #65 / commit 41dc22b) — it is deliberately NOT on the host. Without
`container: { enabled: true }` the SDK tries to spawn `penpot-mcp` on
the host, fails with ENOENT, and the agent's MCP surface is missing
the `mcp__penpot__*` namespace at runtime.

Closes the last implicit AC on #56 ("container enabled with the
`claude-hooks:dev` image") — the flag wasn't flipped when the agents
landed in fd59eb0, and the gap surfaced when dispatching the #62
smoke ticket.

Scope kept minimal: only the two design agents are flipped.
`boss`/`dev`/`reviewer` stay in host mode (their containers exist but
go unused) — changing those is a separate decision.

charles added 1 commit

2026-04-18 21:55:37 +00:00

fix(container): seed .claude.json alongside .credentials.json at startup

qa / qa (pull_request) Successful in 2m29s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

be753daab8

Container-mode agents hang indefinitely on their first Anthropic API
call when the bind-mount dir contains only `.credentials.json`. The
Claude CLI needs a populated `.claude.json` with `oauthAccount` to
bind the credentials to an org; in host mode the CLI writes its own
(the isolated config dir is writable), but the container's mount is
read-only, so the write fails silently and the API call stalls —
observed as 10+ minutes of zero progress while the CLI waits on a
response the server is holding.

Add `seedContainerClaudeJson(credsDir)` (container.ts) that copies
just the `oauthAccount` field from `~/.claude/.claude.json` into the
bind dir if no `.claude.json` is there yet. Idempotent, writes `{}`
when no `oauthAccount` is on the host (fail-loud instead of hang),
no-op when the host file is missing entirely.

Wire it into `main.ts` startup, once per distinct `credentialsHostDir`
across the agents that have `container.enabled: true`. Runs after
webhook config loads and before workers register — tasks see a valid
bind dir on first dispatch.

Unit tests in `container.test.ts` cover the five branches: copy,
idempotent overwrite-skip, empty-stub fallback, missing-host no-op,
and dir-creation. Parameterised via an optional `userClaudeJsonPath`
so tests don't depend on the runner's real home dir.

Surfaced while running the #56 operational smoke on #62 — the
designer task hung on Opus 4.7/4.6 until this was seeded by hand.
With the seed in place the task completed in ~2 min and posted a
real diagnostic comment listing Penpot-side blockers (expired OIDC
cookie + missing shape-creation MCP tools).

claude-desktop referenced this pull request

2026-04-18 22:44:39 +00:00

Patch Penpot MCP: add canvas primitive tools (create_file / create_page / create_frame / create_text / export_frame_png) #69

charles requested review from reviewer

2026-04-18 22:47:28 +00:00

reviewer approved these changes

2026-04-18 22:51:20 +00:00

reviewer left a comment

CI green (run #1579, 234 tests, 2m39s). ✅

config/agents.json — Correct and minimal. "container": { "enabled": true } added to designer and design-reviewer only; boss/dev/reviewer stay in host mode as intended.

container.ts — seedContainerClaudeJson — Well-implemented. Idempotent early-return on existsSync, mkdir({ recursive: true }) before write, 0o600 mode enforced, stat post-write confirms actual mode in the log line. The no-op-with-warn design on a missing host file is the right call — a console warning that says "may hang" is far clearer than silently writing a {} stub that causes a different failure mode.

container.test.ts — Five meaningful scenarios with no gaps: copy oauthAccount, idempotent (existing file wins even with a different host value), no oauthAccount → write {} stub, missing host file → no-op, create missing credentialsHostDir. Good explicit assertion in test 1 that userID / unrelated fields are not leaked into the target file.

main.ts — Startup seeding loop correctly deduplicates by credentialsHostDir with a Set before calling seedContainerClaudeJson. Errors propagate naturally from the callee (which handles its own gracefully with console.warn).

Closes the last implicit AC on #56. Approved.

CI green (run #1579, 234 tests, 2m39s). ✅ **`config/agents.json`** — Correct and minimal. `"container": { "enabled": true }` added to `designer` and `design-reviewer` only; `boss`/`dev`/`reviewer` stay in host mode as intended. **`container.ts` — `seedContainerClaudeJson`** — Well-implemented. Idempotent early-return on `existsSync`, `mkdir({ recursive: true })` before write, 0o600 mode enforced, `stat` post-write confirms actual mode in the log line. The no-op-with-warn design on a missing host file is the right call — a console warning that says "may hang" is far clearer than silently writing a `{}` stub that causes a different failure mode. **`container.test.ts`** — Five meaningful scenarios with no gaps: copy `oauthAccount`, idempotent (existing file wins even with a different host value), no `oauthAccount` → write `{}` stub, missing host file → no-op, create missing `credentialsHostDir`. Good explicit assertion in test 1 that `userID` / unrelated fields are *not* leaked into the target file. **`main.ts`** — Startup seeding loop correctly deduplicates by `credentialsHostDir` with a `Set` before calling `seedContainerClaudeJson`. Errors propagate naturally from the callee (which handles its own gracefully with `console.warn`). Closes the last implicit AC on #56. Approved.