refactor(scripts): migrate documentation and configuration tools to .mts - #6936
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocumentation and configuration tooling was migrated to explicit ChangesDocumentation and configuration tooling migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SourceFiles
participant checkEnvVarDocs
participant CommandsMdx
participant AllowlistJson
SourceFiles->>checkEnvVarDocs: scan NEMOCLAW_* reads
CommandsMdx->>checkEnvVarDocs: extract documented variables
AllowlistJson->>checkEnvVarDocs: load allowlist entries
checkEnvVarDocs->>checkEnvVarDocs: compute audit results
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-6936.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
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: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-env-var-docs.mts`:
- Around line 221-249: The isAssignmentOperator helper currently treats compound
assignments as write-only, incorrectly skipping reads of environment variables.
Narrow isAssignmentOperator to return true only for ts.SyntaxKind.EqualsToken,
while isAssignmentOrDeleteTarget continues handling delete expressions
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4d632d12-c10d-459d-a188-10e6704ef8fa
📒 Files selected for processing (29)
.coderabbit.yaml.github/workflows/docs-preview-pr.yaml.github/workflows/docs-publish-staging.yaml.pre-commit-config.yamldocs/CONTRIBUTING.mdpackage.jsonscripts/check-docs-published-routes.mtsscripts/check-env-var-docs.mtsscripts/checks/local-credential-helper-pin.tsscripts/fern-preview-config.mtsscripts/generate-starter-prompt.mtsscripts/sync-agent-variant-docs.mtsscripts/validate-configs.mtsscripts/watch-fern-preview.mtstest/agent-variant-docs.test.tstest/changelog-docs.test.tstest/check-docs-published-routes.test.tstest/check-env-var-docs.test.tstest/fern-preview-config.test.tstest/generate-platform-docs.test.tstest/internal-commands-docs.test.tstest/network-policies-published-routes.test.tstest/onboard-performance-config-schema.test.tstest/repro-5445-docs-published-route.test.tstest/starter-prompt-docs.test.tstest/sync-agent-variant-docs.test.tstest/validate-blueprint.test.tstest/validate-config-schemas.test.tstest/validate-configs-dangerous-hosts.test.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/check-env-var-docs.mts`:
- Around line 221-249: The isAssignmentOperator helper currently treats compound
assignments as write-only, incorrectly skipping reads of environment variables.
Narrow isAssignmentOperator to return true only for ts.SyntaxKind.EqualsToken,
while isAssignmentOrDeleteTarget continues handling delete expressions
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4d632d12-c10d-459d-a188-10e6704ef8fa
📒 Files selected for processing (29)
.coderabbit.yaml.github/workflows/docs-preview-pr.yaml.github/workflows/docs-publish-staging.yaml.pre-commit-config.yamldocs/CONTRIBUTING.mdpackage.jsonscripts/check-docs-published-routes.mtsscripts/check-env-var-docs.mtsscripts/checks/local-credential-helper-pin.tsscripts/fern-preview-config.mtsscripts/generate-starter-prompt.mtsscripts/sync-agent-variant-docs.mtsscripts/validate-configs.mtsscripts/watch-fern-preview.mtstest/agent-variant-docs.test.tstest/changelog-docs.test.tstest/check-docs-published-routes.test.tstest/check-env-var-docs.test.tstest/fern-preview-config.test.tstest/generate-platform-docs.test.tstest/internal-commands-docs.test.tstest/network-policies-published-routes.test.tstest/onboard-performance-config-schema.test.tstest/repro-5445-docs-published-route.test.tstest/starter-prompt-docs.test.tstest/sync-agent-variant-docs.test.tstest/validate-blueprint.test.tstest/validate-config-schemas.test.tstest/validate-configs-dangerous-hosts.test.ts
🛑 Comments failed to post (1)
scripts/check-env-var-docs.mts (1)
221-249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== File outline ==\n' ast-grep outline scripts/check-env-var-docs.mts --view expanded || true printf '\n== Relevant lines in scripts/check-env-var-docs.mts ==\n' sed -n '210,265p' scripts/check-env-var-docs.mts printf '\n== Test file outline ==\n' ast-grep outline scripts/check-env-var-docs.test.ts --view expanded || true printf '\n== Relevant lines in scripts/check-env-var-docs.test.ts ==\n' sed -n '1,260p' scripts/check-env-var-docs.test.tsRepository: NVIDIA/NemoClaw
Length of output: 3708
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== TS SyntaxKind assignment range probe ==\n' node - <<'JS' const ts = require('typescript'); const assignmentKinds = Object.entries(ts.SyntaxKind) .filter(([k, v]) => typeof v === 'number' && k.endsWith('EqualsToken')) .sort((a, b) => a[1] - b[1]); console.log('FirstAssignment:', ts.SyntaxKind[ts.SyntaxKind.FirstAssignment], ts.SyntaxKind.FirstAssignment); console.log('LastAssignment:', ts.SyntaxKind[ts.SyntaxKind.LastAssignment], ts.SyntaxKind.LastAssignment); console.log('Assignment tokens around range:'); for (const [name, val] of assignmentKinds) { if (val >= ts.SyntaxKind.FirstAssignment && val <= ts.SyntaxKind.LastAssignment) { console.log(name); } } console.log('Current explicit checks in file:'); const fs = require('fs'); const text = fs.readFileSync('scripts/check-env-var-docs.mts', 'utf8'); const m = text.match(/function isAssignmentOperator[\s\S]*?return \(([\s\S]*?)\n\s*\);/); console.log(m ? m[1].trim() : 'not found'); JSRepository: NVIDIA/NemoClaw
Length of output: 851
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Search for uses of isAssignmentOperator ==\n' rg -n "isAssignmentOperator|QuestionQuestionEqualsToken|AmpersandAmpersandEqualsToken|BarBarEqualsToken" scripts/check-env-var-docs.mts scripts/check-env-var-docs.test.ts || trueRepository: NVIDIA/NemoClaw
Length of output: 733
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Relevant source around wrapper helpers ==\n' sed -n '150,250p' scripts/check-env-var-docs.mts printf '\n== Relevant test cases around env var reads/writes ==\n' sed -n '1,220p' scripts/check-env-var-docs.test.tsRepository: NVIDIA/NemoClaw
Length of output: 3268
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '1,220p' scripts/check-env-var-docs.mtsRepository: NVIDIA/NemoClaw
Length of output: 6565
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '\n== Full env-var read scanner ==\n' sed -n '1,220p' scripts/check-env-var-docs.mts printf '\n== Repo file names matching check-env-var-docs ==\n' git ls-files | rg 'check-env-var-docs'Repository: NVIDIA/NemoClaw
Length of output: 6712
Only skip plain
=
Compound assignments also read the current value, soprocess.env.FOO += 1/??=should still count as reads. Narrow this check tots.SyntaxKind.EqualsToken; the current list can hide undocumented env vars touched through compound assignment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-env-var-docs.mts` around lines 221 - 249, The isAssignmentOperator helper currently treats compound assignments as write-only, incorrectly skipping reads of environment variables. Narrow isAssignmentOperator to return true only for ts.SyntaxKind.EqualsToken, while isAssignmentOrDeleteTarget continues handling delete expressions unchanged.
…onfigs Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
cv
left a comment
There was a problem hiding this comment.
Reviewed the documentation/configuration tool module migration, workflow path updates, configuration and credential-helper behavior, focused tests, and completed CI/E2E gates. Approved for v0.0.84.
Summary
Renames the seven ESM-shaped documentation and configuration tools under
scripts/from.tsto.mtsso each tool's module contract is explicit, and updates every reference to the old paths. Documentation generation, route validation, config validation, and Fern preview behaviour are unchanged; the root package keeps its CommonJS contract.Related Issue
Resolves #6920
Changes
.mts:check-docs-published-routes,check-env-var-docs,fern-preview-config,generate-starter-prompt,sync-agent-variant-docs,validate-configs,watch-fern-preview..mts(check-docs-published-routes→sync-agent-variant-docs;watch-fern-preview→fern-preview-config).package.jsonscript entries,.pre-commit-config.yamlhook entries and thecheck-env-var-docsfiles:filter, and thedocs-preview-pr/docs-publish-stagingworkflow path filters..mtsextensions to the ten extensionless importers (docs/config test files plusscripts/checks/local-credential-helper-pin.ts);moduleResolution: bundlerdoes not resolve extensionless specifiers to.mts.validate-configs.mtsdirect-invocation guard (process.argv[1]now ends.mts) and its usage strings.sync-agent-variant-docs.mtsandgenerate-starter-prompt.mtsto name the new source paths, and the path references indocs/CONTRIBUTING.mdand.coderabbit.yaml.Type of Change
Quality Gates
.mtspaths and passes (15 files, 285 tests)CONTRIBUTING.mdand provenance path references were updated to the new.mtsfilenamesVerification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm run typecheck:cliclean;npx vitest run --project integrationon the 15 touched docs/config test files → 285 passednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests