Skip to content

Commit ed8f06d

Browse files
committed
fix(web): bind inbox reportModelResolver + guard host capabilities
The web host never bound REPORT_MODEL_RESOLVER, so creating a cloud run from canvas/home/inbox threw "No bindings found" at the point of use. Bind it over the web host router's getPreviewConfigOptions, mirroring the desktop adapter. Prevent the whole class of gap: useService<T>(TOKEN) takes its type from the caller, so a host forgetting to bind a capability shared UI resolves is invisible to the compiler and only surfaces when a user hits the code path. Add assertHostCapabilities() in @posthog/di, a canonical REQUIRED_HOST_CAPABILITIES registry in @posthog/ui/shell, and call it at the end of both composition roots so a missing binding fails at boot. Stand up Vitest for apps/web with a container smoke test that locks the contract in for CI without a browser. Generated-By: PostHog Code Task-Id: 77d13a30-444d-4045-9d80-5e2a9f2e68ae
1 parent ac83888 commit ed8f06d

12 files changed

Lines changed: 392 additions & 24 deletions

File tree

apps/code/src/renderer/main.tsx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ import { Providers } from "@components/Providers";
1919
import { DevToolbarHost } from "@features/dev-toolbar/DevToolbarHost";
2020
import { preloadHighlighter } from "@pierre/diffs";
2121
import { boot } from "@posthog/di/contribution";
22+
import { assertHostCapabilities } from "@posthog/di/hostCapabilities";
2223
import { ServiceProvider } from "@posthog/di/react";
2324
import App from "@posthog/ui/shell/App";
2425
import { logger } from "@posthog/ui/shell/logger";
2526
import { initializePostHog } from "@posthog/ui/shell/posthogAnalyticsImpl";
27+
import { REQUIRED_HOST_CAPABILITIES } from "@posthog/ui/shell/requiredHostCapabilities";
2628
import { registerDesktopContributions } from "@renderer/desktop-contributions";
2729
import { container } from "@renderer/di/container";
2830
import "@renderer/desktop-services";
@@ -95,6 +97,11 @@ const root = ReactDOM.createRoot(rootElement);
9597

9698
try {
9799
registerDesktopContributions();
100+
// Fail loudly (into BootErrorScreen) if a capability the shared app resolves
101+
// via service location is unbound, rather than deferring to the first
102+
// navigation that needs it. The renderer container backs every useService, so
103+
// all required tokens must resolve here. Shared with the web host.
104+
assertHostCapabilities(container, REQUIRED_HOST_CAPABILITIES);
98105
boot(container).catch((error: unknown) => {
99106
bootLog.error("Renderer boot sequence failed", error);
100107
// Replaces the mounted tree without running effect cleanup; acceptable

apps/web/package.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
"scripts": {
88
"dev": "vite",
99
"build": "vite build",
10-
"typecheck": "tsc --noEmit"
10+
"typecheck": "tsc --noEmit",
11+
"test": "vitest run"
1112
},
1213
"dependencies": {
1314
"@agentclientprotocol/sdk": "0.22.1",
@@ -38,8 +39,10 @@
3839
"@types/react": "^19.1.0",
3940
"@types/react-dom": "^19.1.0",
4041
"@vitejs/plugin-react": "^4.2.1",
42+
"jsdom": "^26.0.0",
4143
"tailwindcss": "^4.2.2",
4244
"typescript": "^5.9.3",
43-
"vite": "^6.0.7"
45+
"vite": "^6.0.7",
46+
"vitest": "^4.1.8"
4447
}
4548
}

apps/web/src/test/setup.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { vi } from "vitest";
2+
3+
// jsdom leaves several browser globals that the web composition root touches at
4+
// import time undefined. Provide minimal stand-ins so importing web-container.ts
5+
// (and the stores it pulls in) doesn't throw before the container is built.
6+
7+
// localStorage: several per-device stores read it at module init.
8+
if (typeof window.localStorage?.setItem !== "function") {
9+
const store = new Map<string, string>();
10+
const localStoragePolyfill: Storage = {
11+
get length() {
12+
return store.size;
13+
},
14+
clear: () => store.clear(),
15+
getItem: (key) => store.get(key) ?? null,
16+
key: (index) => [...store.keys()][index] ?? null,
17+
removeItem: (key) => {
18+
store.delete(key);
19+
},
20+
setItem: (key, value) => {
21+
store.set(key, String(value));
22+
},
23+
};
24+
Object.defineProperty(window, "localStorage", {
25+
configurable: true,
26+
value: localStoragePolyfill,
27+
});
28+
Object.defineProperty(globalThis, "localStorage", {
29+
configurable: true,
30+
value: localStoragePolyfill,
31+
});
32+
}
33+
34+
// matchMedia: UI stores (e.g. themeStore) read it at module load.
35+
if (typeof window.matchMedia !== "function") {
36+
Object.defineProperty(window, "matchMedia", {
37+
configurable: true,
38+
writable: true,
39+
value: vi.fn().mockImplementation((query: string) => ({
40+
matches: false,
41+
media: query,
42+
onchange: null,
43+
addListener: vi.fn(),
44+
removeListener: vi.fn(),
45+
addEventListener: vi.fn(),
46+
removeEventListener: vi.fn(),
47+
dispatchEvent: vi.fn(),
48+
})),
49+
});
50+
}

apps/web/src/web-container.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { assertHostCapabilities } from "@posthog/di/hostCapabilities";
2+
import { REQUIRED_HOST_CAPABILITIES } from "@posthog/ui/shell/requiredHostCapabilities";
3+
import { describe, expect, it } from "vitest";
4+
5+
// The web host is the portability smoke test: a single in-browser container, no
6+
// Electron. This locks in that it binds every capability the shared app resolves
7+
// via service location, so a gap like the missing inbox reportModelResolver
8+
// fails in CI instead of at the first navigation that needs it.
9+
describe("web composition root", () => {
10+
it("binds every required host capability", async () => {
11+
// Importing the module builds the container and (as of the fix) already runs
12+
// assertHostCapabilities at the end; re-run it explicitly so a regression
13+
// fails here with the descriptive list rather than as a bare import error.
14+
const { container } = await import("./web-container");
15+
expect(() =>
16+
assertHostCapabilities(container, REQUIRED_HOST_CAPABILITIES),
17+
).not.toThrow();
18+
});
19+
});

apps/web/src/web-container.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ import {
5959
GIT_INTERACTION_SERVICE,
6060
GIT_WRITE_CLIENT,
6161
} from "@posthog/core/git-interaction/identifiers";
62+
import {
63+
REPORT_MODEL_RESOLVER,
64+
type ReportModelResolver,
65+
} from "@posthog/core/inbox/identifiers";
66+
import { selectModelFromOptions } from "@posthog/core/inbox/reportTaskCreation";
6267
import { githubConnectModule } from "@posthog/core/integrations/githubConnect.module";
6368
import {
6469
GITHUB_CONNECT_CLIENT as INTEGRATIONS_GITHUB_CONNECT_CLIENT,
@@ -144,6 +149,7 @@ import {
144149
import type { TaskDeletionService } from "@posthog/core/tasks/taskDeletionService";
145150
import { tasksModule } from "@posthog/core/tasks/tasks.module";
146151
import { setRootContainer } from "@posthog/di/container";
152+
import { assertHostCapabilities } from "@posthog/di/hostCapabilities";
147153
import { ROOT_LOGGER, type RootLogger } from "@posthog/di/logger";
148154
import {
149155
HOST_TRPC_CLIENT,
@@ -165,7 +171,7 @@ import {
165171
type IPowerManager,
166172
POWER_MANAGER_SERVICE,
167173
} from "@posthog/platform/power-manager";
168-
import { SYNC_CLOUD_TASKS_FLAG } from "@posthog/shared";
174+
import { type Adapter, SYNC_CLOUD_TASKS_FLAG } from "@posthog/shared";
169175
import { sandboxProxyHtml } from "@posthog/shared/mcp-sandbox-proxy";
170176
import { authUiModule } from "@posthog/ui/features/auth/auth.module";
171177
import {
@@ -259,6 +265,7 @@ import {
259265
IMPERATIVE_QUERY_CLIENT,
260266
type ImperativeQueryClient,
261267
} from "@posthog/ui/shell/queryClient";
268+
import { REQUIRED_HOST_CAPABILITIES } from "@posthog/ui/shell/requiredHostCapabilities";
262269
import { QueryClient } from "@tanstack/react-query";
263270
import {
264271
WebAuthConnectivity,
@@ -367,6 +374,7 @@ interface WebBindings {
367374
[NOTIFICATIONS_SERVICE]: INotifications;
368375
[NOTIFICATION_SETTINGS_PROVIDER]: INotificationSettings;
369376
[ACTIVE_VIEW_PROVIDER]: IActiveView;
377+
[REPORT_MODEL_RESOLVER]: ReportModelResolver;
370378
}
371379

372380
export const queryClient = new QueryClient();
@@ -756,4 +764,37 @@ container
756764
.toConstantValue(webNotificationSettings);
757765
container.bind(ACTIVE_VIEW_PROVIDER).toConstantValue(webActiveView);
758766

767+
// ── Inbox: resolve the default cloud-run model from the LLM gateway ──
768+
// Host capability consumed by UI hooks (canvas/home/inbox) that create cloud
769+
// runs. Same logic as desktop-services.ts, over the web host router's
770+
// getPreviewConfigOptions (backed by the CORS-open gateway, web-agent-config.ts).
771+
const reportModelResolverLog = scoped("report-model-resolver");
772+
container.bind(REPORT_MODEL_RESOLVER).toConstantValue({
773+
async resolveDefaultModel(
774+
apiHost: string,
775+
adapter: Adapter,
776+
preferredModel?: string | null,
777+
): Promise<string | undefined> {
778+
try {
779+
const options = await hostTrpcClient.agent.getPreviewConfigOptions.query({
780+
apiHost,
781+
adapter,
782+
});
783+
return selectModelFromOptions(options, preferredModel);
784+
} catch (error) {
785+
reportModelResolverLog.warn("Failed to resolve default model", {
786+
error,
787+
adapter,
788+
});
789+
return undefined;
790+
}
791+
},
792+
} satisfies ReportModelResolver);
793+
794+
// Fail loudly at composition time if a capability the shared app resolves via
795+
// service location is unbound, instead of limping to the first navigation that
796+
// needs it (how the missing reportModelResolver first surfaced). CI locks this
797+
// in via web-container.test.ts.
798+
assertHostCapabilities(container, REQUIRED_HOST_CAPABILITIES);
799+
759800
setRootContainer(container);

apps/web/vite.aliases.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import path from "node:path";
2+
import { fileURLToPath } from "node:url";
3+
4+
const dir = path.dirname(fileURLToPath(import.meta.url));
5+
const src = (name: string) => path.resolve(dir, `../../packages/${name}/src`);
6+
7+
// Mirror apps/code's vite.shared.mts: resolve @posthog/<pkg>/<sub> to package
8+
// src, since the packages' array-fallback `exports` don't resolve under Rollup.
9+
// Shared by vite.config.ts (build/dev) and vitest.config.ts (tests) so the two
10+
// can't drift.
11+
const subpath = (name: string) => ({
12+
find: new RegExp(`^@posthog/${name}/(.+)$`),
13+
replacement: `${src(name)}/$1`,
14+
});
15+
16+
export const posthogSrcAliases = [
17+
subpath("di"),
18+
subpath("ui"),
19+
subpath("core"),
20+
subpath("shared"),
21+
subpath("host-router"),
22+
subpath("host-trpc"),
23+
subpath("platform"),
24+
subpath("workspace-client"),
25+
subpath("api-client"),
26+
subpath("agent"),
27+
subpath("enricher"),
28+
];

apps/web/vite.config.ts

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,9 @@ import tailwindcss from "@tailwindcss/vite";
44
import { TanStackRouterVite } from "@tanstack/router-plugin/vite";
55
import react from "@vitejs/plugin-react";
66
import { defineConfig } from "vite";
7+
import { posthogSrcAliases } from "./vite.aliases";
78

89
const dir = path.dirname(fileURLToPath(import.meta.url));
9-
const src = (name: string) => path.resolve(dir, `../../packages/${name}/src`);
10-
11-
// Mirror apps/code's vite.shared.mts: resolve @posthog/<pkg>/<sub> to package src,
12-
// since the packages' array-fallback `exports` don't resolve under Rollup.
13-
const subpath = (name: string) => ({
14-
find: new RegExp(`^@posthog/${name}/(.+)$`),
15-
replacement: `${src(name)}/$1`,
16-
});
1710

1811
// Peel the biggest statically-imported vendors out of the single ~8.6 MB entry
1912
// chunk into cacheable groups so the browser downloads them in parallel and
@@ -74,19 +67,7 @@ export default defineConfig({
7467
// never gets a key.
7568
envDir: path.resolve(dir, "../.."),
7669
resolve: {
77-
alias: [
78-
subpath("di"),
79-
subpath("ui"),
80-
subpath("core"),
81-
subpath("shared"),
82-
subpath("host-router"),
83-
subpath("host-trpc"),
84-
subpath("platform"),
85-
subpath("workspace-client"),
86-
subpath("api-client"),
87-
subpath("agent"),
88-
subpath("enricher"),
89-
],
70+
alias: posthogSrcAliases,
9071
},
9172
server: { port: 5273 },
9273
build: {

apps/web/vitest.config.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import react from "@vitejs/plugin-react";
2+
import { defineConfig } from "vitest/config";
3+
import { trunkTestOptions } from "../../vitest.config.base";
4+
import { posthogSrcAliases } from "./vite.aliases";
5+
6+
// Deliberately does NOT load vite.config.ts: that would run TanStackRouterVite
7+
// and regenerate routeTree.gen.ts as a side effect of running tests. We only
8+
// need the @posthog/* → src aliases, shared via vite.aliases.ts.
9+
export default defineConfig({
10+
plugins: [react()],
11+
resolve: { alias: posthogSrcAliases },
12+
test: {
13+
globals: true,
14+
...trunkTestOptions,
15+
environment: "jsdom",
16+
setupFiles: ["./src/test/setup.ts"],
17+
include: ["src/**/*.test.ts", "src/**/*.test.tsx"],
18+
exclude: ["**/node_modules/**", "**/dist/**"],
19+
},
20+
});
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { describe, expect, it } from "vitest";
2+
import { assertHostCapabilities } from "./hostCapabilities";
3+
4+
const TOKEN_A = Symbol.for("posthog.test.capabilityA");
5+
const TOKEN_B = Symbol.for("posthog.test.capabilityB");
6+
7+
function containerBinding(bound: symbol[]): {
8+
isBound: (t: symbol) => boolean;
9+
} {
10+
const set = new Set(bound);
11+
return { isBound: (t) => set.has(t) };
12+
}
13+
14+
describe("assertHostCapabilities", () => {
15+
it("passes when every required token is bound", () => {
16+
const container = containerBinding([TOKEN_A, TOKEN_B]);
17+
expect(() =>
18+
assertHostCapabilities(container, [
19+
{ token: TOKEN_A, description: "A" },
20+
{ token: TOKEN_B, description: "B" },
21+
]),
22+
).not.toThrow();
23+
});
24+
25+
it("passes when there are no requirements", () => {
26+
expect(() =>
27+
assertHostCapabilities(containerBinding([]), []),
28+
).not.toThrow();
29+
});
30+
31+
it("throws listing every missing token and its description", () => {
32+
const container = containerBinding([TOKEN_A]);
33+
expect(() =>
34+
assertHostCapabilities(container, [
35+
{ token: TOKEN_A, description: "A is fine" },
36+
{ token: TOKEN_B, description: "B drives cloud runs" },
37+
]),
38+
).toThrowError(/missing 1 required capability/);
39+
});
40+
41+
it("reports all missing tokens at once, not just the first", () => {
42+
const container = containerBinding([]);
43+
let message = "";
44+
try {
45+
assertHostCapabilities(container, [
46+
{ token: TOKEN_A, description: "A drives X" },
47+
{ token: TOKEN_B, description: "B drives Y" },
48+
]);
49+
} catch (error) {
50+
message = (error as Error).message;
51+
}
52+
expect(message).toContain("missing 2 required capability");
53+
expect(message).toContain("A drives X");
54+
expect(message).toContain("B drives Y");
55+
expect(message).toContain(String(TOKEN_A));
56+
expect(message).toContain(String(TOKEN_B));
57+
});
58+
});
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import type { ServiceIdentifier } from "inversify";
2+
import type { ServiceContainer } from "./container";
3+
4+
/**
5+
* A DI token that shared `@posthog/ui` / `@posthog/core` resolves via service
6+
* location (`useService` / `resolveService`) and that every host mounting the
7+
* shared app must therefore bind in its composition root.
8+
*/
9+
export interface HostCapabilityRequirement {
10+
/** The token the shared app resolves at runtime. */
11+
readonly token: ServiceIdentifier;
12+
/** What breaks when it's missing — surfaced in the error message. */
13+
readonly description: string;
14+
}
15+
16+
/**
17+
* Throw if the host forgot to bind a capability the shared app resolves at
18+
* runtime.
19+
*
20+
* These gaps are invisible to the compiler: `useService<T>(TOKEN)`'s type
21+
* argument is supplied by the caller, not derived from any host's binding map,
22+
* so a `TypedContainer<HostBindings>` only checks the *provider* side. A missing
23+
* binding otherwise surfaces only when a user reaches the code path that
24+
* resolves the token (as happened with the inbox `reportModelResolver` on web).
25+
*
26+
* Call this synchronously at the end of each composition root, once every
27+
* binding is registered, so a broken container fails to start instead of
28+
* limping to the first unlucky navigation.
29+
*/
30+
export function assertHostCapabilities(
31+
container: Pick<ServiceContainer, "isBound">,
32+
requirements: readonly HostCapabilityRequirement[],
33+
): void {
34+
const missing = requirements.filter((r) => !container.isBound(r.token));
35+
if (missing.length === 0) {
36+
return;
37+
}
38+
39+
const lines = missing.map((r) => ` - ${String(r.token)}: ${r.description}`);
40+
throw new Error(
41+
`Host container is missing ${missing.length} required capability ` +
42+
`binding(s) that shared UI/core resolves at runtime:\n${lines.join("\n")}\n` +
43+
"Bind them in this host's composition root. See REQUIRED_HOST_CAPABILITIES " +
44+
"in @posthog/ui/shell/requiredHostCapabilities.",
45+
);
46+
}

0 commit comments

Comments
 (0)