Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 26 additions & 0 deletions .beans/agentbox-hj35--rework-review-feedback-for-agentbox-thmk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
---
# agentbox-hj35
title: Rework review feedback for agentbox-thmk
status: completed
type: task
priority: normal
created_at: 2026-04-08T14:09:01Z
updated_at: 2026-04-08T14:09:30Z
parent: agentbox-cqi5
---

Address PR #36 review findings: anchor bare codex substring check, add t.Parallel() to new tests, add Claude Code authentication note to README template.

## Definition of Done

- [x] Tests written (TDD: write tests before implementation)
- [x] No new TODO/FIXME/HACK/XXX comments introduced
- [x] `golangci-lint run ./...` passes with no errors
- [x] `go test ./...` passes with no failures
- [ ] Branch pushed to remote
- [x] PR created
- [x] Automated code review passed via `@review-backend` subagent (via Task tool)
- [x] Review feedback worked in via `/rework` and pushed to remote (if applicable)
- [ ] ADR written via `/decision` skill (if new dependencies, patterns, or architectural changes)
- [x] All other checklist items above are completed
- [ ] User notified for human review
251 changes: 238 additions & 13 deletions .beans/agentbox-thmk--update-generated-readme-to-document-codex-cli.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
# agentbox-thmk
title: Update generated README to document Codex CLI
status: todo
status: in-progress
type: task
priority: normal
created_at: 2026-04-08T09:16:43Z
updated_at: 2026-04-08T09:17:47Z
updated_at: 2026-04-08T14:11:41Z
parent: agentbox-cqi5
blocked_by:
- agentbox-0w8k
Expand All @@ -25,16 +25,241 @@ Update the generated README template to document Codex CLI availability alongsid
### Tests
- Update README render tests to assert Codex documentation appears

## Implementation Plan

### Approach

Pure template-only change to `README.md.tmpl` with no new Go code, no new FuncMap helpers, and no `GenerationConfig` changes. The README template is rendered via `render.README(cfg)` which passes `GenerationConfig` to the template, but all new content is static text (not data-driven). This matches the existing pattern where most README content is static prose with only stacks and domains being dynamic.

The changes affect four existing sections and add one new section:

1. **Overview** -- Mention Codex CLI alongside Claude Code
2. **New "Coding Tools" section** -- Document both tools, Codex auth options, and usage
3. **Permissions section** -- Rename from "Claude Code Permissions" to "Coding Tool Permissions" and cover both tools
4. **Settings Sync section** -- Mention both sync scripts
5. **Customization > devcontainer.json** -- Mention `containerEnv` with `OPENAI_API_KEY` as an existing example

### Files to Modify

- `internal/render/templates/README.md.tmpl` -- Template text changes (no template action changes)
- `internal/render/readme_test.go` -- Add tests for Codex documentation, update section heading assertions

### Steps

#### Step 1: Write tests first (TDD)

Add and update the following tests in `internal/render/readme_test.go`:

1. **Update `TestREADME_ContainsAllSections`** -- Update the `sections` slice:
- Replace `"## Claude Code Permissions"` with `"## Coding Tool Permissions"`
- Add `"## Coding Tools"` (the new section)
- All other section headings remain unchanged

2. **Add `TestREADME_CodingToolsSection`** -- New test using `Merge([]stack.StackID{stack.Go}, nil)`:
- Assert `"## Coding Tools"` heading present
- Assert `"Claude Code"` mentioned
- Assert `"Codex CLI"` or `"Codex"` mentioned
- Assert `"OPENAI_API_KEY"` mentioned (API key auth option)
- Assert `"ChatGPT"` mentioned (interactive login option)
- Assert `"codex"` command mentioned (interactive TUI usage)
- Assert `"codex --full-auto"` mentioned (autonomous mode usage)
- Assert `"volume mount"` or `"persists across container rebuilds"` mentioned (persistence note)

3. **Add `TestREADME_CodingToolPermissionsSection`** -- New test:
- Assert `"## Coding Tool Permissions"` heading present
- Assert both `"Claude Code"` and `"Codex"` mentioned in the permissions context
- Assert `"bypass"` mentioned for Claude Code
- Assert `"full-access"` or `"danger-full-access"` or equivalent mentioned for Codex

4. **Add `TestREADME_SettingsSyncMentionsBothTools`** -- New test:
- Assert `"sync-claude-settings.sh"` mentioned
- Assert `"sync-codex-settings.sh"` mentioned

5. **Update `TestREADME_EmptyConfig`** -- Update the `staticSections` slice to match the renamed/added sections:
- Replace `"## Claude Code Permissions"` with `"## Coding Tool Permissions"`
- Add `"## Coding Tools"`

All tests should initially fail (red phase of TDD).

#### Step 2: Update `internal/render/templates/README.md.tmpl`

**2a. Update Overview section (lines 7-9)**

Change line 3 (the intro paragraph) from:
> This devcontainer was generated by [agentbox](...) to run Claude Code in a sandboxed environment with network isolation.

To:
> This devcontainer was generated by [agentbox](...) to run AI coding tools in a sandboxed environment with network isolation.

Change the Overview bullet list to mention both tools:
- Replace `Full Claude Code installation with bypass permissions (safe due to network isolation)` with `Claude Code and Codex CLI pre-installed with full permissions (safe due to network isolation)`

**2b. Add new "Coding Tools" section after "Getting Started" (after line 31)**

Insert a new section between "Getting Started" and "Firewall Architecture":

```markdown
## Coding Tools

This container includes two AI coding tools, ready to use out of the box:

### Claude Code

Claude Code is available globally via `claude`. It runs in bypass mode with full permissions — safe because the container's firewall restricts all network access.

### Codex CLI

Codex CLI is available globally via `codex`.

**Authentication** (choose one):

- **API key**: Set `OPENAI_API_KEY` on your host machine before opening the container. It is automatically forwarded via `containerEnv` in `devcontainer.json`.
- **ChatGPT login**: Run `codex` inside the container and select "Sign in with ChatGPT." Login tokens are stored in `~/.codex/` and persist across container rebuilds via a volume mount.

**Usage**:

- `codex` — Interactive TUI mode
- `codex --full-auto` — Autonomous mode (no approval prompts)
```

**2c. Rename "Claude Code Permissions" to "Coding Tool Permissions" (line 76)**

Change `## Claude Code Permissions` to `## Coding Tool Permissions`.

Update the section body to cover both tools:
- Keep the existing explanation about bypass mode and safety rationale.
- Mention that Claude Code runs in `bypassPermissions` mode and Codex runs with `sandbox_mode = "danger-full-access"` and `approval_policy = "never"`.
- Keep the three bullet points about firewall, filesystem isolation, and credentials.

Proposed content:
```markdown
## Coding Tool Permissions

Both Claude Code and Codex CLI run with full permissions inside this container:

- **Claude Code**: `bypassPermissions` mode — can read/write files, execute commands, and access allowed network endpoints
- **Codex CLI**: `sandbox_mode = "danger-full-access"` with `approval_policy = "never"` — equivalent full access

This is safe because:

- The container's firewall restricts all network access to explicitly allowed domains
- The container filesystem is isolated from your host (only the workspace is mounted)
- No sensitive credentials are available inside the container by default
```

**2d. Update "Settings Sync" section (line 82-84)**

Change from only mentioning Claude Code to mentioning both tools:

```markdown
## Settings Sync

Settings for both coding tools are synced into the container at startup:

- `sync-claude-settings.sh` deep-merges Claude Code settings from the template into `~/.claude/settings.json`, preserving any runtime state.
- `sync-codex-settings.sh` copies Codex CLI settings to `~/.codex/config.toml` on first run. If the config already exists, it is left untouched to preserve your customizations.
```

**2e. Update "Customization > devcontainer.json" section (line 109-114)**

Add `containerEnv` to the existing list of customizable devcontainer.json fields, mentioning `OPENAI_API_KEY` as an already-configured example:

Change the bullet list to include:
```markdown
- **Environment variables**: Add entries to `containerEnv` for variables forwarded from your host (e.g., `OPENAI_API_KEY` is already configured). Use `remoteEnv` for container-only runtime variables.
```

This replaces the current bullet that says `containerEnv` for build-time and `remoteEnv` for runtime (the current text incorrectly calls `containerEnv` "build-time" — it is actually host-to-container forwarding).

#### Step 3: Run tests and lint

Run `go test ./internal/render/...` to confirm all tests pass (green phase).
Run `golangci-lint run ./...` to confirm lint passes.

### Testing Strategy

**Updated existing tests:**
- `TestREADME_ContainsAllSections` -- Updated section list (renamed heading, new heading)
- `TestREADME_EmptyConfig` -- Updated static sections list to match

**New tests:**
- `TestREADME_CodingToolsSection` -- Validates the new Coding Tools section content: both tool names, auth options (OPENAI_API_KEY, ChatGPT login), usage commands (codex, codex --full-auto), persistence note
- `TestREADME_CodingToolPermissionsSection` -- Validates the renamed permissions section covers both tools
- `TestREADME_SettingsSyncMentionsBothTools` -- Validates both sync scripts are documented

**Existing tests that validate without modification:**
- `TestREADME_NoError` -- Still renders without error
- `TestREADME_DetectedStacksListed` -- Stack listing is unchanged
- `TestREADME_StaticDomainsInTable` -- Domain tables are unchanged
- `TestREADME_DynamicDomainsInTable` -- Domain tables are unchanged
- `TestREADME_AllStacks` -- All stacks still listed
- `TestREADME_NoTemplateArtifacts` -- No template artifacts introduced (all new content is static text)
- `TestREADME_Deterministic` -- No non-deterministic elements (no new template actions)
- `TestREADME_WildcardDomainsStripped` -- Unrelated to changes
- `TestREADME_NilDomainSlices` -- Unrelated to changes
- `TestREADME_NoDoNotEditFooter` -- README still has no do-not-edit footer
- `TestREADME_UpdatingSection` -- Updating section is unchanged

### Open Questions

None. All changes are static text additions to the README template. The content is well-defined by the bean scope and corroborated by the existing devcontainer.json template (which already has `OPENAI_API_KEY` in `containerEnv`, `agentbox-codex-config` volume mount, and `sync-codex-settings.sh` in `postStartCommand`), the Codex config template (`approval_policy = "never"`, `sandbox_mode = "danger-full-access"`), and the Dockerfile template (`npm install -g @anthropic-ai/claude-code @openai/codex`). No architectural decisions needed — this is a documentation-only change.

## Definition of Done

- [ ] Tests written (TDD: write tests before implementation)
- [ ] No new TODO/FIXME/HACK/XXX comments introduced
- [ ] `golangci-lint run ./...` passes with no errors
- [ ] `go test ./...` passes with no failures
- [ ] Branch pushed to remote
- [ ] PR created
- [ ] Automated code review passed via `@review-backend` subagent (via Task tool)
- [ ] Review feedback worked in via `/rework` and pushed to remote (if applicable)
- [ ] ADR written via `/decision` skill (if new dependencies, patterns, or architectural changes)
- [ ] All other checklist items above are completed
- [ ] User notified for human review
- [x] Tests written (TDD: write tests before implementation)
- [x] No new TODO/FIXME/HACK/XXX comments introduced
- [x] `golangci-lint run ./...` passes with no errors
- [x] `go test ./...` passes with no failures
- [x] Branch pushed to remote
- [x] PR created
- [x] Automated code review passed via `@review-backend` subagent (via Task tool)
- [x] Review feedback worked in via `/rework` and pushed to remote (if applicable)
- [x] ADR written via `/decision` skill (if new dependencies, patterns, or architectural changes) — N/A, no architectural changes
- [x] All other checklist items above are completed
- [x] User notified for human review

## Pipeline State

| Phase | Status | Iteration | Timestamp |
|-------|--------|-----------|-----------|
| refine | complete | 1 | 2026-04-08 |
| challenge | complete | 1 | 2026-04-08 |
| implement | completed | 1 | 2026-04-08 |
| pr | completed | 1 | 2026-04-08 |
| review | completed | 2 | 2026-04-08 |
| codify | completed | 1 | 2026-04-08 |


## Challenge Report

**Scope: SMALL CHANGE** (2 files)

### Scope Assessment

| Metric | Value | Threshold |
|--------|-------|-----------|
| Files | 2 | >15 = recommend split |

### Findings

#### Go Engineer

> **Finding 1** (severity: SUGGESTION)
> Step 2e proposes correcting the existing `containerEnv` description from "build-time variables" to "variables forwarded from your host." While factually correct, the plan does not call this out as a deliberate correction in the test suite. If someone later "fixes" it back, no test would catch it. Consider adding a spot-check assertion in `TestREADME_CodingToolsSection` or a dedicated small test that asserts the Customization section contains `"forwarded from your host"` or `"${localEnv:"` phrasing, so the correction is protected by a test.
>
> **Suggestion:** Add a one-line assertion in the existing `TestREADME_SettingsSyncMentionsBothTools` or a new test that the Customization section mentions `containerEnv` with forwarding semantics (e.g., assert `"forwarded"` or `"${localEnv:"` appears near `containerEnv`). Low effort, prevents regression of the factual correction.

### Verdict

**APPROVED**

This is a clean, well-scoped documentation-only change. The plan is thorough for what it is:

- All new content is static template text; no `GenerationConfig` changes, no new FuncMap helpers, no new Go code paths. The risk surface is minimal.
- The plan correctly references existing infrastructure (devcontainer.json `containerEnv`, `codex-config.toml` settings, Dockerfile `npm install` line, sync scripts) and the proposed README content is factually accurate against those artifacts.
- The TDD test strategy is appropriate: five test functions covering the new section, the renamed section, and the settings sync update. Existing tests that should continue passing are explicitly enumerated.
- The plan correctly identifies that `TestREADME_ContainsAllSections` and `TestREADME_EmptyConfig` need section heading updates (rename + addition), which is the kind of detail often missed.
- No over-engineering: the plan resists adding template conditionals or data-driven content where static prose suffices.
- No under-engineering: auth options, persistence behavior, and permission modes are all documented with specific values (`bypassPermissions`, `danger-full-access`, `approval_policy = "never"`).

The single suggestion is minor polish. No blockers.
Loading