refactor(onboard): resolve create intent before mutation

src/lib/onboard/extra-provider-reconciliation.test.ts

@@ -2,7 +2,11 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { afterEach, describe, expect, it, vi } from "vitest";
-import { reconcileRegisteredExtraProviders } from "./extra-provider-reconciliation";
+import {
+  applyExtraProviderReconciliation,
+  planRegisteredExtraProviders,
+  reconcileRegisteredExtraProviders,
+} from "./extra-provider-reconciliation";
 import {
   LIMIT,
   missing,
@@ -73,4 +77,22 @@ describe("reconcileRegisteredExtraProviders", () => {
       }),
     ).toEqual(["healthy-provider", "indeterminate-provider"]);
   });
+
+  it("plans stale-provider cleanup without mutation until apply (#6226)", () => {
+    const removeExtraProvider = vi.fn(() => true);
+    const plan = planRegisteredExtraProviders("nemoclaw", {
+      listExtraProviders: () => ["healthy-provider", "stale-provider"],
+      removeExtraProvider,
+      runOpenshell: (args) => (args.at(-1) === "stale-provider" ? missing("stale-provider") : ok()),
+    });
+
+    expect(plan).toEqual({
+      extraProviders: ["healthy-provider"],
+      staleExtraProviders: ["stale-provider"],
+    });
+    expect(removeExtraProvider).not.toHaveBeenCalled();
+
+    applyExtraProviderReconciliation(plan, { removeExtraProvider });
+    expect(removeExtraProvider).toHaveBeenCalledWith("stale-provider");
+  });
 });

src/lib/onboard/extra-provider-reconciliation.ts

@@ -23,6 +23,11 @@ export type ReconcileExtraProvidersDeps = {
   warn?: (message: string) => void;
 };
 
+export type ExtraProviderReconciliationPlan = {
+  readonly extraProviders: readonly string[];
+  readonly staleExtraProviders: readonly string[];
+};
+
 type IndeterminateProbeReason =
   | "aggregate-time-budget"
   | "ambiguous-diagnostic"
@@ -139,9 +144,10 @@ function probeExtraProvider(context: ProviderProbeContext): ProviderProbeOutcome
  *
  * Each recorded name is checked independently in the selected gateway. Only an
  * exact provider-specific not-found diagnostic omits that name from this sandbox
- * create and prunes it from the local extra-provider registry, so retries and
- * `--fresh` starts no longer inherit the stale attachment. Successful probes and
- * every indeterminate outcome (including throws, timeouts, transport failures,
+ * create plan. Applying the completed plan later prunes it from the local
+ * extra-provider registry, so retries and `--fresh` starts no longer inherit
+ * the stale attachment. Successful probes and every indeterminate outcome
+ * (including throws, timeouts, transport failures,
  * and missing-gateway diagnostics) preserve the recorded name. Probes share an
  * aggregate time budget; any names left after that budget are preserved. Sandbox
  * creation is still the final authority if gateway state changes after a probe.
@@ -151,17 +157,18 @@ function probeExtraProvider(context: ProviderProbeContext): ProviderProbeOutcome
  * Removal condition: delete this defensive prune once OpenShell/NemoClaw gateway
  * reset owns extra-provider lifecycle cleanup before sandbox creation (#6501).
  */
-export function reconcileRegisteredExtraProviders(
+export function planRegisteredExtraProviders(
   gatewayName: string,
   deps: ReconcileExtraProvidersDeps = {},
-): string[] {
+): ExtraProviderReconciliationPlan {
   const recorded = (deps.listExtraProviders ?? defaultListExtraProviders)();
-  if (recorded.length === 0) return recorded;
+  if (recorded.length === 0) {
+    return { extraProviders: [], staleExtraProviders: [] };
+  }
   if (!gatewayName) throw new Error("OpenShell gateway name is required.");
   assertNoOpenShellGatewayEndpointOverride();
 
   const runOpenshell = deps.runOpenshell ?? defaultRunOpenshell;
-  const removeExtraProvider = deps.removeExtraProvider ?? defaultRemoveExtraProvider;
   const nowMs = deps.nowMs ?? monotonicNowMs;
   const warn = deps.warn ?? ((message: string) => console.warn(message));
   const deadlineMs = nowMs() + PROVIDER_RECONCILIATION_BUDGET_MS;
@@ -174,6 +181,7 @@ export function reconcileRegisteredExtraProviders(
   };
 
   const reconciled: string[] = [];
+  const staleExtraProviders: string[] = [];
   for (const name of recorded) {
     const outcome = probeExtraProvider({
       gatewayName,
@@ -186,7 +194,7 @@ export function reconcileRegisteredExtraProviders(
     if (outcome.keep) {
       reconciled.push(name);
     } else {
-      removeExtraProvider(name);
+      staleExtraProviders.push(name);
     }
   }
 
@@ -198,5 +206,23 @@ export function reconcileRegisteredExtraProviders(
     );
   }
 
-  return reconciled;
+  return { extraProviders: reconciled, staleExtraProviders };
+}
+
+export function applyExtraProviderReconciliation(
+  plan: ExtraProviderReconciliationPlan,
+  deps: Pick<ReconcileExtraProvidersDeps, "removeExtraProvider"> = {},
+): void {
+  const removeExtraProvider = deps.removeExtraProvider ?? defaultRemoveExtraProvider;
+  for (const name of plan.staleExtraProviders) removeExtraProvider(name);
+}
+
+export function reconcileRegisteredExtraProviders(
+  gatewayName: string,
+  deps: ReconcileExtraProvidersDeps = {},
+): string[] {
+  // Compatibility wrapper for focused #6501 tests; remove with that defensive prune.
+  const plan = planRegisteredExtraProviders(gatewayName, deps);
+  applyExtraProviderReconciliation(plan, deps);
+  return [...plan.extraProviders];
 }

src/lib/onboard/lifecycle-contracts.md

@@ -57,7 +57,7 @@ The FSM checkpoint is step-granular. It cannot resume inside gateway startup, cr
 ```text
 fresh onboard
   resolve entry options -> save session/machine* -> host preflight
-  -> gateway/provider effects -> sandbox-create preflight
+  -> gateway/provider effects -> resolve complete sandbox-create intent
   -> materialize create plan -> create -> ready -> live validation -> register*
   -> finalize session*
 
@@ -66,16 +66,16 @@ fresh onboard
   -> force base-image resolution -> follow new-onboard or live-recreate flow
 
 ordinary live recreate
-  resolve drift/conflicts -> conditional backup* -> provider cleanup -> ! delete
+  resolve drift/conflicts + complete create intent -> conditional backup* -> provider cleanup -> ! delete
   -> remove registry -> materialize plan -> create -> ready -> restore/validate -> register*
 
 resume drift
   load session* -> reject hint conflicts -> validate replacement credential
-  -> optional early registry removal -> ordinary path conditional backup* -> ! delete
+  -> resolve complete create intent -> optional registry removal -> ordinary path conditional backup* -> ! delete
   -> remove registry -> materialize plan -> create -> ready -> restore/validate -> register*
 
 not-ready repair
-  load session* -> repair event -> ! delete -> remove registry
+  load session* -> resolve complete create intent -> repair event -> ! delete -> remove registry
   -> materialize plan -> create -> ready -> validate -> register*
 
 rebuild / installer upgrade
@@ -96,7 +96,7 @@ runtime mutation
 |---|---|---|---|---|
 | **New interactive or non-interactive onboard** — `onboard()` and `resolveOnboardEntryOptions` | Current flags, environment, and prompts. `MessagingWorkflowPlanner.buildPlan`, `prepareSandboxMessagingPreflight`, resource-profile selection, `resolveSandboxCreateIntent`, and `materializeSandboxCreatePlan` assemble policy, provider, package, resource, host-forward, and runtime-setup contributions. Non-interactive mode replaces prompts with defaults or hard aborts. | Consent/session/lock setup and preflight can persist local state, install OpenShell, or clean stale gateway artifacts before the gateway handler. Gateway reuse/recovery/start is the first provider-routing effect; inference-provider upserts follow. A name with no live sandbox has no sandbox-destructive boundary; an existing target enters the recreate contract below. | Whole-step session plus machine snapshot; registry registration is deferred until readiness and live validation. The session stores credential environment names, redacted endpoint metadata, and legacy-value digests; real values are rebound from the process at apply time. | A non-Docker-GPU readiness failure attempts to delete the failed sandbox; the Docker-GPU patch path preserves it and emits patch-specific recovery diagnostics. Temp build-context cleanup is attempted inline with an exit-handler fallback; cancel rollback applies only to a brand-new sandbox. Coverage: `transition-traces.test.ts` and `sandbox-create-plan.test.ts`. Gap: gateway upserts can outlive a failed/interrupted create. |
 | **`--fresh` onboard** — `resolveOnboardEntryOptions`, `prepareFreshSession`, `createBaseImageResolutionContext` | Current flags/environment/prompts replace resumable intent. `--fresh` disables auto-resume and forces base-image resolution; it does not prove that the selected sandbox name is unused. | The first destructive effect is local: the prior onboard session is cleared before a new session is saved. A matching live sandbox can later reuse or recreate through the normal sandbox decision; `--fresh` does not itself delete it. | The new session and machine snapshot replace the old resume checkpoint. Credential and effect boundaries then match new onboard or live recreate. | The discarded resume checkpoint is not restored on later failure. Covered by `entry-options.test.ts`, `session-bootstrap.test.ts`, and base-image resolution tests. |
-| **Resume, re-onboard, or recreate** — `onboard()`, `prepareOnboardSession`, `decideSandboxResume`, live-sandbox handling in `createSandbox` | For `--resume`, the recorded session is authoritative and conflicting current name/provider/model/image/tool-disclosure hints are rejected. A new re-onboard run takes current flags, environment, and prompts as intent while registry/gateway state provides drift evidence. Recorded `sandboxName` is trusted only after the sandbox step completed. Replacement web-search credentials are checked before `applySandboxResumeDecision`; other create-plan validation can still occur later. | Ordinary live recreation conditionally backs up before provider cleanup, **delete**, image removal, and registry removal. A selected pre-upgrade backup suppresses a new one; an explicit override permits recreation without backup. Some resume-drift decisions remove the registry row earlier. `repair-and-recreate` deletes the not-ready sandbox and row before the normal create path, without the generic backup. Create-plan materialization follows deletion (#6226 gap). | Resume continues the recorded session/machine snapshot; non-resume re-onboard writes a new session first. Backup is fail-closed only when the ordinary path requires a new backup and no bypass is set; it does not protect early repair cleanup. Registration follows readiness, restore, and live validation. Raw credentials remain process/gateway inputs. | Restore failures warn and can still publish the replacement; managed-DCode live-selection failure leaves a running, unregistered sandbox with manual-delete guidance. Cancel rollback is not armed and there is no rebuild-style receipt rollback. Coverage: transition traces and sandbox handler tests. Gaps: #5961/#5783, #6040, early removal/backup asymmetry, and delete-to-register window (#6228). |
+| **Resume, re-onboard, or recreate** — `onboard()`, `prepareOnboardSession`, `decideSandboxResume`, live-sandbox handling in `createSandbox` | For `--resume`, the recorded session is authoritative and conflicting current name/provider/model/image/tool-disclosure hints are rejected. A new re-onboard run takes current flags, environment, and prompts as intent while registry/gateway state provides drift evidence. The machine resolves a complete secret-free create intent, including policy, messaging/provider, GPU, resource, disabled-channel, and agent inputs, before repair/removal or live recreation. | Ordinary live recreation conditionally backs up before provider cleanup, **delete**, image removal, and registry removal. A selected pre-upgrade backup suppresses a new one; an explicit override permits recreation without backup. Resume registry removal and `repair-and-recreate` now follow complete intent validation. Temporary policy/build artifacts remain materialization effects after the delete boundary. | Resume continues the recorded session/machine snapshot; non-resume re-onboard writes a new session first. The selected sandbox name is recorded when the sandbox step starts, while the complete intent stays process-local and is not persisted or emitted. Credential values are rebound and checked before mutation and again immediately before materialization. | Restore failures warn and can still publish the replacement; managed-DCode live-selection failure leaves a running, unregistered sandbox with manual-delete guidance. Cancel rollback is not armed and there is no rebuild-style receipt rollback. Coverage: transition traces, sandbox handler tests, create-intent characterization, and the real create boundary. Gaps: #5961/#5783, #6040, early backup asymmetry, and delete-to-register window (#6228). |
 | **Rebuild or installer-driven upgrade** — `rebuildSandbox` in `rebuild-pipeline.ts`; `upgradeSandboxes` | Registry state is authoritative. A matching session may fill guarded legacy gaps only when its selection agrees; an unrelated/global session is never used. Ambient provider/model selection is quarantined by `isolateAmbientRecreateEnv`, apart from narrowly scoped legacy recovery. Preflight assembles target config, messaging/policy/runtime inputs, recovery inputs, and a retained replacement context. Generic agents use `preflightRebuildImage`; DCode uses its specialized managed-context preflight instead and proves the live route only for normal live rebuild. Resource profile is not part of preflight. | Consent persistence, target-gateway selection/recovery, and target-preflight registry updates can precede disposable image build/probes. Backup is the first durable recovery checkpoint when available. Shields unlock, MCP detach/scrub, and NIM stop are destructive in-place effects before the **sandbox delete** boundary. Prepared context and mutation-edge conditions are rechecked before delete, proving buildability/input identity but not replacement health or atomic swap. | Durable checkpoints are the backup/recovery manifest when one exists and the rewritten recreate session; stale recovery can reach deletion without a manifest, making that session its first new durable checkpoint. Rollback receipts/snapshots are process-local. Credential metadata comes from the target or guarded fallback; raw credentials/providers are checked against current process/gateway state, while prepared installer recovery may reconstruct a missing gateway provider from a validated host credential. | In-process rollback best-effort restores registry/MCP retry metadata, but process death after non-MCP delete can still lose it. The inner onboard selects resource profile after deletion from non-quarantined ambient input. Covered by rebuild, image-preflight, DCode, and messaging tests. Gaps: post-delete resource intent plus health-before-delete/atomic swap (#5801). |
 | **Channel add/remove/start/stop** — `addSandboxChannel`, `removeSandboxChannel`, `sandboxChannelsSetEnabled` in `policy-channel.ts` | Add compiles and merges a manifest-derived channel delta with `MessagingWorkflowPlanner`. Start, stop, and remove transform the registry plan and rehydrate executable render/build/runtime/forward details from current manifests. | Token-backed add can mutate gateway credentials before policy and plan persistence; QR/in-sandbox-auth add skips that credential upsert. Start persists the enabled plan before policy; stop persists the disabled plan before the rebuild prompt. Remove clears QR-backed durable state when applicable, detaches gateway/bridge state, removes policy, then persists the plan. A queued rebuild has a separate delete boundary. | The compact registry messaging plan is authoritative; render/build/runtime/state/health entries and nested host-forward details are rehydrated rather than persisted. Session policy-preset sync is best-effort, and channel mutations do not rewrite `Session.messagingPlan`. Raw tokens stay in process/gateway bindings. | `rollbackChannelAdd`, re-disable after failed start, and fail-closed QR-state cleanup provide partial compensation. Covered by `policy-channel*.test.ts`, `workflow-planner.test.ts`, and channel integration tests. Gaps: channel add has a separate `--force` conflict policy; add/remove effects can precede plan persistence, and persistence failures are not fully rolled back. |
 | **Provider, model, or credential-binding change** — `runInferenceSet` | CLI intent plus registry/session metadata. Target resolution and OpenShell preparation occur before locking. The target is re-resolved in the mutating phase under the sandbox lifecycle and timer-bound shields locks; that phase validates provider/model syntax, selected agent, shields state, and local reachability before the first write. | First mutation is the gateway route, then a minimal registry write, API-family/config resolution, registry refresh, best-effort config/hash sync, matching-session update, and audit. An OpenClaw API-family change can then restart the managed gateway after the shields lock is released but while the outer sandbox lock remains held. No sandbox deletion. | Registry and matching session store logical provider/model/credential-environment metadata. Audit records the action, sandbox, and reason rather than credentials; raw values remain gateway-bound. | Forward-only; no rollback. `rebuild` is the repair path for degraded state. Covered by `inference-set*.test.ts`. Gap: several stores can diverge after a mid-sequence failure. |

src/lib/onboard/machine/core-flow-phases.test.ts

@@ -208,7 +208,36 @@ function createPhases(
       selectResourceProfileForSandbox: vi.fn(async () => null),
       stopStaleDashboardListenersForSandbox: vi.fn(),
       listRegistrySandboxes: () => ({ sandboxes: [] }),
-      reconcileRegisteredExtraProviders: vi.fn(() => []),
+      planRegisteredExtraProviders: vi.fn(() => ({
+        extraProviders: [],
+        staleExtraProviders: [],
+      })),
+      resolveSandboxCreateIntent: vi.fn(
+        async ({ sandboxName, extraProviders, staleExtraProviders }) => ({
+          sandboxName,
+          activeMessagingChannels: [],
+          messagingProviderRequests: [],
+          reusableMessagingProviders: [],
+          extraProviders: [...extraProviders],
+          staleExtraProviders: [...staleExtraProviders],
+          hermesToolGateways: [],
+          policy: {
+            basePolicyPath: "/repo/policy.yaml",
+            activeMessagingChannels: [],
+            options: {
+              directGpu: false,
+              additionalPresets: [],
+              policyTier: null,
+            },
+          },
+          gpuCreateArgs: [],
+          resourceCreateArgs: [],
+          gpuRoutePlan: "none" as const,
+          sandboxGpuLogMessage: null,
+          disabledChannelNames: [],
+          extraPlaceholderKeys: [],
+        }),
+      ),
       createSandbox: vi.fn(async () => "created-sandbox"),
       updateSandboxRegistry: vi.fn(),
       getSandboxAgentRegistryFields: () => ({ agent: "openclaw" }),
@@ -236,8 +265,16 @@ function createPhases(
 describe("core onboard flow phases", () => {
   it("carries provider selection output into sandbox setup", async () => {
     const updateSandboxRegistry = vi.fn();
+    const createSandbox = vi.fn(async () => "created-sandbox");
     const [providerPhase, sandboxPhase] = createPhases({
-      sandboxDeps: { updateSandboxRegistry },
+      sandboxDeps: {
+        createSandbox,
+        planRegisteredExtraProviders: vi.fn(() => ({
+          extraProviders: ["current-provider"],
+          staleExtraProviders: ["stale-provider"],
+        })),
+        updateSandboxRegistry,
+      },
     });
 
     const providerResult = await providerPhase.run(context());
@@ -279,6 +316,12 @@ describe("core onboard flow phases", () => {
         credentialEnv: "NVIDIA_INFERENCE_API_KEY",
       }),
     );
+    expect(createSandbox.mock.calls[0]?.at(-1)).toMatchObject({
+      resolved: {
+        extraProviders: ["current-provider"],
+        staleExtraProviders: ["stale-provider"],
+      },
+    });
   });
 
   it("passes fresh context through to provider setup recovery policy", async () => {