Skip to content

feat(ai-chat): agent workspace phases 1 to 3, assistant mode, approval floor, session isolation - #2384

Open
J2TeamNNL wants to merge 41 commits into
TableProApp:mainfrom
J2TeamNNL:feat/agent-workspace
Open

feat(ai-chat): agent workspace phases 1 to 3, assistant mode, approval floor, session isolation#2384
J2TeamNNL wants to merge 41 commits into
TableProApp:mainfrom
J2TeamNNL:feat/agent-workspace

Conversation

@J2TeamNNL

@J2TeamNNL J2TeamNNL commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

All seven phases of the agent workspace plan. The assistant gets the whole window, every write it proposes waits for a human, sessions are isolated and run in parallel, the pane beside the conversation shows what the session actually did, the welcome window is a second way in, and a session can call an MCP server that is not TablePro.

What is here

Phase Commit What it does
1 f67878136 Assistant mode in one window: a Browse/Assistant toolbar control swaps rootView on the three panes WorkspacePanes already owns, rather than nesting a split view
2 965fc20d6 Approval floor: Assistant mode holds a connection at Confirm Writes, approvals are keyed ApprovalRequestID(sessionId:toolUseId:), and a call is evaluated against the connection it targets
3 a9a35ad7e One session isolated: per-session tool mode, transcript, provider lease and tool scope
4 3001d2693 Sessions live in AgentSessionRegistry, not in a window
5 b2435e4cc The result pane: proposed SQL, steps, results, schema changes
6 183c9093f The welcome window starts or reopens a session
7 be5b966c6 Outside MCP servers as tool sources, under approval and audit

Phase 4, sessions are not owned by a window

The four process-global singletons phase 3 split up were still reached through a field on the window's right panel, so a session's lifetime was the window's. AgentSessionRegistry holds them instead.

  • RightPanelState.aiViewModel was a creating getter read from inside SwiftUI bodies. It is now session, a read, plus startSession(), which only a user action or an explicit .task calls. Opening a connection creates no session.
  • teardown() used to call clearSessionData(), which emptied messages. Window close, disconnect and session loss all reach that path, so a transcript the user never asked to lose was gone from three ordinary places. It now stops the session: cancel, persist the partial turn, mark stopped, and release only the derived context a reopened session rebuilds.
  • Status is stored on the session, not computed where it is read: two of its inputs (ToolApprovalCenter, ProviderStreamLease) are outside the observation graph, so a rail row that asked them a question would render once and never update.
  • applicationWillTerminate persists every session and marks a working one failed. Nothing persisted AI state at quit before, so a session killed mid-stream came back with its last turn missing. The transcript write needed a synchronous path (AIChatStorage.saveSync) because an actor hop at terminate may never be scheduled.
  • Restore is lazy: a session's turns are pulled in by conversation id when it is opened, not at launch, because reading them all would be quadratic in the number of sessions.

Phase 5, the result pane

AgentArtifactProjection is a pure function over the session's own ChatTurn history rather than a second observable store. That is what makes a restored session's pane correct with no replay: the transcript is what was restored, and there is one record of "waiting" instead of two that can disagree.

Two things had to change underneath it:

  • Out-of-order approval. resolveAndAwaitApprovals appended every pending block at once but awaited them one at a time, so only the first had a continuation registered: a click on the third card hit resolve's missing-continuation guard and did nothing while the stream stayed parked on the first. Every waiting call now registers up front, and ToolApprovalCenter also buffers a decision that arrives before its turn.
  • Three default actions. Every approval row carried .keyboardShortcut(.defaultAction), so Return fired whichever button AppKit reached first. Only the first row still waiting takes it, resolved from the transcript through \.chatPrimaryPendingToolUseId.

ExplainQueryChatTool wraps the server-side explain tool per decision 5, deliberately without its analyze parameter: a .readOnly chat tool is auto-approved, and analyze runs the statement for real. DDLChangeReader is certainty-or-raw-SQL, and it does not reuse QueryClassifier.strippingStringLiterals because that treats a backticked identifier as a literal and removes it, which would lose the only object DROP TABLE `order items` names.

Phase 7, outside MCP servers

Answered the two blocking questions as the plan recommended: per connection and HTTP-only.

  • Namespace is ext__<serverUUID>__, keyed on the id rather than the name, and tablepro, table-pro, table_pro are reserved slugs. A server the user called "TablePro" would otherwise land inside ClaudeAgentProvider's pre-approved mcp__tablepro__* wildcard.
  • Allowlist is per connection and consulted on resolution as well as on listing, so a model that saw a tool in an earlier turn cannot call it by name from a connection that does not allow it.
  • computeInitialApprovalState forces every remote call to .pending, checked ahead of the .readOnly shortcut, in every chat mode, whatever aiAlwaysAllowedTools holds. "Read-only" is the server's claim about itself.
  • The audit digest is versioned before any field was added to it. It hashes an ordered array, so appending would have reported every existing row as tampered; v1 rows verify under the frozen v1 list and v2 rows under the v2 one, tested in one database.
  • Each call is recorded before the request leaves, with the payload's SHA-256 and byte count and none of its contents.
  • A call carries a 30-second deadline of its own, so a server that never answers fails the call instead of parking the chat stream behind URLSession's timeout.
  • Non-loopback endpoints must be https.

Bugs found and fixed along the way

Each of these was pre-existing, not introduced here:

  • Closing a window, disconnecting, or losing a session erased that connection's chat transcript.
  • The last turn of a chat was lost when the app quit mid-reply.
  • A session holding a conversation id it had not listed took the new-conversation branch on save, orphaning the transcript the user was reading and starting a second one beside it.
  • Explain with AI and Fix Error did nothing until the chat panel had been opened once: the coordinator's aiViewModel was a weak snapshot taken in onAppear, before a session existed.
  • Approving any tool call but the first in a turn did nothing.
  • Return acted on whichever approval button AppKit reached first.
  • VoiceOver read the Browse/Assistant control as "tablecells" and "sparkles": an expanded toolbar group takes each segment's name from the image's accessibilityDescription, which was nil.
  • docs/scripts/check-writing-style.sh failed under a C locale, because its bracket expressions over non-ASCII glyphs match individual bytes there and every glyph it checks starts 0xE2. It reported every ellipsis in the corpus as a modifier glyph.
  • ImportFromAppSourcePicker had a legacy_swiftui_aspect_ratio violation on main.

Verification

Merged upstream/main (8 commits, the Compare & Sync and routines work) into the branch; the only conflict was CHANGELOG.md, resolved into one [Unreleased] block in canonical section order. ** BUILD SUCCEEDED ** after the merge. swiftlint lint --strict clean over TablePro TableProTests TableProUITests (5,187 files). docs/scripts/check-writing-style.sh and docs/scripts/check-docs-against-source.py both pass.

New suites, all green: AgentSessionRegistryTests (13), AgentSessionStatusTests (13), AgentSessionPendingPromptTests (6), AIChatPersistenceTests (5), AgentArtifactProjectionTests (15), DDLChangeReaderTests (14), ToolApprovalCenterOrderingTests (6), AgentLaunchRoutingTests (4), MCPServerConfigurationTests (8), MCPRemoteToolPolicyTests (12), MCPRemoteToolApprovalTests (5), MCPAuditChainVersioningTests (6). ConnectionWindowPaneResolverTests extended with the mode matrix.

CI

Unit tests, Package Tests, Validate docs, Lint workflows and scripts and Build for testing all pass.

UI tests fails on all three shards, and the same tests fail on upstream/main itself (run 32631046978, 3848a21a1), so this is not a regression from this branch:

Test On this PR On upstream/main
testCompareSyncOpensFromFileMenu fail fail
testCompareIsDisabledUntilBothEndpointsAreChosen fail fail
testTargetPickerStartsWithNoConnectionChosen fail fail
testSwapIsDisabledWhenNoEndpointIsChosen fail fail
testBannerStatesNothingHasBeenWrittenBeforeAnyRun fail fail
testRunInNewTabOpensATabAndActuallyRunsTheQuery fail fail
testCommandDeleteDeletesTheEditorLineAfterSelectingAResultRow fail fail
testCommandReturnOpensTheResultInANewTab fail passed that run
testAFailedQueryShowsTheDatabaseError passed fail
testHelpMenuOpensTheSampleDatabase passed fail

The first five are CompareSyncUITests, which arrived with the Compare & Sync window in 3848a21a1 and have never been green. The rest are the "sample database never finished opening" family, whose membership drifts run to run.

Nothing in either list touches assistant mode, sessions, the result pane, the welcome panel or the MCP client.

Confirmed locally as well: the eight suites that touch a surface this PR changes all pass (18 cases across SingleWindowMenuContractUITests, AuxiliaryWindowCloseUITests, TableProLaunchUITests, NewConnectionCommandUITests, DataSettingsUITests, SettingsWindowTitleUITests, ConnectionCloseUITests), and the only local failures are the same five CompareSyncUITests.

CompareSyncUITests is now quarantined (33525726f), with the root cause written into the entry: CompareSyncLauncher.open gates on LicenseManager.isFeatureAvailable(.compareSync) and calls NSAlert.runModal() when the licence is absent, which it always is under UITestCase.launchApp()'s throwaway container. The suite's own guard item.isEnabled else { throw XCTSkip(...) } cannot fire, because the gate is in the launcher rather than in menu validation. The modal then holds the main thread, which is why cases after it in the same shard fail on unrelated assertions ("The sample database never finished opening", "Not hittable"): one licence gate takes several unrelated tests with it. That is worth fixing on main; it is not this PR's to fix.

Not done

  • TableProUITests coverage for assistant mode. Written and then withdrawn rather than landed: the mode control sits in the toolbar's overflow menu at the test window's width, and the suite has no AI provider, so an approval card cannot be reached at all. A suite that self-skips reads as coverage without being any. The deterministic parts it would have asserted (the pane's four views and their empty states, the approval ordering, the remote-tool gate) are covered by the unit suites above.
  • Manual pass over phase 1's nine success criteria. Needs a person at the keyboard, above all for the divider drag and resize cursor (missing resize cursor #1905) and the window frame being unchanged across a Browse to Assistant to Browse round trip.
  • The 28 unit tests that fail on my machine but not in CI. See below.

The local-only unit failures

Worth recording, because the previous version of this description called the suite "red before this branch" with 33 failing entries, and CI says otherwise: Unit tests passes on this branch in CI. The failures are specific to the machine I ran on, not to the branch.

29 tests failed locally; one was a real test bug and is fixed. The other 28 are environment-sensitive and reproduce on that machine deterministically, in isolation, on upstream/main as well:

  • Fixed (79f7c788b): ValidateDriverDescriptorTests (2) asserted "MySQL" was already claimed "by the built-in MySQL plugin". Nothing claims it under XCTest, because applicationDidFinishLaunching returns early when XCTestConfigurationFilePath is set, so no plugin ever loads and driverPlugins is empty. The duplicate check the tests exist to prove had nothing to collide with. The tests now seed the occupant themselves.
  • A latent source defect, not fixed: StructureChangeManagerUndoTests (3). StructureChangeManager's UndoManager leaves groupsByEvent at its default true, so undo granularity is decided by run-loop boundaries: the same two column edits are one undo step or two depending on when the loop turns. multipleUndos passes as a single test, fails with its suite, and .serialized does not help. CI's timing happens to fall the right way. The fix is to make each mutation an explicit undo group instead of depending on the run loop, which changes Structure-tab undo behaviour and wants its own PR.
  • Environment-dependent, uninvestigated: AWSSSOFetchTests (7), SSEEventStreamTests (3), SaveCompletionTests (3), DataChangeManagerExtendedTests (2), MCPHttpServerTransportTests / MCPHttpKeepAliveTests / MCPHttpServerTransportPairingTests (3, ports), SequelAceImporterTests / TablePlusImporterTests (2, these read for other apps' files on disk), SchemaColumnStoreCancellationTests / ScopedDriverCancellationTests (2), SQLCompletionProviderTests (1), SSHMatchExecutorTests (1).

Rework pass

main merged in (44 commits), then the branch reworked against its own review findings. The phase structure, AgentSessionRegistry, the purity of AgentArtifactProjection and the pane resolver are unchanged: what follows is defects in the code around them.

The merge

One semantic conflict, not a textual one. main gave ConnectionWindowPane a .preparing case for the sub-grace connect (#2609's launch work), and showsPreConnectAssistant had no arm for it. Assistant mode takes .preparing too: the grace exists to keep a progress indicator off screen for a wait too short to report, and the assistant surface is not one, it carries the prompt the user typed. Withholding it drew nothing for the grace and then flashed the conversation in.

Outside MCP servers: a client of its own

The client reused MCPStreamableHttpClientTransport, which exists to talk to TablePro's own bridge. Against somebody else's server that was wrong in six ways, so the outside client now has its own transport (MCPRemoteServerTransport):

  • notifications/initialized was never sent. The specification has the client send it once the initialize response is in, and a server that holds itself to the lifecycle refuses tools/list until it arrives. TablePro listed no tools at all on exactly the servers that implement the protocol most carefully.
  • Mcp-Session-Id was never captured or resent. Nothing read the initialize response head and no request carried the header, so every sessionful server answered 404 to everything after initialize. It is now captured, sent on every later request, and an expired session re-initializes once and retries rather than failing the call.
  • didInitialize = true was set before the round trip, so a failed initialize left the session permanently marked handshaken and every later call ran against a server that never handshook. The flag is now the handshake Task itself: a second caller awaits the first one's, and a throw clears it.
  • TablePro's own headers and error copy reached third parties. Mcp-Method and Mcp-Name mean nothing to another server, and an unreachable one reported "TablePro's MCP server is not reachable. Make sure TablePro is running and the MCP server is enabled in Settings > Integrations.", unlocalized, about somebody else's machine.
  • tools/list was read one page deep. It is paginated, so a server with more tools than it sends at once answers with a nextCursor, and stopping there offered the model a subset of what the server has with nothing anywhere saying so. Pages are followed now, bounded so a server that always returns a cursor cannot loop.
  • The negotiated protocol version was ignored. Every request carried the newest version TablePro knows whatever the server answered with, which tells a server that chose an older one that its choice was disregarded. The version from the initialize response is adopted, unless it names one this client does not implement.

The transport is request-and-response rather than fire-and-forget, which is what the specification actually describes, so the JSON-RPC id correlation, the reader task and the per-call deadline task all go. It also reads the body incrementally: the specification says a server SHOULD close its event stream after the response, and against one that does not, buffering the whole body meant every call sat until the timeout having already been answered.

Auth is unchanged on purpose. A server with no stored token is still not called.

Sessions

  • Restore raced session creation. It was an unstructured Task in applicationDidFinishLaunching, so a window opened before store.load() resumed found an empty list, minted a session, and was then joined by the stored one: two sessions on one conversation, both persisted, both in the rail. AgentSessionStore.load() is now nonisolated and restore is synchronous and runs before any window exists, which closes the window by construction. Measured on the record shape it reads: 0.08ms for ten sessions, 0.6ms for two hundred, against a 261ms launch.
  • Restored sessions never got their MCP tools. Only makeSession called MCPRemoteToolCoordinator.attach, so an allowlisted server's tools were missing from every session that came back from disk.
  • A slow server leaked adapters. attach registered tools after an await, so a detachAll during the listing left them in the registry pointing at a closed transport, with nothing tracking their names to unregister.

One Copilot conversation per session

AIProviderFactory caches one provider per configuration and CopilotChatProvider held a single conversationId, so two sessions on one Copilot configuration appended their turns to one server-side conversation. Each was answered with the other's context, across connections included, while their local transcripts stayed correctly separate. ProviderStreamLease cannot help: it serializes turns and never swaps what the provider points at.

Conversation state is now keyed by session, and resetConversation / deleteLastTurn name a session as well as a configuration. This one is not new to this branch, so it has its own CHANGELOG entry and the limitation it documented is gone from assistant-mode.mdx.

The result pane stopped reparsing the transcript at 20Hz

AgentArtifactPaneView.artifact is computed on every render so it can never disagree with the transcript, which is the right call. But messages is rewritten every 50ms while a reply streams, and each pass ran QueryClassifier.classifyTier and the whole of DDLChangeReader.preview over every statement the conversation had ever proposed, so the cost of drawing the pane grew with the conversation and was paid twenty times a second.

The projection is still pure. What a statement means is memoized on the statement and the engine, which is everything that analysis reads; the state, the order and the per-call id are still recomputed every pass. Streaming text changes neither key, so the cache answers every pass between one tool call and the next.

Native and HIG

  • The pre-connect surface was a second, degraded copy of the panel. Every turn drew as one unstyled paragraph, and the composer was a Text inside a stroked RoundedRectangle imitating an NSTextField, beside a send button with an empty action. It now uses AIChatMessageView and ChatComposerView, the same two the connected panel uses, so the surface does not change appearance the instant the connection lands. The prompt is editable while the connection is being made, which a connect long enough to notice a typo in needs; what is typed is what sendPendingPromptIfReady sends.
  • The mode control's overflow menu did nothing. A group's default menu form sends the group's action with an NSMenuItem as the sender, and the action could only read a selection off a group, so choosing Browse or Assistant from the overflow menu was inert at the ordinary window widths where the control lives there. The menu form is now built explicitly with each item carrying its segment in tag, and the tick follows the same sync pass the segments do.
  • Cancel on the pre-connect surface is a push button rather than .buttonStyle(.link); a link style says the control navigates.
  • New Session in the rail is a bordered + in the bottom bar, the shape a source list uses, instead of a full-width .plain button with no press, hover or focus ring.
  • The result pane's segment lives on the session rather than in the view's @State, so a reader who opened Schema to check a DROP is not returned to SQL by looking at another connection and coming back.
  • The segmented picker takes a real label and hides it, rather than an empty one with an accessibility label bolted on beside it.
  • The rail's empty state said "Ask a question below", where there is no composer below.

The surface itself, against the app's own conventions

Measured against QueryInsightsGroupList, which is the app's other list of statements with metadata under them, and against the HIG.

Typography was a step small throughout. The house scale is statements at .system(.callout, design: .monospaced) and metadata at .caption, with 3pt between a row's lines and 7pt around it. The result pane set SQL at .caption and metadata at .caption2, so one statement read as content in Query Insights and as a footnote here. Every .caption2 is gone.

Four hand-built controls are native ones now.

  • The result grid was HStacks in a stroked rectangle, and worse than it looked: each cell took minWidth: 60 independently, so columns did not line up between rows, and one long value shifted everything to its right on that row alone. It is a Grid, which sizes a column once from every cell in it. (Table would bring resizable headers and selection, but TableColumnForEach needs macOS 14.4 against a 14.0 target.)
  • The welcome panel's session list was a ScrollView of .plain buttons with no selection, hover, keyboard navigation or row semantics. It is a List.
  • Its send control was a bare arrow.up.circle.fill with .plain: no border, no press state, no focus ring, no name. It is an Ask push button taking .defaultAction.
  • "Destructive" was a capsule at .red.opacity(0.15), where the app also had 0.08, 0.16 and 0.18 for the same idea and none of them tracked system contrast. A shared StatusBadge uses Color.red.quaternary, sibling to TypeBadge.

Cancel, Browse database, Open as Query and Remove were .buttonStyle(.link); a link says a control navigates to content. The Confirm Writes notice sat above the divider and moved the whole pane by its height whenever the mode changed; it is a bottom bar the pane supplies, with the notice itself left neutral because the chat composer shows it too. The rail's empty state offers New Session rather than describing where the button is. Right-click now offers Copy on statements, schema changes and results.

Accessibility. The result grid published a flat run of Text in which a value carried no column; rows read "column: value" now. Status icons in the plan, schema, SQL and rail rows were unlabelled images, so a row's state was drawn and never spoken. A turn proposing three writes gave three identical "Run" buttons to VoiceOver and Voice Control; each names its tool while the visible title stays the verb.

Two defects in the outside-server settings

  • Test created the server before probing it. The token lives in the Keychain under the server's id, so Test wrote the configuration and the credential and then probed what it had written. Checking a mistyped endpoint left a server in the list nobody asked to add. probe takes the token explicitly now and builds a throwaway session.
  • Add accepted an empty token, and MCPClientSession.make refuses an endpoint with no credential, so the entry could be ticked onto a connection and would never answer. Add requires it, as Test already did.

Remove also asks first: it deletes a Keychain token and every connection's permission, and none of it comes back.

A queued prompt could be dropped

AgentSessionLauncher sets pendingPrompt and routes, and the only flush was inside adoptSession, which runs when a connect lands. A connection whose window is already open and connected changes nothing about its session, so nothing adopted it and the queue was never read; setContentMode did not rescue it either, because it returns early when the workspace is already in assistant mode. Asking about the database already on screen queued the text and dropped it silently. The flush now also runs from applyContentMode and from the launcher's already-hosting path, and because sendPendingPromptIfReady clears before it dispatches, three call sites still send once.

Tests

New: MCPClientHandshakeTests (12, over a URLProtocol stub: the initialized notification, session id capture and resend, the stateless case, a failed handshake retrying, an expired session recovering, a response read out of an event stream, a stream the server never closes, and no bridge headers or TablePro error copy reaching an outside server), MainWindowToolbarContentModeTests (5, the menu form's items, their tags, targets and names), AssistantModeSwitchUITests (2, what the control publishes to assistive clients), plus a case pinning that every prompt-flush site sends once.

Extended: AgentArtifactProjectionTests (+4 over what a memo can get wrong), ConnectionWindowPaneResolverTests (+1 for .preparing).

XCUIElement.waitToBeHittable(timeout:) joins waitToExist: an element mid-animation exists and hit-tests to nothing, so a click on it lands somewhere else.

Switching mode is not driven from a UI test, because XCUITest cannot drive it. Measured rather than assumed, against a dumped accessibility tree: the group publishes as a radio group of radio buttons (not buttons, which is why an earlier attempt found nothing), the Assistant segment reports exists and isHittable, and click() leaves isSelected false with the window unchanged. AppKit does not route a synthetic click to a segment inside a toolbar item group, and the mode has no menu command to reach it by instead. Rather than land a suite that self-skips, the UI test asserts what a UI test can see, and the switch itself is covered by ConnectionWindowPaneResolverTests, MainWindowToolbarContentModeTests and MainWindowToolbarValidationTests.contentModeFollowsTheSession.

Found while in here, not fixed

All three are pre-existing on main, none of them is this branch's to fix, and each is small.

  • The sidebar toggle reads its SF Symbol names to VoiceOver. MainWindowToolbar.makeSidebarSegmentGroup passes accessibilityDescription: nil for list.bullet and star, so the two segments announce as "List" and "favorite". Confirmed in the dumped accessibility tree. This is the same defect 90c3b58f9 fixed for the mode control, on the control next to it.
  • The sidebar toggle's overflow menu is inert. sidebarSegmentChanged(_:) returns unless the sender is an NSToolbarItemGroup, so choosing Tables or Favorites from the toolbar overflow does nothing. Identical in shape to the mode-control defect fixed here, and the fix is the same shape too.
  • The mode has no menu command. MainSplitViewController.toggleContentMode() exists and nothing calls it: there is no View menu item and no shortcut, so the toolbar control is the only way to switch, which is also why the switch cannot be UI-tested. Wiring it is a product decision about placement and shortcut rather than a defect fix.

Considered and left alone

AssistantSafeModeFloor.isActive answers a live security question from WorkspaceContentModeStore, which is a UserDefaults mirror rather than the live ConnectionWorkspace.contentMode. It is correct as it stands: the contentMode didSet writes the store synchronously, so the two cannot diverge for a hosted connection, and a connection no window hosts reads its last mode, which errs toward the floor being on. Reading the live workspace instead would need a third record of one fact for no behavioural gain, and getting it wrong turns a write gate off.

https://claude.ai/code/session_01S9ckdzeugurfqNmDpGU2M7

@J2TeamNNL
J2TeamNNL marked this pull request as ready for review August 23, 2026 13:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79f7c788b0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

else { return }
self?.providerWaitReason = ProviderStreamLease.waitMessage(providerName: leaseProviderName)
}
await ProviderStreamLease.shared.acquire(configId: leaseConfigId, sessionId: leaseSessionId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Isolate Copilot conversation state per session

When two agent sessions use the same Copilot provider configuration, this lease only serializes their turns; it does not switch the cached CopilotChatProvider's conversationId. After session A releases the lease, session B therefore appends its prompt to A's server-side conversation, and subsequent turns from both sessions share context despite having separate local transcripts. Store Copilot conversation state per agent session or explicitly swap/reset it when ownership changes.

Useful? React with 👍 / 👎.

"version": .string(Bundle.main.appVersion)
])
])
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Send the MCP initialized notification

For an outside MCP server that enforces the protocol lifecycle, completing initialize is not sufficient: the client must send notifications/initialized before issuing tools/list or tools/call. This method returns immediately after the initialize response, so such servers reject the following tools/list request and none of their tools are registered.

Useful? React with 👍 / 👎.

Comment on lines +83 to +85
return MCPClientSession(
configuration: configuration,
transport: MCPStreamableHttpClientTransport(credentialsProvider: provider),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve negotiated MCP session IDs

When an outside Streamable HTTP MCP server returns an Mcp-Session-Id during initialization, this transport cannot preserve it: MCPStreamableHttpClientTransport exposes only response bodies and its later requests never include the negotiated header. Consequently the initialize request can succeed while the immediately following tools/list is rejected by any sessionful server. Use a client transport that captures the initialization header and sends it on subsequent requests.

Useful? React with 👍 / 👎.

Comment thread TablePro/AppDelegate.swift Outdated
/// Sessions are listed again before any window asks for one, so a session whose window was
/// closed last run is in the rail from the start rather than appearing once its connection
/// happens to be opened.
Task { await AgentSessionRegistry.shared.restore() }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Finish restoration before creating sessions

On launch, this unstructured task does not actually ensure restoration finishes before a window or welcome action calls session(for:). If that happens while store.load() is suspended, the registry creates a new default session and later appends the stored session as well, leaving duplicate sessions for the same conversation in the rail and persisting both. Gate session creation on restoration completion or perform restoration before exposing the registry to UI actions.

Useful? React with 👍 / 👎.

updatedAt: record.updatedAt,
approvals: approvals
)
restored.append(session)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reattach MCP tools for restored sessions

After relaunch, restored sessions never authorize or register their outside MCP tools. MCPRemoteToolCoordinator.attach is invoked only from makeSession, while this restore path constructs and appends sessions directly; reopening one through session(for:) returns the existing session without attaching it. Thus an allowlisted server's tools disappear from every restored session until the user creates a brand-new session.

Useful? React with 👍 / 👎.

J2TeamNNL and others added 9 commits August 23, 2026 21:24
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Nguyễn Nam Long <J2TeamNNL@users.noreply.github.com>
Co-authored-by: Nguyễn Nam Long <J2TeamNNL@users.noreply.github.com>
Co-authored-by: Nguyễn Nam Long <J2TeamNNL@users.noreply.github.com>
…tions-acdf

docs: note macOS/Xcode-only build for cloud agents
Co-authored-by: Nguyễn Nam Long <J2TeamNNL@users.noreply.github.com>
Co-authored-by: Nguyễn Nam Long <J2TeamNNL@users.noreply.github.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

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.

3 participants