chore(tooling): adopt Oxlint and Oxfmt - #8930
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
📝 WalkthroughWalkthroughThe repository replaces Biome with Oxfmt and Oxlint, adds added-file formatter and lint scripts, introduces an anti-slop Oxlint plugin, updates documentation and dependencies, and refreshes the npm cache manifest. ChangesToolchain migration
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🟡 Moderate · up to The change updates contributor linting and formatting enforcement, but an applicable broad validation gate has not passed because the manual coverage hook exceeded the 300-second host limit. Merge readiness remains moderate until the gate completes successfully or an explicit maintainer waiver is recorded. Sequence Diagram(s)sequenceDiagram
participant PreCommit
participant AddedFileScripts
participant OxfmtOxlint
PreCommit->>AddedFileScripts: run added-file formatter or linter
AddedFileScripts->>AddedFileScripts: resolve base revision and select files
AddedFileScripts->>OxfmtOxlint: pass added JavaScript and TypeScript files
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 43d263c in the TypeScript / code-coverage/cliThe overall coverage in commit 43d263c in the Show a code coverage summary of the most impacted files.
Updated |
|
🌿 Preview your docs: https://nvidia-preview-pr-8930.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite for the commit under review. Recommended E2E: Manual-only E2E: 3 optional E2E recommendations
1 warning · 0 suggestionsWarningsWarnings do not block.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
tools/oxlint/anti-slop/shared/dictionary-types.ts (1)
78-88: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNote the ancestor rescan cost in
shouldReportType's caller contract.
shouldReportType(intools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts) callsclassifyUnsafeDictionaryonce per ancestor type node.classifyUnsafeDictionaryperforms full alias and substitution resolution through this shared module. For deeply nested generic type expressions, the plugin repeats the same resolution work at every nesting level.This cost is upstream behavior and it is bounded by file size, so it does not block the migration. Record it as a known lint-runtime cost, and measure Oxlint duration on the largest TypeScript files in the repository before enabling these rules repository-wide.
🤖 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 `@tools/oxlint/anti-slop/shared/dictionary-types.ts` around lines 78 - 88, Record the repeated ancestor-rescan cost of shouldReportType calling classifyUnsafeDictionary as a known lint-runtime limitation, and benchmark Oxlint duration on the repository’s largest TypeScript files before enabling these rules repository-wide. Do not change the shared dictionary classification logic or its current bounded behavior.tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts (1)
15-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
typeNodeKindsduplicates AST knowledge that can drift.The set enumerates TypeScript type node kinds by hand.
isTypeNodeuses it, andshouldReportTypeusesisTypeNodeto suppress duplicate reports on nested type nodes. If a future@oxlint/pluginsversion adds a type node kind, that kind is absent here, suppression stops, and the rule reports the same dictionary at more than one nesting level.The consequence is duplicate diagnostics, not a crash. This file is vendored, so do not edit it locally. Add the drift risk to the vendored-upgrade checklist in
tools/oxlint/anti-slop/UPSTREAM.md.🤖 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 `@tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts` around lines 15 - 57, Add the AST type-node drift risk to the vendored-upgrade checklist in UPSTREAM.md, noting that manually maintained typeNodeKinds can become incomplete when `@oxlint/plugins` adds new TypeScript node kinds and cause duplicate diagnostics through isTypeNode and shouldReportType. Do not modify the vendored no-unsafe-dictionary-type.ts file.tools/oxlint/anti-slop/rules/no-known-value-widening.ts (1)
32-43: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
resolveVariablecan resolve to an unrelated same-named binding.The function walks the scope chain and matches on
identifier.namefromscope.set. It does not use the resolved reference recorded by the scope manager. If an inner scope does not declare the name but an outer scope declares a different variable with the same name, the lookup returns that outer variable.The sibling rule
tools/oxlint/anti-slop/rules/no-widen-then-assert.tsresolves identifiers throughscopeManagerreferences instead, which is precise. This file is vendored, so keep it aligned with upstream rather than patching it locally. Track the difference as a known false-positive source forno-known-value-widening.🤖 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 `@tools/oxlint/anti-slop/rules/no-known-value-widening.ts` around lines 32 - 43, Do not modify resolveVariable’s scope-chain lookup in this vendored file. Record this same-name binding mismatch as a known false-positive source for no-known-value-widening, and track the fix against upstream by aligning with the scopeManager-reference resolution used by no-widen-then-assert.ts.tools/oxlint/anti-slop/rules/no-widen-then-assert.ts (2)
174-192: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRecord the linear reference scan as a lint-runtime cost.
resolvedVariableForIdentifieriterates every scope and runsfindover each scope's references, matching by source offsets.checkAssertioncalls it for everyTSAsExpressionandTSTypeAssertion, andknownValueEvidencecalls it again for each identifier hop. The total work therefore scales with assertions multiplied by references in the file.Assertion-dense files make this the slowest part of the rule. The file is vendored, so do not optimize it here. Measure Oxlint duration on the largest assertion-heavy TypeScript files before you enable
no-widen-then-assertrepository-wide, and prefer an offset-keyed lookup map upstream.🤖 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 `@tools/oxlint/anti-slop/rules/no-widen-then-assert.ts` around lines 174 - 192, Measure Oxlint runtime on the largest assertion-heavy TypeScript files before enabling no-widen-then-assert repository-wide, accounting for repeated scans in checkAssertion and knownValueEvidence through resolvedVariableForIdentifier. Do not optimize the vendored resolvedVariableForIdentifier implementation here; instead, document or report the measured cost and recommend an upstream offset-keyed lookup map.
42-85: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
Record,Readonly, andPropertyKeyare matched by name without a shadowing check.
isBroadRecordKeyTypeandisBroadRecordTypecompare the type reference name directly. A file that declares its ownRecord,Readonly, orPropertyKeytype still matches, so the rule can classify a narrow local type as broad and report a false positive.
tools/oxlint/anti-slop/shared/dictionary-types.tshandles this case withshadowedBuiltInsandisBuiltIn. The two rules therefore disagree on the same question. Keep this vendored file aligned with upstream, and raise the inconsistency upstream instead of patching it locally.🤖 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 `@tools/oxlint/anti-slop/rules/no-widen-then-assert.ts` around lines 42 - 85, Update isBroadRecordKeyType and isBroadRecordType to use the vendored upstream shadowing-aware built-in detection, including shadowedBuiltIns and isBuiltIn, before matching PropertyKey, Readonly, or Record. Keep behavior aligned with tools/oxlint/anti-slop/shared/dictionary-types.ts and do not implement a local-only workaround; raise the inconsistency upstream instead.tools/oxlint/anti-slop/UPSTREAM.md (1)
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the vendored-copy refresh procedure.
Add the commands to update
dmmulroy/anti-slopand record the responsible owner. The TypeScript files match the cited commit apart from the SPDX header and its blank separator line.🤖 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 `@tools/oxlint/anti-slop/UPSTREAM.md` around lines 6 - 10, Update the vendored-copy documentation in UPSTREAM.md to include the commands for refreshing the dmmulroy/anti-slop sources and identify the responsible owner. Preserve the stated commit, SPDX-header exception, blank separator line, and upstream license details.
🤖 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 `@oxlint.config.ts`:
- Around line 161-162: Update the lint configuration around
typescript/no-floating-promises so it is either backed by compatible type-aware
linting with oxlint-tsgolint pinned in the lockfile, or remove the rule when
type-aware support is not available; ensure the final configuration does not
retain an unsupported type-aware rule.
In `@tools/lint/format-added-files.sh`:
- Around line 7-15: Update the mode-parsing logic in the script before the
existing shift so it only evaluates and removes the first argument when at least
one argument was provided. Preserve the --check/--write validation and default
mode behavior, while allowing no-argument execution to continue to candidate
discovery without triggering shift failure.
- Around line 17-20: Update the base-ref resolution in the format-added-files
script to fail closed when the configured ref cannot be resolved, rather than
falling back to HEAD and producing an empty diff. Ensure the script exits with a
clear error unless it can resolve a valid base or merge base, and add a
regression test covering a clean PR checkout where origin/main is unavailable.
- Around line 33-43: Update the candidate filtering loop around candidates and
added_files to resolve each file against the repository root, reject absolute or
traversal paths that escape that root, and only then perform the file-type and
git cat-file checks. Preserve valid repository-relative formatter paths and
continue excluding unsupported extensions.
In `@tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts`:
- Around line 15-24: Update isConditionalEmptyObjectSpread to unwrapParentheses
on both conditional.consequent and conditional.alternate before passing them to
isEmptyObjectExpression, so parenthesized empty-object branches are detected.
Add a RuleTester case covering condition ? value : ({}).
---
Nitpick comments:
In `@tools/oxlint/anti-slop/rules/no-known-value-widening.ts`:
- Around line 32-43: Do not modify resolveVariable’s scope-chain lookup in this
vendored file. Record this same-name binding mismatch as a known false-positive
source for no-known-value-widening, and track the fix against upstream by
aligning with the scopeManager-reference resolution used by
no-widen-then-assert.ts.
In `@tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts`:
- Around line 15-57: Add the AST type-node drift risk to the vendored-upgrade
checklist in UPSTREAM.md, noting that manually maintained typeNodeKinds can
become incomplete when `@oxlint/plugins` adds new TypeScript node kinds and cause
duplicate diagnostics through isTypeNode and shouldReportType. Do not modify the
vendored no-unsafe-dictionary-type.ts file.
In `@tools/oxlint/anti-slop/rules/no-widen-then-assert.ts`:
- Around line 174-192: Measure Oxlint runtime on the largest assertion-heavy
TypeScript files before enabling no-widen-then-assert repository-wide,
accounting for repeated scans in checkAssertion and knownValueEvidence through
resolvedVariableForIdentifier. Do not optimize the vendored
resolvedVariableForIdentifier implementation here; instead, document or report
the measured cost and recommend an upstream offset-keyed lookup map.
- Around line 42-85: Update isBroadRecordKeyType and isBroadRecordType to use
the vendored upstream shadowing-aware built-in detection, including
shadowedBuiltIns and isBuiltIn, before matching PropertyKey, Readonly, or
Record. Keep behavior aligned with
tools/oxlint/anti-slop/shared/dictionary-types.ts and do not implement a
local-only workaround; raise the inconsistency upstream instead.
In `@tools/oxlint/anti-slop/shared/dictionary-types.ts`:
- Around line 78-88: Record the repeated ancestor-rescan cost of
shouldReportType calling classifyUnsafeDictionary as a known lint-runtime
limitation, and benchmark Oxlint duration on the repository’s largest TypeScript
files before enabling these rules repository-wide. Do not change the shared
dictionary classification logic or its current bounded behavior.
In `@tools/oxlint/anti-slop/UPSTREAM.md`:
- Around line 6-10: Update the vendored-copy documentation in UPSTREAM.md to
include the commands for refreshing the dmmulroy/anti-slop sources and identify
the responsible owner. Preserve the stated commit, SPDX-header exception, blank
separator line, and upstream license details.
🪄 Autofix
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: 440573cb-0d1f-4618-b0b1-ff51098a3f98
⛔ Files ignored due to path filters (2)
nemoclaw/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (142)
.pre-commit-config.yamlAGENTS.mdCONTRIBUTING.mdbiome.jsonci/test-file-size-budget.jsonnemoclaw/package.jsonnemoclaw/src/commands/migration-state.test.tsoxfmt.config.tsoxlint.anti-slop.config.tsoxlint.config.tspackage.jsonscripts/checks/run.mtssrc/lib/onboard.tssrc/lib/onboard/credential-navigation.tstest/anti-slop-rules.test.tstest/format-added-files.test.tstest/mcp-tool-discovery-image-contract.test.tstest/nemoclaw-start-extra-placeholder-breadcrumb-helpers.tstest/onboard-inference-reconciliation.test.tstest/onboard-messaging.test.tstools/e2e/hermes-gpu-startup-workflow-boundary.mtstools/lint/format-added-files.shtools/lint/lint-added-files.shtools/mcp-tool-discovery-runtime/npm-cache-seed/acorn-8.18.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/ansi-escapes-7.3.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/ansi-regex-6.3.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/ansi-styles-6.2.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-0.63.0.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-0.63.0.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-1.78.0.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-1.78.0.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-1.78.0.tgz.part-002tools/mcp-tool-discovery-runtime/npm-cache-seed/binding-linux-x64-gnu-1.78.0.tgz.part-003tools/mcp-tool-discovery-runtime/npm-cache-seed/biome-2.4.14.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/braces-3.0.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/citty-0.2.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/cli-cursor-5.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-002tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-003tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-004tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-005tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-006tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-007tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-008tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-009tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-2.4.14.tgz.part-010tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-002tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-003tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-004tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-005tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-006tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-007tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-008tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-009tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-linux-x64-musl-2.4.14.tgz.part-010tools/mcp-tool-discovery-runtime/npm-cache-seed/cli-truncate-6.1.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/commander-15.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/confbox-0.1.8.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/core-1.4.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/deepmerge-4.3.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/empathic-2.0.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/environment-1.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/execa-10.0.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fast-glob-3.3.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fast-string-truncated-width-3.0.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fast-string-width-3.0.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fast-wrap-ansi-0.2.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fastq-1.20.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fill-range-7.1.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/find-workspaces-0.3.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fs.scandir-2.1.5.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fs.stat-2.0.5.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/fs.walk-1.2.8.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/get-east-asian-width-1.6.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/glob-parent-5.1.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/helper-string-parser-7.29.7.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/helper-validator-identifier-7.29.7.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/is-extglob-2.1.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/is-fullwidth-code-point-5.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/is-glob-4.0.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/is-number-7.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/jsonc-parser-3.3.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/log-update-8.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/magicast-0.5.4.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/manifest.jsontools/mcp-tool-discovery-runtime/npm-cache-seed/merge2-1.4.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/micromatch-4.0.8.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/mimic-function-5.0.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/mlly-1.8.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/nypm-0.6.9.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/onetime-7.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/oxfmt-0.63.0.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/oxfmt-0.63.0.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/oxlint-1.78.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/parser-7.29.8.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/picomatch-2.3.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/pkg-types-1.3.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/prompts-1.7.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/queue-microtask-1.2.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/resolve.exports-2.0.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/restore-cursor-5.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/reusify-1.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/run-parallel-1.2.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/sisteransi-1.0.5.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/slice-ansi-9.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/string-width-8.2.2.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/strip-ansi-7.2.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/tinyexec-1.0.4.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/tinyexec-1.3.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/tinypool-2.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/to-regex-range-5.0.1.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/types-7.29.8.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/ufo-1.6.4.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/ultracite-7.10.3.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/which-command-0.1.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/wrap-ansi-10.0.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/yaml-2.9.0.tgztools/mcp-tool-discovery-runtime/npm-cache-seed/zod-4.4.3.tgztools/oxlint/anti-slop/LICENSEtools/oxlint/anti-slop/UPSTREAM.mdtools/oxlint/anti-slop/index.tstools/oxlint/anti-slop/rules/no-chained-type-assertions.tstools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.tstools/oxlint/anti-slop/rules/no-known-value-widening.tstools/oxlint/anti-slop/rules/no-module-mocking.tstools/oxlint/anti-slop/rules/no-object-parameters.tstools/oxlint/anti-slop/rules/no-reflect-apply.tstools/oxlint/anti-slop/rules/no-reflect-get.tstools/oxlint/anti-slop/rules/no-runtime-typeof.tstools/oxlint/anti-slop/rules/no-shape-in-symbol-names.tstools/oxlint/anti-slop/rules/no-unknown-parameters.tstools/oxlint/anti-slop/rules/no-unknown-returns.tstools/oxlint/anti-slop/rules/no-unknown-type-aliases.tstools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.tstools/oxlint/anti-slop/rules/no-widen-then-assert.tstools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.tstools/oxlint/anti-slop/shared/dictionary-types.tstools/oxlint/anti-slop/shared/reflect-method.ts
💤 Files with no reviewable changes (2)
- biome.json
- nemoclaw/src/commands/migration-state.test.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tools/lint/format-added-files.sh (1)
47-50: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject candidates whose ancestors are symbolic links.
[[ -L "${file}" ]]checks only the final component. Iflinked-diris a symlink,linked-dir/outside.tspasses[[ -f "${file}" ]]and is added to the Oxfmt or Oxlint command. Resolve each existing candidate to a canonical path and reject it unless it is contained by the canonical repository root. Add a regression test for a symlinked directory that targets a file outside the repository.
tools/lint/format-added-files.sh#L47-L50tools/lint/lint-added-files.sh#L24-L27🤖 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 `@tools/lint/format-added-files.sh` around lines 47 - 50, Update the candidate validation in tools/lint/format-added-files.sh at lines 47-50 and tools/lint/lint-added-files.sh at lines 24-27 to canonicalize each existing candidate, reject any resolved path outside the canonical repository root, and retain rejection of symlink candidates. Add a regression test covering a symlinked directory whose target file is outside the repository; apply the same containment behavior to both formatting and linting flows.
🤖 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.
Duplicate comments:
In `@tools/lint/format-added-files.sh`:
- Around line 47-50: Update the candidate validation in
tools/lint/format-added-files.sh at lines 47-50 and
tools/lint/lint-added-files.sh at lines 24-27 to canonicalize each existing
candidate, reject any resolved path outside the canonical repository root, and
retain rejection of symlink candidates. Add a regression test covering a
symlinked directory whose target file is outside the repository; apply the same
containment behavior to both formatting and linting flows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e9df05e8-29c2-406a-84c2-8016ee928b9b
⛔ Files ignored due to path filters (2)
nemoclaw/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
.pre-commit-config.yamlAGENTS.mdCONTRIBUTING.mdci/test-file-size-budget.jsonnemoclaw/package.jsonoxlint.config.tsoxlint.type-aware.config.tspackage.jsonsrc/lib/onboard.tssrc/lib/onboard/credential-navigation.tstest/mcp-tool-discovery-image-contract.test.tstest/nemoclaw-start-extra-placeholder-breadcrumb-helpers.tstest/onboard-inference-reconciliation.test.tstest/onboard-messaging.test.tstools/e2e/hermes-gpu-startup-workflow-boundary.mtstools/lint/format-added-files.shtools/lint/lint-added-files.shtools/mcp-tool-discovery-runtime/npm-cache-seed/linux-x64-7.0.2001.tgz.part-000tools/mcp-tool-discovery-runtime/npm-cache-seed/linux-x64-7.0.2001.tgz.part-001tools/mcp-tool-discovery-runtime/npm-cache-seed/linux-x64-7.0.2001.tgz.part-002tools/mcp-tool-discovery-runtime/npm-cache-seed/linux-x64-7.0.2001.tgz.part-003tools/mcp-tool-discovery-runtime/npm-cache-seed/linux-x64-7.0.2001.tgz.part-004tools/mcp-tool-discovery-runtime/npm-cache-seed/manifest.jsontools/mcp-tool-discovery-runtime/npm-cache-seed/oxlint-tsgolint-7.0.2001.tgztools/oxlint/anti-slop/UPSTREAM.mdtools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.tstools/oxlint/anti-slop/rules/no-runtime-typeof.ts
💤 Files with no reviewable changes (6)
- test/onboard-inference-reconciliation.test.ts
- src/lib/onboard/credential-navigation.ts
- tools/e2e/hermes-gpu-startup-workflow-boundary.mts
- test/nemoclaw-start-extra-placeholder-breadcrumb-helpers.ts
- test/onboard-messaging.test.ts
- src/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- test/mcp-tool-discovery-image-contract.test.ts
- CONTRIBUTING.md
- .pre-commit-config.yaml
- tools/oxlint/anti-slop/rules/no-runtime-typeof.ts
- ci/test-file-size-budget.json
- package.json
- tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts
- nemoclaw/package.json
- oxlint.config.ts
- tools/mcp-tool-discovery-runtime/npm-cache-seed/manifest.json
- AGENTS.md
|
Advisor warning dispositions for
All review threads are resolved. The independent documentation review passed on this commit. |
|
Advisor warning disposition for
The tools' numeric scores are not interchangeable. Rule tightening remains separate from this migration. |
|
Advisor warning disposition for
No change is required in this review cycle. |
Summary
This change replaces Biome with direct Oxlint and Oxfmt contributor checks. It preserves existing lint boundaries, adds type-aware plugin validation, and formats only added source files to avoid a repository-wide rewrite.
Changes
oxlint.config.ts, including SonarJS cognitive-complexity limits and plugin-specific TypeScript rules. Current SonarJS scores preserve existing hotspot limits while rejecting further growth.oxlint.type-aware.config.tsso plugin checks enforcetypescript/no-floating-promiseswithout adding type-aware cost to the general lint pass.Type of Change
Quality Gates
AGENTS.md,CONTRIBUTING.md, andtools/lint/DEPENDENCY-REVIEW.mddocument contributor behavior.Documentation Writer Review
docs-updatedAGENTS.md,CONTRIBUTING.md, andtools/lint/DEPENDENCY-REVIEW.md. Contributor guidance matches the direct Oxlint and Oxfmt package declarations, hook configuration, NemoClaw-owned Oxc configs, lockfiles, and package contracts. No publicdocs/update is needed because supported runtime behavior does not change.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpm ci --ignore-scriptsinstalls passed without Ultracite;npm run lint,npm --prefix nemoclaw run check, andnpm run typecheck:clipassed; package contracts passed 406/406; cache seed image contracts passed 13/13; full pre-commit, plugin coverage, normal commit and pre-push hooks passed;npm run docspassed with 0 errors and 2 existing Fern upgrade warnings.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — the combined broad baseline and standalone CLI coverage each exceeded the 300-second host limit; GitHub CI owns the remaining broad result.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com