cline/.github/pull_request_template.md at main

Dominic Cooney 4829f08b3f fix(vscode): reliable MCP OAuth on the SDK extension (ENG-2108, CLINE-2304) (#11529 )
* fix(vscode): store MCP OAuth in shared settings file like the CLI (ENG-2108)

VSCode stored MCP OAuth tokens in a single mcpOAuthSecrets secrets blob
keyed by sha256(name:url), while the CLI/SDK store per-server oauth state in
cline_mcp_settings.json. The two never interoperated (CLI auth was invisible to
VSCode), and VSCode's read-whole-blob/write-whole-blob through StateManager's
non-refreshing cache meant concurrent windows clobbered each other's tokens.

- Store MCP OAuth state in the shared settings file in @cline/core's format.
- Reads are fresh from disk; writes are scoped read-modify-write of one
  server's oauth key via updateMcpServerOAuthState (now atomic temp+rename).
- Replace the vscode:// callback flow with HTTP-based token collection via
  authorizeMcpServerOAuth (same local loopback flow the CLI uses).
- Reconnect an unauthenticated server when its tokens appear (e.g. CLI auth).
- One-time migration of legacy mcpOAuthSecrets tokens into the shared file.
- Remove McpOAuthRedirectResolver, mcpOAuthFlow, completeOAuth, and the
  mcp-auth URI callback route.

* feat(vscode): add --instances/--random-port to MCP OAuth test server

Lets you start several independent test servers, each on its own OS-assigned
random port, so you can add multiple streamableHttp MCP servers to Cline at
once and exercise concurrent OAuth flows. baseUrl now reflects the actually
bound port so discovery metadata and redirect URIs stay correct under random
ports.

* fix(vscode): stop MCP OAuth handshake writes from livelocking the settings watcher (ENG-2108)

Now that codeVerifier/clientInformation live in the shared settings file, the
MCP SDK's per-connect-attempt saveCodeVerifier() writes were tripping the
settings watcher, which re-entered updateServerConnections -> connectToServer
-> another write, looping forever. It was especially bad with two+
unauthenticated servers, where each server's verifier churn re-triggered the
other (visible as a flickering, ever-changing codeVerifier nonce).

The watcher now compares a connection-relevant fingerprint (full per-server
config minus the oauth block, plus a boolean for whether an access token
exists) and skips writes that only churn OAuth-handshake fields. A token
appearing/disappearing still changes the fingerprint, so CLI/other-window
authorization continues to trigger a reconnect via serverGainedOAuthTokens.

* feat(vscode): print paste-ready MCP settings fragment from OAuth test server

On startup the test server now emits an mcpServers JSON fragment (nested
transport shape, matching cline_mcp_settings.json) alongside the banner, so you
can paste it straight into the settings file instead of hand-writing it. With
--instances the entries get distinct names (oauth-test-1, ...), each carrying
its actual bound port.

* fix(vscode): atomic MCP settings writes + fingerprint gate; drop timer guards (CLINE-2097)

Deleting one MCP server could empty the whole list. Root cause: settings
writes were non-atomic (fs.writeFile), so chokidar (and any other process)
could read a transient empty/torn file mid-write and reconcile to zero servers.
The previous fix only masked this with a per-process isUpdatingClineSettings
boolean cleared on a 300ms timer — it did nothing for the CLI or other windows
and was racy.

Replace both timer guards (isUpdatingClineSettings, isUpdatingFromRemoteConfig)
with two deterministic, process-agnostic mechanisms:

- writeSettingsFile(): atomic temp-file + rename for every settings write, so
  any reader always sees a complete file. Holds for any number of concurrent
  writers (CLI, multiple windows, SDK OAuth handshake).
- content fingerprint: the watcher reconciles only when the connection-relevant
  view changed. writeSettingsFile pre-seeds the fingerprint so our own write is
  a no-op, while a genuine change from any other process is still processed.
  Because reconcile is idempotent and reads are never torn, a missed
  suppression is at worst a redundant reconnect, never data loss.

All RPC writers (toggle disabled, autoApprove x2, timeout, add, delete) and the
remote-config sync now go through writeSettingsFile. Removes all setTimeout(.,
300) flag juggling.

* feat(vscode): add a non-guessable 'frozzle' tool to the MCP OAuth test server

The MCP OAuth test server now serves tools/list + tools/call exposing a
'frozzle' tool whose output cannot be derived without calling it (reverse the
string and swap each letter's case, wrapped in guillemets). This gives an eval
a reliable end-to-end signal that the OAuth-authenticated MCP round-trip really
happened: a correct 'frozzle <text>' answer can't be hallucinated. The
transform is easy to verify at a glance and invertible. Adds frozzle.test.ts.

* fix(sdk): drop lingering OAuth callback sockets on close so deny->approve re-auth works (ENG-2108)

The local OAuth callback server's close() called Server.close(), which only
stops accepting new connections and lets existing keep-alive sockets linger.
The browser / global-fetch connection pool keeps such a socket to the fixed
callback port (1456) alive. So after the user denied an MCP OAuth request and
retried, the retry's approve callback could be delivered over the pooled socket
to the FIRST (already-settled) server. That server's settle() was a no-op, so
waitForCallback() never resolved, finishAuth()/token exchange never ran, and no
token was saved — the server stayed unauthenticated (the deny->approve repro).

Call server.closeAllConnections() in close() so no pooled socket outlives the
server. Adds a regression test driving a keep-alive agent across close().

* fix(vscode): actually reconnect MCP server when toggled back on (ENG-2108)

toggleServerDisabledRPC only flipped the in-memory disabled flag and set status
to 'connecting', but never rebuilt the connection. A disabled server's
connection has no live transport/client, so re-enabling left it stuck on the
yellow 'connecting' indicator forever and never re-advertised its tools to the
agent.

Tear down and rebuild the connection through deleteConnection + connectToServer
(which opens a real transport when enabled, or a disconnected stub when
disabled), then notifyWebviewOfServerChanges so the SDK session's tool list is
refreshed. OAuth state is preserved (deleteConnection doesn't clear it). Adds
McpHub.toggleServerDisabledRPC.test.ts.

* fix(vscode): reload MCP tools silently without chat spam (ENG-2108)

Restarting the SDK session to pick up MCP tool changes appended visible chat
messages ('MCP tools changed - reloading...' and 'MCP tools reloaded
successfully...') plus a completion_result banner. Toggling several servers
piled up many of these. Tool reloading should be transparent.

Emit only the session status transitions (running -> idle) via
emitSessionEvents([], ...) instead of appendAndEmit, so no chat messages or
completion banner are shown. Genuine reload failures still surface an error
message. Updates sdk-mcp-coordinator.test.ts accordingly.

* docs(mcp): clean up comments to describe current behavior

Revise comments across the MCP OAuth and settings code to document the code as
it stands, dropping references to prior implementations, task IDs, and
before/after narration. Also reflow the auth-server regression test to the
repository's formatter. No behavior change.

* fix(vscode): atomic fallback write in remote MCP sync; document sync OAuth I/O

Make the no-McpHub branch of syncRemoteMcpServersToSettings write via an
atomic temp-file + rename so a concurrent reader never observes a torn or
empty settings file, matching every other settings write.

Document why the OAuth state read-modify-write in McpOAuthManager is
synchronous: it serializes this process's shared-file updates without a
Promise queue, which we prefer over async I/O for reliability of the
cross-process settings file.

* fix(mcp): serialize settings read-modify-writes

* docs(vscode): clarify MCP settings create race

* fix(vscode): create MCP settings atomically

* fix(cli): keep clearing missing MCP OAuth state a no-op

* fix(vscode): avoid yielding while holding MCP settings lock (#11596)

* fix(mcp): async lock acquisition for VSCode MCP settings/OAuth writes

Add updateMcpSettingsFile/updateMcpServerOAuthStateAsync to @cline/core that
yield the event loop while acquiring the cross-process settings lock instead of
blocking it with Atomics.wait. The critical section stays synchronous and the
mutator stays pure, so the lock is never held across an await and serialization
is preserved without an in-process queue.

Route the VSCode extension host's OAuth state writes (McpOAuthManager) through
the async variant so a connection-time OAuth callback can no longer freeze the
extension host event loop or deadlock against an in-flight updateMcpSettingsFile
whose lock-releasing continuation needs the loop.

Unify the sync and async acquisition paths on a shared reentrancy guard
(activeLocks) so a nested settings update on the same file fails fast instead of
self-deadlocking.

Tests: contended async serialization asserting zero Atomics.wait calls, async
stale-lock reclaim, reentrancy fail-fast, and uncontended run+release.

* fix(mcp): bootstrap missing settings file inside the lock; tidy docs

Creating the MCP settings file now happens in one place: the locked
read-modify-write helpers. A missing file reads as an empty settings object, so
the first write to a fresh path (e.g. a fresh-install `cline mcp add`) creates
it inside the lock instead of throwing ENOENT. The SDK (updateMcpSettingsFile /
updateMcpSettingsFileSync) and the VSCode lock helper share this contract, so
callers no longer need to pre-create the file. Add regression tests for the
SDK, the CLI wizard addServer(), and the VSCode helper on a missing path.

Also flag the synchronous SDK entry points (updateMcpSettingsFileSync,
updateMcpServerOAuthState) as preferring their async siblings, with a TODO to
delete them once all callers migrate, and tighten the lock-helper doc comments
to describe current behavior.

* fix(vscode): finish npm->bun migration in dev tooling, tasks, and docs

The npm->bun migration (#11632) updated package scripts, .vscodeignore and .vscode-test.mjs but left a trail of npm/npx/node invocations in editor configs, dev scripts, and docs. Following the breadcrumbs from 'npm run protos':

- .vscode/launch.json: standalone-core debug uses 'bun <file>.ts' (was npx tsx); Open Storybook uses 'bun run' (was npm run).
- .vscode/tasks.json: all task commands use 'bun run' (was npm run).
- scripts/run-extension-host.sh and .claude/hooks/claude-code-for-web-setup.sh: 'bun run' (was npm run).
- debug-harness/server.ts: shebang 'bun'; build steps use 'bun run protos', 'bun esbuild.mjs', 'bunx vite build' (were npm/node/npx).
- dev script shebangs (test-hostbridge-server, test-standalone-core-api-server, testing-platform-orchestrator, interactive-playwright): '#!/usr/bin/env bun' (was npx tsx).
- WebviewProvider HMR hint, e2e README, copilot-instructions, PR template, mcp-oauth-test-server docs, generate-state-proto message, tsconfig.test comment, state-keys test comment: bun.

Left untouched (correct per .clinerules/bun-and-node): Node-runtime invocations (node build.mjs), prebuild-install --target=<node>, vsce, 'npm install -g cline' (user CLI install), and App.stories.tsx mock chat fixtures.

---------

Co-authored-by: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com>
Co-authored-by: Cline Agent <cline-agent@users.noreply.github.com>
3.6 KiB

Raw Permalink Blame History

Description

Test Procedure

Type of Change

Pre-flight Checklist

Screenshots

Additional Notes

3.6 KiB Raw Permalink Blame History

Related Issue

Description

Test Procedure

Type of Change

Pre-flight Checklist

Screenshots

Additional Notes

3.6 KiB

Raw Permalink Blame History
