Skip to content

fix(onboard): refresh compatible route on messaging resume - #5878

Merged
jyaunches merged 1 commit into
mainfrom
fix/rebuild-resume-provider-refresh
Jun 26, 2026
Merged

fix(onboard): refresh compatible route on messaging resume#5878
jyaunches merged 1 commit into
mainfrom
fix/rebuild-resume-provider-refresh

Conversation

@jyaunches

@jyaunches jyaunches commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • refresh the compatible-endpoint inference route during OpenClaw messaging resume/rebuild instead of trusting provider/model-only route metadata
  • reuse the stored gateway credential when the local key is absent, skipping only the unauthenticated host smoke
  • add resume-path regression coverage for selected messaging channels and session-recorded messaging plans

Validation

  • npm run build:cli
  • npm test -- --run src/lib/onboard/machine/handlers/provider-inference.test.ts test/onboard-build-recreate-credential-reuse.test.ts

Nightly context

Addresses channels-add-remove-e2e post-add rebuild failure where the recreated sandbox reached an empty/non-JSON inference.local response and provider details did not report the selected endpoint.

Summary by CodeRabbit

  • Bug Fixes
    • Improved resume behavior when reconnecting a compatible endpoint during messaging setup.
    • Ensures the connection route is refreshed in cases where messaging is active, while preserving the existing shortcut when no messaging channels are selected.
    • Uses hydrated credentials when available to keep resume behavior consistent across environments.
    • Added clearer logging for the refresh flow during resume.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: 08e039ff-1b95-44c5-b828-0a3d5d68a84c

📥 Commits

Reviewing files that changed from the base of the PR and between 5b001aa and e7411bb.

📒 Files selected for processing (2)
  • src/lib/onboard/machine/handlers/provider-inference.test.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts

📝 Walkthrough

Walkthrough

Provider-inference resume handling now refreshes compatible-endpoint routes when OpenClaw messaging is active, and uses the hydrated gateway credential to decide whether host smoke is skipped. Tests cover messaging-selected, messagingPlan-based, shortcut, Hermes, and hydrated-credential resume paths.

Changes

Compatible-endpoint resume refresh

Layer / File(s) Summary
Resume refresh logic
src/lib/onboard/machine/handlers/provider-inference.ts
hydrateCredentialEnv now returns a hydrated credential value, new helpers detect active messaging and compatible-endpoint refresh conditions, and the provider-selection resume path reruns inference setup with skipHostInferenceSmoke derived from hydration.
Resume coverage
src/lib/onboard/machine/handlers/provider-inference.test.ts
Resume tests cover messaging-selected refresh, messagingPlan refresh, the no-messaging shortcut, the Hermes shortcut, and the hydrated-credential host-smoke path.

Sequence Diagram(s)

sequenceDiagram
  participant "handleProviderInferenceState" as handleProviderInferenceState
  participant "deps.hydrateCredentialEnv" as hydrateCredentialEnv
  participant "setupInference" as setupInference
  participant "logger" as logger
  handleProviderInferenceState->>hydrateCredentialEnv: hydrate credentialEnv
  handleProviderInferenceState->>handleProviderInferenceState: check compatible-endpoint + messaging state
  alt refresh required
    handleProviderInferenceState->>setupInference: rerun with forceInferenceSetup and skipHostInferenceSmoke
    handleProviderInferenceState->>logger: log resume refresh message
  else shortcut
    handleProviderInferenceState->>handleProviderInferenceState: keep compatible-endpoint resume skip
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#5829: Also changes handleProviderInferenceState resume behavior in the same provider-inference path.

Poem

🐰 I hopped through resume trails so neat,
With messaging on, the route found feet.
Hydrated creds made smoke checks sway,
And compatible-endpoint kept its way.
Hop hop, the inference path feels spry!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 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 change: refreshing the compatible route during messaging resume.
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/rebuild-resume-provider-refresh

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

@github-code-quality

github-code-quality Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the fix/rebuild-resume-p... 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/rebuild-resume-p... e7411bb +/-
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/rebuild-resume-p... branch is 47%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main fix/rebuild-resume-p... e7411bb +/-
src/lib/state/o...oard-session.ts 91%
src/lib/actions...dbox/rebuild.ts 73%
src/lib/sandbox/config.ts 72%
src/lib/onboard/preflight.ts 62%
src/lib/shields/index.ts 62%
src/lib/actions...licy-channel.ts 60%
src/lib/state/sandbox.ts 56%
src/lib/policy/index.ts 48%
src/lib/onboard...er-gpu-patch.ts 47%
src/lib/onboard.ts 19%

Updated June 26, 2026 18:23 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: onboard-resume-e2e, onboard-repair-e2e, cloud-onboard-e2e, rebuild-openclaw-e2e
Optional E2E: messaging-providers-e2e, cloud-inference-e2e

Dispatch hint: onboard-resume-e2e,onboard-repair-e2e,cloud-onboard-e2e,rebuild-openclaw-e2e

Auto-dispatched E2E: onboard-resume-e2e via nightly-e2e.yaml at e7411bb41101ebb8379292bac9c3a297c1b31327nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e: Required by the onboarding resume compatibility rule: this modifies the onboarding machine provider/inference resume path and the conditions under which inference setup is skipped or rerun.
  • onboard-repair-e2e: Required by the onboarding resume compatibility rule for src/lib/onboard/machine changes. This path also coexists with resume repair handling for local inference and must not regress repair/resume state behavior.
  • cloud-onboard-e2e: The provider inference state handler is in the hosted onboarding path and changes credential hydration/setupInference behavior for compatible endpoints, so full hosted onboarding should be validated.
  • rebuild-openclaw-e2e: The regression being addressed is a resumed/rebuilt OpenClaw sandbox where compatible-endpoint inference.local routing must be refreshed. This job exercises OpenClaw rebuild/resume and verifies sandbox-side inference after rebuild.

Optional E2E

  • messaging-providers-e2e: Adjacent confidence for messaging provider credential placeholders and L7 proxy behavior. Useful because the changed path is specifically enabled by active OpenClaw messaging channels, but it may not cover the resume compatible-endpoint route refresh directly.
  • cloud-inference-e2e: Adjacent hosted inference smoke for compatible endpoint routing and credentials. Useful if time permits, but cloud-onboard-e2e plus onboard resume/repair are the merge-blocking checks for this PR.

New E2E recommendations

  • openclaw-messaging-compatible-endpoint-resume (high): Existing jobs appear adjacent but not exact: add a focused E2E that creates or seeds an OpenClaw sandbox/session with compatible-endpoint plus an active messaging plan, runs onboard --resume after sandbox recreation/rebuild, and verifies inference.local from inside the sandbox uses the refreshed route without requiring a locally hydrated API key.
    • Suggested test: Add a targeted nightly E2E scenario for OpenClaw messaging compatible-endpoint resume route refresh.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: onboard-resume-e2e,onboard-repair-e2e,cloud-onboard-e2e,rebuild-openclaw-e2e

@github-actions

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: onboard-resume-vitest, onboard-repair-vitest
Optional Vitest E2E scenarios: None

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • onboard-resume-vitest: The PR changes src/lib/onboard/machine provider-inference resume behavior, including persisted session handling and forcing compatible-endpoint route refresh during OpenClaw messaging resume. The onboarding resume compatibility rule requires the dedicated onboard resume live Vitest job for these state-machine resume paths.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • onboard-repair-vitest: The same resume-path change can affect persisted-session repair/backstop execution by changing when resumed provider/inference setup is skipped or re-run. Per the onboarding resume compatibility rule, repair is required, not optional, for state-machine resume paths that can affect repair from persisted sessions.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • src/lib/onboard/machine/handlers/provider-inference.ts
  • src/lib/onboard/machine/handlers/provider-inference.test.ts

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: OpenClaw compatible-endpoint messaging resume route refresh.
Open items: 0 required · 2 warnings · 0 suggestions · 7 test follow-ups
Top item: Use canonical active messaging plan semantics before forcing compatible-endpoint refresh

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: OpenClaw compatible-endpoint messaging resume route refresh
  • PRA-2 Resolve or justify: Use canonical active-channel semantics before skipping host smoke in src/lib/onboard/machine/handlers/provider-inference.ts:183
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Acceptance clause
  • PRA-T4 Add or justify test follow-up: Acceptance clause
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Acceptance clause
  • PRA-T7 Add or justify test follow-up: OpenClaw compatible-endpoint messaging resume route refresh

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 Resolve/justify security src/lib/onboard/machine/handlers/provider-inference.ts:183 Replace the manual session-plan scan with the canonical active-channel helper, e.g. derive active channels via `getActiveChannelsFromPlan(session?.messagingPlan)` and test for a non-empty result. If `selectedMessagingChannels` must stay trusted as an already-filtered upstream value, keep that branch but make the session fallback use the shared helper.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 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: OpenClaw compatible-endpoint messaging resume route refresh

  • 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: Unit tests now prove the handler re-runs `setupInference` for selected messaging channels and session-recorded active plans; the missing regression is the disabled-channel session-plan case described in the finding.
  • 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: The added comment describes stale provider/model-only metadata after rebuild recreate, and the code forces `forceInferenceSetup = true` for compatible-endpoint OpenClaw messaging resume.

PRA-2 Resolve/justify — Use canonical active-channel semantics before skipping host smoke

  • Location: src/lib/onboard/machine/handlers/provider-inference.ts:183
  • Category: security
  • Problem: `hasActiveMessagingChannels` treats any non-empty `selectedMessagingChannels` as active and, for session plans, checks only `channel.active === true && channel.disabled !== true`. The existing source of truth in `getActiveChannelIdsFromPlan` also excludes channel IDs listed in the plan-level `disabledChannels` array. Because this predicate now forces compatible-endpoint setup and can set `skipHostInferenceSmoke` when the local credential is absent, a top-level-disabled stale/recorded channel can still alter gateway setup behavior and skip the host direct smoke.
  • Impact: A disabled messaging channel in a persisted session plan can incorrectly trigger compatible-endpoint route refresh and bypass the host inference smoke. That does not appear to leak credentials, but it weakens validation on a credentials/inference/network path and can make disabled-channel policy state diverge from inference setup behavior.
  • Recommended action: Replace the manual session-plan scan with the canonical active-channel helper, e.g. derive active channels via `getActiveChannelsFromPlan(session?.messagingPlan)` and test for a non-empty result. If `selectedMessagingChannels` must stay trusted as an already-filtered upstream value, keep that branch but make the session fallback use the shared helper.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `src/lib/messaging/plan-validation.ts#getActiveChannelIdsFromPlan` and compare it with `src/lib/onboard/machine/handlers/provider-inference.ts#hasActiveMessagingChannels`; the changed helper ignores plan-level `disabledChannels`.
  • Missing regression test: Add a `handleProviderInferenceState` unit test where the session has provider `compatible-endpoint`, route ready, `channels: [{ channelId: 'telegram', active: true, disabled: false }]`, and `disabledChannels: ['telegram']`; assert `setupInference` is not called and the inference resume shortcut is recorded.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `src/lib/messaging/plan-validation.ts#getActiveChannelIdsFromPlan` and compare it with `src/lib/onboard/machine/handlers/provider-inference.ts#hasActiveMessagingChannels`; the changed helper ignores plan-level `disabledChannels`.
  • Evidence: The PR adds `channels.some((channel) => channel.active === true && channel.disabled !== true)` while existing `getActiveChannelIdsFromPlan` filters `channel.active && !channel.disabled && !disabled.has(channel.channelId)`.

💡 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.

  • None.
Simplification opportunities: 1 possible cut, net -2 lines possible

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

  • PRA-2 shrink (src/lib/onboard/machine/handlers/provider-inference.ts:183): The custom `session?.messagingPlan?.channels.some(...)` implementation in `hasActiveMessagingChannels`.
    • Replacement: Use the existing `getActiveChannelsFromPlan(session?.messagingPlan)` source-of-truth helper for the session-plan fallback.
    • Net: -2 lines
    • Safety boundary: Do not remove the provider, OpenClaw-agent, credential-presence, or `skipHostInferenceSmoke` trust-boundary decisions; only replace duplicated active-channel derivation.
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 Runtime validation — Unit: does not refresh compatible-endpoint route when the only session-plan channel is disabled via `disabledChannels`.. The changed unit tests are targeted and useful, but this patch changes runtime gateway/sandbox inference routing and host-smoke suppression. Static tests cannot prove that a recreated messaging sandbox's `inference.local` endpoint is actually refreshed end-to-end.
  • PRA-T2 Runtime validation — Runtime/integration: OpenClaw compatible-endpoint rebuild with one messaging channel refreshes the gateway route and sandbox-side `inference.local` returns valid JSON from the selected endpoint after resume/recreate.. The changed unit tests are targeted and useful, but this patch changes runtime gateway/sandbox inference routing and host-smoke suppression. Static tests cannot prove that a recreated messaging sandbox's `inference.local` endpoint is actually refreshed end-to-end.
  • PRA-T3 Acceptance clause — refresh the compatible-endpoint inference route during OpenClaw messaging resume/rebuild instead of trusting provider/model-only route metadata — add test evidence or identify existing coverage. `shouldRefreshCompatibleEndpointRouteForMessaging(...)` forces `forceInferenceSetup = true`, and `refreshes compatible-endpoint route on OpenClaw messaging resume` asserts `setupInference` runs instead of the inference skip path. The route-refresh decision is partially weakened by the disabled-channel source-of-truth issue reported above.
  • PRA-T4 Acceptance clause — npm run build:cli — add test evidence or identify existing coverage. This is PR-provided validation text only; this review did not execute build commands.
  • PRA-T5 Acceptance clause — npm test -- --run src/lib/onboard/machine/handlers/provider-inference.test.ts test/onboard-build-recreate-credential-reuse.test.ts — add test evidence or identify existing coverage. This is PR-provided validation text only; static inventory confirms the changed unit test file exists and targets the changed handler, but this review did not execute tests.
  • PRA-T6 Acceptance clause — Addresses channels-add-remove-e2e post-add rebuild failure where the recreated sandbox reached an empty/non-JSON inference.local response and provider details did not report the selected endpoint. — add test evidence or identify existing coverage. The diff re-runs compatible-endpoint setup on OpenClaw messaging resume, which addresses the described stale provider/model-only route metadata path. Unit tests verify the handler calls, but the actual sandbox-side `inference.local` rebuild behavior still needs runtime validation.
  • PRA-T7 OpenClaw compatible-endpoint messaging resume route refresh — Unit tests now prove the handler re-runs `setupInference` for selected messaging channels and session-recorded active plans; the missing regression is the disabled-channel session-plan case described in the finding.. The added comment describes stale provider/model-only metadata after rebuild recreate, and the code forces `forceInferenceSetup = true` for compatible-endpoint OpenClaw messaging resume.

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

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 28257142916
Target ref: e7411bb41101ebb8379292bac9c3a297c1b31327
Workflow ref: main
Requested jobs: onboard-resume-e2e
Summary: 1 passed, 0 failed, 0 cancelled, 0 skipped

Job Result
onboard-resume-e2e ✅ success

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after addressing any feedback comments

@jyaunches
jyaunches merged commit 9f643c7 into main Jun 26, 2026
46 checks passed
@jyaunches
jyaunches deleted the fix/rebuild-resume-provider-refresh branch June 26, 2026 19:47
@wscurran wscurran added area: messaging Messaging channels, bridges, manifests, or channel lifecycle area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: messaging Messaging channels, bridges, manifests, or channel lifecycle area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants