feat(app): warn when one-shot handlers are registered after connect() (#629)
Mirrors the outbound-side guard from #623: registering a toolinput / toolinputpartial / toolresult / toolcancelled handler after the ui/initialize handshake has completed now logs a console.warn (or throws under { strict: true }), since the host may have already fired the notification by then. hostcontextchanged is exempt — late listeners for repeating events are legitimate. Covers both the on* setters and addEventListener.
This commit is contained in:
parent
9d68315720
commit
6fa5d4e6ed
2 changed files with 136 additions and 0 deletions
|
|
@ -1965,6 +1965,99 @@ describe("App <-> AppBridge integration", () => {
|
|||
errSpy.mockRestore();
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
|
||||
it("throws instead of warning when strict: true", async () => {
|
||||
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
|
||||
const [strictAppT, strictBridgeT] =
|
||||
InMemoryTransport.createLinkedPair();
|
||||
const strictBridge = new AppBridge(
|
||||
createMockClient() as Client,
|
||||
testHostInfo,
|
||||
testHostCapabilities,
|
||||
);
|
||||
const strictApp = new App(
|
||||
testAppInfo,
|
||||
{},
|
||||
{ autoResize: false, strict: true },
|
||||
);
|
||||
await strictBridge.connect(strictBridgeT);
|
||||
await strictApp.connect(strictAppT);
|
||||
|
||||
expect(() => {
|
||||
strictApp.ontoolresult = () => {};
|
||||
}).toThrow(lateMsg);
|
||||
expect(() => {
|
||||
strictApp.addEventListener("toolinput", () => {});
|
||||
}).toThrow(lateMsg);
|
||||
expect(warnSpy).not.toHaveBeenCalled();
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
it("AppBridge warns on tools/call from a View that skipped the handshake", async () => {
|
||||
const warnSpy = spyOn(console, "warn").mockImplementation(() => {});
|
||||
bridge.oncalltool = async () => ({ content: [] });
|
||||
|
|
|
|||
43
src/app.ts
43
src/app.ts
|
|
@ -386,6 +386,49 @@ export class App extends ProtocolWithEvents<
|
|||
hostcontextchanged: McpUiHostContextChangedNotificationSchema,
|
||||
};
|
||||
|
||||
/**
|
||||
* Events the host typically sends once, shortly after the handshake.
|
||||
* Registering a handler for one of these *after* {@link connect `connect`}
|
||||
* resolves risks missing the notification entirely.
|
||||
*/
|
||||
private static readonly ONE_SHOT_EVENTS: ReadonlySet<keyof AppEventMap> =
|
||||
new Set(["toolinput", "toolinputpartial", "toolresult", "toolcancelled"]);
|
||||
|
||||
/**
|
||||
* Warn (or throw under `strict`) when a one-shot event handler is registered
|
||||
* after the `ui/initialize` → `ui/notifications/initialized` handshake has
|
||||
* completed. The host may have already fired the notification by then.
|
||||
*
|
||||
* Mirrors {@link _assertInitialized `_assertInitialized`} (the outbound-side guard).
|
||||
*/
|
||||
private _assertHandlerTiming(event: keyof AppEventMap): void {
|
||||
if (!this._initializedSent || !App.ONE_SHOT_EVENTS.has(event)) return;
|
||||
const msg =
|
||||
`[ext-apps] "${String(event)}" handler registered after connect() ` +
|
||||
`completed the ui/initialize handshake. The host may have already sent ` +
|
||||
`this notification. Register handlers before calling app.connect().`;
|
||||
if (this.options?.strict) {
|
||||
throw new Error(msg);
|
||||
}
|
||||
console.warn(msg);
|
||||
}
|
||||
|
||||
protected override setEventHandler<K extends keyof AppEventMap>(
|
||||
event: K,
|
||||
handler: ((params: AppEventMap[K]) => void) | undefined,
|
||||
): void {
|
||||
if (handler) this._assertHandlerTiming(event);
|
||||
super.setEventHandler(event, handler);
|
||||
}
|
||||
|
||||
override addEventListener<K extends keyof AppEventMap>(
|
||||
event: K,
|
||||
handler: (params: AppEventMap[K]) => void,
|
||||
): void {
|
||||
this._assertHandlerTiming(event);
|
||||
super.addEventListener(event, handler);
|
||||
}
|
||||
|
||||
protected override onEventDispatch<K extends keyof AppEventMap>(
|
||||
event: K,
|
||||
params: AppEventMap[K],
|
||||
|
|
|
|||
Loading…
Reference in a new issue