Handle missing directories in retention cleanup

[KristopherKubicki]

Summary

• skip retention cleanup loops when video or screenshot directories are absent
• add regression test confirming retention cleanup exits cleanly when directories are missing
• update existing invocation test to mock directory existence

Testing

• pytest tests/test_retention_policy.py

---

Codex Task

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/update-retention_cleanup-for-directory-checks

---

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 and usage tips.}

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[Copilot]

Pull Request Overview

This PR enhances the retention_cleanup() function to gracefully handle missing video or screenshot directories by adding existence checks before attempting to iterate over them. This prevents potential crashes when the application starts before these directories are created.

Key changes:

• Added os.path.isdir() checks before iterating over VIDEO_DIRECTORY and SCREENSHOT_DIRECTORY
• Added a regression test confirming cleanup exits cleanly when directories are missing
• Updated an existing mock test to account for the new directory existence checks

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
app/utils/retention_policy.py	Added os.path.isdir() guards before directory iteration loops to prevent errors when directories don't exist
tests/test_retention_policy.py	Added regression test for missing directories and updated existing mock test to include isdir mocking

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] The test is mocking os.path.isdir to always return True, which will cause cleanup_clips() (called at the end of retention_cleanup()) to attempt execution. The mock_listdir.side_effect provides three values, but this creates an implicit dependency on cleanup_clips() calling os.listdir() once.

Consider explicitly mocking cleanup_clips() to make the test more focused and resilient to changes in cleanup_clips() implementation:

@patch("app.utils.retention_policy.cleanup_clips")
@patch("app.utils.retention_policy.os.path.isdir", return_value=True)
@patch("app.utils.retention_policy.os.listdir")
@patch("app.utils.retention_policy.get_files_sorted_by_creation_time")
@patch("app.utils.retention_policy.delete_old_files")
def test_retention_cleanup_invokes_deletion(
    self, mock_delete, mock_get_files, mock_listdir, _mock_isdir, _mock_cleanup_clips
):
    mock_listdir.side_effect = [["cam1"], ["cam1"]]
    ...

This would reduce the side_effect list from 3 to 2 values and make the test independent of cleanup_clips behavior.

[Copilot]

The result of retention_cleanup is used even though it is always None.

Suggested change

	self.assertIsNone(retention_cleanup())
	retention_cleanup()

[github-actions]

Stale pull request message