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
111 changes: 98 additions & 13 deletions .beans/agentbox-comb--update-integration-tests-for-codex-support.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
---
# agentbox-comb
title: Update integration tests for Codex support
status: todo
status: in-progress
type: task
priority: normal
created_at: 2026-04-08T09:16:54Z
updated_at: 2026-04-08T09:17:47Z
updated_at: 2026-04-08T14:25:56Z
parent: agentbox-cqi5
blocked_by:
- agentbox-qr1p
Expand Down Expand Up @@ -36,14 +36,99 @@ This bean covers only integration test changes. Unit tests for individual render

## 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)
- [x] All other checklist items above are completed
- [x] User notified for human review

## Implementation Plan

### Approach

Most of the init integration test work described in the original scope has already been completed as part of the blocking beans (primarily commit `529be90` which wired Codex into init/update commands and updated test expectations). The remaining work is in the **update integration tests**, which lack codex-specific content assertions, permission checks, and regeneration verification. There is also a small gap in the update unit tests.

The plan adds targeted assertions to existing update test functions and one new integration test that verifies codex files are freshly regenerated (not stale) after an update with a stack change.

### Current State Assessment

**Init integration tests (COMPLETE -- no changes needed)**:
- `expectedFiles` already includes `codex-config.toml` and `sync-codex-settings.sh` (lines 48-49)
- `executableScripts` already includes `sync-codex-settings.sh` (line 60)
- `TestIntegration_SingleGoStack` already asserts codex-config.toml contains `approval_policy`, `sandbox_mode`, and `apps = false` (lines 188-196)
- `TestIntegration_SingleGoStack` already asserts sync-codex-settings.sh references `codex-config.toml` and `$HOME/.codex` (lines 199-205)

**Update integration tests (GAPS)**:
1. `TestIntegration_UpdatePreservesCustomizations` -- no codex content assertions after update
2. `TestIntegration_UpdateWithStackChange` -- no assertion that codex files exist or have correct content after stack change
3. `TestIntegration_UpdateForceMode` -- no assertion that codex files are regenerated during force mode
4. `TestIntegration_UpdateIdempotent` -- implicitly covers codex via `expectedFiles` loop (adequate)
5. No test checks executable permissions on scripts after `update` (not just codex -- this is a broader gap)

**Update unit tests (GAPS)**:
1. `TestUpdateCommand_ForceRegeneration` -- no assertion that codex files are produced
2. `TestUpdateCommand_RegeneratesAgentboxStage` -- only checks Dockerfile, does not verify other regenerated files

### Files to Modify

- `cmd/update_integration_test.go` -- Add codex content and permission assertions to existing tests; add one focused test for codex file regeneration after update
- `cmd/update_test.go` -- Add codex assertions to `TestUpdateCommand_ForceRegeneration`

### Steps

1. **Add codex content assertions to `TestIntegration_UpdatePreservesCustomizations`** -- After the existing Dockerfile and mise-config preservation assertions (line 70), add assertions that:
- `codex-config.toml` exists and contains `approval_policy` (verifies regeneration)
- `sync-codex-settings.sh` exists and contains `codex-config.toml` (verifies regeneration)
- Both files are non-empty

2. **Add codex and permission assertions to `TestIntegration_UpdateWithStackChange`** -- After the existing Dockerfile and config assertions (line 112), add assertions that:
- `codex-config.toml` exists, is non-empty, and contains expected TOML keys (`approval_policy`, `sandbox_mode`, `apps = false`)
- `sync-codex-settings.sh` exists, is non-empty, and references `codex-config.toml` and `$HOME/.codex`
- All `executableScripts` have executable permissions after the update (covers `sync-codex-settings.sh` plus the others)

3. **Add permission assertions to `TestIntegration_UpdateForceMode`** -- After the existing custom stage assertions (line 162), add assertions that:
- All `executableScripts` have executable permissions after `--force` update
- `codex-config.toml` exists and is non-empty (force mode regenerates everything)

4. **Add codex assertion to unit test `TestUpdateCommand_ForceRegeneration`** in `cmd/update_test.go` -- After the existing `FROM agentbox AS custom` assertion (line 127), add assertions that:
- `codex-config.toml` exists and is non-empty in the output directory
- `sync-codex-settings.sh` exists and is non-empty in the output directory

5. **Verify all tests pass** -- Run `go test ./...` and `go test -tags integration ./...` and `golangci-lint run ./...`

### Testing Strategy

- All changes are test code; no production code changes
- Run `go test -tags integration ./cmd/...` to verify all integration tests pass (including the new assertions)
- Run `go test ./cmd/...` to verify unit tests pass
- Run `golangci-lint run ./...` to verify no lint issues
- Verify the new assertions fail when codex files are absent (manually confirm by temporarily commenting out codex rendering in `renderFiles` -- but do not commit this)

### Scope Boundaries

- No changes to `cmd/init_integration_test.go` -- all init-side codex assertions are already present
- No changes to production code (`cmd/init.go`, `cmd/update.go`, `internal/render/codex.go`)
- No new test functions needed in the integration test file -- all additions fit naturally into existing test functions
- The `seedInitDir` helper does not need changes -- it already produces codex files via `init --stack`
- No ADR needed -- this is purely additive test coverage with no architectural changes

### Open Questions

- The blocker `agentbox-thmk` (Update generated README to document Codex CLI) is still in `todo` status. This bean should not be blocked by it since README content changes do not affect the codex file integration test assertions described here. However, if README template changes introduce new content that should be spot-checked in integration tests, that would be a separate concern.

## Pipeline State

| Phase | Status | Iteration | Timestamp |
|-------|--------|-----------|-----------|
| refine | complete | 1 | 2026-04-08T14:00:00Z |
| challenge | completed | 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 |
73 changes: 73 additions & 0 deletions cmd/update_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,25 @@ func TestIntegration_UpdatePreservesCustomizations(t *testing.T) {
if !strings.Contains(updated, "WORKDIR /workspace\n\nFROM agentbox AS custom") {
t.Error("Dockerfile should have exactly one blank line between agentbox stage and custom stage")
}

// Verify codex files are regenerated after update.
codexConfig := readFile(t, filepath.Join(devDir, "codex-config.toml"))
if !strings.Contains(codexConfig, "approval_policy") {
t.Error("codex-config.toml should contain approval_policy after update")
}
if !strings.Contains(codexConfig, "sandbox_mode") {
t.Error("codex-config.toml should contain sandbox_mode after update")
}
if !strings.Contains(codexConfig, "apps = false") {
t.Error("codex-config.toml should contain apps = false after update")
}
syncCodex := readFile(t, filepath.Join(devDir, "sync-codex-settings.sh"))
if !strings.Contains(syncCodex, "codex-config.toml") {
t.Error("sync-codex-settings.sh should reference codex-config.toml after update")
}
if !strings.Contains(syncCodex, "$HOME/.codex") {
t.Error("sync-codex-settings.sh should reference $HOME/.codex after update")
}
}

func TestIntegration_UpdateWithStackChange(t *testing.T) {
Expand Down Expand Up @@ -109,6 +128,33 @@ func TestIntegration_UpdateWithStackChange(t *testing.T) {
if len(cfg.Stacks) != 2 {
t.Errorf("expected 2 stacks, got %d: %v", len(cfg.Stacks), cfg.Stacks)
}

// Verify codex files exist with expected content after stack change.
codexConfig := readFile(t, filepath.Join(devDir, "codex-config.toml"))
if !strings.Contains(codexConfig, "approval_policy") {
t.Error("codex-config.toml should contain approval_policy after stack change")
}
if !strings.Contains(codexConfig, "sandbox_mode") {
t.Error("codex-config.toml should contain sandbox_mode after stack change")
}
if !strings.Contains(codexConfig, "apps = false") {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[SUGGESTION] Inconsistent assertion depth across update integration tests.

TestIntegration_UpdateWithStackChange (lines 123-140) checks three TOML keys and two sync-script references, while this test checks only one of each, and TestIntegration_UpdateForceMode checks only len > 0. Consider aligning the assertion depth so each test that claims "codex files are regenerated" verifies at least the same baseline content (e.g., approval_policy in all three). The force-mode test is the weakest -- len(codexConfig) == 0 would only catch a completely empty file, not a partially rendered one.

t.Error("codex-config.toml should contain apps = false after stack change")
}
syncCodex := readFile(t, filepath.Join(devDir, "sync-codex-settings.sh"))
if !strings.Contains(syncCodex, "codex-config.toml") {
t.Error("sync-codex-settings.sh should reference codex-config.toml after stack change")
}
if !strings.Contains(syncCodex, "$HOME/.codex") {
t.Error("sync-codex-settings.sh should reference $HOME/.codex after stack change")
}

// Verify executable permissions on all scripts after update.
for _, name := range executableScripts {
info := assertFileExists(t, filepath.Join(devDir, name))
if info.Mode().Perm()&0o111 == 0 {
t.Errorf("%s should be executable after update", name)
}
}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[SUGGESTION] Force-mode codex assertions are weaker than the other tests.

The len(codexConfig) == 0 and len(syncCodex) == 0 checks here only guard against empty files. Since readFile already calls t.Fatalf on read error (covering the "file missing" case), these len checks add minimal value. A single content assertion like strings.Contains(codexConfig, "approval_policy") would make this consistent with the other two tests and actually verify the content was rendered, not just that bytes were written.

}

func TestIntegration_UpdateForceMode(t *testing.T) {
Expand Down Expand Up @@ -160,6 +206,33 @@ func TestIntegration_UpdateForceMode(t *testing.T) {
if !strings.Contains(updated, "USER CUSTOMIZATIONS") {
t.Error("Dockerfile should contain custom stage comments after --force")
}

// Verify executable permissions on all scripts after --force update.
for _, name := range executableScripts {
info := assertFileExists(t, filepath.Join(devDir, name))
if info.Mode().Perm()&0o111 == 0 {
t.Errorf("%s should be executable after --force update", name)
}
}

// Verify codex files are regenerated during force mode.
codexConfig := readFile(t, filepath.Join(devDir, "codex-config.toml"))
if !strings.Contains(codexConfig, "approval_policy") {
t.Error("codex-config.toml should contain approval_policy after --force update")
}
if !strings.Contains(codexConfig, "sandbox_mode") {
t.Error("codex-config.toml should contain sandbox_mode after --force update")
}
if !strings.Contains(codexConfig, "apps = false") {
t.Error("codex-config.toml should contain apps = false after --force update")
}
syncCodex := readFile(t, filepath.Join(devDir, "sync-codex-settings.sh"))
if !strings.Contains(syncCodex, "codex-config.toml") {
t.Error("sync-codex-settings.sh should reference codex-config.toml after --force update")
}
if !strings.Contains(syncCodex, "$HOME/.codex") {
t.Error("sync-codex-settings.sh should reference $HOME/.codex after --force update")
}
}

func TestIntegration_UpdateIdempotent(t *testing.T) {
Expand Down
16 changes: 16 additions & 0 deletions cmd/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,22 @@ func TestUpdateCommand_ForceRegeneration(t *testing.T) {
if !strings.Contains(string(dockerfile), "FROM agentbox AS custom") {
t.Error("Dockerfile should contain FROM agentbox AS custom after --force")
}

// Verify codex files are produced during force regeneration.
codexConfig, err := os.ReadFile(filepath.Join(outDir, "codex-config.toml"))
if err != nil {
t.Fatalf("read codex-config.toml: %v", err)
}
if len(codexConfig) == 0 {
t.Error("codex-config.toml should be non-empty after --force")
}
syncCodex, err := os.ReadFile(filepath.Join(outDir, "sync-codex-settings.sh"))
if err != nil {
t.Fatalf("read sync-codex-settings.sh: %v", err)
}
if len(syncCodex) == 0 {
t.Error("sync-codex-settings.sh should be non-empty after --force")
}
}

func TestUpdateCommand_PreservesCustomStage(t *testing.T) {
Expand Down