Skip to content

refactor: evict UI-test harness and git prober from TabManager#112

Closed
arzafran wants to merge 38 commits into
mainfrom
nr/tm
Closed

refactor: evict UI-test harness and git prober from TabManager#112
arzafran wants to merge 38 commits into
mainfrom
nr/tm

Conversation

@arzafran

@arzafran arzafran commented Jul 9, 2026

Copy link
Copy Markdown
Member

What this does

TabManager loses the 40 percent of its bulk that never belonged to it, and workspace lookup happens in exactly one place now.

Summary

  • TabManager+UITestHarness.swift: the DEBUG-only harness (plus its vsync-capture helper), same API, no test changes needed
  • GitMetadataProber.swift: the stateless git/gh probing library as a standalone type
  • workspace(withId:) replaces all 50 inline scans (drifted from the review's 48)
  • typealias Tab = Workspace deleted, 4 call sites migrated (refs Delete orphaned npm manifest, stale CLAUDE.md web/ reference, and verify build-sign-upload.sh #103)
  • NotificationSoundStaging.swift: the 520-line sound-transcoding subsystem out of TerminalNotificationStore

Fixes #90. Refs #103.

Test Plan

  • build + build-for-testing green locally
  • CI green

arzafran added 30 commits July 9, 2026 13:48
SurfacePool was fully wired but permanently disabled behind
programaSurfacePoolEnabled, a UserDefaults flag with no writer
anywhere in the repo (verified via repo-wide grep including
tests_v2/, CLI/, daemon/). The claim()/warmIfNeeded() calls
always no-op, so this collapses each call site to the only
path that ever ran:

- TabManager.swift: drop the canUsePool/claim branch, call
  makeWorkspaceForCreation directly (the only reachable path).
- AppDelegate.swift: drop the teardownAll() call at app
  termination.
- GhosttyTerminalView.swift: drop the dead pre-warm
  DispatchQueue.main.async block after app init.

Also removes the pbxproj entries for the deleted file.

Refs #89.
Move TabItemView and its exclusive helper-view cluster
(SidebarWorkspaceDescriptionText, SidebarMarkdownRenderer,
SidebarMetadataRows/EntryRow/MarkdownBlocks/BlockRow) out of
ContentView.swift into a new file, verbatim.

Access-level widening: TabItemView was file-private (private struct);
widened to internal (dropped the private modifier) because
VerticalTabsSidebar, which stays in ContentView.swift, constructs it
at its ForEach call site. No other behavior change. Equatable
conformance, precomputed let parameters, and the .equatable() call
site are untouched.

Refs #94.
…late

Introduce Sources/FileWatcher.swift, a low-level primitive that wraps the
open-fd / DispatchSource.makeFileSystemObjectSource / resume / cancel-close
boilerplate that was duplicated 6 times: ProgramaConfigStore's local+global
file/directory watchers, and ShortcutSettingsFileWatcher's file/directory
watchers.

Call-site retry/backoff/fallback policy is preserved exactly, including the
drift between copies:
- ProgramaConfigStore's local watcher only makes a single delayed (0.5s)
  existence check before permanently falling back to directory watching,
  while the global watcher actually recurses up to maxReattachAttempts (5)
  times with backoff before falling back. This drift (a leftover bug where
  the local reattach never recurses despite the shared constant) is
  preserved as-is and documented in a code comment.
- ShortcutSettingsFileWatcher has no delayed retry at all; it reattaches
  synchronously and immediately on delete/rename. Preserved as-is.

No behavior change intended beyond sharing the DispatchSource plumbing.

Refs #100.
…move)

Move the FileDropOverlayView NSView class out of ContentView.swift
into its own file, verbatim (no access-level changes needed — it was
already internal).

Refs #94.
…anical move)

Move the command-palette fuzzy-search engine (switcher search
metadata/indexer, fuzzy matcher, search corpus entry/result types,
search engine) out of ContentView.swift into its own file, verbatim.
Zero coupling to ContentView state.

Refs #94.
…ingsFileStore.swift

The type inside was already renamed to ProgramaSettingsFileStore (with a
KeyboardShortcutSettingsFileStore typealias kept for existing call sites),
but the file itself kept the old name. Rename the file to match; no source
changes beyond the project.pbxproj path/name entries for the file's stable
UUIDs.

Refs #100.
TerminalSurface.desiredFocusState and GhosttyNSView.desiredFocus both
track intended focus, and could diverge silently. The concrete bug:
clearSuppressReparentFocus() called recordExternalFocusState(true)
unconditionally, then (on the immediate-apply branch) called
reassertTerminalSurfaceFocus() -> setFocus(true). setFocus's dedup
guard (focused != desiredFocusState) saw no change, because
recordExternalFocusState had already set desiredFocusState = true
moments earlier, so the real ghostty_surface_set_focus push was
silently skipped. A surface could end up looking focused in the UI
(bonsplit selection, desiredFocus true) while the Ghostty renderer
never actually received focus, after a programmatic split/reparent.

Fix: recordExternalFocusState(true) is now only called on the
branches that don't reach reassertTerminalSurfaceFocus (deferred:
window not key, or hidden/tiny geometry). The immediate-apply branch
lets setFocus own desiredFocusState as its sole writer, so the dedup
guard sees the real transition and pushes ghostty_surface_set_focus.
Each branch now has exactly one writer of desiredFocusState for its
transition. No change to forceRefresh or keyDown paths.

Refs #86.
…eHostedView

WindowTerminalPortal.synchronizeHostedView tripled a near-identical
hide-and-schedule-transient-recovery block across the missingAnchorOrWindow,
anchorWindowMismatch, and hostBoundsNotReady guards. Only the
hostBoundsNotReady copy had the production-build fallback (calling
scheduleDeferredFullSynchronizeAll() when Self.transientRecoveryEnabled is
false) — the other two copies would silently drop the hide with no
follow-up resync scheduled in that configuration, and a genuinely-transient
condition could leave a hosted view stuck hidden.

Extracted hideHostedViewSchedulingRecovery(hostedId:entry:hostedView:reason:)
with the fallback included, and call it from all three sites. Behavior is
now consistent across all three guards; each caller's own distinct
diagnostic dlog (anchorWindowMismatch's anchorWindow=, hostBoundsNotReady's
portal.sync.defer) is kept as a separate pre-log alongside the shared
generic 'portal.hidden value=1 reason=X' log the helper emits, so a couple
of call sites may emit one additional DEBUG-only log line per hide compared
to before; no functional/production behavior change beyond fixing the
missing fallback for the two previously-unprotected reasons.

Refs #85.
Mechanical eviction of ~2,950 zero-dependency-on-AppDelegate free-standing
types into their own files:

- MainWindowHostingView.swift: MainWindowHostingView (SwiftUI hosting view)
- TypingProfiler.swift: ProgramaTypingTiming, ProgramaMainRunLoopStallMonitor,
  ProgramaMainThreadTurnProfiler (all #if DEBUG)
- TerminalDirectoryOpener.swift: FinderServicePathResolver,
  TerminalDirectoryOpenTarget
- VSCodeIntegration.swift: VSCodeServeWebURLBuilder,
  VSCodeCLILaunchConfiguration(Builder), VSCodeServeWebController,
  ServeWebOutputCollector, ServeWebPortStore
- WorkspaceShortcutMapper.swift: WorkspaceShortcutMapper
- CLIInstaller.swift: ProgramaCLIPathInstaller
- MenuBarIconRenderer.swift: MenuBarExtraController, notification menu
  snapshot/formatting types, MenuBarIconRenderer and its settings
- WindowSwizzles.swift: the NSWindow/NSApplication method-swizzle
  implementations and their supporting file-scoped state

Access-level widening required for cross-file visibility (no other code
changes):
- AppDelegate.recordTypingActivity(): fileprivate -> internal. Called from
  NSWindow.programa_sendEvent(_:), now in WindowSwizzles.swift.
- extension NSWindow / extension NSApplication (swizzle methods): private
  -> internal. AppDelegate.installWindowResponderSwizzles() references
  programa_makeFirstResponder/programa_sendEvent/programa_performKeyEquivalent/
  programa_applicationSendEvent via #selector(...) from AppDelegate.swift.
- programaFirstResponderGuardCurrentEventOverride/HitViewOverride: private
  -> internal. AppDelegate.setWindowFirstResponderGuardTesting(...) and
  .clearWindowFirstResponderGuardTesting() write these directly.

private extension AppDelegate (handleThemesReloadNotification) stays in
AppDelegate.swift since it extends AppDelegate itself, not a zero-dependency
type.

Refs #95.
…nspectorDock

Extract the WKInspector string-sniffing and side-dock/divider geometry math
that was independently reimplemented in BrowserPanel.swift, BrowserPanelView.swift
(WebViewRepresentable.Coordinator.HostContainerView), and BrowserWindowPortal.swift
(WindowBrowserHostView / WindowBrowserSlotView helpers) into one canonical
Sources/Panels/InspectorDock.swift, and route all three through it.

A real behavioral divergence is preserved explicitly: BrowserPanel's side-dock
*detection* path required sibling width > 1, while the divider *hit-testing*
paths in BrowserPanelView/BrowserWindowPortal intentionally omit that width
check (siblings can be transiently zero-width mid-drag). This is now expressed
as InspectorDock.isVisibleSiblingCandidate(_:requireMinWidth:) with the
difference documented at the call sites instead of silently duplicated.

AppDelegate.swift has its own copy of the responder-sniffing half
(programaIsLikelyWebInspectorResponder) — left untouched since AppDelegate
belongs to a different maintenance cluster.

Also consolidates responderChainContains (previously separately implemented
in BrowserPanel.swift and BrowserPanelView.swift) into InspectorDock, and
folds three copies of relatedWebKitTransferSubviews' WKInspector-visibility
check (BrowserPanelView.swift, BrowserWindowPortal.swift) onto the same
InspectorDock primitives.

Refs #91. Refs #103.
…nect

configureRemoteConnection and disconnectRemoteConnection each carried a
drifted copy of the remote-state reset block. configureRemoteConnection
never cleared the per-panel bookkeeping fields (activeRemoteTerminalSurfaceIds,
activeRemoteTerminalSessionCount, pendingRemoteSurfaceTTYName/SurfaceId,
pendingRemoteSurfacePortKickReason/SurfaceId) that disconnectRemoteConnection
did, so reconfiguring an already-connected workspace to a new destination left
stale per-panel remote-session bookkeeping behind — including causing
seedInitialRemoteTerminalSessionIfNeeded to skip seeding the new session
because activeRemoteTerminalSurfaceIds wasn't actually empty.

Extract resetRemoteState(), the union of both blocks, and call it from both
paths. configureRemoteConnection now captures pendingRemoteForegroundAuthToken
before the reset (since the reset nulls it) and seeds the initial terminal
session after the reset instead of before, so seeding sees a clean slate.

Refs #83.
… scp upload

Three pieces of logic were copy-pasted across WorkspaceRemoteSession.swift,
WorkspaceRemoteDaemon.swift, and TerminalSSHSessionDetector.swift:

- shellSingleQuoted: byte-identical POSIX single-quoting helper, present 4
  times (twice in WorkspaceRemoteDaemon.swift alone — one copy was entirely
  dead code with no call sites). Consolidated into
  RemoteSSHConnectionPolicy.shellSingleQuoted.

- The SSH connection-policy flag set (ConnectTimeout/ServerAliveInterval/
  ServerAliveCountMax keepalive triple, the StrictHostKeyChecking=accept-new
  default, the BatchMode=yes/ControlMaster=no pairing) and the underlying
  -o key=value option parsing (optionKey/hasOptionKey/normalizedOptions/
  backgroundOptions/optionValue) were each defined 2-3 times with matching
  behavior. Consolidated into RemoteSSHConnectionPolicy; each call site still
  assembles its own full argument list (they legitimately differ in jump-host/
  agent-forwarding/compression handling and port-flag spelling) but the
  identical fragments now have one definition, spliced in at the same
  position so argument order is unchanged.

- The scp-upload-with-cancel-cleanup control flow (upload each file, checking
  for cancellation, cleaning up whatever was already uploaded and rethrowing
  on any failure) was independently implemented for the ad-hoc detected-SSH
  path (TerminalSSHSessionDetector.uploadDroppedFilesSync) and the
  daemon-relay path (WorkspaceRemoteSession.uploadDroppedFilesLocked).
  Extracted performSCPUploadWithCancelCleanup, parameterized by a per-item
  upload closure; each call site still decides whether to record a file's
  remote path before or after the transfer completes, since that differed
  between the two existing implementations and isn't part of this dedup.

New files RemoteSSHConnectionPolicy.swift and RemoteSCPUpload.swift are
registered in GhosttyTabs.xcodeproj/project.pbxproj (main app target only —
these are only used by main-app-only files).

Refs #92.
handleCustomShortcut(event:) was a single 966-line function with ~55
sequential if-checks. Documents the discovered precedence in a comment
(palette > browser/terminal pre-checks > app-shortcut) and extracts the
lowest-precedence phase -- the flat table of ~55 matchConfiguredShortcut /
numberedConfiguredShortcutDigit / matchConfiguredDirectionalShortcut /
matchTabShortcut checks -- into handleConfiguredAppShortcutActions(event:
commandPaletteTargetWindow:hasFocusedAddressBarInShortcutContext:).

This is pure tail-call code motion: the extracted function is byte-identical
to the original tail of handleCustomShortcut (same statements, same order,
same early returns), so evaluation order and behavior are unchanged. The
higher-precedence palette and browser/terminal pre-check phases are left
grouped in place per the conservative fallback (group into sub-functions
without changing evaluation order) rather than risking a full reorder of a
typing-latency-sensitive, tightly state-coupled dispatch chain.

Refs #95.
…moves

TabItemView.swift, FileDropOverlayView.swift, and CommandPaletteSearchEngine.swift
now live in their own files, so file-private symbols they depend on (or that
depend on them) that remain in ContentView.swift need at least internal
visibility:

- SidebarDragAutoScrollController, SidebarTabItemSettingsSnapshot,
  SidebarTabDragPayload, BonsplitTabDragPayload, SidebarBonsplitTabDropDelegate,
  SidebarTabDropDelegate, MiddleClickCapture, MiddleClickCaptureView — used by
  the extracted TabItemView.
- coloredCircleImage, debugCommandPaletteTextPreview, the Color(hex:)
  initializer — top-level helpers used by the extracted TabItemView.

FileDropOverlayView.swift needs `import Bonsplit` for dlog() (per CLAUDE.md's
dlog-needs-bonsplit-import note).

No behavior changes — access-level widening only.

Refs #94.
WorkspaceRemoteDaemon.swift (2226 lines) held nine top-level type
declarations covering unrelated concerns: RPC pending-call bookkeeping,
SSH batch-command argument building, the daemon RPC client, HTTP
request/response rewriting for the loopback proxy alias host, the local
proxy tunnel plus its broker, and the CLI relay server. Split into six
files along those seams, moving each type verbatim (no code changes):

- WorkspaceRemoteDaemonPendingCallRegistry.swift
- WorkspaceRemoteSSHBatchCommandBuilder.swift
- WorkspaceRemoteDaemonRPCClient.swift
- WorkspaceRemoteLoopbackHTTPRewriting.swift (RemoteLoopbackHTTPRequestRewriter,
  RemoteLoopbackHTTPRequestStreamRewriter, RemoteLoopbackHTTPResponseRewriter —
  kept together, they're the cohesive HTTP-rewriting subsystem)
- WorkspaceRemoteProxyBroker.swift (WorkspaceRemoteDaemonProxyTunnel +
  WorkspaceRemoteProxyBroker — kept together since the tunnel is declared
  private and is only used by the broker; splitting them would have required
  widening its access level, which this move-only pass avoids)
- WorkspaceRemoteCLIRelayServer.swift

No access-level changes were needed — every moved type was already internal
by default, aside from the private WorkspaceRemoteDaemonProxyTunnel which
stayed paired with its only caller. project.pbxproj updated: old
WorkspaceRemoteDaemon.swift entries replaced with the six new files
(main app target only).

Refs #98.
…nalView.swift

GhosttyTerminalView.swift (9,490 lines) hosts 6+ subsystems; apply the
existing +Extension convention (+Keyboard, +Mouse, +DragDrop,
+Accessibility) to two more, clearly self-contained ones:

- GhosttyTerminalView+IME.swift: the GhosttyNSView: NSTextInputClient
  conformance (marked-text/composition state, committed-text delivery,
  character-coordinate queries for system input methods). Moved verbatim.
- GhosttyTerminalView+SwiftUIWrapper.swift: the GhosttyTerminalView
  NSViewRepresentable struct, its Coordinator, and the private
  HostContainerView it manages — the SwiftUI/AppKit bridge. Moved verbatim.

Both are pure file moves; the only code change is widening
GhosttyNSView.imePointOverrideForTesting from private to private(set) so
the moved IME extension (a #if DEBUG testing hook, firstRect override) can
still read it — it is written only inside GhosttyNSView's own declaration,
which stays in GhosttyTerminalView.swift, so private(set) preserves the
single-writer invariant.

GhosttyTerminalView.swift drops from 9,490 to 8,592 lines. Registered both
new files in project.pbxproj (NRGV0001-NRGV0004).

Refs #97.
…onary

Replace 7 parallel [UUID: _] dictionaries (visibility, pendingOpen,
recentRequestAt, escapeSuppression set, escapeSuppressionStartedAt,
selection, snapshot) with a single CommandPaletteWindowState struct stored
in one commandPaletteStateByWindowId: [UUID: CommandPaletteWindowState]
dictionary.

Adds three small helpers (commandPaletteState(for:),
updateCommandPaletteState(for:_:), teardownCommandPaletteState(for:)) so the
two previously-duplicated 7-line teardown blocks (window context discard,
window close) collapse to a single teardownCommandPaletteState(for:) call
each. Every read/write site is a mechanical per-field translation with no
behavior change (removeValue(forKey:) on a field-specific dictionary maps to
setting that field back to its default; Set.contains maps to an Optional
Bool comparison, etc).

Refs #95.
…ITestHarness.swift

Pure move of the ~1,340-line DEBUG-only UI-test harness (workspace/terminal-panel
readiness waiters, focus-shortcut/split-close-right/child-exit-split/child-exit-keyboard
test setup, and the vsync IOSurface timeline capture helper it drives) out of the
TabManager class body into its own extension file, following the same pattern as
AppDelegate+UITestHarnesses.swift. Same API, still #if DEBUG-gated.

Access widening (fileprivate/private -> internal), each required only for cross-file
visibility from the new extension file:
- didSetupSplitCloseRightUITest, didSetupUITestFocusShortcuts,
  didSetupChildExitSplitUITest, didSetupChildExitKeyboardUITest, uiTestCancellables:
  stored properties, so they must stay on the TabManager class body (Swift extensions
  cannot add stored properties) but are now read/written from the new file.
- setupUITestFocusShortcutsIfNeeded/setupSplitCloseRightUITestIfNeeded/
  setupChildExitSplitUITestIfNeeded/setupChildExitKeyboardUITestIfNeeded: called from
  TabManager.init() in TabManager.swift.
- VsyncIOSurfaceTimelineState / programaVsyncIOSurfaceTimelineCallback: file-scoped
  helpers used only by the split-close-right visual capture harness; moved alongside it.

Registered the new file in project.pbxproj (PBXFileReference + PBXBuildFile + group +
Sources phase), following the TabManager+GitMetadataPolling.swift wiring pattern.

Refs #90.
Move-only file reorganization (#99). Extracts self-contained top-level
types out of the ~6.5k-line BrowserPanel.swift god object into dedicated
files, one concern each:

- BrowserProfileStore.swift: BrowserProfileDefinition + BrowserProfileStore
  (profiles concern)
- BrowserHistoryStore.swift: normalizedBrowserHistoryNamespace(bundleIdentifier:)
  + BrowserHistoryStore (history concern)
- BrowserDownloadDelegate.swift: BrowserDownloadDelegate (downloads concern)
- BrowserPanelWebDelegates.swift: browserNavigationShouldOpenInNewTab /
  browserNavigationShouldCreatePopup / browserNavigationShouldFallbackNilTargetToNewTab
  + BrowserNavigationDelegate + BrowserUIDelegate (navigation delegates concern)
- BrowserUserProxySettings.swift: BrowserUserProxySettings (remote proxy concern)
- IMECompositionMessageHandler.swift: IMECompositionMessageHandler (JS
  injection/IME concern)

BrowserPanel.swift: 6517 -> 4677 lines.

Access-level widening required for cross-file visibility (all move-only,
no behavior change):
- BrowserInsecureHTTPNavigationIntent: private -> internal (used by
  BrowserNavigationDelegate/BrowserUIDelegate, now in a different file)
- BrowserNavigationDelegate, BrowserUIDelegate: private class -> class
  (instantiated from BrowserPanel in BrowserPanel.swift)
- IMECompositionMessageHandler: private final class -> final class
  (instantiated from BrowserPanel in BrowserPanel.swift)

The stale "// MARK: - Browser Data Import" comment that sat directly above
BrowserUserProxySettings (mislabeled leftover, unrelated to the actual
Browser Data Import feature which lives in BrowserDataImport.swift) was
corrected to "// MARK: - Remote/User Proxy Settings" when moved.

Not attempted here (left in BrowserPanel.swift, ~4.7k lines remaining):
the BrowserPanel class body itself (profiles/history/JS-injection/remote-proxy
call sites, plus the automation surface) is still one large class — pulling
its methods into extensions in other files would require widening many
individual stored-property access levels and was judged too risky to do
without dedicated design time. BrowserPanelView.swift's WebViewRepresentable
and BrowserWindowPortal.swift's remaining seams are also unaddressed.

Refs #99.
…ls.swift

Consolidate the drag-and-drop domain (DragOverlayRoutingPolicy, drag payload
enums, drop delegates, drop edge/indicator/planner, auto-scroll planner, drag
lifecycle/failsafe types) and the sidebar visual chrome (footer/help-menu/
scrim/blur/material/tint/preset enums, visual-effect/backdrop views, NSColor
extension), both previously fragmented across several non-contiguous locations
in ContentView.swift, into two new files. Done together in one pass since both
extractions needed the same verification cycle.

Access-level widening (dropped `private`, no other behavior change):
- SidebarDragFailsafeMonitor, SidebarExternalDropOverlay — constructed from
  VerticalTabsSidebar / ContentView, which stay in ContentView.swift.
- SidebarFooter, SidebarTopScrim, SidebarScrollViewResolver,
  SidebarScrollViewResolverView, SidebarEmptyArea, ClearScrollBackground,
  DraggableFolderIcon, TitlebarLeadingInsetReader, SidebarTrailingBorder,
  SidebarBackdrop — same reason.

The fileDropOverlayKey/commandPaletteWindowOverlayKey/
tmuxWorkspacePaneWindowOverlayKey associated-object keys and their overlay
container identifiers stay in ContentView.swift (they back
WindowCommandPaletteOverlayController / WindowTmuxWorkspacePaneOverlayController,
not the drag-drop domain) — they only transited SidebarDragDrop.swift briefly
during the mechanical move and were moved back.

Refs #94.
…tMetadataProber.swift

Moves the ~640-line stateless git/GitHub CLI probing library (branch/dirty-state
lookup, gh pr list/checks polling, process running, GitHub remote/slug parsing) out
of the TabManager class body into a standalone GitMetadataProber type. TabManager
keeps a thin owned instance (gitMetadataProber); the API itself stays static since
none of it touches TabManager instance state (all of it was already
'nonisolated static' on TabManager).

WorkspaceGitProbeKey stays on TabManager: unlike the prober helpers, it is a plain
dictionary-key type used throughout TabManager's own probe-tracking state
(workspaceGitProbeGenerationByKey and friends), not something the stateless prober
itself needs.

Access widening, both required for cross-file use from TabManager.swift now that the
declarations live in a different file:
- InitialWorkspaceGitMetadataSnapshot / WorkspacePullRequestSnapshot: private -> internal,
  since TabManager.applyWorkspaceGitMetadataSnapshot/shouldStopWorkspaceGitMetadataRefresh
  take/switch over these types.
- initialWorkspaceGitMetadataSnapshot(for:) / normalizedBranchName(_:): private -> internal,
  called from TabManager.scheduleWorkspaceGitMetadataRefresh /
  TabManager.updateSurfaceGitBranch respectively.

Updated programaTests/TabManagerUnitTests.swift call sites (TabManager.X ->
GitMetadataProber.X) for the moved API: githubRepositorySlugs(fromGitRemoteVOutput:),
GitHubPullRequestProbeItem, preferredPullRequest(from:),
shouldSkipWorkspacePullRequestLookup(branch:), resolvedCommandPathForTesting(...).

Registered the new file in project.pbxproj.

Refs #90.
…ttySurfaceScrollView

GhosttySurfaceScrollView (~2,780 lines) is one of the god-view subsystems
called out by Nuclear Review #97. Extract its debug-only render/frame
introspection (debugRenderStats, debugCopyIOSurfaceCGImage,
debugSampleIOSurface + their DebugRenderStats/DebugFrameSample structs;
entirely #if DEBUG) into GhosttyTerminalView+RenderStats.swift as a
same-type extension, matching the existing +Extension convention.

Pure move; three access-level widenings were required for cross-file
visibility from the moved extension (each noted inline at the
declaration):
- GhosttySurfaceScrollView.surfaceView: private let -> internal let
  (immutable, so this only grants read access).
- GhosttySurfaceScrollView.isActive: private var -> private(set) var
  (write stays confined to the class's own declaration in
  GhosttyTerminalView.swift; the moved extension only reads it).
- GhosttySurfaceScrollView.contentsKey(for:) and .updatePresentStats(...):
  private static func -> internal static func (called, not just read,
  from the moved extension).

Refs #97.
…e inline scans

Adds a single func workspace(withId:) on TabManager and replaces all 50 inline
'tabs.first(where: { $0.id == X })' scans across TabManager.swift and its
TabManager+Browser/GitMetadataPolling/Splits extensions with calls to it, including
the selectedWorkspace computed property itself and the sites that re-derive it from
selectedTabId. Mechanical, behavior-identical (same O(n) scan, now in one place).

Refs #90.
…sites to Workspace

typealias Tab = Workspace collided with Bonsplit.Tab (a different type). Migrates
its remaining call sites to spell out Workspace directly and deletes the alias:
- AppDelegate.resolveTerminalPanelForTextSend(in:preferredPanelId:) / sendTextWhenReady(_:to:...)
- ContentView.TabItemView.tab (the typing-latency-protected sidebar row view — this
  is a type-annotation spelling change only, not a reshape: Tab was already an alias
  for Workspace, so the stored property's actual type is unchanged)
- TerminalController.resolveTab/resolveSidebarMutationTab/tabForSidebarMutation/
  scheduleSidebarMutation

These call sites live outside the TabManager cluster's core files, but the alias is
declared in TabManager.swift and deleting it without updating every call site would
not compile, so all of them needed to move together with the deletion.

Refs #90, #103.
…ificationSoundStaging.swift

Pure move of the ~520-line NotificationSoundSettings enum (custom notification sound
validation, staging/transcoding into a UNNotificationSound-compatible format via
afconvert, and local playback preview) out of TerminalNotificationStore.swift into its
own file. It was already a top-level enum, not nested in TerminalNotificationStore,
so no access changes were needed.

Also flags (does not fix) the AppDelegate.shared layering smell in
TerminalNotificationStore.updateFocusedReadIndicator: reaching back into a global to
find the owning TabManager needs a real design pass on how TerminalNotificationStore
is scoped/injected, not a mechanical move.

Registered the new file in project.pbxproj.

Refs #90.
….swift

Move-only file reorganization (#99), done after the inspector-dock dedup
(#91) landed as the issue calls for. WebViewRepresentable (the WKWebView
NSViewRepresentable wrapper, including its nested Coordinator and
HostContainerView) was the last top-level declaration in
BrowserPanelView.swift and fully self-contained (~2.3k lines) — moved
verbatim into Sources/Panels/WebViewRepresentable.swift.

BrowserPanelView.swift: 6728 -> 4395 lines.

Access-level widening required for cross-file visibility: the
`private extension WKWebView` holding programaBrowserPanelNotifyHidden/
programaBrowserPanelReattachRenderingState/programaBrowserPanelForceRenderingStateRefresh
(called from WebViewRepresentable, now in a different file) is widened to
a plain (internal) extension. The genuinely private members of that
extension (programaBrowserPanelNeedsRenderingStateReattach,
programaBrowserPanelApplyRenderingStateRefresh) keep their own explicit
`private` modifiers and remain file-scoped to BrowserPanelView.swift.

Refs #99.
… concern

WorkspaceRemoteSession.swift (2582 lines) held a single class,
WorkspaceRemoteSessionController, whose ~100 methods spanned five
unrelated concerns. Split along those seams into extension files, moving
every method verbatim:

- WorkspaceRemoteSessionController+ConnectionOrchestration.swift:
  connection-attempt/reverse-relay/proxy-lease orchestration and status
  publishing (stopAllLocked...stopReverseRelayViaControlMasterLocked,
  plus the bootstrapRemoteTTYRetry* constants).
- WorkspaceRemoteSessionController+ProcessExecution.swift: ssh/scp
  argument assembly and the underlying Process execution primitive
  (sshCommonArguments...runProcess).
- WorkspaceRemoteSessionController+DaemonInstall.swift: remote daemon
  bootstrap/build/download/upload and dropped-file upload
  (bootstrapDaemonLocked...helloRemoteDaemonLocked, plus the
  remotePlatformProbe* markers and remoteDaemonManifestInfoKey).
- WorkspaceRemoteSessionController+ScriptBuilders.swift: debug logging,
  remote shell script builders, and process/PID utilities
  (debugLog...shouldEscalateProxyErrorToBootstrap, plus
  cachedRemoteDaemonSourceFingerprint).
- WorkspaceRemoteSessionController+PortScanning.swift: remote
  listening-port scan scheduling, polling, and script builders
  (updateRemotePortScanTTYs...remoteAllPortsScanScript).

WorkspaceRemoteSession.swift itself now holds only the class declaration,
its nested types, stored properties, init, and the start/stop/
uploadDroppedFiles entry points.

Access-level widening: every member of WorkspaceRemoteSessionController
(nested types, stored properties, and ~100 methods) was declared
private, which in Swift restricts visibility to the declaring file. To
split the class into extensions across files, all of these were widened
from private to the Swift default internal access level -- still
module-scoped, not exposed outside the app target. This is a blanket,
mechanical widening across the whole type rather than an itemized one
(the type's entire private surface needed it to support the split),
verified behavior-identical by a non-blank-line multiset diff against
the pre-split file before committing.

project.pbxproj updated: five new extension files registered (main app
target only).

Refs #98.
…ingsModels/SettingsComponents

ProgramaApp.swift was 5,854 lines. Pure file reorganization, no behavior
change intended:

- DebugWindows.swift: all Debug-menu window controllers + their SwiftUI
  views (Settings/About Titlebar Debug, Debug Window Controls, Browser
  Profile Popover Debug, Sidebar Debug, Menu Bar Extra Debug, Split Button
  Layout Debug, Background Debug), plus their supporting option/store types.
  None of these were actually under #if DEBUG at the type level in the
  original file (only the CommandMenu that surfaces them, and a couple of
  AppDelegate methods, are #if DEBUG-gated) — preserved exactly as-is, no
  #if DEBUG added or removed.

- SettingsModels.swift: the small @AppStorage-key settings-namespace enums
  (WorkspacePresentationModeSettings, PaneFirstClickFocusSettings,
  AppearanceMode/AppearanceSettings, QuitWarningSettings,
  ScrollbackPersistenceSettings, CommandPaletteRenameSelectionSettings,
  CommandPaletteSwitcherSearchSettings, ClaudeCodeIntegrationSettings,
  WelcomeSettings, PreferredEditorSettings).

- SettingsComponents.swift: the reusable settings design-system views
  (HexColorPicker, SettingsSectionHeader, SettingsCard, SettingsCardRow,
  SettingsPickerRow, SettingsCardDivider, SettingsCardNote,
  SettingsHeaderActionButton) plus their private View.applyIf helper.

- SettingsView.swift: the SettingsView struct itself (unchanged state/
  properties, no state moves), its SettingsView-specific helper views
  (ThemeWindowThumbnail, ThemePickerRow, ShortcutSettingRow,
  SettingsTopOffsetPreferenceKey, SettingsTitleLeadingInsetReader), and
  SettingsRootView which hosts it. Its 1,200-line body was decomposed into
  per-section @ViewBuilder computed properties matching the view's existing
  section headers (appSection, workspaceColorsSection,
  sidebarAppearanceSection, automationSection, customCommandsSection,
  browserSection, keyboardShortcutsSection, resetSection) — pure structural
  extraction, each section's content moved verbatim, no logic changes.

Access-level widening (private -> internal) applied only where required for
cross-file visibility introduced by the split, each because some other type
that stayed in ProgramaApp.swift (or moved to a different new file) still
references it:
- SettingsAboutWindowKind, SettingsAboutTitlebarDebugOptions,
  TitlebarVisibilityOption, TitlebarToolbarStyleOption,
  SettingsAboutTitlebarDebugStore: referenced by AboutWindowController /
  SettingsWindowController, which stayed in ProgramaApp.swift.
- The *DebugWindowController classes (SettingsAboutTitlebarDebug,
  DebugWindowControls, BrowserProfilePopoverDebug, SidebarDebug,
  MenuBarExtraDebug, SplitButtonLayoutDebug, BackgroundDebug): referenced by
  the Debug CommandMenu and openAllDebugWindows(), which stayed in
  ProgramaApp.swift.
- AboutVisualEffectBackground: stayed in ProgramaApp.swift (used by
  AboutPanelView) but is also used by SettingsView's title bar overlay,
  which moved to SettingsView.swift.
- SettingsRootView: hosted by SettingsWindowController.init in
  ProgramaApp.swift, moved to SettingsView.swift.
- openProgramaSettingsFileInTextEdit(): a free function in ProgramaApp.swift
  called from SettingsView's body, which moved to SettingsView.swift.
- The SettingsComponents.swift design-system views: used throughout
  SettingsView.swift, a different file after the split.

AboutWindowController, AcknowledgmentsWindowController, AcknowledgmentsView,
SettingsWindowController, SettingsNavigationTarget/Request, AboutPanelView,
AboutPropertyRow, AppIconAppearanceObserver, ProgramaUITestCapture, and
ProgramaRuntimeDebugCapture were left in ProgramaApp.swift — none matched
one of the four named destination buckets cleanly.

The @AppStorage-vs-resetAllSettings double enumeration did not fall out of
this split (resetAllSettings() still manually re-lists every @AppStorage
property); left as remaining work.

Refs #100.
…nfig and settings.json

GhosttyConfig.applySidebarAppearanceToUserDefaults() (driven by the legacy
~/.config/ghostty/config file) and ProgramaSettingsFileStore.applyManagedSettings
(driven by ~/.config/programa/settings.json) both write the sidebarTintHex,
sidebarTintHexLight, sidebarTintHexDark, and sidebarTintOpacity UserDefaults
keys, with no coordination between them.

Traced the current effective behavior and documented it in place at both
write sites rather than changing it:
- At app launch, settings.json applies first (ProgramaApp.init() forces
  ProgramaSettingsFileStore.shared to initialize synchronously before any
  SwiftUI view exists); GhosttyConfig applies afterward, when the first
  WorkspaceContentView's @State config is constructed.
- GhosttyConfig's write is sparse: it only touches a key when the parsed
  ghostty config file actually sets a value for it, otherwise leaving
  whatever settings.json (or a previous write) left in place.
- After launch, both sides are independently reactive (settings.json on its
  file watcher and .reload() call sites; GhosttyConfig on view lifecycle
  events and notifications), so the effective winner per key is whichever
  side fired most recently, not a fixed precedence.

Unifying this into one ordered apply path would mean coordinating two
independently triggered reactive systems, which is a design change, not a
file-reorg-safe edit — noting here instead per the escape hatch.

Refs #100.
…tate ownership

Move all 34 command-palette-only @State properties plus the 2 @AppStorage
settings (commandPaletteQuery, commandPaletteMode, search corpus/results,
rename/workspace-description drafts, focus-restore targets, usage history,
etc.) off ContentView and onto a new CommandPaletteController: ObservableObject,
as @Published/@AppStorage properties. ContentView keeps exactly one
`@StateObject private var commandPaletteController`.

ContentView's existing (unqualified) call sites — the ~4000 lines of palette
orchestration (registerCommandPaletteHandlers, commandPaletteCommandContributions,
rename/workspace-description flows, focus-restore, usage-history persistence,
the view body) — are preserved unchanged: each former @State name becomes a
thin computed proxy on ContentView that forwards get/set to the controller
(`private var commandPaletteQuery: String { get { commandPaletteController.
commandPaletteQuery } nonmutating set { ... } }`). This is a real ownership
transfer (the controller is the only place these values are stored; ContentView
holds no @State backing them anymore), done without touching the orchestration
method bodies, which keeps this change mechanically verifiable.

5 properties that were bound via SwiftUI's $name projection at their call
sites (commandPaletteQuery, commandPaletteRenameDraft,
commandPaletteWorkspaceDescriptionDraft, commandPaletteWorkspaceDescriptionHeight,
commandPaletteShouldFocusWorkspaceDescriptionEditor) get a matching
`nameBinding: Binding<T>` proxy instead, since computed properties can't
project a $-binding; call sites were updated to use .

The 2 @focusstate properties (isCommandPaletteSearchFocused,
isCommandPaletteRenameFocused) stay on ContentView — @focusstate is a SwiftUI
View-only property wrapper and cannot be hosted on an ObservableObject. This
is the actual reason the file's previous "CV1" extraction attempt called a
full split impossible; it wasn't, once the @Published-storage-plus-proxy
shape is used instead of moving @State verbatim.

Access-level widening (dropped `private`, no other behavior change) for
nested types now referenced from CommandPaletteController.swift (a different
file): CommandPaletteMode, CommandPaletteRestoreFocusTarget,
CommandPaletteTextSelectionBehavior, CommandPaletteMultilineTextEditorRepresentable,
CommandPaletteRenameTarget, CommandPaletteWorkspaceDescriptionTarget (enum-case
associated types must be at least as visible as the enum), and their AppKit
backing views CommandPaletteMultilineTextEditorView / CommandPaletteMultilineTextView
(NSViewRepresentable makeNSView/updateNSView must be at least as visible as
the representable struct).

The ~4000-line orchestration block and the ContentView+CommandPalette.swift
pure helpers were NOT physically relocated onto the controller in this pass —
they continue to compile and run correctly against the controller through the
proxies, but remain textually in ContentView.swift/ContentView+CommandPalette.swift.
Physically moving that method block is left for a follow-up; the state
ownership change (the core ask) is complete and verified.

Refs #88.
…ce.swift

Workspace.swift (7430 lines) held the Workspace class plus several
already-separate top-level extension blocks. Extracted three of those
existing seams into their own files, moving content verbatim:

- Workspace+Persistence.swift: session snapshot/restore
  (sessionSnapshot, restoreSessionSnapshot, and their layout/panel
  helpers) -- was a standalone `extension Workspace { ... }` block.
- Workspace+Layout.swift: programa.json custom layout application
  (applyCustomLayout and its tree/pane helpers) -- was a standalone
  `extension Workspace { ... }` block.
- Workspace+Bonsplit.swift: the `extension Workspace: BonsplitDelegate`
  conformance.

Workspace.swift drops from 7430 to 5626 lines; it now holds the class
declaration, its nested types, stored properties, and the methods that
don't yet have a dedicated extension file.

Access-level widening (required for cross-file visibility, since Swift's
`private` is file-scoped even across extensions of the same type):

- Blanket: every direct member of `final class Workspace` (nested types,
  stored properties, methods) was widened from `private` to the Swift
  default `internal`, mirroring the same treatment already applied to
  WorkspaceRemoteSessionController -- the class's entire private surface
  needed it to support extraction, so this was done as one mechanical
  pass rather than itemized per-symbol.
- Individually noted, since they were outside that blanket sweep:
  - `SessionPaneRestoreEntry` (a private top-level struct, not a class
    member) -- used by the extracted persistence helpers.
  - `panels`, `panelCustomTitles`, `pinnedPanelIds`, `manualUnreadPanelIds`,
    `tmuxLayoutSnapshot` -- `private(set)` published properties whose
    setters are called from Workspace+Bonsplit.swift; widened to plain
    internal-settable `@Published var`.
  - `manualUnreadClearDelayAfterFocusFlash` -- a `nonisolated private
    static let` read from Workspace+Bonsplit.swift.
  - `applyTabSelection`, `beginNonFocusSplitFocusReassert`,
    `clearNonFocusSplitFocusReassert`, `markExplicitFocusIntent`,
    `matchesPendingNonFocusSplitFocusReassert` -- private methods
    declared inside the BonsplitDelegate extension itself but called
    from the remaining Workspace.swift class body.

Verified behavior-identical by a non-blank-line multiset diff between
the pre-split file and the four post-split files (normalizing only the
access-modifier changes above); the only remaining differences were the
added file-header comments, the per-file import blocks every new Swift
file needs, and two now-dangling "// MARK:" section comments that were
dropped since the content they marked moved to dedicated files.

project.pbxproj updated: three new files registered (main app target
only).

The remaining seams called out in #98 for Workspace.swift -- theming,
remote-connection glue, surface creation, and focus/geometry -- are not
yet extracted; they live inside the still-5626-line class body rather
than as pre-existing standalone extension blocks, so pulling them out
safely needs the same per-symbol cross-reference verification done here,
at a larger scale. Left for a follow-up pass; noted in issuesPartial.

Refs #98.
@arzafran

arzafran commented Jul 9, 2026

Copy link
Copy Markdown
Member Author

Superseded by #116, which consolidates the seven remaining nuclear-review PRs into one CI run per maintainer request. Every commit from this branch is contained in #116 verbatim.

@arzafran arzafran closed this Jul 9, 2026
arzafran added a commit that referenced this pull request Jul 9, 2026
Nuclear review: consolidated fixes and splits (supersedes #106-#112)
@arzafran arzafran deleted the nr/tm branch July 9, 2026 22:00
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.

Evict the two squatters in TabManager: 1,340 lines of DEBUG harness and a 640-line stateless git/gh library

1 participant