feat(fix-trash): chain after stacking runs and harden the trash decisions

README.md

@@ -57,7 +57,7 @@ Scans your library and reports duplicate assets based on filename and timestamp.
 immich-stack fix-trash
 ```
 
-Identifies trashed assets and moves their related stack members to trash for consistency. With `--trash-orphaned-raws`, it also removes RAW files left without a developed companion (JPG, HEIC, ...).
+Identifies trashed assets and moves their related stack members to trash for consistency. With `--trash-orphaned-raws`, it also removes RAW files left without a developed companion (JPG, HEIC, ...). Pass `--fix-trash-after-stacking` (or set `FIX_TRASH_AFTER_STACKING=true`) to chain it after every stacking run, including in cron mode.
 
 ## Features
 

cmd/config.go

@@ -36,7 +36,10 @@ var logLevel string
 var logFormat string
 var removeSingleAssetStacks bool
 var trashOrphanedRAWs bool
+var trashOrphanedRAWsFlagSet bool
 var rawOrphanExtensions string
+var fixTrashAfterStacking bool
+var fixTrashAfterStackingFlagSet bool
 var includeVideos bool
 var includeVideosFlagSet bool
 var preventSelfRekt bool
@@ -162,6 +165,7 @@ func logStartupSummary(logger *logrus.Logger) {
 			"withDeleted":             withDeleted,
 			"removeSingleAssetStacks": removeSingleAssetStacks,
 			"trashOrphanedRAWs":       trashOrphanedRAWs,
+			"fixTrashAfterStacking":   fixTrashAfterStacking,
 			"includeVideos":           includeVideos,
 			"preventSelfRekt":         preventSelfRekt,
 			"stackConcurrency":        stackConcurrency,
@@ -218,6 +222,9 @@ func logStartupSummary(logger *logrus.Logger) {
 		if rawOrphanExtensions != "" {
 			summary = append(summary, fmt.Sprintf("raw-orphan-extensions=%s", rawOrphanExtensions))
 		}
+		if fixTrashAfterStacking {
+			summary = append(summary, "fix-trash-after-stacking=true")
+		}
 		if includeVideos {
 			summary = append(summary, "include-videos=true")
 		}
@@ -319,12 +326,19 @@ func LoadEnvForTesting() LoadEnvConfig {
 	if !removeSingleAssetStacks {
 		removeSingleAssetStacks = os.Getenv("REMOVE_SINGLE_ASSET_STACKS") == "true"
 	}
-	if !trashOrphanedRAWs {
-		trashOrphanedRAWs = os.Getenv("TRASH_ORPHANED_RAWS") == "true"
+	if !trashOrphanedRAWsFlagSet {
+		if envVal := os.Getenv("TRASH_ORPHANED_RAWS"); envVal != "" {
+			trashOrphanedRAWs = envVal == "true"
+		}
 	}
 	if rawOrphanExtensions == "" {
 		rawOrphanExtensions = os.Getenv("RAW_ORPHAN_EXTENSIONS")
 	}
+	if !fixTrashAfterStackingFlagSet {
+		if envVal := os.Getenv("FIX_TRASH_AFTER_STACKING"); envVal != "" {
+			fixTrashAfterStacking = envVal == "true"
+		}
+	}
 	if !includeVideosFlagSet {
 		if envInclude := os.Getenv("INCLUDE_VIDEOS"); envInclude != "" {
 			includeVideos = envInclude == "true"

cmd/config_test.go

@@ -47,6 +47,7 @@ func TestStartupConfigurationSummary(t *testing.T) {
 				"REMOVE_SINGLE_ASSET_STACKS": "true",
 				"TRASH_ORPHANED_RAWS":        "true",
 				"RAW_ORPHAN_EXTENSIONS":      "dng",
+				"FIX_TRASH_AFTER_STACKING":   "true",
 			},
 			wantInLog: []string{
 				"Configuration loaded",
@@ -61,6 +62,7 @@ func TestStartupConfigurationSummary(t *testing.T) {
 				`"removeSingleAssetStacks":true`,
 				`"trashOrphanedRAWs":true`,
 				`"rawOrphanExtensions":"dng"`,
+				`"fixTrashAfterStacking":true`,
 			},
 		},
 		{
@@ -375,7 +377,8 @@ func resetTestEnv() {
 		"LOG_LEVEL", "LOG_FORMAT", "LOG_FILE",
 		"DRY_RUN", "RESET_STACKS", "CONFIRM_RESET_STACK",
 		"REPLACE_STACKS", "WITH_ARCHIVED", "WITH_DELETED",
-		"REMOVE_SINGLE_ASSET_STACKS", "TRASH_ORPHANED_RAWS", "RAW_ORPHAN_EXTENSIONS", "CRITERIA",
+		"REMOVE_SINGLE_ASSET_STACKS", "TRASH_ORPHANED_RAWS", "RAW_ORPHAN_EXTENSIONS",
+		"FIX_TRASH_AFTER_STACKING", "CRITERIA",
 		"PARENT_FILENAME_PROMOTE", "PARENT_EXT_PROMOTE",
 		"FILTER_ALBUM_IDS", "FILTER_TAKEN_AFTER", "FILTER_TAKEN_BEFORE",
 	}
@@ -400,7 +403,10 @@ func resetTestEnv() {
 	logLevel = ""
 	removeSingleAssetStacks = false
 	trashOrphanedRAWs = false
+	trashOrphanedRAWsFlagSet = false
 	rawOrphanExtensions = ""
+	fixTrashAfterStacking = false
+	fixTrashAfterStackingFlagSet = false
 	filterAlbumIDs = nil
 	filterTakenAfter = ""
 	filterTakenBefore = ""
@@ -589,3 +595,25 @@ func TestDateFilterEnvVarParsing(t *testing.T) {
 		})
 	}
 }
+
+func TestExplicitFalseFlagsBeatEnv(t *testing.T) {
+	resetTestEnv()
+	defer resetTestEnv()
+
+	os.Setenv("API_KEY", "test-key")
+	os.Setenv("TRASH_ORPHANED_RAWS", "true")
+	os.Setenv("FIX_TRASH_AFTER_STACKING", "true")
+	trashOrphanedRAWsFlagSet = true
+	fixTrashAfterStackingFlagSet = true
+
+	config := LoadEnvForTesting()
+	if config.Error != nil {
+		t.Fatalf("unexpected error: %v", config.Error)
+	}
+	if trashOrphanedRAWs {
+		t.Error("explicit --trash-orphaned-raws=false must not be overridden by TRASH_ORPHANED_RAWS=true")
+	}
+	if fixTrashAfterStacking {
+		t.Error("explicit --fix-trash-after-stacking=false must not be overridden by FIX_TRASH_AFTER_STACKING=true")
+	}
+}

cmd/fixtrash.go

@@ -16,8 +16,8 @@ import (
 )
 
 /**************************************************************************************************
-** Main execution logic for fixing incomplete trash operations. Identifies trashed assets
-** and moves their stack-related assets to trash to maintain consistency.
+** Entry point for the fix-trash command: warns about settings that have no effect here,
+** splits the comma-separated API keys, and runs fixTrashForAPIKey for each of them.
 **
 ** @param cmd - Cobra command instance
 ** @param args - Command line arguments
@@ -47,115 +47,131 @@ func runFixTrash(cmd *cobra.Command, args []string) {
 		if i > 0 {
 			logger.Infof("\n")
 		}
-		client := immich.NewClient(apiURL, key, false, false, dryRun, withArchived, withDeleted, false, includeVideos, stackConcurrency, nil, "", "", logger)
-		if client == nil {
-			logger.Errorf("Invalid client for API key: %s", key)
-			continue
-		}
-		user, err := client.GetCurrentUser()
-		if err != nil {
-			logger.Errorf("Failed to fetch user for API key: %s: %v", key, err)
-			continue
-		}
-		logger.Infof("=====================================================================================")
-		logger.Infof("Fixing trash for user: %s (%s)", user.Name, user.Email)
-		logger.Infof("=====================================================================================")
-
-		/**********************************************************************************************
-		** Fetch trashed assets and all assets.
-		**********************************************************************************************/
-		trashedAssets, err := client.FetchTrashedAssets(1000)
-		if err != nil {
-			logger.Errorf("Error fetching trashed assets: %v", err)
-			continue
-		}
-		trashedAssets = filterOutPartnerAssets(trashedAssets, user.ID, logger)
+		fixTrashForAPIKey(key, logger)
+	}
+}
 
-		if len(trashedAssets) == 0 {
-			logger.Info("No trashed assets found. Nothing to fix.")
-			continue
-		}
+/**************************************************************************************************
+** fixTrashForAPIKey runs the full fix-trash flow for one API key: stack cascade from the
+** trashed assets, then the opt-in orphaned-RAW pass. Called per key by the fix-trash
+** command, and by the stacker after each run when --fix-trash-after-stacking is set.
+** A fresh client is created here on purpose: fix-trash must not inherit the stacker's
+** reset/replace/filter options. Archived assets are always fetched (withArchived=true):
+** an archived companion must be able to protect its group, but archived assets are never
+** trashed by either pass.
+**
+** @param key - API key of the user to process
+** @param logger - Logger instance
+**************************************************************************************************/
+func fixTrashForAPIKey(key string, logger *logrus.Logger) {
+	client := immich.NewClient(apiURL, key, false, false, dryRun, true, withDeleted, false, includeVideos, stackConcurrency, nil, "", "", logger)
+	if client == nil {
+		logger.Errorf("Invalid client for API key: %s", key)
+		return
+	}
+	user, err := client.GetCurrentUser()
+	if err != nil {
+		logger.Errorf("Failed to fetch user for API key: %s: %v", key, err)
+		return
+	}
+	logger.Infof("=====================================================================================")
+	logger.Infof("Fixing trash for user: %s (%s)", user.Name, user.Email)
+	logger.Infof("=====================================================================================")
 
-		logger.Infof("🗑️  Found %d trashed assets", len(trashedAssets))
+	/**********************************************************************************************
+	** Fetch trashed assets and all assets.
+	**********************************************************************************************/
+	trashedAssets, err := client.FetchTrashedAssets(1000)
+	if err != nil {
+		logger.Errorf("Error fetching trashed assets: %v", err)
+		return
+	}
+	trashedAssets = filterOutPartnerAssets(trashedAssets, user.ID, logger)
 
-		existingStacks, err := client.FetchAllStacks()
-		if err != nil {
-			logger.Errorf("Error fetching stacks: %v", err)
-			continue
-		}
+	if len(trashedAssets) == 0 {
+		logger.Info("No trashed assets found. Nothing to fix.")
+		return
+	}
 
-		allAssets, err := client.FetchAssets(1000, existingStacks)
-		if err != nil {
-			logger.Errorf("Error fetching all assets: %v", err)
-			continue
-		}
-		allAssets = filterOutPartnerAssets(allAssets, user.ID, logger)
+	logger.Infof("🗑️  Found %d trashed assets", len(trashedAssets))
 
-		activeAssets := make([]utils.TAsset, 0, len(allAssets))
-		for _, asset := range allAssets {
-			if !asset.IsTrashed {
-				activeAssets = append(activeAssets, asset)
-			}
+	existingStacks, err := client.FetchAllStacks()
+	if err != nil {
+		logger.Errorf("Error fetching stacks: %v", err)
+		return
+	}
+
+	allAssets, err := client.FetchAssets(1000, existingStacks)
+	if err != nil {
+		logger.Errorf("Error fetching all assets: %v", err)
+		return
+	}
+	allAssets = filterOutPartnerAssets(allAssets, user.ID, logger)
+
+	activeAssets := make([]utils.TAsset, 0, len(allAssets))
+	for _, asset := range allAssets {
+		if !asset.IsTrashed {
+			activeAssets = append(activeAssets, asset)
 		}
+	}
+
+	/**********************************************************************************************
+	** Find the active assets that would stack with the trashed ones (replaced files are
+	** skipped), then the orphaned RAWs.
+	**********************************************************************************************/
+	logger.Infof("🔍 Analyzing %d trashed assets against %d active assets...", len(trashedAssets), len(activeAssets))
+	assetsToTrash, triggeredBy, replacedCount, err := findStackRelatedAssets(
+		trashedAssets, activeAssets, criteria, parentFilenamePromote, parentExtPromote, logger)
+	if err != nil {
+		logger.Errorf("Error using stacker criteria: %v", err)
+		return
+	}
+	if replacedCount > 0 {
+		logger.Infof("🔄 Skipped %d trashed assets that still have an active copy", replacedCount)
+	}
 
-		/**********************************************************************************************
-		** Find the active assets that would stack with the trashed ones (replaced files are
-		** skipped), then the orphaned RAWs.
-		**********************************************************************************************/
-		logger.Infof("🔍 Analyzing %d trashed assets against %d active assets...", len(trashedAssets), len(activeAssets))
-		assetsToTrash, triggeredBy, replacedCount, err := findStackRelatedAssets(
-			trashedAssets, activeAssets, criteria, parentFilenamePromote, parentExtPromote, logger)
+	if trashOrphanedRAWs {
+		logger.Info("🔍 Looking for orphaned RAW files...")
+		orphanedRAWs, keptStackedRAWCount, err := findOrphanedRAWs(activeAssets, criteria, parentFilenamePromote, parentExtPromote, rawOrphanExtensions, logger)
 		if err != nil {
-			logger.Errorf("Error using stacker criteria: %v", err)
-			continue
+			logger.Errorf("Error detecting orphaned RAW files (continuing with pass 1 results): %v", err)
 		}
-		if replacedCount > 0 {
-			logger.Infof("🔄 Skipped %d trashed assets that appear to have been replaced", replacedCount)
+		for id, asset := range orphanedRAWs {
+			assetsToTrash[id] = asset
+			triggeredBy[id] = orphanedRAWTrigger
 		}
-
-		if trashOrphanedRAWs {
-			logger.Info("🔍 Looking for orphaned RAW files...")
-			orphanedRAWs, keptStackedRAWCount, err := findOrphanedRAWs(activeAssets, criteria, parentFilenamePromote, parentExtPromote, rawOrphanExtensions, logger)
-			if err != nil {
-				logger.Errorf("Error detecting orphaned RAW files (continuing with pass 1 results): %v", err)
-			}
-			for id, asset := range orphanedRAWs {
-				assetsToTrash[id] = asset
-				triggeredBy[id] = orphanedRAWTrigger
-			}
-			if len(orphanedRAWs) > 0 {
-				logger.Infof("📸 Found %d orphaned RAW files without a developed companion", len(orphanedRAWs))
-			}
-			if keptStackedRAWCount > 0 {
-				logger.Infof("✅ Kept %d RAW files already stacked with a developed file", keptStackedRAWCount)
-			}
-		} else {
-			logger.Info("⏭️  Orphaned RAW cleanup disabled (enable with --trash-orphaned-raws)")
+		if len(orphanedRAWs) > 0 {
+			logger.Infof("📸 Found %d orphaned RAW files without a developed companion", len(orphanedRAWs))
 		}
-
-		/**********************************************************************************************
-		** Move the identified assets to trash. The volume warning is non-blocking on purpose:
-		** fix-trash is documented for cron usage, so a confirmation gate would break
-		** unattended runs.
-		**********************************************************************************************/
-		if len(assetsToTrash) == 0 {
-			logger.Info("✅ No related assets need to be trashed.")
-			continue
+		if keptStackedRAWCount > 0 {
+			logger.Infof("✅ Kept %d RAW files already stacked with a developed file", keptStackedRAWCount)
 		}
+	} else {
+		logger.Info("⏭️  Orphaned RAW cleanup disabled (enable with --trash-orphaned-raws)")
+	}
 
-		if len(activeAssets) > 0 && len(assetsToTrash)*10 > len(activeAssets) {
-			logger.Warnf("⚠️  About to trash %d assets — more than 10%% of your %d active assets. Review the summary below carefully (use DRY_RUN=true first if unsure).", len(assetsToTrash), len(activeAssets))
-		}
+	/**********************************************************************************************
+	** Move the identified assets to trash. The volume warning is non-blocking on purpose:
+	** fix-trash is documented for cron usage, so a confirmation gate would break
+	** unattended runs.
+	**********************************************************************************************/
+	if len(assetsToTrash) == 0 {
+		logger.Info("✅ No related assets need to be trashed.")
+		return
+	}
 
-		logTrashSummary(logger, assetsToTrash, triggeredBy)
+	if len(activeAssets) > 0 && len(assetsToTrash)*10 > len(activeAssets) {
+		logger.Warnf("⚠️  About to trash %d assets — more than 10%% of your %d active assets. Review the summary below carefully (use DRY_RUN=true first if unsure).", len(assetsToTrash), len(activeAssets))
+	}
 
-		assetIDs := make([]string, 0, len(assetsToTrash))
-		for id := range assetsToTrash {
-			assetIDs = append(assetIDs, id)
-		}
-		if err := client.TrashAssets(assetIDs); err != nil {
-			logger.Errorf("Error moving assets to trash: %v", err)
-		}
+	logTrashSummary(logger, assetsToTrash, triggeredBy)
+
+	assetIDs := make([]string, 0, len(assetsToTrash))
+	for id := range assetsToTrash {
+		assetIDs = append(assetIDs, id)
+	}
+	if err := client.TrashAssets(assetIDs); err != nil {
+		logger.Errorf("Error moving assets to trash: %v", err)
 	}
 }