mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-13 17:07:40 +00:00
refactor(channels): migrate internal callers off deprecated reply facades (#104673)
This commit is contained in:
committed by
GitHub
parent
7a466b33f3
commit
a89e6d9870
@@ -3,7 +3,10 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { collectDeprecatedInternalConfigApiViolations } from "./lib/deprecated-config-api-guard.mjs";
|
||||
import { buildDeprecatedPluginSdkModuleSpecifiers } from "./lib/deprecated-plugin-sdk-usage.mjs";
|
||||
import {
|
||||
BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES,
|
||||
buildDeprecatedPluginSdkModuleSpecifiers,
|
||||
} from "./lib/deprecated-plugin-sdk-usage.mjs";
|
||||
import { escapeRegExp } from "./lib/regexp.mjs";
|
||||
|
||||
const repoRoot = process.cwd();
|
||||
@@ -96,7 +99,7 @@ function collectModuleSpecifierRuleViolations(rule) {
|
||||
`\\bexport\\s+(?:type\\s+)?(?:\\*\\s+from\\s+|[^"']+?\\s+from\\s+)["'](${specifierPattern})["']`,
|
||||
"gu",
|
||||
),
|
||||
new RegExp(`\\bimport\\(\\s*["'](${specifierPattern})["']\\s*\\)`, "gu"),
|
||||
new RegExp(`\\bimport\\s*\\(\\s*["'](${specifierPattern})["']\\s*[,)]`, "gu"),
|
||||
];
|
||||
const violations = [];
|
||||
|
||||
@@ -129,6 +132,54 @@ function collectRuleViolations(rule) {
|
||||
return collectIdentifierRuleViolations(rule);
|
||||
}
|
||||
|
||||
const internalFacadeImportPatterns = [
|
||||
/\bimport\s+(?:type\s+)?(?:[^"']+?\s+from\s+)?["']([^"']+)["']/gu,
|
||||
/\bexport\s+(?:type\s+)?(?:\*\s+(?:as\s+\w+\s+)?from\s+|[^"']+?\s+from\s+)["']([^"']+)["']/gu,
|
||||
// Trailing [,)] keeps `import("spec", { with: ... })` attribute forms covered.
|
||||
/\bimport\s*\(\s*["']([^"']+)["']\s*[,)]/gu,
|
||||
/\brequire\s*\(\s*["']([^"']+)["']\s*\)/gu,
|
||||
];
|
||||
|
||||
// Maps any import form (package specifier or relative path) to an extension-less
|
||||
// repo module path so banned facades cannot be reached through any spelling.
|
||||
// tsconfig aliases both openclaw/plugin-sdk/* and @openclaw/plugin-sdk/* to src/plugin-sdk/*.
|
||||
function resolveInternalFacadeModulePath(repoPath, specifier) {
|
||||
const stripped = specifier.replace(/\.[cm]?[jt]sx?$/u, "");
|
||||
const packageSubpath = stripped.replace(/^@?openclaw\/plugin-sdk\//u, "");
|
||||
if (packageSubpath !== stripped) {
|
||||
return `src/plugin-sdk/${packageSubpath}`;
|
||||
}
|
||||
if (!stripped.startsWith(".")) {
|
||||
return null;
|
||||
}
|
||||
return path.posix.normalize(path.posix.join(path.posix.dirname(repoPath), stripped));
|
||||
}
|
||||
|
||||
function collectBannedInternalFacadeImportViolations(rule) {
|
||||
const bansByModulePath = new Map(
|
||||
BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES.map((ban) => [ban.modulePath, ban]),
|
||||
);
|
||||
const violations = [];
|
||||
for (const root of rule.roots) {
|
||||
for (const filePath of walk(path.join(repoRoot, root), rule)) {
|
||||
const repoPath = toRepoPath(filePath);
|
||||
const source = fs.readFileSync(filePath, "utf8");
|
||||
for (const pattern of internalFacadeImportPatterns) {
|
||||
for (const match of source.matchAll(pattern)) {
|
||||
const resolved = resolveInternalFacadeModulePath(repoPath, match[1]);
|
||||
const ban = resolved ? bansByModulePath.get(resolved) : undefined;
|
||||
if (!ban || (ban.allowedImporters ?? []).includes(repoPath)) {
|
||||
continue;
|
||||
}
|
||||
const line = source.slice(0, match.index).split("\n").length;
|
||||
violations.push(`${repoPath}:${line}: ${match[1]} (use ${ban.canonical})`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return violations;
|
||||
}
|
||||
|
||||
const rules = [
|
||||
{
|
||||
id: "internal-config-api",
|
||||
@@ -146,6 +197,12 @@ const rules = [
|
||||
moduleSpecifiers: buildDeprecatedPluginSdkModuleSpecifiers(),
|
||||
message: "extensions must use focused non-deprecated plugin SDK subpaths",
|
||||
},
|
||||
{
|
||||
// Deprecated facades stay exported for third-party plugins, but internal code
|
||||
// must not reach them via package specifier or relative import.
|
||||
id: "facade-internal-imports",
|
||||
collect: () => collectBannedInternalFacadeImportViolations({ roots: ["src", "extensions"] }),
|
||||
},
|
||||
{
|
||||
id: "message-api",
|
||||
roots: ["src", "extensions", "packages"],
|
||||
@@ -161,8 +218,6 @@ const rules = [
|
||||
"deliverDurableInboundReplyPayload",
|
||||
],
|
||||
allowedFiles: [
|
||||
"src/channels/turn/durable-delivery.ts",
|
||||
"src/channels/turn/kernel.ts",
|
||||
"src/channels/message/inbound-reply-dispatch.ts",
|
||||
"src/infra/outbound/deliver-runtime.ts",
|
||||
"src/infra/outbound/deliver.ts",
|
||||
|
||||
@@ -1,2 +1,15 @@
|
||||
/** Build fully qualified deprecated plugin SDK module specifiers from subpath metadata. */
|
||||
export function buildDeprecatedPluginSdkModuleSpecifiers(deprecatedSubpaths?: string[]): string[];
|
||||
|
||||
/** Deprecated facade module banned for internal importers outside its compat re-export chain. */
|
||||
export type BannedInternalPluginSdkFacadeModule = {
|
||||
/** Extension-less repo path of the banned facade module. */
|
||||
modulePath: string;
|
||||
/** Canonical plugin SDK subpath internal callers should import instead. */
|
||||
canonical: string;
|
||||
/** Repo paths of the compat re-export chain allowed to import the facade. */
|
||||
allowedImporters?: string[];
|
||||
};
|
||||
|
||||
/** Table of deprecated facade modules with zero allowed internal importers. */
|
||||
export const BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES: BannedInternalPluginSdkFacadeModule[];
|
||||
|
||||
@@ -11,10 +11,59 @@ const DEPRECATED_PLUGIN_SDK_EXTRA_SPECIFIERS = [
|
||||
export function buildDeprecatedPluginSdkModuleSpecifiers(
|
||||
deprecatedSubpaths = deprecatedPublicPluginSdkSubpaths,
|
||||
) {
|
||||
return [
|
||||
...new Set([
|
||||
...DEPRECATED_PLUGIN_SDK_EXTRA_SPECIFIERS,
|
||||
...deprecatedSubpaths.map((subpath) => `openclaw/plugin-sdk/${subpath}`),
|
||||
]),
|
||||
].toSorted();
|
||||
const unscoped = [
|
||||
...DEPRECATED_PLUGIN_SDK_EXTRA_SPECIFIERS,
|
||||
...deprecatedSubpaths.map((subpath) => `openclaw/plugin-sdk/${subpath}`),
|
||||
];
|
||||
// tsconfig aliases the scoped @openclaw/plugin-sdk package to the same
|
||||
// src/plugin-sdk modules, so ban both spellings of every deprecated specifier.
|
||||
return [...new Set(unscoped.flatMap((specifier) => [specifier, `@${specifier}`]))].toSorted();
|
||||
}
|
||||
|
||||
/**
|
||||
* Deprecated facade modules that stay exported for third-party plugins until the
|
||||
* documented break train, but must have zero internal importers (src/**,
|
||||
* extensions/**) via package specifier or relative path. Table-driven and
|
||||
* additive: future facade collapses (e.g. config-schema) append rows here.
|
||||
* `modulePath` is the extension-less repo path; `allowedImporters` lists the
|
||||
* compat re-export chain that keeps the public subpath alive.
|
||||
*/
|
||||
export const BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES = [
|
||||
// Reply facades: canonical seams are openclaw/plugin-sdk/channel-inbound and
|
||||
// openclaw/plugin-sdk/channel-outbound (defineChannelMessageAdapter family).
|
||||
{
|
||||
modulePath: "src/plugin-sdk/channel-envelope",
|
||||
canonical: "openclaw/plugin-sdk/channel-inbound",
|
||||
},
|
||||
{
|
||||
modulePath: "src/plugin-sdk/channel-message",
|
||||
canonical: "openclaw/plugin-sdk/channel-outbound",
|
||||
allowedImporters: ["src/plugin-sdk/channel-message-runtime.ts"],
|
||||
},
|
||||
{
|
||||
modulePath: "src/plugin-sdk/channel-message-runtime",
|
||||
canonical: "openclaw/plugin-sdk/channel-outbound",
|
||||
},
|
||||
{
|
||||
modulePath: "src/plugin-sdk/channel-reply-pipeline",
|
||||
canonical: "openclaw/plugin-sdk/channel-outbound",
|
||||
},
|
||||
{
|
||||
modulePath: "src/plugin-sdk/inbound-reply-dispatch",
|
||||
canonical: "openclaw/plugin-sdk/channel-inbound",
|
||||
allowedImporters: [
|
||||
"src/plugin-sdk/channel-message-runtime.ts",
|
||||
"src/plugin-sdk/channel-message.ts",
|
||||
],
|
||||
},
|
||||
// Shared dispatch bridge backing the facades above; only the SDK seams may
|
||||
// consume it directly so channel code stays on channel-inbound/channel-outbound.
|
||||
{
|
||||
modulePath: "src/channels/message/inbound-reply-dispatch",
|
||||
canonical: "openclaw/plugin-sdk/channel-inbound",
|
||||
allowedImporters: [
|
||||
"src/plugin-sdk/channel-inbound.ts",
|
||||
"src/plugin-sdk/inbound-reply-dispatch.ts",
|
||||
],
|
||||
},
|
||||
];
|
||||
|
||||
@@ -234,6 +234,3 @@ export async function deliverInboundReplyWithMessageSendContext(
|
||||
}
|
||||
return { status: "handled_visible", delivery };
|
||||
}
|
||||
|
||||
/** @deprecated Use `deliverInboundReplyWithMessageSendContext`. */
|
||||
export const deliverDurableInboundReplyPayload = deliverInboundReplyWithMessageSendContext;
|
||||
|
||||
@@ -12,7 +12,6 @@ import {
|
||||
import { createSubsystemLogger } from "../../logging/subsystem.js";
|
||||
import { toHistoryMediaEntries } from "../inbound-event/media.js";
|
||||
import { createChannelReplyPipeline } from "../message/reply-pipeline.js";
|
||||
import type { CreateChannelReplyPipelineParams } from "../message/reply-pipeline.js";
|
||||
import { recordChannelBotPairLoopAndCheckSuppression } from "./bot-loop-protection.js";
|
||||
import {
|
||||
EMPTY_CHANNEL_TURN_DISPATCH_COUNTS,
|
||||
@@ -37,7 +36,6 @@ export { createChannelHistoryWindow } from "./history-window.js";
|
||||
export type { ChannelHistoryWindow } from "./history-window.js";
|
||||
export type { ChannelBotLoopProtectionFacts } from "./bot-loop-protection.js";
|
||||
export {
|
||||
deliverDurableInboundReplyPayload,
|
||||
deliverInboundReplyWithMessageSendContext,
|
||||
isDurableInboundReplyDeliveryHandled,
|
||||
throwIfDurableInboundReplyDeliveryFailed,
|
||||
@@ -108,18 +106,6 @@ const DEFAULT_EVENT_CLASS: ChannelEventClass = {
|
||||
};
|
||||
const log = createSubsystemLogger("channels/turn/kernel");
|
||||
|
||||
/**
|
||||
* @deprecated Compatibility assembly for legacy buffered reply dispatchers.
|
||||
* New channel plugins should expose `defineChannelMessageAdapter(...)` from
|
||||
* `openclaw/plugin-sdk/channel-outbound` and route send/receive behavior through
|
||||
* the message lifecycle helpers.
|
||||
*/
|
||||
export function createChannelTurnReplyPipeline(
|
||||
params: CreateChannelReplyPipelineParams,
|
||||
): ReturnType<typeof createChannelReplyPipeline> {
|
||||
return createChannelReplyPipeline(params);
|
||||
}
|
||||
|
||||
function isAdmission(value: unknown): value is ChannelTurnAdmission {
|
||||
if (!value || typeof value !== "object") {
|
||||
return false;
|
||||
|
||||
@@ -1,8 +1,40 @@
|
||||
// Check Deprecated Api Usage tests cover check deprecated api usage script behavior.
|
||||
import { spawnSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { buildDeprecatedPluginSdkModuleSpecifiers } from "../../scripts/lib/deprecated-plugin-sdk-usage.mjs";
|
||||
import {
|
||||
BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES,
|
||||
buildDeprecatedPluginSdkModuleSpecifiers,
|
||||
} from "../../scripts/lib/deprecated-plugin-sdk-usage.mjs";
|
||||
import deprecatedPublicPluginSdkSubpaths from "../../scripts/lib/plugin-sdk-deprecated-public-subpaths.json" with { type: "json" };
|
||||
|
||||
const GUARD_SCRIPT_PATH = path.resolve(
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
"../../scripts/check-deprecated-api-usage.mjs",
|
||||
);
|
||||
|
||||
function runFacadeImportRule(sourceByRepoPath: Record<string, string>) {
|
||||
// realpath first: macOS os.tmpdir() is a /var -> /private/var symlink and the
|
||||
// script reports repo-relative paths from its resolved cwd.
|
||||
const fixtureRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), "deprecated-guard-")));
|
||||
try {
|
||||
for (const [repoPath, source] of Object.entries(sourceByRepoPath)) {
|
||||
const filePath = path.join(fixtureRoot, repoPath);
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true });
|
||||
fs.writeFileSync(filePath, source);
|
||||
}
|
||||
return spawnSync(process.execPath, [GUARD_SCRIPT_PATH, "--rule=facade-internal-imports"], {
|
||||
cwd: fixtureRoot,
|
||||
encoding: "utf8",
|
||||
});
|
||||
} finally {
|
||||
fs.rmSync(fixtureRoot, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("scripts/check-deprecated-api-usage", () => {
|
||||
it("bans every curated deprecated public plugin SDK subpath", () => {
|
||||
const specifiers = new Set(buildDeprecatedPluginSdkModuleSpecifiers());
|
||||
@@ -21,4 +53,75 @@ describe("scripts/check-deprecated-api-usage", () => {
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("bans the scoped @openclaw/plugin-sdk spelling of every deprecated specifier", () => {
|
||||
const specifiers = new Set(buildDeprecatedPluginSdkModuleSpecifiers());
|
||||
|
||||
for (const specifier of [...specifiers]) {
|
||||
if (!specifier.startsWith("@")) {
|
||||
expect(specifiers.has(`@${specifier}`), specifier).toBe(true);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("bans internal imports of every deprecated reply facade", () => {
|
||||
const modulePaths = new Set(
|
||||
BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES.map((ban) => ban.modulePath),
|
||||
);
|
||||
|
||||
for (const facade of [
|
||||
"src/plugin-sdk/channel-envelope",
|
||||
"src/plugin-sdk/channel-message",
|
||||
"src/plugin-sdk/channel-message-runtime",
|
||||
"src/plugin-sdk/channel-reply-pipeline",
|
||||
"src/plugin-sdk/inbound-reply-dispatch",
|
||||
"src/channels/message/inbound-reply-dispatch",
|
||||
]) {
|
||||
expect(modulePaths.has(facade), facade).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("limits facade import allowlists to the plugin-sdk compat re-export chain", () => {
|
||||
for (const ban of BANNED_INTERNAL_PLUGIN_SDK_FACADE_MODULES) {
|
||||
for (const importer of ban.allowedImporters ?? []) {
|
||||
expect(importer.startsWith("src/plugin-sdk/"), `${ban.modulePath} -> ${importer}`).toBe(
|
||||
true,
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("flags internal facade imports across static, relative, scoped, and dynamic forms", () => {
|
||||
const result = runFacadeImportRule({
|
||||
"src/channels/probe.ts": [
|
||||
'import { createChannelReplyPipeline } from "openclaw/plugin-sdk/channel-reply-pipeline";',
|
||||
'export { runInboundReplyTurn } from "./message/inbound-reply-dispatch.js";',
|
||||
'const facade = await import ("../plugin-sdk/channel-message.js", { with: {} });',
|
||||
'import { formatInboundEnvelope } from "@openclaw/plugin-sdk/channel-envelope";',
|
||||
].join("\n"),
|
||||
});
|
||||
|
||||
expect(result.status).toBe(1);
|
||||
expect(result.stderr).toContain(
|
||||
"src/channels/probe.ts:1: openclaw/plugin-sdk/channel-reply-pipeline",
|
||||
);
|
||||
expect(result.stderr).toContain("src/channels/probe.ts:2: ./message/inbound-reply-dispatch.js");
|
||||
expect(result.stderr).toContain("src/channels/probe.ts:3: ../plugin-sdk/channel-message.js");
|
||||
expect(result.stderr).toContain(
|
||||
"src/channels/probe.ts:4: @openclaw/plugin-sdk/channel-envelope",
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps the compat re-export chain and test files off the facade import rule", () => {
|
||||
const result = runFacadeImportRule({
|
||||
"src/plugin-sdk/channel-message-runtime.ts": 'export * from "./channel-message.js";',
|
||||
"src/plugin-sdk/channel-inbound.ts":
|
||||
'export { runChannelInboundEvent } from "../channels/message/inbound-reply-dispatch.js";',
|
||||
"src/plugin-sdk/channel-message.test.ts":
|
||||
'const mod = await import("openclaw/plugin-sdk/channel-message");',
|
||||
});
|
||||
|
||||
expect(result.stderr).toBe("");
|
||||
expect(result.status).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user