[ptr727]

Summary

ProcessFile.Refresh(string) seeds a new SidecarFile with the in-memory state plus FileReNamed. When a sidecar already exists under the new name, the sidecar load replaced that state, so FileReNamed and any flag the step had just set were lost.

SidecarFile.Open() now keeps the state it was seeded with on the first open and ORs it back in after the sidecar load settles the state, including the hash-mismatch reset to FileModified. One rule covers both rename paths (MakeExtensionLowercase and RemuxByExtension), with no per-step branch and no sidecar delete, per the maintainer decision on the issue.

Tests

PlexCleanerTests/SidecarRenameStateTests.cs covers both shapes with constructed files:

• name.MKV over a matching name.PlexCleaner: seeded flags and the stored flag are both kept.
• A modified media file over a stale name.PlexCleaner: the stale stored flag is cleared, the seeded flags and FileModified are kept.

Both fail without the change. The full suite passes.

Deferred

HISTORY.md lines and the ARCHITECTURE.md Suspect Branches entry removal are deferred to one final docs pull request by the maintainer's choice, so this pull request does not edit them.

Closes on promotion: #1082

🤖 Generated with Claude Code

[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 34 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews 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: ASSERTIVE
• Plan: Advanced
• Run ID: 826f5d4b-a4ae-459f-bda1-367e188d689d

Commits

Reviewing files that changed from the base of the PR and between 77d9be9 and 34b3898.

Files selected for processing: 2

• PlexCleaner/SidecarFile.cs
• PlexCleanerTests/SidecarRenameStateTests.cs

• 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

[ptr727]

A recorded local strict-review pass covers head dbddf9da9ce1547f4ac6f819f14cbdc620ac8ed2, the content this pull request carries at that commit against develop, and it recorded 0 findings.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Preserve seeded state when a renamed file has a sidecar

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Preserve rename and processing flags when a renamed file loads an existing sidecar.
• Retain stored flags for matching media while clearing stale flags for modified media.
• Add regression tests for both matching and stale sidecars.

Diagram

graph TD
  A["Rename Step"] --> B["ProcessFile Refresh"] --> C["Sidecar Open"] --> D["Read Sidecar"] --> E{"Media current?"}
  E -->|yes| G["Merge Seeded Flags"] --> H["Persist State"]
  E -->|no| F["Reset Modified"] --> G

Loading

High-Level Assessment

Merging seeded flags in SidecarFile.Open is the appropriate shared fix for both rename paths. Per-step handling would duplicate the rule, while deleting an existing sidecar would discard valid stored data.

Files changed (2) +111 / -0

Bug fix (1) +4 / -0

SidecarFile.csRestore caller-seeded flags after loading a sidecar +4/-0

Restore caller-seeded flags after loading a sidecar

• On the first open of an existing sidecar, captures the caller's state and merges it back after Read completes. This preserves seeded flags whether the stored state remains valid or a media mismatch resets it to FileModified.

PlexCleaner/SidecarFile.cs

Tests (1) +107 / -0

SidecarRenameStateTests.csCover seeded state with matching and stale sidecars +107/-0

Cover seeded state with matching and stale sidecars

• Adds tests asserting both in-memory and persisted flags when renamed media encounters a matching sidecar. A second test verifies that stale stored flags are cleared while seeded flags and FileModified survive.

PlexCleanerTests/SidecarRenameStateTests.cs

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Changed files can bypass verification 🐞 Bug ≡ Correctness

Description

SidecarFile.Open restores every seeded flag after Read clears state for a media mismatch,
including a seeded Verified flag. When a caller opens a changed file with that flag already set,
the sidecar persists Verified alongside FileModified, and conditional verification skips
checking the changed media.

Code

PlexCleaner/SidecarFile.cs[233]

+                State |= seeded;

Evidence

Read resets state to FileModified on a media mismatch, but the new OR restores the saved flags
before Open writes state through Update and SetJsonInfo. ProcessFile.Verify then returns
early when Verified is present. The new tests construct SidecarFile with a seeded state but do
not exercise a seeded Verified flag on the stale-sidecar path.

PlexCleaner/SidecarFile.cs[132-164]
PlexCleaner/SidecarFile.cs[223-250]
PlexCleaner/SidecarFile.cs[493-516]
PlexCleaner/ProcessFile.cs[1709-1719]
PlexCleanerTests/SidecarRenameStateTests.cs[98-105]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Open` restores seeded verification and scan flags even when `Read` invalidates them because the media or tools changed.
## Fix Focus Areas
- PlexCleaner/SidecarFile.cs[226-237]
- PlexCleaner/SidecarFile.cs[132-164]
- PlexCleanerTests/SidecarRenameStateTests.cs[69-105]
## Recommended Fix
Preserve seeded rename and operation flags, but exclude seeded flags that `Read` invalidates for a media or tool mismatch before merging state. Add tests with seeded `Verified` and scan flags to confirm invalidation still takes effect.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🟡 Changes recommended

The new test has two critical Windows-compatibility issues: it runs shell scripts without the required platform skip and performs a case-only rename without the temporary path used by production code.

2 open issues

• Use a temporary path for case-only rename · New
• Skip shell-script test on Windows · New

What changed in this PR

This pull request preserves state seeded during file renames when an existing sidecar is loaded, while retaining FileModified for stale media. It also adds regression coverage for matching and stale sidecars.

Changes:

• Merge caller-seeded state back after sidecar loading and validation.
• Preserve seeded flags when a sidecar hash is stale.
• Add tests for matching and stale sidecar scenarios.

File	Summary
PlexCleanerTests/​SidecarRenameStateTests.cs	Adds regression tests. Critical [P1] findings: skip the shell-script case on Windows (2 votes), and use the production temporary-name sequence for the case-only rename (3 votes).
PlexCleaner/​SidecarFile.cs	Preserves caller-seeded state across existing-sidecar loads and media validation.

_{Review effort: Lite}

[ptr727]

A recorded local strict-review pass covers head 34b38981e6a78233ce7f6d00b4725e63cace2027, the content this pull request carries at that commit against develop, and it recorded 0 findings.

[ptr727]

Answering Qodo's finding "1. Changed files can bypass verification" (SidecarFile.cs:233), declined with evidence.

The merge restores the state the caller seeded, and that is the same state the no-sidecar path already keeps. When no sidecar exists under the new name, Open() calls Create(), which does not modify State ("Do not modify the state, it is managed external to the create path"), so a seeded Verified is written as is. Clearing seeded flags only when a sidecar happens to exist would make the two paths disagree, which is the per-step or per-case branching the maintainer ruled out on the issue ("one rule for both rename paths ... State |= seeded ... no per-step branch"). The seeded state also comes from the in-memory state, which every ProcessFile clear persists via SaveState() or Refresh(true) before a rename, so a stale flag cannot enter through the seed. The test file covers both shapes (matching sidecar, stale sidecar).

[ptr727]

1. Changed files can bypass verification. SidecarFile.Open restores every seeded flag after Read clears state for a media mismatch, including a seeded Verified flag.

Declined: the seeded flags are the state the caller carries, and Create() keeps that same seeded state (including Verified) when no sidecar exists under the new name. The merge makes the sidecar-exists path agree with it, as the maintainer decided on the issue (one rule, State |= seeded, no per-step branch). Details are in the earlier conversation comment. Marking this finding dismissed.