Skip to content

Fix cleanup helper actor isolation#641

Merged
r3dbars merged 1 commit intomainfrom
codex/fix-cleanup-helper-actor-isolation
May 2, 2026
Merged

Fix cleanup helper actor isolation#641
r3dbars merged 1 commit intomainfrom
codex/fix-cleanup-helper-actor-isolation

Conversation

@r3dbars
Copy link
Copy Markdown
Owner

@r3dbars r3dbars commented May 2, 2026

Summary

  • mark cleanup URL helper methods as nonisolated so package/dependency builds can call them from nonisolated cleanup paths

Verification

  • bash run-tests.sh
  • bash build-deps.sh --force

@r3dbars r3dbars merged commit c3d1191 into main May 2, 2026
@r3dbars r3dbars deleted the codex/fix-cleanup-helper-actor-isolation branch May 2, 2026 02:43
Copy link
Copy Markdown
Owner Author

r3dbars commented May 2, 2026

Code Review — PR #641

Verdict: APPROVE

Summary: This PR adds nonisolated to three private static pure-function helpers on TranscriptionTaskManager (canonicalURL, canonicalDirectoryURL, isFile(_:containedIn:)). Because the class is @MainActor, these static methods inherited main-actor isolation by default, which prevented the existing nonisolated cleanup path (isSafeCleanupURL → static helpers) from compiling under strict concurrency without async bridging. The fix is correct, minimal, and safe.

Blockers: None.

Should fix: None.

Nits: None.

What looks good:

  • Correct diagnosis: The three methods are pure functions — they take URL arguments, return URL/Bool, and touch zero instance or class-level mutable state. nonisolated is the right annotation here, not @Sendable or async bridging.
  • Consistent with existing pattern: The callers isSafeCleanupURL (line 519) and removeRecordingFile (line 487) were already nonisolated. This change completes the chain so the entire cleanup path is consistently non-isolated.
  • Security-critical path preserved: The isSafeCleanupURL containment check that prevents arbitrary file deletion via startImportedTranscription is functionally unchanged — only the concurrency annotation changed, not the logic.
  • Minimal blast radius: 3 lines changed, 1 file, all private static — no public API surface affected.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant