feat(onboard): add agents.yaml declarative manifest - #5440
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…sed file Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
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:
📝 WalkthroughWalkthroughIntroduces a declarative ChangesDeclarative Multi-Agent Manifest Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
🚥 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 |
|
🌿 Preview your docs: https://nvidia-preview-pr-5440.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
PR Review AdvisorFindings: 2 needs attention, 8 worth checking, 1 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/inference/declarative-agents-manifest.mdx (1)
148-155:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a bottom
Next Stepssection with related page links.The page currently ends without the required terminal
Next Stepssection.
As per coding guidelines, “A ‘Next Steps’ section at the bottom links to related pages.”🤖 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 `@docs/inference/declarative-agents-manifest.mdx` around lines 148 - 155, Add a "Next Steps" section at the end of the document (after the existing content that ends at line 155) that links to related pages relevant to declarative agents and the onboarding/manifest workflow. Follow the existing documentation guidelines by including a terminal Next Steps section with links to related documentation pages that would help users continue their workflow after understanding the agent iteration concepts covered in this page.Source: Coding guidelines
🧹 Nitpick comments (1)
docs/inference/declarative-agents-manifest.mdx (1)
120-120: ⚡ Quick winRemove the colon from this section title.
Titles should not contain colons.
As per coding guidelines, “No colons in titles. Flag ‘Inference: Cloud and Local’ — should be ‘Cloud and Local Inference.’”🤖 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 `@docs/inference/declarative-agents-manifest.mdx` at line 120, The section title "Example: Manager-Worker" at line 120 contains a colon, which violates the documentation style guidelines that prohibit colons in titles. Remove the colon from this section heading and restructure it if needed to maintain clarity, such as changing it to "Manager-Worker Example" or "Example Manager-Worker".Source: Coding guidelines
🤖 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 `@docs/inference/declarative-agents-manifest.mdx`:
- Around line 1-19: The frontmatter metadata in the
declarative-agents-manifest.mdx file is missing required fields that are part of
the documentation standards. Add the missing frontmatter keys `topics`, `tags`,
`difficulty`, `audience`, and `status` to the existing frontmatter section
(which currently contains title, description, keywords, content type, and skill
priority). These fields should be added alongside the existing metadata to
comply with the coding guidelines for documentation pages.
In `@scripts/generate-openclaw-config.mts`:
- Around line 627-642: The validateModelRef function accepts model references
with whitespace-only model portions (after the slash), such as `test-provider/
`, which should be rejected. Add a check to extract the model portion using
`raw.slice(slash + 1)` after validating the slash position, and reject the input
if this model portion is empty or contains only whitespace characters. This
ensures that both the provider and model components are substantive before
accepting the model reference.
In `@test/generate-openclaw-config.test.ts`:
- Around line 1184-1188: The legacy test-size budget for
generate-openclaw-config.test.ts is out of sync with the current file size.
Update the budget entry for generate-openclaw-config.test.ts in the
test-file-size-budget configuration from 1989 to 1984 to reflect the file's
current line count and satisfy the codebase-growth guardrail requirement that
legacy budgets ratchet down when tests shrink.
---
Outside diff comments:
In `@docs/inference/declarative-agents-manifest.mdx`:
- Around line 148-155: Add a "Next Steps" section at the end of the document
(after the existing content that ends at line 155) that links to related pages
relevant to declarative agents and the onboarding/manifest workflow. Follow the
existing documentation guidelines by including a terminal Next Steps section
with links to related documentation pages that would help users continue their
workflow after understanding the agent iteration concepts covered in this page.
---
Nitpick comments:
In `@docs/inference/declarative-agents-manifest.mdx`:
- Line 120: The section title "Example: Manager-Worker" at line 120 contains a
colon, which violates the documentation style guidelines that prohibit colons in
titles. Remove the colon from this section heading and restructure it if needed
to maintain clarity, such as changing it to "Manager-Worker Example" or "Example
Manager-Worker".
🪄 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: 6f58eea9-0e80-424c-816d-6dc5a9ccc48c
📒 Files selected for processing (11)
docs/index.ymldocs/inference/declarative-agents-manifest.mdxscripts/generate-openclaw-config.mtssrc/lib/onboard/agents-manifest.test.tssrc/lib/onboard/agents-manifest.tssrc/lib/onboard/command-support.tssrc/lib/onboard/dockerfile-patch-extra-agents.test.tssrc/lib/onboard/legacy-command.test.tssrc/lib/onboard/legacy-command.tstest/generate-openclaw-config-agents-manifest.test.tstest/generate-openclaw-config.test.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/inference/declarative-agents-manifest.mdx (1)
1-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the required docs frontmatter metadata fields.
Lines 4-18 still omit required keys for new docs pages:
topics,tags,difficulty,audience, andstatus.As per coding guidelines, frontmatter must include
title,description,keywords,topics,tags,content type,difficulty,audience, andstatusfields.🤖 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 `@docs/inference/declarative-agents-manifest.mdx` around lines 1 - 19, Add the five missing required frontmatter fields to the YAML metadata block in declarative-agents-manifest.mdx: topics, tags, difficulty, audience, and status. These fields must be added to the frontmatter section (currently lines 1-18) alongside the existing title, description, keywords, and content type fields to comply with the documentation guidelines that require all new docs pages to include these mandatory metadata fields.Source: Coding guidelines
🤖 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 `@docs/inference/declarative-agents-manifest.mdx`:
- Around line 1-19: Add the five missing required frontmatter fields to the YAML
metadata block in declarative-agents-manifest.mdx: topics, tags, difficulty,
audience, and status. These fields must be added to the frontmatter section
(currently lines 1-18) alongside the existing title, description, keywords, and
content type fields to comply with the documentation guidelines that require all
new docs pages to include these mandatory metadata fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dfff194e-e3ec-46c4-9f62-be45f9bd0a95
📒 Files selected for processing (5)
ci/test-file-size-budget.jsondocs/inference/declarative-agents-manifest.mdxscripts/generate-openclaw-config.mtssrc/lib/onboard/agents-manifest.tstest/generate-openclaw-config-agents-manifest.test.ts
✅ Files skipped from review due to trivial changes (1)
- ci/test-file-size-budget.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/onboard/agents-manifest.ts
- scripts/generate-openclaw-config.mts
…parity Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…gents.test.ts Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
…nformance Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
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 `@docs/reference/commands-nemohermes.mdx`:
- Line 1485: Update the documentation to show required arguments for flags in
the command descriptions. Specifically, change `--agents` to `--agents
<agents.yaml>` in the flag lists at both line 1485 and line 1498 where the alias
command flags are documented. For consistency, also update other flags that
require values such as `--from <Dockerfile>` and `--name <sandbox>` to clearly
show their required argument placeholders in both locations.
🪄 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: e2b458c9-665d-4681-9a5c-e45826510285
📒 Files selected for processing (5)
docs/inference/declarative-agents-manifest.mdxdocs/reference/commands-nemohermes.mdxsrc/lib/onboard/agents-manifest.test.tssrc/lib/onboard/agents-manifest.tstest/agents-manifest-policy-conformance.test.ts
✅ Files skipped from review due to trivial changes (1)
- docs/inference/declarative-agents-manifest.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/onboard/agents-manifest.test.ts
- src/lib/onboard/agents-manifest.ts
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/actions/sandbox/agents/apply.test.ts (1)
82-193: ⚡ Quick winConsider adding test coverage for the interactive-mode exit path.
The test suite covers the
nonInteractive: truewithout--yescase (line 146-169, expects exit 1), but there's no test for thenonInteractive: false(or omitted) without--yescase that exits with code 2 (implementation line 244-246 inapply.ts).Adding a test case for this path would improve coverage and document the expected exit-code differentiation behavior.
🤖 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 `@src/lib/actions/sandbox/agents/apply.test.ts` around lines 82 - 193, Add a new test case to the runAgentsApply describe block that covers the interactive-mode exit path. Create a test similar to the existing "refuses to apply roster changes without --yes in non-interactive mode" test, but instead set nonInteractive to false (or omit it entirely) while still not providing the --yes flag. This test should verify that the exit function is called with code 2 to differentiate from the non-interactive mode behavior which exits with code 1. This will complete coverage of both exit paths documented in the implementation at lines 244-246 of apply.ts.src/lib/actions/sandbox/agents/apply.ts (1)
240-247: ⚡ Quick winClarify interactive mode behavior and exit code semantics.
The implementation exits with code 2 when
!yes && !nonInteractive, but it doesn't actually prompt the user interactively. The help text in the command file (line 32) says--yesconfirms "without an interactive prompt," implying that without--yesthere would BE a prompt.Current behavior:
--yes→ proceed--yesmissing +--non-interactive→ exit 1--yesmissing + no--non-interactive→ exit 2 (but no prompt)Either implement an interactive prompt for the exit-2 path, or update the help text to clarify that
--yesis currently required in all cases (with different exit codes allowing automation to distinguish the contexts).🤖 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 `@src/lib/actions/sandbox/agents/apply.ts` around lines 240 - 247, The code exits with different codes based on the nonInteractive flag, but the `!options.yes && !options.nonInteractive` case does not actually prompt the user for confirmation as the help text suggests it should. Either implement an interactive prompt (for example, using a library like inquirer or a built-in prompt mechanism) in the branch where `!options.yes && !options.nonInteractive` to ask the user to confirm the roster changes before proceeding, or update the help text in the command file (line 32) to clarify that the `--yes` flag is required in all cases with different exit codes distinguishing between interactive and non-interactive contexts. Choose one approach and ensure the behavior matches the documented expectations.
🤖 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 `@docs/reference/commands.mdx`:
- Line 1257: The rebuild guidance command on line 1257 uses the literal
`nemoclaw onboard` command instead of the shared placeholder `$$nemoclaw`.
Replace the literal `nemoclaw` command name with the `$$nemoclaw` placeholder to
maintain consistency with other CLI command examples on this shared reference
page, so the line reads `$$nemoclaw onboard --agents <file> --recreate-sandbox`.
- Around line 1253-1263: The command heading and examples in the documentation
for the agents apply command show incorrect syntax that does not match the
actual CLI implementation. Update the heading from `$$nemoclaw <name> agents
apply` to `$$nemoclaw sandbox agents apply <name>`, and update all command
examples below it to use the correct syntax order: `$$nemoclaw sandbox agents
apply <name>` followed by the flags, instead of the currently documented
`$$nemoclaw <name> agents apply` pattern. This ensures the documentation
accurately reflects the actual CLI routing and passes parity checks.
---
Nitpick comments:
In `@src/lib/actions/sandbox/agents/apply.test.ts`:
- Around line 82-193: Add a new test case to the runAgentsApply describe block
that covers the interactive-mode exit path. Create a test similar to the
existing "refuses to apply roster changes without --yes in non-interactive mode"
test, but instead set nonInteractive to false (or omit it entirely) while still
not providing the --yes flag. This test should verify that the exit function is
called with code 2 to differentiate from the non-interactive mode behavior which
exits with code 1. This will complete coverage of both exit paths documented in
the implementation at lines 244-246 of apply.ts.
In `@src/lib/actions/sandbox/agents/apply.ts`:
- Around line 240-247: The code exits with different codes based on the
nonInteractive flag, but the `!options.yes && !options.nonInteractive` case does
not actually prompt the user for confirmation as the help text suggests it
should. Either implement an interactive prompt (for example, using a library
like inquirer or a built-in prompt mechanism) in the branch where `!options.yes
&& !options.nonInteractive` to ask the user to confirm the roster changes before
proceeding, or update the help text in the command file (line 32) to clarify
that the `--yes` flag is required in all cases with different exit codes
distinguishing between interactive and non-interactive contexts. Choose one
approach and ensure the behavior matches the documented expectations.
🪄 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: 9e59dff5-1866-445b-b8f9-1443006f8bd2
📒 Files selected for processing (11)
docs/inference/declarative-agents-manifest.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/commands/sandbox/agents.tssrc/commands/sandbox/agents/apply.tssrc/lib/actions/sandbox/agents/apply.test.tssrc/lib/actions/sandbox/agents/apply.tssrc/lib/actions/sandbox/agents/passthrough.tssrc/lib/onboard/agents-manifest.test.tssrc/lib/onboard/agents-manifest.tstest/agents-manifest-policy-conformance.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/commands/sandbox/agents.ts
- docs/inference/declarative-agents-manifest.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/onboard/agents-manifest.test.ts
- src/lib/onboard/agents-manifest.ts
- test/agents-manifest-policy-conformance.test.ts
…ant sync Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Summary
Add
nemoclaw onboard --agents <agents.yaml>to bake a declarative manifest into the sandbox image at build time, andnemoclaw <name> agents apply -f <agents.yaml>to reconcile the manifest against an already-running sandbox without a rebuild. The manifest schema mirrors OpenClaw's ownagents.list[]field names verbatim, and per-agentmodeloverrides fan out intomodels.providers[<onboard-provider>].models[]automatically.Related Issue
Resolves #2853
Complements #2854 (host-side OpenClaw agent lifecycle commands): the manifest is the fixed checked-in path that bakes a layout into the image, while the
nemoclaw <sandbox> agents add|delete|listlifecycle commands cover ad-hoc edits inside an existing sandbox. The two paths share the sameagents.list[]semantics and OpenClaw-native field vocabulary, and the newagents applyverb sits on top ofagents add|deleteto drive declarative roster reconciliation from the same manifest the build path consumes.This PR extends the
NEMOCLAW_EXTRA_AGENTS_JSONbake path introduced in #4653. While extending it, the previous validator was found to acceptsubagents.maxSpawnDepthper-agent and to allow value0. OpenClaw's per-agent zod schema is.strict()and never accepted that field, and the global default schema requiresmin(1).max(5). Any non-emptyNEMOCLAW_EXTRA_AGENTS_JSON_B64payload would have tripped OpenClaw's runtime parser. This PR strict-rejects per-agentmaxSpawnDepthwith a migration hint pointing at the top-leveldefaults.subagents.maxSpawnDepth.Changes
scripts/generate-openclaw-config.mts: accept either the legacy bare-array payload or a new{agents, defaults?, main?}object. Extend the per-agent allowlist withmodel; replaceALLOWED_SUBAGENTS_KEYSwith OpenClaw-native fields{delegationMode, allowAgents, model, thinking, requireAgentId}. Strict-reject per-agentmaxSpawnDepthwith a clear migration hint. AddvalidateAgentsDefaultsandvalidateMainOverridessodefaults.subagents.maxSpawnDepthandmain.{tools, subagents}bake into the canonical places. Widenproviders[<onboard-provider>].models[]with one entry per unique referencedprovider/modelref; reject cross-provider refs.src/lib/onboard/agents-manifest.ts(new): YAML loader that parses the manifest, auto-fills canonicalworkspace/agentDirpaths fromid, and setsNEMOCLAW_EXTRA_AGENTS_JSONfor the downstream Dockerfile patcher. Includes a defence-in-depth credential-name denylist (apiKey,token,secret, etc.) before the payload reaches the staged Dockerfile/build context.src/lib/onboard/command-support.ts,src/lib/onboard/legacy-command.ts: thread--agents <file>through the oclif flag definition, legacy-args parser,OnboardCommandOptions.agentsManifest, andrunOnboardCommand(invokesapplyAgentsManifestEnvbefore dispatching to the underlyingrunOnboard).src/lib/actions/sandbox/agents/apply.ts(new),src/commands/sandbox/agents/apply.ts(new): in-sandboxagents applyverb. Lists current agents viaopenclaw agents list --json, diffs against the manifest, runsopenclaw agents add|deleteper item. Per-agentmodel,subagents.*, top-leveldefaults, andmainoverrides require a rebuild and are reported as warnings before exit. Confirmation gates:--yesto apply,--non-interactiveto fail fast when--yesis absent.test/generate-openclaw-config.test.ts,test/generate-openclaw-config-agents-manifest.test.ts(new): drop the per-agentmaxSpawnDepthfixture; add coverage in the dedicated file for the new payload object shape, per-agentmodelaccept + provider-prefix reject,allowAgentsround-trip,delegationModeenum rejection,defaults.subagents.maxSpawnDepthrange,main.{subagents, tools}merge, multi-model provider expansion, and the per-agentmaxSpawnDepthstrict-reject with migration hint. Split keeps the original file under the legacy size budget.test/agents-manifest-policy-conformance.test.ts(new): policy mirror of OpenClaw'sresolveSubagentTargetPolicyagainst the bakedagents.list[].subagents.allowAgentsshape — covers configured target allow, unlisted target deny, wildcard"*"+ unknown target deny, and self-only default.src/lib/onboard/agents-manifest.test.ts(new),src/lib/onboard/agents-manifest-credential-denylist: 12 cases for the YAML loader plus the credential denylist matrix.src/lib/onboard/legacy-command.test.ts,src/lib/onboard/legacy-command-agents.test.ts(new):--agentslifecycle moved into the focused file (parse, missing-path / missing-value rejection, env-var application beforerunOnboard).src/lib/actions/sandbox/agents/apply.test.ts(new): diff computation + rebuild-only warning +--yes/--non-interactivegating + no-op when roster matches.src/lib/onboard/dockerfile-patch-extra-agents.test.ts: rotate the fixture fromsubagents: { maxSpawnDepth: 0 }tosubagents: { allowAgents: ["analyst"] }.docs/inference/declarative-agents-manifest.mdx(new),docs/reference/commands.mdx,docs/reference/commands-nemohermes.mdx,docs/index.yml: user-facing reference for the manifest schema, themain/defaults/agentsblocks, the OpenClaw-native sub-agent fields, the multi-model bake semantics, a manager-worker walkthrough, the newagents applyverb, and the--agentsonboard flag listing.Type of Change
Verification
npx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Release Notes
--agents <agents.yaml>to onboarding (including compatibility aliases).sandbox <name> agents applyto reconcile a sandbox’s agents from a manifest, with rebuild-required warnings.