fix(credentials): recover from wrapped attachment errors#7232
Conversation
|
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)
📝 WalkthroughWalkthroughThe attached-sandbox diagnostic parser now accepts additional separators in wrapped OpenShell output. Tests cover multiline sandbox extraction and provider deletion recovery that force-detaches the parsed sandbox before retrying deletion. ChangesSandbox cleanup parsing
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7232.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit c4a9b21 in the TypeScript / code-coverage/cliThe overall coverage in commit c4a9b21 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@test/e2e/support/bedrock-runtime-leak-scan.test.ts`:
- Around line 4-8: Add the required `// `@module-tag` e2e/credential-free`
annotation at the top of the `bedrock-runtime-leak-scan.test.ts` suite so
discovery classifies this credential-free E2E test in the appropriate matrix.
🪄 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: b009480a-58a1-4187-833b-2e4e8479612a
📒 Files selected for processing (6)
docs/reference/commands.mdxdocs/security/credential-rotation.mdxtest/e2e/fixtures/bedrock-runtime-leak-scan.tstest/e2e/live/bedrock-runtime-compatible-anthropic.test.tstest/e2e/live/diagnostics.test.tstest/e2e/support/bedrock-runtime-leak-scan.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
sandl99
changed the title
E2E Target Results — ✅ All selected tests passedRun: 29733063311
|
sandl99
added
nightly-e2e
…chment # Conflicts: # test/e2e/live/bedrock-runtime-compatible-anthropic.test.ts
E2E Target Results — ✅ All requested tests passedRun: 29753394331
|
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Maintainer review: exact current-base gates pass (54 checks), all commits are Verified with DCO, and the narrow wrapped-attachment parser change preserves bounded name validation plus the fail-closed allowed-sandbox authorization boundary. Focused regression coverage includes wrapped diagnostics, invalid names, foreign-sandbox rejection, detach/retry behavior, and selected E2E. No unresolved correctness, security, or product-scope issue found.
|
Update: the author refreshed once, and exact head Maintainer edits remain disabled for this branch. Please merge current |
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Approved exact head 6950310 on current main 5b547cd. The patch is unchanged from the reviewed credential-recovery fix, all 52 current checks are green, the selected onboard-repair and onboard-resume E2E targets passed, CodeRabbit and both Advisor lanes are clean, and all commits are GitHub Verified.
senthilr-nv
left a comment
senthilr-nv
left a comment
There was a problem hiding this comment.
Maintainer review complete: product-scope gate passes because this restores existing credential-reset behavior without adding a supported surface. Exact-current-base gate is green, selected E2E passed, CodeRabbit has no blocking findings, DCO is present, and all commits are Verified.
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `## v0.0.90` entry to `docs/changelog/2026-07-20.mdx` before the release tag is planned. The update also corrects the documented custom-image migration window so the compatibility fallback that first ships in v0.0.90 remains available until v0.0.91. ## Changes - Add the v0.0.90 summary and detailed release bullets for managed-image routing, provider-reset recovery, WhatsApp health reporting, and DGX Station guidance. - Keep the newest release first in the shared dated changelog and use root-absolute links to the canonical OpenClaw routes. - Correct `docs/reference/commands.mdx` to state that the legacy image route selector remains supported through v0.0.90 and is removed in v0.0.91. - Release source summary: - [#7264](#7264) -> `docs/resources/prompt-assets/dgx-station.md`, `docs/changelog/2026-07-20.mdx`: Record the versioned Station installer path, Nemotron 3 Ultra 550B default, and explicit DeepSeek override. - [#7261](#7261) -> `docs/get-started/dgx-station-preparation.mdx`, `docs/manage-sandboxes/recover-rebuild-sandboxes.mdx`, `docs/changelog/2026-07-20.mdx`: Include the OpenIB, legacy recovery, and Additional Setup documentation follow-ups. - [#7232](#7232) -> `docs/changelog/2026-07-20.mdx`: Document provider-reset recovery for wrapped OpenShell attachment diagnostics. - [#7189](#7189) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-20.mdx`: Document the managed-image route-selector rename and correct its one-release migration window. - [#7015](#7015) -> `docs/changelog/2026-07-20.mdx`: Document corrected OpenClaw WhatsApp health reporting. - No additional user-facing page update is needed for [#7193](#7193), [#7110](#7110), [#6783](#6783), or [#7263](#7263) because they change contributor governance, internal CI or release automation, or editorial style without changing supported user behavior. - [#7242](#7242) and [#7225](#7225) are already ancestors of and documented in v0.0.89, so this entry does not duplicate them despite their stale v0.0.90 labels. ## 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 - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog heading, SPDX form, version order, and published links. - [ ] 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: ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable; this PR does not change `scripts/prepare-dgx-station-host.sh` or runtime behavior. - Station profile/scenario: Not applicable. - Result: Not applicable. - Supporting evidence: Not applicable. ## Verification - [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 — `npx vitest run test/changelog-docs.test.ts` (6 passed). - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run; this is a focused documentation-only change. - [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) — passed with 0 errors and two unrelated baseline warnings for unauthenticated redirect checks and the existing light-mode contrast ratio. - [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) --- Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Added release notes for v0.0.90 covering inference routing, credential reset behavior, WhatsApp status detection, and DGX Station coding-agent guidance. - Updated custom Dockerfile guidance to document continued support for the legacy provider argument through v0.0.90. - Clarified that legacy declarations must be renamed to `NEMOCLAW_INFERENCE_PROVIDER_ID` before v0.0.91. - Added and refreshed related documentation links. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
4 participants
Summary
Restore automatic credential-reset recovery when OpenShell wraps an attached-provider error across decorated lines. Also align the Bedrock E2E leak scan with provider attachment by allowing the non-secret managed credential name while continuing to reject credential values.
Changes
│ sandbox(es): ...continuation line socredentials resetcan detach the sandbox and retry provider deletion.Type of Change
Quality Gates
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 integration --project e2e-support test/sandbox-provider-cleanup.test.ts test/e2e/support/bedrock-runtime-leak-scan.test.ts(2 files, 31 passed);npm run test:projects:check(exact membership)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: San Dang sdang@nvidia.com
Summary by CodeRabbit