Repository navigation
fix(sdk): run connect(), ConfigPlugin and skill search in the browser build - #762
Conversation
connect() (and connectOpenAI() / connectClaude()) builds its server through FrontMcpInstance.createForGraph(), which still selected the Express-backed server provider. #748 made createDirect() and createFetchHandler() pass serve: false; createForGraph() did not, so in the browser build connect() failed with "ExpressHostAdapter is not available in browser environments" (#761). It never listens, so it now passes serve: false too. createEdgeMcp() already passed it.
loadConfig() and loadEnvFiles() resolved .env, .env.local and config.yml with pathResolve() from @frontmcp/utils, whose browser build throws, so every ConfigPlugin failed at startup in a browser (#761). Where the runtime context is the browser build, the file sources now count as absent, and process.env is read only where it exists, so the config comes from the schema defaults and process.env. The browser-bundle e2e app runs connect() and a ConfigPlugin server in Chromium. Its other result checks now wait for the page summary, as the first ones do, instead of reading the results before the async checks finish. The browser compatibility, config and build-for-browser pages describe both entry points and the 'browser' runtime value.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughThe SDK skips filesystem configuration loading in browsers while retaining schema defaults and available ChangesRuntime compatibility
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change fixes browser-build behavior for configuration loading, entry points and skill search. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-10-05T14:55:34.109Z |
… browser
vectoriadb was an optional peer that the SDK loaded with a literal
import('vectoriadb') on the first skill search. A browser bundle of the
SDK therefore failed with 'Could not resolve "vectoriadb"' unless the
consumer installed it (#761), and installing it did not help: in 2.3.2
its worker entry imported Node's crypto and an undeclared tslib, fixed in
vectoriadb 2.3.3 (agentfront/vectoriadb#17).
vectoriadb (^2.3.3) is now a regular dependency. The SDK imports its
worker-safe entry, vectoriadb/worker: the TF-IDF/BM25 index the skill
search uses, with no dependencies beyond tslib and no Node built-ins. The
index is still created on the first search; the optional-peer loading
and the spec that pinned it are gone. The browser-bundle e2e app runs a
skill search in Chromium, and the skills, testing and create-skill pages
no longer call vectoriadb an optional peer.
The subpath vectoriadb/worker exists only in the package's exports map, which moduleResolution "node" ignores. tsconfig.base.json uses it, so the webpack builds that compile the SDK source, such as apps/demo, failed with TS2307 "Cannot find module 'vectoriadb/worker'". The SDK imports the package root instead. Its types resolve through the package's types field under any resolution mode, and its exports map lists the worker and browser conditions before import, so browser and worker bundles still get the TF-IDF entry without Node built-ins; Node gets the full entry. A browser bundle of the SDK includes only vectoriadb's worker.js, vectoria-tfidf.js, tfidf.embedding.service.js and sanitize.utils.js.
Cherry-pick CreatedA cherry-pick PR to Please review and merge if this change should also be in If the cherry-pick is not needed, close the PR. |
…re-verifying 1.9.1 (#771) * fix(cli): list compiled CLI daemons, install from ".", and see wildcard port listeners frontmcp list and frontmcp status crashed with "Cannot read properties of undefined (reading 'replace')" once a compiled CLI ran `<bin> daemon start`: the daemon's PID file holds only pid, startedAt and port/socketPath, and the process table padded the missing name. readPidFile() now fills in the name from the file name and defaults the other fields, skips a file without a numeric pid, and the daemon writes its name, entry and restart count. frontmcp install treated a bare "." (and "..") as an npm package name, so it packed and rebuilt the project; only "./" was a local path. ".", "..", backslash forms and Windows drive paths are now local sources. On macOS a bind to 127.0.0.1 succeeds next to a 0.0.0.0 or :: listener, so frontmcp dev --port <port> started anyway when another server held the port on all interfaces. isPortFree() also probes the wildcard address of the host's family. Refs #768 * fix(cli,sdk,observability): start Cloudflare Workers with metrics and honor --out-dir in wrangler.toml A cloudflare build with metrics: { enabled: true } answered every request 503 SERVER_START_FAILED under wrangler dev, logging No such module "@frontmcp/observability", even with the package installed. The SDK loads observability with a lazy require() to keep it optional, and workerd's require() cannot reach modules in the bundle, so the module was never found. The SDK now exports registerOptionalModule(): when the entry's config enables metrics or observability, the generated worker entry imports @frontmcp/observability and registers it, and the SDK's metrics service and observability auto-load check that registry before require(). The build warns when the config needs the package and it is not installed. With the module found, the ProcessStatsCollector threw next: Cloudflare's unenv process does not implement process.cpuUsage(), memoryUsage() or the event-loop histogram's reset(). Each probe now drops its gauges when the runtime throws instead of failing the scrape. frontmcp build --target cloudflare --out-dir X wrote main = "dist/cloudflare/index.js" into wrangler.toml whatever the output directory was. getConfig() and mergeConfig() now receive the target's output directory, and main points at its index.js. The cloudflare e2e fixture now enables metrics and the worker spec scrapes /metrics under workerd. Refs #768 * fix(sdk,utils): merge redis fields written beside a url, and accept url in throttle storage config redis: { url: "redis://cache:6379", password: "p" } dropped the password silently: the URL branch of the redis schema stripped every other field. redis: { url, host } ignored the URL entirely, port included, because the host-only branch was tried first and stripped the url. The URL is now the base and the fields beside it fill in only what the URL leaves out (port, password, db, tls: true). A field that contradicts the URL (another host, port, password or db, or tls: false with rediss://) is a validation error naming the field, without echoing any value. The URL branch is tried first and the host-only branches refuse a url, in redis and pubsub alike. mergeRedisUrlFields() in @frontmcp/utils holds the rule. throttle.storage's redis.config only took host fields, while the top-level redis option takes { url }. config now also accepts { url }, and the Redis storage adapter applies the same rule: fields beside the URL fill in its gaps (passed to ioredis, which keeps what the URL states), contradictions and two different URLs fail at construction, and the subscriber connection gets the same fill-ins. Refs #768 * fix(cli): harden the scaffolded Dockerfile, compose file and SAM template frontmcp create's ci/Dockerfile ran the server as root and declared no health check. The runner stage now copies the build as the image's node user, makes /app writable by it, switches to USER node, and declares a HEALTHCHECK that fetches /healthz with node itself (the slim image has no curl). ci/docker-compose.yml published Redis on 6379 on every host interface, and both compose variants defaulted the app's NODE_ENV to development though the image is a production build. Redis is now published on 127.0.0.1 only (a server on the host still reaches it for local development), NODE_ENV defaults to production, and MCP_SESSION_SECRET, which production requires, is passed through and listed in ci/.env.docker. The Lambda SAM template set no NODE_ENV. It now runs the function with NODE_ENV=production and takes MCP_SESSION_SECRET from a NoEcho McpSessionSecret parameter. Refs #768 * fix(nx): build and test UI packages in Nx TypeScript-solution workspaces In a create-nx-workspace --preset=ts workspace, nx g @frontmcp/nx:ui-shell followed by nx build ui-shells stopped in build-esm with TS5069 on declarationMap and emitDeclarationOnly: @nx/esbuild type-checks a target without declarations in noEmit mode with composite: false, and the preset's base config sets those options. build-esm now skips the type check; build-cjs still type-checks the same sources and emits the declarations. The package had no test target there: the generated jest.config.cjs only becomes a target through @nx/jest/plugin, which that preset does not register. When nx.json has no @nx/jest/plugin, the package now gets a test target that runs jest with its config, and the generator installs Jest and @swc/jest. The generator added @nx/esbuild 22.6.4 to a workspace on Nx 23.2.0. It now installs @nx/esbuild at the workspace's nx version, falling back to the plugin's when package.json declares none. Refs #768 * fix(react): export bindDynamicTools and hand a shared tool back to its remaining registrant bindDynamicTools was internal to FrontMcpProvider, although DynamicRegistry is public and #684 presents the binder as API. It is now exported from @frontmcp/react with its options type, and the dynamic tools page shows it. When two mounted components called useDynamicTool({ name: "x" }), the second registration replaced the definition, and when that component unmounted the registry only decremented a count: calls kept running the unmounted component's execute. The registry now keeps each registration, and when the one in use goes away the newest remaining one takes over (and the change is announced, so the server follows it). Refs #769 * fix(uipack): HTML-escape plain string template results by default, as 1.9 documents The 1.9 docs say "FrontMCP 1.9 escapes string results by default", and the notice added in 1.8 (#612) said 1.9 would. 1.9.0 shipped without the switch: a template function's plain string that looked like HTML was still rendered as markup, while the notice kept announcing the change. With ui.escapeStringResults unset, a plain string result is now escaped and shown as text. A string that looked like markup logs a one-time notice per tool that says so and names html`...` / trustedHtml() and the escapeStringResults: false opt-out. true escapes without the notice; false keeps the 1.8 behaviour. html`...` and trustedHtml() results, static string templates and the other content paths are unchanged. The demo-e2e-ui and demo-e2e-hr fixtures return plain markup strings and escape values themselves, so they set ui.escapeStringResults: false. The Tool UI guide, tools and @tool reference pages and the ui-widgets skill describe the 1.9 default and the migration. Refs #769 * fix(sdk,uipack): warn about a React component ui.template at startup #692 made the "ui.template is a React component reference" warning appear once, at startup. For tools in the default inline serving mode it appeared on the first call instead: the startup step compiles their lean widget shell without the template, so the template was first rendered by a call. uipack exports warnIfComponentReference(), which runs the renderer's own detection (a class, memo / forwardRef, or a capitalized function that throws or returns an element when called outside React) and shares its once-per-tool record. The scope calls it through ToolUIRegistry.checkTemplate() while it registers each UI tool, whatever the serving mode, and the later render does not warn again. Refs #769 * docs(ui): show useCallTool's tuple return in the @frontmcp/ui README The README destructured useCallTool('get_weather') as { call, loading }; the hook returns [call, state, reset]. The example now destructures the tuple. Refs #769 * fix(plugin-webmcp): make the default WebMCP caller anonymous; document fallback elicitation The docs promise an anonymous webmcp caller when no authContext is given, but the bridge called with sub: "webmcp", which is a signed-in subject: this.auth.isAnonymous was false, and authorities and quotas treated every page visitor as one user named "webmcp". A subject is anonymous when it starts with "anon:", so the default caller is now sub: "anon:webmcp", marked anonymous. An authContext user keeps its own subject. The limitations said a tool that elicits fails for in-browser agents. With elicitation.enabled, the tool answers with the fallback instructions, the agent is offered sendElicitationResult and the round trip completes. The WebMCP page and the official-plugins skill now say so, and that sampling is unavailable. Refs #769 * fix(sdk,edge): name the invalid config fields in the start-failure log A Worker whose config failed validation answered 500 CONFIG_INVALID with "the server log names the invalid fields", but the log printed only ZodError and a stack: the config schema is a union, so its single issue was "Invalid input" at the root. describeConfigIssues() (exported from @frontmcp/sdk) lists a validation error's fields as path: message pairs, reporting a failed union through its closest branch (apps: Invalid input: expected array, received undefined). createFetchHandler's deferred build, createEdgeMcp and the session Durable Object add it to their start-failure log line. The response itself is unchanged and still echoes no message. Refs #769 * fix(edge,adapters): make skilled-openapi a dependency of @frontmcp/edge; name the KV cache in the pull-failure error A minimal createEdgeMcp() Worker, without managed mode, failed under wrangler dev with Could not resolve "@frontmcp/plugin-skilled-openapi": the package was an optional peer, but createEdgeMcp imports it with a literal import() for managed mode, so every bundle has to resolve it. As with vectoriadb in #762, it is now a regular dependency of @frontmcp/edge. It is still imported only when managed mode starts. When the boot pull failed and no last-good bundle was cached, the error named a file path even when the cache was an injected store such as Cloudflare KV (and computing that path is what the browser build of @frontmcp/utils refuses). With an injected cache it now says no bundle is available in the injected bundle cache. The Cloudflare page, the @frontmcp/edge README and the skills-only deploy skill no longer ask for bundler stubs (on 1.9.1 wrangler resolves the SDK's browser build), say that managed mode boots under wrangler dev, and describe the CONFIG_INVALID log and the cache message. Refs #769 * fix(cli): read ci/.env.docker in the scaffolded compose scripts and stop on a missing session secret c9ff99f made the scaffolded compose file default NODE_ENV to production and read MCP_SESSION_SECRET with ${MCP_SESSION_SECRET:-}, but the docker:up / docker:down / docker:build scripts ran docker compose without --env-file. Compose reads only .env by itself, so the secret users put in ci/.env.docker never arrived: every initialize answered 500 SESSION_SECRET_REQUIRED while the health check reported healthy. The scripts now pass --env-file ci/.env.docker, and the compose file reads ${MCP_SESSION_SECRET:?set MCP_SESSION_SECRET in ci/.env.docker ...}, so compose refuses to start until the secret is set (checked with docker compose config). A value exported in the shell still wins over the file. The generated README, compose comments, .env.docker template, Redis and local-dev pages and the setup-redis skill pass --env-file too. Refs #768 * fix(cli): bundle @frontmcp/observability into Workers whose metrics block is env-gated 0653031 imported @frontmcp/observability in the worker entry only when the config, as evaluated at build time, enabled metrics or observability. metrics: process.env.METRICS ? { enabled: true } : undefined, built with METRICS unset and deployed with it set, therefore shipped without the peer, and the Worker answered 503 again. The build now bundles the installed peer whenever the @FrontMcp source names metrics or observability. It still warns about a missing peer only when the evaluated config turns them on. Refs #768 * fix(utils): treat a Redis config whose url is undefined as a host config The Redis storage adapter decided between a URL config and a host config with 'url' in config, which is true for { host: 'cache', url: undefined }, a shape a config assembled from optional settings easily has. The adapter then took no URL from it and refused to build host options ("Redis config is required when URL is not provided"). Both checks now test typeof config.url === 'string', and the host config type allows url?: undefined. Refs #768 * fix(uipack): decide a template is a React component at startup without calling it 4b397cf's startup check called a capitalized template function with an empty context and took any exception as "this is a component". A capitalized HTML builder that reads its call data, such as function OrderCard(ctx) { return `<b>${ctx.output.order.id}</b>` } (the #645 pattern), threw there, so the server warned at boot about a template that renders fine, and user template code ran at startup. warnIfComponentReference() no longer calls the template. It warns for a class, a memo / forwardRef object, or a function whose source holds compiled JSX (jsx / jsxs / jsxDEV / React.createElement). A function that only throws without call data is inconclusive and is left to the existing render-time check. Refs #769 * test(sdk): cover the startup React component reference warning through a booted server The startup warning is wired in the scope (ToolUIRegistry.checkTemplate while UI tools register), which no test exercised. A server booted with createDirect() now has an inline-mode tool whose template is a class component, which must be named in a warning before any call, and a capitalized HTML builder that reads its call data, which must neither warn nor run at startup. The spec fails without the scope wiring. Refs #769 * docs(skills): show the inline UI template example with ctx.helpers.html The create-tool skill, its example 22 and the manifest still described the inline widget as ui: { template: (ctx) => '<div>…</div>' }, a plain string that 1.9 now escapes and shows as text. They now show ctx.helpers.html`<div>…</div>`. The example relies on the 1.9 default instead of setting escapeStringResults: true, and explains that a plain markup string is escaped. No other catalog or docs page returns a plain markup string from a template function. Refs #769 * docs: describe escaped plain string template results as the 1.9 default The Tool UI warning in servers/tools.mdx still said a plain string result renders as markup and told readers to set escapeStringResults: true, which is now the default. It now says strings are escaped since 1.9 and that escapeStringResults: false restores the 1.8 behaviour. The @tool reference and the CSP-and-bridge skill example no longer describe the default as upcoming. Refs #769 * fix(cli): skip pid files whose pid is not a positive integer readPidFile accepted any number, so a PID file holding 0, -1 or 1.5 was listed. process.kill(0, 0) signals the caller's own process group and -1 every process the user may signal, so those records read as live and could be targeted by stop. A pid must now be a positive integer, or the file is skipped like one without a pid. Refs #768 * fix(nx): add the UI package's test target unless @nx/jest/plugin infers one named test The generator skipped its own test target whenever @nx/jest/plugin was registered, so a workspace that sets the plugin's targetName to another name (unit, for instance) got a UI package with no test target. It now adds the target unless the plugin infers one named test: registered as a string, or with targetName unset or 'test'. Refs #768 * fix(observability): take the CPU baseline on a later scrape when the first probe fails ProcessStatsCollector read its CPU baseline once, in the constructor. If cpuUsage() threw then, the baseline stayed unset and every later scrape skipped the CPU gauges, even after the probe recovered. A scrape now takes the baseline when there is none, and the following scrapes report CPU time from that point. Refs #768 * docs(cli): describe the default Workers compatibility date precisely The cloudflare build's default compatibility_date, 2024-09-23, was described as "the date that enables full nodejs_compat". It is the first date on which nodejs_compat turns on nodejs_compat_v2; which Node.js APIs a Worker gets still depends on the date, and some (node:http, node:https) need a later one. The deploy-to-cloudflare reference, the production checklist, the config type, its JSON schema and the adapter spec now say so. Refs #768 * docs(deployment): pass MCP_SESSION_SECRET in the hand-written SAM templates The scaffolded Lambda template takes MCP_SESSION_SECRET from the NoEcho parameter McpSessionSecret and runs with NODE_ENV=production, but the hand-written templates in the deploy-to-lambda reference and the serverless page did not define the parameter, so a copied template started without the secret that production requires. Both now declare McpSessionSecret and map it into the function's environment. Refs #768 * fix(uipack): warn about _jsxDEV components and React.memo / forwardRef templates The startup check for a React component reference missed two kinds: - Components compiled by the development JSX runtime call _jsxDEV(...). The pattern allowed the underscore only before jsx / jsxs, and \bjsxDEV has no word boundary after an underscore, so these warned only at render time. - React.memo() and React.forwardRef() return objects tagged with $$typeof. The check accepted functions only, and the renderer sent any non-FileSource object to the empty shell without a warning, so these never warned at all. Both are now recognized at startup, and the renderer warns once for a memo / forwardRef object it is given directly. Refs #769 * fix(cli): keep the scaffolded ci/.env.docker, which holds MCP_SESSION_SECRET, out of git The scaffold tells users to put MCP_SESSION_SECRET (and REDIS_PASSWORD) in ci/.env.docker, but the generated .gitignore ignored only .env, .env.local and .env.*.local, so following the instructions committed the secret. The generated .gitignore now ignores .env.docker, .dockerignore keeps ci/.env.docker out of the image's build context, and the scaffold also writes ci/.env.docker.example as the committed template to copy after cloning. The generated README, the file's own comments, the quickstart tree, the Redis setup page and the setup-redis skill say so. Refs #768 * fix(utils): type a Redis config that only fills in what a top-level url leaves out RedisStorageAdapter merges config fields into a top-level url at runtime, as its JSDoc says, but the type of config accepted only a host config or a config with its own url. So the useful form { url, config: { password } } failed to compile, and the adapter spec worked around it by adding a host. config now also accepts RedisUrlFillInConfig (no host, no url), exported from @frontmcp/utils; without a url the adapter still rejects it at runtime, as before. Refs #768
Fixes #761. This fixes the three things that still failed in the SDK's browser build on 1.9.1-rc.2:
connect(),ConfigPlugin, and bundling the SDK withoutvectoriadbinstalled. It also documents thebrowserruntime value the issue notes.connect()built the Express hostconnect(),connectOpenAI()andconnectClaude()build their server throughFrontMcpInstance.createForGraph(). That function still selected the Express-backed server provider. #748 madecreateDirect()andcreateFetchHandler()passserve: false, butcreateForGraph()didn't. So in a browser,connect()threwFailed to construct provider "FrontMcpServer": ExpressHostAdapter is not available in browser environments.createForGraph()never listens, so it now passesserve: falsetoo.createEdgeMcp()already passed it.ConfigPluginthrew at startuploadConfig()andloadEnvFiles()resolved.env,.env.localandconfig.ymlwithpathResolve(). That function throws in the browser build of@frontmcp/utils, andloadEnvis on by default, so everyConfigPluginfailed withpath.resolve() is not available in browser environments.Where the runtime context is the browser build, the file sources now count as absent, and
process.envis read only where it exists. The config then comes from the schema defaults andprocess.env, as the issue expects. The legacy (no-schema) path uses the same helper. In Node nothing changes.vectoriadbcould not be bundledThe SDK loaded its optional peer
vectoriadbwith a literalimport('vectoriadb')on the first skill search. A browser bundle of the SDK failed withCould not resolve "vectoriadb"unless the consumer installed it. Installing it didn't help either: in 2.3.2, its worker entry imported Node'scryptoand an undeclaredtslib. That is fixed in vectoriadb 2.3.3 (agentfront/vectoriadb#17; agentfront/vectoriadb#19 fixed its publish workflow).vectoriadb@^2.3.3is now a regular dependency of@frontmcp/sdk.workerandbrowserconditions first, so browser and worker bundles get its TF-IDF/BM25 entry, with no Node built-ins and no ML runtime. A browser bundle of the SDK includes onlyworker.js,vectoria-tfidf.js,tfidf.embedding.service.jsandsanitize.utils.js. In Node it resolves to the full entry.vectoriadb/worker: a subpath exists only in the exports map, whichmoduleResolution: "node"(used bytsconfig.base.json) ignores. The webpack builds that compile the SDK source, such asapps/demo, failed withTS2307.Verification
Clean consumer repro: I installed the published
@frontmcp/sdk@1.9.1-rc.2with no optional peers and bundled the issue's repro with esbuild (platform: browser,conditions: browser,import).createFetchHandlerconnect()ExpressHostAdapter is not available...ConfigPluginpath.resolve() is not available...20)searchSkillsCould not resolve "vectoriadb"deploy-appThe branch run used this branch's build and
vectoriadb@2.3.3from npm, with onlynode:*external: no shims, no--external:vectoriadb. On this branch,PAGESIZE=50still reaches the config.Unit tests:
no-listen-entry-points.browser-host.spec.tsgains aconnect()case.config-loader.browser.spec.tsstubs@frontmcp/utilsthe way its browser build behaves. It checks defaults with no file reads, YAML skipped,process.envstill applied, and no.envfiles found.Both fail on the unfixed code.
Chromium e2e:
demo-e2e-browser-bundlerunsconnect(), aConfigPluginserver and a skill search in the browser, and 11/11 tests pass. Without the fixes, the new test fails with the rc.2 error. Its other result checks now wait for the page summary instead of reading results before the async checks finish.Full SDK suite: 457 suites, 6188 tests; the deleted optional-peer spec accounts for the drop.
tsc --noEmitis clean, eslint has 0 errors,yarn install --immutablepasses, and Prettier and the skills manifest are clean.Not in this PR
@frontmcp/utils: its ESM entry importsnode:cryptoat the top, so a plain esbuild browser bundle needs Node built-ins marked external. Vite already handles this.