Repository navigation
stage 3c (stacked): final five domains — System/Project/Debug/Sidebar/Browser - #5909
Conversation
…s (drafts integrated) Five domains drafted by the orchestrator's agents (handed off), repaired (browserNavContext accessor, allocateElementRef state call, v1 handlers unhooked from the v2 chain), wired into the umbrella + dispatch, with test stubs completed. 140 package tests green. App-side surgery follows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e coordinator TerminalController.swift 18,033 -> 10,748 (-7,285). The five remaining domains now dispatch through ControlCommandCoordinator: System (identify/tree/auth.login/ session.restore/settings.open/feedback.open/extension snapshot/workspace.action/ tab.action/drag_to_split/split_off), Project (project.* + markdown.open + file.open), Debug (39 debug.* methods), Sidebar v1 (44 verbs via a new handleSidebarV1 hook ahead of the v1 switch), Browser panel v1 (8 verbs), and all 89 main-actor browser.* methods. Browser per-surface state moved off the controller: ControlBrowserAutomationState (package) + dialog responders keyed by dialogID app-side (the Sendable V2BrowserPendingDialog redesign); cleanupSurfaceState purges the new state, faithfully mirroring the legacy eviction. Two conformances the drafts never included (ControlBrowserContext, ControlBrowserPanelContext) were authored byte-faithfully from the legacy bodies. Shared bodies kept + relaxed to internal (v2Identify, v2WorkspaceAction, v2SurfaceSplitOff, v2FileOpen, the 18 v1-debug impls, the JS pump, the worker-lane browser.download.wait cluster). Deliberate deltas (documented): controlFeedbackOpen drops the deprecated .activateIgnoringOtherApps activation option (documented no-op on macOS 14+, the project floor; keeping it fails the new-file warning budget); a sequence id bridges Int64->Int (lossless on arm64). Gates: package swift build + 140/140 tests; tagged app build BUILD SUCCEEDED; live socket sweep green across all domains (system.tree, auth.login parity, browser.open_split -> get.title returns the real page title end-to-end, project validation errors, v1 set-status via the new hook, debug.terminals, plus regression of the ten prior domains); zero new warnings; both budgets pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
The System+Project adversarial review found file.open had been reimplemented in the coordinator/conformance while the original v2FileOpen stayed behind (it is driven directly by FilePreviewReviewFeedbackTests and MarkdownPanelTests) - two copies that could drift, and a stale dispatcher comment claiming forwarding. file.open now forwards to the single shared body and bridges its result, like workspace.create; the reimplementation and its now-unused ControlFileOpenResolution/ControlFileOpenSurface types are deleted. Review verdicts so far: System+Project all faithful (this was the only finding, not a behavior bug); Debug (39 verbs) + Sidebar v1 (44) + Browser-panel v1 (8) all faithful, zero divergences, #if DEBUG gating verified end-to-end. 140 package tests green; app build green; live probe of file.open through the shared body (happy path + both error shapes) byte-faithful. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h residue The Browser adversarial review (87/89 faithful) found its only two divergences share one root cause: focus_mode.set and zoom.set validated mode/direction BEFORE the TabManager/handle guards (legacy order: guards first). The shared browserFocusedAction helper gains a post-guard validate step; both methods' validation moves there. Live-verified: double-fault now returns unavailable/'TabManager not available', single-fault the mode/direction error. Residue: socketFastPathState drops its 'nonisolated' (after the cutover its only callers are the @mainactor sidebar/surface conformances; the worker-thread fast path retired with the legacy dispatcher). ServerEventTarget's @unchecked Sendable and the V2CallResult/V2SocketRequest twins stay deliberately: they serve the worker-lane and kept-shared bodies, which move in a later wave (the target itself dissolves with TerminalControlComposition in Wave 5). Verification totals for the five stacked domains: 143 methods/verbs reviewed per-method vs the pre-deletion originals; 141 faithful as-lifted, 2 fixed here. 140 package tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TLDR
The final five socket-command domains of stage 3c, stacked on PR #5816 so the diff reviews standalone: System, Project, Debug, Sidebar (v1), and Browser move from
TerminalControllerintoControlCommandCoordinator.TerminalController.swiftdrops 18,033 → 10,748 lines (−7,285); the legacy v2 switch is now justsystem.ping/system.capabilities. Merges into the 3c branch, not main.What moved
{"authenticated": true, "required": accessMode.requiresPasswordAuth}), session.restore_previous, settings.open, feedback.open, extension.sidebar.snapshot, workspace.action, surface.action/tab.action, drag_to_split/split_off.debug.*methods (both former#if DEBUGblocks).handleSidebarV1(command:args:)hook ahead of the v1 switch; raw-string responses preserved.browser.*methods + the panel v1 verbs. TheV2BrowserPendingDialogresponder-closure DTO is redesigned Sendable:ControlBrowserPendingDialogcarries adialogID; the WKWebView completion handlers stay app-side keyed by that id. Per-surface browser state moved to the package'sControlBrowserAutomationState;cleanupSurfaceStatepurges it faithfully.Shared bodies kept app-side + relaxed to internal (v2Identify, v2WorkspaceAction, v2SurfaceSplitOff, v2FileOpen, 18 v1-debug impls, the JS pump, the worker-lane browser.download.wait cluster). Worker-lane methods untouched.
Two deliberate deltas, both documented in the commit:
controlFeedbackOpendrops the deprecated.activateIgnoringOtherApps(documented no-op on macOS 14+, the project floor); one sequence id bridges Int64→Int (lossless on arm64).Verification so far
Package
swift build+ 140/140 tests; tagged app build green; zero new warnings; both budgets pass; live socket sweep across every domain (system.tree, auth.login parity,browser.open_split→browser.get.titlereturns the real page title end-to-end, project validation errors, v1 set-status through the new hook, plus regression of the ten prior domains). The adversarial per-method review against the pre-deletion originals is running now; its findings land as follow-up commits here before this merges.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Moves the final five socket-command domains — System, Project, Debug, Sidebar (v1), and Browser — from
TerminalControllertoControlCommandCoordinator, dedupesfile.opento the sharedv2FileOpen, and fixes legacy error precedence forbrowser.focus_mode.setandbrowser.zoom.set. This finishes stage 3c, keeps wire replies byte-identical, and shrinksTerminalController.swiftby ~7.3k lines.Refactors
ControlSystemContext,ControlProjectContext,ControlDebugContext,ControlSidebarContext,ControlBrowserContext,ControlBrowserAutomationContext, andControlBrowserPanelContexttoControlCommandContext.ControlBrowserAutomationState. RedesignedV2BrowserPendingDialogtoControlBrowserPendingDialogwith adialogID.handleSidebarV1/handleBrowserPanelV1with raw string replies preserved.file.opento forward to the sharedv2FileOpen; removed the coordinator-specific reimplementation; behavior unchanged.feedback.opendrops.activateIgnoringOtherApps; one sequence id bridges Int64→Int (lossless on arm64). Corrected validation order forbrowser.focus_mode.setandbrowser.zoom.setto match legacy guards. Budgets updated; package builds and 140/140 tests pass.Migration
handleSidebarV1(command:args:)andhandleBrowserPanelV1(command:args:)before the legacy v1 switch.ControlBrowserAutomationStateand purge it in surface cleanup; keep WKWebView completions keyed bydialogID.Written for commit 42f9332. Summary will update on new commits.