fix: capture compiled CommonJS plugins with SDK mocks - #73
Merged
Conversation
|
馃馃憖 Pull request received. I will update this pull request when review starts. ClawSweeper review in progressClawSweeper is reviewing this revision. This supersedes any previous blocked status. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What Problem This Solves
Fixes an issue where users capturing or synthetically probing compiled CommonJS plugins would get missing SDK modules instead of their actual registrations and handler results. Static inspection also missed literal SDK
require()references.Why This Change Was Made
Use feature-detected synchronous resolution hooks alongside the existing asynchronous loader. This keeps CommonJS source and native loading unchanged while routing SDK requirements to generated mocks, including lazy requirements in retained handlers. Hooks remain scoped to their capture and become inactive after completion.
The same literal-require scanner feeds mock generation and source references, excluding comments and quoted examples. Builtins, relative modules, traversal rejection, and import-versus-registration failure classes remain covered.
User Impact
module.registerHooks()(Node.js 22.15+). Older runtimes return actionable upgrade guidance for this capability. The package engine remains>=22; existing ESM/TypeScript and plain CommonJS capture are not gated.Evidence
401d48d3889690c4726af61cfabe2c977c7a07a9, integrating landed PR fix: time out hung capture and profile child processes聽#59 atf5e66bb6fb3b6168186a9b9fba45ad2a18a0c5a1. Conflicts required only the runner import union and release-note placement. Owned-child limits, complete JSON flush, file artifacts, and loadertry/finallycleanup are preserved.401d48d:npm run checkon Node 24.19.0, 332 tests plus package-content validation, including capture timeout/output/profile and CJS suites. Integrated Node 22.14 focused matrix: 20 passed, two unsupported CJS runtime cases skipped.853e5588, using public package exports and the installed CLI: 22/22 focused checks on Node 22.15.0 and 24.19.0; 20 passed and two CJS-only controls skipped on 22.14.0. Explicit capability failures and existing ESM/TS/plain-CJS behavior passed. These unchanged boundaries carry forward after the narrow namespace-escaping correction.586862b: all nine changed capture API/CLI/synthetic cases passed on both Node 22.15.0 and 24.19.0. Final401d48darchive: normal offline installation and compiled CommonJS capture API/CLI/synthetic cases passed on Node 22.15.0. No real OpenClaw package is installed.sha512-mI3rkQ7929BqigutpeYf914EhqqEbnMUGTkqsBS/P+6kRR579+UZoAK+WUtshkkT0x3WevRE27CjZYS/5Uwh0w==. All 59 installed package files match the final source tree.401d48dis green: Node 22, both CodeQL analyses, aggregate CodeQL, and dispatch passed; optional[code]smithwas skipped. The earlier CodeQL escaping finding was fixed in586862band remains resolved. Final maintainer clearance is granted for the exact head; live identity, rules, reviews, and mergeability were revalidated before squash landing.3ecd6a53d5c174aa5640074d424022ee625b477e. Remote main contains the merge; its tree5171f6802e0c9eb7857f62ce167197d9547b19cdis identical to reviewed head401d48d3889690c4726af61cfabe2c977c7a07a9. No admin bypass or release was used.Crabpot integration followthrough is assigned to release integration, not repeated in this unit. No package version change or release.