Skip to content

fix: auto-cleanup targets newly pulled images instead of old ones#884

Merged
Aureliolo merged 2 commits intomainfrom
feat/improve-auto-cleanup
Mar 27, 2026
Merged

fix: auto-cleanup targets newly pulled images instead of old ones#884
Aureliolo merged 2 commits intomainfrom
feat/improve-auto-cleanup

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Summary

  • pullAndPersist saved new VerifiedDigests to disk but didn't return them to the caller. updateContainerImages built updatedState with the new ImageTag but stale digests, so collectCurrentImageIDs resolved old image IDs as "current". Both keep sets pointed at old images, leaving the just-pulled (in-use) images as cleanup candidates -- causing synthorg update to try removing the images it just pulled (all skipped with "in use").
  • Return digest pins from pullAndPersist so the caller propagates them into updatedState before auto-cleanup runs.

Test plan

  • go -C cli vet ./... passes
  • go -C cli test ./... passes
  • go -C cli build succeeds
  • Run synthorg update with auto_cleanup: true -- verify new images are kept and only genuinely old images are removed

🤖 Generated with Claude Code

pullAndPersist saved new VerifiedDigests to disk but did not return
them. The caller built updatedState with the new ImageTag but stale
digests, so collectCurrentImageIDs resolved old image IDs as "current".
Both the current and previous keep sets pointed at old images, causing
the just-pulled (in-use) images to be flagged for removal.

Return digest pins from pullAndPersist so the caller can propagate them
into updatedState before auto-cleanup runs.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f727de1-0a9f-4573-ae6f-23854c503f5f

📥 Commits

Reviewing files that changed from the base of the PR and between 70f1c97 and 2967aa3.

📒 Files selected for processing (1)
  • cli/cmd/update.go
📜 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). (4)
  • GitHub Check: CLI Test (windows-latest)
  • GitHub Check: CLI Test (macos-latest)
  • GitHub Check: Dependency Review
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
cli/**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Go CLI: golangci-lint + go vet checked in pre-commit hooks and CI (conditional on cli/**/*.go changes)

Files:

  • cli/cmd/update.go
🧠 Learnings (1)
📓 Common learnings
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.
🔇 Additional comments (3)
cli/cmd/update.go (3)

322-327: LGTM! Correctly propagates the updated state with new digests.

This fix addresses the root cause: previously the caller built updatedState locally with stale digests, causing autoCleanupOldImages to identify old images as "current" via collectCurrentImageIDs. Now the caller receives the persisted state with correct VerifiedDigests directly from pullAndPersist.


390-391: Good refactor: returning the persisted state eliminates duplicated state construction.

The updated signature and documentation clearly communicate that callers receive the canonical persisted state, preventing the previous bug where state was constructed inconsistently in multiple places.


420-437: Correct control flow: error paths return original state, success returns persisted state.

The error handling is sound:

  • On verification/pull/save failure, rollback restores compose.yml and returning the original state maintains consistency (caller discards it anyway since it returns the error).
  • On success, the returned updatedState contains the VerifiedDigests from digestPins (line 432), which is the critical fix ensuring collectCurrentImageIDs resolves to newly pulled images rather than stale ones.

Walkthrough

The pullAndPersist function in cli/cmd/update.go now returns (config.State, error) instead of only error. On failure paths that trigger rollback it returns the original incoming state paired with the error; on success it returns an updatedState containing the new ImageTag and VerifiedDigests. updateContainerImages no longer constructs updatedState locally and forwards the state returned by pullAndPersist into postPullActions.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing auto-cleanup to target old images instead of newly pulled ones, which directly addresses the bug documented in the PR description.
Description check ✅ Passed The description clearly explains the bug, root cause, and solution, demonstrating strong correlation with the changeset's scope and intent.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 40.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 2967aa3.
Ensure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice.

Scanned Files

None

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the pullAndPersist function to return verified digest pins, which are then stored in the application state. The review suggests refactoring the function to return the entire config.State object instead of just the digest pins, which would centralize state construction and simplify the calling code in updateContainerImages.

// existing compose instead of regenerating from the template.
func pullAndPersist(ctx context.Context, cmd *cobra.Command, info docker.Info, state config.State, tag, safeDir string, preserveCompose bool) error {
// Returns the verified digest pins (nil when --skip-verify is used).
func pullAndPersist(ctx context.Context, cmd *cobra.Command, info docker.Info, state config.State, tag, safeDir string, preserveCompose bool) (map[string]string, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Instead of returning just digestPins, consider returning the entire updatedState (config.State) from this function. This would encapsulate the state creation logic within pullAndPersist and prevent the caller (updateContainerImages) from needing to reconstruct the state, reducing code duplication.

The function signature would become func pullAndPersist(...) (config.State, error). Then, updateContainerImages could be simplified:

// in updateContainerImages
updatedState, err := pullAndPersist(ctx, cmd, info, state, tag, safeDir, preserveCompose)
if err != nil {
    return err
}
return postPullActions(cmd, info, safeDir, state, updatedState, previousIDs)

This would make the code more maintainable by ensuring that the logic for creating the updated state lives in only one place.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 27, 2026
Eliminates duplicated state construction between pullAndPersist and its
caller. The function already builds and persists the updated state
internally -- now it returns it directly instead of just digest pins.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Aureliolo Aureliolo merged commit 50e6591 into main Mar 27, 2026
39 checks passed
@Aureliolo Aureliolo deleted the feat/improve-auto-cleanup branch March 27, 2026 17:34
Aureliolo added a commit that referenced this pull request Mar 30, 2026
🤖 I have created a release *beep* *boop*
---
#MAJOR CHANGES; We got a somewhat working webui :)

##
[0.5.0](v0.4.9...v0.5.0)
(2026-03-30)


### Features

* add analytics trends and budget forecast API endpoints
([#798](#798))
([16b61f5](16b61f5))
* add department policies to default templates
([#852](#852))
([7a41548](7a41548))
* add remaining activity event types (task_started, tool_used,
delegation, cost_incurred)
([#832](#832))
([4252fac](4252fac))
* agent performance, activity, and history API endpoints
([#811](#811))
([9b75c1d](9b75c1d))
* Agent Profiles and Detail pages (biography, career, performance)
([#874](#874))
([62d7880](62d7880))
* app shell, Storybook, and CI/CD pipeline
([#819](#819))
([d4dde90](d4dde90))
* Approvals page with risk grouping, urgency indicators, batch actions
([#889](#889))
([4e9673d](4e9673d))
* Budget Panel page (P&L dashboard, breakdown charts, forecast)
([#890](#890))
([b63b0f1](b63b0f1))
* build infrastructure layer (API client, auth, WebSocket)
([#815](#815))
([9f01d3e](9f01d3e))
* CLI global options infrastructure, UI modes, exit codes, env vars
([#891](#891))
([fef4fc5](fef4fc5))
* CodeMirror editor and theme preferences toggle
([#905](#905),
[#807](#807))
([#909](#909))
([41fbedc](41fbedc))
* Company page (department/agent management)
([#888](#888))
([cfb88b0](cfb88b0))
* comprehensive hint coverage across all CLI commands
([#900](#900))
([937974e](937974e))
* config system extensions, per-command flags for
init/start/stop/status/logs
([#895](#895))
([32f83fe](32f83fe))
* configurable currency system replacing hardcoded USD
([#854](#854))
([b372551](b372551))
* Dashboard page (metric cards, activity feed, budget burn)
([#861](#861))
([7d519d5](7d519d5))
* department health, provider status, and activity feed endpoints
([#818](#818))
([6d5f196](6d5f196))
* design tokens and core UI components
([#833](#833))
([ed887f2](ed887f2))
* extend approval, meeting, and budget API responses
([#834](#834))
([31472bf](31472bf))
* frontend polish -- real-time UX, accessibility, responsive,
performance ([#790](#790),
[#792](#792),
[#791](#791),
[#793](#793))
([#917](#917))
([f04a537](f04a537))
* implement human roles and access control levels
([#856](#856))
([d6d8a06](d6d8a06))
* implement semantic conflict detection in workspace merge
([#860](#860))
([d97283b](d97283b))
* interaction components and animation patterns
([#853](#853))
([82d4b01](82d4b01))
* Login page + first-run bootstrap + Company page
([#789](#789),
[#888](#888))
([#896](#896))
([8758e8d](8758e8d))
* Meetings page with timeline viz, token bars, contribution formatting
([#788](#788))
([#904](#904))
([b207f46](b207f46))
* Messages page with threading, channel badges, sender indicators
([#787](#787))
([#903](#903))
([28293ad](28293ad))
* Org Chart force-directed view and drag-drop reassignment
([#872](#872),
[#873](#873))
([#912](#912))
([a68a938](a68a938))
* Org Chart page (living nodes, status, CRUD, department health)
([#870](#870))
([0acbdae](0acbdae))
* per-command flags for remaining commands, auto-behavior wiring,
help/discoverability
([#897](#897))
([3f7afa2](3f7afa2))
* Providers page with backend rework -- health, CRUD, subscription auth
([#893](#893))
([9f8dd98](9f8dd98))
* scaffold React + Vite + TypeScript + Tailwind project
([#799](#799))
([bd151aa](bd151aa))
* Settings page with search, dependency indicators, grouped rendering
([#784](#784))
([#902](#902))
([a7b9870](a7b9870))
* Setup Wizard rebuild with template comparison, cost estimator, theme
customization ([#879](#879))
([ae8b50b](ae8b50b))
* setup wizard UX -- template filters, card metadata, provider form
reuse ([#910](#910))
([7f04676](7f04676))
* setup wizard UX overhaul -- mode choice, step reorder, provider fixes
([#907](#907))
([ee964c4](ee964c4))
* structured ModelRequirement in template agent configs
([#795](#795))
([7433548](7433548))
* Task Board page (rich Kanban, filtering, dependency viz)
([#871](#871))
([04a19b0](04a19b0))


### Bug Fixes

* align frontend types with backend and debounce WS refetches
([#916](#916))
([134c11b](134c11b))
* auto-cleanup targets newly pulled images instead of old ones
([#884](#884))
([50e6591](50e6591))
* correct wipe backup-skip flow and harden error handling
([#808](#808))
([c05860f](c05860f))
* improve provider setup in wizard, subscription auth, dashboard bugs
([#914](#914))
([87bf8e6](87bf8e6))
* improve update channel detection and add config get command
([#814](#814))
([6b137f0](6b137f0))
* resolve all ESLint warnings, add zero-warnings enforcement
([#899](#899))
([079b46a](079b46a))
* subscription auth uses api_key, base URL optional for cloud providers
([#915](#915))
([f0098dd](f0098dd))


### Refactoring

* semantic analyzer cleanup -- shared filtering, concurrency, extraction
([#908](#908))
([81372bf](81372bf))


### Documentation

* brand identity and UX design system from
[#765](#765) exploration
([#804](#804))
([389a9f4](389a9f4))
* page structure and information architecture for v0.5.0 dashboard
([#809](#809))
([f8d6d4a](f8d6d4a))
* write UX design guidelines with WCAG-verified color system
([#816](#816))
([4a4594e](4a4594e))


### Tests

* add unit tests for agent hooks and page components
([#875](#875))
([#901](#901))
([1d81546](1d81546))


### CI/CD

* bump actions/deploy-pages from 4.0.5 to 5.0.0 in the major group
([#831](#831))
([01c19de](01c19de))
* bump astral-sh/setup-uv from 7.6.0 to 8.0.0 in
/.github/actions/setup-python-uv in the all group
([#920](#920))
([5f6ba54](5f6ba54))
* bump codecov/codecov-action from 5.5.3 to 6.0.0 in the major group
([#868](#868))
([f22a181](f22a181))
* bump github/codeql-action from 4.34.1 to 4.35.0 in the all group
([#883](#883))
([87a4890](87a4890))
* bump sigstore/cosign-installer from 4.1.0 to 4.1.1 in the
minor-and-patch group
([#830](#830))
([7a69050](7a69050))
* bump the all group with 3 updates
([#923](#923))
([ff27c8e](ff27c8e))
* bump wrangler from 4.76.0 to 4.77.0 in /.github in the minor-and-patch
group ([#822](#822))
([07d43eb](07d43eb))
* bump wrangler from 4.77.0 to 4.78.0 in /.github in the all group
([#882](#882))
([f84118d](f84118d))


### Maintenance

* add design system enforcement hook and component inventory
([#846](#846))
([15abc43](15abc43))
* add dev-only auth bypass for frontend testing
([#885](#885))
([6cdcd8a](6cdcd8a))
* add pre-push rebase check hook
([#855](#855))
([b637a04](b637a04))
* backend hardening -- eviction/size-caps and model validation
([#911](#911))
([81253d9](81253d9))
* bump axios from 1.13.6 to 1.14.0 in /web in the all group across 1
directory ([#922](#922))
([b1b0232](b1b0232))
* bump brace-expansion from 5.0.4 to 5.0.5 in /web
([#862](#862))
([ba4a565](ba4a565))
* bump eslint-plugin-react-refresh from 0.4.26 to 0.5.2 in /web
([#801](#801))
([7574bb5](7574bb5))
* bump faker from 40.11.0 to 40.11.1 in the minor-and-patch group
([#803](#803))
([14d322e](14d322e))
* bump https://github.com/astral-sh/ruff-pre-commit from v0.15.7 to
0.15.8 ([#864](#864))
([f52901e](f52901e))
* bump nginxinc/nginx-unprivileged from `6582a34` to `f99cc61` in
/docker/web in the all group
([#919](#919))
([df85e4f](df85e4f))
* bump nginxinc/nginx-unprivileged from `ccbac1a` to `6582a34` in
/docker/web ([#800](#800))
([f4e9450](f4e9450))
* bump node from `44bcbf4` to `71be405` in /docker/sandbox
([#827](#827))
([91bec67](91bec67))
* bump node from `5209bca` to `cf38e1f` in /docker/web
([#863](#863))
([66d6043](66d6043))
* bump picomatch in /site
([#842](#842))
([5f20bcc](5f20bcc))
* bump recharts 2-&gt;3 and @types/node 22-&gt;25 in /web
([#802](#802))
([a908800](a908800))
* Bump requests from 2.32.5 to 2.33.0
([#843](#843))
([41daf69](41daf69))
* bump smol-toml from 1.6.0 to 1.6.1 in /site
([#826](#826))
([3e5dbe4](3e5dbe4))
* bump the all group with 3 updates
([#921](#921))
([7bace0b](7bace0b))
* bump the minor-and-patch group across 1 directory with 2 updates
([#829](#829))
([93e611f](93e611f))
* bump the minor-and-patch group across 1 directory with 3 updates
([#841](#841))
([7010c8e](7010c8e))
* bump the minor-and-patch group across 1 directory with 3 updates
([#869](#869))
([548cee5](548cee5))
* bump the minor-and-patch group in /site with 2 updates
([#865](#865))
([9558101](9558101))
* bump the minor-and-patch group with 2 updates
([#867](#867))
([4830706](4830706))
* consolidate Dependabot groups to 1 PR per ecosystem
([06d2556](06d2556))
* consolidate Dependabot groups to 1 PR per ecosystem
([#881](#881))
([06d2556](06d2556))
* improve worktree skill with full dep sync and status enhancements
([#906](#906))
([772c625](772c625))
* remove Vue remnants and document framework decision
([#851](#851))
([bf2adf6](bf2adf6))
* update web dependencies and fix brace-expansion CVE
([#880](#880))
([a7a0ed6](a7a0ed6))
* upgrade to Storybook 10 and TypeScript 6
([#845](#845))
([52d95f2](52d95f2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant