fix(rebuild): reuse gateway web-search credential in preflight - #7129
Conversation
The rebuild web-search preflight demanded a host BRAVE_API_KEY / TAVILY_API_KEY even though saveCredential stages web-search keys to the process env only and the OpenShell gateway provider is the durable system of record. A fresh rebuild process therefore failed preflight with 'Brave Search credential is invalid' for a sandbox whose web search works, unless the key was re-exported by hand. Accept the same gateway credential-only provider binding the OpenClaw recreate path already reuses (messaging-prep requiresExactOpenClawProviderBinding), and keep the host-key validation path for staged keys and for agents that never reuse the binding. Signed-off-by: Shawn Xie <shaxie@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRebuild runtime preflight now reuses matching web-search credentials registered on the sandbox gateway when no host key is staged. Otherwise it preserves credential validation and failure handling. Tests add gateway-related mocks and cover reuse, fallback validation, staged-key precedence, and non-OpenClaw behavior. ChangesWeb-search rebuild preflight
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant RebuildPreflight
participant GatewayMetadata
participant CredentialValidation
RebuildPreflight->>GatewayMetadata: read sandbox gateway provider metadata
GatewayMetadata-->>RebuildPreflight: matching binding or no match
RebuildPreflight->>CredentialValidation: validate and stage host credential when needed
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
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/actions/sandbox/rebuild-target-runtime.test.ts`:
- Around line 219-233: Add a regression test alongside the existing preflight
failure test that supplies gateway provider metadata with a present but
non-matching binding, then exercises preflightRebuildTargetRuntime. Assert the
preflight fails closed and verifies the expected credential validation and bail
behavior, covering the mismatched-binding path separately from missing metadata.
🪄 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: a81b7d12-cabb-4252-9c5c-38f94ed3b706
📒 Files selected for processing (2)
src/lib/actions/sandbox/rebuild-target-runtime.test.tssrc/lib/actions/sandbox/rebuild-target-runtime.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 3 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
CodeRabbit review follow-up on PR #7129: the suite covered the missing-metadata path but not a present-but-mismatched gateway binding. Assert the preflight falls through to credential validation and fails closed when the binding's credential keys do not match the recorded provider. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Shawn Xie <shaxie@nvidia.com>
Co-authored-by: Shawn Xie <shaxie@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed exact head 664200b after the current-main refresh. Focused tests, full CI, CodeQL, advisors, and selected E2E checks are green; all review threads are resolved. Security review found no correctness or security blocker.
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical `docs/changelog/2026-07-18.mdx` release-prep entry with the exact `## v0.0.88` heading. The entry summarizes every user-visible change on `main` since v0.0.87 and links each release theme to the focused user documentation. ## Changes - Add one parser-safe dated changelog entry for v0.0.88 covering DGX Station preparation, inference health, multi-gateway sandbox operations and recovery, onboarding policy defaults, and rebuild credential reuse. - Reconcile the changelog against the merged v0.0.88-labeled PRs and the complete `v0.0.87..origin/main` commit range. - Source mapping: - [#7152](#7152) -> `docs/changelog/2026-07-18.mdx`: Document RDMA-aware OpenIB service remediation during DGX Station preparation. - [#7155](#7155) -> `docs/changelog/2026-07-18.mdx`: Document stopped-container preservation and fail-closed restart-policy boundaries. - [#7158](#7158) -> `docs/changelog/2026-07-18.mdx`: Document bounded packaged CDI refresh for the exact AI Developer Tools Station profile. - [#7074](#7074) -> `docs/changelog/2026-07-18.mdx`: Document authenticated upstream model probes and precise route-reachability claims. - [#7007](#7007) -> `docs/changelog/2026-07-18.mdx`: Document the explicit serving-process health gap in `status` and `doctor`. - [#7113](#7113) -> `docs/changelog/2026-07-18.mdx`: Document owning-gateway selection for sandbox-scoped status and exec operations. - [#7092](#7092) -> `docs/changelog/2026-07-18.mdx`: Document idempotent recovery for target-owned active port forwards. - [#7133](#7133) -> `docs/changelog/2026-07-18.mdx`: Document web-search-aware policy preset defaults during onboarding. - [#7129](#7129) -> `docs/changelog/2026-07-18.mdx`: Document gateway-registered web-search credential reuse during rebuild preflight. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, exact release heading, and parser-safe MDX structure. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed successfully with 0 errors and 2 existing Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — not applicable because native changelog entries use the required parser-safe MDX SPDX comment without frontmatter. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved DGX Station preparation workflows. * Enhanced sandbox status and diagnostic reporting for inference health. * Improved state selection and recovery across multiple gateways. * Added safer onboarding defaults for web search policies. * Improved rebuild preflight handling for credential reuse and fail-closed behavior. * **Documentation** * Added release notes for version 0.0.88. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Summary
nemoclaw rebuild --yesfailed preflight withBrave Search credential is invalid. Brave Search requires BRAVE_API_KEY or a saved Brave Search credential in non-interactive mode.for sandboxes whose web search works.saveCredentialstages web-search keys to the process env only — the OpenShell gateway provider is the durable system of record — so a fresh rebuild process holds no host key, and the preflight demanded one it would never use: the OpenClaw recreate path already reuses the gateway-registered credential (messaging-preprequiresExactOpenClawProviderBinding). After this change the preflight accepts that same gateway credential-only provider binding, and rebuild succeeds without re-exportingBRAVE_API_KEY.Related Issue
Fixes #7097
Changes
src/lib/actions/sandbox/rebuild-target-runtime.ts: before demanding a host key,preflightRebuildWebSearchCredentialaccepts a matching gateway credential-only provider binding (<sandbox>-<provider>-search, provider type, recorded credential key) read viareadGatewayProviderMetadata, scoped to the sandbox's resolved gateway. The reuse is gated to the OpenClaw agent (target.agentDefinition === null) — the only recreate path that reuses the binding — and only when no host key is staged; a staged key and non-OpenClaw agents keep the existing validation path, and a missing/mismatched binding still fails closed.src/lib/actions/sandbox/rebuild-target-runtime.test.ts: cover gateway-binding reuse, fail-closed on missing binding, staged-host-key validation, and the non-OpenClaw validation path.Scope note: issue #7097 also reports the balanced tier creating a global
braveprovider profile as a side effect. Theprovider profile importin NemoClaw has toleratedalready existssince #6165, and decoupling the egress preset from the provider profile is a design decision (the profile drives the L7 proxy token rewrite), so it is not changed here; details on the issue.Type of Change
Quality Gates
readGatewayProviderMetadatanever reads or exports credential values), requires the exact recorded name/type/credential-key binding with no config keys, is scoped to the sandbox's resolved gateway, and fails closed for mismatches, staged host keys, and non-OpenClaw agents.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project cli src/lib/actions/sandbox/rebuild-target-runtime.test.ts(7 passed);npm run test:changed(60 passed);npx vitest run --project integration test/rebuild-credential-preflight.test.ts test/rebuild-stale-recovery.test.ts test/rebuild-shields-auto-unlock.test.ts(13 passed);npm run typecheck:cliclean.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Shawn Xie shaxie@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests