fix: restore app target compilation broken by #8567 worktree identity fields - #10777
Conversation
Commit 6cf5630 (#8567) declared worktreeDeviceID/worktreeFileID as 'let ... = nil', which removes them from the synthesized memberwise initializer, so the createWorktree call passing both labels failed to compile. Drop the defaults so the fields enter the memberwise init and the captured identity keeps flowing to rollback. Also unwrap the optional identity tuple in bestEffortCleanupFailedWorktree before comparing, since == is not lifted over optional tuples.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe worktree creation result now requires explicit filesystem identity values. Cleanup keeps its existing identity guard. Tests provide explicit nil identifiers and return awaited mutation results directly. ChangesWorktree identity handling
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This PR makes localized fixes to restore compilation and update affected test calls, with build and targeted test verification reported; no actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Cmux Swift Actor IsolationExplanation PASS. The production diff only removes default values from two stored properties and binds the optional filesystem identity to Full details: Cmux Swift Blocking RuntimeExplanation PASS. The production diff only removes Full details: Cmux Browser Automation Off-MainExplanation PASS: The pull-request diff contains only Full details: Cmux Expensive Synchronous LoadExplanation The production diff changes only Full details: Cmux Cache Substitution CorrectnessExplanation PASS: The production diff does not replace a fresh authoritative read with a cached or opportunistic value. It removes default values from Full details: Cmux No Hacky SleepsExplanation PASS. The custom check covers TypeScript, JavaScript, shell, and non-Swift build/runtime scripts. The PR changes only Full details: Cmux Algorithmic ComplexityExplanation PASS: The PR diff introduces no algorithmic-complexity violation. Production changes only remove initializer defaults and bind one optional filesystem identity before a constant-time tuple comparison in Full details: Cmux Swift ConcurrencyExplanation PASS: The diff adds no prohibited legacy async pattern. The production changes only adjust initializer fields and optional identity comparison. The test changes add explicit Full details: Cmux Swift `@Concurrent`Explanation PASS: The combined PR diff changes only initializer arguments, optional-identity comparison, and a test task-group return. It does not add or remove Full details: Cmux Swift Package BoundariesExplanation PASS. The production diff only fixes initialization and optional tuple comparison in Full details: Cmux Swiftpm LockfilesExplanation PASS — The PR diff from c819cdf to HEAD changes only Full details: Cmux Swift LoggingExplanation PASS. The PR diff adds only worktree identity initialization, optional-identity comparison, and an explicit test-task return. It adds or materially changes no Full details: Cmux User-Facing Error PrivacyExplanation PASS: The production diff only removes default values from two identity fields and unwraps the filesystem identity before comparison. It adds no user-facing error, alert, command output, API body, or recovery text. The test changes only supply nil values and return an iterator result. Existing generic error text and private logging are unchanged, so no prohibited implementation detail is introduced. Full details: Cmux Full InternationalizationExplanation PASS: The PR changes only Swift data-flow and cleanup logic in production code, plus test fixtures and async test control flow. The production diff adds no user-facing text, localization keys, catalog entries, web messages, metadata, or locale changes. Existing Full details: Cmux Swiftui State LayoutExplanation PASS. The diff changes only worktree identity initialization, cleanup comparison, and a test task-group return. The changed Swift files import Foundation/OSLog, not SwiftUI, and introduce no ObservableObject, Full details: Cmux Architecture RethinkExplanation PASS: The diff contains small local Swift correctness fixes. It removes default values so Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS. The diff changes only worktree result initialization, filesystem-identity cleanup, and a test fixture. It adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, window identifier, or close-shortcut code. The test-only fixture is explicitly allowed. The deterministic Full details: Cmux Source ArtifactsExplanation The full PR diff changes only Full details: Cmux No Test Or Debug Seam In Production SourceExplanation PASS — The PR adds no test or debug seam in production source. The only changed production lines remove default values from Full details: Cmux No Ambient Global StateExplanation PASS: The production diff only removes default values from two stored Full details: Description checkExplanation The description clearly explains what changed, why the compilation failed, and how the fix was tested. It omits the template checklist and review-trigger block, but these omissions are non-critical because the implementation and verification details are complete.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
A multi-statement closure gets no implicit return, so the group.addTask closure returning Int32? failed to compile. Found while running the touched test class on the remote builder.
2 participants
main's app target does not compile since commit 6cf5630 (#8567). Evidence: speculative merge-gate run https://github.com/manaflow-ai/cmux/actions/runs/32919392868, job tests-build-and-lag, step "Build for runtime regressions", exit 65 with two errors in
Sources/ExtensionWorktreePrototype.swift.This is a forward-fix, not a revert: #8567's behavior (rollback refuses to touch a replaced checkout, PTY respawn fix) is worth keeping.
A
letproperty with an initial value is excluded from Swift's synthesized memberwise initializer, so declaringlet worktreeDeviceID: UInt64? = nilmade thecreateWorktreecall that passesworktreeDeviceID:/worktreeFileID:fail with "extra arguments at positions #8, #9 in call". Dropping the= nildefaults puts both fields back into the memberwise init, and the real captured identity keeps flowing into the result so rollback can still refuse a replaced checkout. The test-only call site incmuxTests/ExtensionWorktreeSpawnArgsTests.swiftnow passesnilfor both labels.Second error, in
bestEffortCleanupFailedWorktree:filesystemIdentity(at:)returns an optional tuple and==is not lifted over optional tuples, so the guard now bindscurrentIdentityfirst and compares the unwrapped values.Third error, found while running the touched test class on a remote builder: the
group.addTaskclosure inrollbackRetainsEditsWrittenThroughOpenArtifactDescriptorhas more than one statement, so it gets no implicit return and failed with "missing return in closure expected to return 'Int32?'". An explicitreturnfixes it.Verification: the app target builds clean from this branch on a fleet Mac (cloud reload run https://github.com/manaflow-ai/cmux/actions/runs/32921947474, blacksmith-6vcpu-macos-26), and
xcodebuild -scheme cmux-unit -only-testing:cmuxTests/ExtensionWorktreeSpawnArgsTests testruns on the AWS builder.Summary by CodeRabbit
Bug Fixes
Tests