[ptr727]

Summary

Fixes #1095.

A repair tool run that a signal kills never runs to a result. The out-of-memory killer during a long re-encode is one example. Such a run used to read as a genuine failure. VerifyAndRepair() then set VerifyFailed and RepairFailed, and with DeleteInvalidFiles on, DeleteFailedFile() deleted the file. The maintainer chose choice 1 on #1095: count a signal-ended repair run with the tool runs that did not run.

• MediaTool.Execute(), the shared buffered path, records a run that a signal ended in ToolErrorScope.Count. EndedBySignal() reads 129 to 159 outside Windows. FfMpeg overrides it to add 255, the exit FFmpeg gives once it handles a signal.
• This covers every repair tier, which are the MkvMerge remux, the FfMpeg re-encode, the HandBrake re-encode, and the post-encode remux. It also covers every other buffered tool run, so a signal-killed run in any step keeps the file. ToolErrorScope.Count is read only by DeleteFailedFile() and by the two repair paths' before-and-after comparisons, so the wider reach only keeps files.
• FfProbe.GetPacketsAsync() has its own execution path and records a signal-ended packet analysis the same way.
• The FfMpeg verify run keeps its separate SignalCount, so a killed verify still runs the repair ladder.
• As Decide How a Signal-Killed Repair Run Affects DeleteInvalidFiles #1095's choice 1 states, a repair tool that crashes on the same input on every run repeats the repair on every run. ARCHITECTURE.md "Verify and Repair" says so.
• ARCHITECTURE.md and the unreleased 3.23 HISTORY.md entry are updated to match.

The comments label covers the edited ToolErrorScope summary comment and the new comment explaining the signal exit range.

Tests

• VerifyAndRepair_RepairSignalExit_KeepsStateAndFiles kills each repair tier in turn. It also includes FFmpeg's 255 exit. Each case leaves the state unchanged and keeps the file. All four cases fail without the fix.
• VerifyAndRepair_PacketAnalysisSignalExit_KeepsStateAndFiles kills the FfProbe packet analysis in the timestamp ladder. It fails without the FfProbe record.
• EndedBySignal_FfMpeg_MatchesSignalExitCodes and EndedBySignal_OtherTool_MatchesSignalExitCodes check the exit code boundaries.
• dotnet test: 338 passed, 0 failed.

Local review

Four recorded local-strict-review passes, the last over the current head with no findings.

Fixed after the local passes

• ARCHITECTURE.md:707 (introduced, wording): the RF node claimed any tool run that did not finish leads to a failure with the state unchanged. The code compares ToolErrorScope.Count only against a value taken just before the repair starts. The node now reads "a tool run during the repair failed to launch, threw, or was ended by a signal, or a signal ended a verify", fixed in 4df2c42. Qodo raised the same point.

Declined with evidence

• Treat only external-kill signals as "did not run", and let crash signals such as SIGSEGV count as failures. Declined, because Decide How a Signal-Killed Repair Run Affects DeleteInvalidFiles #1095's chosen option 1 accepts this trade-off: "A repair that crashes on the same input every time re-runs on every run". It is also consistent with the verify path, which reads the same range.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Processing runs interrupted by a signal are now recognized as errors. When repair does not complete, the media file and sidecar remain in place, and the file’s state is unchanged.
  • Signal-terminated packet analysis is now recorded as a tool error.
• Documentation
  • Updated processing guidance to clarify how incomplete tool runs affect file retention, repair state, and failure counts.

[ptr727]

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

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

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: afbfbc58-5b60-4ea3-8c0f-32b378238758

Commits

Reviewing files that changed from the base of the PR and between 6cc1980 and 4df2c42.

Files selected for processing: 7

• ARCHITECTURE.md
• HISTORY.md
• PlexCleaner/FfMpegTool.cs
• PlexCleaner/FfProbeTool.cs
• PlexCleaner/MediaTool.cs
• PlexCleaner/ToolErrorScope.cs
• PlexCleanerTests/VerifyToolErrorTests.cs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

Signal-ended tool runs are now classified and recorded as tool errors. Repair and packet-analysis signal exits leave state unchanged and retain the media and sidecar files. The architecture and history descriptions now cover these outcomes.

Changes: Signal-Ended Tool Run Handling

Layer / File(s)	Summary
Signal detection and tool error recording PlexCleaner/MediaTool.cs, PlexCleaner/FfMpegTool.cs, PlexCleaner/FfProbeTool.cs, PlexCleaner/ToolErrorScope.cs, PlexCleanerTests/VerifyToolErrorTests.cs	MediaTool adds EndedBySignal and records an error when a completed command exits by signal. FFmpeg extends signal classification for exit code 255, and FFprobe records errors for signal-ended packet analysis. Tests cover signal classification for FFmpeg and MkvMerge.
Incomplete-run state and file handling PlexCleanerTests/VerifyToolErrorTests.cs, ARCHITECTURE.md, HISTORY.md	Tests cover signal exits during repair and packet analysis, including tool-error counts, unchanged state, and retained files. The architecture and history descriptions cover incomplete tool runs and their effects on repair state and file handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• Decide How a Signal-Killed Repair Run Affects DeleteInvalidFiles #1095: The changes record signal-ended repair runs as tool errors and preserve state and files.
• Session Handoff [process-findings]: Promote and Release 3.23 #1085: The changes update the repair-path behavior and its architecture description.
• Clear BitrateExceeded and Handle Failed Bitrate and Packet Analysis #1078: The changes record signal-ended FFprobe packet-analysis runs as tool errors and preserve state and files.

Possibly related PRs

• ptr727/PlexCleaner#1093: This PR extends signal-exit handling from FFmpeg verification to repair and packet-analysis runs.
• ptr727/PlexCleaner#1091: This PR applies the existing tool-error and unchanged-state rules to signal-ended runs.
• ptr727/PlexCleaner#1089: This PR extends the non-completion handling for FFmpeg verification to signal-ended tool runs.

Merge Risk: Minimal · up to 4df2c

No merge-blocking issue is established for the signal-handling change. Merge readiness remains subject to normal build and test checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title identifies the main change: retaining a file when its repair run ends by signal. It is specific and related to the pull request.
Linked Issues check		Issue #1095 selects choice 1. The reviewed code records non-Windows signal exits in ToolErrorScope.Count for the shared buffered path. FfMpeg also recognizes exit 255. FfProbe.GetPacketsAsync() …
Out of Scope Changes check		The changes stay connected to issue #1095. The shared MediaTool.Execute() change covers the repair tools through their common execution path. The FfProbe change covers packet analysis in the repai…

Full details: Linked Issues check

Explanation

Issue #1095 selects choice 1. The reviewed code records non-Windows signal exits in ToolErrorScope.Count for the shared buffered path. FfMpeg also recognizes exit 255. FfProbe.GetPacketsAsync() records signal-ended packet analysis. The tests cover all four repair tiers and packet analysis. They verify unchanged state, retained media and sidecar files, and no modification. The existing state and count checks allow a later run to retry repair. The reported test result is 338 passed and 0 failed.

Full details: Out of Scope Changes check

Explanation

The changes stay connected to issue #1095. The shared MediaTool.Execute() change covers the repair tools through their common execution path. The FfProbe change covers packet analysis in the repair ladder. The FfMpeg override, tests, and documentation define and verify the required signal exit behavior. No unrelated change is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 5 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• 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

_{Comment @coderabbitai help to get the list of available commands.}

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

PR Summary by Qodo

Keep files when a repair tool is terminated by a signal

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

AI Description

• Treat signal-ended tool runs as inconclusive, preventing failed repairs from marking or deleting
 files.
• Apply signal handling to buffered tools and FfProbe packet analysis while preserving separate
 FFmpeg verify behavior.
• Add regression tests and document why repeatedly crashing repairs are retried.

Diagram

graph TD
  BT["Buffered tools"] --> SC{"Signal exit?"} -->|yes| TE["Tool error count"] --> RF{"Repair fails?"} -->|yes| SU["State unchanged"] --> KF["File retained"]
  PA["Packet analysis"] --> SC
  TE --> KF
  VS["Verify signal"] --> RF
  VS --> KF

Loading

High-Level Assessment

The shared buffered-execution check plus a check in FfProbe's separate execution path covers the affected tools without duplicating signal rules across repair tiers. Keeping verify's signal count separate preserves its existing repair behavior; narrowing the change to repair callers would leave other signal-ended runs eligible for deletion.

Files changed (7) +128 / -16

Bug fix (3) +17 / -5

FfMpegTool.csShare signal detection while retaining FFmpeg's 255 exit +5/-5

Share signal detection while retaining FFmpeg's 255 exit

• Overrides the common signal-exit check to recognize FFmpeg exit code 255. Verification continues to record signal-ended runs in its separate signal count.

PlexCleaner/FfMpegTool.cs

FfProbeTool.csCount signal-ended packet analysis as inconclusive +4/-0

Count signal-ended packet analysis as inconclusive

• Records a signal exit in the packet-analysis execution path, which does not use the shared buffered executor.

PlexCleaner/FfProbeTool.cs

MediaTool.csCount signal exits across buffered media-tool runs +8/-0

Count signal exits across buffered media-tool runs

• Adds a platform-aware signal-exit check and records matching exits in the per-file tool-error count. This lets existing repair-state and deletion checks protect files after signal-ended runs.

PlexCleaner/MediaTool.cs

Tests (1) +101 / -2

VerifyToolErrorTests.csCover signal exits across repair tiers and packet analysis +101/-2

Cover signal exits across repair tiers and packet analysis

• Tests signal-exit boundaries for FFmpeg and other tools. Adds repair and packet-analysis cases asserting that signal-ended runs leave state unchanged and retain the media file and sidecar.

PlexCleanerTests/VerifyToolErrorTests.cs

Documentation (3) +10 / -9

ARCHITECTURE.mdDocument signal-ended runs in retention and repair decisions +6/-6

Document signal-ended runs in retention and repair decisions

• Clarifies that signal-ended tool runs prevent deletion and that failed repairs after inconclusive runs leave state unchanged for a later retry. Updates the repair diagram and packet-analysis notes.

ARCHITECTURE.md

HISTORY.mdRecord signal-ended run protections in the release history +3/-2

Record signal-ended run protections in the release history

• Expands the unreleased entry to describe file retention and unchanged repair state when a tool run does not finish.

HISTORY.md

ToolErrorScope.csClarify what the tool-error count represents +1/-1

Clarify what the tool-error count represents

• Updates the class comment to describe runs that did not reach a result, including newly counted signal exits.

PlexCleaner/ToolErrorScope.cs

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

Code Review by Qodo

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

1. Killed verifies are shown as terminal ✓ Resolved

Description

The RF node routes any signal-ended tool run to terminal failure, but VerifyAndRepair() can
continue a signal-ended verification into the repair ladder because it checks SignalCount
separately from ordinary tool errors counted after its pre-repair snapshot. If the bitrate packet
probe ends on a signal during Verify() and a subsequent repair fails normally, the code can record
VerifyFailed and RepairFailed through the diagram’s RFF path, while DeleteFailedFile() still
keeps the file.

Code

ARCHITECTURE.md[707]

+  RF{"Cancelled, or a tool run failed to launch, threw, or was ended by a signal?"} -->|yes| FAIL

Evidence

The architecture diagram sends any signal-ended tool run through RF to terminal failure. In the
implementation, verification calls VerifyBitrate() before repair and ignores its result; the
packet probe’s signal handling increments Count, but VerifyAndRepair() snapshots that count
after verification and checks for later ordinary tool errors separately from SignalCount. A
signal-ended verify can therefore reach the repair ladder, and a later ordinary repair failure can
set both failure flags, while DeleteFailedFile() checks the full count and keeps the file.

Decide and consistently apply deletion behavior for signal-killed repairs
ARCHITECTURE.md[707-708]
PlexCleaner/ProcessFile.cs[1822-1910]
PlexCleaner/ProcessFile.cs[1737-1757]
PlexCleaner/ProcessFile.cs[1878-1907]
PlexCleaner/ProcessFile.cs[1777-1793]
PlexCleaner/ProcessFile.cs[2595-2604]
PlexCleaner/FfProbeTool.cs[180-186]

Agent prompt

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

## Issue description
The repair diagram treats every signal-ended tool run as a terminal failure, including verification and packet-analysis runs that the implementation can carry into the repair ladder. Its description of the keep-file path is therefore inconsistent with the code.
## Fix Focus Areas
- ARCHITECTURE.md[707-708]
## Recommended Fix
Update the `RF` wording to distinguish signal-ended repair runs from signal-ended verification runs. Qualify the ordinary tool-error condition as applying to runs during repair, and describe how a signal-ended verify can continue into repair and how the resulting failure path retains the file.

ⓘ 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 route each severity your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🟡 Changes recommended

The repair-state branch condition remains incorrect because a signal-ended FfProbe run during verification is included in the baseline, causing failed repairs to persist failure states even when the file is retained.

1 open issue

• Distinguish repair signals from verification signals · New

What changed in this PR

This pull request updates tool-error handling so signal-terminated repair and packet-analysis runs are treated as incomplete, preserving affected files and allowing later retries. It also adds regression coverage and updates project documentation.

Changes:

• Detects signal-ended buffered tool executions and packet-analysis runs, including FFmpeg exit code 255.
• Preserves media and sidecar files when interrupted repair operations produce no trusted result.
• Adds signal-boundary, repair-tier, and packet-analysis tests.
• Updates the architecture documentation and unreleased history.

This pull request requires changes.

File	Description
PlexCleanerTests/​VerifyToolErrorTests.cs	Tests signal classification and preservation behavior across repair and packet-analysis paths.
PlexCleaner/​ToolErrorScope.cs	Updates the scope documentation for incomplete tool runs.
PlexCleaner/​MediaTool.cs	Records signal-ended buffered tool executions as tool errors.
PlexCleaner/​FfProbeTool.cs	Records signal-ended packet-analysis executions.
PlexCleaner/​FfMpegTool.cs	Adds FFmpeg-specific signal handling, including exit code 255.
HISTORY.md	Documents file preservation and retry behavior for incomplete tool runs.
ARCHITECTURE.md	Updates the repair and deletion flow documentation.

_{Review effort: Lite}

[ptr727]

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