fix(dcode): use OpenRouter provider for OpenRouter routes - #6616
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:
📝 WalkthroughWalkthroughDeep Agents Code now supports managed OpenRouter routing alongside OpenAI, propagates optional upstream endpoint URLs through sandbox creation and Dockerfile patching, and updates runtime labels, profile aliases, dependency metadata, integrity hashes, and tests. ChangesDeep Agents provider support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Settings
participant GenerateConfig
participant ConfigToml
participant DeepAgentsCode
Settings->>GenerateConfig: provide upstream provider, endpoint, and model
GenerateConfig->>ConfigToml: write OpenAI or OpenRouter route
ConfigToml->>DeepAgentsCode: select managed provider and model
DeepAgentsCode->>DeepAgentsCode: apply provider-specific arguments
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
agents/langchain-deepagents-code/generate-config.ts (2)
169-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: provider/model recomputed instead of reusing
buildConfig's values.
managedDeepAgentsProviderFor(settings)andmodelNameForManagedProvider(settings.model)are already computed insidebuildConfig(Lines 111-112) and recomputed again here for the log line. Purely cosmetic/duplication, not a correctness issue since both are pure functions ofsettings.🤖 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 `@agents/langchain-deepagents-code/generate-config.ts` at line 169, In the config write log, avoid recomputing the provider and model values already produced by buildConfig; expose or retain those values from buildConfig and reuse them in the message assembled after writing the file, including the existing managedDeepAgentsProviderFor and modelNameForManagedProvider symbols.
98-149: 📐 Maintainability & Code Quality | 🔵 TrivialProvider-aware config generation looks correct; consider trimming
buildConfigcomplexity.Logic for provider selection, model-prefix stripping, and conditional Ultra/openai-only sections is correct and matches the test expectations in
test/langchain-deepagents-code-config.test.ts(openrouter route omits[models.providers.openai],use_responses_api, andforce_nonempty_content). One thing worth double-checking: the Nemotron Ultraforce_nonempty_contentworkaround (Line 119-127) is gated toprovider === "openai"only, so the same Ultra model routed throughopenroutergets no such override. The inline comment points todependency-review.mdfor the rationale, so this appears deliberate, but worth confirming the openrouter-native request path doesn't hit the same empty-content bug.Separately,
buildConfigmixes provider selection, model normalization, and three levels of conditional TOML-section construction in one function. As per coding guidelines,**/*.{js,ts}: Keep function complexity low in JavaScript and TypeScript code, consider extracting the provider-block builder (Lines 135-149) into a small helper to make disabled/openai-only params more skimmable.
[medium_effort_and_medium_reward]🤖 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 `@agents/langchain-deepagents-code/generate-config.ts` around lines 98 - 149, Extract the provider-specific TOML construction from buildConfig into a focused helper, such as a provider-block builder, while preserving managedDeepAgentsProviderFor, modelNameForManagedProvider, and all existing openai-only parameter conditions and output ordering. Confirm the openrouter route still intentionally omits use_responses_api and force_nonempty_content, and retain that behavior in the refactor.Source: Coding guidelines
agents/langchain-deepagents-code/managed-dcode-runtime.py (1)
1302-1322: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect relabeling logic; managed-provider allow-list is now duplicated across 3 files.
The alias-mapping logic reads correctly: NVIDIA and OpenRouter alias sets are disjoint so branch order doesn't matter, and the function safely falls back to the raw adapter/upstream string for unrecognized inputs (validated by the regex guard). Confirmed against
test/langchain-deepagents-code-provider-label.test.ts:114-116.Note that the
{"openai", "openrouter"}managed-provider set is now defined independently in four places:generate-config.ts'sManagedDeepAgentsProvidertype, this file's_MANAGED_ADAPTER_PROVIDERS, and twice insidepatch-managed-deepagents-code.py's injected patch strings. If a third provider is ever added, all four need updating in lockstep. Given the Python patches are raw template strings, full consolidation may not be worth the effort right now, but worth keeping in mind.🤖 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 `@agents/langchain-deepagents-code/managed-dcode-runtime.py` around lines 1302 - 1322, The managed-provider allow-list {"openai", "openrouter"} is duplicated across multiple files and patch templates, creating a synchronization risk. Consolidate the shared provider definitions where practical, especially between managed_display_provider(), the ManagedDeepAgentsProvider type, and patch-managed-deepagents-code.py’s injected strings; otherwise document the intentional duplication and ensure any future provider additions update every occurrence.
🤖 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 `@agents/langchain-deepagents-code/generate-config.ts`:
- Line 169: In the config write log, avoid recomputing the provider and model
values already produced by buildConfig; expose or retain those values from
buildConfig and reuse them in the message assembled after writing the file,
including the existing managedDeepAgentsProviderFor and
modelNameForManagedProvider symbols.
- Around line 98-149: Extract the provider-specific TOML construction from
buildConfig into a focused helper, such as a provider-block builder, while
preserving managedDeepAgentsProviderFor, modelNameForManagedProvider, and all
existing openai-only parameter conditions and output ordering. Confirm the
openrouter route still intentionally omits use_responses_api and
force_nonempty_content, and retain that behavior in the refactor.
In `@agents/langchain-deepagents-code/managed-dcode-runtime.py`:
- Around line 1302-1322: The managed-provider allow-list {"openai",
"openrouter"} is duplicated across multiple files and patch templates, creating
a synchronization risk. Consolidate the shared provider definitions where
practical, especially between managed_display_provider(), the
ManagedDeepAgentsProvider type, and patch-managed-deepagents-code.py’s injected
strings; otherwise document the intentional duplication and ensure any future
provider additions update every occurrence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f019b7e0-66ee-4cdf-b49f-da6ed3b2b56e
⛔ Files ignored due to path filters (1)
agents/langchain-deepagents-code/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/dependency-review.mdagents/langchain-deepagents-code/generate-config.tsagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pyagents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/__init__.pyagents/langchain-deepagents-code/requirements.inagents/langchain-deepagents-code/validate-nemotron-ultra-profile.pytest/langchain-deepagents-code-config.test.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-nemotron-profile-plugin.test.tstest/langchain-deepagents-code-provider-label.test.ts
230fe3e to
0463b52
Compare
|
The growth-guard red is infrastructure-only after four attempts of run 29097287509: each attempt failed on a different GitHub Contents API HTTP 502 while reading the base or fork file, before producing a policy verdict. Stopping blind reruns; this is not evidence of a source growth violation. The ordinary source/security review is otherwise clean. Trusted advisors are being dispatched now; the remaining product evidence is still the exact Brev/DCode/OpenRouter reproduction from #6549, not another copy of this HTTP-502 check. |
cv
left a comment
There was a problem hiding this comment.
Reviewed exact head 107e9c99c with trusted advisor runs 29107248087 and 29107247973. Both return merge_after_fixes; all nine security categories pass.
The functional blocker is that managed_inference_provider() maps only upstream tokens openrouter / openrouter-api to the native OpenRouter adapter. Issue #6549's accepted reproduction registers an OpenRouter URL as provider compatible-endpoint. Unless that state is canonically converted before config generation, it still falls through to OpenAI request shaping and the reported DCode failure remains.
Please either:
- canonicalize a trusted
compatible-endpointOpenRouter route before generation, or - derive the native adapter from trusted routed endpoint metadata at this boundary,
and add an exact regression using provider=compatible-endpoint with https://openrouter.ai/api/v1. Also add the installed/headless runtime assertion for native OpenRouter + https://inference.local/v1 + placeholder credential and managed-gateway request behavior.
After the fix, run the required exact-head evidence:
cloud-onboardubuntu-repo-cloud-langchain-deepagents-code- an OpenRouter-specific DCode headless turn through
inference.local(new focused target or equivalent captured acceptance)
The persistent growth-guard red is a separate GitHub Contents API HTTP-502 failure across four attempts; it is not a source finding.
## Summary
`.github/workflows/codebase-growth-guardrails.yaml` — batch HEAD-side
blob fetches through GraphQL aliases and add retry-with-backoff on
transient GitHub API failures. The "Require changed test files to stay
within size budget" step is currently the highest-flake required check
in the repo.
## Problem
The size-budget step issues one `REST
/repos/{owner}/{name}/contents/{path}?ref=SHA` call per legacy budget
entry (currently 11 in `ci/test-file-size-budget.json`) plus one per
changed test file. Each `fetch()` has zero retry, and GitHub's contents
endpoint intermittently returns HTTP 502/503. Any single 5xx anywhere in
the sequence fails the entire required check.
Evidence:
- Today, `codebase-growth-guardrails` failed **9/30 runs (30%)** across
all PRs, every failure with the same signature: `Error:
https://api.github.com/repos/<owner>/NemoClaw/contents/<file>?ref=<sha>:
HTTP 502`.
- On PR #6616 the workflow failed **3 consecutive rerun attempts** on
three *different* files (`ci/test-file-size-budget.json`,
`nemoclaw/src/commands/migration-state.test.ts`,
`src/lib/inference/nim.test.ts`) — confirms intermittent per-request 5xx
rather than a hard outage, and confirms reruns don't converge because
each rerun is a fresh coin flip over the same N calls.
- `main` runs of this workflow are essentially always green (last
failure 2026-06-26), because they run against the trusted repo and hit
the endpoint less. Fork PRs are disproportionately affected.
## Change
- Batch every needed HEAD-side blob fetch into one GraphQL request per
`repo/SHA` using aliases (up to 25 blobs per request, larger sets are
chunked). The BASE-side budget file is fetched separately so the script
can parse legacy entries before deciding which HEAD blobs to load.
- Wrap every outbound request (REST and GraphQL) in `withRetry` — 4
attempts with jittered exponential backoff (250 ms base, 4 s cap) that
only retries on 408, 425, 429, 5xx, and network errors.
- Fall back to REST `/contents/` for any individual blob GraphQL returns
as `null` text or `isTruncated: true` (e.g. very large files).
Policy is unchanged: budget monotonicity, legacy line-count enforcement
(`lines > maxLines` and `lines < maxLines`), `defaultMaxLines` guarding,
changed-file line-count checks, and the "removed legacy budget must not
exceed defaultMaxLines" invariant all match the previous implementation.
This is data-only; the workflow stays on `pull_request_target` and still
does not check out PR code.
## API-call and flake math
| | Current | After |
|---|---|---|
| Sequential calls per run | ~13 + T | 3 (1 REST files + 2 GraphQL) |
| Retries on transient 5xx | none | 4 attempts, backoff |
| Run-level failure at p = 0.05 per call | ~51% | ~0.04% |
`T` = number of changed test files matching the size-budget regex.
## Type of Change
- [x] 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
- YAML lints (repo `check yaml` hook passed at commit time).
- Manual dry-run of the new script against PR #6616's file set (13
blobs) succeeds locally with `GH_TOKEN` set; batched query returns all
13 blobs in one round-trip.
- Retry path exercised by pointing `fetch` at a 502-returning mock;
verified backoff order and eventual success.
- `pull_request_target` guarantee preserved: no `actions/checkout`, no
PR-authored code executed, tokens read-only.
Refs the ambient flake surfaced on PR #6616 and every other fork PR
hitting `codebase-growth-guardrails` today.
Signed-off-by: J. Yaunches <jyaunches@nvidia.com>
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Chores**
* Improved CI validation reliability when checking changed test file
size budgets.
* Reduced the number of repository content requests through batched
retrieval.
* Added automatic retries for transient network, rate-limit, and server
errors.
* Added fallback handling for unavailable or truncated content and
explicit detection of binary files.
* Preserved budget monotonicity checks, including legacy budget
comparisons and line-count validation.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Signed-off-by: J. Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
107e9c9 to
b41fa2d
Compare
b41fa2d to
65f60b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard/dockerfile-patch.ts (1)
61-67: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject
upstreamEndpointUrlbefore baking it into the Dockerfile ARG
src/lib/onboard/dockerfile-patch.ts:175-178still writes the raw URL into the staged Dockerfile, whileagents/langchain-deepagents-code/generate-config.tsonly rejects credentials/query/fragment later. Add the same URL-shape check here so unsafe endpoints never land in image history.🤖 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/onboard/dockerfile-patch.ts` around lines 61 - 67, Validate upstreamEndpointUrl in PatchStagedDockerfileOptions before the Dockerfile ARG is generated in the staging logic around the Dockerfile patch function. Reuse or centralize the existing URL-shape validation from generate-config.ts so URLs containing credentials, queries, or fragments are rejected before being written into the staged Dockerfile.
🤖 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 `@agents/langchain-deepagents-code/Dockerfile`:
- Line 82: Update normalizeOptionalEndpointUrl() to reject all control
characters, not just CR/LF, before persisting endpoint values; add a
negative-path test covering a NEMOCLAW_UPSTREAM_ENDPOINT_URL containing another
control byte and verify it is rejected.
---
Outside diff comments:
In `@src/lib/onboard/dockerfile-patch.ts`:
- Around line 61-67: Validate upstreamEndpointUrl in
PatchStagedDockerfileOptions before the Dockerfile ARG is generated in the
staging logic around the Dockerfile patch function. Reuse or centralize the
existing URL-shape validation from generate-config.ts so URLs containing
credentials, queries, or fragments are rejected before being written into the
staged Dockerfile.
🪄 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: 8a68f2a7-f409-405b-9294-83161f96e1da
⛔ Files ignored due to path filters (1)
agents/langchain-deepagents-code/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/dependency-review.mdagents/langchain-deepagents-code/generate-config.tsagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pyagents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/__init__.pyagents/langchain-deepagents-code/requirements.inagents/langchain-deepagents-code/validate-nemotron-ultra-profile.pysrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.test.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.tssrc/lib/onboard/types.tstest/langchain-deepagents-code-config.test.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-nemotron-profile-plugin.test.tstest/langchain-deepagents-code-provider-label.test.ts
✅ Files skipped from review due to trivial changes (4)
- agents/langchain-deepagents-code/validate-nemotron-ultra-profile.py
- test/langchain-deepagents-code-provider-label.test.ts
- agents/langchain-deepagents-code/dependency-review.md
- test/langchain-deepagents-code-image.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- agents/langchain-deepagents-code/requirements.in
- agents/langchain-deepagents-code/managed-dcode-runtime.py
- test/langchain-deepagents-code-direct-module-patch.test.ts
- agents/langchain-deepagents-code/patch-managed-deepagents-code.py
- agents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/init.py
- agents/langchain-deepagents-code/generate-config.ts
- test/langchain-deepagents-code-nemotron-profile-plugin.test.ts
|
Exact-head follow-up at CI now exposes a deterministic source gate: run 29111042242, shard 4, rejects the new |
65f60b2 to
f1103f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/onboard/dockerfile-patch.test.ts (1)
317-334: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover clearing an existing endpoint.
Add a case that starts with a non-empty endpoint ARG and calls
patchStagedDockerfilewithoutupstreamEndpointUrl, then asserts the value is cleared. This protects theoptions.upstreamEndpointUrl ?? ""behavior and prevents stale routing configuration from surviving reused Dockerfiles.🤖 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/onboard/dockerfile-patch.test.ts` around lines 317 - 334, Extend the relevant patchStagedDockerfile test with a Dockerfile containing a non-empty NEMOCLAW_UPSTREAM_ENDPOINT_URL ARG, invoke patchStagedDockerfile without upstreamEndpointUrl, and assert the patched content sets that ARG to an empty value, covering the options.upstreamEndpointUrl ?? "" behavior and preventing stale configuration.
🤖 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 `@src/lib/onboard/dockerfile-patch.test.ts`:
- Around line 300-306: Update the reviewed security-contract allowlist to
include NEMOCLAW_UPSTREAM_ENDPOINT_URL, and add a focused regression test
verifying the deterministic source gate accepts this ARG. Locate the allowlist
and gate tests by their existing symbols rather than changing the Dockerfile
unit test only.
---
Nitpick comments:
In `@src/lib/onboard/dockerfile-patch.test.ts`:
- Around line 317-334: Extend the relevant patchStagedDockerfile test with a
Dockerfile containing a non-empty NEMOCLAW_UPSTREAM_ENDPOINT_URL ARG, invoke
patchStagedDockerfile without upstreamEndpointUrl, and assert the patched
content sets that ARG to an empty value, covering the
options.upstreamEndpointUrl ?? "" behavior and preventing stale configuration.
🪄 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: 6fa5c893-7e80-4d56-8652-a407c36089e1
⛔ Files ignored due to path filters (1)
agents/langchain-deepagents-code/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/dependency-review.mdagents/langchain-deepagents-code/generate-config.tsagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pyagents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/__init__.pyagents/langchain-deepagents-code/requirements.inagents/langchain-deepagents-code/validate-nemotron-ultra-profile.pyscripts/check-dcode-profile-import-gate.shsrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.test.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.tssrc/lib/onboard/types.tstest/langchain-deepagents-code-config.test.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-nemotron-profile-plugin.test.tstest/langchain-deepagents-code-provider-label.test.ts
✅ Files skipped from review due to trivial changes (2)
- agents/langchain-deepagents-code/requirements.in
- agents/langchain-deepagents-code/dependency-review.md
🚧 Files skipped from review as they are similar to previous changes (16)
- test/langchain-deepagents-code-provider-label.test.ts
- src/lib/onboard/types.ts
- agents/langchain-deepagents-code/patch-managed-deepagents-code.py
- agents/langchain-deepagents-code/validate-nemotron-ultra-profile.py
- agents/langchain-deepagents-code/Dockerfile
- agents/langchain-deepagents-code/managed-dcode-runtime.py
- test/langchain-deepagents-code-image.test.ts
- src/lib/onboard.ts
- agents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/init.py
- src/lib/onboard/sandbox-dockerfile-patch-flow.ts
- test/langchain-deepagents-code-direct-module-patch.test.ts
- src/lib/onboard/machine/handlers/sandbox.ts
- test/langchain-deepagents-code-config.test.ts
- src/lib/onboard/dockerfile-patch.ts
- agents/langchain-deepagents-code/generate-config.ts
- test/langchain-deepagents-code-nemotron-profile-plugin.test.ts
| it("writes the user-selected upstream endpoint into NEMOCLAW_UPSTREAM_ENDPOINT_URL", () => { | ||
| const dockerfilePath = dockerfileWith( | ||
| [ | ||
| "ARG NEMOCLAW_MODEL=old", | ||
| "ARG NEMOCLAW_PROVIDER_KEY=old", | ||
| "ARG NEMOCLAW_UPSTREAM_PROVIDER=old", | ||
| "ARG NEMOCLAW_UPSTREAM_ENDPOINT_URL=old", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the security-contract allowlist for the new ARG.
This test introduces NEMOCLAW_UPSTREAM_ENDPOINT_URL, but the current deterministic source gate rejects that ARG because it is missing from the reviewed security-contract allowlist. Add the ARG there and include a focused gate regression; otherwise validation remains blocked even though this unit test passes.
🤖 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/onboard/dockerfile-patch.test.ts` around lines 300 - 306, Update the
reviewed security-contract allowlist to include NEMOCLAW_UPSTREAM_ENDPOINT_URL,
and add a focused regression test verifying the deterministic source gate
accepts this ARG. Locate the allowlist and gate tests by their existing symbols
rather than changing the Dockerfile unit test only.
30a531f to
887b516
Compare
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 `@src/lib/onboard/dockerfile-patch.ts`:
- Around line 59-81: The function normalizeOptionalEndpointUrlArg currently
returns the original text instead of the parsed canonical URL. After validating
the URL and protocol in normalizeOptionalEndpointUrlArg, return url.href so
normalized backslashes and other URL representations are canonicalized before
embedding in the Dockerfile.
🪄 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: 151f299b-8a37-44c0-a63b-014258e20e42
⛔ Files ignored due to path filters (1)
agents/langchain-deepagents-code/requirements.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/dependency-review.mdagents/langchain-deepagents-code/generate-config.tsagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pyagents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/__init__.pyagents/langchain-deepagents-code/requirements.inagents/langchain-deepagents-code/validate-nemotron-ultra-profile.pyscripts/check-dcode-profile-import-gate.shsrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.test.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.tssrc/lib/onboard/types.tstest/langchain-deepagents-code-config.test.tstest/langchain-deepagents-code-direct-module-patch.test.tstest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-nemotron-profile-plugin.test.tstest/langchain-deepagents-code-provider-label.test.ts
✅ Files skipped from review due to trivial changes (4)
- agents/langchain-deepagents-code/requirements.in
- agents/langchain-deepagents-code/validate-nemotron-ultra-profile.py
- test/langchain-deepagents-code-image.test.ts
- agents/langchain-deepagents-code/dependency-review.md
🚧 Files skipped from review as they are similar to previous changes (14)
- src/lib/onboard/types.ts
- test/langchain-deepagents-code-direct-module-patch.test.ts
- src/lib/onboard/sandbox-dockerfile-patch-flow.ts
- test/langchain-deepagents-code-provider-label.test.ts
- scripts/check-dcode-profile-import-gate.sh
- src/lib/onboard/machine/handlers/sandbox.ts
- src/lib/onboard.ts
- agents/langchain-deepagents-code/profile-plugin/src/nemoclaw_deepagents_profile/init.py
- agents/langchain-deepagents-code/Dockerfile
- src/lib/onboard/dockerfile-patch.test.ts
- agents/langchain-deepagents-code/patch-managed-deepagents-code.py
- agents/langchain-deepagents-code/generate-config.ts
- agents/langchain-deepagents-code/managed-dcode-runtime.py
- test/langchain-deepagents-code-nemotron-profile-plugin.test.ts
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
887b516 to
9aef157
Compare
Signed-off-by: Ho Lim <subhoya@gmail.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
cv's functional concern (compatible-endpoint canonicalization) was addressed in follow-up commits; cv explicitly flagged the persistent growth-guard red as a transient GitHub Contents API HTTP 502, not a source finding. Growth-guardrails is now green on head 9aef157.
9aef157 to
ef06c77
Compare
|
Thank you for the fix, @HOYALIM! I really appreciate how quickly you responded. |
## Summary Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80` section to `docs/about/release-notes.mdx` summarizing user-facing changes since v0.0.79, each bullet linking to the relevant deeper page. Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned `v0.0.79..HEAD`, applied the docs skip list (no violations), and confirmed the 8 commits that already shipped in-PR docs are complete. No new pages needed. ## Source summary - #6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block Kit (rich rendering, digest-pinned base image). - #6584 / #6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter runtime attribution adapter (port `11437`, `NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents `openrouter` provider. - #6210 / #6292 -> `docs/about/release-notes.mdx`: host corporate proxy CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`, `NEMOCLAW_CORPORATE_CA_IMPORT`). - #6624 / #6623 / #6656 -> `docs/about/release-notes.mdx`: release-matched base-image selection, surfaced cluster-image build diagnostics, preserved Nemotron profile registration. - #6629 / #6637 -> `docs/about/release-notes.mdx`: bare `connect` default-sandbox behavior and route-probe hardening. - #6634 / #6626 / #6596 / #5569 / #6610 / #6655 -> `docs/about/release-notes.mdx`: onboarding/recovery preservation, stale-gateway-PID fix, installer backup message, vLLM label on managed platforms. - #6578 / #5670 -> `docs/about/release-notes.mdx`: automatic Hermes light terminal skin and non-interactive `npx` MCP server startup. ## Verification `npm run docs`: 0 errors, all internal links resolve (2 pre-existing hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep Agents all regenerate with the v0.0.80 section. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.80. * Documented Hermes upgrades, including Slack Block Kit rendering. * Added details on OpenRouter traffic routing and attribution headers. * Documented improved proxy certificate handling and sandbox reliability. * Highlighted enhanced connection defaults, route-probing safeguards, onboarding recovery, and terminal/MCP startup behavior. * Added references to relevant user-guide documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…IA#6641) ## Summary `.github/workflows/codebase-growth-guardrails.yaml` — batch HEAD-side blob fetches through GraphQL aliases and add retry-with-backoff on transient GitHub API failures. The "Require changed test files to stay within size budget" step is currently the highest-flake required check in the repo. ## Problem The size-budget step issues one `REST /repos/{owner}/{name}/contents/{path}?ref=SHA` call per legacy budget entry (currently 11 in `ci/test-file-size-budget.json`) plus one per changed test file. Each `fetch()` has zero retry, and GitHub's contents endpoint intermittently returns HTTP 502/503. Any single 5xx anywhere in the sequence fails the entire required check. Evidence: - Today, `codebase-growth-guardrails` failed **9/30 runs (30%)** across all PRs, every failure with the same signature: `Error: https://api.github.com/repos/<owner>/NemoClaw/contents/<file>?ref=<sha>: HTTP 502`. - On PR NVIDIA#6616 the workflow failed **3 consecutive rerun attempts** on three *different* files (`ci/test-file-size-budget.json`, `nemoclaw/src/commands/migration-state.test.ts`, `src/lib/inference/nim.test.ts`) — confirms intermittent per-request 5xx rather than a hard outage, and confirms reruns don't converge because each rerun is a fresh coin flip over the same N calls. - `main` runs of this workflow are essentially always green (last failure 2026-06-26), because they run against the trusted repo and hit the endpoint less. Fork PRs are disproportionately affected. ## Change - Batch every needed HEAD-side blob fetch into one GraphQL request per `repo/SHA` using aliases (up to 25 blobs per request, larger sets are chunked). The BASE-side budget file is fetched separately so the script can parse legacy entries before deciding which HEAD blobs to load. - Wrap every outbound request (REST and GraphQL) in `withRetry` — 4 attempts with jittered exponential backoff (250 ms base, 4 s cap) that only retries on 408, 425, 429, 5xx, and network errors. - Fall back to REST `/contents/` for any individual blob GraphQL returns as `null` text or `isTruncated: true` (e.g. very large files). Policy is unchanged: budget monotonicity, legacy line-count enforcement (`lines > maxLines` and `lines < maxLines`), `defaultMaxLines` guarding, changed-file line-count checks, and the "removed legacy budget must not exceed defaultMaxLines" invariant all match the previous implementation. This is data-only; the workflow stays on `pull_request_target` and still does not check out PR code. ## API-call and flake math | | Current | After | |---|---|---| | Sequential calls per run | ~13 + T | 3 (1 REST files + 2 GraphQL) | | Retries on transient 5xx | none | 4 attempts, backoff | | Run-level failure at p = 0.05 per call | ~51% | ~0.04% | `T` = number of changed test files matching the size-budget regex. ## Type of Change - [x] 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 - YAML lints (repo `check yaml` hook passed at commit time). - Manual dry-run of the new script against PR NVIDIA#6616's file set (13 blobs) succeeds locally with `GH_TOKEN` set; batched query returns all 13 blobs in one round-trip. - Retry path exercised by pointing `fetch` at a 502-returning mock; verified backoff order and eventual success. - `pull_request_target` guarantee preserved: no `actions/checkout`, no PR-authored code executed, tokens read-only. Refs the ambient flake surfaced on PR NVIDIA#6616 and every other fork PR hitting `codebase-growth-guardrails` today. Signed-off-by: J. Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved CI validation reliability when checking changed test file size budgets. * Reduced the number of repository content requests through batched retrieval. * Added automatic retries for transient network, rate-limit, and server errors. * Added fallback handling for unavailable or truncated content and explicit detection of binary files. * Preserved budget monotonicity checks, including legacy budget comparisons and line-count validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: J. Yaunches <jyaunches@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
## Summary Route OpenRouter-backed DCode sandboxes through Deep Agents Code's native `openrouter` provider while preserving NemoClaw's managed `https://inference.local/v1` gateway. This also carries the trusted upstream endpoint metadata through the Dockerfile/config path so `compatible-endpoint` OpenRouter routes no longer fall back to OpenAI request shaping. ## Related Issue Fixes NVIDIA#6549. ## Changes - Detect `compatible-endpoint` routes whose trusted endpoint is `https://openrouter.ai/api/v1` and generate a native `openrouter:` Deep Agents config. - Add and validate `NEMOCLAW_UPSTREAM_ENDPOINT_URL` through onboard Dockerfile patching, sandbox creation, image env, and DCode config generation. - Reject unsafe upstream endpoint values before baking them into Dockerfile ARGs or writing config, including credentials, query/fragment values, and control characters. - Update the DCode profile import gate allowlist for the reviewed endpoint ARG and add focused regression coverage. ## Verification - [x] PR description includes the DCO sign-off declaration and all commits are signed off - [x] `npx vitest run src/lib/onboard/machine/handlers/sandbox.test.ts test/langchain-deepagents-code-config.test.ts src/lib/onboard/dockerfile-patch.test.ts test/langchain-deepagents-code-direct-module-patch.test.ts test/langchain-deepagents-code-profile-build-gate.test.ts` - [x] `npm run typecheck` - [x] `npm run check:diff` --- Signed-off-by: Ho Lim <subhoya@gmail.com> --------- Signed-off-by: Ho Lim <subhoya@gmail.com>
## Summary Release-prep documentation for **v0.0.80**. Adds the `## v0.0.80` section to `docs/about/release-notes.mdx` summarizing user-facing changes since v0.0.79, each bullet linking to the relevant deeper page. Produced via `nemoclaw-contributor-update-docs` (pre-tag path): scanned `v0.0.79..HEAD`, applied the docs skip list (no violations), and confirmed the 8 commits that already shipped in-PR docs are complete. No new pages needed. ## Source summary - NVIDIA#6507 -> `docs/about/release-notes.mdx`: Hermes v0.18 + Slack Block Kit (rich rendering, digest-pinned base image). - NVIDIA#6584 / NVIDIA#6616 -> `docs/about/release-notes.mdx`: host-local OpenRouter runtime attribution adapter (port `11437`, `NEMOCLAW_OPENROUTER_RUNTIME_ADAPTER_PORT`) and native Deep Agents `openrouter` provider. - NVIDIA#6210 / NVIDIA#6292 -> `docs/about/release-notes.mdx`: host corporate proxy CA import into sandbox trust (`NEMOCLAW_CORPORATE_CA_BUNDLE`, `NEMOCLAW_CORPORATE_CA_IMPORT`). - NVIDIA#6624 / NVIDIA#6623 / NVIDIA#6656 -> `docs/about/release-notes.mdx`: release-matched base-image selection, surfaced cluster-image build diagnostics, preserved Nemotron profile registration. - NVIDIA#6629 / NVIDIA#6637 -> `docs/about/release-notes.mdx`: bare `connect` default-sandbox behavior and route-probe hardening. - NVIDIA#6634 / NVIDIA#6626 / NVIDIA#6596 / NVIDIA#5569 / NVIDIA#6610 / NVIDIA#6655 -> `docs/about/release-notes.mdx`: onboarding/recovery preservation, stale-gateway-PID fix, installer backup message, vLLM label on managed platforms. - NVIDIA#6578 / NVIDIA#5670 -> `docs/about/release-notes.mdx`: automatic Hermes light terminal skin and non-interactive `npx` MCP server startup. ## Verification `npm run docs`: 0 errors, all internal links resolve (2 pre-existing hidden-page warnings). `_build/` variants for OpenClaw, Hermes, and Deep Agents all regenerate with the v0.0.80 section. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.80. * Documented Hermes upgrades, including Slack Block Kit rendering. * Added details on OpenRouter traffic routing and attribution headers. * Documented improved proxy certificate handling and sandbox reliability. * Highlighted enhanced connection defaults, route-probing safeguards, onboarding recovery, and terminal/MCP startup behavior. * Added references to relevant user-guide documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Route OpenRouter-backed DCode sandboxes through Deep Agents Code's native
openrouterprovider while preserving NemoClaw's managedhttps://inference.local/v1gateway. This also carries the trusted upstream endpoint metadata through the Dockerfile/config path socompatible-endpointOpenRouter routes no longer fall back to OpenAI request shaping.Related Issue
Fixes #6549.
Changes
compatible-endpointroutes whose trusted endpoint ishttps://openrouter.ai/api/v1and generate a nativeopenrouter:Deep Agents config.NEMOCLAW_UPSTREAM_ENDPOINT_URLthrough onboard Dockerfile patching, sandbox creation, image env, and DCode config generation.Verification
npx vitest run src/lib/onboard/machine/handlers/sandbox.test.ts test/langchain-deepagents-code-config.test.ts src/lib/onboard/dockerfile-patch.test.ts test/langchain-deepagents-code-direct-module-patch.test.ts test/langchain-deepagents-code-profile-build-gate.test.tsnpm run typechecknpm run check:diffSigned-off-by: Ho Lim subhoya@gmail.com