Keep temp cleanup failures from stranding the reports

[bjk7119]

Summary by CodeRabbit

• Bug Fixes
  • Improved cleanup of temporary files and directories, including read-only items.
  • Prevented publishing results when raw data cannot be removed successfully.
  • Improved handling of output directories during finalization.
  • Added clearer reporting when individual file moves fail.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 99a7be0b-8667-4946-a7d1-57ef6aca7101

📥 Commits

Reviewing files that changed from the base of the PR and between 3edd65e and 98af892.

📒 Files selected for processing (1)

• src/fosslight_scanner/fosslight_scanner.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/fosslight_scanner/fosslight_scanner.py

---

📝 Walkthrough

Walkthrough

The scanner adds retryable removal for read-only files. Temporary extraction and finalization cleanup use this helper. Finalization tracks failed removal and move operations.

Changes

Cleanup resilience

Layer / File(s)	Summary
Retryable directory removal src/fosslight_scanner/fosslight_scanner.py	Adds remove_tree(path) with permission recovery, retry logic, failure logging, and a boolean result.
Temporary-data cleanup integration src/fosslight_scanner/fosslight_scanner.py	Uses remove_tree for extracted and staging directories. Skips undeleted raw data during publication, logs move failures, preserves staging output, and retains paths for files that were not moved.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: dd-jy

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: preventing temporary cleanup failures from leaving reports stranded.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch temp-cleanup

---

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.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/fosslight_scanner/fosslight_scanner.py (1)

72-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Limit cleanup handlers to filesystem errors.

except Exception also hides invalid arguments and programming errors. Catch OSError for filesystem operations and let unexpected exceptions surface.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fosslight_scanner/fosslight_scanner.py` around lines 72 - 85, Update the
cleanup logic around the os.walk, os.chmod, and shutil.rmtree operations to
catch only OSError rather than broad Exception, while preserving the existing
warning and successful return behavior. Allow invalid arguments and other
unexpected programming errors to propagate, and apply the same filesystem-error
restriction to the earlier retry handler in this cleanup flow.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fosslight_scanner/fosslight_scanner.py`:
- Around line 594-604: Track each failed move in the move loop around
shutil.move, preserving failed items under output_path instead of deleting them
during remove_tree(output_path). Only rewrite final_reports paths when the
corresponding report was successfully moved and exists under final_dir; leave
failed-report paths pointing to their retained staging locations.
- Around line 67-82: Update remove_tree to preserve existing permission bits by
OR-ing stat.S_IWRITE with each entry’s mode from os.stat, and apply the same
update to the root path before walking so read-only roots remain traversable.
Add a regression test covering removal of nested read-only directories.

---

Nitpick comments:
In `@src/fosslight_scanner/fosslight_scanner.py`:
- Around line 72-85: Update the cleanup logic around the os.walk, os.chmod, and
shutil.rmtree operations to catch only OSError rather than broad Exception,
while preserving the existing warning and successful return behavior. Allow
invalid arguments and other unexpected programming errors to propagate, and
apply the same filesystem-error restriction to the earlier retry handler in this
cleanup flow.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 380225ab-785d-4576-97e2-bf2c4d8c7574

📥 Commits

Reviewing files that changed from the base of the PR and between 3bc404a and 3edd65e.

📒 Files selected for processing (1)

• src/fosslight_scanner/fosslight_scanner.py