-
Notifications
You must be signed in to change notification settings - Fork 434
feat: add support for disabling default plugins and configuring #3221
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.
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
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3c63601
feat: add support for disabling default plugins and configuring integ…
manucorporat 7e4547e
docs: resolve merge conflict in environment-variables.md
Copilot ed681e2
Merge branch 'main' into fix-config-disable-plugins
manucorporat 5f9c43e
fix: surface invalid plugin and integration-platform configuration
manucorporat 77fbc01
Merge remote-tracking branch 'origin/main' into fix-config-disable-pl…
manucorporat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| --- | ||
| "@agent-native/core": minor | ||
| --- | ||
|
|
||
| Let a deployment refuse framework default plugins and narrow which integration | ||
| platforms mount, without writing a stub plugin file. | ||
|
|
||
| `plugins.disabled` (env `AGENT_NATIVE_DISABLED_PLUGINS`) names default plugin | ||
| slots the framework should not auto-mount — the same list that shows up as | ||
| `[agent-native] Auto-mounting N default plugin(s)` under `DEBUG`. It is honored | ||
| by the runtime bootstrap and by the generated edge worker entry, so a slot is | ||
| withheld on every host. An app that ships its own `server/plugins/<slot>.ts` is | ||
| unaffected. | ||
|
|
||
| `integrations.platforms` (env `AGENT_NATIVE_INTEGRATION_PLATFORMS`) is an | ||
| allow-list of platforms for the integrations plugin, matched against each | ||
| adapter's `platform` id. Unset mounts every adapter, as before; a name no | ||
| adapter provides throws at plugin init rather than silently mounting a set | ||
| nobody asked for. | ||
|
|
||
| Both switches withhold registration rather than reject at request time: a | ||
| refused slot never runs its plugin, so its routes are absent from the | ||
| middleware chain and its background jobs and pollers never start. The | ||
| allow-list now also gates the routes mounted under a platform's literal name — | ||
| `/slack/interactions`, `/slack/manifest`, and the two Slack OAuth endpoints | ||
| previously stayed mounted whatever the adapter set was. They are gated only | ||
| when `integrations.platforms` is declared, so a deployment that does not set it | ||
| keeps today's behavior. | ||
|
|
||
| A misconfigured value in either switch is reported, not absorbed. An unknown | ||
| slot name in `plugins.disabled` fails at `getH3App()` rather than inside the | ||
| best-effort auto-mount catch, and the allow-list mismatch throws a typed | ||
| `AppConfigurationError` that the auto-mount catch rethrows — otherwise a typo | ||
| left the deployment reporting success with whole route trees missing. |
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| /** | ||
| * An invalid deployment configuration value, as opposed to a runtime failure. | ||
| * | ||
| * Best-effort regions that log and continue — plugin auto-mount, most of all — | ||
| * must rethrow this rather than absorb it: a typo in a deployment variable | ||
| * silently drops whole route trees, so the deployment looks accepted while the | ||
| * app is missing. | ||
| */ | ||
| export class AppConfigurationError extends Error { | ||
| constructor(message: string) { | ||
| super(message); | ||
| this.name = "AppConfigurationError"; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { afterEach, beforeEach, describe, expect, it } from "vitest"; | ||
|
|
||
| import { DEFAULT_PLUGIN_REGISTRY } from "../deploy/route-discovery.js"; | ||
| import { DEFAULT_PLUGIN_SLOTS } from "./plugins.js"; | ||
| import { | ||
| defineAppConfig, | ||
| getAppConfig, | ||
| resetAppConfigForTests, | ||
| } from "./store.js"; | ||
|
|
||
| const originalEnv = { ...process.env }; | ||
|
|
||
| describe("plugins config", () => { | ||
| beforeEach(() => { | ||
| resetAppConfigForTests(); | ||
| process.env = { ...originalEnv }; | ||
| delete process.env.AGENT_NATIVE_DISABLED_PLUGINS; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| resetAppConfigForTests(); | ||
| process.env = { ...originalEnv }; | ||
| }); | ||
|
|
||
| // The enum is spelled out in the schema so it stays edge-safe, which only | ||
| // works if a new default plugin slot fails here instead of quietly becoming | ||
| // the one plugin nobody can turn off. | ||
| it("covers every slot in DEFAULT_PLUGIN_REGISTRY", () => { | ||
| expect([...DEFAULT_PLUGIN_SLOTS].sort()).toEqual( | ||
| Object.keys(DEFAULT_PLUGIN_REGISTRY).sort(), | ||
| ); | ||
| }); | ||
|
|
||
| it("defaults to refusing nothing", () => { | ||
| expect(getAppConfig().plugins.disabled).toEqual([]); | ||
| }); | ||
|
|
||
| it("reads a comma-separated environment alias", () => { | ||
| process.env.AGENT_NATIVE_DISABLED_PLUGINS = "terminal, integrations"; | ||
| expect(getAppConfig().plugins.disabled).toEqual([ | ||
| "terminal", | ||
| "integrations", | ||
| ]); | ||
| }); | ||
|
|
||
| it("rejects a slot name that does not exist", () => { | ||
| process.env.AGENT_NATIVE_DISABLED_PLUGINS = "termnial"; | ||
| expect(() => getAppConfig()).toThrow(); | ||
| }); | ||
|
|
||
| it("lets an explicit value win over the environment alias", () => { | ||
| process.env.AGENT_NATIVE_DISABLED_PLUGINS = "terminal"; | ||
| defineAppConfig({ plugins: { disabled: [] } }); | ||
| expect(getAppConfig().plugins.disabled).toEqual([]); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { z } from "zod"; | ||
|
|
||
| /** | ||
| * The framework's default plugin slots, mirroring `DEFAULT_PLUGIN_REGISTRY`. | ||
| * | ||
| * Spelled out here rather than imported: that registry lives in the deploy | ||
| * layer, which reaches for `node:fs`, and this schema is parsed on edge | ||
| * runtimes too. `plugins.spec.ts` fails when the two lists drift. | ||
| */ | ||
| export const DEFAULT_PLUGIN_SLOTS = [ | ||
| "agent-chat", | ||
| "auth", | ||
| "context-xray", | ||
| "core-routes", | ||
| "integrations", | ||
| "observational-memory", | ||
| "onboarding", | ||
| "org", | ||
| "resources", | ||
| "sentry", | ||
| "terminal", | ||
| ] as const; | ||
|
|
||
| export type DefaultPluginSlot = (typeof DEFAULT_PLUGIN_SLOTS)[number]; | ||
|
|
||
| /** | ||
| * Which framework default plugins this deployment refuses. | ||
| * | ||
| * A refused slot mounts nothing, so every route it owns 404s and the UI and | ||
| * agent surfaces that call them stop working — `agent-chat`, `auth`, and | ||
| * `core-routes` carry most of an app with them. Only the framework's own | ||
| * default is withheld: an app that ships `server/plugins/<slot>.ts` mounted | ||
| * that plugin deliberately and keeps it. | ||
| */ | ||
| export const pluginsConfig = z.object({ | ||
| disabled: z | ||
| .array(z.enum(DEFAULT_PLUGIN_SLOTS)) | ||
| .default([]) | ||
| .meta({ | ||
| env: ["AGENT_NATIVE_DISABLED_PLUGINS"], | ||
| doc: "Framework default plugins this deployment refuses to auto-mount, comma-separated. A refused slot mounts none of its routes; an app supplying its own `server/plugins/<slot>.ts` is unaffected.", | ||
| }), | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Keep generated edge plugin gating edge-safe
The generated edge entry calls
isDefaultPluginDisabled()for every default plugin, but that helper delegates togetAppConfig(), which unconditionally reads bareprocess.env. Edge runtimes without Node’sprocesscan therefore throwReferenceError: process is not definedduring plugin mounting, even with no disabled plugins configured. Use the worker’s runtime environment mechanism or inject the resolved disabled-slot list at build time instead of calling the Node-oriented config store.Additional Info
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not reproducible — the same generated worker entry already requires
process.envbefore this line. It statically mountsdefaultAgentChatPlugin, whose mount body reads bareprocess.env.NODE_ENV(agent-chat-plugin.ts:654) and callsgetAppConfig()(:659), anddefaultCoreRoutesPlugin, which does the same.isDefaultPluginDisabled()adds no requirement the edge bundle did not already have, so gating it here would not make edge mountingprocess-free. Leaving as is.