fix(plugin-sdk): add export verification to prevent runtime breakage#28575
fix(plugin-sdk): add export verification to prevent runtime breakage#28575steipete merged 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryAdded comprehensive guards to prevent missing plugin-sdk exports that would cause runtime failures in channel extensions.
Critical issue found: The release guard is incomplete - it's missing 4 exports that are checked by both the standalone script and the test file: Confidence Score: 3/5
Last reviewed commit: daecae9 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daecae987e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "logTypingFailure", | ||
| "resolveControlCommandGate", | ||
| "resolveDmGroupAccessWithLists", | ||
| "resolveAllowlistProviderRuntimeGroupPolicy", | ||
| "resolveDefaultGroupPolicy", |
There was a problem hiding this comment.
Align release export guard with plugin runtime imports
requiredPluginSdkExports omits several plugin-sdk symbols that channel extensions import at runtime (buildPendingHistoryContextFromMap, clearHistoryEntriesIfEnabled, recordPendingHistoryEntryIfEnabled, warnMissingProviderGroupPolicyFallbackOnce), so pnpm release:check can pass even when a future build drops one of these exports and extensions still crash with TypeError (for example the import set in extensions/mattermost/src/mattermost/monitor.ts includes them). This is also inconsistent with the stricter list in scripts/check-plugin-sdk-exports.mjs, which means the automated release gate is weaker than the standalone verifier introduced in the same commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 2e53fb3 — aligned the requiredPluginSdkExports list with the full set from check-plugin-sdk-exports.mjs, adding the 4 missing exports.
| const requiredPluginSdkExports = [ | ||
| "isDangerousNameMatchingEnabled", | ||
| "createAccountListHelpers", | ||
| "buildAgentMediaPayload", | ||
| "createReplyPrefixOptions", | ||
| "createTypingCallbacks", | ||
| "logInboundDrop", | ||
| "logTypingFailure", | ||
| "resolveControlCommandGate", | ||
| "resolveDmGroupAccessWithLists", | ||
| "resolveAllowlistProviderRuntimeGroupPolicy", | ||
| "resolveDefaultGroupPolicy", | ||
| "resolveChannelMediaMaxBytes", | ||
| "emptyPluginConfigSchema", | ||
| "normalizePluginHttpPath", | ||
| "registerPluginHttpRoute", | ||
| "DEFAULT_ACCOUNT_ID", | ||
| "DEFAULT_GROUP_HISTORY_LIMIT", | ||
| ]; |
There was a problem hiding this comment.
4 critical exports are missing from this list compared to scripts/check-plugin-sdk-exports.mjs (which checks 21 exports):
| const requiredPluginSdkExports = [ | |
| "isDangerousNameMatchingEnabled", | |
| "createAccountListHelpers", | |
| "buildAgentMediaPayload", | |
| "createReplyPrefixOptions", | |
| "createTypingCallbacks", | |
| "logInboundDrop", | |
| "logTypingFailure", | |
| "resolveControlCommandGate", | |
| "resolveDmGroupAccessWithLists", | |
| "resolveAllowlistProviderRuntimeGroupPolicy", | |
| "resolveDefaultGroupPolicy", | |
| "resolveChannelMediaMaxBytes", | |
| "emptyPluginConfigSchema", | |
| "normalizePluginHttpPath", | |
| "registerPluginHttpRoute", | |
| "DEFAULT_ACCOUNT_ID", | |
| "DEFAULT_GROUP_HISTORY_LIMIT", | |
| ]; | |
| const requiredPluginSdkExports = [ | |
| "isDangerousNameMatchingEnabled", | |
| "createAccountListHelpers", | |
| "buildAgentMediaPayload", | |
| "createReplyPrefixOptions", | |
| "createTypingCallbacks", | |
| "logInboundDrop", | |
| "logTypingFailure", | |
| "buildPendingHistoryContextFromMap", | |
| "clearHistoryEntriesIfEnabled", | |
| "recordPendingHistoryEntryIfEnabled", | |
| "resolveControlCommandGate", | |
| "resolveDmGroupAccessWithLists", | |
| "resolveAllowlistProviderRuntimeGroupPolicy", | |
| "resolveDefaultGroupPolicy", | |
| "resolveChannelMediaMaxBytes", | |
| "warnMissingProviderGroupPolicyFallbackOnce", | |
| "emptyPluginConfigSchema", | |
| "normalizePluginHttpPath", | |
| "registerPluginHttpRoute", | |
| "DEFAULT_ACCOUNT_ID", | |
| "DEFAULT_GROUP_HISTORY_LIMIT", | |
| ]; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/release-check.ts
Line: 92-110
Comment:
4 critical exports are missing from this list compared to `scripts/check-plugin-sdk-exports.mjs` (which checks 21 exports):
```suggestion
const requiredPluginSdkExports = [
"isDangerousNameMatchingEnabled",
"createAccountListHelpers",
"buildAgentMediaPayload",
"createReplyPrefixOptions",
"createTypingCallbacks",
"logInboundDrop",
"logTypingFailure",
"buildPendingHistoryContextFromMap",
"clearHistoryEntriesIfEnabled",
"recordPendingHistoryEntryIfEnabled",
"resolveControlCommandGate",
"resolveDmGroupAccessWithLists",
"resolveAllowlistProviderRuntimeGroupPolicy",
"resolveDefaultGroupPolicy",
"resolveChannelMediaMaxBytes",
"warnMissingProviderGroupPolicyFallbackOnce",
"emptyPluginConfigSchema",
"normalizePluginHttpPath",
"registerPluginHttpRoute",
"DEFAULT_ACCOUNT_ID",
"DEFAULT_GROUP_HISTORY_LIMIT",
];
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Addressed in 2e53fb3 — added buildPendingHistoryContextFromMap, clearHistoryEntriesIfEnabled, recordPendingHistoryEntryIfEnabled, and warnMissingProviderGroupPolicyFallbackOnce to align with the 21-export list in check-plugin-sdk-exports.mjs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9857489ad9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Critical functions that channel extension plugins import from openclaw/plugin-sdk. | ||
| // If any are missing from the compiled output, plugins crash at runtime (#27569). | ||
| const requiredPluginSdkExports = [ |
There was a problem hiding this comment.
Include all runtime plugin-sdk imports in release guard
checkPluginSdkExports() only verifies the 21 names in requiredPluginSdkExports, but runtime imports used by shipped extensions are missing from that list (for example createDedupeCache/formatInboundFromLabel in extensions/mattermost/src/mattermost/monitor-helpers.ts:1-7 and buildChannelKeyCandidates in extensions/msteams/src/policy.ts:12-16). If bundling drops one of those exports (the same failure mode as #27569), pnpm release:check will still pass and the affected extension will crash at runtime with TypeError.
Useful? React with 👍 / 👎.
|
You have reached your Codex rate limits. Please try again later. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28221a4471
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Critical functions that channel extension plugins import from openclaw/plugin-sdk. | ||
| // If any are missing from the compiled output, plugins crash at runtime (#27569). | ||
| const requiredPluginSdkExports = [ |
There was a problem hiding this comment.
Close appcast blocks before adding plugin-sdk export checks
The plugin-sdk export guard was inserted before checkAppcastSparkleVersions() closes its for/if blocks, leaving scripts/release-check.ts syntactically invalid; running node scripts/release-check.ts now fails with ERR_INVALID_TYPESCRIPT_SYNTAX: Expected '}', got '<eof>'. In this state pnpm release:check cannot run at all, so the release validation gate is effectively broken whenever release checks are invoked.
Useful? React with 👍 / 👎.
|
Landed via temp rebase onto main.
Thanks @Glucksberg! |
Summary
Adds regression guards for #27569 where
isDangerousNameMatchingEnabledwas missing from the compileddist/plugin-sdk/index.js, causing mattermost (and potentially googlechat, msteams, irc) channel plugins to crash at runtime withTypeError: isDangerousNameMatchingEnabled is not a function.src/plugin-sdk/index.test.ts): adds two new test cases that verify critical function and constant exports are present and callable from the plugin-sdk barrel. CoversisDangerousNameMatchingEnabledand 30+ other functions/constants that channel extensions depend on.scripts/release-check.ts): adds acheckPluginSdkExports()step that parses the compileddist/plugin-sdk/index.jsexport statement and verifies 21 critical exports are present. Runs as part ofpnpm release:checkbefore every release.scripts/check-plugin-sdk-exports.mjs): can be run independently afterpnpm buildto verify plugin-sdk exports.Test plan
pnpm vitest run src/plugin-sdk/index.test.tspasses (3 tests)pnpm vitest run src/plugins/loader.test.tspasses (23 tests)node --import tsx scripts/release-check.tspassesnode scripts/check-plugin-sdk-exports.mjspassespnpm tsgocleanCloses #27569