fix(cli): validate metadata.gatewayName type in normalizeSession#1286
fix(cli): validate metadata.gatewayName type in normalizeSession#1286latenighthackathon wants to merge 1 commit intoNVIDIA:mainfrom
Conversation
|
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:
📝 WalkthroughWalkthroughNormalize session metadata by preserving Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard-session.test.ts`:
- Around line 136-145: The regression test writes a session fixture missing the
required version so normalizeSession rejects it and loadSession returns null;
update the test fixture written to session.SESSION_FILE to include a valid
version (e.g., "version": 1) along with metadata.gatewayName: 123 so that
session.loadSession() calls normalizeSession and exercises the non-string
gatewayName normalization path; ensure the test still asserts
loaded!.metadata.gatewayName equals "nemoclaw" after normalization.
🪄 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: Pro
Run ID: 06a1279e-ccba-46b6-85e6-0e756bc4e5f8
📒 Files selected for processing (2)
src/lib/onboard-session.test.tssrc/lib/onboard-session.ts
ad39453 to
1900e7f
Compare
|
✨ Thanks for submitting this pull request, which proposes a way to improve session normalization by validating the type of gatewayName in metadata. Possibly related open issues: |
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (5)
agents/hermes/manifest.yaml (1)
1-102: Same newline issue here; consider enforcing LF at repo level.YAMLlint reports CRLF in this manifest too. Please normalize to LF, and consider adding a
.gitattributesrule (e.g.,*.yaml text eol=lf) to prevent repeat churn.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agents/hermes/manifest.yaml` around lines 1 - 102, manifest.yaml contains CRLF line endings; convert it to LF and add a repo-level rule to prevent recurrence. Replace CRLF with LF for agents/hermes/manifest.yaml (use dos2unix, editor setting, or git add --renormalize after updating core.autocrlf), commit the normalized file, and add a .gitattributes entry like "*.yaml text eol=lf" to the repository; ensure you stage and commit both the manifest.yaml normalization and the new .gitattributes so future YAML files are checked out with LF..agents/skills/nemoclaw-contributor-update-docs/SKILL.md (1)
1-188: Keep this docs-skill rewrite in a separate PR from the gatewayName bugfix.This file appears unrelated to issue
#1283(normalizeSessiontype validation). Splitting it out will keep traceability, risk assessment, and release notes cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/nemoclaw-contributor-update-docs/SKILL.md around lines 1 - 188, Summary: The SKILL.md docs-skill rewrite was included in the same PR as the gatewayName/normalizeSession bugfix; split it into its own PR. Fix: remove the changes to nemoclaw-contributor-update-docs/SKILL.md from the current branch and create a new branch/PR containing only that file (or cherry-pick the SKILL.md commit(s) into a new branch), leaving the gatewayName/normalizeSession-related commits (issue `#1283`, normalizeSession type validation) in the original PR; ensure commit messages and the new PR title/description reference "nemoclaw-contributor-update-docs" and the original PR keeps only bugfix changes so traceability and release notes remain clear.CONTRIBUTING.md (1)
146-146: Minor wording cleanup on Line 146.“outside of a commit” can be shortened to “outside a commit” for tighter phrasing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CONTRIBUTING.md` at line 146, Update the phrasing in the CONTRIBUTING.md sentence that currently reads "To regenerate skills manually (for example, after rebasing or outside of a commit), run from the repo root:" by replacing "outside of a commit" with the tighter "outside a commit" so the line reads "To regenerate skills manually (for example, after rebasing or outside a commit), run from the repo root:".README.md (1)
1-215: Consider dropping this README-only reformat from the CLI bugfix PR.This broad doc churn appears unrelated to issue
#1283(normalizeSessiongatewayName type validation) and makes the functional review harder than necessary. Prefer a separate docs-only PR if these formatting changes are intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 1 - 215, This PR mixes unrelated README-only formatting changes with the bugfix for normalizeSession (gatewayName type validation, issue `#1283`); remove the README.md edits from this branch by either reverting those changes or moving them into a separate docs-only PR, leaving only the functional fix around normalizeSession and any files/methods that touch gatewayName validation in the gateway/session code so reviewers can focus on the normalizeSession change.AGENTS.md (1)
1-194: Please drop unrelated line-ending-only churn from this PR.This PR is scoped to
normalizeSessionbehavior, but this file appears to be CRLF-only formatting churn. Reverting this file (or moving it to a separate docs/formatting PR) will keep review and blame cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` around lines 1 - 194, This PR includes unrelated CRLF-only churn in AGENTS.md; revert that change or move it into a separate formatting commit so the PR only contains the normalizeSession behavior change. Restore AGENTS.md to the canonical version from main (e.g., git restore --source=main AGENTS.md) or create a new branch/PR with the line-ending normalization, and ensure the current branch/PR excludes AGENTS.md before pushing so review and blame remain focused on normalizeSession.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/nemoclaw-maintainer-day/RISKY-AREAS.md:
- Line 10: Fix the typo in the workflow tooling label: replace the phrase "prek
hooks" in the table entry that currently reads ".github/workflows/, prek hooks,
DCO, signing, version/tag flows" with "pre-commit hooks" (or the exact hook type
used in this repo if different), and update any other occurrences of "prek
hooks" in the file to ensure consistency in the "Workflow / enforcement" row.
In
@.agents/skills/nemoclaw-user-configure-security/references/best-practices.md:
- Around line 1-490: This file (best-practices.md) is an autogenerated skill
artifact and must not be edited by hand; revert your manual changes, regenerate
the skill output using the docs-to-skills generator (run: python3
scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user), and
commit the generated file (or remove it from the PR if it was added
accidentally); do not make manual edits to the autogenerated content or its
header text that says "Never manually edit autogenerated skill files".
In @.agents/skills/nemoclaw-user-manage-policy/SKILL.md:
- Around line 1-171: The SKILL.md file "nemoclaw-user-manage-policy" was
manually edited but must be regenerated; revert manual changes by restoring the
file from the generated output or git history, then update the source docs under
docs/ (not .agents/skills/), and run the regeneration script python3
scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user to
produce a new .agents/skills/nemoclaw-user-manage-policy/SKILL.md; finally
commit the regenerated skill file.
- Around line 1-171: The SKILL.md documentation change appears unrelated to the
stated PR goal (fixing metadata.gatewayName type validation in
src/lib/onboard-session.ts and adding a regression test); either remove
.agents/skills/nemoclaw-user-manage-policy/SKILL.md from this branch/commit or
move it to a separate PR, or if the doc change is intentional, update the PR
title/body to explicitly mention the documentation update and why it accompanies
the metadata.gatewayName fix and tests; ensure commit messages reflect the doc
change as well so reviewers can see the rationale.
In @.agents/skills/nemoclaw-user-overview/references/overview.md:
- Around line 1-66: This README was manually edited but lives in an
autogenerated skill (nemoclaw-user-overview); revert your manual changes to that
skill file and regenerate the artifact using the documented generator command
(run the docs-to-skills script: python3 scripts/docs-to-skills.py docs/
.agents/skills/ --prefix nemoclaw-user) so the content is reproduced from source
docs rather than edited in-place; ensure you commit the regenerated output
instead of manual edits.
In @.agents/skills/nemoclaw-user-overview/SKILL.md:
- Around line 1-204: The SKILL.md under .agents/skills/nemoclaw-user-overview is
autogenerated and must not be edited manually; revert this file from the PR and
regenerate the skill from source docs using the documented script (run the
documented regeneration command that invokes python3 scripts/docs-to-skills.py
with docs/ and .agents/skills/ and the --prefix nemoclaw-user flag) so the
canonical source in docs/ produces the skill, or remove the manual edits and
re-run the generator instead of committing changes to the autogenerated file.
In @.github/actions/basic-checks/action.yaml:
- Around line 1-47: The YAML uses CRLF line endings causing yamllint failures;
convert the entire basic-checks composite action (the "name: basic-checks"
workflow in action.yaml) to use LF (\n) line endings—e.g., run dos2unix on the
file or configure Git/core.eol and a .gitattributes entry for *.yml and *.yaml
to enforce LF—and commit the normalized file so the runs/steps block and all
lines use LF endings.
In @.github/workflows/base-image.yaml:
- Around line 1-3: The workflow file uses CRLF endings which YAMLlint rejects;
convert the file to LF-only line endings for the header lines starting with "#
SPDX-FileCopyrightText" and "# SPDX-License-Identifier" (the top of the
workflow) and commit the change; you can fix this by running a newline
normalization (e.g., dos2unix on the file or set git core.autocrlf=false and
re-save with LF) or add/update a .gitattributes entry to enforce LF for YAML,
then re-commit the normalized base-image.yaml so CI/YAMLlint no longer reports
"wrong new line character".
In @.github/workflows/legacy-path-guard.yaml:
- Around line 1-35: This PR unintentionally includes an unrelated GitHub Actions
workflow ("legacy-path-guard" / job "guard-migrated-paths") that was reformatted
to CRLF line endings; remove this workflow change from the PR by reverting the
commit or excluding the file so the workflow .yaml remains unchanged (restore LF
line endings if necessary) and ensure only changes related to normalizeSession
metadata.gatewayName validation remain in the diff.
In @.github/workflows/pr.yaml:
- Around line 1-3: The workflow file contains CRLF line endings (the SPDX header
lines at the top show CRLF), which YAML linter fails on; convert the file to use
LF-only line endings by normalizing the file (e.g., run a dos2unix conversion or
set the repository to commit LF line endings and update .gitattributes), ensure
the file is saved with Unix LF (\n) and re-commit so the YAML linter passes.
In @.pre-commit-config.yaml:
- Line 1: Convert the CRLF line endings in the file that begins with the header
comment "# NemoClaw — prek hook configuration" to LF-only across the entire
file; replace any trailing \r characters so YAML linter sees \n line endings,
then ensure the repository stores the file with LF (e.g., normalize via your
editor or run a single-file conversion and recommit) so pre-commit/CI
`new-lines` checks pass.
In `@AGENTS.md`:
- Line 1: Add the required Markdown SPDX header to AGENTS.md by inserting an
HTML comment block at the top containing both the SPDX-FileCopyrightText entry
and the SPDX-License-Identifier entry (use the project/copyright owner text for
SPDX-FileCopyrightText and "Apache-2.0" for SPDX-License-Identifier) so the file
follows the guideline that "*.md" files must use HTML comments for SPDX headers.
In `@agents/hermes/plugin/plugin.yaml`:
- Around line 1-8: The YAML file has CRLF line endings causing YAMLlint
failures; open the file containing the manifest (look for the plugin.yaml with
name: nemoclaw and manifest_version) and convert/save it with LF-only line
endings (Unix-style \n), then commit the change so CI picks up the normalized
file; ensure your editor or .gitattributes enforces LF for *.yaml to prevent
regressions.
In `@agents/hermes/policy-additions.yaml`:
- Line 1: The file policy-additions.yaml contains CRLF line endings causing YAML
lint failures; open policy-additions.yaml and re-save or convert its line
endings to Unix LF (remove \r characters), then commit the updated file so
YAMLlint passes; optionally ensure consistent repository behavior by adding or
updating .gitattributes to enforce LF for *.yaml files.
In `@CONTRIBUTING.md`:
- Around line 1-3: Prepend the required SPDX HTML comment header to the very top
of CONTRIBUTING.md before the title line: add the HTML comment containing
"SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All
rights reserved." followed by "SPDX-License-Identifier: Apache-2.0" (both inside
HTML comment markers), ensuring the header appears as an HTML comment on a
single line or two lines immediately above the "# Contributing to NVIDIA
NemoClaw" title.
In `@README.md`:
- Line 6: The main README heading "# 🦞 NVIDIA NemoClaw: Reference Stack for
Running OpenClaw in OpenShell" includes an emoji that violates the Markdown
prose guideline; remove the lobster emoji so the heading reads "# NVIDIA
NemoClaw: Reference Stack for Running OpenClaw in OpenShell" (update the README
heading text accordingly).
- Around line 1-4: Replace the current multi-line HTML comment SPDX block with
the repository-required single-line Markdown SPDX header: consolidate the two
SPDX entries into inline HTML comments (the exact strings
"SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All
rights reserved." and "SPDX-License-Identifier: Apache-2.0") placed
consecutively on one line as HTML comments, removing the multi-line wrapper so
the README uses the required '<!-- ... --> <!-- ... -->' SPDX header format.
---
Nitpick comments:
In @.agents/skills/nemoclaw-contributor-update-docs/SKILL.md:
- Around line 1-188: Summary: The SKILL.md docs-skill rewrite was included in
the same PR as the gatewayName/normalizeSession bugfix; split it into its own
PR. Fix: remove the changes to nemoclaw-contributor-update-docs/SKILL.md from
the current branch and create a new branch/PR containing only that file (or
cherry-pick the SKILL.md commit(s) into a new branch), leaving the
gatewayName/normalizeSession-related commits (issue `#1283`, normalizeSession type
validation) in the original PR; ensure commit messages and the new PR
title/description reference "nemoclaw-contributor-update-docs" and the original
PR keeps only bugfix changes so traceability and release notes remain clear.
In `@AGENTS.md`:
- Around line 1-194: This PR includes unrelated CRLF-only churn in AGENTS.md;
revert that change or move it into a separate formatting commit so the PR only
contains the normalizeSession behavior change. Restore AGENTS.md to the
canonical version from main (e.g., git restore --source=main AGENTS.md) or
create a new branch/PR with the line-ending normalization, and ensure the
current branch/PR excludes AGENTS.md before pushing so review and blame remain
focused on normalizeSession.
In `@agents/hermes/manifest.yaml`:
- Around line 1-102: manifest.yaml contains CRLF line endings; convert it to LF
and add a repo-level rule to prevent recurrence. Replace CRLF with LF for
agents/hermes/manifest.yaml (use dos2unix, editor setting, or git add
--renormalize after updating core.autocrlf), commit the normalized file, and add
a .gitattributes entry like "*.yaml text eol=lf" to the repository; ensure you
stage and commit both the manifest.yaml normalization and the new .gitattributes
so future YAML files are checked out with LF.
In `@CONTRIBUTING.md`:
- Line 146: Update the phrasing in the CONTRIBUTING.md sentence that currently
reads "To regenerate skills manually (for example, after rebasing or outside of
a commit), run from the repo root:" by replacing "outside of a commit" with the
tighter "outside a commit" so the line reads "To regenerate skills manually (for
example, after rebasing or outside a commit), run from the repo root:".
In `@README.md`:
- Around line 1-215: This PR mixes unrelated README-only formatting changes with
the bugfix for normalizeSession (gatewayName type validation, issue `#1283`);
remove the README.md edits from this branch by either reverting those changes or
moving them into a separate docs-only PR, leaving only the functional fix around
normalizeSession and any files/methods that touch gatewayName validation in the
gateway/session code so reviewers can focus on the normalizeSession change.
🪄 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: Pro
Run ID: cf6c1f5d-df6d-495f-bd91-6b178bbd7827
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (251)
.agents/skills/nemoclaw-contributor-update-docs/SKILL.md.agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md.agents/skills/nemoclaw-maintainer-day/HOTSPOTS.md.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md.agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md.agents/skills/nemoclaw-maintainer-day/RISKY-AREAS.md.agents/skills/nemoclaw-maintainer-day/SALVAGE-PR.md.agents/skills/nemoclaw-maintainer-day/SECURITY-SWEEP.md.agents/skills/nemoclaw-maintainer-day/SEQUENCE-WORK.md.agents/skills/nemoclaw-maintainer-day/SKILL.md.agents/skills/nemoclaw-maintainer-day/STATE-SCHEMA.md.agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md.agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts.agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts.agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts.agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts.agents/skills/nemoclaw-maintainer-day/scripts/shared.ts.agents/skills/nemoclaw-maintainer-day/scripts/state.ts.agents/skills/nemoclaw-maintainer-day/scripts/triage.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts.agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts.agents/skills/nemoclaw-maintainer-evening/SKILL.md.agents/skills/nemoclaw-maintainer-find-review-pr/SKILL.md.agents/skills/nemoclaw-maintainer-morning/SKILL.md.agents/skills/nemoclaw-maintainer-security-code-review/SKILL.md.agents/skills/nemoclaw-skills-guide/SKILL.md.agents/skills/nemoclaw-user-configure-inference/SKILL.md.agents/skills/nemoclaw-user-configure-inference/references/inference-options.md.agents/skills/nemoclaw-user-configure-security/SKILL.md.agents/skills/nemoclaw-user-configure-security/references/best-practices.md.agents/skills/nemoclaw-user-configure-security/references/credential-storage.md.agents/skills/nemoclaw-user-deploy-remote/SKILL.md.agents/skills/nemoclaw-user-deploy-remote/references/sandbox-hardening.md.agents/skills/nemoclaw-user-get-started/SKILL.md.agents/skills/nemoclaw-user-manage-policy/SKILL.md.agents/skills/nemoclaw-user-monitor-sandbox/SKILL.md.agents/skills/nemoclaw-user-overview/SKILL.md.agents/skills/nemoclaw-user-overview/references/ecosystem.md.agents/skills/nemoclaw-user-overview/references/how-it-works.md.agents/skills/nemoclaw-user-overview/references/overview.md.agents/skills/nemoclaw-user-overview/references/release-notes.md.agents/skills/nemoclaw-user-reference/SKILL.md.agents/skills/nemoclaw-user-reference/references/architecture.md.agents/skills/nemoclaw-user-reference/references/commands.md.agents/skills/nemoclaw-user-reference/references/network-policies.md.agents/skills/nemoclaw-user-reference/references/troubleshooting.md.agents/skills/nemoclaw-user-skills-coding/SKILL.md.agents/skills/nemoclaw-user-skills-coding/references/agent-skills.md.agents/skills/nemoclaw-user-workspace/SKILL.md.agents/skills/nemoclaw-user-workspace/references/workspace-files.md.github/PULL_REQUEST_TEMPLATE.md.github/actions/basic-checks/action.yaml.github/workflows/base-image.yaml.github/workflows/docs-links-pr.yaml.github/workflows/e2e-brev.yaml.github/workflows/legacy-path-guard.yaml.github/workflows/nightly-e2e.yaml.github/workflows/pr.yaml.pre-commit-config.yamlAGENTS.mdCONTRIBUTING.mdDockerfileDockerfile.baseREADME.mdagents/hermes/Dockerfileagents/hermes/Dockerfile.baseagents/hermes/decode-proxy.pyagents/hermes/generate-config.tsagents/hermes/manifest.yamlagents/hermes/plugin/__init__.pyagents/hermes/plugin/plugin.yamlagents/hermes/policy-additions.yamlagents/hermes/policy-permissive.yamlagents/hermes/start.shagents/openclaw/manifest.yamlagents/openclaw/policy-permissive.yamlbin/lib/agent-defs.jsbin/lib/agent-onboard.jsbin/lib/agent-runtime.jsbin/lib/config-io.jsbin/lib/credentials.jsbin/lib/onboard.jsbin/lib/platform.jsbin/lib/policies.jsbin/lib/registry.jsbin/lib/runner.jsbin/lib/sandbox-build-context.jsbin/nemoclaw.jsci/platform-matrix.jsondocs/.docs-skipdocs/CONTRIBUTING.mddocs/_ext/json_output/core/json_formatter.pydocs/_ext/search_assets/enhanced-search.cssdocs/_ext/search_assets/modules/DocumentLoader.jsdocs/_ext/search_assets/modules/SearchEngine.jsdocs/about/ecosystem.mddocs/about/how-it-works.mddocs/about/overview.mddocs/about/release-notes.mddocs/conf.pydocs/deployment/deploy-to-remote-gpu.mddocs/deployment/sandbox-hardening.mddocs/deployment/set-up-telegram-bridge.mddocs/get-started/quickstart.mddocs/index.mddocs/inference/inference-options.mddocs/inference/switch-inference-providers.mddocs/inference/use-local-inference.mddocs/monitoring/monitor-sandbox-activity.mddocs/network-policy/approve-network-requests.mddocs/network-policy/customize-network-policy.mddocs/reference/architecture.mddocs/reference/commands.mddocs/reference/network-policies.mddocs/reference/troubleshooting.mddocs/resources/agent-skills.mddocs/resources/license.mddocs/security/best-practices.mddocs/security/credential-storage.mddocs/versions1.jsondocs/workspace/backup-restore.mddocs/workspace/workspace-files.mdeslint.config.mjsinstall.shk8s/nemoclaw-k8s.yamlnemoclaw-blueprint/blueprint.yamlnemoclaw-blueprint/policies/openclaw-sandbox-permissive.yamlnemoclaw-blueprint/policies/openclaw-sandbox.yamlnemoclaw-blueprint/policies/presets/brave.yamlnemoclaw-blueprint/policies/presets/brew.yamlnemoclaw-blueprint/policies/presets/discord.yamlnemoclaw-blueprint/policies/presets/github.yamlnemoclaw-blueprint/policies/presets/huggingface.yamlnemoclaw-blueprint/policies/presets/npm.yamlnemoclaw-blueprint/policies/presets/pypi.yamlnemoclaw/src/blueprint/runner.test.tsnemoclaw/src/blueprint/runner.tsnemoclaw/src/blueprint/ssrf.test.tsnemoclaw/src/blueprint/ssrf.tsnemoclaw/src/commands/migration-state.test.tsnemoclaw/src/commands/migration-state.tspackage.jsonpyproject.tomlschemas/blueprint.schema.jsonschemas/openclaw-plugin.schema.jsonschemas/policy-preset.schema.jsonschemas/sandbox-policy.schema.jsonscripts/benchmark-sandbox-image-build.jsscripts/brev-launchable-ci-cpu.shscripts/bump-version.tsscripts/check-legacy-migrated-paths.tsscripts/debug.shscripts/docs-to-skills.pyscripts/fix-coredns.shscripts/generate-platform-docs.pyscripts/install-openshell.shscripts/install.shscripts/lib/runtime.shscripts/migrate-js-to-ts.tsscripts/nemoclaw-start.shscripts/setup-dns-proxy.shscripts/smoke-macos-install.shscripts/start-services.shscripts/ts-migration-assist.tsscripts/ts-migration-bulk-fix-prs.tsscripts/ts-migration/README.mdscripts/ts-migration/move-map.jsonscripts/ts-migration/phases/01-tests-a.jsonscripts/ts-migration/phases/02-tests-b.jsonscripts/ts-migration/phases/03-tests-c.jsonscripts/ts-migration/phases/04-tests-d.jsonscripts/ts-migration/phases/05-runtime-leaves.jsonscripts/ts-migration/phases/06-runner.jsonscripts/ts-migration/phases/07-policies.jsonscripts/ts-migration/phases/08-tests-e.jsonscripts/ts-migration/phases/09-onboard-cli.jsonscripts/ts-migration/phases/README.mdscripts/ts-migration/phases/example.jsonscripts/validate-configs.tsspark-install.mdsrc/lib/agent-defs.tssrc/lib/agent-onboard.tssrc/lib/agent-runtime.tssrc/lib/config-io.test.tssrc/lib/config-io.tssrc/lib/credentials.tssrc/lib/dashboard.tssrc/lib/debug-command.test.tssrc/lib/debug-command.tssrc/lib/deploy.test.tssrc/lib/deploy.tssrc/lib/http-probe.test.tssrc/lib/http-probe.tssrc/lib/inference-config.tssrc/lib/inventory-commands.test.tssrc/lib/inventory-commands.tssrc/lib/model-prompts.test.tssrc/lib/model-prompts.tssrc/lib/onboard-session.test.tssrc/lib/onboard-session.tssrc/lib/onboard-types.tssrc/lib/onboard.tssrc/lib/openshell.test.tssrc/lib/openshell.tssrc/lib/paths.test.tssrc/lib/paths.tssrc/lib/platform.tssrc/lib/policies.tssrc/lib/preflight.test.tssrc/lib/preflight.tssrc/lib/provider-models.test.tssrc/lib/provider-models.tssrc/lib/registry.tssrc/lib/runner.tssrc/lib/runtime-recovery.tssrc/lib/sandbox-build-context.tssrc/lib/sandbox-create-stream.test.tssrc/lib/sandbox-create-stream.tssrc/lib/services-command.test.tssrc/lib/services-command.tssrc/lib/services.test.tssrc/lib/services.tssrc/lib/uninstall-command.test.tssrc/lib/uninstall-command.tssrc/lib/validation-recovery.test.tssrc/lib/validation-recovery.tssrc/lib/validation.test.tssrc/lib/validation.tssrc/lib/web-search.test.tssrc/lib/web-search.tssrc/nemoclaw.tstest/check-docs-links.test.tstest/cli.test.tstest/credential-exposure.test.tstest/credentials.test.tstest/dns-proxy.test.tstest/e2e-gateway-isolation.shtest/e2e-test.shtest/e2e/brev-e2e.test.tstest/e2e/e2e-cloud-experimental/check-docs.shtest/e2e/e2e-cloud-experimental/checks/04-landlock-readonly.shtest/e2e/test-hermes-e2e.shtest/e2e/test-messaging-providers.shtest/e2e/test-runtime-overrides.shtest/e2e/test-sandbox-survival.shtest/e2e/test-spark-install.shtest/gateway-cleanup.test.tstest/install-preflight.test.tstest/nemoclaw-cli-recovery.test.tstest/nemoclaw-start.test.tstest/onboard-readiness.test.ts
✅ Files skipped from review due to trivial changes (49)
- agents/hermes/decode-proxy.py
- .agents/skills/nemoclaw-user-overview/references/release-notes.md
- .agents/skills/nemoclaw-maintainer-day/STATE-SCHEMA.md
- agents/hermes/plugin/init.py
- .agents/skills/nemoclaw-maintainer-day/PR-REVIEW-PRIORITIES.md
- .agents/skills/nemoclaw-maintainer-day/scripts/state.ts
- .agents/skills/nemoclaw-maintainer-evening/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/SECURITY-SWEEP.md
- .agents/skills/nemoclaw-maintainer-day/SEQUENCE-WORK.md
- .agents/skills/nemoclaw-maintainer-find-review-pr/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/scripts/handoff-summary.ts
- .agents/skills/nemoclaw-maintainer-day/HOTSPOTS.md
- .agents/skills/nemoclaw-user-configure-inference/references/inference-options.md
- .agents/skills/nemoclaw-user-skills-coding/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/scripts/version-target.ts
- .agents/skills/nemoclaw-user-overview/references/how-it-works.md
- .agents/skills/nemoclaw-user-configure-security/SKILL.md
- .agents/skills/nemoclaw-maintainer-morning/SKILL.md
- .agents/skills/nemoclaw-user-overview/references/ecosystem.md
- .agents/skills/nemoclaw-maintainer-day/scripts/version-progress.ts
- .agents/skills/nemoclaw-user-deploy-remote/references/sandbox-hardening.md
- .agents/skills/nemoclaw-maintainer-cut-release-tag/SKILL.md
- .agents/skills/nemoclaw-user-configure-security/references/credential-storage.md
- .agents/skills/nemoclaw-maintainer-day/scripts/bump-stragglers.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/hotspots.ts
- agents/hermes/Dockerfile
- .agents/skills/nemoclaw-user-skills-coding/references/agent-skills.md
- .agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md
- .agents/skills/nemoclaw-user-workspace/SKILL.md
- agents/hermes/Dockerfile.base
- .github/PULL_REQUEST_TEMPLATE.md
- .agents/skills/nemoclaw-user-monitor-sandbox/SKILL.md
- .agents/skills/nemoclaw-user-reference/SKILL.md
- .agents/skills/nemoclaw-user-deploy-remote/SKILL.md
- .agents/skills/nemoclaw-user-get-started/SKILL.md
- .agents/skills/nemoclaw-user-workspace/references/workspace-files.md
- .agents/skills/nemoclaw-maintainer-day/SALVAGE-PR.md
- .agents/skills/nemoclaw-maintainer-security-code-review/SKILL.md
- .agents/skills/nemoclaw-maintainer-day/scripts/shared.ts
- .agents/skills/nemoclaw-maintainer-day/TEST-GAPS.md
- .agents/skills/nemoclaw-user-configure-inference/SKILL.md
- .agents/skills/nemoclaw-user-reference/references/architecture.md
- .agents/skills/nemoclaw-skills-guide/SKILL.md
- .agents/skills/nemoclaw-user-reference/references/commands.md
- agents/hermes/generate-config.ts
- .agents/skills/nemoclaw-maintainer-day/scripts/check-gates.ts
- .agents/skills/nemoclaw-user-reference/references/troubleshooting.md
- .agents/skills/nemoclaw-maintainer-day/scripts/triage.ts
6526fe1 to
270719e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard-session.test.ts (1)
129-129: Avoid hardcoding schema version in this fixture.Line 129 and Line 153 use
version: 1; this test will become brittle ifSESSION_VERSIONchanges. Prefersession.SESSION_VERSIONhere.Proposed change
- JSON.stringify({ version: 1, metadata: { gatewayName: 123 } }), + JSON.stringify({ + version: session.SESSION_VERSION, + metadata: { gatewayName: 123 }, + }),Also applies to: 153-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard-session.test.ts` at line 129, The test fixtures hardcode version: 1 which will break if SESSION_VERSION changes; update the JSON fixtures in src/lib/onboard-session.test.ts to use session.SESSION_VERSION instead of the literal 1 (replace the two occurrences that build JSON.stringify({ version: 1, ... }) with version: session.SESSION_VERSION), ensuring the test imports/uses the same session module (session.SESSION_VERSION) used by the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/onboard-session.test.ts`:
- Around line 149-158: Remove the duplicated Jest test titled "drops non-string
gatewayName during normalization" by deleting one of the identical it(...)
blocks; locate the block that writes JSON with metadata.gatewayName: 123 to
session.SESSION_FILE and calls session.loadSession(), and keep only a single
instance of that test (the other can be removed) so the test suite no longer
runs the same assertion twice.
---
Nitpick comments:
In `@src/lib/onboard-session.test.ts`:
- Line 129: The test fixtures hardcode version: 1 which will break if
SESSION_VERSION changes; update the JSON fixtures in
src/lib/onboard-session.test.ts to use session.SESSION_VERSION instead of the
literal 1 (replace the two occurrences that build JSON.stringify({ version: 1,
... }) with version: session.SESSION_VERSION), ensuring the test imports/uses
the same session module (session.SESSION_VERSION) used by the implementation.
🪄 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: Pro
Run ID: 2d12d20d-7f3a-433d-bccf-c9b134fcbec4
📒 Files selected for processing (1)
src/lib/onboard-session.test.ts
5fbbbdf to
4c28a9b
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
e085043 to
139e7cd
Compare
|
@wscurran Thanks for flagging the drift. Rebased onto latest Cheers! |
ed4d572 to
49e9afe
Compare
prekshivyas
left a comment
There was a problem hiding this comment.
Clean defensive fix — type guard on gatewayName with safe fallback to default. Test covers the case. @latenighthackathon can you rebase onto main?
ericksoa
left a comment
There was a problem hiding this comment.
@latenighthackathon — the DCO check is currently failing because your commits are missing the Signed-off-by line. Could you please sign off on your commits? You can do this by rebasing and amending:
git rebase HEAD~2 --signoff
git push --force-with-leaseWe'll merge once the DCO check passes. Thanks!
352eee0 to
1f62470
Compare
Non-string gatewayName values (e.g. numbers from corrupt session files) now fall back to 'nemoclaw' instead of passing through as raw types. Adds a regression test that writes gatewayName: 123 and asserts the normalized value is the string 'nemoclaw'. Closes NVIDIA#1283 Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
1f62470 to
c521874
Compare
|
@ericksoa — Update Branch click added an unsigned merge commit on top of the signed one, so I rebased onto current |
Sincerest apologies for the messed up commit. Gave you credit on the PR that moved this test in. |
Summary
typeofcheck forgatewayNameinnormalizeSessionso non-string values are droppedRelated Issue
Closes #1283
Changes
In
src/lib/onboard-session.ts,normalizeSessioncopiesmetadata.gatewayNamewithout checking its type. A persisted payload like{ "metadata": { "gatewayName": 123 } }would pass through andloadSession()would return a Session wheremetadata.gatewayNameis not a string.Added
typeof ... === "string"check. When gatewayName is not a string, metadata falls back to undefined and the default "nemoclaw" is used.Added regression test for numeric gatewayName.
Testing
Checklist
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit
Bug Fixes
Tests