Repository navigation
Conversation
|
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 (4)
📝 WalkthroughWalkthroughThe changes update modal-result return flow in resume approval and make Dock session restoration imports, closure return types, and detached transfer test initialization explicit. Existing snapshot construction and observation filtering logic remain unchanged. ChangesSwift updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ Finishing Touches🧪 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 |
ejc3
changed the title
ejc3
changed the title
|
Verified on a clean worktree at 94a329e: |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Greptile SummaryThis PR fixes four Swift compiler errors that caused
Confidence Score: 5/5All four changes are narrowly scoped compiler fixes with no logic changes; the builds that were broken on main are restored and no new behavior is introduced. Every change is a minimal syntactic correction: adding a missing import, annotating return types to break circular type inference, prepending No files require special attention; all four fixes are straightforward and correct. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["DockSplitStore+RestoredAgentLifecycle.swift\n+ import CmuxWorkspaces"] -->|"resolves PanelShellActivityState"| B["✅ Compiles"]
C["DockSplitStore+SessionSnapshot.swift\ncompactMap { panel -> (UUID, UUID)? }"] -->|"breaks Key/Value inference cycle"| D["✅ Compiles"]
E["DockSplitStore+SessionSnapshot.swift\nflatMap { entry -> Entry? }"] -->|"breaks U inference cycle"| D
F["ControlSurfaceResumeTarget.swift\nreturn switch alert.runModal()"] -->|"provides contextual type for .auto/.prompt/.manual"| G["✅ Compiles"]
H["DockWorkingDirectoryInheritanceTests.swift\nsessionRestoreSourceWorkspaceId: nil"] -->|"satisfies updated memberwise init"| I["✅ Test compiles"]
Reviews (3): Last reviewed commit: "cmuxTests: pass the new transfer field a..." | Re-trigger Greptile |
|
Correction to my previous comment, and a fourth commit. That
Giving the property a default would not work: a Re-verifying with |
…eds it Building main at 4dc00ae fails with a single error: Sources/DockSplitStore+RestoredAgentLifecycle.swift:13:62: error: cannot find type 'PanelShellActivityState' in scope That file arrived with manaflow-ai#8690 and declares updatePanelShellActivityState(panelId:state:), whose state parameter is PanelShellActivityState. That type is a public enum in the CmuxWorkspaces package, and Swift imports are per-file, so the app target linking the package is not enough on its own. Every other file in Sources that names the type imports CmuxWorkspaces; this one imports only Foundation. ci.yml is dispatch-only, so no PR build runs and nothing caught it.
Second and third errors from the same build, after the missing import: Sources/DockSplitStore+SessionSnapshot.swift:44:43: error: generic parameter 'Key' could not be inferred Sources/DockSplitStore+SessionSnapshot.swift:44:43: error: generic parameter 'Value' could not be inferred Sources/DockSplitStore+SessionSnapshot.swift:332:47: error: generic parameter 'U' could not be inferred Both are the same shape: a multi-statement closure whose bail-out is a bare `return nil`, handed to something generic. At line 44 that is Dictionary(uniqueKeysWithValues:), which has to solve Key and Value; at line 332 it is Optional.flatMap, which has to solve U. A bare `return nil` carries no type, so the closure result and the generic parameters each depend on the other and the solver gives up. Naming the return types breaks the cycle. The types are the ones already required by the surrounding code: SessionSplitContainerSnapshot declares sourceWorkspaceIdsByPanelId as [UUID: UUID]?, DetachedSurfaceTransfer's sessionRestoreWorkspaceId is a UUID, and observation is a RestorableAgentSessionIndex.Entry?. Behaviour is unchanged.
Fourth error from the same build: Sources/ControlSurfaceResumeTarget.swift:282:40: error: reference to member 'auto' cannot be resolved without a contextual type Sources/ControlSurfaceResumeTarget.swift:283:41: error: reference to member 'prompt' cannot be resolved without a contextual type Sources/ControlSurfaceResumeTarget.swift:284:19: error: reference to member 'manual' cannot be resolved without a contextual type surfacePromptForResumeApproval builds an NSAlert over several statements and then ends with a bare `switch alert.runModal()` whose cases are `.auto`, `.prompt` and `.manual`. Swift gives a function body an implicit return only when the body is a single expression, so in a multi-statement body that switch is an expression statement with nothing to type it, and the leading-dot members have no SurfaceResumeApprovalPolicy to resolve against. Returning it supplies the contextual type. Every other trailing switch of this shape under Sources is the single-expression body of a computed property or a one-statement function, which is why this is the only one that fails.
…missed The test target does not build either: cmuxTests/DockWorkingDirectoryInheritanceTests.swift:216:49: error: missing argument for parameter 'sessionRestoreSourceWorkspaceId' in call DetachedSurfaceTransfer gained a `sessionRestoreSourceWorkspaceId: UUID?` stored property. The struct has no explicit init, so that field became a required argument of the memberwise one. Four places under cmuxTests construct the type and three of them were updated; this one was missed. `nil` is the behaviour-preserving value and the one the other three pass: `sessionRestoreWorkspaceId` resolves to `sessionRestoreSourceWorkspaceId ?? sourceWorkspaceId`, so nil keeps the restore id equal to the source id, as it was before the field existed. Giving the property a default instead would not work, because a `let` with an initial value is excluded from the memberwise init, which would break the five call sites under Sources that pass it explicitly.
6e2938a to
62066a0
Compare
|
Rebased onto current I checked whether any of it had become unnecessary rather than assuming. All four errors are still present on
None of the four files this touches were modified by those 15 commits, so every line number cited in the commits and in the description above still points at the right line. Re-verifying the build on the rebased tip, since my earlier |
|
Re-verified after the rebase onto current
Incidental evidence of why this one matters: I tried to build an unrelated one-file test fix (#8867) on plain |
|
Heads up: the three app-target fixes here (the |
|
Closing:
Verified by reading the current code at all four sites rather than pattern-matching for this branch's text. Nothing left here to merge. |
2 participants
Building
mainfails. Three errors, all in the two Dock session-restore files thatarrived with #8690, and the first one hides the other two:
Repro on a clean checkout of
mainat 4dc00ae:The missing import.
DockSplitStore+RestoredAgentLifecycle.swiftdeclaresupdatePanelShellActivityState(panelId:state:), whosestateis aPanelShellActivityState. That type is apublic enumin the CmuxWorkspaces package, andSwift resolves imports per file, so the app target depending on the package is not enough on
its own. Every other file under
Sources/that names the type imports CmuxWorkspaces; thisone imports only Foundation.
The two inference failures. Both are the same shape: a multi-statement closure whose
bail-out is a bare
return nil, handed to something generic. Line 44 gives it toDictionary(uniqueKeysWithValues:), which then has to solveKeyandValue; line 332 givesit to
Optional.flatMap, which has to solveU. A barereturn nilcarries no type, so theclosure's result type and the generic parameters each wait on the other. Naming the return
types breaks the cycle, using the types the surrounding code already fixes:
SessionSplitContainerSnapshotdeclaressourceWorkspaceIdsByPanelIdas[UUID: UUID]?,sessionRestoreWorkspaceIdis aUUID, andobservationis aRestorableAgentSessionIndex.Entry?. Behaviour is unchanged.A fourth error, in a third file.
surfacePromptForResumeApprovalbuilds anNSAlertoverseveral statements and ends with a bare
switch alert.runModal()whose cases are.auto,.promptand.manual:Swift gives a function body an implicit return only when the body is a single expression, so
in a multi-statement body that switch is an expression statement with nothing to type it, and
the leading-dot members have no
SurfaceResumeApprovalPolicyto resolve against. Returning itsupplies the contextual type. Every other trailing switch of this shape under
Sources/is thesingle-expression body of a computed property or a one-statement function, which is why this is
the only one that fails.
Worth saying how this got in, because it will happen again otherwise:
ci.ymlisworkflow_dispatch:-only, so no pull request is ever built. The checks that appear on a PRare reviewers, not a compile, so a commit that does not build can land looking green.
Summary by CodeRabbit