Skip to content

refactor(scripts): migrate the OpenClaw diagnostics patch tool to .mts (#6931) - #6942

Merged
cv merged 6 commits into
NVIDIA:mainfrom
atulya-singh:refactor/6931-diagnostics-mts
Jul 16, 2026
Merged

refactor(scripts): migrate the OpenClaw diagnostics patch tool to .mts (#6931)#6942
cv merged 6 commits into
NVIDIA:mainfrom
atulya-singh:refactor/6931-diagnostics-mts

Conversation

@atulya-singh

@atulya-singh atulya-singh commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates scripts/patch-openclaw-issue-4434-diagnostics.ts to .mts, following the scripts/patch-openclaw-mcp-npx.mts precedent. Module-boundary migration only — no change to the patch shape, selectors, markers, or output.

Related Issue

Fixes #6931
Parent: #6918

Changes

  • scripts/patch-openclaw-issue-4434-diagnostics.ts.mts (git detects the rename at 97% similarity). The file was already TypeScript, so the port is small:
    • require("node:fs") / require("node:path") → ESM imports. These were the only CommonJS globals; there is no module.exports and no require.main guard (the script runs its mode dispatch at top level, which is unchanged).
    • Shebang #!/usr/bin/env node#!/usr/bin/env -S node --experimental-strip-types. The old shebang could not execute a TypeScript file standalone; it now can.
    • Usage string renamed to .mts, since it names the file it prints for.
  • Dockerfile: COPY and chmod paths. The RUN invocation is unchanged — line 762 already passed --experimental-strip-types, because the tool was already .ts.
  • src/lib/sandbox/build-context.ts: staged build-context copy path.
  • Tests updated: openclaw-issue-4434-diagnostics-patch.test.ts, openclaw-dependency-review.test.ts, issue-4434-error-fields.test.ts, openclaw-integrity-pin-suite.ts, openclaw-real-patched-dist-harness.test.ts, sandbox-build-context.test.ts.
  • docs/security/openclaw-2026.6.10-dependency-review.md: the reviewed-patch references, per the epic's rule to update every documentation reference affected by a rename.

No live reference to the old .ts path remains.

Note on the coupled guards

Two reference sites are load-bearing rather than cosmetic, and are worth a reviewer's eye:

  • test/openclaw-dependency-review.test.ts greps the Dockerfile for exact literal strings (grep -Fq 'COPY scripts/patch-openclaw-issue-4434-diagnostics.ts ...'). A rename that missed these would leave the guard asserting against a path that no longer exists.
  • docs/security/openclaw-2026.6.10-dependency-review.md is the reviewed-dependency record for OpenClaw 2026.6.10, and issue-4434-error-fields.test.ts / openclaw-integrity-pin-suite.ts assert the doc names this script. The doc's security content is unchanged; only the path is updated.

Preserved contracts

Shebang, executable mode (755, carried through git mv), SPDX header, --audit flag, patch/legacy markers, and all three exit codes are unchanged. Verified by direct execution:

$ ./scripts/patch-openclaw-issue-4434-diagnostics.mts
Usage: patch-openclaw-issue-4434-diagnostics.mts [--audit] <openclaw-dist-dir>   # exit 2

$ ./scripts/patch-openclaw-issue-4434-diagnostics.mts <empty-dist>
ERROR: expected exactly one OpenClaw assistant error formatter file, found 0     # exit 1

$ ./scripts/patch-openclaw-issue-4434-diagnostics.mts --audit <empty-dist>
Summary: 1 recognizer · 0 OK · 1 missing · 1 file(s) NOT FOUND                   # exit 3

The fail-closed behavior the security review depends on is intact: unknown shapes still exit non-zero rather than silently passing.

Testing

  • npx vitest run test/openclaw-issue-4434-diagnostics-patch.test.ts test/openclaw-dependency-review.test.ts test/issue-4434-error-fields.test.ts test/sandbox-build-context.test.ts21 passed
  • npm run typecheck:cli → clean
  • Direct execution verified for all three exit paths (usage 2, apply failure 1, audit failure 3) and via the shebang standalone.

Docs

docs/security/openclaw-2026.6.10-dependency-review.md updated for the rename (path references only; no security content changed). No other documentation references this tool.

Note for reviewers

The Docker image build is the one contract not exercised locally. The RUN line is unchanged by this PR and already used --experimental-strip-types, so the risk is limited to the COPY/chmod paths — worth a CI image build to confirm.

Checklist

  • Follows the repository code style and conventions
  • Tests updated for the changed module boundary
  • Docs updated (see above)
  • No secrets, API keys, or credentials committed

Signed-off-by: Atulya Singh atulyarajsingh@gmail.com

Summary by CodeRabbit

  • Bug Fixes

    • Updated the OpenClaw #4434 diagnostics compatibility patch to the correct .mts module format and ensure it’s copied, permissioned, and executed properly during Docker build/staging.
    • Updated staging/sandbox build context to include the .mts diagnostics patch.
  • Documentation

    • Updated the OpenClaw dependency review guidance to reference the .mts diagnostics patch script.
  • Tests

    • Updated issue-4434, dependency review, integrity pin, sandbox build-context, and related harness tests to expect the .mts filename/usage.
    • Updated cold-path calibration configuration to use .mts patch script inputs.

NVIDIA#6931)

Signed-off-by: Atulya Singh <atulyarajsingh@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Issue #4434 diagnostics patch is migrated from .ts to .mts, using ESM imports and Node type stripping. Docker staging, build execution, documentation, calibration configuration, and tests are updated to reference the new entrypoint.

Changes

OpenClaw diagnostics patch migration

Layer / File(s) Summary
Convert diagnostics patch entrypoint
scripts/patch-openclaw-issue-4434-diagnostics.mts
The script uses an ESM-compatible shebang, imports Node core modules with ESM syntax, and reports the .mts usage name.
Update build and staging wiring
Dockerfile, src/lib/sandbox/build-context.ts
Docker permissions, copying, execution, and optimized sandbox staging now use the .mts patch script.
Align documentation, configuration, and tests
docs/security/..., ci/..., test/*
Dependency-review references, image input paths, and diagnostics, Docker, harness, integrity, and staging assertions now expect the .mts path and variants.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: chore, platform: container

Suggested reviewers: cv, cjagwani, hoyalim

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also rewrites calibration entries for unrelated OpenClaw patch scripts in ci/full-e2e-cold-path-calibration.json, which is beyond the diagnostics tool migration. Remove or justify the unrelated calibration script updates, and keep the PR scoped to the diagnostics patch tool migration.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR updates the diagnostics patch script, Docker paths, docs, tests, and usage text while preserving the required behavior and validation contract.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: migrating the OpenClaw diagnostics patch tool to .mts.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

Since last review: 0 prior items resolved · 0 still apply · 0 new items found

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: cloud-onboard

1 optional E2E recommendation
  • issue-4434-tui-unreachable-inference

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions refactor PR restructures code without intended behavior change labels Jul 15, 2026
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for the migration. The ESM module boundary update follows the established precedent and keeps the patch behavior intact. Ready for maintainer review.


Related open issues:

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the diagnostics tool migration and its merge resolution against current main. References consistently use the .mts entrypoint, the combined dependency-review coverage is preserved, and focused tests plus CLI build pass.

@prekshivyas

Copy link
Copy Markdown
Collaborator

@cv Normal CI is green for fork head 2ad8ea5f2281dacb2580509d6b2194b6c1bc5d5b; PR Advisor is 0/0/0, CodeRabbit is green, and there are no unresolved threads. I reseeded CI run 29463807042 so the exact-head fork gate can record its no-secret result. Please approve the protected fork-skip environment when it appears for this SHA.

@prekshivyas
prekshivyas requested a review from cv July 16, 2026 01:56
cv added 3 commits July 16, 2026 00:25
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer gate complete for exact head f1e829c: mechanical migration conflicts resolved, calibration provenance corrected, focused tests and hooks green, DCO/GitHub verification valid, automated review clear, and the trusted fork workflow recorded the audited credentialed-E2E skip without exposing secrets (coordination check 87574815173; observer 87574748983).

@cv
cv marked this pull request as draft July 16, 2026 08:57
@cv
cv marked this pull request as ready for review July 16, 2026 08:57

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reconfirming exact-head approval after the ready-for-review retrigger: all 46 current checks are green, including the fresh required E2E observer and both fresh advisor lanes; no unresolved review threads remain.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at exact head f3dd55f after current-main refresh, all 44 checks green, DCO/Verified history, fork credentialed-E2E skip approval, focused regression coverage, security review, and documentation review.

@cv
cv merged commit 92bcf20 into NVIDIA:main Jul 16, 2026
69 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate the OpenClaw diagnostics patch tool to .mts

4 participants