refactor: typed lifecycle, single auth gate, imperative window factories#958
Merged
datlechin merged 32 commits intofeat/raycast-integrationfrom May 1, 2026
Merged
refactor: typed lifecycle, single auth gate, imperative window factories#958datlechin merged 32 commits intofeat/raycast-integrationfrom
datlechin merged 32 commits intofeat/raycast-integrationfrom
Conversation
# Conflicts: # CHANGELOG.md
- Restore PairingRequest/PairingExchange types deleted with DeeplinkHandler - Add reason: label to AuthDecision.denied call sites in MCPAuthPolicy - Convert PairingApproval.allowedConnectionIds to ConnectionAccess - Mark TransientConnectionFactory @mainactor for ConnectionURLParser access - Rename AppLaunchCoordinator.didFinishLaunching property to hasFinishedLaunching - Drop tables binding from MainContentView preview (SchemaService now owns it)
… auto-open
Welcome window was the first SwiftUI Scene declared, so SwiftUI auto-opened
it on every cold launch before AppLaunchCoordinator could check for pending
intents. The coordinator orderOut'd the window after URL events arrived, but
SwiftUI had already painted the welcome frame.
Move Welcome to imperative NSWindow + NSHostingController pattern, matching
how main windows are already created. AppLaunchCoordinator becomes the only
place that decides whether welcome appears.
- Add WelcomeWindowFactory with openOrFront / close / orderOut
- Drop Window("Welcome to TablePro", id: "welcome") Scene from TableProApp
- Drop openWelcomeWindow Notification.Name (no remaining posters)
- Drop AppDelegate.configureWelcomeWindowStyle (factory owns style)
- Drop AppDelegate.openWelcomeWindow private helper
- Replace 7 openWindow/closeWindows(id: "welcome") sites with factory calls
- Ignore .claude/worktrees, .profraw, scratch reference repos
After the Wave-2 lifecycle refactor, several Notification.Name declarations have no posters or no listeners. Delete them and rewire the only path that still needed cross-component messaging. - Drop .openWelcomeWindow (factory replaced it) - Drop .openMainWindow (no posters; WindowManager.openTab is direct) - Drop .switchSchemaFromURL (TabRouter calls coordinator directly) - Drop .applyURLFilter (method on coordinator, not a notification) - Drop .connectionShareFileOpened, .deeplinkImportRequested (WelcomeRouter pendingImport replaces them) - Drop OpenWindowHandler.onReceive(.openMainWindow) (dead listener) - Wire WelcomeRouter pendingSQLFiles to .databaseDidConnect → posts .openSQLFiles for drain
…UI Scene The connection-form WindowGroup was the only remaining declared Scene besides Settings, so SwiftUI auto-restored it on cold launch and produced the same flash bug the welcome refactor fixed. Apply the same imperative factory pattern to make the entire user-facing window lifecycle imperative and AppLaunchCoordinator-driven. - Add ConnectionFormWindowFactory (imperative open / close, per-id NSWindow) - Drop WindowGroup(id: "connection-form", for: UUID?.self) Scene - Replace 7 openWindow / closeWindows(id: "connection-form") sites - Replace OpenWindowHandler with SettingsNotificationBridge inside Settings (only forward .openSettingsWindow to SwiftUI's openSettings action) - Drop WindowOpener helper (no consumers after factory migration) - Drop @Environment(\\.openWindow) from ConnectionFormView and WelcomeWindowView - Drop WelcomeViewModel.setUp(openWindow:) signature; setUp() takes no args After this commit Settings is the only declared Scene, which is special: SwiftUI never auto-opens Settings on launch.
…y-open table Native macOS pattern: opening the same target twice focuses the existing window/tab, it does not create duplicates (Finder folders, Xcode files). Before opening a new native window tab from a deeplink, scan all active coordinators for a tab matching (connectionId, tabType: .table, tableName, optional database, optional schema). If found, select that tab and bring its window to front; skip openTab. If not found, fall through to creating a new tab as before.
…plies Connection switcher built EditorTabPayload(connectionId:) directly with the type defaults (tabType: .query, intent: .openContent). That always forced a fresh empty query tab instead of restoring the user's last tabs or opening the connection's default tab. Single path: both switchToSession and connectToSaved now call TabRouter.route(.openConnection(id)). TabRouter focuses an existing window for the connection if any; otherwise creates a new window with intent: .restoreOrDefault and ensures the connection is live. Drop the manual NSWindow.tabbingMode = .disallowed workaround — per- connection tabbingIdentifier in WindowManager.openTab already prevents cross-connection tab merging when groupAllConnectionTabs is off. Drop the now-unused isConnecting per-row spinner state — popover dismisses synchronously now, so the spinner had no chance to render.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf377c9b0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+331
to
+333
| if PluginManager.shared.supportsSchemaSwitching(for: coordinator.connection.type) { | ||
| await coordinator.switchSchema(to: target) | ||
| } else { |
There was a problem hiding this comment.
This helper treats every target as a schema whenever the plugin supports schema switching, but openTable passes database names here for /database/.../table/... intents. On schema-capable engines (for example PostgreSQL), database-only deep links will call switchSchema(to:) with the database name instead of switching databases, so the session can stay on the wrong database and fail to open the requested table.
Useful? React with 👍 / 👎.
Comment on lines
+245
to
+250
| try await session.transition(to: .active( | ||
| tokenId: authenticatedToken?.id, | ||
| tokenName: authenticatedToken?.name | ||
| )) | ||
| } catch { | ||
| return encodeError(MCPError.invalidRequest("Cannot initialize session in current phase"), id: request.id) |
There was a problem hiding this comment.
The new phase transition check turns repeated notifications/initialized calls into an error response. Since this method is a notification (no response expected) and clients may resend it during retries/replays, rejecting duplicates can break otherwise valid sessions after they are already active. This path should accept no-op repeats instead of emitting a JSON-RPC error.
Useful? React with 👍 / 👎.
- MCPAuthPolicy.promptApproval: defer group.cancelAll() so timeout task cannot leak when the dialog task throws on a future code change - TabRouter.focusExistingTableTab: tighten matching so a deeplink that specifies a database does not match a tab whose databaseName is nil; similarly for schema - AlertHelper.swift: strip /// doc comments and inline // comments per CLAUDE.md "no comments" rule (function names are self-descriptive) - AppDelegate: drop configureConnectionFormWindowStyle, configuredWindows tracking, windowDidBecomeKey observer — ConnectionFormWindowFactory applies style at creation time, no post-hoc restyle needed - MCPConnectionBridge: drop unused connectDedup property and route connect/connectIfNeeded through DatabaseManager.ensureConnected so dedup lives in one place (manager-level OnceTask) - Delete dead MCPPendingPairing/MCPPairingRegistry — pairing service uses PairingExchangeStore + NSLock; the actor-based duplicate from Wave 2B was never wired up
…rsisted state handleRestoreOrDefault left the new window with zero tabs when restoreFromDisk returned an empty result (first-time connection, persistence cleared, etc.). Tab manager being empty causes updateWindowTitleAndFileState's fallback branch to use connection.name as the window title, so the user sees an empty "<Connection>" tab next to any subsequent tab opened from a deeplink or Cmd+T. Hoist the "add default empty query tab" logic to a helper and run it in both paths: when other windows already exist for the connection, and when restoreFromDisk returns empty. Window now always has at least one tab.
QueryTabManager.addTab defaulted the new tab title to nextQueryTitle computed against this manager's local tabs. Each window has its own tab manager, so a fresh window numbering its first tab always returned "Query 1" — even if a sibling window already had Query 1, Query 2. Inject globalTabsProvider into QueryTabManager. SessionStateFactory supplies a closure that returns MainContentCoordinator.allTabs(for: connectionId), so the title generator sees every existing query tab in the connection's tab group, not just this window's. The closure is lazy (called only when title is computed) so manager init stays decoupled from the registry's MainActor isolation at use time. The default empty closure keeps the type usable in tests and preserves a sensible per-window fallback when no provider is wired.
…ery tab)
Reverts the addDefaultEmptyTab fallback in handleRestoreOrDefault. Opening a
fresh connection no longer creates a default "Query 1" tab — the window
shows the existing emptyStateView placeholder ("No tabs open" + Cmd+T /
click-a-table hint) instead.
Native macOS pattern: opening a workspace shouldn't pre-fill content. The
user explicitly creates content via Cmd+T, sidebar table click, Raycast
deeplink, etc. The empty-tabs path is a supported state with proper UI.
…st-path, drop fragile string match, fix openDatabaseURL double-open
…ments, type session-term reason, dedup preview tab, fix Cmd+1..9 guard, fix denial string format
…calate save errors
…ecycle leak fixes
Critical: - MCPServer.stop() runs cleanup handlers before clearing sessions so future handlers can read session data High: - TabRouter.focusExistingTableTab assigns tabManager.selectedTabId directly instead of delegating to coordinator.openTableTab — the latter has its own create-new-tab fast-path that produced duplicate tabs - SessionStateFactory tracks pending-expiration tasks per payload and cancels on consume; eliminates 5s dormant Task per successful window open - OnceTask.execute uses a generation counter so the defer cleanup only removes the entry when it still matches the caller's task; cancel + re-execute no longer evicts a sibling caller's in-flight task - SchemaService.load now joins the in-flight load when state is .loading instead of returning empty-handed; callers can await completion Medium: - Remove unused TabPersistenceCoordinator.saveNow(persistedTabs:) overload that lacked an empty-array guard - TabRouter.previewForSQL uses NSString.substring(to:) for O(1) head slice consistent with the project's NSString convention - WelcomeViewModel.awaitWelcomeRouterChange wraps the continuation in a ContinuationBox + withTaskCancellationHandler so deinit cancellation resolves cleanly instead of leaving a dormant Task Low: - Strip explanatory /// and inline comments from WindowManager, SessionStateFactory, TabDiskActor, TabPersistenceCoordinator, MCPServer per CLAUDE.md "no comments" rule
datlechin
added a commit
that referenced
this pull request
May 1, 2026
* feat(integrations): UUID-keyed deep links and external access foundation * feat(integrations): per-connection external access setting UI * docs(integrations): external API documentation and CHANGELOG * feat(integrations): tablepro-mcp stdio CLI binary Renames the existing mcp-server target's product to tablepro-mcp and moves source from TablePro/MCPBridge to TablePro/CLI per plan. Adds auto-launch of TablePro via tablepro://integrations/start-mcp with a 10-second poll for the handshake file, plus stale-PID detection that relaunches the host. Bridges SSE responses by emitting each data: line as a separate JSON-RPC message on stdout. Adds an explicit TablePro -> mcp-server target dependency so the helper binary always builds before the app embed step. Sets PRODUCT_BUNDLE_IDENTIFIER=com.TablePro.tablepro-mcp. Verification: xcodebuild -scheme mcp-server -configuration Debug build succeeds; tablepro-mcp ends up at TablePro.app/Contents/MacOS/tablepro-mcp, executable, codesigned, hardened runtime on. swiftlint --strict clean on TablePro/CLI/. build-release.sh now fails fast if the embedded helper is missing from the release bundle. The internal Xcode target name stays "mcp-server" to keep the pbxproj diff minimal; only the product name, bundle id, and source path change. * feat(integrations): pairing flow with PKCE-flavored exchange * feat(integrations): Raycast extension * feat(integrations): audit log storage and new MCP tools * docs: fix mintlify parse errors in plugin-registry and ai-assistant * docs(integrations): align with shipped contract * feat(extension): align wire shapes with shipped contract * docs: fold tablepro://import params into external-api/url-scheme Move the full parameter list (core, SSH, SSL, plugin-specific af_* fields) from features/deep-links into external-api/url-scheme. The url-scheme page previously punted to deep-links via 'See Deep Links for the full parameter list', which becomes a dangling link once deep-links is removed. * docs: rewrite features/mcp as orientation page that links to external-api The previous page duplicated the External API tool catalog, token model, and stdio bridge config in less detail. It also hardcoded a stale default port (23508) and the internal Xcode target name (mcp-server) instead of the shipped binary name (tablepro-mcp). Replace with a focused page that covers the in-app Settings > MCP UI: enable toggle, lazy-start, remote access, TLS, audit log. Link to the canonical external-api pages for protocol details, the tool catalog, tokens, pairing, and client setup. Section anchors #enabling-the-server and #remote-access are preserved so existing cross-references from customization/settings.mdx and external-api/cursor-claude.mdx still resolve. * docs: remove stale features/deep-links page, update nav and cards The page documented the pre-0.37 'tablepro://connect/<name>' syntax, which contradicts external-api/url-scheme.mdx (UUID-keyed paths only). Database URL schemes (mysql://, postgresql://, etc.) are already canonical in databases/connection-urls.mdx, and the tablepro://import parameter table now lives in external-api/url-scheme.mdx. - Delete docs/features/deep-links.mdx - Update features/overview.mdx Workflow card to point to external-api/url-scheme - Remove features/deep-links from docs.json Workflow group * docs: rename cursor-claude to mcp-clients with generic guide and 9 client sections * docs: surface External API in landing page and features overview * docs: add External Access and external client notes to connection-related pages * docs: cross-link External API from tabs, query history, and keyboard shortcuts * docs: tighten MCP page lazy-start description and update settings reference * feat(settings): rename MCP tab to Integrations * fix(integrations): filter blocked connections from list_connections response * feat(integrations): add date filter params to search_query_history tool * docs(integrations): align mcp-resources shapes with implementation * feat(extension): set Raycast author to ngoquocdat and update callback URLs * fix(extension): read connections.json from TablePro support dir not com.TablePro * refactor: unify MCP files into TablePro support dir, drop com.TablePro path The MCP subsystem wrote its handshake, tokens, and audit DB under ~/Library/Application Support/com.TablePro/, while every other on-disk file (connections.json, TabState/, LastQuery/, known_hosts, themes) lives under ~/Library/Application Support/TablePro/. Bundle-id paths are the sandboxed-app convention; TablePro is non-sandboxed and should use the friendly app-name directory throughout. Updates the three MCP path constructions, the bundled tablepro-mcp bridge, the Raycast extension, and every docs reference. The com.TablePro identifier is preserved in OSLog subsystems, Keychain, UserDefaults keys, the bundle id, AppGroup ids, and NSUserActivity types since those are not file paths. No migration code or compat shim per CLAUDE.md "no backward-compat hacks". After upgrading, users must re-pair Raycast, Cursor, Claude Desktop, and any other external MCP clients, and may delete the stale directory with `rm -rf ~/Library/Application Support/com.TablePro`. * docs: drop Past breaking changes table from versioning page * fix(extension): drop redundant TablePro subtitles from commands * fix(extension): polish action titles, empty states, and pair form for Raycast Store * refactor(extension): replace useState/useEffect data loading with useCachedPromise * refactor(extension): adopt useForm and add validation to pair flow Also log previously swallowed best-effort failures from updateCommandMetadata and the post-pair redirect so they surface in dev. The setInterval polling in WaitingView remains, since LocalStorage cannot signal cross-command writes. * chore(extension): add discovery keywords and use Action.Open variants in empty states * chore: remove extensions/ — Raycast extension lives in raycast/extensions repo The Raycast extension was developed in extensions/tablepro/ for convenience but its canonical source belongs in github.com/raycast/extensions per Raycast's distribution model. After this branch ships, the extension will be PR'd separately to that repo. Local copy preserved at ~/Projects/raycast-tablepro-staging/tablepro for the upcoming raycast/extensions PR. * fix(integrations): use Raycast launchContext format for raycast:// redirect * fix(mcp): drop blocked-connection results from list_recent_tabs and search_query_history * fix(mcp): enforce externalAccess readOnly on state-mutating tools * fix(mcp): push search_query_history allowlist into SQL to avoid post-LIMIT trim * docs: drop em dashes from pairing redirect format section * chore: fix lint errors after rebase onto main * fix(mcp): run auth checks before raising window in focus_query_tab * fix(mcp): validate and gate export_data table names against SQL injection * fix(mcp): allow readOnly tokens to navigate window/tab/focus tools * fix(mcp): filter list_connections by token allowed connection ids * fix(mcp): refuse focus_query_tab on tabs without a connection * fix(mcp): restrict export_data output path to Downloads directory * refactor(mcp): rename history allowlist var to clarify token scope * style(mcp): remove function comment in MCPAuthGuard * test(mcp): add export_data table injection regression tests * fix(mcp): treat empty allowedConnectionIds set as no-access in list_connections * fix(deeplink): support schema-qualified table URL path * fix(mcp): handle OPTIONS preflight on integrations exchange endpoint * fix(mcp): guard pairing approval continuation against double resume * fix(deeplink): use restoreOrDefault intent for connect URL to avoid forcing query tab * fix(deeplink): suppress welcome window when handling tablepro URL on cold launch * fix(launch): skip auto-reconnect when a deeplink is being handled on cold launch * fix(launch): do not close restored windows when handling deeplink (race destroyed connection window) * fix(mcp): dedupe concurrent ai-access approval prompts via in-flight task * fix(sidebar): mark sidebar loaded when schema already loaded by sibling window * fix(sidebar): render table list when tables binding has data, regardless of loading state machine * fix(deeplink): skip placeholder window when deeplink targets specific tab content * fix(sidebar): hydrate session.tables from cached schema provider when skipping reload * fix(schema): hydrate session.tables from provider after loadSchema fetch * fix(deeplink): route openTable into existing window's tab manager instead of new native window * fix(mcp): auto-connect session when tool requires driver * fix(mcp): activate app before auto-connect so any prompt has a key window * fix(mcp): dedupe concurrent auto-connect tasks for same connection * fix(mcp): allow tool meta resolution from disk so auto-connect can run later * fix(mcp): use defer for inFlightConnects cleanup * refactor: typed lifecycle, single auth gate, imperative window factories (#958) * fix(mcp): app-modal auth alert, strip trailing semicolons, decode + as space * refactor(ui): centralize cross-process alerts in AlertHelper * refactor(concurrency): introduce OnceTask, migrate MCP dedup sites * refactor(mcp): replace router switch with route registry * refactor(schema): single SchemaService for sidebar state and table list (WIP) * refactor(mcp): MCPAuthPolicy gate, typed session/pairing lifecycles (WIP) * refactor(lifecycle): typed launch coordinator, grammar parser, tab router (WIP) * fix(refactor): post-merge build fixes for Wave 2 integration - Restore PairingRequest/PairingExchange types deleted with DeeplinkHandler - Add reason: label to AuthDecision.denied call sites in MCPAuthPolicy - Convert PairingApproval.allowedConnectionIds to ConnectionAccess - Mark TransientConnectionFactory @mainactor for ConnectionURLParser access - Rename AppLaunchCoordinator.didFinishLaunching property to hasFinishedLaunching - Drop tables binding from MainContentView preview (SchemaService now owns it) * refactor(welcome): imperative NSWindow factory replaces SwiftUI Scene auto-open Welcome window was the first SwiftUI Scene declared, so SwiftUI auto-opened it on every cold launch before AppLaunchCoordinator could check for pending intents. The coordinator orderOut'd the window after URL events arrived, but SwiftUI had already painted the welcome frame. Move Welcome to imperative NSWindow + NSHostingController pattern, matching how main windows are already created. AppLaunchCoordinator becomes the only place that decides whether welcome appears. - Add WelcomeWindowFactory with openOrFront / close / orderOut - Drop Window("Welcome to TablePro", id: "welcome") Scene from TableProApp - Drop openWelcomeWindow Notification.Name (no remaining posters) - Drop AppDelegate.configureWelcomeWindowStyle (factory owns style) - Drop AppDelegate.openWelcomeWindow private helper - Replace 7 openWindow/closeWindows(id: "welcome") sites with factory calls - Ignore .claude/worktrees, .profraw, scratch reference repos * chore(refactor): drop dead notifications, wire SQL file drain After the Wave-2 lifecycle refactor, several Notification.Name declarations have no posters or no listeners. Delete them and rewire the only path that still needed cross-component messaging. - Drop .openWelcomeWindow (factory replaced it) - Drop .openMainWindow (no posters; WindowManager.openTab is direct) - Drop .switchSchemaFromURL (TabRouter calls coordinator directly) - Drop .applyURLFilter (method on coordinator, not a notification) - Drop .connectionShareFileOpened, .deeplinkImportRequested (WelcomeRouter pendingImport replaces them) - Drop OpenWindowHandler.onReceive(.openMainWindow) (dead listener) - Wire WelcomeRouter pendingSQLFiles to .databaseDidConnect → posts .openSQLFiles for drain * refactor(connection-form): imperative NSWindow factory replaces SwiftUI Scene The connection-form WindowGroup was the only remaining declared Scene besides Settings, so SwiftUI auto-restored it on cold launch and produced the same flash bug the welcome refactor fixed. Apply the same imperative factory pattern to make the entire user-facing window lifecycle imperative and AppLaunchCoordinator-driven. - Add ConnectionFormWindowFactory (imperative open / close, per-id NSWindow) - Drop WindowGroup(id: "connection-form", for: UUID?.self) Scene - Replace 7 openWindow / closeWindows(id: "connection-form") sites - Replace OpenWindowHandler with SettingsNotificationBridge inside Settings (only forward .openSettingsWindow to SwiftUI's openSettings action) - Drop WindowOpener helper (no consumers after factory migration) - Drop @Environment(\\.openWindow) from ConnectionFormView and WelcomeWindowView - Drop WelcomeViewModel.setUp(openWindow:) signature; setUp() takes no args After this commit Settings is the only declared Scene, which is special: SwiftUI never auto-opens Settings on launch. * fix(tabrouter): focus existing table tab when deeplink targets already-open table Native macOS pattern: opening the same target twice focuses the existing window/tab, it does not create duplicates (Finder folders, Xcode files). Before opening a new native window tab from a deeplink, scan all active coordinators for a tab matching (connectionId, tabType: .table, tableName, optional database, optional schema). If found, select that tab and bring its window to front; skip openTab. If not found, fall through to creating a new tab as before. * fix(switcher): route through TabRouter so .restoreOrDefault intent applies Connection switcher built EditorTabPayload(connectionId:) directly with the type defaults (tabType: .query, intent: .openContent). That always forced a fresh empty query tab instead of restoring the user's last tabs or opening the connection's default tab. Single path: both switchToSession and connectToSaved now call TabRouter.route(.openConnection(id)). TabRouter focuses an existing window for the connection if any; otherwise creates a new window with intent: .restoreOrDefault and ensures the connection is live. Drop the manual NSWindow.tabbingMode = .disallowed workaround — per- connection tabbingIdentifier in WindowManager.openTab already prevents cross-connection tab merging when groupAllConnectionTabs is off. Drop the now-unused isConnecting per-row spinner state — popover dismisses synchronously now, so the spinner had no chance to render. * Update .gitignore * refactor(review): address PR #958 reviewer findings - MCPAuthPolicy.promptApproval: defer group.cancelAll() so timeout task cannot leak when the dialog task throws on a future code change - TabRouter.focusExistingTableTab: tighten matching so a deeplink that specifies a database does not match a tab whose databaseName is nil; similarly for schema - AlertHelper.swift: strip /// doc comments and inline // comments per CLAUDE.md "no comments" rule (function names are self-descriptive) - AppDelegate: drop configureConnectionFormWindowStyle, configuredWindows tracking, windowDidBecomeKey observer — ConnectionFormWindowFactory applies style at creation time, no post-hoc restyle needed - MCPConnectionBridge: drop unused connectDedup property and route connect/connectIfNeeded through DatabaseManager.ensureConnected so dedup lives in one place (manager-level OnceTask) - Delete dead MCPPendingPairing/MCPPairingRegistry — pairing service uses PairingExchangeStore + NSLock; the actor-based duplicate from Wave 2B was never wired up * fix(setup): always create default tab when restoreOrDefault has no persisted state handleRestoreOrDefault left the new window with zero tabs when restoreFromDisk returned an empty result (first-time connection, persistence cleared, etc.). Tab manager being empty causes updateWindowTitleAndFileState's fallback branch to use connection.name as the window title, so the user sees an empty "<Connection>" tab next to any subsequent tab opened from a deeplink or Cmd+T. Hoist the "add default empty query tab" logic to a helper and run it in both paths: when other windows already exist for the connection, and when restoreFromDisk returns empty. Window now always has at least one tab. * fix(tabs): number new query tabs across all windows for the connection QueryTabManager.addTab defaulted the new tab title to nextQueryTitle computed against this manager's local tabs. Each window has its own tab manager, so a fresh window numbering its first tab always returned "Query 1" — even if a sibling window already had Query 1, Query 2. Inject globalTabsProvider into QueryTabManager. SessionStateFactory supplies a closure that returns MainContentCoordinator.allTabs(for: connectionId), so the title generator sees every existing query tab in the connection's tab group, not just this window's. The closure is lazy (called only when title is computed) so manager init stays decoupled from the registry's MainActor isolation at use time. The default empty closure keeps the type usable in tests and preserves a sensible per-window fallback when no provider is wired. * fix(setup): empty workspace as default for new connection (no auto query tab) Reverts the addDefaultEmptyTab fallback in handleRestoreOrDefault. Opening a fresh connection no longer creates a default "Query 1" tab — the window shows the existing emptyStateView placeholder ("No tabs open" + Cmd+T / click-a-table hint) instead. Native macOS pattern: opening a workspace shouldn't pre-fill content. The user explicitly creates content via Cmd+T, sidebar table click, Raycast deeplink, etc. The empty-tabs path is a supported state with proper UI. * refactor(tab-router): add openQuery dedup, route openTable through fast-path, drop fragile string match, fix openDatabaseURL double-open * refactor(polish): fix safeMode dialog window race, strip Sessions comments, type session-term reason, dedup preview tab, fix Cmd+1..9 guard, fix denial string format * refactor(routing): unify connection-open paths through TabRouter * refactor(persistence): track save Task, clear empty-payload files, escalate save errors * refactor(state): cross-window correctness + lifecycle leak fixes * refactor(round2-review): fix 9 PR #958 review findings Critical: - MCPServer.stop() runs cleanup handlers before clearing sessions so future handlers can read session data High: - TabRouter.focusExistingTableTab assigns tabManager.selectedTabId directly instead of delegating to coordinator.openTableTab — the latter has its own create-new-tab fast-path that produced duplicate tabs - SessionStateFactory tracks pending-expiration tasks per payload and cancels on consume; eliminates 5s dormant Task per successful window open - OnceTask.execute uses a generation counter so the defer cleanup only removes the entry when it still matches the caller's task; cancel + re-execute no longer evicts a sibling caller's in-flight task - SchemaService.load now joins the in-flight load when state is .loading instead of returning empty-handed; callers can await completion Medium: - Remove unused TabPersistenceCoordinator.saveNow(persistedTabs:) overload that lacked an empty-array guard - TabRouter.previewForSQL uses NSString.substring(to:) for O(1) head slice consistent with the project's NSString convention - WelcomeViewModel.awaitWelcomeRouterChange wraps the continuation in a ContinuationBox + withTaskCancellationHandler so deinit cancellation resolves cleanly instead of leaving a dormant Task Low: - Strip explanatory /// and inline comments from WindowManager, SessionStateFactory, TabDiskActor, TabPersistenceCoordinator, MCPServer per CLAUDE.md "no comments" rule * fix: register custom file UTIs as exported so Finder enables Open With * style: strip MARK comments from MCP source and test files * fix(mcp): tighten identifier validation, focus_query_tab TOCTOU, and pairing pending cap * feat(integrations): extract shared status indicator, copyable code block, client enum * fix(integrations): pairing approval default action and expiry countdown * fix(integrations): require confirmation for token delete and disconnect * refactor(integrations): lift activity log out of form, add search and export * chore(integrations): localization sweep, rename stale MCP user-facing strings * test(integrations): cover status indicator accessibility and document changes * fix(integrations): use minWidth on sheet frames, import Combine for Timer.publish * fix(integrations): replace nested TabView with segmented picker to avoid Settings toolbar bleed * fix(integrations): activity log search no longer bleeds into Settings toolbar, friendlier time and token labels * fix: restore Libs/checksums.sha256 accidentally replaced by build-time symlink * fix(integrations): center activity log empty state, distinct message for empty search * fix(integrations): force empty state to claim full available height with explicit spacers
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Architectural refactor of MCP / deeplink / cold-launch / alert / schema-loading layers. Origin: 5 audit reports surfaced ~50 anti-patterns from the 20 hack-fix commits accumulated during Raycast integration debugging. This PR replaces the implicit flag-based machinery with typed coordinators, single-source-of-truth services, and consistent native macOS patterns.
What changed
Foundation primitives
OnceTask<Key, Value>actor — replaces 5 hand-rolled[Key: Task]dedup patterns (auth approval, auto-connect, URL connect dedup, etc.)AlertHelper.runApprovalModal/runPairingApproval— single cross-process auth-prompt primitive (app-modalNSAlert.runModal())MCPRouteregistry +MCPRouteHandlerprotocol — replaces 600-line switch inMCPRouter; generic CORS preflight at the routerState management
SchemaServiceactor — single source of truth for schema loading state, keyed per-connection. Replaces per-coordinatorSQLSchemaProvider+sidebarLoadingState+ healing methodsMCPAuthPolicy— single authorization gate (token tier × connection access × external access × tool sensitivity). DeletesMCPAuthGuardMCPSessionPhase+MCPPendingPairing— typed lifecycle replaces flag-based session/pairing stateDatabaseManager.connectionState(_:)— single ConnectionState enum used by 3 sites that previously each derived state independentlyLifecycle / routing
AppLaunchCoordinator+LaunchPhasemachine — replaces 6 boolean flags (isHandlingFileOpen,fileOpenSuppressionCount, etc.) and the cold-launch race between auto-reconnect, URL handler, and welcome windowLaunchIntentenum — typed handoff for every URL/file/deeplink typeDeeplinkParser— pure grammar-driven URL parser, replaces magic-numbercomponents.count == 7checksTabRouter— owns window/tab decision; deeplinks always open new tab in connection's group, focuses existing tab if target already openWelcomeRouter— replacesPendingActionStoresingletonWindow lifecycle (this commit set)
WelcomeWindowFactoryandConnectionFormWindowFactory— imperativeNSWindow + NSHostingController. Settings is the only declared SwiftUI Scene now (which is special and never auto-opens).AppLaunchCoordinatorcould route the deeplink.Files deleted
TablePro/AppDelegate+FileOpen.swift(354 lines) →AppLaunchCoordinator+DeeplinkParser+LaunchIntentRouterTablePro/AppDelegate+ConnectionHandler.swift(589 lines) →TabRouterTablePro/AppDelegate+WindowConfig.swift(406 lines) →AppLaunchCoordinatorTablePro/Core/Services/Infrastructure/DeeplinkHandler.swift(301 lines) →DeeplinkParserTablePro/Core/Services/Infrastructure/PendingActionStore.swift→WelcomeRouterTablePro/Core/MCP/MCPAuthGuard.swift(250 lines) →MCPAuthPolicyTablePro/Core/Services/Infrastructure/WindowOpener.swift→ no consumers after factory migrationTablePro/Views/Settings/Sections/PairingApprovalSheet.swift::PairingApprovalPresenterenum →AlertHelper.runPairingApprovalPlus 6 dead
Notification.Nameentries (.openWelcomeWindow,.openMainWindow,.switchSchemaFromURL,.applyURLFilter,.connectionShareFileOpened,.deeplinkImportRequested).Manual test plan
tablepro://connect/<uuid>/table/<name>with app quit) — opens straight to table tab, no welcome flash, no connection-form flash.restoreOrDefaultintent (not forced query tab)SchemaService), no secondfetchTables().sqlfile from Finder — drains viaWelcomeRouterafter first connectNotes
feat/raycast-integration→refactor/foundation(this branch) via 18 commits across 6 wave merges plus post-merge fixes. Squash recommended on merge to keepfeat/raycast-integrationhistory clean.OnceTaskTests,MCPRouterTests, partial coverage forSchemaService/MCPAuthPolicy/MCPSessionPhase/MCPPendingPairing/DeeplinkParser.