Skip to content

refactor(app): unify pre-handshake guard to console.warn #630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ochafik merged 1 commit into from
Apr 21, 2026
+34 −49
Merged

refactor(app): unify pre-handshake guard to console.warn #630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 31 additions & 46 deletions src/app-bridge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1907,50 +1907,53 @@ describe("App <-> AppBridge integration", () => {
describe("pre-handshake guard (claude-ai-mcp#149)", () => {
const guardMsg =
/called before connect\(\) completed the ui\/initialize handshake/;
let warnSpy: ReturnType<typeof spyOn<Console, "warn">>;

beforeEach(() => {
warnSpy = spyOn(console, "warn").mockImplementation(() => {});
});
afterEach(() => {
warnSpy.mockRestore();
});

const warnings = () => warnSpy.mock.calls.map((c) => String(c[0]));

it("callServerTool warns when called before connect() completes", async () => {
const errSpy = spyOn(console, "error").mockImplementation(() => {});
bridge.oncalltool = async () => ({ content: [] });
await bridge.connect(bridgeTransport);

const connecting = app.connect(appTransport);
// Handshake is in flight; _initializedSent is still false.
await app.callServerTool({ name: "t", arguments: {} }).catch(() => {});

expect(errSpy).toHaveBeenCalledTimes(1);
expect(errSpy.mock.calls[0][0]).toMatch(guardMsg);
expect(errSpy.mock.calls[0][0]).toContain("App.callServerTool()");
const appSide = warnings().filter((m) => guardMsg.test(m));
expect(appSide).toHaveLength(1);
expect(appSide[0]).toContain("App.callServerTool()");

await connecting;
errSpy.mockRestore();
});

it("callServerTool does not warn after connect() resolves", async () => {
const errSpy = spyOn(console, "error").mockImplementation(() => {});
bridge.oncalltool = async () => ({ content: [] });
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

await app.callServerTool({ name: "t", arguments: {} });

expect(errSpy).not.toHaveBeenCalled();
errSpy.mockRestore();
expect(warnings()).toEqual([]);
});

it("sendMessage and readServerResource also warn before handshake", async () => {
const errSpy = spyOn(console, "error").mockImplementation(() => {});

await app.sendMessage({ role: "user", content: [] }).catch(() => {});
await app.readServerResource({ uri: "test://r" }).catch(() => {});

expect(errSpy).toHaveBeenCalledTimes(2);
expect(errSpy.mock.calls[0][0]).toContain("App.sendMessage()");
expect(errSpy.mock.calls[1][0]).toContain("App.readServerResource()");
errSpy.mockRestore();
const appSide = warnings().filter((m) => guardMsg.test(m));
expect(appSide).toHaveLength(2);
expect(appSide[0]).toContain("App.sendMessage()");
expect(appSide[1]).toContain("App.readServerResource()");
});

it("throws instead of warning when strict: true", async () => {
const errSpy = spyOn(console, "error").mockImplementation(() => {});
const strictApp = new App(
testAppInfo,
{},
Expand All @@ -1960,78 +1963,65 @@ describe("App <-> AppBridge integration", () => {
await expect(
strictApp.callServerTool({ name: "t", arguments: {} }),
).rejects.toThrow(guardMsg);
expect(errSpy).not.toHaveBeenCalled();

errSpy.mockRestore();
expect(warnings()).toEqual([]);
});

describe("late handler registration", () => {
const lateMsg =
/handler registered after connect\(\) completed the ui\/initialize handshake/;

it("warns when ontoolresult is set after connect() resolves", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

app.ontoolresult = () => {};

expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy.mock.calls[0][0]).toMatch(lateMsg);
expect(warnSpy.mock.calls[0][0]).toContain('"toolresult"');
warnSpy.mockRestore();
expect(warnings()).toHaveLength(1);
expect(warnings()[0]).toMatch(lateMsg);
expect(warnings()[0]).toContain('"toolresult"');
});

it("warns when addEventListener('toolinput', …) is called after connect()", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

app.addEventListener("toolinput", () => {});

expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy.mock.calls[0][0]).toContain('"toolinput"');
warnSpy.mockRestore();
expect(warnings()).toHaveLength(1);
expect(warnings()[0]).toContain('"toolinput"');
});

it("does not warn for handlers set before connect()", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
app.ontoolinput = () => {};
app.addEventListener("toolresult", () => {});

await bridge.connect(bridgeTransport);
await app.connect(appTransport);

expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
expect(warnings()).toEqual([]);
});

it("does not warn for hostcontextchanged (repeating event)", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

app.onhostcontextchanged = () => {};
app.addEventListener("hostcontextchanged", () => {});

expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
expect(warnings()).toEqual([]);
});

it("does not warn when clearing a handler (set to undefined)", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
app.ontoolinput = () => {};
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

app.ontoolinput = undefined;

expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
expect(warnings()).toEqual([]);
});

it("throws instead of warning when strict: true", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
const [strictAppT, strictBridgeT] =
InMemoryTransport.createLinkedPair();
const strictBridge = new AppBridge(
Expand All @@ -2053,13 +2043,11 @@ describe("App <-> AppBridge integration", () => {
expect(() => {
strictApp.addEventListener("toolinput", () => {});
}).toThrow(lateMsg);
expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
expect(warnings()).toEqual([]);
});
});

it("AppBridge warns on tools/call from a View that skipped the handshake", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
bridge.oncalltool = async () => ({ content: [] });
await bridge.connect(bridgeTransport);

Expand All @@ -2073,23 +2061,20 @@ describe("App <-> AppBridge integration", () => {
});
await flush();

expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy.mock.calls[0][0]).toContain(
expect(warnings()).toHaveLength(1);
expect(warnings()[0]).toContain(
"received 'tools/call' before ui/notifications/initialized",
);
warnSpy.mockRestore();
});

it("AppBridge does not warn after initialized is received", async () => {
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
bridge.oncalltool = async () => ({ content: [] });
await bridge.connect(bridgeTransport);
await app.connect(appTransport);

await app.callServerTool({ name: "t", arguments: {} });

expect(warnSpy).not.toHaveBeenCalled();
warnSpy.mockRestore();
expect(warnings()).toEqual([]);
});
});

Expand Down
6 changes: 3 additions & 3 deletions src/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,12 @@ export type AppOptions = ProtocolOptions & {
*/
autoResize?: boolean;
/**
* Throw on detected misuse instead of logging a console error.
* Throw on detected misuse instead of logging a console warning.
*
* Currently this affects calling host-bound methods (e.g.
* {@link App.callServerTool `callServerTool`}, {@link App.sendMessage `sendMessage`})
* before {@link App.connect `connect`} has completed the `ui/initialize`
* handshake. With `strict: false` (default) a `console.error` is emitted;
* handshake. With `strict: false` (default) a `console.warn` is emitted;
* with `strict: true` an `Error` is thrown.
*
* @remarks Throwing will become the default in a future release.
Expand Down Expand Up @@ -375,7 +375,7 @@ export class App extends ProtocolWithEvents<
throw new Error(msg);
}
// TODO(next-minor): make `strict: true` the default.
console.error(`${msg}. This will throw in a future release.`);
console.warn(`${msg}. This will throw in a future release.`);
}

protected readonly eventSchemas = {
Expand Down
Loading