fix(openclaw): bound managed inference compaction - #6484
Conversation
Signed-off-by: Ho Lim <subhoya@gmail.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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a managed-inference safeguard compaction policy in the OpenClaw config generator, wires it into ChangesManaged inference safeguard compaction
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/ollama-local-openclaw-config-propagation.test.ts (1)
192-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest title doesn't describe both asserted cases.
The title "does not enable managed-inference safeguards outside
inference.local" only matches the first assertion (hostname mismatch). The second call (providerKey: "nvidia-prod") is actually testing a provider-key mismatch while the hostname isinference.local— it's not "outside inference.local". Consider splitting into twoit()blocks with precise titles, and adding a local issue reference suffix consistent with the neighboring test.As per coding guidelines: "Write behavior-oriented test titles, and put local issue references in a final
(#1234)suffix."♻️ Suggested split
- it("does not enable managed-inference safeguards outside inference.local", () => { - expect( - buildManagedInferenceSafeguardCompaction( - "inference", - "nvidia-prod", - "https://integrate.api.nvidia.com/v1", - ), - ).toBeUndefined(); - expect( - buildManagedInferenceSafeguardCompaction( - "nvidia-prod", - "nvidia-prod", - "https://inference.local/v1", - ), - ).toBeUndefined(); - }); + it("does not enable managed-inference safeguards outside inference.local (`#4781`)", () => { + expect( + buildManagedInferenceSafeguardCompaction( + "inference", + "nvidia-prod", + "https://integrate.api.nvidia.com/v1", + ), + ).toBeUndefined(); + }); + + it("does not enable managed-inference safeguards for a non-inference provider key (`#4781`)", () => { + expect( + buildManagedInferenceSafeguardCompaction( + "nvidia-prod", + "nvidia-prod", + "https://inference.local/v1", + ), + ).toBeUndefined(); + });🤖 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 `@test/ollama-local-openclaw-config-propagation.test.ts` around lines 192 - 208, The test title in buildManagedInferenceSafeguardCompaction’s coverage is too broad and only describes the hostname-mismatch case, while the second assertion covers a provider-key mismatch with inference.local. Split the current it() into two behavior-focused tests with precise titles that match each assertion, and add the local issue reference as a final (`#1234`) suffix consistent with the surrounding test names.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/ollama-local-openclaw-config-propagation.test.ts`:
- Around line 192-208: The test title in
buildManagedInferenceSafeguardCompaction’s coverage is too broad and only
describes the hostname-mismatch case, while the second assertion covers a
provider-key mismatch with inference.local. Split the current it() into two
behavior-focused tests with precise titles that match each assertion, and add
the local issue reference as a final (`#1234`) suffix consistent with the
surrounding test names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f544dd14-a511-45d2-82af-0590f3d8b5a2
📒 Files selected for processing (2)
scripts/generate-openclaw-config.mtstest/ollama-local-openclaw-config-propagation.test.ts
apurvvkumaria
left a comment
There was a problem hiding this comment.
Requesting changes for acceptance coverage, not for the test-title nit. This PR currently proves only the generated JSON shape. Issue #4781 reports live manual compaction taking more than seven minutes, increasing the displayed token count, and returning no completion signal. The proposed settings do not cap that path to the requested one-to-two-minute budget because timeoutSeconds is omitted, and pinned OpenClaw v2026.5.27 defaults it to 900 seconds. They also omit notifyUser, whose pinned default is false. No behavior-level test or exact-head live evidence shows that the successor transcript rotates, the displayed token count falls, or manual compaction completes within the expected budget. Please reproduce on the pinned OpenClaw version, then add a behavior-level regression or exact-head live evidence for those acceptance criteria. Set and test timeoutSeconds and notifyUser, or document with evidence why the manual path meets the criteria without them. This PR has no release tag, so I will leave the contributor 24 hours from this review, until 2026-07-09 20:42 UTC, before any maintainer takeover.
Co-authored-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer salvage pushed as |
Signed-off-by: Ho Lim <subhoya@gmail.com>
apurvvkumaria
left a comment
There was a problem hiding this comment.
The exact head is now DCO-compliant, conflict-free, and green across all required CI; CodeRabbit has no unresolved threads. The timeout, user notification, and narrowed mitigation scope address the earlier review. Two exact-head advisor runs still recommend merge_after_fixes, however, for one remaining boundary:
- Document the source-of-truth boundary and removal condition near
MANAGED_INFERENCE_SAFEGUARD_COMPACTION: the invalid behavior is in the pinned OpenClaw compaction runtime, NemoClaw is applying a generated-config mitigation, and this override should be removed when the pinned runtime fixes that behavior. - Add targeted runtime/schema evidence that the pinned OpenClaw build accepts the generated
agents.defaults.compactionobject for a managedinference.localroute. The current tests prove only JSON emission. Full proof that/compactreduces tokens can remain in #4781, which should stay open, but this PR needs at least config-acceptance/runtime-override coverage so a future OpenClaw schema change cannot silently invalidate the mitigation.
The security review is otherwise clean: the exact hostname/provider guard is narrow, Local Ollama remains excluded, no secrets or authorization boundary changes are present, and negative route tests pass.
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Addressed at exact head 0106fa1: source/removal boundary is documented, pinned OpenClaw 2026.6.10 runtime config validation/readback passed, all 35 checks are green, and trusted manual advisor run 29000033108 found no required blocker.
cv
left a comment
There was a problem hiding this comment.
Approved at exact head 0106fa1f43.
- All 35 GitHub checks pass; DCO is present and all five commits are Verified.
- The pinned OpenClaw 2026.6.10 production image passed the exact-head
runtime-overrideslive test (1/1), includingopenclaw config validate --jsonand full compaction-object readback. - The source/runtime ownership boundary and removal condition are now documented; #4781 correctly remains open for live
/compactcompletion and token-reduction proof. - Trusted manual fork review run 29000033108: GPT-5.5 recommends
merge_as_iswith zero findings. Nemotron has no required-before-merge finding; I explicitly disposition its residualmerge_after_fixeswarnings as already covered, intentionally deferred to #4781, or incorrect (same-host case normalization/explicit ports should remain valid, and Local Ollama precedence already has coverage). - CodeRabbit has no actionable exact-head finding; security review is clean.
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - #6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - #6271 and #6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - #6465, #6539, #6570, and #6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - #6523, #6551, #6484, #6488, #6324, and #6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - #6559, #6538, #6560, #6568, #6552, #6567, and #6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - #6541, #5415, #6246, #6496, and #6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - #6253, #6572, #6444, #6536, and #5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - #6508, #6527, #5506, #6588, #6446, #6447, #6582, #6296, #6367, #6397, and #6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## 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 exactly 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [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 the DCO sign-off declaration 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: Tests not applicable, release-note prose only. - [ ] 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) - [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) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> 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.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Mitigates NVIDIA#4781. This bounds each managed remote-inference compaction attempt on `inference.local` routes to 120 seconds, enables OpenClaw lifecycle notifications, uses safeguard compaction with an aggressive history budget, preserves one recent turn verbatim, disables quality-regeneration retries, and requests transcript rotation after successful compaction. The existing Local Ollama small-context policy from NVIDIA#5468 is preserved. `ollama-local` continues to use the lowered reserve-token policy and is excluded from this managed remote-inference safeguard. This configuration prevents an unbounded seven-minute wait and improves visibility, but it does not by itself prove successful compaction, transcript rotation, or reduced context tokens. Issue NVIDIA#4781 should remain open until an exact-head live run demonstrates those outcomes. The original implementation is by @HOYALIM. The maintainer follow-up retains that credit through `Co-authored-by: Ho Lim <subhoya@gmail.com>`. ## Validation - `npx vitest run --project integration test/ollama-local-openclaw-config-propagation.test.ts test/generate-openclaw-config.test.ts` (137 tests passed) - `npx @biomejs/biome check scripts/generate-openclaw-config.mts test/ollama-local-openclaw-config-propagation.test.ts` - `npm run test-size:check` - `npm run check:diff` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an OpenClaw safeguard-based context compaction policy for managed inference routes to help preserve conversation quality as context grows. * **Bug Fixes** * Refines compaction selection so safeguards are only applied within the intended managed-inference environment, preventing unintended overrides in other setups. * **Tests** * Updated and expanded coverage to validate managed-route safeguard behavior and to confirm safeguards are disabled when outside the expected managed-inference base URL. * **Documentation** * Added a guide to “OpenClaw Context Compaction,” including behavior, limits, and how compaction is applied. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Ho Lim <subhoya@gmail.com> --------- Signed-off-by: Ho Lim <subhoya@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - NVIDIA#6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - NVIDIA#6271 and NVIDIA#6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - NVIDIA#6465, NVIDIA#6539, NVIDIA#6570, and NVIDIA#6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - NVIDIA#6523, NVIDIA#6551, NVIDIA#6484, NVIDIA#6488, NVIDIA#6324, and NVIDIA#6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - NVIDIA#6559, NVIDIA#6538, NVIDIA#6560, NVIDIA#6568, NVIDIA#6552, NVIDIA#6567, and NVIDIA#6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - NVIDIA#6541, NVIDIA#5415, NVIDIA#6246, NVIDIA#6496, and NVIDIA#6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - NVIDIA#6253, NVIDIA#6572, NVIDIA#6444, NVIDIA#6536, and NVIDIA#5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - NVIDIA#6508, NVIDIA#6527, NVIDIA#5506, NVIDIA#6588, NVIDIA#6446, NVIDIA#6447, NVIDIA#6582, NVIDIA#6296, NVIDIA#6367, NVIDIA#6397, and NVIDIA#6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## 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 exactly 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [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 the DCO sign-off declaration 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: Tests not applicable, release-note prose only. - [ ] 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) - [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) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> 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.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Mitigates #4781.
This bounds each managed remote-inference compaction attempt on
inference.localroutes to 120 seconds, enables OpenClaw lifecycle notifications, uses safeguard compaction with an aggressive history budget, preserves one recent turn verbatim, disables quality-regeneration retries, and requests transcript rotation after successful compaction.The existing Local Ollama small-context policy from #5468 is preserved.
ollama-localcontinues to use the lowered reserve-token policy and is excluded from this managed remote-inference safeguard.This configuration prevents an unbounded seven-minute wait and improves visibility, but it does not by itself prove successful compaction, transcript rotation, or reduced context tokens.
Issue #4781 should remain open until an exact-head live run demonstrates those outcomes.
The original implementation is by @HOYALIM.
The maintainer follow-up retains that credit through
Co-authored-by: Ho Lim <subhoya@gmail.com>.Validation
npx vitest run --project integration test/ollama-local-openclaw-config-propagation.test.ts test/generate-openclaw-config.test.ts(137 tests passed)npx @biomejs/biome check scripts/generate-openclaw-config.mts test/ollama-local-openclaw-config-propagation.test.tsnpm run test-size:checknpm run check:diffSummary by CodeRabbit
Signed-off-by: Ho Lim subhoya@gmail.com