test: Add codex file assertions to update integration and unit tests by BjRo · Pull Request #37 · BjRo/agentbox

BjRo · 2026-04-08T14:01:11Z

Summary

Add codex-config.toml and sync-codex-settings.sh content assertions to 3 update integration tests (PreservesCustomizations, WithStackChange, ForceMode)
Add executable permission checks on all scripts after update operations
Add codex file existence/non-empty assertions to TestUpdateCommand_ForceRegeneration unit test

Closes agentbox-comb

Test plan

go test ./... passes
go test -tags integration ./cmd/... passes
golangci-lint run ./... clean

🤖 Generated with Claude Code

Add codex-config.toml and sync-codex-settings.sh content assertions, executable permission checks, and non-empty file assertions to the three update integration tests and the ForceRegeneration unit test.

BjRo

Go Code Review

Summary

This PR adds codex file assertions (codex-config.toml, sync-codex-settings.sh) to the update integration and unit tests. The changes are clean, well-structured, and follow the established testing patterns in the codebase. The assertions are placed logically within existing test functions, the readFile/assertFileExists helpers are reused correctly, and the executableScripts package-level slice is leveraged for permission checks rather than hardcoding file names.

A few observations on assertion consistency across the test functions -- nothing blocking.

Findings

[SUGGESTION] Inconsistent assertion depth for codex files across update integration tests. TestIntegration_UpdateWithStackChange checks three TOML keys (approval_policy, sandbox_mode, apps = false) and two sync-script references (codex-config.toml, $HOME/.codex), while TestIntegration_UpdatePreservesCustomizations only checks one of each, and TestIntegration_UpdateForceMode only checks non-emptiness. If the intent is to verify "codex files are correctly regenerated," the depth of assertion should match across all three tests. The force-mode test in particular would benefit from at least one content assertion (e.g., approval_policy) beyond the len > 0 check, since an empty-file guard is already provided by readFile calling t.Fatalf on read error.

[SUGGESTION] Unit test TestUpdateCommand_ForceRegeneration uses os.ReadFile directly while integration tests use readFile helper. This is fine since unit tests do not have access to the integration-test-only helper (build tag boundary), but it is worth noting that the readFile helper is defined in init_integration_test.go behind //go:build integration. If more unit tests need this pattern, extracting a shared readFile into a non-tagged test helper file would reduce duplication.

Verdict

LGTM -- the assertions are correct, follow project conventions, and fill a real coverage gap.

Automated review by Go Review Agent

BjRo · 2026-04-08T14:02:18Z

+	if !strings.Contains(codexConfig, "sandbox_mode") {
+		t.Error("codex-config.toml should contain sandbox_mode after stack change")
+	}
+	if !strings.Contains(codexConfig, "apps = false") {


[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.

BjRo · 2026-04-08T14:02:18Z

+		if info.Mode().Perm()&0o111 == 0 {
+			t.Errorf("%s should be executable after update", name)
+		}
+	}


[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.

BjRo · 2026-04-08T14:12:20Z

Codex Code Review

The Codex CLI review tool (codex exec review) failed to connect to the OpenAI API after multiple retry attempts. The tool reported connection timeouts and exhausted all 5 reconnection attempts on two separate runs.

Error observed:

ERROR: Reconnecting... 2/5
ERROR: Reconnecting... 3/5
ERROR: Reconnecting... 4/5
ERROR: Reconnecting... 5/5

No automated review output was produced. A manual re-run of the Codex review may be needed once the OpenAI API connectivity is restored.

Automated review by Codex Review Agent

Replace weak len==0 guards in force-mode test with actual content assertions (approval_policy, sandbox_mode, apps = false). Add missing sandbox_mode, apps = false, and $HOME/.codex checks to the preserves-customizations test so all three update tests verify the same baseline content.

BjRo

Go Code Review (Re-review)

Summary

Re-review after rework. The two SUGGESTION-level findings from the initial review have been addressed correctly:

Inconsistent assertion depth -- FIXED. All three integration tests (TestIntegration_UpdatePreservesCustomizations, TestIntegration_UpdateWithStackChange, TestIntegration_UpdateForceMode) now assert the same five codex content properties: approval_policy, sandbox_mode, apps = false in codex-config.toml, and codex-config.toml / $HOME/.codex references in sync-codex-settings.sh. The force-mode test previously only checked non-emptiness; it now has full content assertions matching the other two tests.
Unit test os.ReadFile vs readFile helper -- Acknowledged as acceptable given the build tag boundary. No change needed.

No new issues found. The assertions are consistent, the executable permission checks via executableScripts are correctly placed in both the stack-change and force-mode tests, and all tests pass (unit and integration).

Verdict

LGTM

Automated review by Go Review Agent

BjRo · 2026-04-08T14:24:47Z

Codex Code Review

The Codex CLI review tool failed to produce output. The tool encountered repeated connection errors when attempting to reach the OpenAI API (ERROR: Reconnecting... 5/5). This is an infrastructure issue, not a problem with the PR itself.

Tool details:

Codex CLI v0.118.0
Model: gpt-5.4
Error: timeout waiting for child process to exit / reconnection failures

A manual re-review can be triggered once the API connectivity issue is resolved.

Automated review by Codex Review Agent

BjRo · 2026-04-08T15:45:21Z

Codex Code Review

The changes only add test coverage, and the new assertions align with the current update/init behavior for Codex-generated files and script permissions. I did not identify any introduced bug or incorrect expectation in the modified tests.

Automated review by Codex Review Agent

…37) * test: Add codex file assertions to update integration and unit tests Add codex-config.toml and sync-codex-settings.sh content assertions, executable permission checks, and non-empty file assertions to the three update integration tests and the ForceRegeneration unit test. * chore: Update bean agentbox-comb checklist * chore: Mark branch pushed in bean agentbox-comb checklist * fix: Align codex assertion depth across update integration tests Replace weak len==0 guards in force-mode test with actual content assertions (approval_policy, sandbox_mode, apps = false). Add missing sandbox_mode, apps = false, and $HOME/.codex checks to the preserves-customizations test so all three update tests verify the same baseline content. * chore: Update bean agentbox-comb pipeline state (all phases done)

BjRo added 3 commits April 8, 2026 14:00

test: Add codex file assertions to update integration and unit tests

bfbbc1d

Add codex-config.toml and sync-codex-settings.sh content assertions, executable permission checks, and non-empty file assertions to the three update integration tests and the ForceRegeneration unit test.

chore: Update bean agentbox-comb checklist

c4c564d

chore: Mark branch pushed in bean agentbox-comb checklist

aeec9ff

BjRo commented Apr 8, 2026

View reviewed changes

chore: Update bean agentbox-comb pipeline state (all phases done)

7a5e2cc

BjRo merged commit d8351fa into main Apr 8, 2026
3 checks passed

BjRo deleted the deliver-agentbox-comb branch April 9, 2026 08:56

Navigation Menu

test: Add codex file assertions to update integration and unit tests - #37

test: Add codex file assertions to update integration and unit tests#37
BjRo merged 5 commits into
mainfrom
deliver-agentbox-comb

BjRo commented Apr 8, 2026

Uh oh!

BjRo left a comment

Uh oh!

BjRo Apr 8, 2026

Uh oh!

BjRo Apr 8, 2026

Uh oh!

BjRo commented Apr 8, 2026

Uh oh!

BjRo left a comment

Uh oh!

BjRo commented Apr 8, 2026

Uh oh!

BjRo commented Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

BjRo commented Apr 8, 2026

Summary

Test plan

Uh oh!

BjRo left a comment

Choose a reason for hiding this comment

Go Code Review

Summary

Findings

Verdict

Uh oh!

BjRo Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

BjRo Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

BjRo commented Apr 8, 2026

Codex Code Review

Uh oh!

BjRo left a comment

Choose a reason for hiding this comment

Go Code Review (Re-review)

Summary

Verdict

Uh oh!

BjRo commented Apr 8, 2026

Codex Code Review

Uh oh!

BjRo commented Apr 8, 2026

Codex Code Review

The changes only add test coverage, and the new assertions align with the current update/init behavior for Codex-generated files and script permissions. I did not identify any introduced bug or incorrect expectation in the modified tests.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant