fix(deepagents): preserve Ollama colon-tag model id in generated config.toml

[yanyunl1991]

Summary

Onboarding a Deep Agents (dcode / langchain-deepagents-code) sandbox on a local Ollama model with a colon tag (e.g. qwen2.5:7b) wrote a truncated model id into ~/.deepagents/config.toml: default = "openai:7b" and models = ["7b"] instead of default = "openai:qwen2.5:7b" and models = ["qwen2.5:7b"]. Root cause: modelNameForOpenAiProvider in agents/langchain-deepagents-code/generate-config.ts split the input on the first : under the assumption it was a <provider>:<model> prefix — but NEMOCLAW_MODEL is always the bare model id, and the : in an Ollama tag is the name:tag separator, not a provider prefix. Pass the model id through verbatim.

Closes #6325.

Reproduction

Reporter's argv (from #6325):

NEMOCLAW_PROVIDER=ollama nemoclaw onboard --agent dcode --name <name> --non-interactive --fresh --yes
# ... after onboard succeeds ...
nemoclaw <name> exec -- grep -E "default|models" /sandbox/.deepagents/config.toml

Environment

• Reporter: Ubuntu 26.04 x86_64 with RTX PRO 6000 Blackwell GPU, NemoClaw v0.0.74, Ollama model qwen2.5:7b.
• Our validation: reasoning-based (see below). We do not have an x86_64 Linux discrete-GPU host with Ollama on the test fleet; running a live end-to-end dcode onboard against a real Ollama daemon is out of reach here. The reporter's config.toml payload is deterministic in buildConfig, so a targeted unit test that feeds Settings.model = "qwen2.5:7b" into the same buildConfig the Dockerfile invokes at image-build time exercises the exact code path — see Verification.

Observed on main (before fix)

buildConfig called with Settings.model = "qwen2.5:7b" produces (trimmed to the two lines that regressed):

[models]
default = "openai:7b"
...
models = ["7b"]

Reporter's transcript from the actual sandbox on v0.0.74:

host model tag: qwen2.5:7b
config.toml default = "openai:7b"
config.toml models  = ["7b"]

Observed on fix/dcode-model-colon-truncation-6325 (after fix)

buildConfig called with the same Settings.model = "qwen2.5:7b" now produces:

[models]
default = "openai:qwen2.5:7b"
...
models = ["qwen2.5:7b"]

Analysis

agents/langchain-deepagents-code/generate-config.ts::modelNameForOpenAiProvider was:

function modelNameForOpenAiProvider(model: string): string {
  const trimmed = model.trim();
  const providerSeparator = trimmed.indexOf(":");
  if (providerSeparator > 0) {
    return trimmed.slice(providerSeparator + 1);
  }
  return trimmed;
}

The strip is only meaningful if callers pass NEMOCLAW_MODEL in <provider>:<model> shape. They do not: src/lib/onboard/providers.ts::getNonInteractiveModel reads the resolved model id off process.env.NEMOCLAW_MODEL after the wizard's model-selection step, and src/lib/onboard/dockerfile-patch.ts::125 bakes it into the Dockerfile ARG. The build-arg validator (isSafeModelId) explicitly allows : in a legal model id — a decision NemoClaw made precisely because Ollama tags carry a colon. So the strip was a documentation-only claim, and on any Ollama model with a colon tag it dropped the leading segment silently, leaving models = ["7b"] on the reporter's box.

The openai: provider prefix in default = "openai:<model>" is prepended by buildConfig, not by the caller — so removing the strip does not risk double-prefixing.

Fix

agents/langchain-deepagents-code/generate-config.ts:

• Replace the strip in modelNameForOpenAiProvider with return model.trim();. Keep the function name for auditability (external code does not import it, but preserving the identifier keeps git blame clean and the audit comment cites [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325).
• export buildConfig, main, Settings, and modelNameForOpenAiProvider so a co-located test can drive them without spawning the build script. Gate the top-level main() invocation behind isMainModule() — same pattern already used by the sibling agents/hermes/generate-config.ts.

test/generate-dcode-config.test.ts (new):

• qwen2.5:7b round-trips verbatim through modelNameForOpenAiProvider (would have caught the original regression).
• Additional Ollama variant llama3.1:8b-instruct-q4_0 round-trips verbatim.
• Non-colonized ids (nvidia/nemotron-3-super-120b-a12b, gpt-5.4-mini) pass through unchanged.
• buildConfig end-to-end: Settings.model = "qwen2.5:7b" produces both default = "openai:qwen2.5:7b" and models = ["qwen2.5:7b"], and explicitly checks the old "openai:7b" / ["7b"] shape does NOT appear.
• Sanity guard: only one "openai: prefix appears in the generated TOML (no accidental double-prefix such as openai:openai:qwen2.5:7b).

Changes

• agents/langchain-deepagents-code/generate-config.ts: fix modelNameForOpenAiProvider, export the pure functions + Settings, gate main() with isMainModule().
• test/generate-dcode-config.test.ts (new): 7 regression tests covering the reporter's scenario and non-regressions.

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Verification

• Targeted vitest: npx vitest run --project integration test/generate-dcode-config.test.ts → 7/7 pass.
• npm run build:cli passes on the branch.
• git diff --check clean; only the two files above changed (+101 / -12).
• No secrets, API keys, or credentials committed.
• Live end-to-end onboard against a real Ollama daemon on generic Linux is NOT included — we don't own an x86_64 discrete-GPU Linux host on the test fleet, and the reporter's platform scope note ("config-generation defect, platform-independent") explicitly says the defect is not platform-conditional. The unit test drives the same buildConfig function the Dockerfile's build-arg pipeline invokes at image-build time.

AI Disclosure

• AI-assisted — tool: Claude Code

Signed-off-by: Yanyun Liao yanyunl@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Preserved full OpenAI/Ollama model identifiers that contain : so tagged models aren’t truncated.
  • Updated generated configuration to retain complete model IDs and avoid duplicated/incorrect openai: prefixing.
  • Prevented automatic execution when the configuration generator is imported, running only when launched directly.

• Tests

  • Added Vitest coverage for colon-tagged model names, whitespace trimming, and verifying generated config output correctness.

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/dcode-model-colo... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	fix/dcode-model-colo... 899d22c	+/-
nemoclaw/src/se...cret-scanner.ts	—	100%	—
nemoclaw/src/commands/slash.ts	—	100%	—
nemoclaw/src/li...bprocess-env.ts	—	100%	—
nemoclaw/src/bl...eprint/state.ts	—	98%	—
nemoclaw/src/onboard/config.ts	—	98%	—
nemoclaw/src/bl...int/snapshot.ts	—	97%	—
nemoclaw/src/bl...print/runner.ts	—	95%	—
nemoclaw/src/co...ration-state.ts	—	94%	—
nemoclaw/src/bl...ate-networks.ts	—	94%	—
nemoclaw/src/index.ts	—	94%	—

TypeScript / code-coverage/cli

The overall coverage in the fix/dcode-model-colo... branch is 71%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.

File	main	fix/dcode-model-colo... 899d22c	+/-
src/lib/shields...nsition-lock.ts	—	87%	—
src/lib/actions...all/run-plan.ts	—	81%	—
src/lib/state/o...oard-session.ts	—	81%	—
src/lib/onboard/preflight.ts	—	71%	—
src/lib/state/sandbox.ts	—	71%	—
src/lib/onboard...er-gpu-patch.ts	—	69%	—
src/lib/shields/index.ts	—	68%	—
src/lib/policy/index.ts	—	66%	—
src/lib/actions...licy-channel.ts	—	60%	—
src/lib/onboard.ts	—	24%	—

---

_{Updated July 06, 2026 12:13 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4186eb2f-e6f5-43ad-a1e2-7545d235060d

📥 Commits

Reviewing files that changed from the base of the PR and between 75e3ac9 and 899d22c.

📒 Files selected for processing (2)

• agents/langchain-deepagents-code/generate-config.ts
• test/generate-dcode-config.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• test/generate-dcode-config.test.ts
• agents/langchain-deepagents-code/generate-config.ts

---

📝 Walkthrough

Walkthrough

Updates generate-config.ts to preserve colon-containing model ids by stripping only a leading openai: prefix, exports Settings and main, adds a main-module guard, and adds tests for the model-name and config generation behavior.

Changes

Model Config Generation Fix

Layer / File(s)	Summary
Preserve full model id in modelNameForOpenAiProvider agents/langchain-deepagents-code/generate-config.ts	Exports Settings, imports resolve and pathToFileURL, and changes modelNameForOpenAiProvider to trim input and remove only a leading openai: prefix.
Export main and guard execution agents/langchain-deepagents-code/generate-config.ts	Exports main and replaces the unconditional entrypoint call with if (isMainModule()) main(); using an import.meta.url check.
Tests for model name preservation and config output test/generate-dcode-config.test.ts	Adds Vitest coverage for colon-tagged model ids, no-colon ids, whitespace trimming, exact openai: prefix handling, and config.toml output checks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325: Matches the colon-tag truncation fix in modelNameForOpenAiProvider and config.toml generation.

Suggested labels: refactor

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main fix: preserving colon-tagged Ollama model IDs in the generated Deep Agents config.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dcode-model-colon-truncation-6325

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Fix PRA-2: Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through; then add or justify PRA-T1.
Open items: 1 required · 3 warnings · 3 suggestions · 6 test follow-ups
Since last review: 1 prior item resolved · 3 still apply · 2 new items found

Action checklist

• PRA-2 Fix: Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through in test/generate-dcode-config.test.ts:1
• PRA-1 Resolve or justify: Source-of-truth review needed: normalizeInferenceBaseUrl — SSRF host validation
• PRA-3 Resolve or justify: CLI integration test missing for Ollama colon-tagged model — end-to-end path not exercised for [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 scenario in test/langchain-deepagents-code-config.test.ts:1
• PRA-4 Resolve or justify: normalizeInferenceBaseUrl lacks SSRF metadata endpoint blocking — accepted risk, document explicitly in agents/langchain-deepagents-code/generate-config.ts:65
• PRA-T1 Add or justify test follow-up: Mocked behavioral coverage
• PRA-T2 Add or justify test follow-up: Mocked behavioral coverage
• PRA-T3 Add or justify test follow-up: Mocked behavioral coverage
• PRA-T4 Add or justify test follow-up: Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through
• PRA-T5 Add or justify test follow-up: CLI integration test missing for Ollama colon-tagged model — end-to-end path not exercised for [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 scenario
• PRA-T6 Add or justify test follow-up: normalizeInferenceBaseUrl — SSRF host validation
• PRA-5 In-scope improvement: Exported testability functions lack @internal JSDoc — signal they're not stable public API in agents/langchain-deepagents-code/generate-config.ts:1
• PRA-6 In-scope improvement: Unit/integration test layer gap — both should cover critical Ollama colon-tag path in test/generate-dcode-config.test.ts:1
• PRA-7 In-scope improvement: Simplification: modelNameForOpenAiProvider shrunk from 6 lines to 1 — safe shrink applied in agents/langchain-deepagents-code/generate-config.ts:95

Findings index

ID	Severity	Category	Location	Required action
PRA-1	Resolve/justify	architecture	—	Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2	Required	tests	test/generate-dcode-config.test.ts:1	Add test case: expect(modelNameForOpenAiProvider('myorg/mymodel:v1.0:prod')).toBe('myorg/mymodel:v1.0:prod') to test/generate-dcode-config.test.ts.
PRA-3	Resolve/justify	tests	test/langchain-deepagents-code-config.test.ts:1	Add integration test case to langchain-deepagents-code-config.test.ts that runs the generator with NEMOCLAW_MODEL=qwen2.5:7b and asserts config.toml contains 'openai:qwen2.5:7b'.
PRA-4	Resolve/justify	security	agents/langchain-deepagents-code/generate-config.ts:65	No code change required. Explicitly document the accepted risk in PR description or add TODO comment for follow-up if URL becomes user-controllable.
PRA-5	Improvement	correctness	agents/langchain-deepagents-code/generate-config.ts:1	Add @internal JSDoc tags to exported functions that are only public for testability (Settings, buildConfig, modelNameForOpenAiProvider, main).
PRA-6	Improvement	correctness	test/generate-dcode-config.test.ts:1	Ensure both unit and integration test layers cover the critical Ollama colon-tag path. Unit tests for pure function logic; integration test for full CLI execution path.
PRA-7	Improvement	correctness	agents/langchain-deepagents-code/generate-config.ts:95	Keep as-is. The shrink is complete and correct.

🚨 Required before merge

Address these before merging unless a maintainer explicitly overrides the advisor with rationale.

PRA-2 Required — Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through

• Location: test/generate-dcode-config.test.ts:1
• Category: tests
• Problem: The isSafeModelId regex ^[A-Za-z0-9._:/-]+$ permits multiple colons in model IDs (e.g., 'myorg/mymodel:v1.0:prod'). Current tests cover single-colon Ollama tags but not multi-colon identifiers from namespaced registries. Previous review (PRA-3, PRA-T1) requested this test.
• Impact: If a multi-colon model ID is used, the current implementation correctly passes it through verbatim (only strips exact 'openai:' prefix), but lack of test means regression could be introduced silently.
• Required action: Add test case: expect(modelNameForOpenAiProvider('myorg/mymodel:v1.0:prod')).toBe('myorg/mymodel:v1.0:prod') to test/generate-dcode-config.test.ts.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Run vitest after adding the test — should pass with current implementation since it only strips exact 'openai:' prefix.
• Missing regression test: Add test for multi-colon model ID (e.g., 'org/model:tag:variant') to verify modelNameForOpenAiProvider passes through verbatim — isSafeModelId regex allows ':' anywhere.
• Done when: The required change is committed and verification passes: Run vitest after adding the test — should pass with current implementation since it only strips exact 'openai:' prefix.
• Evidence: test/generate-dcode-config.test.ts covers single-colon (qwen2.5:7b, llama3.1:8b-instruct-q4_0) but not multi-colon. isSafeModelId at src/lib/validation.ts:255 allows ':' anywhere.

Review findings by urgency: 1 required fix, 3 items to resolve/justify, 3 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: normalizeInferenceBaseUrl — SSRF host validation

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Add test for localhost/RFC1918/metadata rejection if hardening added
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: normalizeInferenceBaseUrl validates structure but not host; Dockerfile confirms build-time ARG.

PRA-3 Resolve/justify — CLI integration test missing for Ollama colon-tagged model — end-to-end path not exercised for #6325 scenario

• Location: test/langchain-deepagents-code-config.test.ts:1
• Category: tests
• Problem: The existing integration test file spawns the CLI with env vars and verifies config.toml end-to-end, but has no test case for the [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 Ollama tag scenario (qwen2.5:7b). Previous review (PRA-4, PRA-T2) requested this.
• Impact: If the main() entry point or isMainModule() guard breaks, the Docker build would fail at image build time. Unit tests only test pure functions; integration test would catch CLI wiring issues.
• Recommended action: Add integration test case to langchain-deepagents-code-config.test.ts that runs the generator with NEMOCLAW_MODEL=qwen2.5:7b and asserts config.toml contains 'openai:qwen2.5:7b'.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run the existing integration test suite — it spawns node --experimental-strip-types generate-config.ts with env vars and reads config.toml.
• Missing regression test: Add integration test: runGenerator({ NEMOCLAW_MODEL: 'qwen2.5:7b' }) and expect config to contain 'default = "openai:qwen2.5:7b"' and 'models = ["qwen2.5:7b"]'.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the existing integration test suite — it spawns node --experimental-strip-types generate-config.ts with env vars and reads config.toml.
• Evidence: test/langchain-deepagents-code-config.test.ts has 4 integration tests but none for Ollama colon tags. Dockerfile line 106 runs the script at build time.

PRA-4 Resolve/justify — normalizeInferenceBaseUrl lacks SSRF metadata endpoint blocking — accepted risk, document explicitly

• Location: agents/langchain-deepagents-code/generate-config.ts:65
• Category: security
• Problem: URL validation rejects credentials, query strings, fragments, non-HTTPS protocols, and line breaks, but does not block localhost (127.0.0.1, ::1), RFC1918 ranges (10/8, 172.16/12, 192.168/16), or cloud metadata endpoint (169.254.169.254). NEMOCLAW_INFERENCE_BASE_URL is a Docker build ARG controlled by the image builder, not end users, so practical risk is low.
• Impact: If NEMOCLAW_INFERENCE_BASE_URL ever becomes user-controllable (e.g., via future runtime override feature), SSRF to metadata endpoints would be possible. Current threat model (build-time, builder-controlled) makes this acceptable but should be documented.
• Recommended action: No code change required. Explicitly document the accepted risk in PR description or add TODO comment for follow-up if URL becomes user-controllable.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check normalizeInferenceBaseUrl — it uses new URL() parser and checks protocol/credentials/query/hash but not host allow/deny lists. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG set by image builder.
• Missing regression test: If SSRF hardening is added later, add test that normalizeInferenceBaseUrl('http://169.254.169.254/latest/meta-data/'\) throws.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check normalizeInferenceBaseUrl — it uses new URL() parser and checks protocol/credentials/query/hash but not host allow/deny lists. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG set by image builder.
• Evidence: normalizeInferenceBaseUrl at lines 65-79 validates protocol, credentials, query, hash, line breaks but not host. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-5 Improvement — Exported testability functions lack @internal JSDoc — signal they're not stable public API

• Location: agents/langchain-deepagents-code/generate-config.ts:1
• Category: correctness
• Problem: Settings, buildConfig, modelNameForOpenAiProvider, main are exported for testability without @internal JSDoc tags. The ES module main guard (isMainModule) correctly prevents auto-execution on import.
• Impact: Public API surface grows slightly. If more exports are added for testing, consumers might depend on internal functions.
• Suggested action: Add @internal JSDoc tags to exported functions that are only public for testability (Settings, buildConfig, modelNameForOpenAiProvider, main).
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check exports at top of file — all four are exported without @internal.
• Missing regression test: No regression test needed; this is a documentation/API signaling improvement.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Lines 1-4 export Settings, buildConfig, modelNameForOpenAiProvider, main. isMainModule() at line 142 gates main().

PRA-6 Improvement — Unit/integration test layer gap — both should cover critical Ollama colon-tag path

• Location: test/generate-dcode-config.test.ts:1
• Category: correctness
• Problem: New unit test file duplicates some integration test coverage but misses the multi-colon case. The unit tests are valuable for fast feedback on pure functions, but the integration test should also cover the [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 Ollama tag scenario to catch any CLI/main() wiring issues.
• Impact: Risk of divergence between unit and integration test coverage for the critical bug fix path.
• Suggested action: Ensure both unit and integration test layers cover the critical Ollama colon-tag path. Unit tests for pure function logic; integration test for full CLI execution path.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare test/generate-dcode-config.test.ts (unit) with test/langchain-deepagents-code-config.test.ts (integration) — the latter lacks the Ollama tag case.
• Missing regression test: Add the Ollama colon-tag case to the integration test file as described in finding feature: custom settings for using build endpoints #2.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Unit test file has 9 tests including Ollama tags; integration test file has 4 tests but none for Ollama colon tags.

PRA-7 Improvement — Simplification: modelNameForOpenAiProvider shrunk from 6 lines to 1 — safe shrink applied

• Location: agents/langchain-deepagents-code/generate-config.ts:95
• Category: correctness
• Problem: Old implementation used indexOf/slice on first ':' (6 lines). New implementation returns trimmed model verbatim unless it starts with exact 'openai:' prefix (4 lines with JSDOC, 1 line logic). Net -5 lines.
• Impact: Reduces code surface, eliminates silent truncation bug, improves readability.
• Suggested action: Keep as-is. The shrink is complete and correct.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare old vs new function — old split on first ':', new only strips exact 'openai:' prefix.
• Missing regression test: Already covered by existing tests.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: PR diff shows modelNameForOpenAiProvider reduced from 6 lines to 1 line (return model.trim() for non-openai:; slice for openai:). JSDoc cites [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 and explains isSafeModelId allows ':'.

Simplification opportunities: 1 possible cut, net -5 lines possible

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

• PRA-7 shrink (agents/langchain-deepagents-code/generate-config.ts:95): Old modelNameForOpenAiProvider with providerSeparator indexOf/slice logic (6 lines)
  • Replacement: return trimmed.startsWith(PROVIDER_PREFIX) ? trimmed.slice(PROVIDER_PREFIX.length) : trimmed; (1 line logic)
  • Net: -5 lines
  • Safety boundary: Must preserve trim() for whitespace handling; must not reintroduce any splitting on ':'

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Mocked behavioral coverage — Add unit test: modelNameForOpenAiProvider preserves multi-colon model IDs (org/model:tag:variant). Changed code has I/O, filesystem, and process boundaries (main() writes config.toml to ~/.deepagents). Unit tests cover pure functions; integration tests cover CLI execution path. Both layers should cover the critical Ollama colon-tag fix.
• PRA-T2 Mocked behavioral coverage — Add integration test: CLI with NEMOCLAW_MODEL=qwen2.5:7b produces config.toml with openai:qwen2.5:7b. Changed code has I/O, filesystem, and process boundaries (main() writes config.toml to ~/.deepagents). Unit tests cover pure functions; integration tests cover CLI execution path. Both layers should cover the critical Ollama colon-tag fix.
• PRA-T3 Mocked behavioral coverage — Future: Add test for normalizeInferenceBaseUrl SSRF host rejection if hardening implemented. Changed code has I/O, filesystem, and process boundaries (main() writes config.toml to ~/.deepagents). Unit tests cover pure functions; integration tests cover CLI execution path. Both layers should cover the critical Ollama colon-tag fix.
• PRA-T4 Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through — Add test case: expect(modelNameForOpenAiProvider('myorg/mymodel:v1.0:prod')).toBe('myorg/mymodel:v1.0:prod') to test/generate-dcode-config.test.ts.
• PRA-T5 CLI integration test missing for Ollama colon-tagged model — end-to-end path not exercised for [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 scenario — Add integration test case to langchain-deepagents-code-config.test.ts that runs the generator with NEMOCLAW_MODEL=qwen2.5:7b and asserts config.toml contains 'openai:qwen2.5:7b'.
• PRA-T6 normalizeInferenceBaseUrl — SSRF host validation — Add test for localhost/RFC1918/metadata rejection if hardening added. normalizeInferenceBaseUrl validates structure but not host; Dockerfile confirms build-time ARG.

Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: normalizeInferenceBaseUrl — SSRF host validation

• Location: not file-specific
• Category: architecture
• Problem: The advisor marked localized patch analysis as needs_followup.
• Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
• Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Missing regression test: Add test for localhost/RFC1918/metadata rejection if hardening added
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
• Evidence: normalizeInferenceBaseUrl validates structure but not host; Dockerfile confirms build-time ARG.

PRA-2 Required — Multi-colon model ID test missing — isSafeModelId allows org/model:tag:variant but no test confirms verbatim pass-through

• Location: test/generate-dcode-config.test.ts:1
• Category: tests
• Problem: The isSafeModelId regex ^[A-Za-z0-9._:/-]+$ permits multiple colons in model IDs (e.g., 'myorg/mymodel:v1.0:prod'). Current tests cover single-colon Ollama tags but not multi-colon identifiers from namespaced registries. Previous review (PRA-3, PRA-T1) requested this test.
• Impact: If a multi-colon model ID is used, the current implementation correctly passes it through verbatim (only strips exact 'openai:' prefix), but lack of test means regression could be introduced silently.
• Required action: Add test case: expect(modelNameForOpenAiProvider('myorg/mymodel:v1.0:prod')).toBe('myorg/mymodel:v1.0:prod') to test/generate-dcode-config.test.ts.
• Expected follow-up: Fix before merge or get explicit maintainer override.
• Verification: Run vitest after adding the test — should pass with current implementation since it only strips exact 'openai:' prefix.
• Missing regression test: Add test for multi-colon model ID (e.g., 'org/model:tag:variant') to verify modelNameForOpenAiProvider passes through verbatim — isSafeModelId regex allows ':' anywhere.
• Done when: The required change is committed and verification passes: Run vitest after adding the test — should pass with current implementation since it only strips exact 'openai:' prefix.
• Evidence: test/generate-dcode-config.test.ts covers single-colon (qwen2.5:7b, llama3.1:8b-instruct-q4_0) but not multi-colon. isSafeModelId at src/lib/validation.ts:255 allows ':' anywhere.

PRA-3 Resolve/justify — CLI integration test missing for Ollama colon-tagged model — end-to-end path not exercised for #6325 scenario

• Location: test/langchain-deepagents-code-config.test.ts:1
• Category: tests
• Problem: The existing integration test file spawns the CLI with env vars and verifies config.toml end-to-end, but has no test case for the [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 Ollama tag scenario (qwen2.5:7b). Previous review (PRA-4, PRA-T2) requested this.
• Impact: If the main() entry point or isMainModule() guard breaks, the Docker build would fail at image build time. Unit tests only test pure functions; integration test would catch CLI wiring issues.
• Recommended action: Add integration test case to langchain-deepagents-code-config.test.ts that runs the generator with NEMOCLAW_MODEL=qwen2.5:7b and asserts config.toml contains 'openai:qwen2.5:7b'.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Run the existing integration test suite — it spawns node --experimental-strip-types generate-config.ts with env vars and reads config.toml.
• Missing regression test: Add integration test: runGenerator({ NEMOCLAW_MODEL: 'qwen2.5:7b' }) and expect config to contain 'default = "openai:qwen2.5:7b"' and 'models = ["qwen2.5:7b"]'.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Run the existing integration test suite — it spawns node --experimental-strip-types generate-config.ts with env vars and reads config.toml.
• Evidence: test/langchain-deepagents-code-config.test.ts has 4 integration tests but none for Ollama colon tags. Dockerfile line 106 runs the script at build time.

PRA-4 Resolve/justify — normalizeInferenceBaseUrl lacks SSRF metadata endpoint blocking — accepted risk, document explicitly

• Location: agents/langchain-deepagents-code/generate-config.ts:65
• Category: security
• Problem: URL validation rejects credentials, query strings, fragments, non-HTTPS protocols, and line breaks, but does not block localhost (127.0.0.1, ::1), RFC1918 ranges (10/8, 172.16/12, 192.168/16), or cloud metadata endpoint (169.254.169.254). NEMOCLAW_INFERENCE_BASE_URL is a Docker build ARG controlled by the image builder, not end users, so practical risk is low.
• Impact: If NEMOCLAW_INFERENCE_BASE_URL ever becomes user-controllable (e.g., via future runtime override feature), SSRF to metadata endpoints would be possible. Current threat model (build-time, builder-controlled) makes this acceptable but should be documented.
• Recommended action: No code change required. Explicitly document the accepted risk in PR description or add TODO comment for follow-up if URL becomes user-controllable.
• Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
• Verification: Check normalizeInferenceBaseUrl — it uses new URL() parser and checks protocol/credentials/query/hash but not host allow/deny lists. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG set by image builder.
• Missing regression test: If SSRF hardening is added later, add test that normalizeInferenceBaseUrl('http://169.254.169.254/latest/meta-data/'\) throws.
• Done when: The risk is fixed or explicitly justified in the PR. Verification: Check normalizeInferenceBaseUrl — it uses new URL() parser and checks protocol/credentials/query/hash but not host allow/deny lists. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG set by image builder.
• Evidence: normalizeInferenceBaseUrl at lines 65-79 validates protocol, credentials, query, hash, line breaks but not host. Dockerfile shows NEMOCLAW_INFERENCE_BASE_URL is a build ARG.

PRA-5 Improvement — Exported testability functions lack @internal JSDoc — signal they're not stable public API

• Location: agents/langchain-deepagents-code/generate-config.ts:1
• Category: correctness
• Problem: Settings, buildConfig, modelNameForOpenAiProvider, main are exported for testability without @internal JSDoc tags. The ES module main guard (isMainModule) correctly prevents auto-execution on import.
• Impact: Public API surface grows slightly. If more exports are added for testing, consumers might depend on internal functions.
• Suggested action: Add @internal JSDoc tags to exported functions that are only public for testability (Settings, buildConfig, modelNameForOpenAiProvider, main).
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Check exports at top of file — all four are exported without @internal.
• Missing regression test: No regression test needed; this is a documentation/API signaling improvement.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Lines 1-4 export Settings, buildConfig, modelNameForOpenAiProvider, main. isMainModule() at line 142 gates main().

PRA-6 Improvement — Unit/integration test layer gap — both should cover critical Ollama colon-tag path

• Location: test/generate-dcode-config.test.ts:1
• Category: correctness
• Problem: New unit test file duplicates some integration test coverage but misses the multi-colon case. The unit tests are valuable for fast feedback on pure functions, but the integration test should also cover the [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 Ollama tag scenario to catch any CLI/main() wiring issues.
• Impact: Risk of divergence between unit and integration test coverage for the critical bug fix path.
• Suggested action: Ensure both unit and integration test layers cover the critical Ollama colon-tag path. Unit tests for pure function logic; integration test for full CLI execution path.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare test/generate-dcode-config.test.ts (unit) with test/langchain-deepagents-code-config.test.ts (integration) — the latter lacks the Ollama tag case.
• Missing regression test: Add the Ollama colon-tag case to the integration test file as described in finding feature: custom settings for using build endpoints #2.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: Unit test file has 9 tests including Ollama tags; integration test file has 4 tests but none for Ollama colon tags.

PRA-7 Improvement — Simplification: modelNameForOpenAiProvider shrunk from 6 lines to 1 — safe shrink applied

• Location: agents/langchain-deepagents-code/generate-config.ts:95
• Category: correctness
• Problem: Old implementation used indexOf/slice on first ':' (6 lines). New implementation returns trimmed model verbatim unless it starts with exact 'openai:' prefix (4 lines with JSDOC, 1 line logic). Net -5 lines.
• Impact: Reduces code surface, eliminates silent truncation bug, improves readability.
• Suggested action: Keep as-is. The shrink is complete and correct.
• Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
• Verification: Compare old vs new function — old split on first ':', new only strips exact 'openai:' prefix.
• Missing regression test: Already covered by existing tests.
• Done when: The local improvement is applied, or the PR notes why it should be deferred.
• Evidence: PR diff shows modelNameForOpenAiProvider reduced from 6 lines to 1 line (return model.trim() for non-openai:; slice for openai:). JSDoc cites [All Platforms][Inference] Deep Agents (dcode) onboard on Ollama truncates a colon model tag in config.toml — qwen2.5:7b becomes "7b" (name lost) #6325 and explains isSafeModelId allows ':'.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[github-actions]

E2E Advisor Recommendation

Required E2E: ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E: inference-routing

Dispatch hint: targets=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

• ubuntu-repo-cloud-langchain-deepagents-code (high): Run the existing Deep Agents Code live target because this PR changes the generator for /sandbox/.deepagents/config.toml. This target onboards langchain-deepagents-code, validates the sandbox runtime marker, and runs the Deep Agents cloud-experimental checks including headless dcode inference, secret-boundary, and TUI startup checks.

Optional E2E

• inference-routing (medium): Optional adjacent confidence for inference.local routing behavior. It does not specifically exercise the Deep Agents Code config generator or Ollama colon-tagged model ids, so it is not a substitute for the Deep Agents Code live target.

New E2E recommendations

• Deep Agents Code with Ollama colon-tagged models (high): The exact regression is that NEMOCLAW_MODEL values like qwen2.5:7b were truncated when config.toml was generated. Existing live Deep Agents Code coverage appears to use hosted NVIDIA inference and may not exercise an Ollama-style colon-tagged model id end-to-end.
  • Suggested test: Add a live or hermetic E2E target that onboards langchain-deepagents-code with an Ollama/compatible model id containing a tag separator such as qwen2.5:7b, then asserts /sandbox/.deepagents/config.toml contains default = "openai:qwen2.5:7b" and models = ["qwen2.5:7b"] and that dcode can start or perform a minimal headless inference through the configured route.

Dispatch hint

• Workflow: .github/workflows/e2e.yaml
• jobs input: targets=ubuntu-repo-cloud-langchain-deepagents-code

[github-actions]

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E targets: None

Dispatch required E2E targets:

• gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

• ubuntu-repo-cloud-langchain-deepagents-code: The PR changes the LangChain Deep Agents Code config generator used by the deepagents-code onboarding path. Run the live-supported deepagents-code typed target to verify generated config behavior in the E2E environment.
  • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Optional E2E targets

• None.

Relevant changed files

• agents/langchain-deepagents-code/generate-config.ts

[github-actions]

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

• PRA-T1 Add or justify test follow-up: Acceptance clause

Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

• PRA-T1 Acceptance clause — 2. Onboard a Deep Agents sandbox on the local Ollama provider so it picks that model: `NEMOCLAW_PROVIDER=ollama nemoclaw onboard --agent dcode --name {name} --non-interactive --fresh --yes`. — add test evidence or identify existing coverage. No live Ollama onboard is added, but the changed behavior is in deterministic config generation. Existing adjacent tests spawn the generator script with a temporary HOME, and the new test calls the same `buildConfig` path used by the generator.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

[cv]

Closing as superseded by #6332 (merged as 48e680b and shipped in v0.0.75). That change already replaced the first-colon split with an exact openai: prefix strip and added regression coverage preserving colon-bearing model IDs. Thank you for the fix and tests here.