refactor(scripts): migrate the OpenClaw tool-catalog patch tool to .mts (#6929) - #6941
Conversation
…ts (NVIDIA#6929) Signed-off-by: Atulya Singh <atulyarajsingh@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe OpenClaw tool-catalog patch script is migrated from CommonJS JavaScript to typed ESM TypeScript. Docker execution, sandbox staging, cross-references, and tests now use the ChangesOpenClaw tool-catalog .mts migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
✨ Thanks for the migration. The .mts conversion follows the existing precedent cleanly with no behavior changes. Ready for maintainer review. Related open issues: |
cv
left a comment
There was a problem hiding this comment.
Reviewed the accepted #6929 migration and the added Dockerfile contract coverage. The patch behavior is unchanged, stale .js invocation is explicitly rejected, and local verification passed: 7 focused dependency-contract tests, 18 related tests, and 33 onboard tests after building the CLI. Approving pending completion of required CI and E2E evidence.
Summary
Migrates
scripts/patch-openclaw-tool-catalog.jstoscripts/patch-openclaw-tool-catalog.mts, following thescripts/patch-openclaw-mcp-npx.mtsprecedent already in the tree. This is a module-boundary migration only — no behavior, patch shape, or output contract changes.Related Issue
Fixes #6929
Parent: #6918
Changes
scripts/patch-openclaw-tool-catalog.js→.mts(git detects the rename at 89% similarity):#!/usr/bin/env node→#!/usr/bin/env -S node --experimental-strip-types, matching the mcp-npx precedent.require("node:fs")/require("node:path")→ ESM imports; dropped"use strict"(implicit in ESM).module.exports→ named exports (MARKER,patchOpenClawToolCatalog,patchSelectionText) — the same three the CommonJS export block exposed.require.main === module→path.resolve(process.argv[1]) === fileURLToPath(import.meta.url), the precedent's ESM main guard.catch (err)blocks that interpolatederr.messagenow narrow viaerr instanceof Error ? err.message : String(err), sinceerrisunknownin TypeScript — the message text is unchanged forErrorinstances, which is what both paths (ENOENTfromreadFileSync/readdirSync) actually throw.Dockerfile:COPYandchmodpaths updated; theRUNinvocation now passes--experimental-strip-types, matching how the adjacent.mtspatch tools are already invoked (lines 204/221/772).src/lib/sandbox/build-context.ts: staged build-context copy path updated.test/openclaw-tool-catalog-patch.test.ts: replaced thecreateRequire(import.meta.url)bridge with a static ESM import (a.mtsmodule cannot berequire()d), and the direct-execution spawn now passes--experimental-strip-types. Mirrorstest/openclaw-mcp-npx-patch.test.ts.test/onboard.test.ts,test/sandbox-build-context.test.ts: build-context fixture/assertion paths updated.nemoclaw-blueprint/scripts/nemotron-inference-fix.js: stale path in an explanatory comment.No live reference to the old
.jspath remains (git grep patch-openclaw-tool-catalog.js→ no matches).Preserved contracts
Per the epic's scope rules — shebang, executable mode (
755, carried throughgit mv), SPDX header, exit codes, and the stdout/stderr contract are unchanged. Verified by direct execution:The usage string now names
.mts— it is derived from the filename it prints for, so leaving it as.jswould have misnamed the tool.Testing
npx vitest run test/openclaw-tool-catalog-patch.test.ts test/sandbox-build-context.test.ts→ 11 passednpx vitest run test/onboard.test.ts→ 33 passednpm run typecheck:cli→ cleanDocs
No change needed — this tool has no contributor-facing command examples in
docs/; it is invoked only by the Dockerfile build.Note for reviewers
The Docker image build is the one contract I could not exercise locally. The
RUNline depends on--experimental-strip-typesbeing available in the image's Node, which the three adjacent.mts/.tspatch tools already rely on at lines 204/221/747/762/772 of the same Dockerfile, so the capability is established. Worth a CI image build to confirm.Checklist
Signed-off-by: Atulya Singh atulyarajsingh@gmail.com
Summary by CodeRabbit
Refactor
Tests