fix(plugin): tolerate empty/malformed onboard config.json#3906
Conversation
📝 WalkthroughWalkthroughloadOnboardConfig() now returns null instead of throwing on unreadable or invalid JSON; tests added for empty/whitespace/malformed files; Dockerfile initializes ChangesConfig Loading Resilience
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
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.
🧹 Nitpick comments (2)
nemoclaw/src/onboard/config.ts (1)
633-633: Verify with E2E tests.This Dockerfile change seeds the initial config file and affects sandbox startup behavior. As per coding guidelines, changes affecting the sandbox container should be validated with real container builds.
The coding guidelines recommend running these E2E tests to confirm the fix across startup scenarios:
gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2eAs per coding guidelines: "Dockerfile affects the sandbox container image. Layer ordering, permissions, and baked config changes are only testable with a real container build."
🤖 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 `@nemoclaw/src/onboard/config.ts` at line 633, This change modifies the Dockerfile that seeds the initial config used by the sandbox startup (affecting sandbox container image and baked config), so validate it by running the recommended E2E workflows: trigger the nightly-e2e.yaml with the jobs cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e (use gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e) to ensure sandbox startup, layer ordering, permissions and the seeded config (onboard/config.ts related behavior) work correctly in a real container build.Dockerfile (1)
633-634: LGTM!Reminder: Run E2E tests for Dockerfile changes.
As per coding guidelines, Dockerfile changes affecting the sandbox container image should be validated with E2E tests. Consider running:
gh workflow run nightly-e2e.yaml --ref fix/3884-plugin-config-empty-pr -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e🤖 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 `@Dockerfile` around lines 633 - 634, The Dockerfile change that creates and chowns /sandbox/.nemoclaw/config.json (the lines with "printf '%s' '{}' > /sandbox/.nemoclaw/config.json" and "chown sandbox:sandbox /sandbox/.nemoclaw/config.json") must be validated by running the E2E suites for sandbox-related images; run the GitHub Actions workflow suggested (e.g. gh workflow run nightly-e2e.yaml --ref fix/3884-plugin-config-empty-pr -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e) and verify sandbox survival and cloud e2e results before merging.
🤖 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.
Nitpick comments:
In `@Dockerfile`:
- Around line 633-634: The Dockerfile change that creates and chowns
/sandbox/.nemoclaw/config.json (the lines with "printf '%s' '{}' >
/sandbox/.nemoclaw/config.json" and "chown sandbox:sandbox
/sandbox/.nemoclaw/config.json") must be validated by running the E2E suites for
sandbox-related images; run the GitHub Actions workflow suggested (e.g. gh
workflow run nightly-e2e.yaml --ref fix/3884-plugin-config-empty-pr -f
jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e) and verify
sandbox survival and cloud e2e results before merging.
In `@nemoclaw/src/onboard/config.ts`:
- Line 633: This change modifies the Dockerfile that seeds the initial config
used by the sandbox startup (affecting sandbox container image and baked
config), so validate it by running the recommended E2E workflows: trigger the
nightly-e2e.yaml with the jobs
cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e (use gh workflow
run nightly-e2e.yaml --ref <branch> -f
jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e) to ensure
sandbox startup, layer ordering, permissions and the seeded config
(onboard/config.ts related behavior) work correctly in a real container build.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b24b7214-39ec-48c4-99e7-d82e3d01ff57
📒 Files selected for processing (3)
Dockerfilenemoclaw/src/onboard/config.test.tsnemoclaw/src/onboard/config.ts
1 similar comment
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
655-663: Run the Dockerfile-targeted E2E suite for this layer change.Since this modifies container runtime behavior (
HEALTHCHECK) and image build logic, please run:
cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2evianightly-e2e.yamlbefore merge.As per coding guidelines, “Dockerfile … changes are only testable with a real container build” and the recommended selective workflow is
gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e.🤖 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 `@Dockerfile` around lines 655 - 663, This change updates the Dockerfile HEALTHCHECK logic (see HEALTHCHECK and the env vars NEMOCLAW_DASHBOARD_PORT, OPENCLAW_GATEWAY_PORT and CHAT_UI_URL), so before merging run the container-level E2E suites via the nightly workflow: execute gh workflow run nightly-e2e.yaml --ref <branch> -f jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e (this will build the image and validate the HEALTHCHECK/runtime behavior); wait for those jobs to pass and attach results to the PR before merging.
🤖 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.
Nitpick comments:
In `@Dockerfile`:
- Around line 655-663: This change updates the Dockerfile HEALTHCHECK logic (see
HEALTHCHECK and the env vars NEMOCLAW_DASHBOARD_PORT, OPENCLAW_GATEWAY_PORT and
CHAT_UI_URL), so before merging run the container-level E2E suites via the
nightly workflow: execute gh workflow run nightly-e2e.yaml --ref <branch> -f
jobs=cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e (this will
build the image and validate the HEALTHCHECK/runtime behavior); wait for those
jobs to pass and attach results to the PR before merging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d7a97670-464b-4e52-9d75-efcc108221ae
📒 Files selected for processing (1)
Dockerfile
Summary
Dockerfile:633has always seeded/sandbox/.nemoclaw/config.jsonas a 0-byte file viatouch— the file exists so the next line'schown sandbox:sandboxhas a target.nemoclaw/src/onboard/config.ts:loadOnboardConfigcallsJSON.parse(readFileSync(path, "utf-8"))with notry/catch.JSON.parse("")throwsSyntaxError.Related Issue
Fixes #3884
Changes
Dockerfile:633- seed valid empty objectType of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: rluo8 ruluo@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests