fix(cli): restore Encodable synthesis for PendingCursorShellApproval - #10808
lawrencecchen wants to merge 3 commits into
Conversation
#9349 added a decode-only legacyCommand CodingKey plus a custom init(from:), which kills Encodable synthesis: the cmux-cli target fails to compile on main (error: CodingKey case 'legacyCommand' does not match any stored properties). Encode the stored fields explicitly and never emit the legacy key.
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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; 2 remain after this review. 📝 WalkthroughWalkthroughThe record type now defines custom ChangesRecord serialization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized serialization fix restores CLI compilation and preserves the existing decode migration without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
Full details: Description checkExplanation The description gives a detailed summary, explains the compilation failure, and describes the encoding fix. It does not include the required Testing section, test results, checklist, or review-trigger confirmation. The Demo Video section is not necessary because this is not a UI change. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 too large.) Full details: Cmux Swift Actor IsolationExplanation PASS: The diff only adds Full details: Cmux Swift Blocking RuntimeExplanation PASS: The PR changes only Full details: Cmux Browser Automation Off-MainExplanation PASS: The pull request changes only Full details: Cmux Expensive Synchronous LoadExplanation PASS: The pull request adds only Full details: Cmux Cache Substitution CorrectnessExplanation PASS. The actual commit changes only Full details: Cmux No Hacky SleepsExplanation PASS. The pull request changes only Full details: Cmux Algorithmic ComplexityExplanation PASS: The pull request adds only seven direct keyed encoding operations in Full details: Cmux Swift ConcurrencyExplanation PASS: The commit changes only Full details: Cmux Swift `@Concurrent`Explanation PASS — The diff adds only a synchronous Full details: Cmux Swift Package BoundariesExplanation PASS. The diff adds only Full details: Cmux Swiftpm LockfilesExplanation PASS: The pull request changes only Full details: Cmux Swift LoggingExplanation PASS: The commit changes only Full details: Cmux User-Facing Error PrivacyExplanation PASS — The diff only adds Full details: Cmux Full InternationalizationExplanation PASS: The diff adds only a developer comment and Full details: Cmux Swiftui State LayoutExplanation PASS: The diff changes only Full details: Cmux Architecture RethinkExplanation PASS. The PR changes only Full details: Cmux Swift Auxiliary Window Close ShortcutsExplanation PASS — The PR changes only Full details: Cmux Source ArtifactsExplanation PASS: The only changed path is Full details: Cmux No Test Or Debug Seam In Production SourceExplanation PASS: The only changed file is Full details: Cmux No Ambient Global StateExplanation PASS. The diff adds only ✨ Finishing Touches📝 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 |
Same-scope 'let projectRoot = projectRoot ?? cwdURL' after 'var projectRoot: URL?' is an invalid redeclaration and breaks the build; bind the resolved root under a new name.
|
Pushed 32f4c3e: main has a second compile error from #9349 at CLI/cmux.swift:33116 (same-scope |
TerminalNotificationPolicyInFlightStore's guarded dictionary compactMap fails ElementOfResult inference on the fleet toolchain; annotate it and the sibling guarded closure in TerminalNotificationStore explicitly.
… no connection-driven pops (#10821) * iOS: fix main-red compile in MobileWorkspaceAggregation Same fix as #10813 (identical hunk, merges clean); carried here so this branch's packages build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: quiet reconnecting UX on the workspace detail screen Reconnecting now surfaces as a mini spinner in the title bar's existing indicator slot instead of the terminal-covering status pill; the pill remains only for the unavailable state, where it still offers Reconnect. The terminal stays fully interactive while a reconnect is in flight: input blocking (and keyboard resignation) now trigger only on unavailable or failed foreground recovery, and a send racing the dead window fails visibly through the send-status pill. Connection churn can no longer push the user out of the workspace detail screen. WorkspaceAbsenceAuthority decides whether a workspace's absence from a freshly derived list is authoritative (deleted, closed, unpaired - selection retargets and the detail pops as before) or a transient hole from a degraded connection (selection holds and the detail keeps rendering its last-known snapshot). The held-selection window also stops the selectedWorkspace first-row fallback from clobbering the terminal selection or mispairing input ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * cli: fix main-red Encodable synthesis for PendingCursorShellApproval Same fix as #10808 (encode(to:) hunk only); carried here so this branch's Mac build compiles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: fix second main-red from #10662 (fileprivate groupActionCapabilities) Same hunk as #10813; the +Actions file in the same target calls it, so archive fails on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: gate terminal input on the effective (recovery-aware) status Preflight showed the redial path downgrades the retained row to unavailable in the same turn it marks recovery as reconnecting, so the raw-status gate resigned the keyboard right as the title spinner started. effectiveConnectionStatus reads reconnecting through that window (and folds a failed recovery into unavailable), so the keyboard now survives the blip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: disconnected state moves into the title bar too The terminal-covering Disconnected pill is gone. The title's indicator slot turns red while disconnected, the subtitle line reads Disconnected (the input gate blocks typing there, so the state explains itself), and manual Reconnect moves into the title tap menu. Reauthentication keeps its blocking banner, and background auto-reconnect is unchanged. The now-unused status pill view and the detail's dead host parameter are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… no connection-driven pops (#10821) * iOS: fix main-red compile in MobileWorkspaceAggregation Same fix as manaflow-ai/cmux#10813 (identical hunk, merges clean); carried here so this branch's packages build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: quiet reconnecting UX on the workspace detail screen Reconnecting now surfaces as a mini spinner in the title bar's existing indicator slot instead of the terminal-covering status pill; the pill remains only for the unavailable state, where it still offers Reconnect. The terminal stays fully interactive while a reconnect is in flight: input blocking (and keyboard resignation) now trigger only on unavailable or failed foreground recovery, and a send racing the dead window fails visibly through the send-status pill. Connection churn can no longer push the user out of the workspace detail screen. WorkspaceAbsenceAuthority decides whether a workspace's absence from a freshly derived list is authoritative (deleted, closed, unpaired - selection retargets and the detail pops as before) or a transient hole from a degraded connection (selection holds and the detail keeps rendering its last-known snapshot). The held-selection window also stops the selectedWorkspace first-row fallback from clobbering the terminal selection or mispairing input ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * cli: fix main-red Encodable synthesis for PendingCursorShellApproval Same fix as manaflow-ai/cmux#10808 (encode(to:) hunk only); carried here so this branch's Mac build compiles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: fix second main-red from #10662 (fileprivate groupActionCapabilities) Same hunk as manaflow-ai/cmux#10813; the +Actions file in the same target calls it, so archive fails on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: gate terminal input on the effective (recovery-aware) status Preflight showed the redial path downgrades the retained row to unavailable in the same turn it marks recovery as reconnecting, so the raw-status gate resigned the keyboard right as the title spinner started. effectiveConnectionStatus reads reconnecting through that window (and folds a failed recovery into unavailable), so the keyboard now survives the blip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * iOS: disconnected state moves into the title bar too The terminal-covering Disconnected pill is gone. The title's indicator slot turns red while disconnected, the subtitle line reads Disconnected (the input gate blocks typing there, so the state explains itself), and manual Reconnect moves into the title tap menu. Reauthentication keeps its blocking banner, and background auto-reconnect is unchanged. The now-unused status pill view and the detail's dead host parameter are removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#9349 added a decode-only
legacyCommand = "command"CodingKey plus a custominit(from:)toClaudeHookSessionRecord.PendingCursorShellApproval, but noencode(to:). Swift refuses to synthesizeEncodablewhen a CodingKeys case matches no stored property, so thecmux-clitarget fails to compile on current main:Hit by two independent fleet builds of main (35e888d) tonight. Fix: explicit
encode(to:)that writes the seven stored fields and never emits the legacy key, preserving the decode-side migration exactly. No behavior change for records written by the new code.Open question for reviewers: how did #9349 pass its checks? Worth confirming the merge gate actually compiles
cmux-cli.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit