From 546ec3486b606bd5ee7efb1a30681290eb4fc180 Mon Sep 17 00:00:00 2001 From: Major Date: Sun, 12 Jul 2026 22:55:41 +0200 Subject: [PATCH 1/7] feat(stacker): add FIX_TRASH_AFTER_STACKING to chain fix-trash (#66) --fix-trash-after-stacking / FIX_TRASH_AFTER_STACKING=true runs the fix-trash flow after each stacking run, in both once and cron modes. The per-key fix-trash body is extracted into fixTrashForAPIKey, called by the fix-trash command loop and by both stacker loops. The chained run creates its own client on purpose: fix-trash must not inherit the stacker's reset/replace/filter options (a reset-configured client would re-trigger stack deletion from FetchAllStacks). Closes #66 --- README.md | 2 +- cmd/config.go | 8 + cmd/config_test.go | 6 +- cmd/fixtrash.go | 206 +++++++++++--------- cmd/main.go | 1 + cmd/stacker.go | 16 ++ cmd/stacker_test.go | 9 + docs/api-reference/environment-variables.md | 1 + docs/commands/fix-trash.md | 19 +- 9 files changed, 169 insertions(+), 99 deletions(-) diff --git a/README.md b/README.md index fa99238..b0a18b9 100644 --- a/README.md +++ b/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, ...). Set `FIX_TRASH_AFTER_STACKING=true` to chain it after every stacking run, including in cron mode. ## Features diff --git a/cmd/config.go b/cmd/config.go index 964bf79..1b9e0b5 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -37,6 +37,7 @@ var logFormat string var removeSingleAssetStacks bool var trashOrphanedRAWs bool var rawOrphanExtensions string +var fixTrashAfterStacking bool var includeVideos bool var includeVideosFlagSet bool var preventSelfRekt bool @@ -162,6 +163,7 @@ func logStartupSummary(logger *logrus.Logger) { "withDeleted": withDeleted, "removeSingleAssetStacks": removeSingleAssetStacks, "trashOrphanedRAWs": trashOrphanedRAWs, + "fixTrashAfterStacking": fixTrashAfterStacking, "includeVideos": includeVideos, "preventSelfRekt": preventSelfRekt, "stackConcurrency": stackConcurrency, @@ -218,6 +220,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") } @@ -325,6 +330,9 @@ func LoadEnvForTesting() LoadEnvConfig { if rawOrphanExtensions == "" { rawOrphanExtensions = os.Getenv("RAW_ORPHAN_EXTENSIONS") } + if !fixTrashAfterStacking { + fixTrashAfterStacking = os.Getenv("FIX_TRASH_AFTER_STACKING") == "true" + } if !includeVideosFlagSet { if envInclude := os.Getenv("INCLUDE_VIDEOS"); envInclude != "" { includeVideos = envInclude == "true" diff --git a/cmd/config_test.go b/cmd/config_test.go index 2ed9dea..3149e82 100644 --- a/cmd/config_test.go +++ b/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", } @@ -401,6 +404,7 @@ func resetTestEnv() { removeSingleAssetStacks = false trashOrphanedRAWs = false rawOrphanExtensions = "" + fixTrashAfterStacking = false filterAlbumIDs = nil filterTakenAfter = "" filterTakenBefore = "" diff --git a/cmd/fixtrash.go b/cmd/fixtrash.go index aa4e3a6..4c63215 100644 --- a/cmd/fixtrash.go +++ b/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,129 @@ 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. +** +** @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, withArchived, 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 appear to have been replaced", 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) } } diff --git a/cmd/main.go b/cmd/main.go index afc107c..1d42233 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -34,6 +34,7 @@ func bindFlags(rootCmd *cobra.Command) { rootCmd.PersistentFlags().BoolVar(&removeSingleAssetStacks, "remove-single-asset-stacks", false, "Remove stacks with only one asset (or set REMOVE_SINGLE_ASSET_STACKS=true)") rootCmd.PersistentFlags().BoolVar(&trashOrphanedRAWs, "trash-orphaned-raws", false, "fix-trash: also trash RAW files without a developed companion (or set TRASH_ORPHANED_RAWS=true)") rootCmd.PersistentFlags().StringVar(&rawOrphanExtensions, "raw-orphan-extensions", "", "fix-trash: comma-separated RAW extensions the orphaned-RAW pass may trash, e.g. \"dng,nef\" (default: all known RAW formats; or set RAW_ORPHAN_EXTENSIONS)") + rootCmd.PersistentFlags().BoolVar(&fixTrashAfterStacking, "fix-trash-after-stacking", false, "Run the fix-trash flow after each stacking run, in once and cron modes (or set FIX_TRASH_AFTER_STACKING=true)") rootCmd.PersistentFlags().BoolVar(&includeVideos, "include-videos", false, "Include VIDEO assets alongside IMAGE in stacking (or set INCLUDE_VIDEOS=true)") rootCmd.PersistentFlags().BoolVar(&preventSelfRekt, "prevent-self-rekt", false, "Insert a 50ms delay before each stack write to prevent overwhelming Immich on huge libraries (or set PREVENT_SELF_REKT=true)") rootCmd.PersistentFlags().IntVar(&stackConcurrency, "stack-concurrency", 0, "Parallel stack writes (default 1 = sequential; values like 10-20 speed up large libraries; or set STACK_CONCURRENCY)") diff --git a/cmd/stacker.go b/cmd/stacker.go index 8079812..562ab80 100644 --- a/cmd/stacker.go +++ b/cmd/stacker.go @@ -153,6 +153,14 @@ func getChildrenWithStack(stack []utils.TAsset) ([]string, bool) { func runStacker(cmd *cobra.Command, args []string) { logger := loadEnv() + /********************************************************************************************** + ** The chained fix-trash run never goes through runFixTrash, so its inert-setting + ** diagnostic must also live here. + **********************************************************************************************/ + if fixTrashAfterStacking && !trashOrphanedRAWs && rawOrphanExtensions != "" { + logger.Warnf("--raw-orphan-extensions (RAW_ORPHAN_EXTENSIONS) has no effect without --trash-orphaned-raws") + } + /********************************************************************************************** ** Support multiple API keys (comma-separated). **********************************************************************************************/ @@ -184,6 +192,10 @@ func runStacker(cmd *cobra.Command, args []string) { logger.Infof("=====================================================================================") logger.Info("Running in once mode") runStackerOnce(client, logger, user.ID) + if fixTrashAfterStacking { + logger.Infof("\n") + fixTrashForAPIKey(key, logger) + } } } } @@ -381,6 +393,10 @@ func runCronLoopForAllUsers(apiKeys []string, apiURL string, logger *logrus.Logg logger.Infof("Running for user: %s (%s)", user.Name, user.Email) logger.Infof("=====================================================================================") runStackerOnce(client, logger, user.ID) + if fixTrashAfterStacking { + logger.Infof("\n") + fixTrashForAPIKey(key, logger) + } } logger.Infof("Sleeping for %d seconds until next run", cronInterval) time.Sleep(time.Duration(cronInterval) * time.Second) diff --git a/cmd/stacker_test.go b/cmd/stacker_test.go index a570674..4efcc20 100644 --- a/cmd/stacker_test.go +++ b/cmd/stacker_test.go @@ -39,6 +39,7 @@ func resetGlobalConfig() { removeSingleAssetStacks = false trashOrphanedRAWs = false rawOrphanExtensions = "" + fixTrashAfterStacking = false includeVideos = false includeVideosFlagSet = false preventSelfRekt = false @@ -116,6 +117,14 @@ func TestRealCommandStructure(t *testing.T) { t.Errorf("Expected --raw-orphan-extensions to default to empty, got %s", rawOrphanExtensionsFlag.DefValue) } + fixTrashAfterStackingFlag := cmd.PersistentFlags().Lookup("fix-trash-after-stacking") + if fixTrashAfterStackingFlag == nil { + t.Fatal("Expected --fix-trash-after-stacking flag to be present") + } + if fixTrashAfterStackingFlag.DefValue != "false" { + t.Errorf("Expected --fix-trash-after-stacking to default to false, got %s", fixTrashAfterStackingFlag.DefValue) + } + // Verify subcommands are present duplicatesCmd := cmd.Commands() foundDuplicates := false diff --git a/docs/api-reference/environment-variables.md b/docs/api-reference/environment-variables.md index 5dcc35c..85f406e 100644 --- a/docs/api-reference/environment-variables.md +++ b/docs/api-reference/environment-variables.md @@ -27,6 +27,7 @@ This document provides a complete reference of all environment variables support | `REMOVE_SINGLE_ASSET_STACKS` | Remove stacks containing only one asset | false | `true` | | `TRASH_ORPHANED_RAWS` | fix-trash only: trash RAW files without a developed companion | false | `true` | | `RAW_ORPHAN_EXTENSIONS` | fix-trash only: RAW extensions the orphaned-RAW pass may trash | all RAW | `dng,nef` | +| `FIX_TRASH_AFTER_STACKING` | Run the fix-trash flow after each stacking run (once and cron modes) | false | `true` | Note: diff --git a/docs/commands/fix-trash.md b/docs/commands/fix-trash.md index 7a4a53b..927f850 100644 --- a/docs/commands/fix-trash.md +++ b/docs/commands/fix-trash.md @@ -64,6 +64,17 @@ See what would be deleted without making changes: immich-stack fix-trash --api-key your_key --dry-run ``` +### Run After Each Stacking Run + +Instead of scheduling fix-trash separately, the main stacking command can chain it after every run — including in cron mode: + +```bash +immich-stack --api-key your_key --fix-trash-after-stacking +# or in your .env: FIX_TRASH_AFTER_STACKING=true +``` + +The chained run behaves exactly like the standalone command: it ignores the stacker's album/date filters and reset/replace options, and honors `--trash-orphaned-raws` and `--raw-orphan-extensions`. + ### With Custom Criteria Use specific stacking criteria for matching: @@ -145,7 +156,13 @@ immich-stack fix-trash --api-key your_key ### 4. Scheduled Maintenance -Add to a cron job for automatic cleanup: +Either chain it to the stacker's cron mode: + +```bash +RUN_MODE=cron CRON_INTERVAL=3600 FIX_TRASH_AFTER_STACKING=true immich-stack --api-key your_key +``` + +Or schedule the standalone command: ```bash # Run weekly to maintain consistency From b8dcce9820258d0759b5dcf78d618606c009d22d Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 09:30:23 +0200 Subject: [PATCH 2/7] fix(config): explicit fix-trash flags beat their environment variables --trash-orphaned-raws=false and --fix-trash-after-stacking=false were silently overridden by TRASH_ORPHANED_RAWS=true / FIX_TRASH_AFTER_STACKING=true because the plain 'if !x' env fallback cannot tell an omitted flag from an explicit false. Both flags now use the existing Changed-tracking in PersistentPreRun, like replace-stacks and include-videos. --- cmd/config.go | 14 ++++++++++---- cmd/config_test.go | 24 ++++++++++++++++++++++++ cmd/main.go | 6 ++++++ cmd/stacker_test.go | 2 ++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 1b9e0b5..1988013 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -36,8 +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 @@ -324,14 +326,18 @@ 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 !fixTrashAfterStacking { - fixTrashAfterStacking = os.Getenv("FIX_TRASH_AFTER_STACKING") == "true" + if !fixTrashAfterStackingFlagSet { + if envVal := os.Getenv("FIX_TRASH_AFTER_STACKING"); envVal != "" { + fixTrashAfterStacking = envVal == "true" + } } if !includeVideosFlagSet { if envInclude := os.Getenv("INCLUDE_VIDEOS"); envInclude != "" { diff --git a/cmd/config_test.go b/cmd/config_test.go index 3149e82..e0e670e 100644 --- a/cmd/config_test.go +++ b/cmd/config_test.go @@ -403,8 +403,10 @@ func resetTestEnv() { logLevel = "" removeSingleAssetStacks = false trashOrphanedRAWs = false + trashOrphanedRAWsFlagSet = false rawOrphanExtensions = "" fixTrashAfterStacking = false + fixTrashAfterStackingFlagSet = false filterAlbumIDs = nil filterTakenAfter = "" filterTakenBefore = "" @@ -593,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") + } +} diff --git a/cmd/main.go b/cmd/main.go index 1d42233..203b643 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -97,6 +97,12 @@ func CreateRootCommand() *cobra.Command { if cmd.Flags().Lookup("stack-concurrency") != nil && cmd.Flags().Lookup("stack-concurrency").Changed { stackConcurrencyFlagSet = true } + if cmd.Flags().Lookup("trash-orphaned-raws") != nil && cmd.Flags().Lookup("trash-orphaned-raws").Changed { + trashOrphanedRAWsFlagSet = true + } + if cmd.Flags().Lookup("fix-trash-after-stacking") != nil && cmd.Flags().Lookup("fix-trash-after-stacking").Changed { + fixTrashAfterStackingFlagSet = true + } }, } diff --git a/cmd/stacker_test.go b/cmd/stacker_test.go index 4efcc20..4334110 100644 --- a/cmd/stacker_test.go +++ b/cmd/stacker_test.go @@ -38,8 +38,10 @@ func resetGlobalConfig() { logLevel = "" removeSingleAssetStacks = false trashOrphanedRAWs = false + trashOrphanedRAWsFlagSet = false rawOrphanExtensions = "" fixTrashAfterStacking = false + fixTrashAfterStackingFlagSet = false includeVideos = false includeVideosFlagSet = false preventSelfRekt = false From 70afba5fd67dadc99aa9c595b134dc523ad3fc25 Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 09:30:40 +0200 Subject: [PATCH 3/7] fix(fix-trash): detect surviving copies by capture time, not file timestamps The replacement heuristic compared file-system timestamps of same-name assets, which failed in both directions: - a duplicate with identical timestamps was not detected, so trashing one copy cascaded the surviving copy and its RAW companions to trash - a recycled filename from an unrelated photo (DSC_0001.jpg every 10k shots) or a metadata touch bumping UpdatedAt looked like a replacement and silently suppressed a legitimate cascade forever A trashed asset is now skipped when an active asset has the same OriginalFileName and the same LocalDateTime: an active copy of the same photo exists, so the deletion applies to that duplicate only. Also make archived assets protective but untouchable: fix-trash now always fetches archived assets so an archived companion can protect its group, and never cascades an archived group member to trash. --- cmd/fixtrash.go | 8 ++- cmd/fixtrash_analysis.go | 51 ++++++---------- cmd/fixtrash_analysis_test.go | 108 ++++++++++++++-------------------- 3 files changed, 67 insertions(+), 100 deletions(-) diff --git a/cmd/fixtrash.go b/cmd/fixtrash.go index 4c63215..75be4ef 100644 --- a/cmd/fixtrash.go +++ b/cmd/fixtrash.go @@ -56,13 +56,15 @@ func runFixTrash(cmd *cobra.Command, args []string) { ** 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. +** 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, withArchived, withDeleted, false, includeVideos, stackConcurrency, nil, "", "", 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 @@ -125,7 +127,7 @@ func fixTrashForAPIKey(key string, logger *logrus.Logger) { return } if replacedCount > 0 { - logger.Infof("🔄 Skipped %d trashed assets that appear to have been replaced", replacedCount) + logger.Infof("🔄 Skipped %d trashed assets that still have an active copy", replacedCount) } if trashOrphanedRAWs { diff --git a/cmd/fixtrash_analysis.go b/cmd/fixtrash_analysis.go index 8f6ebb2..404ec78 100644 --- a/cmd/fixtrash_analysis.go +++ b/cmd/fixtrash_analysis.go @@ -8,7 +8,6 @@ package main import ( "io" - "time" "github.com/majorfi/immich-stack/pkg/stacker" "github.com/majorfi/immich-stack/pkg/utils" @@ -16,39 +15,19 @@ import ( ) /************************************************************************************************** -** timestampAfter reports whether timestamp a is strictly after timestamp b. -** Immich returns RFC3339 timestamps; parsing them makes the comparison correct even when -** timezone offsets differ. If either side fails to parse, it falls back to a lexicographic -** comparison, which matches the previous behavior for uniform-format values. -** -** @param a - First RFC3339 timestamp -** @param b - Second RFC3339 timestamp -** @return bool - True if a is after b -**************************************************************************************************/ -func timestampAfter(a, b string) bool { - timeA, errA := time.Parse(time.RFC3339Nano, a) - timeB, errB := time.Parse(time.RFC3339Nano, b) - if errA != nil || errB != nil { - return a > b - } - return timeA.After(timeB) -} - -/************************************************************************************************** -** isReplacedByNewerCopy reports whether a trashed asset appears to have been re-uploaded: -** an active asset with the same filename exists and any of its timestamps is newer. Such -** trashed assets must not trigger a cascade, otherwise the fresh copy's companions would be -** dragged into the trash. +** hasActiveCopy reports whether a trashed asset still has an active copy of the same file: +** same OriginalFileName and same capture time. Trashing one duplicate must not cascade to +** the surviving copy or its companions, so such trashed assets never become triggers. +** File-system timestamps are deliberately not compared: recycled filenames from unrelated +** photos would look like replacements, and equal-timestamp duplicates would not. ** ** @param trashed - The trashed asset to check ** @param activeByFilename - Active assets indexed by OriginalFileName -** @return bool - True if a newer active copy exists +** @return bool - True if an active copy of the same photo exists **************************************************************************************************/ -func isReplacedByNewerCopy(trashed utils.TAsset, activeByFilename map[string][]utils.TAsset) bool { +func hasActiveCopy(trashed utils.TAsset, activeByFilename map[string][]utils.TAsset) bool { for _, active := range activeByFilename[trashed.OriginalFileName] { - if timestampAfter(active.FileCreatedAt, trashed.FileCreatedAt) || - timestampAfter(active.FileModifiedAt, trashed.FileModifiedAt) || - timestampAfter(active.UpdatedAt, trashed.UpdatedAt) { + if active.LocalDateTime == trashed.LocalDateTime { return true } } @@ -57,9 +36,9 @@ func isReplacedByNewerCopy(trashed utils.TAsset, activeByFilename map[string][]u /************************************************************************************************** ** findStackRelatedAssets finds the active assets that would stack with a trashed asset and -** should therefore follow it into the trash. Replaced trashed assets (newer active copy with -** the same filename) are excluded first, then a single StackBy run over the remaining trashed -** assets plus all active assets yields the groups to cascade. +** should therefore follow it into the trash. Trashed assets that still have an active copy +** (same filename and capture time) are excluded first, then a single StackBy run over the +** remaining trashed assets plus all active assets yields the groups to cascade. ** ** @param trashedAssets - Assets currently in the trash ** @param activeAssets - Assets not in the trash @@ -91,8 +70,8 @@ func findStackRelatedAssets( replacedCount := 0 triggers := make([]utils.TAsset, 0, len(trashedAssets)) for _, trashed := range trashedAssets { - if isReplacedByNewerCopy(trashed, activeByFilename) { - logger.Debugf(" 🔄 Skipping %s - appears to be replaced (newer version exists)", trashed.OriginalFileName) + if hasActiveCopy(trashed, activeByFilename) { + logger.Debugf(" 🔄 Skipping %s - an active copy with the same capture time exists", trashed.OriginalFileName) replacedCount++ continue } @@ -138,6 +117,10 @@ func findStackRelatedAssets( if asset.IsTrashed { continue } + if asset.IsArchived { + logger.Debugf(" ⏭️ Keeping archived asset %s (archived assets are never trashed)", asset.OriginalFileName) + continue + } assetsToTrash[asset.ID] = asset triggeredBy[asset.ID] = triggerName logger.Debugf(" ➡️ %s (active → will trash, stacks with %s)", asset.OriginalFileName, triggerName) diff --git a/cmd/fixtrash_analysis_test.go b/cmd/fixtrash_analysis_test.go index ff2b6de..39526a5 100644 --- a/cmd/fixtrash_analysis_test.go +++ b/cmd/fixtrash_analysis_test.go @@ -14,58 +14,17 @@ func quietFixTrashLogger() *logrus.Logger { return logger } -func TestTimestampAfter(t *testing.T) { - tests := []struct { - name string - a string - b string - want bool - }{ - {name: "plainly newer", a: "2024-01-01T10:00:00Z", b: "2024-01-01T09:00:00Z", want: true}, - {name: "plainly older", a: "2024-01-01T08:00:00Z", b: "2024-01-01T09:00:00Z", want: false}, - {name: "equal", a: "2024-01-01T10:00:00Z", b: "2024-01-01T10:00:00Z", want: false}, - {name: "fractional seconds", a: "2024-01-01T10:00:00.500Z", b: "2024-01-01T10:00:00.400Z", want: true}, - { - // Lexicographic comparison would wrongly say true here: "10:..." > "09:...", - // but +02:00 puts a at 08:00 UTC, before b. - name: "offset earlier despite larger string", - a: "2024-01-01T10:00:00+02:00", - b: "2024-01-01T09:00:00Z", - want: false, - }, - {name: "unparseable falls back to lexicographic", a: "b", b: "a", want: true}, - {name: "both empty", a: "", b: "", want: false}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := timestampAfter(tt.a, tt.b); got != tt.want { - t.Fatalf("timestampAfter(%q, %q) = %v, want %v", tt.a, tt.b, got, tt.want) - } - }) - } -} - -func TestIsReplacedByNewerCopy(t *testing.T) { - t0 := "2024-01-01T10:00:00Z" - older := "2023-12-01T10:00:00Z" - newer := "2024-01-02T10:00:00Z" +func TestHasActiveCopy(t *testing.T) { + t0 := "2024-01-01T10:00:00.000000000Z" + otherDay := "2025-05-05T10:00:00.000000000Z" trashed := utils.TAsset{ ID: "trashed", OriginalFileName: "IMG_1234.jpg", - FileCreatedAt: t0, - FileModifiedAt: t0, - UpdatedAt: t0, + LocalDateTime: t0, IsTrashed: true, } - activeAsset := func(fileName, created, modified, updated string) utils.TAsset { - return utils.TAsset{ - ID: "active", - OriginalFileName: fileName, - FileCreatedAt: created, - FileModifiedAt: modified, - UpdatedAt: updated, - } + activeAsset := func(fileName, localDateTime string) utils.TAsset { + return utils.TAsset{ID: "active", OriginalFileName: fileName, LocalDateTime: localDateTime} } tests := []struct { @@ -73,12 +32,9 @@ func TestIsReplacedByNewerCopy(t *testing.T) { active utils.TAsset want bool }{ - {name: "newer FileCreatedAt", active: activeAsset("IMG_1234.jpg", newer, t0, t0), want: true}, - {name: "newer FileModifiedAt only", active: activeAsset("IMG_1234.jpg", t0, newer, t0), want: true}, - {name: "newer UpdatedAt only", active: activeAsset("IMG_1234.jpg", t0, t0, newer), want: true}, - {name: "identical timestamps", active: activeAsset("IMG_1234.jpg", t0, t0, t0), want: false}, - {name: "all timestamps older", active: activeAsset("IMG_1234.jpg", older, older, older), want: false}, - {name: "newer but different filename", active: activeAsset("IMG_9999.jpg", newer, t0, t0), want: false}, + {name: "same name and capture time is a copy", active: activeAsset("IMG_1234.jpg", t0), want: true}, + {name: "recycled name from another photo is not a copy", active: activeAsset("IMG_1234.jpg", otherDay), want: false}, + {name: "different filename is not a copy", active: activeAsset("IMG_9999.jpg", t0), want: false}, } for _, tt := range tests { @@ -86,15 +42,15 @@ func TestIsReplacedByNewerCopy(t *testing.T) { byFilename := map[string][]utils.TAsset{ tt.active.OriginalFileName: {tt.active}, } - if got := isReplacedByNewerCopy(trashed, byFilename); got != tt.want { - t.Fatalf("isReplacedByNewerCopy() = %v, want %v", got, tt.want) + if got := hasActiveCopy(trashed, byFilename); got != tt.want { + t.Fatalf("hasActiveCopy() = %v, want %v", got, tt.want) } }) } t.Run("no active asset with that filename", func(t *testing.T) { - if isReplacedByNewerCopy(trashed, map[string][]utils.TAsset{}) { - t.Fatal("isReplacedByNewerCopy() = true with empty active map, want false") + if hasActiveCopy(trashed, map[string][]utils.TAsset{}) { + t.Fatal("hasActiveCopy() = true with empty active map, want false") } }) } @@ -112,10 +68,6 @@ func TestFindStackRelatedAssets(t *testing.T) { IsTrashed: trashed, } } - newerCopy := func(a utils.TAsset) utils.TAsset { - a.FileCreatedAt = "2024-01-02T10:00:00Z" - return a - } t0 := "2024-01-01T10:00:00.000000000Z" tests := []struct { @@ -134,17 +86,47 @@ func TestFindStackRelatedAssets(t *testing.T) { wantTriggerOf: map[string]string{"a1": "IMG_1234.jpg"}, }, { - name: "replaced trashed asset does not cascade", + // The HIGH regression from the holistic review: trashing one duplicate must + // not cascade the surviving copy or its RAW companion. + name: "trashed duplicate keeps the surviving copy and its companions", trashed: []utils.TAsset{ asset("t1", "IMG_1234.jpg", t0, true), }, active: []utils.TAsset{ - newerCopy(asset("a1", "IMG_1234.jpg", t0, false)), + asset("a1", "IMG_1234.jpg", t0, false), asset("a2", "IMG_1234.dng", t0, false), }, wantToTrash: []string{}, wantReplaced: 1, }, + { + // A recycled filename from an unrelated photo must not suppress the cascade. + name: "recycled filename does not suppress the cascade", + trashed: []utils.TAsset{ + asset("t1", "DSC_0001.jpg", t0, true), + }, + active: []utils.TAsset{ + asset("a1", "DSC_0001.dng", t0, false), + asset("b1", "DSC_0001.jpg", "2025-05-05T10:00:00.000000000Z", false), + }, + wantToTrash: []string{"a1"}, + wantTriggerOf: map[string]string{"a1": "DSC_0001.jpg"}, + }, + { + name: "archived group members are never cascaded", + trashed: []utils.TAsset{ + asset("t1", "IMG_1234.jpg", t0, true), + }, + active: []utils.TAsset{ + asset("a1", "IMG_1234.dng", t0, false), + func() utils.TAsset { + a := asset("a2", "IMG_1234.heic", t0, false) + a.IsArchived = true + return a + }(), + }, + wantToTrash: []string{"a1"}, + }, { name: "trashed asset without companions", trashed: []utils.TAsset{asset("t1", "IMG_1234.jpg", t0, true)}, From 077dd01cb15339360609dc9ab14df5f21ae0e6dd Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 09:31:14 +0200 Subject: [PATCH 4/7] fix(fix-trash): only flag criteria-matched RAWs as orphans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orphan pass treated any RAW absent from the stacker's groups as companionless. But assets the criteria cannot evaluate (no regex match, empty localDateTime under a time criterion) are skipped by StackBy before grouping, so their absence says nothing about companions — narrow criteria turned protected RAWs into false orphans. pkg/stacker gains MatchedAssetIDs, which reports the assets the criteria can key on the same path StackBy takes (legacy, expression, OR-groups). Ungrouped RAWs are now only orphan candidates when the criteria matched them; archived RAWs are never candidates and an archived developed companion now protects its group. --- cmd/fixtrash_orphans.go | 32 ++++++-- cmd/fixtrash_orphans_test.go | 49 ++++++++++++ pkg/stacker/stacker_matches.go | 113 ++++++++++++++++++++++++++++ pkg/stacker/stacker_matches_test.go | 85 +++++++++++++++++++++ 4 files changed, 273 insertions(+), 6 deletions(-) create mode 100644 pkg/stacker/stacker_matches.go create mode 100644 pkg/stacker/stacker_matches_test.go diff --git a/cmd/fixtrash_orphans.go b/cmd/fixtrash_orphans.go index f723c8f..0045e42 100644 --- a/cmd/fixtrash_orphans.go +++ b/cmd/fixtrash_orphans.go @@ -78,8 +78,10 @@ func parseOrphanExtensions(raw string, logger *logrus.Logger) map[string]bool { /************************************************************************************************** ** findOrphanedRAWs finds active RAW assets that have no developed companion: neither in ** their stacking group (computed with the user's criteria), nor in their existing Immich -** stack. RAW assets that end up in no group at all have no companion by definition, since -** the stacker drops single-asset groups. +** stack. RAW assets that the criteria match but that end up in no group are companionless +** singletons (the stacker drops single-asset groups); RAW assets the criteria cannot +** evaluate are never flagged, and neither are archived RAWs — archived assets protect +** their group but are never trashed. ** ** @param activeAssets - Assets not in the trash ** @param criteria - Stacking criteria JSON (empty = defaults), same as pass 1 @@ -123,7 +125,10 @@ func findOrphanedRAWs( } /********************************************************************************************** - ** Candidates: every RAW in an all-RAW group, plus every RAW in no group at all. + ** Candidates: every allowed RAW in an all-RAW group, plus every allowed RAW that the + ** criteria matched but that grouped with nothing (the stacker drops singletons). A RAW + ** the criteria cannot evaluate is left alone: its absence from the groups says nothing + ** about its companions. **********************************************************************************************/ candidates := make(map[string]utils.TAsset) grouped := make(map[string]bool) @@ -139,14 +144,29 @@ func findOrphanedRAWs( continue } for _, asset := range stack { - if hasExtension(candidateExtensions, asset.OriginalFileName) { + if !asset.IsArchived && hasExtension(candidateExtensions, asset.OriginalFileName) { candidates[asset.ID] = asset } } } + + ungroupedRAWs := make([]utils.TAsset, 0) for _, asset := range activeAssets { - if !grouped[asset.ID] && hasExtension(candidateExtensions, asset.OriginalFileName) { - candidates[asset.ID] = asset + if !grouped[asset.ID] && !asset.IsArchived && hasExtension(candidateExtensions, asset.OriginalFileName) { + ungroupedRAWs = append(ungroupedRAWs, asset) + } + } + if len(ungroupedRAWs) > 0 { + matchedIDs, err := stacker.MatchedAssetIDs(ungroupedRAWs, criteria) + if err != nil { + return nil, 0, err + } + for _, asset := range ungroupedRAWs { + if matchedIDs[asset.ID] { + candidates[asset.ID] = asset + } else { + logger.Debugf(" ⏭️ Skipping RAW %s - the criteria cannot evaluate it", asset.OriginalFileName) + } } } diff --git a/cmd/fixtrash_orphans_test.go b/cmd/fixtrash_orphans_test.go index 3001130..afb120c 100644 --- a/cmd/fixtrash_orphans_test.go +++ b/cmd/fixtrash_orphans_test.go @@ -41,6 +41,11 @@ func TestFindOrphanedRAWs(t *testing.T) { a.Stack = &utils.TStack{ID: stackID} return a } + archivedAsset := func(id, fileName string) utils.TAsset { + a := asset(id, fileName) + a.IsArchived = true + return a + } leicaCriteria := `{"mode":"advanced","groups":[{"operator":"AND","criteria":[{"key":"originalFileName","regex":{"key":"^(?:L|DO0)(\\d+)","index":1}},{"key":"localDateTime","delta":{"milliseconds":1000}}]}]}` tests := []struct { @@ -167,6 +172,50 @@ func TestFindOrphanedRAWs(t *testing.T) { orphanExtensions: "jpg,bogus", wantOrphans: []string{}, }, + { + // Narrow criteria must not turn unevaluated RAWs into orphans: this pair does + // not match the BURST regex, so nothing can be concluded about companions. + name: "criteria that cannot evaluate a RAW never flag it", + active: []utils.TAsset{ + asset("d1", "IMG_1234.dng"), + asset("j1", "IMG_1234.jpg"), + }, + criteria: `[{"key":"originalFileName","regex":{"key":"BURST(\\d+)","index":1}}]`, + wantOrphans: []string{}, + }, + { + name: "criteria-matched lone RAW is still flagged under narrow criteria", + active: []utils.TAsset{ + asset("d1", "BURST001.dng"), + asset("j1", "IMG_1234.jpg"), + }, + criteria: `[{"key":"originalFileName","regex":{"key":"BURST(\\d+)","index":1}}]`, + wantOrphans: []string{"d1"}, + }, + { + // An empty LocalDateTime makes the AND expression unevaluable for the pair: + // the DNG must be protected, not flagged. + name: "empty capture time under time criteria never flags the RAW", + active: []utils.TAsset{ + {ID: "d1", OriginalFileName: "L1001336.dng"}, + {ID: "j1", OriginalFileName: "DO01001336.jpg"}, + }, + criteria: leicaCriteria, + wantOrphans: []string{}, + }, + { + name: "archived developed companion protects the RAW", + active: []utils.TAsset{ + asset("d1", "IMG_1234.dng"), + archivedAsset("j1", "IMG_1234.jpg"), + }, + wantOrphans: []string{}, + }, + { + name: "archived RAW is never flagged", + active: []utils.TAsset{archivedAsset("d1", "L1001336.dng")}, + wantOrphans: []string{}, + }, } for _, tt := range tests { diff --git a/pkg/stacker/stacker_matches.go b/pkg/stacker/stacker_matches.go new file mode 100644 index 0000000..91d1591 --- /dev/null +++ b/pkg/stacker/stacker_matches.go @@ -0,0 +1,113 @@ +/************************************************************************************************** +** Criteria-match detection: which assets can the configured criteria actually evaluate and +** key. Consumers that reason about StackBy's output need this to distinguish "grouped with +** nothing" (a real singleton) from "never evaluated" (the criteria skipped the asset — +** nothing can be concluded about its companions). +**************************************************************************************************/ + +package stacker + +import ( + "fmt" + "strings" + + "github.com/majorfi/immich-stack/pkg/utils" +) + +/************************************************************************************************** +** MatchedAssetIDs returns the IDs of the assets the criteria can evaluate: those that +** produce a non-empty grouping key on the same path StackBy would take. Assets absent from +** the result are skipped by StackBy regardless of companions. +** +** @param assets - Assets to check +** @param criteria - Stacking criteria JSON (empty = defaults) +** @return map[string]bool - IDs of the assets the criteria match +** @return error - Any error from criteria parsing or evaluation +**************************************************************************************************/ +func MatchedAssetIDs(assets []utils.TAsset, criteria string) (map[string]bool, error) { + matched := make(map[string]bool, len(assets)) + if len(assets) == 0 { + return matched, nil + } + + config, err := getCriteriaConfig(criteria) + if err != nil { + return nil, fmt.Errorf("failed to get criteria config: %w", err) + } + + if config.Mode != "advanced" { + return matchedByLegacyCriteria(assets, config.Legacy) + } + + // Mirror StackBy's routing: AND-only groups are converted to an expression. + if config.Expression == nil && len(config.Groups) > 0 && canConvertGroupsToExpression(config.Groups) { + config.Expression = convertGroupsToExpression(config.Groups) + } + if config.Expression != nil { + return matchedByExpression(assets, config.Expression) + } + if len(config.Groups) > 0 { + return matchedByGroups(assets, config.Groups) + } + return nil, fmt.Errorf("advanced mode specified but no expression or groups provided") +} + +func matchedByLegacyCriteria(assets []utils.TAsset, criteria []utils.TCriteria) (map[string]bool, error) { + if err := PrecompileRegexes(criteria); err != nil { + return nil, fmt.Errorf("failed to precompile legacy criteria regexes: %w", err) + } + matched := make(map[string]bool, len(assets)) + var keyBuilder strings.Builder + for _, asset := range assets { + values, _, err := applyCriteriaWithPromote(asset, criteria) + if err != nil { + return nil, fmt.Errorf("failed to apply criteria to asset %s: %w", asset.OriginalFileName, err) + } + if buildGroupKey(values, &keyBuilder) != "" { + matched[asset.ID] = true + } + } + return matched, nil +} + +func matchedByExpression(assets []utils.TAsset, expression *utils.TCriteriaExpression) (map[string]bool, error) { + if err := PrecompileRegexes(expression); err != nil { + return nil, fmt.Errorf("failed to precompile expression regexes: %w", err) + } + exprCriteria := flattenCriteriaFromExpression(expression) + matched := make(map[string]bool, len(assets)) + for _, asset := range assets { + matches, err := EvaluateExpression(expression, asset) + if err != nil { + return nil, fmt.Errorf("failed to evaluate expression for asset %s: %w", asset.OriginalFileName, err) + } + if !matches { + continue + } + key, err := buildExpressionGroupingKey(asset, expression, exprCriteria) + if err != nil { + return nil, fmt.Errorf("failed to build grouping key for asset %s: %w", asset.OriginalFileName, err) + } + if key != "" { + matched[asset.ID] = true + } + } + return matched, nil +} + +func matchedByGroups(assets []utils.TAsset, groups []utils.TCriteriaGroup) (map[string]bool, error) { + if err := PrecompileRegexes(groups); err != nil { + return nil, fmt.Errorf("failed to precompile group regexes: %w", err) + } + matched := make(map[string]bool, len(assets)) + for _, asset := range assets { + groupKeys, err := applyAdvancedCriteria(asset, groups) + if err != nil { + return nil, fmt.Errorf("failed to apply advanced criteria to asset %s: %w", asset.OriginalFileName, err) + } + if len(groupKeys) > 0 { + matched[asset.ID] = true + } + } + return matched, nil +} diff --git a/pkg/stacker/stacker_matches_test.go b/pkg/stacker/stacker_matches_test.go new file mode 100644 index 0000000..688b0b7 --- /dev/null +++ b/pkg/stacker/stacker_matches_test.go @@ -0,0 +1,85 @@ +package stacker + +import ( + "testing" + + "github.com/majorfi/immich-stack/pkg/utils" +) + +func TestMatchedAssetIDs(t *testing.T) { + t0 := "2024-01-01T10:00:00.000000000Z" + asset := func(id, fileName, localDateTime string) utils.TAsset { + return utils.TAsset{ID: id, OriginalFileName: fileName, LocalDateTime: localDateTime} + } + + tests := []struct { + name string + assets []utils.TAsset + criteria string + wantMatched []string + }{ + { + name: "default criteria match named assets", + assets: []utils.TAsset{ + asset("a1", "IMG_1234.jpg", t0), + asset("a2", "IMG_5678.dng", t0), + }, + criteria: "", + wantMatched: []string{"a1", "a2"}, + }, + { + name: "legacy regex criteria only match the pattern", + assets: []utils.TAsset{ + asset("a1", "BURST001.jpg", t0), + asset("a2", "IMG_1234.jpg", t0), + }, + criteria: `[{"key":"originalFileName","regex":{"key":"BURST(\\d+)","index":1}}]`, + wantMatched: []string{"a1"}, + }, + { + name: "AND group expression requires every leaf", + assets: []utils.TAsset{ + asset("a1", "L1001336.dng", t0), + asset("a2", "L1001337.dng", ""), + asset("a3", "HOLIDAY.jpg", t0), + }, + criteria: `{"mode":"advanced","groups":[{"operator":"AND","criteria":[{"key":"originalFileName","regex":{"key":"^L(\\d+)","index":1}},{"key":"localDateTime","delta":{"milliseconds":1000}}]}]}`, + wantMatched: []string{"a1"}, + }, + { + name: "OR groups match through the connectivity path", + assets: []utils.TAsset{ + asset("a1", "BURST001.jpg", t0), + asset("a2", "IMG_1234.jpg", t0), + }, + criteria: `{"mode":"advanced","groups":[{"operator":"OR","criteria":[{"key":"originalFileName","regex":{"key":"BURST(\\d+)","index":1}}]}]}`, + wantMatched: []string{"a1"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + matched, err := MatchedAssetIDs(tt.assets, tt.criteria) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(matched) != len(tt.wantMatched) { + t.Fatalf("matched %d assets (%v), want %d (%v)", len(matched), matched, len(tt.wantMatched), tt.wantMatched) + } + for _, id := range tt.wantMatched { + if !matched[id] { + t.Fatalf("asset %s not matched; got %v", id, matched) + } + } + }) + } +} + +func TestMatchedAssetIDsInvalidCriteria(t *testing.T) { + _, err := MatchedAssetIDs( + []utils.TAsset{{ID: "a1", OriginalFileName: "a.jpg"}}, + `{"mode":"advanced"}`) + if err == nil { + t.Fatal("expected an error for advanced criteria without groups or expression") + } +} From 5a85301e68fa11fef374cbee0d8a6d905bf94bac Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 09:31:31 +0200 Subject: [PATCH 5/7] docs(fix-trash): document the safety fixes - Pass 1 describes the active-copy skip (same filename and capture time) instead of the removed timestamp heuristic - Archived assets: always fetched for protection, never trashed - Scheduled example no longer recommends --log-level warn: the summary of what was trashed is info-level and is the only record an unattended run leaves --- docs/commands/fix-trash.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/commands/fix-trash.md b/docs/commands/fix-trash.md index 927f850..0d1e825 100644 --- a/docs/commands/fix-trash.md +++ b/docs/commands/fix-trash.md @@ -24,7 +24,7 @@ The command runs two passes: ### Pass 1: stack cascade 1. Fetches your trashed assets and your active assets. Partner-owned assets are dropped from both lists. -1. Skips trashed assets that appear to have been re-uploaded: if an active asset with the same filename has a newer created/modified/updated timestamp, cascading from the old copy would drag the new copy's companions into the trash. +1. Skips trashed assets that still have an active copy (same filename and same capture time): when you trash one duplicate of a photo, the surviving copy and its companions must not be cascaded into the trash. 1. Runs the stacking algorithm once over the remaining trashed assets plus all active assets, using the same criteria as the main command (`--criteria`, `--parent-filename-promote`, `--parent-ext-promote`). 1. Every active asset that lands in a group containing a trashed asset is marked for trash. @@ -39,6 +39,7 @@ This pass only runs with `--trash-orphaned-raws` (or `TRASH_ORPHANED_RAWS=true`) ### Safety - Assets are moved to trash (`force=false`), not deleted permanently. They can be restored from Immich's trash until Immich empties it. This tool has no undo command. +- Archived assets are always fetched so they can protect their group (an archived JPG keeps its RAW safe), but fix-trash never moves an archived asset to trash. - If more than 10% of your active assets are about to be trashed, a warning is logged before the summary. The command does not stop — it is designed to run unattended. - Deletion requests are sent in batches of 1000 assets. @@ -119,7 +120,6 @@ The command uses all global flags, particularly: - `--parent-filename-promote` - Filename patterns for stacking - `--trash-orphaned-raws` - Enable the orphaned RAW cleanup pass (off by default) - `--raw-orphan-extensions` - Restrict the RAW pass to specific extensions, e.g. `dng,nef` (default: all RAW formats) -- `--with-archived` - Also look for archived assets in the trash scan - `--log-level` - Set to `debug` for detailed matching information ## Use Cases @@ -166,9 +166,11 @@ Or schedule the standalone command: ```bash # Run weekly to maintain consistency -0 2 * * 0 immich-stack fix-trash --api-key your_key --log-level warn +0 2 * * 0 immich-stack fix-trash --api-key your_key ``` +Keep the log level at `info` (the default) for scheduled runs: the summary of what was moved to trash is logged at that level, and it is your only record of what an unattended run did. + ## Important Notes 1. **Uses Stacking Criteria**: Both passes use the same criteria as the main stacking command, so the cascade and the orphan matching follow what the stacker would group. From 4bedd833212fa96ea3b5322d16e663d1f8e4adfc Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 10:10:01 +0200 Subject: [PATCH 6/7] fix(fix-trash): harden copy detection and orphan matching Adversarial re-review of the previous fixes broke them in two spots, both reproduced by execution: - MatchedAssetIDs mirrored StackBy's keying but not the time merge: mergeTimeBasedGroups drops keyed assets whose time field does not parse, so under the default criteria a DNG with an empty localDateTime was matched, dropped from every group, and flagged as a false orphan despite its JPG companion. MatchedAssetIDs now also requires every time-based criterion to extract a parseable value. - hasActiveCopy compared capture times by string equality: sub-second precision drift between two uploads of the same photo (stripped SubSecTimeOriginal) defeated the copy detection while both copies still shared a grouping bucket, cascading the survivor. Capture times now compare with a one-second tolerance. Also refresh the docs: pass 2 describes the criteria-match and archived restrictions, the example output uses the current log line, and the archived-protection note mentions servers that ignore the archived option on /search/metadata. --- cmd/fixtrash_analysis.go | 29 ++++++++++++++++++++++++++- cmd/fixtrash_analysis_test.go | 16 +++++++++++++++ cmd/fixtrash_orphans_test.go | 11 ++++++++++ docs/commands/fix-trash.md | 6 +++--- pkg/stacker/stacker_matches.go | 31 ++++++++++++++++++++++++++--- pkg/stacker/stacker_matches_test.go | 11 ++++++++++ 6 files changed, 97 insertions(+), 7 deletions(-) diff --git a/cmd/fixtrash_analysis.go b/cmd/fixtrash_analysis.go index 404ec78..540c9cf 100644 --- a/cmd/fixtrash_analysis.go +++ b/cmd/fixtrash_analysis.go @@ -8,6 +8,7 @@ package main import ( "io" + "time" "github.com/majorfi/immich-stack/pkg/stacker" "github.com/majorfi/immich-stack/pkg/utils" @@ -27,13 +28,39 @@ import ( **************************************************************************************************/ func hasActiveCopy(trashed utils.TAsset, activeByFilename map[string][]utils.TAsset) bool { for _, active := range activeByFilename[trashed.OriginalFileName] { - if active.LocalDateTime == trashed.LocalDateTime { + if sameCaptureTime(active.LocalDateTime, trashed.LocalDateTime) { return true } } return false } +/************************************************************************************************** +** sameCaptureTime compares two capture times with a one-second tolerance: two uploads of +** the same photo can drift in sub-second precision (a stripped SubSecTimeOriginal), and +** such near-equal copies share a grouping bucket, so missing them would cascade the +** surviving copy. Unparseable values only match on exact string equality. +** +** @param a - First LocalDateTime value +** @param b - Second LocalDateTime value +** @return bool - True if both refer to the same capture time +**************************************************************************************************/ +func sameCaptureTime(a, b string) bool { + if a == b { + return true + } + timeA, errA := time.Parse(time.RFC3339Nano, a) + timeB, errB := time.Parse(time.RFC3339Nano, b) + if errA != nil || errB != nil { + return false + } + diff := timeA.Sub(timeB) + if diff < 0 { + diff = -diff + } + return diff <= time.Second +} + /************************************************************************************************** ** findStackRelatedAssets finds the active assets that would stack with a trashed asset and ** should therefore follow it into the trash. Trashed assets that still have an active copy diff --git a/cmd/fixtrash_analysis_test.go b/cmd/fixtrash_analysis_test.go index 39526a5..d349104 100644 --- a/cmd/fixtrash_analysis_test.go +++ b/cmd/fixtrash_analysis_test.go @@ -33,6 +33,8 @@ func TestHasActiveCopy(t *testing.T) { want bool }{ {name: "same name and capture time is a copy", active: activeAsset("IMG_1234.jpg", t0), want: true}, + {name: "sub-second capture drift is still a copy", active: activeAsset("IMG_1234.jpg", "2024-01-01T10:00:00.123000000Z"), want: true}, + {name: "two seconds apart is not a copy", active: activeAsset("IMG_1234.jpg", "2024-01-01T10:00:02.000000000Z"), want: false}, {name: "recycled name from another photo is not a copy", active: activeAsset("IMG_1234.jpg", otherDay), want: false}, {name: "different filename is not a copy", active: activeAsset("IMG_9999.jpg", t0), want: false}, } @@ -112,6 +114,20 @@ func TestFindStackRelatedAssets(t *testing.T) { wantToTrash: []string{"a1"}, wantTriggerOf: map[string]string{"a1": "DSC_0001.jpg"}, }, + { + // Sub-second capture drift between two uploads of the same photo: the copy + // detection must still fire, otherwise the survivor gets cascaded. + name: "trashed duplicate with sub-second drift keeps the survivor", + trashed: []utils.TAsset{ + asset("t1", "IMG_1234.jpg", "2024-01-01T10:00:00.123000000Z", true), + }, + active: []utils.TAsset{ + asset("a1", "IMG_1234.jpg", t0, false), + asset("a2", "IMG_1234.dng", t0, false), + }, + wantToTrash: []string{}, + wantReplaced: 1, + }, { name: "archived group members are never cascaded", trashed: []utils.TAsset{ diff --git a/cmd/fixtrash_orphans_test.go b/cmd/fixtrash_orphans_test.go index afb120c..47cc618 100644 --- a/cmd/fixtrash_orphans_test.go +++ b/cmd/fixtrash_orphans_test.go @@ -203,6 +203,17 @@ func TestFindOrphanedRAWs(t *testing.T) { criteria: leicaCriteria, wantOrphans: []string{}, }, + { + // mergeTimeBasedGroups drops timeless assets from merged buckets, so this DNG + // lands in no group despite its companion — the time-evaluability gate in + // MatchedAssetIDs must keep it out of the candidates. + name: "empty capture time under default criteria never flags the RAW", + active: []utils.TAsset{ + {ID: "d1", OriginalFileName: "IMG_1234.dng"}, + asset("j1", "IMG_1234.jpg"), + }, + wantOrphans: []string{}, + }, { name: "archived developed companion protects the RAW", active: []utils.TAsset{ diff --git a/docs/commands/fix-trash.md b/docs/commands/fix-trash.md index 0d1e825..156e1be 100644 --- a/docs/commands/fix-trash.md +++ b/docs/commands/fix-trash.md @@ -33,13 +33,13 @@ The command runs two passes: This pass only runs with `--trash-orphaned-raws` (or `TRASH_ORPHANED_RAWS=true`). 1. Groups your active assets with the same stacking criteria as pass 1, so filename matching follows your `--criteria` configuration. Cameras that pair a RAW and a JPG under different names (for example Leica's `L1001336.dng` + `DO01001336.jpg`) need a regex criterion that maps both to the same key, such as `{"key":"originalFileName","regex":{"key":"^(?:L|DO0)(\\d+)","index":1}}`. -1. A RAW file whose group contains no developed file — or that groups with nothing at all — is marked for trash, unless it already sits in an Immich stack that contains a developed file. +1. A RAW file whose group contains no developed file — or that the criteria matched but that groups with nothing at all — is marked for trash, unless it already sits in an Immich stack that contains a developed file. A RAW the criteria cannot fully evaluate (no filename match, missing capture time) is never flagged, and neither is an archived RAW. 1. `--raw-orphan-extensions` (or `RAW_ORPHAN_EXTENSIONS`) restricts which RAW extensions may be flagged, e.g. `dng` to clean up orphaned DNGs while never touching NEF/ARW/... files from a RAW-only workflow. It only restricts the candidates: another RAW file never counts as a developed companion, whatever the restriction. Unknown extensions are ignored with a warning. ### Safety - Assets are moved to trash (`force=false`), not deleted permanently. They can be restored from Immich's trash until Immich empties it. This tool has no undo command. -- Archived assets are always fetched so they can protect their group (an archived JPG keeps its RAW safe), but fix-trash never moves an archived asset to trash. +- Archived assets are always fetched so they can protect their group (an archived JPG keeps its RAW safe), but fix-trash never moves an archived asset to trash. Note: some Immich versions ignore the archived option on `/search/metadata`; on those servers archived companions stay invisible and cannot protect their RAW. - If more than 10% of your active assets are about to be trashed, a warning is logged before the summary. The command does not stop — it is designed to run unattended. - Deletion requests are sent in batches of 1000 assets. @@ -97,7 +97,7 @@ immich-stack fix-trash --api-key your_key --log-level debug ``` 🗑️ Found 5 trashed assets 🔍 Analyzing 5 trashed assets against 1000 active assets... -🔄 Skipped 2 trashed assets that appear to have been replaced +🔄 Skipped 2 trashed assets that still have an active copy 🔍 Looking for orphaned RAW files... 📸 Found 2 orphaned RAW files without a developed companion ✅ Kept 1 RAW files already stacked with a developed file diff --git a/pkg/stacker/stacker_matches.go b/pkg/stacker/stacker_matches.go index 91d1591..6bf1017 100644 --- a/pkg/stacker/stacker_matches.go +++ b/pkg/stacker/stacker_matches.go @@ -52,6 +52,30 @@ func MatchedAssetIDs(assets []utils.TAsset, criteria string) (map[string]bool, e return nil, fmt.Errorf("advanced mode specified but no expression or groups provided") } +/************************************************************************************************** +** timeEvaluable reports whether every time-based criterion of the configuration extracts a +** non-empty, parseable value from the asset. mergeTimeBasedGroups drops assets whose time +** field does not parse while rebuilding merged buckets, so a keyed asset without an +** evaluable time can still end up in no group despite having companions — such assets must +** not count as matched. +**************************************************************************************************/ +func timeEvaluable(asset utils.TAsset, criteria []utils.TCriteria) bool { + for _, c := range criteria { + if !isTimeCriteria(c.Key) || c.Delta == nil || c.Delta.Milliseconds <= 0 { + continue + } + extractor, ok := extractors[c.Key] + if !ok { + continue + } + value, err := extractor(asset, c) + if err != nil || value == "" { + return false + } + } + return true +} + func matchedByLegacyCriteria(assets []utils.TAsset, criteria []utils.TCriteria) (map[string]bool, error) { if err := PrecompileRegexes(criteria); err != nil { return nil, fmt.Errorf("failed to precompile legacy criteria regexes: %w", err) @@ -63,7 +87,7 @@ func matchedByLegacyCriteria(assets []utils.TAsset, criteria []utils.TCriteria) if err != nil { return nil, fmt.Errorf("failed to apply criteria to asset %s: %w", asset.OriginalFileName, err) } - if buildGroupKey(values, &keyBuilder) != "" { + if buildGroupKey(values, &keyBuilder) != "" && timeEvaluable(asset, criteria) { matched[asset.ID] = true } } @@ -88,7 +112,7 @@ func matchedByExpression(assets []utils.TAsset, expression *utils.TCriteriaExpre if err != nil { return nil, fmt.Errorf("failed to build grouping key for asset %s: %w", asset.OriginalFileName, err) } - if key != "" { + if key != "" && timeEvaluable(asset, exprCriteria) { matched[asset.ID] = true } } @@ -99,13 +123,14 @@ func matchedByGroups(assets []utils.TAsset, groups []utils.TCriteriaGroup) (map[ if err := PrecompileRegexes(groups); err != nil { return nil, fmt.Errorf("failed to precompile group regexes: %w", err) } + groupCriteria := flattenCriteriaFromGroups(groups) matched := make(map[string]bool, len(assets)) for _, asset := range assets { groupKeys, err := applyAdvancedCriteria(asset, groups) if err != nil { return nil, fmt.Errorf("failed to apply advanced criteria to asset %s: %w", asset.OriginalFileName, err) } - if len(groupKeys) > 0 { + if len(groupKeys) > 0 && timeEvaluable(asset, groupCriteria) { matched[asset.ID] = true } } diff --git a/pkg/stacker/stacker_matches_test.go b/pkg/stacker/stacker_matches_test.go index 688b0b7..5494e32 100644 --- a/pkg/stacker/stacker_matches_test.go +++ b/pkg/stacker/stacker_matches_test.go @@ -27,6 +27,17 @@ func TestMatchedAssetIDs(t *testing.T) { criteria: "", wantMatched: []string{"a1", "a2"}, }, + { + // mergeTimeBasedGroups can drop keyed assets whose time field does not parse, + // so they must not count as matched even though they get a filename-only key. + name: "empty time under a time criterion is not matched", + assets: []utils.TAsset{ + asset("a1", "IMG_1234.jpg", ""), + asset("a2", "IMG_5678.dng", t0), + }, + criteria: "", + wantMatched: []string{"a2"}, + }, { name: "legacy regex criteria only match the pattern", assets: []utils.TAsset{ From e03d433817a6323735f2d83d3c6eb71ef78e5af2 Mon Sep 17 00:00:00 2001 From: Major Date: Mon, 13 Jul 2026 10:39:55 +0200 Subject: [PATCH 7/7] fix(stacker): warn about inert filter flags in chained fix-trash runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review on #72: with --fix-trash-after-stacking, the run never goes through runFixTrash, so the existing filters-have-no-effect warning was bypassed — a user chaining fix-trash with FILTER_ALBUM_IDS could assume the pass is scoped to the album while it processes the whole library. The warning now also fires from runStacker when the chain is enabled. Also mention the CLI flag next to the env var in the README. --- README.md | 2 +- cmd/stacker.go | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b0a18b9..0ce5aac 100644 --- a/README.md +++ b/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, ...). Set `FIX_TRASH_AFTER_STACKING=true` to chain it after every stacking run, including in cron mode. +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 diff --git a/cmd/stacker.go b/cmd/stacker.go index 562ab80..10c944a 100644 --- a/cmd/stacker.go +++ b/cmd/stacker.go @@ -155,11 +155,14 @@ func runStacker(cmd *cobra.Command, args []string) { /********************************************************************************************** ** The chained fix-trash run never goes through runFixTrash, so its inert-setting - ** diagnostic must also live here. + ** diagnostics must also live here. **********************************************************************************************/ if fixTrashAfterStacking && !trashOrphanedRAWs && rawOrphanExtensions != "" { logger.Warnf("--raw-orphan-extensions (RAW_ORPHAN_EXTENSIONS) has no effect without --trash-orphaned-raws") } + if fixTrashAfterStacking && (len(filterAlbumIDs) > 0 || filterTakenAfter != "" || filterTakenBefore != "") { + logger.Warnf("Filter flags (--filter-album-ids, --filter-taken-after, --filter-taken-before) apply to stacking only; the chained fix-trash run processes the whole library") + } /********************************************************************************************** ** Support multiple API keys (comma-separated).