Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@

### Fixed

- `/mcp auth <server>` now completes an interactive OAuth login instead of stalling forever. The command announced
`Opening browser to authorize <server>...` but never launched a browser, and the notification carrying the
authorization URL was emitted immediately before that announcement, so the TUI's consecutive-status coalescing
overwrote it. With no browser and no reachable URL, the flow waited on its loopback callback until the session
ended. The auth command bridge now calls the real browser launcher, and each branch emits a single notification
that includes the authorization URL as a fallback for when the launch fails.
- Steering queued while a provider stream-start timeout retry is running now starts automatically when that managed
retry exhausts its budget, instead of remaining parked until another user prompt. Generic terminal provider errors
and user-aborted retries keep their existing queue-retention behavior
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { openBrowser as launchBrowser } from "../../../../../utils/open-browser.ts";
import type { ExtensionAPI, ExtensionCommandContext } from "../../../types.ts";
import { createMcpLogger } from "../log.ts";
import type { McpService } from "../service.ts";
Expand Down Expand Up @@ -34,7 +35,9 @@ export async function handleMcpAuthCommand(
}
await service.attachSession({ type: "session_start", reason: "reload" }, ctx, pi).catch(() => undefined);
},
openBrowser: (url) => ctx.ui.notify(`Open this URL to authorize ${name}:\n${url.toString()}`),
// Actually launch the browser. The URL is surfaced by the auth flow itself in a single
// status line, because consecutive ui.notify() status lines overwrite each other in the TUI.
openBrowser: (url) => launchBrowser(url.toString()),
pending: service.getPendingAuth(),
interactiveGuard: {
begin: (serverName) => service.beginInteractiveAuth(serverName),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ async function runInteractive(deps: AuthCommandDeps): Promise<void> {
const loopbackResult = channel.usesLoopback ? channel.waitForCode() : undefined;
provider = buildProvider(deps, channel.redirectUrl, (url) => deps.openBrowser?.(url));
const begin = await beginAuthorization(provider, deps.flow);
if (begin.authorizationUrl !== undefined) deps.notify(`Opening browser to authorize ${deps.serverName}...`);
// One notification per branch: consecutive status lines overwrite each other in the TUI, so a
// separate "opening browser" line would erase the URL the user needs when the launch fails.
if (!channel.usesLoopback) {
deps.pending.set(deps.serverName, provider);
if (begin.authorizationUrl === undefined) {
Expand All @@ -134,10 +135,15 @@ async function runInteractive(deps: AuthCommandDeps): Promise<void> {
);
}
deps.notify(
`Complete the browser flow, then run /mcp auth-complete ${deps.serverName} <redirect-url> with the final redirect URL.`,
`Opening browser to authorize ${deps.serverName}. If it does not open, visit:\n${begin.authorizationUrl.toString()}\nComplete the browser flow, then run /mcp auth-complete ${deps.serverName} <redirect-url> with the final redirect URL.`,
);
return;
}
if (begin.authorizationUrl !== undefined) {
deps.notify(
`Opening browser to authorize ${deps.serverName}. If it does not open, visit:\n${begin.authorizationUrl.toString()}`,
);
}
const { code } = await (loopbackResult ?? channel.waitForCode());
await finishAuthorization(provider, code, deps.flow);
await deps.onReconnect();
Expand Down
16 changes: 16 additions & 0 deletions packages/coding-agent/src/core/extensions/builtin/mcp/changes.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
# mcp Extension Changes

## Interactive OAuth actually opens a browser and keeps the URL visible (2026-08-17)

### What changed
- `auth/commands-auth-dispatch.ts` wires `openBrowser` to `utils/open-browser.ts` instead of emitting a notification; the launch now really happens.
- `auth/commands-auth.ts` `runInteractive` emits exactly one notification per branch, carrying the authorization URL, instead of an announcement line that followed and erased the URL line.
- `test/mcp/oauth-callback.test.ts` asserts the single announcement contains the authorization URL for both the loopback and the callback-override branch.

### Why
- `/mcp auth <server>` said "Opening browser..." but nothing ever spawned a browser, and the preceding URL notification was overwritten: consecutive `ui.notify` status lines coalesce in the TUI (`showStatus` replaces the trailing status text in place). The flow then blocked on the loopback callback with no reachable authorization URL, so interactive login could never complete.

### Why extension system couldn't handle this alone
- The auth command bridge owns the `AuthCommandDeps` wiring and the notification sequence of the built-in `/mcp` command namespace; no external extension can reorder them.

### Expected merge conflict zones
- LOW: `auth/commands-auth-dispatch.ts` deps literal; `auth/commands-auth.ts` `runInteractive` notification block.

## Explicit pgrep match-all pattern for process-tree collection (2026-08-12)

### What changed
Expand Down
36 changes: 34 additions & 2 deletions packages/coding-agent/test/mcp/oauth-callback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ async function agentDir(): Promise<string> {
interface Harness {
deps: AuthCommandDeps;
browsered: URL[];
notified: string[];
store: McpTokenStore;
}

function makeHarness(dir: string, mcpUrl: string, overrides: Partial<McpServerConfig> = {}): Harness {
const browsered: URL[] = [];
const notified: string[] = [];
const config: McpServerConfig = {
type: "http",
url: mcpUrl,
Expand All @@ -78,14 +80,21 @@ function makeHarness(dir: string, mcpUrl: string, overrides: Partial<McpServerCo
config,
agentDir: dir,
hasUI: true,
notify: () => undefined,
notify: (message) => {
notified.push(message);
},
openBrowser: (url) => {
browsered.push(url);
},
onReconnect: () => Promise.resolve(),
pending: new Map<string, McpOAuthProvider>(),
};
return { deps, browsered, store: new McpTokenStore({ agentDir: dir, serverName: "fix", serverUrl: mcpUrl }) };
return {
deps,
browsered,
notified,
store: new McpTokenStore({ agentDir: dir, serverName: "fix", serverUrl: mcpUrl }),
};
}

async function followAuthorize(url: URL): Promise<Response> {
Expand Down Expand Up @@ -242,6 +251,26 @@ describe("LoopbackCallbackServer", () => {
expect(harness.browsered).toHaveLength(0);
});

// Regression: the authorization URL used to be announced in its own notification, immediately
// followed by a second status line. Consecutive status lines overwrite each other in the TUI, so
// the URL vanished and a failed browser launch left the flow waiting forever with no way out.
it("carries the authorization URL in the same single notification that announces the browser", async () => {
const fixture = await idp();
const dir = await agentDir();
const harness = makeHarness(dir, fixture.mcpUrl);

const auth = runAuth(harness.deps);
await vi.waitFor(() => expect(harness.browsered).toHaveLength(1));
const authorizationUrl = harness.browsered[0];
if (authorizationUrl === undefined) throw new Error("no authorization URL");
await vi.waitFor(() => expect(harness.notified).toHaveLength(1));
expect(harness.notified[0]).toContain(authorizationUrl.toString());

const callback = await followAuthorize(authorizationUrl);
expect(callback.status).toBe(200);
await auth;
});

it("runAuth with a callback URL override opens no listener and completes through pasted redirect", async () => {
const fixture = await idp();
const dir = await agentDir();
Expand All @@ -256,6 +285,9 @@ describe("LoopbackCallbackServer", () => {
expect(harness.deps.pending.has(harness.deps.serverName)).toBe(true);
const authorizationUrl = harness.browsered[0];
if (authorizationUrl === undefined) throw new Error("no authorization URL");
expect(harness.notified).toHaveLength(1);
expect(harness.notified[0]).toContain(authorizationUrl.toString());
expect(harness.notified[0]).toContain("/mcp auth-complete fix");
const redirect = await authorizeRedirectLocation(authorizationUrl);
await runAuthComplete(harness.deps, redirect);
expect(await portOpen(port)).toBe(false);
Expand Down