diff --git a/AGENTS.md b/AGENTS.md index 9bb22dafbb3..e2b1d76a20b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -115,6 +115,8 @@ - Dynamic import guardrail: do not mix `await import("x")` and static `import ... from "x"` for the same module in production code paths. If you need lazy loading, create a dedicated `*.runtime.ts` boundary (that re-exports from `x`) and dynamically import that boundary from lazy callers only. - Dynamic import verification: after refactors that touch lazy-loading/module boundaries, run `pnpm build` and check for `[INEFFECTIVE_DYNAMIC_IMPORT]` warnings before submitting. - Extension SDK self-import guardrail: inside an extension package, do not import that same extension via `openclaw/plugin-sdk/` from production files. Route internal imports through a local barrel such as `./api.ts` or `./runtime-api.ts`, and keep the `plugin-sdk/` path as the external contract only. +- Extension package boundary guardrail: inside `extensions//**`, do not use relative imports/exports that resolve outside that same `extensions/` package root. If shared code belongs in the plugin SDK, import `openclaw/plugin-sdk/` instead of reaching into `src/plugin-sdk/**` or other repo paths via `../`. +- Extension API surface rule: `openclaw/plugin-sdk/` is the only public cross-package contract for extension-facing SDK code. If an extension needs a new seam, add a public subpath first; do not reach into `src/plugin-sdk/**` by relative path. - Never share class behavior via prototype mutation (`applyPrototypeMixins`, `Object.defineProperty` on `.prototype`, or exporting `Class.prototype` for merges). Use explicit inheritance/composition (`A extends B extends C`) or helper composition so TypeScript can typecheck. - If this pattern is needed, stop and get explicit approval before shipping; default behavior is to split/refactor into an explicit class hierarchy and keep members strongly typed. - In tests, prefer per-instance stubs over prototype mutation (`SomeClass.prototype.method = ...`) unless a test explicitly documents why prototype-level patching is required. diff --git a/package.json b/package.json index 4f898f41b49..6c1d30a51f6 100644 --- a/package.json +++ b/package.json @@ -466,7 +466,7 @@ "build:plugin-sdk:dts": "tsc -p tsconfig.plugin-sdk.dts.json", "build:strict-smoke": "pnpm canvas:a2ui:bundle && node scripts/tsdown-build.mjs && node scripts/runtime-postbuild.mjs && pnpm build:plugin-sdk:dts", "canvas:a2ui:bundle": "bash scripts/bundle-a2ui.sh", - "check": "pnpm check:host-env-policy:swift && pnpm check:bundled-provider-auth-env-vars && pnpm format:check && pnpm tsgo && pnpm plugin-sdk:check-exports && pnpm lint && pnpm lint:tmp:no-random-messaging && pnpm lint:tmp:channel-agnostic-boundaries && pnpm lint:tmp:no-raw-channel-fetch && pnpm lint:agent:ingress-owner && pnpm lint:plugins:no-register-http-handler && pnpm lint:plugins:no-monolithic-plugin-sdk-entry-imports && pnpm lint:plugins:no-extension-src-imports && pnpm lint:plugins:no-extension-test-core-imports && pnpm lint:plugins:no-extension-imports && pnpm lint:extensions:no-src-outside-plugin-sdk && pnpm lint:extensions:no-plugin-sdk-internal && pnpm lint:web-search-provider-boundaries && pnpm lint:webhook:no-low-level-body-read && pnpm lint:auth:no-pairing-store-group && pnpm lint:auth:pairing-account-scope", + "check": "pnpm check:host-env-policy:swift && pnpm check:bundled-provider-auth-env-vars && pnpm format:check && pnpm tsgo && pnpm plugin-sdk:check-exports && pnpm lint && pnpm lint:tmp:no-random-messaging && pnpm lint:tmp:channel-agnostic-boundaries && pnpm lint:tmp:no-raw-channel-fetch && pnpm lint:agent:ingress-owner && pnpm lint:plugins:no-register-http-handler && pnpm lint:plugins:no-monolithic-plugin-sdk-entry-imports && pnpm lint:plugins:no-extension-src-imports && pnpm lint:plugins:no-extension-test-core-imports && pnpm lint:plugins:no-extension-imports && pnpm lint:extensions:no-src-outside-plugin-sdk && pnpm lint:extensions:no-plugin-sdk-internal && pnpm lint:extensions:no-relative-outside-package && pnpm lint:web-search-provider-boundaries && pnpm lint:webhook:no-low-level-body-read && pnpm lint:auth:no-pairing-store-group && pnpm lint:auth:pairing-account-scope", "check:bundled-provider-auth-env-vars": "node scripts/generate-bundled-provider-auth-env-vars.mjs --check", "check:docs": "pnpm format:docs:check && pnpm lint:docs && pnpm docs:check-i18n-glossary && pnpm docs:check-links", "check:host-env-policy:swift": "node scripts/generate-host-env-security-policy-swift.mjs --check", @@ -519,6 +519,7 @@ "lint:docs": "pnpm dlx markdownlint-cli2", "lint:docs:fix": "pnpm dlx markdownlint-cli2 --fix", "lint:extensions:no-plugin-sdk-internal": "node scripts/check-extension-plugin-sdk-boundary.mjs --mode=plugin-sdk-internal", + "lint:extensions:no-relative-outside-package": "node scripts/check-extension-plugin-sdk-boundary.mjs --mode=relative-outside-package", "lint:extensions:no-src-outside-plugin-sdk": "node scripts/check-extension-plugin-sdk-boundary.mjs --mode=src-outside-plugin-sdk", "lint:fix": "oxlint --type-aware --fix && pnpm format", "lint:plugins:no-extension-imports": "node scripts/check-plugin-extension-import-boundary.mjs", diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index 43046d8ab5f..91ed44230fc 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -8,7 +8,11 @@ import ts from "typescript"; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const extensionsRoot = path.join(repoRoot, "extensions"); -const MODES = new Set(["src-outside-plugin-sdk", "plugin-sdk-internal"]); +const MODES = new Set([ + "src-outside-plugin-sdk", + "plugin-sdk-internal", + "relative-outside-package", +]); const baselinePathByMode = { "src-outside-plugin-sdk": path.join( @@ -23,6 +27,12 @@ const baselinePathByMode = { "fixtures", "extension-plugin-sdk-internal-inventory.json", ), + "relative-outside-package": path.join( + repoRoot, + "test", + "fixtures", + "extension-relative-outside-package-inventory.json", + ), }; const ruleTextByMode = { @@ -30,6 +40,8 @@ const ruleTextByMode = { "Rule: production extensions/** must not import src/** outside src/plugin-sdk/**", "plugin-sdk-internal": "Rule: production extensions/** must not import src/plugin-sdk-internal/**", + "relative-outside-package": + "Rule: production extensions/** must not use relative imports that escape their own extension package root", }; function normalizePath(filePath) { @@ -42,8 +54,8 @@ function isCodeFile(fileName) { function isTestLikeFile(relativePath) { return ( - /(^|\/)(__tests__|fixtures)\//.test(relativePath) || - /(^|\/)[^/]*test-support\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) || + /(^|\/)(__tests__|fixtures|test|tests)\//.test(relativePath) || + /(^|\/)[^/]*test-(support|helpers)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) || /\.(test|spec)\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(relativePath) ); } @@ -89,13 +101,34 @@ function resolveSpecifier(specifier, importerFile) { return null; } -function classifyReason(mode, kind, resolvedPath) { +function resolveExtensionRoot(filePath) { + const relativePath = normalizePath(filePath); + const segments = relativePath.split("/"); + if (segments[0] !== "extensions" || !segments[1]) { + return null; + } + return `${segments[0]}/${segments[1]}`; +} + +function classifyReason(mode, kind, resolvedPath, specifier) { const verb = kind === "export" ? "re-exports" : kind === "dynamic-import" ? "dynamically imports" : "imports"; + if (mode === "relative-outside-package") { + if (resolvedPath?.startsWith("src/plugin-sdk/")) { + return `${verb} plugin-sdk via relative path; use openclaw/plugin-sdk/`; + } + if (resolvedPath?.startsWith("src/")) { + return `${verb} core src path via relative path outside the extension package`; + } + if (resolvedPath?.startsWith("extensions/")) { + return `${verb} another extension via relative path outside the extension package`; + } + return `${verb} relative path ${specifier} outside the extension package`; + } if (mode === "plugin-sdk-internal") { return `${verb} src/plugin-sdk-internal from an extension`; } @@ -117,6 +150,9 @@ function compareEntries(left, right) { } function shouldReport(mode, resolvedPath) { + if (mode === "relative-outside-package") { + return false; + } if (!resolvedPath?.startsWith("src/")) { return false; } @@ -128,10 +164,18 @@ function shouldReport(mode, resolvedPath) { function collectFromSourceFile(mode, sourceFile, filePath) { const entries = []; + const extensionRoot = resolveExtensionRoot(filePath); function push(kind, specifierNode, specifier) { const resolvedPath = resolveSpecifier(specifier, filePath); - if (!shouldReport(mode, resolvedPath)) { + if (mode === "relative-outside-package") { + if (!specifier.startsWith(".") || !resolvedPath || !extensionRoot) { + return; + } + if (resolvedPath === extensionRoot || resolvedPath.startsWith(`${extensionRoot}/`)) { + return; + } + } else if (!shouldReport(mode, resolvedPath)) { return; } entries.push({ @@ -140,7 +184,7 @@ function collectFromSourceFile(mode, sourceFile, filePath) { kind, specifier, resolvedPath, - reason: classifyReason(mode, kind, resolvedPath), + reason: classifyReason(mode, kind, resolvedPath, specifier), }); } @@ -195,7 +239,9 @@ export async function readExpectedInventory(mode) { return JSON.parse(await fs.readFile(baselinePathByMode[mode], "utf8")); } catch (error) { if ( - (mode === "plugin-sdk-internal" || mode === "src-outside-plugin-sdk") && + (mode === "plugin-sdk-internal" || + mode === "src-outside-plugin-sdk" || + mode === "relative-outside-package") && error && typeof error === "object" && "code" in error && diff --git a/test/extension-plugin-sdk-boundary.test.ts b/test/extension-plugin-sdk-boundary.test.ts index ea421d2708f..5a7325077c7 100644 --- a/test/extension-plugin-sdk-boundary.test.ts +++ b/test/extension-plugin-sdk-boundary.test.ts @@ -1,10 +1,17 @@ import { execFileSync } from "node:child_process"; +import fs from "node:fs"; import path from "node:path"; import { describe, expect, it } from "vitest"; import { collectExtensionPluginSdkBoundaryInventory } from "../scripts/check-extension-plugin-sdk-boundary.mjs"; const repoRoot = process.cwd(); const scriptPath = path.join(repoRoot, "scripts", "check-extension-plugin-sdk-boundary.mjs"); +const relativeOutsidePackageBaselinePath = path.join( + repoRoot, + "test", + "fixtures", + "extension-relative-outside-package-inventory.json", +); describe("extension src outside plugin-sdk boundary inventory", () => { it("is currently empty", async () => { @@ -65,3 +72,26 @@ describe("extension plugin-sdk-internal boundary inventory", () => { expect(JSON.parse(stdout)).toEqual([]); }); }); + +describe("extension relative-outside-package boundary inventory", () => { + it("matches the checked-in baseline", async () => { + const inventory = await collectExtensionPluginSdkBoundaryInventory("relative-outside-package"); + const expected = JSON.parse(fs.readFileSync(relativeOutsidePackageBaselinePath, "utf8")); + + expect(inventory).toEqual(expected); + }); + + it("script json output matches the checked-in baseline", () => { + const stdout = execFileSync( + process.execPath, + [scriptPath, "--mode=relative-outside-package", "--json"], + { + cwd: repoRoot, + encoding: "utf8", + }, + ); + const expected = JSON.parse(fs.readFileSync(relativeOutsidePackageBaselinePath, "utf8")); + + expect(JSON.parse(stdout)).toEqual(expected); + }); +}); diff --git a/test/fixtures/extension-relative-outside-package-inventory.json b/test/fixtures/extension-relative-outside-package-inventory.json new file mode 100644 index 00000000000..4cedb17d51a --- /dev/null +++ b/test/fixtures/extension-relative-outside-package-inventory.json @@ -0,0 +1,314 @@ +[ + { + "file": "extensions/acpx/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/acpx.js", + "resolvedPath": "src/plugin-sdk/acpx.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/copilot-proxy/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/copilot-proxy.js", + "resolvedPath": "src/plugin-sdk/copilot-proxy.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/feishu/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/feishu.js", + "resolvedPath": "src/plugin-sdk/feishu.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/google/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/google.js", + "resolvedPath": "src/plugin-sdk/google.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/googlechat/runtime-api.ts", + "line": 4, + "kind": "export", + "specifier": "../../src/plugin-sdk/googlechat.js", + "resolvedPath": "src/plugin-sdk/googlechat.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/googlechat/src/channel.ts", + "line": 23, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/imessage/src/channel.ts", + "line": 9, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/irc/src/channel.ts", + "line": 17, + "kind": "import", + "specifier": "../../shared/passive-monitor.js", + "resolvedPath": "extensions/shared/passive-monitor.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/irc/src/config-schema.ts", + "line": 2, + "kind": "import", + "specifier": "../../shared/config-schema-helpers.js", + "resolvedPath": "extensions/shared/config-schema-helpers.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/irc/src/monitor.ts", + "line": 1, + "kind": "import", + "specifier": "../../shared/runtime.js", + "resolvedPath": "extensions/shared/runtime.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/irc/src/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../../src/plugin-sdk/irc.js", + "resolvedPath": "src/plugin-sdk/irc.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/line/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/line-core.js", + "resolvedPath": "src/plugin-sdk/line-core.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/lobster/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/lobster.js", + "resolvedPath": "src/plugin-sdk/lobster.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/matrix/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/matrix.js", + "resolvedPath": "src/plugin-sdk/matrix.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/matrix/src/channel.ts", + "line": 19, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/mattermost/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/mattermost.js", + "resolvedPath": "src/plugin-sdk/mattermost.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/mattermost/src/channel.ts", + "line": 15, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/mattermost/src/config-schema.ts", + "line": 2, + "kind": "import", + "specifier": "../../shared/config-schema-helpers.js", + "resolvedPath": "extensions/shared/config-schema-helpers.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/msteams/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/msteams.js", + "resolvedPath": "src/plugin-sdk/msteams.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/nextcloud-talk/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/nextcloud-talk.js", + "resolvedPath": "src/plugin-sdk/nextcloud-talk.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/nextcloud-talk/src/channel.ts", + "line": 13, + "kind": "import", + "specifier": "../../shared/passive-monitor.js", + "resolvedPath": "extensions/shared/passive-monitor.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/nextcloud-talk/src/config-schema.ts", + "line": 2, + "kind": "import", + "specifier": "../../shared/config-schema-helpers.js", + "resolvedPath": "extensions/shared/config-schema-helpers.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/nextcloud-talk/src/monitor.ts", + "line": 3, + "kind": "import", + "specifier": "../../shared/runtime.js", + "resolvedPath": "extensions/shared/runtime.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/nostr/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/nostr.js", + "resolvedPath": "src/plugin-sdk/nostr.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/nostr/src/channel.ts", + "line": 9, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/open-prose/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/open-prose.js", + "resolvedPath": "src/plugin-sdk/open-prose.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/phone-control/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/phone-control.js", + "resolvedPath": "src/plugin-sdk/phone-control.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/qwen-portal-auth/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/qwen-portal-auth.js", + "resolvedPath": "src/plugin-sdk/qwen-portal-auth.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/signal/src/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../../src/plugin-sdk/signal.js", + "resolvedPath": "src/plugin-sdk/signal.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/slack/src/channel.ts", + "line": 20, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/twitch/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/twitch.js", + "resolvedPath": "src/plugin-sdk/twitch.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/twitch/src/plugin.ts", + "line": 8, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/zai/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/zai.js", + "resolvedPath": "src/plugin-sdk/zai.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/zalo/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/zalo.js", + "resolvedPath": "src/plugin-sdk/zalo.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/zalo/src/status-issues.ts", + "line": 1, + "kind": "import", + "specifier": "../../shared/status-issues.js", + "resolvedPath": "extensions/shared/status-issues.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/zalouser/runtime-api.ts", + "line": 1, + "kind": "export", + "specifier": "../../src/plugin-sdk/zalouser.js", + "resolvedPath": "src/plugin-sdk/zalouser.js", + "reason": "re-exports plugin-sdk via relative path; use openclaw/plugin-sdk/" + }, + { + "file": "extensions/zalouser/src/channel.ts", + "line": 10, + "kind": "import", + "specifier": "../../shared/channel-status-summary.js", + "resolvedPath": "extensions/shared/channel-status-summary.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/zalouser/src/monitor.ts", + "line": 13, + "kind": "import", + "specifier": "../../shared/deferred.js", + "resolvedPath": "extensions/shared/deferred.js", + "reason": "imports another extension via relative path outside the extension package" + }, + { + "file": "extensions/zalouser/src/status-issues.ts", + "line": 1, + "kind": "import", + "specifier": "../../shared/status-issues.js", + "resolvedPath": "extensions/shared/status-issues.js", + "reason": "imports another extension via relative path outside the extension package" + } +]