[SunkenInTime]

Nothing on screen changes: this is disk cleanup, so the proof is a before and after count from a running build.

ci: analyze test

A failed import left a folder behind in the app's data, for a strategy that was never saved, and nothing ever cleans it up. #426 described it as an empty folder, but in the running build it's usually worse: the image files had already been copied in, so a rejected file leaves its pictures on disk.

_importStrategyFile creates the new strategy's image folder before it reads the JSON, so the version check and every parse error come after it. Each one left the folder.

The import now tracks whether the strategy was saved, and if it wasn't, removes that strategy's folder, using the same getImageFolder(id).parent delete that duplicate and move already use. The id is one this import just generated, so nothing else can live there. The removal can't hide the original error: it catches its own failure, so the update message still arrives.

What I checked

Windows release driver builds, main and the fixes, same script: four failed imports and four good ones, listing the app's data folder before and after. The fixes build holds all three import PRs.

Import	main	This branch
Dropped newer-version file	folder left, 7 files	nothing left
Newer-version file inside a dropped folder	folder left, 7 files	nothing left
Import .ica, newer-version file	folder left, 7 files	nothing left
Dropped .ica with no strategy in it	folder left, 1 file	nothing left
4 good imports, 3 of them the shipped fixtures	folder kept	folder kept

On main the script left 8 folders, 4 of them leftovers. This branch left 4, one per strategy saved. canvas-compatibility-v97.ica imported and opened on both builds with the same counts (58 agents, 29 texts on its first page).

Three new tests in strategy_integrity_test.dart run the real import against a throwaway app folder, with a newer file, a zip with no strategy, and a broken strategy. All three fail without the fix.

Web has no import, so nothing changes in a browser.

What this doesn't cover

• Folders already left on people's machines stay. They only cost disk space.
• A failed strategy inside a dropped folder can still leave the library folder that import created, empty. That's a library folder and a separate behaviour.

Confidence Score: 5/5

The PR appears safe to merge; no actionable issues were found.

Summary

Removes the new strategy’s image folder when an import fails before saving.

• Failed imports no longer leave their image folders behind.

Source review found no actionable issues. Tests and the running app were not executed during this review.

SunkenInTime explicitly deferred removing folders left by earlier imports and empty library folders created during failed folder imports. Those remain outside this change.

_{Reviews (1) · Last reviewed commit: "Remove a failed import's image folder" · Reviewed by Greptile}

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 54 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8dcdded6-4419-439b-b43f-d3158254e185

Commits

Reviewing files that changed from the base of the PR and between 571a841 and 44f0748.

Files selected for processing: 2

• lib/strategy/strategy_import_export.dart
• test/strategy_integrity_test.dart

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn