feat: add auto_cleanup config and improve update UX#741
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🧰 Additional context used📓 Path-based instructions (1){**/*.py,cli/**/*.go}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2026-03-15T18:17:43.675ZApplied to files:
📚 Learning: 2026-03-15T18:17:43.675ZApplied to files:
📚 Learning: 2026-03-15T21:32:02.880ZApplied to files:
📚 Learning: 2026-03-22T16:44:15.487ZApplied to files:
📚 Learning: 2026-03-16T19:52:03.656ZApplied to files:
📚 Learning: 2026-03-15T21:32:02.880ZApplied to files:
📚 Learning: 2026-03-19T11:19:40.044ZApplied to files:
📚 Learning: 2026-03-15T21:32:02.880ZApplied to files:
🧬 Code graph analysis (3)cli/cmd/cleanup.go (2)
cli/cmd/update_cleanup.go (3)
cli/cmd/update.go (2)
🔇 Additional comments (11)
WalkthroughAdds a persisted boolean Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the "synthorg" CLI's update and cleanup mechanisms. It introduces an "auto_cleanup" configuration option, allowing users to automatically remove old Docker images after updates, thereby managing disk space more efficiently. Additionally, the update process is streamlined by intelligently distinguishing between structural and boilerplate changes in Docker Compose files, preventing unnecessary user prompts for routine image or version comment updates. The CLI self-update flow is also refined to eliminate redundant update checks, contributing to a smoother user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new auto_cleanup configuration option to automatically remove old Docker images after an update, which is a great UX improvement. It also enhances the update process by auto-applying non-structural changes to the compose file and skipping redundant update checks. The changes are well-implemented and thoroughly tested. I have a couple of suggestions to improve maintainability in the tests and memory efficiency in the compose file comparison logic.
| func TestConfigSetAutoCleanup(t *testing.T) { | ||
| dir := t.TempDir() | ||
| state := config.DefaultState() | ||
| state.DataDir = dir | ||
| if err := config.Save(state); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| var buf bytes.Buffer | ||
| rootCmd.SetOut(&buf) | ||
| rootCmd.SetErr(&buf) | ||
| rootCmd.SetArgs([]string{"config", "set", "auto_cleanup", "true", "--data-dir", dir}) | ||
| if err := rootCmd.Execute(); err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| loaded, err := config.Load(dir) | ||
| if err != nil { | ||
| t.Fatalf("Load after set: %v", err) | ||
| } | ||
| if !loaded.AutoCleanup { | ||
| t.Error("AutoCleanup should be true after setting") | ||
| } | ||
| } | ||
|
|
||
| func TestConfigSetAutoCleanupFalse(t *testing.T) { | ||
| dir := t.TempDir() | ||
| state := config.DefaultState() | ||
| state.DataDir = dir | ||
| state.AutoCleanup = true | ||
| if err := config.Save(state); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| var buf bytes.Buffer | ||
| rootCmd.SetOut(&buf) | ||
| rootCmd.SetErr(&buf) | ||
| rootCmd.SetArgs([]string{"config", "set", "auto_cleanup", "false", "--data-dir", dir}) | ||
| if err := rootCmd.Execute(); err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| loaded, err := config.Load(dir) | ||
| if err != nil { | ||
| t.Fatalf("Load after set: %v", err) | ||
| } | ||
| if loaded.AutoCleanup { | ||
| t.Error("AutoCleanup should be false after setting") | ||
| } | ||
| } |
There was a problem hiding this comment.
These two test functions, TestConfigSetAutoCleanup and TestConfigSetAutoCleanupFalse, are very similar. To improve maintainability and reduce code duplication, consider combining them into a single table-driven test. This would make it easier to read and to add more test cases in the future.
func TestConfigSetAutoCleanup(t *testing.T) {
testCases := []struct {
name string
initialValue bool
setValue string
expectedValue bool
}{
{
name: "set to true",
initialValue: false,
setValue: "true",
expectedValue: true,
},
{
name: "set to false",
initialValue: true,
setValue: "false",
expectedValue: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
dir := t.TempDir()
state := config.DefaultState()
state.DataDir = dir
state.AutoCleanup = tc.initialValue
if err := config.Save(state); err != nil {
t.Fatal(err)
}
var buf bytes.Buffer
rootCmd.SetOut(&buf)
rootCmd.SetErr(&buf)
rootCmd.SetArgs([]string{"config", "set", "auto_cleanup", tc.setValue, "--data-dir", dir})
if err := rootCmd.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}
loaded, err := config.Load(dir)
if err != nil {
t.Fatalf("Load after set: %v", err)
}
if loaded.AutoCleanup != tc.expectedValue {
t.Errorf("AutoCleanup should be %v after setting, but got %v", tc.expectedValue, loaded.AutoCleanup)
}
})
}
}| func isUpdateBoilerplateOnly(existing, fresh []byte) bool { | ||
| oldLines := strings.Split(string(existing), "\n") | ||
| newLines := strings.Split(string(fresh), "\n") | ||
| if len(oldLines) != len(newLines) { | ||
| return false | ||
| } | ||
| for i := range oldLines { | ||
| if oldLines[i] == newLines[i] { | ||
| continue | ||
| } | ||
| // Allow version comment difference (line 1). | ||
| if i == 0 && strings.HasPrefix(oldLines[i], "# Generated by SynthOrg CLI") && | ||
| strings.HasPrefix(newLines[i], "# Generated by SynthOrg CLI") { | ||
| continue | ||
| } | ||
| // Allow image reference differences. | ||
| if imageLinePattern.MatchString(oldLines[i]) && | ||
| imageLinePattern.MatchString(newLines[i]) { | ||
| continue | ||
| } | ||
| return false | ||
| } | ||
| return true | ||
| } |
There was a problem hiding this comment.
The current implementation of isUpdateBoilerplateOnly converts the entire byte slices for existing and fresh into strings and then splits them into lines. For large compose files, this can be inefficient in terms of memory allocation.
Consider using a bufio.Scanner to iterate over the lines. This would avoid loading the entire file content into memory at once and creating a large slice of strings, making the function more memory-efficient. You will need to import the bufio package for this change.
func isUpdateBoilerplateOnly(existing, fresh []byte) bool {
s1 := bufio.NewScanner(bytes.NewReader(existing))
s2 := bufio.NewScanner(bytes.NewReader(fresh))
i := 0
for s1.Scan() {
if !s2.Scan() {
return false // new has fewer lines
}
line1 := s1.Bytes()
line2 := s2.Bytes()
if bytes.Equal(line1, line2) {
i++
continue
}
// Allow version comment difference (line 1).
if i == 0 && bytes.HasPrefix(line1, []byte("# Generated by SynthOrg CLI")) &&
bytes.HasPrefix(line2, []byte("# Generated by SynthOrg CLI")) {
i++
continue
}
// Allow image reference differences.
if imageLinePattern.Match(line1) &&
imageLinePattern.Match(line2) {
i++
continue
}
return false
}
if s2.Scan() {
return false // new has more lines
}
return s1.Err() == nil && s2.Err() == nil
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/config_test.go`:
- Around line 162-257: Add a fuzz test that exercises the "config set
auto_cleanup <value>" CLI validation using testing.F (e.g., create a
TestFuzzConfigSetAutoCleanup(t *testing.F) that seeds valid/invalid values and
calls t.Fuzz), reusing the same setup as TestConfigSetRejectsInvalidAutoCleanup
(create temp dir, save config state, set rootCmd output/err and args) and for
each fuzzed input run rootCmd.Execute(); assert that only "true" and "false"
(case-sensitive) are accepted (no error for those exact strings) and all other
inputs return an error; include a few deterministic seeds like
"true","false","yes","1","True" via f.Add before fuzzing.
In `@cli/cmd/update_cleanup_test.go`:
- Around line 198-259: Add a fuzz test named FuzzMergeKeepIDs that uses
testing.F to generate arbitrary map[string]bool inputs and asserts the union
invariants for mergeKeepIDs: call mergeKeepIDs(current, previous) and verify (1)
every key present in current and in previous exists in the result, (2)
len(result) equals the computed union size of keys from both inputs
(deduplicated), and (3) the result contains no keys outside the union; include a
few seeded examples via f.Add to help fuzzers start. Ensure the fuzz function
references mergeKeepIDs and handles nil maps correctly when computing the
expected union.
In `@cli/cmd/update.go`:
- Around line 489-499: The code currently calls collectCurrentImageIDs before
prompting the user, which wastes work if the user declines; move the capture so
it runs after user confirmation and before the pull/auto-cleanup logic by
invoking collectCurrentImageIDs only when state.AutoCleanup is true and the user
has confirmed the update (preserve the same best-effort error handling that
writes the warning to cmd.ErrOrStderr()), and ensure previousIDs remains nil or
properly scoped until that point so downstream cleanup logic still behaves
correctly when capture is skipped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66b69413-2245-4072-938d-1d3e24abf37a
📒 Files selected for processing (10)
cli/cmd/cleanup.gocli/cmd/config.gocli/cmd/config_test.gocli/cmd/update.gocli/cmd/update_cleanup.gocli/cmd/update_cleanup_test.gocli/cmd/update_test.gocli/internal/config/state.gocli/internal/config/state_test.godocs/user_guide.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
{**/*.py,cli/**/*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Line length is 88 characters (enforced by ruff).
Files:
cli/cmd/cleanup.gocli/cmd/update_cleanup_test.gocli/internal/config/state.gocli/internal/config/state_test.gocli/cmd/config.gocli/cmd/update_cleanup.gocli/cmd/config_test.gocli/cmd/update_test.gocli/cmd/update.go
cli/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing in Go with native
testing.Ffuzz functions.
Files:
cli/cmd/update_cleanup_test.gocli/internal/config/state_test.gocli/cmd/config_test.gocli/cmd/update_test.go
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Dependabot: daily updates (uv, github-actions, npm, pre-commit, docker, gomod), grouped minor/patch, no auto-merge. Use `/review-dep-pr` before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Version bumping (pre-1.0): `fix:`/`feat:` = patch, `feat!:`/`BREAKING CHANGE` = minor. Uses Conventional Commits for Release Please automation.
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/**/*_test.go : Use property-based testing in Go with native `testing.F` fuzz functions.
Applied to files:
cli/cmd/update_cleanup_test.gocli/internal/config/state_test.gocli/cmd/config_test.gocli/cmd/update_test.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Use native `testing.F` fuzz functions (`Fuzz*`) for fuzz testing Go code
Applied to files:
cli/cmd/update_cleanup_test.gocli/cmd/update_test.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/cmd/update_cleanup_test.gocli/cmd/config_test.gocli/cmd/update_test.go
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Applied to files:
cli/cmd/update_cleanup.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Run Go lint via `golangci-lint run`, vet via `go vet`, tests via `go test ./...`, and fuzz via `go test -fuzz=FuzzTarget -fuzztime=30s`
Applied to files:
cli/cmd/config_test.gocli/cmd/update_test.go
📚 Learning: 2026-03-16T19:52:03.656Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T19:52:03.656Z
Learning: Applies to cli/**/*.go : Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/config_test.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/go.mod : CLI dependencies: Go 1.26+, in `cli/go.mod` (Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (5)
cli/internal/config/state_test.go (1)
cli/internal/config/state.go (1)
IsValidBool(137-139)
cli/cmd/config.go (1)
cli/internal/config/state.go (2)
IsValidBool(137-139)BoolNames(142-142)
cli/cmd/update_cleanup.go (3)
cli/internal/docker/client.go (2)
Info(22-29)RunCmd(116-125)cli/internal/config/state.go (1)
State(16-31)cli/internal/ui/ui.go (1)
NewUI(62-76)
cli/cmd/config_test.go (3)
cli/internal/config/state.go (3)
DefaultState(36-48)Save(230-244)Load(65-103)cli/internal/config/paths.go (1)
DataDir(18-31)cli/cmd/root.go (1)
Execute(101-107)
cli/cmd/update.go (2)
cli/internal/docker/client.go (1)
Info(22-29)cli/internal/config/state.go (1)
State(16-31)
🔇 Additional comments (17)
cli/internal/config/state.go (1)
29-30: Good addition of persistedauto_cleanupstate and strict bool helpers.This integrates cleanly with config serialization and avoids ambiguous boolean inputs.
Also applies to: 136-143
cli/internal/config/state_test.go (2)
40-42: Auto-cleanup default and round-trip persistence checks look solid.Also applies to: 331-354
357-387: Add a fuzz target forIsValidBoolto satisfy test policy.The new table test is useful, but this file should also include property-based fuzzing for the validator behavior.
[ suggest_essential_refactor ]
Proposed addition
+func FuzzIsValidBool(f *testing.F) { + f.Add("true") + f.Add("false") + f.Add("") + f.Add("TRUE") + f.Add("1") + f.Add("yes") + + f.Fuzz(func(t *testing.T, s string) { + got := IsValidBool(s) + want := s == "true" || s == "false" + if got != want { + t.Fatalf("IsValidBool(%q) = %v, want %v", s, got, want) + } + }) +}As per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.cli/cmd/config.go (1)
16-17:auto_cleanupwiring in set/show paths is correctly implemented.Validation, persistence, and display behavior are aligned.
Also applies to: 42-44, 92-93, 110-115
cli/cmd/update_cleanup.go (2)
86-97:mergeKeepIDsunion behavior is clean and deterministic.
40-47: No action required—auto-cleanup ordering is correct.The flow properly captures previous image IDs before the update, updates
state.ImageTagto the new target, and only then invokesautoCleanupOldImageswith the updated state. Inside the cleanup function,collectCurrentImageIDsuses the post-updateImageTagto build the keep set, so there is no risk of staleness or inadvertent cleanup of newly pulled images.cli/cmd/cleanup.go (1)
61-65: Nice contextual hint after manual cleanup.The conditional tip is well-placed and improves discoverability of automatic cleanup.
docs/user_guide.md (1)
27-30: Docs update is clear and matches the new UX flow.cli/cmd/update_test.go (3)
450-518: Test cases well-structured and comprehensive.The expanded test matrix correctly covers:
- Version comment-only differences
- Image digest/tag changes (single and multiple)
- Substantive structural changes (ports, new services)
- Edge cases (empty input, different line counts)
One note on lines 469-485 and 487-491: the inline string literals exceed the 88-character line length guideline. Consider using raw string literals with explicit newlines or breaking them across multiple lines for readability.
The change at line 510 (
want: truefor empty input) aligns with the implementation inupdate.gowhereisUpdateBoilerplateOnlyreturnstruefor equal-length (both empty) inputs with no differing lines. The comment at lines 505-506 correctly documents that the caller (refreshCompose) handles this viabytes.Equalfirst.
520-529: LGTM!Test function correctly renamed to match the new predicate name and exercises all cases from the updated test matrix.
531-544: Fuzz function correctly updated with additional seed corpus.The new seed at lines 537-540 provides coverage for indented image reference lines, which is important for exercising the
imageLinePatternregex matching inisUpdateBoilerplateOnly. The fuzz test uses nativetesting.Fas required by the coding guidelines.cli/cmd/update.go (6)
28-42: LGTM! Hidden flag implementation is correct.The
skipCLIUpdateflag is properly:
- Declared as a package-level variable (line 30)
- Bound to the hidden
--skip-cli-updateflag (line 39)- Marked hidden to prevent user invocation (line 40)
This ensures the flag is only used internally during re-exec.
111-114: Early return correctly suppresses redundant messaging.When
skipCLIUpdateis true (set by the re-exec'd child), the function returnsnilimmediately, skipping the "Checking for updates..." and "CLI is up to date" messages that would be redundant after a self-update.
225-234: LGTM! Re-exec correctly propagates the skip flag.The
--skip-cli-updateflag is now included in the reconstructed arguments (line 227), ensuring the child process skips the CLI update check.
296-305: Auto-apply logic correctly updated.The predicate change from version-comment-only to update-boilerplate-only allows automatic application of compose changes when only the version comment and/or image references differ. This improves UX by avoiding unnecessary prompts during routine updates.
310-336: Implementation is correct but has a subtle edge case.The function correctly identifies "boilerplate-only" changes by:
- Requiring equal line counts (line 316-318)
- Allowing version comment differences on line 0 (lines 324-327)
- Allowing image reference differences via
imageLinePattern(lines 329-332)However, note that the
imageLinePatternregex (defined at line 668) only matchesghcr.io/aureliolo/synthorg-*images. If a user has added custom images to their compose file, those image line changes would correctly cause the function to returnfalse, requiring user confirmation. This is the intended behavior.
515-535: Post-pull actions logic is sound.The control flow correctly handles:
- Always attempting restart if containers were running
- Running
autoCleanupOldImageswhenoldState.AutoCleanupis enabled (regardless of restart status, sincedocker rmisafely skips in-use images)- Showing the passive
hintOldImagesonly when auto-cleanup is disabled AND a restart occurred (old containers are stopped)The use of
oldState.AutoCleanup(line 529) rather thanupdatedState.AutoCleanupensures the cleanup decision is based on the setting at the time the update was initiated, which is the correct behavior.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
cli/cmd/config_test.go (1)
163-221: 🧹 Nitpick | 🔵 TrivialAdd a fuzz target for the CLI
auto_cleanuppath.These table tests cover a few literals, but they still do not fuzz the
rootCmd.Execute()validation path added here.FuzzIsValidBoolincli/internal/config/state_test.goonly exercises the helper, not the Cobra argument wiring.As per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/config_test.go` around lines 163 - 221, Add a fuzz test that targets the Cobra argument parsing and validation path for the auto_cleanup flag (so it exercises rootCmd.Execute(), not just FuzzIsValidBool). Create a new fuzz function (e.g., FuzzConfigSetAutoCleanup) that calls rootCmd.SetArgs([]string{"config","set","auto_cleanup", value, "--data-dir", dir}) for varied inputs, runs rootCmd.Execute(), and asserts behavior matches the helper (accepts only "true"/"false" in the expected casing) — use t.TempDir() for data dir, reuse config.DefaultState()/config.Save/load to initialize state, and ensure failures are treated as expected; this will validate the CLI wiring exercised by TestConfigSetAutoCleanup and the existing FuzzIsValidBool.cli/cmd/update_cleanup_test.go (1)
261-291: 🧹 Nitpick | 🔵 TrivialAssert the full union contract in
FuzzMergeKeepIDs.This fuzz target only proves that keys from
currentandprevioussurvive. A brokenmergeKeepIDsimplementation that also injects unrelated IDs would still pass, so add union-size and no-extra-key assertions here as well.Suggested strengthening
func FuzzMergeKeepIDs(f *testing.F) { f.Add("aaa,bbb", "bbb,ccc") f.Add("", "") f.Add("x", "") f.Add("", "y") f.Fuzz(func(t *testing.T, currentCSV, previousCSV string) { toMap := func(csv string) map[string]bool { m := map[string]bool{} for _, p := range strings.Split(csv, ",") { p = strings.TrimSpace(p) if p != "" { m[p] = true } } return m } current := toMap(currentCSV) previous := toMap(previousCSV) got := mergeKeepIDs(current, previous) + union := map[string]bool{} + for id := range current { + union[id] = true + } + for id := range previous { + union[id] = true + } for id := range current { if !got[id] { t.Fatalf("missing current id %q", id) } } for id := range previous { if !got[id] { t.Fatalf("missing previous id %q", id) } } + if len(got) != len(union) { + t.Fatalf("len(result) = %d, want %d", len(got), len(union)) + } + for id := range got { + if !union[id] { + t.Fatalf("unexpected id %q in result", id) + } + } }) }As per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/update_cleanup_test.go` around lines 261 - 291, The fuzz test FuzzMergeKeepIDs only checks that current and previous keys appear but doesn't assert the result is exactly their union; update the test to compute the expected union of keys from current and previous and assert two things on the result of mergeKeepIDs: (1) its length equals the expected union size, and (2) it contains no keys outside that union (i.e., iterate over got and fail if any key is not in the expected union). Locate the FuzzMergeKeepIDs function and the toMap/current/previous variables and add these union-size and no-extra-key assertions against mergeKeepIDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/config_test.go`:
- Around line 239-245: The current test reads the command output into the
variable out and asserts "Auto cleanup" and "false" separately which can falsely
pass; change the assertion to find the specific line/row for the "Auto cleanup"
label and then assert that that same line contains "false" (e.g., split out by
lines, locate the line containing "Auto cleanup" and assert that line includes
"false") so that the "Auto cleanup" row itself carries the expected value
instead of checking strings.Contains(out, ...) independently.
In `@cli/cmd/update.go`:
- Around line 328-331: When suppressing image-line diffs, ensure you only
auto-apply when the same service is being compared: update the condition that
currently checks imageLinePattern.MatchString(oldLines[i]) and
imageLinePattern.MatchString(newLines[i]) so it uses
imageLinePattern.FindStringSubmatch on both lines (or a capturing regex variant)
to extract the service name (e.g., group for service/container) and then only
continue when both matches exist and the extracted service names are equal;
otherwise fall through to treat it as a real diff. This change should be made
where imageLinePattern, oldLines and newLines are used (the image-line diff
suppression block) and must preserve existing behavior for identical-service
image updates.
- Around line 28-40: The flag --skip-cli-update should not be bound to the
package-global skipCLIUpdate because that global retains state across repeated
Execute() calls; instead remove the global binding and any uses of
skipCLIUpdate, keep the flag registered on updateCmd (or register it to a local
var in init if needed), and change runUpdate/updateCLI to read the flag
per-invocation via cmd.Flags().GetBool("skip-cli-update") (handle the returned
value and error) so each execution reads the current flag value rather than a
stale global.
---
Duplicate comments:
In `@cli/cmd/config_test.go`:
- Around line 163-221: Add a fuzz test that targets the Cobra argument parsing
and validation path for the auto_cleanup flag (so it exercises
rootCmd.Execute(), not just FuzzIsValidBool). Create a new fuzz function (e.g.,
FuzzConfigSetAutoCleanup) that calls
rootCmd.SetArgs([]string{"config","set","auto_cleanup", value, "--data-dir",
dir}) for varied inputs, runs rootCmd.Execute(), and asserts behavior matches
the helper (accepts only "true"/"false" in the expected casing) — use
t.TempDir() for data dir, reuse config.DefaultState()/config.Save/load to
initialize state, and ensure failures are treated as expected; this will
validate the CLI wiring exercised by TestConfigSetAutoCleanup and the existing
FuzzIsValidBool.
In `@cli/cmd/update_cleanup_test.go`:
- Around line 261-291: The fuzz test FuzzMergeKeepIDs only checks that current
and previous keys appear but doesn't assert the result is exactly their union;
update the test to compute the expected union of keys from current and previous
and assert two things on the result of mergeKeepIDs: (1) its length equals the
expected union size, and (2) it contains no keys outside that union (i.e.,
iterate over got and fail if any key is not in the expected union). Locate the
FuzzMergeKeepIDs function and the toMap/current/previous variables and add these
union-size and no-extra-key assertions against mergeKeepIDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f88cf6c-c531-491d-ae02-4512b0abe43b
📒 Files selected for processing (4)
cli/cmd/config_test.gocli/cmd/update.gocli/cmd/update_cleanup_test.gocli/internal/config/state_test.go
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
{**/*.py,cli/**/*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Line length is 88 characters (enforced by ruff).
Files:
cli/internal/config/state_test.gocli/cmd/update_cleanup_test.gocli/cmd/config_test.gocli/cmd/update.go
cli/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing in Go with native
testing.Ffuzz functions.
Files:
cli/internal/config/state_test.gocli/cmd/update_cleanup_test.gocli/cmd/config_test.go
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Dependabot: daily updates (uv, github-actions, npm, pre-commit, docker, gomod), grouped minor/patch, no auto-merge. Use `/review-dep-pr` before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Version bumping (pre-1.0): `fix:`/`feat:` = patch, `feat!:`/`BREAKING CHANGE` = minor. Uses Conventional Commits for Release Please automation.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/**/*_test.go : Use property-based testing in Go with native `testing.F` fuzz functions.
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_cleanup_test.gocli/cmd/config_test.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_cleanup_test.gocli/cmd/config_test.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Use native `testing.F` fuzz functions (`Fuzz*`) for fuzz testing Go code
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_cleanup_test.gocli/cmd/config_test.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Run Go lint via `golangci-lint run`, vet via `go vet`, tests via `go test ./...`, and fuzz via `go test -fuzz=FuzzTarget -fuzztime=30s`
Applied to files:
cli/cmd/update_cleanup_test.gocli/cmd/config_test.go
📚 Learning: 2026-03-16T19:52:03.656Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T19:52:03.656Z
Learning: Applies to cli/**/*.go : Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to cli/**/*.go : Lint Go code with `golangci-lint` and `go vet`. Run tests with `-race` flag to detect race conditions.
Applied to files:
cli/cmd/config_test.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/go.mod : CLI dependencies: Go 1.26+, in `cli/go.mod` (Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (3)
cli/internal/config/state_test.go (1)
cli/internal/config/state.go (1)
IsValidBool(137-139)
cli/cmd/config_test.go (3)
cli/internal/config/state.go (3)
DefaultState(36-48)Save(230-244)Load(65-103)cli/internal/config/paths.go (1)
DataDir(18-31)cli/cmd/root.go (1)
Execute(101-107)
cli/cmd/update.go (2)
cli/internal/docker/client.go (1)
Info(22-29)cli/internal/config/state.go (1)
State(16-31)
…update Add `auto_cleanup` boolean config option that, when enabled, automatically removes old Docker images at the end of `synthorg update`, keeping only the current and previous version. Also adds a hint after manual `synthorg cleanup` suggesting the config flag. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…efs differ During `synthorg update`, the compose diff prompt showed image digest and version comment changes that are expected during every update. Rename `isVersionCommentOnly` to `isUpdateBoilerplateOnly` and extend it to also recognize image reference changes as auto-applicable boilerplate. Also add hidden `--skip-cli-update` flag forwarded by re-exec to suppress the redundant "Checking for updates..." message after CLI self-update. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Extract postPullActions helper to keep updateContainerImages under 50 lines - Log warning when previous image ID capture fails (was silently discarded) - Add comment explaining empty-input test case for isUpdateBoilerplateOnly - Document auto_cleanup config and compose auto-apply in user guide Pre-reviewed by 3 agents, 5 findings addressed. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Add FuzzIsValidBool and FuzzMergeKeepIDs fuzz targets (CodeRabbit) - Combine config set auto_cleanup tests into table-driven (Gemini) - Move previousIDs capture after user confirmation (CodeRabbit) - Use strings.Contains instead of bytes.Contains in test (go-reviewer) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
- Assert Auto cleanup label and value on same output line (config_test.go) - Compare service names in isUpdateBoilerplateOnly image line matching - Remove global skipCLIUpdate; read flag per-invocation via cmd.Flags().GetBool - Add FuzzConfigSetAutoCleanup exercising CLI wiring - Assert exact union (size + no extra keys) in FuzzMergeKeepIDs Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Aureliolo
force-pushed
the
feat/cli-add-auto-cleanup-images
branch
from
72979d4 to
585775f
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cli/cmd/update.go (1)
107-110: 🧹 Nitpick | 🔵 TrivialConsider handling the error from
GetBool.While the flag is always registered in
init(), ignoring the error could mask issues during future refactoring. Explicitly returning or logging the error would be more defensive.♻️ Proposed fix
// After re-exec the CLI was just replaced -- skip the redundant check. - if skip, _ := cmd.Flags().GetBool("skip-cli-update"); skip { + skip, err := cmd.Flags().GetBool("skip-cli-update") + if err != nil { + return fmt.Errorf("reading --skip-cli-update: %w", err) + } + if skip { return nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cli/cmd/update.go` around lines 107 - 110, The call to cmd.Flags().GetBool("skip-cli-update") should handle the returned error instead of discarding it; update the code around that call (where GetBool is used, e.g., in the Run/RunE handler that checks skip-cli-update) to check the error and either return it (or wrap with context) or log it via process logger, e.g., if err != nil { return fmt.Errorf("getting skip-cli-update flag: %w", err) } before using skip. Ensure you reference GetBool and the skip variable so the change is applied at the correct spot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cli/cmd/cleanup.go`:
- Around line 57-65: The hint about enabling auto-cleanup should not be shown
unconditionally after confirmAndCleanup returns; update the logic so the hint is
only displayed when images were actually removed or the cleanup completed
successfully. Either move the out.Hint(...) call into confirmAndCleanup and show
it after a successful removal path inside confirmAndCleanup (referencing the
confirmAndCleanup function and its use of out), or have confirmAndCleanup return
a boolean (e.g., removedAny) along with error and only call out.Hint(...) in the
caller when removedAny is true; also keep the existing guard on
state.AutoCleanup.
In `@cli/cmd/update_cleanup.go`:
- Around line 49-53: The current early return after calling
listNonCurrentImages(ctx, errOut, info, keepIDs) hides whether we returned
because of an error or simply because there are no old images; change this to
explicitly handle the error: check if err != nil and in that case write a
warning (using errOut or the same logger used on line 43) that includes the
error context, then return; if err == nil and len(old) == 0, return as before.
Reference listNonCurrentImages, the returned err and old, and the errOut/logger
to locate where to add the explicit error log.
---
Duplicate comments:
In `@cli/cmd/update.go`:
- Around line 107-110: The call to cmd.Flags().GetBool("skip-cli-update") should
handle the returned error instead of discarding it; update the code around that
call (where GetBool is used, e.g., in the Run/RunE handler that checks
skip-cli-update) to check the error and either return it (or wrap with context)
or log it via process logger, e.g., if err != nil { return fmt.Errorf("getting
skip-cli-update flag: %w", err) } before using skip. Ensure you reference
GetBool and the skip variable so the change is applied at the correct spot.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e9fd686-0272-43b1-87cc-4d86966ff8e4
📒 Files selected for processing (10)
cli/cmd/cleanup.gocli/cmd/config.gocli/cmd/config_test.gocli/cmd/update.gocli/cmd/update_cleanup.gocli/cmd/update_cleanup_test.gocli/cmd/update_test.gocli/internal/config/state.gocli/internal/config/state_test.godocs/user_guide.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CLI Test (macos-latest)
- GitHub Check: CLI Test (windows-latest)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (2)
{**/*.py,cli/**/*.go}
📄 CodeRabbit inference engine (CLAUDE.md)
Line length is 88 characters (enforced by ruff).
Files:
cli/cmd/cleanup.gocli/cmd/config.gocli/internal/config/state.gocli/internal/config/state_test.gocli/cmd/update_test.gocli/cmd/update_cleanup.gocli/cmd/config_test.gocli/cmd/update.gocli/cmd/update_cleanup_test.go
cli/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Use property-based testing in Go with native
testing.Ffuzz functions.
Files:
cli/internal/config/state_test.gocli/cmd/update_test.gocli/cmd/config_test.gocli/cmd/update_cleanup_test.go
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Dependabot: daily updates (uv, github-actions, npm, pre-commit, docker, gomod), grouped minor/patch, no auto-merge. Use `/review-dep-pr` before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/*.yml : Dependabot: daily updates for uv + github-actions + npm + pre-commit + docker + gomod, grouped minor/patch, no auto-merge. Use `/review-dep-pr` to review Dependabot PRs before merging.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Version bumping (pre-1.0): `fix:`/`feat:` = patch, `feat!:`/`BREAKING CHANGE` = minor. Uses Conventional Commits for Release Please automation.
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to src/synthorg/**/*.py : Package structure: src/synthorg/ organized as: api/ (REST+WebSocket, Litestar), auth/ (auth subpackage), backup/ (scheduled/manual backups), budget/ (cost tracking, CFO), cli/ (superseded by Go CLI), communication/ (message bus, meetings), config/ (YAML loading), core/ (domain models, resilience config), engine/ (orchestration, task state, coordination, approval gates, stagnation detection, context budget, compaction), hr/ (hiring, performance, promotion), memory/ (pluggable backend, Mem0, retrieval, consolidation), persistence/ (operational data, SQLite, settings), observability/ (logging, correlation, sinks), providers/ (LLM abstraction, LiteLLM, auth types, presets, runtime CRUD), settings/ (runtime-editable, typed definitions, encryption, config bridge), security/ (SecOps, rule engine, output scanning, progressive trust, autonomy levels), templates/ (company templates, personalities), tools/ (registry, built-in tools, git, sandbox, code_runner, MCP...
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-19T07:12:14.508Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T07:12:14.508Z
Learning: Applies to docker/Dockerfile.sandbox : Docker sandbox: `synthorg-sandbox` — Python 3.14 + Node.js + git, non-root (UID 10001), agent code execution sandbox
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to docker/{Dockerfile*,compose.yml} : Docker: Backend uses 3-stage build (builder → setup → distroless runtime), Chainguard Python, non-root (UID 65532), CIS-hardened. Web uses nginxinc/nginx-unprivileged, Vue 3 SPA with PrimeVue + Tailwind CSS, SPA routing, API/WebSocket proxy to backend.
Applied to files:
docs/user_guide.md
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/**/*_test.go : Use property-based testing in Go with native `testing.F` fuzz functions.
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_test.gocli/cmd/config_test.gocli/cmd/update_cleanup_test.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to cli/**/*.go : Use native Go testing with `testing.F` fuzz functions (`Fuzz*`) for fuzz testing.
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_test.gocli/cmd/config_test.gocli/cmd/update_cleanup_test.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Use native `testing.F` fuzz functions (`Fuzz*`) for fuzz testing Go code
Applied to files:
cli/internal/config/state_test.gocli/cmd/update_test.gocli/cmd/config_test.gocli/cmd/update_cleanup_test.go
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Dependabot: auto-updates Docker image digests and versions daily.
Applied to files:
cli/cmd/update_cleanup.go
📚 Learning: 2026-03-19T11:30:29.217Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:30:29.217Z
Learning: Applies to cli/**/*.go : Run Go lint via `golangci-lint run`, vet via `go vet`, tests via `go test ./...`, and fuzz via `go test -fuzz=FuzzTarget -fuzztime=30s`
Applied to files:
cli/cmd/config_test.gocli/cmd/update_cleanup_test.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to .github/workflows/cli.yml : CLI workflow: Go lint (golangci-lint + go vet) + test (-race -coverprofile) + build (cross-compile: linux/darwin/windows × amd64/arm64) + govulncheck + fuzz testing (main-only, 30s/target, continue-on-error, matrix over 4 packages). cli-pass gate includes fuzz as informational. GoReleaser release on v* tags. Cosign keyless signing of checksums.txt. SLSA L3 provenance attestations. Sigstore bundle (.sigstore.json) attached. Post-release appends checksums/verification/provenance to draft release notes.
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-16T19:52:03.656Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-16T19:52:03.656Z
Learning: Applies to cli/**/*.go : Lint CLI Go code with golangci-lint and go vet; test with go test -race; check vulnerabilities with govulncheck
Applied to files:
cli/cmd/config_test.gocli/cmd/update.go
📚 Learning: 2026-03-15T18:17:43.675Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T18:17:43.675Z
Learning: Applies to cli/** : CLI: Go 1.26+, dependencies in cli/go.mod (Cobra, charmbracelet/huh).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/go.mod : Go CLI dependencies: Go 1.26+, Cobra (commands), charmbracelet/huh (interactive CLI), charmbracelet/lipgloss (styled output).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-22T16:44:15.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T16:44:15.487Z
Learning: Applies to cli/go.mod : CLI dependencies: Go 1.26+, in `cli/go.mod` (Cobra, charmbracelet/huh, charmbracelet/lipgloss, sigstore-go, go-containerregistry, go-tuf).
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-15T21:32:02.880Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T21:32:02.880Z
Learning: Applies to cli/**/*.go : Go CLI (Go 1.26+) uses Cobra for commands, charmbracelet/huh for interactive CLI, charmbracelet/lipgloss for styled output. Cross-platform builds (linux/darwin/windows × amd64/arm64). GoReleaser for releases with cosign keyless signing of checksums.txt. SLSA L3 provenance attestations via actions/attest-build-provenance.
Applied to files:
cli/cmd/update.go
📚 Learning: 2026-03-19T11:19:40.044Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-19T11:19:40.044Z
Learning: Applies to go.mod : Maintain Go 1.26+ requirement. Dependencies: Cobra (CLI framework), charmbracelet/huh and charmbracelet/lipgloss (UI), sigstore-go (code signing), go-containerregistry (container image verification), go-tuf (TUF client for Sigstore).
Applied to files:
cli/cmd/update.go
🧬 Code graph analysis (5)
cli/cmd/config.go (1)
cli/internal/config/state.go (2)
IsValidBool(137-139)BoolNames(142-142)
cli/internal/config/state_test.go (1)
cli/internal/config/state.go (1)
IsValidBool(137-139)
cli/cmd/update_cleanup.go (3)
cli/internal/docker/client.go (2)
Info(22-29)RunCmd(116-125)cli/internal/config/state.go (1)
State(16-31)cli/internal/ui/ui.go (1)
NewUI(62-76)
cli/cmd/config_test.go (3)
cli/internal/config/state.go (3)
DefaultState(36-48)Save(230-244)Load(65-103)cli/internal/config/paths.go (1)
DataDir(18-31)cli/cmd/root.go (1)
Execute(101-107)
cli/cmd/update.go (2)
cli/internal/docker/client.go (1)
Info(22-29)cli/internal/config/state.go (1)
State(16-31)
🔇 Additional comments (24)
docs/user_guide.md (1)
27-29: LGTM!The documentation clearly explains the new auto-apply behavior for compose changes and documents the
auto_cleanupconfiguration option with the correct command syntax.cli/internal/config/state.go (2)
29-29: LGTM!The
AutoCleanupfield is correctly added with appropriate JSON serialization. The zero value (false) is the sensible default for opt-in behavior.
136-142: LGTM!The strict boolean validation accepting only
"true"or"false"(case-sensitive) is intentional per PR objectives to avoid ambiguity with values like"yes","1", etc.cli/internal/config/state_test.go (3)
40-42: LGTM!Correctly asserts the default value for
AutoCleanupisfalse.
331-355: LGTM!Good addition to the round-trip test ensuring
AutoCleanup: trueis correctly persisted and loaded.
357-404: LGTM!Comprehensive test coverage for
IsValidBool:
- Table-driven test covers edge cases including case sensitivity, numeric strings, and yes/no variants
- Fuzz test with native
testing.Fvalidates the invariant correctly- Both tests use
t.Parallel()appropriatelyAs per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.cli/cmd/config.go (2)
16-16: LGTM!The
supportedConfigKeysslice correctly includesauto_cleanupand maintains alphabetical ordering.
110-114: LGTM!The
auto_cleanupcase follows the established pattern for other config keys with proper validation and clear error messaging.cli/cmd/update_cleanup_test.go (2)
198-259: LGTM!Comprehensive table-driven test for
mergeKeepIDscovering nil inputs, disjoint sets, and overlapping keys. The assertions verify both the result length and presence of all input keys.
261-305: LGTM!The fuzz test addresses the previous review feedback. It correctly:
- Uses native
testing.Fas required- Builds expected union for comparison
- Verifies exact set equality (correct size, all expected present, no extras)
As per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.cli/cmd/update_cleanup.go (2)
86-97: LGTM!The
mergeKeepIDshelper is simple and correct. It properly handles nil maps (range over nil is a no-op in Go) and pre-allocates reasonable capacity.
31-84: LGTM!The
autoCleanupOldImagesfunction follows the best-effort pattern correctly:
- Graceful error handling (warnings instead of failures)
- Context cancellation check in the removal loop
- Reuses
isImageInUsefor consistent behavior with manual cleanup- Clear progress output via the UI
cli/cmd/update_test.go (2)
450-518: LGTM!Well-structured test cases for
isUpdateBoilerplateOnly:
- Covers version comment changes, image digest/tag changes, multiple image changes
- Correctly expects
falsefor substantive changes (ports, new services)- The empty input case is documented with a clear comment about the caller's early exit
520-545: LGTM!Test and fuzz function correctly renamed to match the new function name. The fuzz seed corpus is appropriately extended with an indented image-line variant to exercise the
imageLinePatternmatching.As per coding guidelines
cli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.cli/cmd/config_test.go (5)
8-8: LGTM!The
stringsimport is appropriately added for the newTestConfigShowAutoCleanuptest that usesstrings.Splitandstrings.Contains.
163-201: LGTM!Well-structured table-driven test that verifies both directions of the
auto_cleanuptoggle. The test correctly sets initial state opposite to the target value and validates persistence viaconfig.Load.
203-233: LGTM!The fuzz test properly exercises the
auto_cleanupvalidation usingtesting.Fwith appropriate seed values. The strict"true"/"false"acceptance logic aligns with the validation requirements. As per coding guidelinescli/**/*_test.go: Use property-based testing in Go with nativetesting.Ffuzz functions.
235-253: LGTM!Deterministic test confirming rejection of common boolean-ish values that don't match the strict
"true"/"false"requirement. Complements the fuzz test with explicit coverage of typical user mistakes.
255-285: LGTM!The row-specific assertion correctly ensures the
Auto cleanuplabel andfalsevalue appear on the same line, addressing the previous review feedback. The test properly validates the defaultAutoCleanupvalue in the config show output.cli/cmd/update.go (5)
306-333: LGTM!The
isUpdateBoilerplateOnlyfunction correctly addresses the previous review feedback by comparing service names (oldSub[2] == newSub[2]) before allowing image-line diffs to be auto-applied. The line count check and version comment handling are appropriate.
223-223: LGTM!Correctly passes
--skip-cli-updateto the re-executed binary to suppress the redundant "Checking for updates..." message after CLI self-update.
494-504: LGTM!The image ID capture is correctly positioned after user confirmation and only runs when
AutoCleanupis enabled. Best-effort error handling with a warning is appropriate—auto-cleanup can still function with partial information.
515-532: LGTM!The
postPullActionshelper cleanly encapsulates the restart, auto-cleanup, and hint logic. The conditional flow correctly handles both auto-cleanup and passive hint paths based on theAutoCleanupsetting.
292-301: LGTM!Auto-applying compose changes when only boilerplate (version comment and image references) differs improves UX by avoiding unnecessary prompts during routine updates while preserving user confirmation for actual template changes.
- Show auto-cleanup hint only when images were actually removed - Separate error and empty-result paths in autoCleanupOldImages - Handle GetBool error for skip-cli-update flag instead of discarding Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
🤖 I have created a release *beep* *boop* --- ## [0.4.8](v0.4.7...v0.4.8) (2026-03-22) ### Features * add auto_cleanup config and improve update UX ([#741](#741)) ([289638f](289638f)) * add reporting lines, escalation paths, and workflow handoffs to templates ([#745](#745)) ([c374cc9](c374cc9)) * differentiate template operational configs ([#742](#742)) ([9b48345](9b48345)) * diversify personality preset assignments across templates ([#743](#743)) ([15487a5](15487a5)) * improve template metadata -- skill taxonomy, descriptions, tags, and display names ([#752](#752)) ([f333f24](f333f24)) ### Bug Fixes * resolve log analysis findings (Ollama prefix, logging, init) ([#748](#748)) ([8f871a4](8f871a4)) * use git tag for dev release container image tags ([#749](#749)) ([f30d071](f30d071)) * use subordinate_id/supervisor_id in HierarchyResolver ([#751](#751)) ([118235b](118235b)) ### Performance * add long-lived cache headers for content-hashed static assets ([#747](#747)) ([4d350b5](4d350b5)) * use worksteal distribution for pytest-xdist ([#750](#750)) ([b7dd7de](b7dd7de)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
1 participant
Summary
auto_cleanupboolean config flag (synthorg config set auto_cleanup true) that automatically removes old Docker images aftersynthorg update, keeping only the current and previous versionsynthorg cleanupsuggesting theauto_cleanupconfig flagTest plan
go -C cli test ./...passes (all new tests included)go -C cli vet ./...cleango -C cli tool golangci-lint runreports 0 issuessynthorg config set auto_cleanup truepersists correctlysynthorg config set auto_cleanup falsepersists correctlysynthorg config set auto_cleanup yesrejected with errorsynthorg config showdisplays "Auto cleanup" fieldsynthorg cleanupshows hint about auto_cleanup when flag is disabledsynthorg cleanupdoes NOT show hint when auto_cleanup is already enabledsynthorg updateauto-cleans old images when auto_cleanup is enabledsynthorg updateshows old images hint (existing behavior) when auto_cleanup is disabledReview coverage
Pre-reviewed by 3 agents (go-reviewer, go-conventions-enforcer, docs-consistency), 5 findings addressed.
🤖 Generated with Claude Code