diff --git a/ci/source-architecture-budget.json b/ci/source-architecture-budget.json index 99239226f02..1b39fa40c48 100644 --- a/ci/source-architecture-budget.json +++ b/ci/source-architecture-budget.json @@ -58,7 +58,7 @@ "src/lib/onboard": 310, "src/lib/actions": 19, "src/lib/actions/sandbox": 183, - "src/lib/state": 38, + "src/lib/state": 37, "src/lib/inference": 62, "scripts": 45 } diff --git a/ci/source-shape-test-budget.json b/ci/source-shape-test-budget.json index fd0e5a9da21..9a297e4ceea 100644 --- a/ci/source-shape-test-budget.json +++ b/ci/source-shape-test-budget.json @@ -531,6 +531,11 @@ "test": "keeps migrated provider identities and implementations behind the one bundle composition", "category": "compatibility" }, + { + "file": "test/source-architecture.test.ts", + "test": "keeps removed step mutation APIs out of production source (#7703)", + "category": "compatibility" + }, { "file": "test/runtime-provider-source-shape.test.ts", "test": "keeps the dormant managed-bootstrap protocol driver-neutral", diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index b625740adb2..f8ac383ba3d 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -480,7 +480,7 @@ const { restoreDefaultAfterRecreate, }: typeof import("./onboard/cancel-rollback") = require("./onboard/cancel-rollback"); // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. -const { createProviderInferenceOnboardFlowPhase, createSandboxOnboardFlowPhase, runCoreOnboardFlowSlice }: typeof import("./onboard/machine/core-flow-phases") = require("./onboard/machine/core-flow-phases"); +const { createProviderInferenceOnboardFlowPhase, createSandboxOnboardFlowPhase, prepareCoreOnboardFlowContext, prepareFinalOnboardFlowContext, runCoreOnboardFlowSlice }: typeof import("./onboard/machine/core-flow-phases") = require("./onboard/machine/core-flow-phases"); const { createFinalOnboardFlowPhases, runFinalOnboardFlowSlice, @@ -4250,35 +4250,23 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { recordRepairEvent, }); - const initialContext = initialFlowResult.context; - if (!initialContext.sandboxGpuConfig) { - throw new Error("Preflight did not produce a sandbox GPU configuration."); - } - session = initialFlowResult.session; - const sandboxGpuConfig = initialContext.sandboxGpuConfig; - const { gpuPassthrough } = initialContext; - const gpu = initialContext.gpu ?? null; - // #2753: for an unfinished sandbox, an explicit requested name precedes // the checkpointed name from the interrupted session. - let sandboxName = - recordedSandboxName || requestedSandboxName || checkpointedSandboxName || null; - if (sandboxName && RESERVED_SANDBOX_NAMES.has(sandboxName)) { - console.error( - ` Reserved name in resumed session: '${sandboxName}' is a ${cliDisplayName()} CLI command.`, - ); - console.error(" Start a fresh onboard with --name to choose a different name."); - process.exit(1); - } - const coreFlowContext: InitialOnboardFlowContext = { - ...initialContext, - session, - sandboxName, + const coreFlowContext = prepareCoreOnboardFlowContext({ + initial: initialFlowResult, + recordedSandboxName, + requestedSandboxName, + checkpointedSandboxName, selectedMessagingChannels, - gpu, - sandboxGpuConfig, - gpuPassthrough, - }; + assertSandboxNameAllowed: (sandboxName) => { + if (!RESERVED_SANDBOX_NAMES.has(sandboxName)) return; + console.error( + ` Reserved name in resumed session: '${sandboxName}' is a ${cliDisplayName()} CLI command.`, + ); + console.error(" Start a fresh onboard with --name to choose a different name."); + process.exit(1); + }, + }); // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. const runCoreGatewayOpenshell = setupInferenceFactory.createGatewayScopedOpenshellRunner(runOpenshell, GATEWAY_NAME); // biome-ignore format: keep src/lib/onboard.ts net-neutral for growth guardrail. @@ -4444,38 +4432,8 @@ async function runOnboard(opts: OnboardOptions = {}): Promise { recordRepairEvent, }); setupInferenceFactory.selectGatewayForFollowupOrExit(GATEWAY_NAME, runOpenshell); - const coreContext = coreFlowResult.context; - session = coreContext.session; - sandboxName = coreContext.sandboxName; - if (!sandboxName || !coreContext.model || !coreContext.provider) { - throw new Error("Onboarding state is incomplete after sandbox setup."); - } - const model = coreContext.model; - const provider = coreContext.provider; - const endpointUrl = coreContext.endpointUrl; - const credentialEnv = coreContext.credentialEnv; - const hermesAuthMethod = coreContext.hermesAuthMethod; - const hermesToolGateways = coreContext.hermesToolGateways; - const nimContainer = coreContext.nimContainer; - let webSearchConfig = coreContext.webSearchConfig as WebSearchConfig | null; - const webSearchSupported = coreContext.webSearchSupported; - - const finalFlowContext: InitialOnboardFlowContext = { - ...coreContext, - session, - sandboxName, - model, - provider, - endpointUrl, - credentialEnv, - hermesAuthMethod, - hermesToolGateways, - nimContainer, - webSearchConfig, - selectedMessagingChannels: coreContext.selectedMessagingChannels, - webSearchSupported, - }; - let liveFinalFlowContext = finalFlowContext; + const finalFlowContext = prepareFinalOnboardFlowContext(coreFlowResult); + let liveFinalFlowContext: InitialOnboardFlowContext = finalFlowContext; const finalFlowPhases = createFinalOnboardFlowPhases< InitialOnboardFlowContext, diff --git a/src/lib/onboard/lifecycle-contracts.md b/src/lib/onboard/lifecycle-contracts.md index baa57a14cd5..18de1ed5b4e 100644 --- a/src/lib/onboard/lifecycle-contracts.md +++ b/src/lib/onboard/lifecycle-contracts.md @@ -191,7 +191,7 @@ The schema and sanitation authority is `Session` plus `normalizeSession`/`filter | Field group | Fields | Writer/owner and state meaning | |---|---|---| | Session envelope | `version`, `sessionId`, `mode`, `startedAt`, `updatedAt`, `status`, `resumable` | `createSession`, save/update helpers, and completion/failure paths. Values are always known after creation. | -| Progress and recovery | `lastStepStarted`, `lastCompletedStep`, `failure`, `steps`, `machine`, `sandboxPromptProgress`, `stagedCredentialProviders`, `checkpoint` | Step-mutation helpers and `OnboardRuntime` own whole-step progress; the OpenClaw sandbox handler owns prompt-group completion markers. `stagedCredentialProviders` contains only names registered before sandbox setup so OpenClaw resume can require both durable ownership and an exact live binding. A marker is trusted only when its matching persisted value is present and valid, including an explicit `null` where supported. `checkpoint` is the dedicated versioned resume contract: a secret-free tri-state decision record plus durable sandbox identity, effect-group receipts, and logical web-search and messaging provider bindings, serialized alongside the session under its own `schemaVersion` with fail-closed handling of an unknown future version. The primary inference provider binding remains owned and revalidated by the provider and inference phases instead of entering this checkpoint ledger. | +| Progress and recovery | `lastStepStarted`, `lastCompletedStep`, `failure`, `steps`, `machine`, `sandboxPromptProgress`, `stagedCredentialProviders`, `checkpoint` | Step helpers record step-progress bookkeeping and context updates accepted by `filterSafeUpdates`. `OnboardRuntime` owns machine transitions, terminal state, and machine events. Explicit session recovery and the process-exit failure backstop are separate recovery boundaries. The OpenClaw sandbox handler owns prompt-group completion markers. `stagedCredentialProviders` contains only names registered before sandbox setup so OpenClaw resume can require both durable ownership and an exact live binding. A marker is trusted only when its matching persisted value is present and valid, including an explicit `null` where supported. `checkpoint` is the dedicated versioned resume contract: a secret-free tri-state decision record plus durable sandbox identity, effect-group receipts, and logical web-search and messaging provider bindings, serialized alongside the session under its own `schemaVersion` with fail-closed handling of an unknown future version. The primary inference provider binding remains owned and revalidated by the provider and inference phases instead of entering this checkpoint ledger. | | Target identity | `agent`, `sandboxName`, `metadata.gatewayName`, `metadata.fromDockerfile` | Onboard selection, sandbox handler/registration, and rebuild session preparation. A completed sandbox step or valid `sandboxPromptProgress.sandboxName` marker is the trust gate for a recorded name. | | Inference intent | `provider`, `model`, `endpointUrl`, `credentialEnv`, `preferredInferenceApi`, `compatibleEndpointReasoning`, `nimContainer`, `webSearchConfig` | Provider/inference handlers and `runInferenceSet`. Known credential state is an environment-variable name or presence metadata, never the value. `redactUrl` masks userinfo and fragments, redacts values under sensitive parameter names, and redacts canonical token-shaped values even under benign parameter names. | | Agent and policy intent | `hermesAuthMethod`, `toolDisclosure`, `hermesToolGateways`, `policyPresets` | Agent setup and policy handling. Channel commands update matching-session `policyPresets` only best-effort. Nullable fields conflate unset, declined, and cleared where the CLI makes those distinctions. | @@ -200,7 +200,7 @@ The schema and sanitation authority is `Session` plus `normalizeSession`/`filter | Runtime metadata | `routerPid`, `routerCredentialHash`, `gpuPassthrough` | Router and sandbox setup/recovery. PID is a live-process hint; credential hash is a digest; GPU is a concrete boolean. | | Legacy migration proof | `migratedLegacyValueHashes` | Onboard legacy migration writes SHA-256 digests keyed by environment name; session filtering guarantees string records but does not independently validate digest shape. | -The registry is separately owned by `src/lib/state/registry.ts`; backup/recovery manifests are owned by their rebuild and recreate modules. Step helpers normally use `RECORD_ONLY_STEP_MUTATION_OPTIONS`; `LEGACY_MACHINE_STEP_MUTATION_OPTIONS` is the compatibility path that also moves the machine snapshot. +The registry is separately owned by `src/lib/state/registry.ts`; backup and recovery manifests are owned by their rebuild and recreate modules. Step helpers do not change the machine snapshot or emit machine events. ## Duplicated decision points diff --git a/src/lib/onboard/machine/README.md b/src/lib/onboard/machine/README.md index f728b68fdf7..bd917cf8116 100644 --- a/src/lib/onboard/machine/README.md +++ b/src/lib/onboard/machine/README.md @@ -19,6 +19,8 @@ The target shape is a machine-driven onboarding runner: In that final shape, `src/lib/onboard.ts` should be a thin entrypoint. State handlers should own state-specific prompts, resume validation, repair decisions, and side effects. +`flow-handoff.ts` validates required data and constructs context at the initial-to-core and core-to-final boundaries. The entrypoint supplies process-bound dependencies and reserved-name output. + The strict runner owns exact `init`, `preflight`, `provider_selection`, `inference`, and `sandbox` entry. If the durable state is later than a slice entry, earlier phases run as evented prerequisite repairs. A repair must return a legal, update-free transition chain and must not change the durable entry state. ## State ownership @@ -41,15 +43,12 @@ A state handler may perform many smaller operations, but it should expose only s ## Session steps versus machine state -The persisted onboarding session still tracks step-level progress for resumability. Step recording is older than the FSM and is currently used as a compatibility bridge. - -Long term: - -- `OnboardRuntime` should own machine transitions and machine revision increments. -- Session step helpers should record only step status (`pending`, `in_progress`, `complete`, `failed`, `skipped`). -- State handlers should return explicit results instead of implicitly moving the machine by calling step helpers. +The persisted onboarding session tracks step-level progress for resumability. -Until that migration completes, step helpers may still infer machine snapshots for compatibility with older sessions and tests. +- `OnboardRuntime` owns normal machine transitions, revision increments, terminal state, and machine events. +- Session step helpers record step-progress bookkeeping and context updates accepted by `filterSafeUpdates`. They cannot change the machine snapshot. +- State handlers return explicit results. They do not move the machine through step helpers. +- Explicit session recovery and the process-exit failure backstop are narrow exceptions. ## Handler contract @@ -87,15 +86,15 @@ without a state transition so a later process can resume the same non-terminal s ## Runtime responsibilities -`OnboardRuntime` is the intended authority for: +`OnboardRuntime` is the authority for: -- validating transitions against `transitions.ts`; +- validating result source, target, kind, and graph transitions; - applying safe session context updates; - marking terminal states; - emitting redacted lifecycle, state, repair, resume-conflict, and hook events; - normalizing older sessions before strict execution. -The runtime should reject invalid transitions before they can be persisted. +Step helpers record step-progress bookkeeping and context updates accepted by `filterSafeUpdates`. They cannot change the machine snapshot or emit machine events. Explicit session recovery and the process-exit failure backstop are separate recovery boundaries. They validate their snapshot changes and run before or outside handler execution. ## Event semantics diff --git a/src/lib/onboard/machine/core-flow-phases.test.ts b/src/lib/onboard/machine/core-flow-phases.test.ts index 8a1796f792f..1806359588d 100644 --- a/src/lib/onboard/machine/core-flow-phases.test.ts +++ b/src/lib/onboard/machine/core-flow-phases.test.ts @@ -46,6 +46,7 @@ function context( hermesToolGateways: [], preferredInferenceApi: null, compatibleEndpointReasoning: null, + compatibleEndpointReasoningEffort: null, nimContainer: null, webSearchConfig: null, @@ -137,6 +138,7 @@ function createPhases( hermesToolGateways: ["local"], preferredInferenceApi: "chat", compatibleEndpointReasoning: null, + compatibleEndpointReasoningEffort: null, nimContainer: "nim-test", })), @@ -162,8 +164,10 @@ function createPhases( recordRepairEvent: vi.fn(async () => createSession()), hydrateCredentialEnv: vi.fn(), configureCompatibleEndpointReasoning: vi.fn(async () => "false" as const), - clearCompatibleEndpointReasoning: vi.fn(() => null), + configureCompatibleEndpointReasoningEffort: vi.fn(async () => null), + clearCompatibleEndpointReasoning: vi.fn(() => null), + clearCompatibleEndpointReasoningEffort: vi.fn(() => null), repairLocalInferenceSystemdOverrideOrExit: vi.fn(), isNonInteractive: () => true, @@ -206,11 +210,8 @@ function createPhases( resolvePath: (value) => value, agentSupportsWebSearch: () => true, note: vi.fn(), + cliName: () => "nemoclaw", - retireReplacedSandboxWorkload: vi.fn(() => ({ - status: "skipped" as const, - reason: "replacement-unproven" as const, - })), updateSession: vi.fn((mutator) => mutator(createSession()) ?? createSession()), getStoredMessagingChannelConfig: () => null, hydrateMessagingChannelConfig: (config) => config, @@ -394,6 +395,7 @@ describe("core onboard flow phases", () => { hermesToolGateways: [], preferredInferenceApi: "chat", compatibleEndpointReasoning: null, + compatibleEndpointReasoningEffort: null, nimContainer: null, })); @@ -883,7 +885,7 @@ describe("core onboard flow phases", () => { resume: true, recordRepairEvent: repairRecorder(), }), - ).rejects.toThrow("Unexpected onboarding live flow state before slice entry"); + ).rejects.toThrow("Unexpected onboarding flow state before slice entry"); expect(providerInference.run).not.toHaveBeenCalled(); expect(sandbox.run).not.toHaveBeenCalled(); }); diff --git a/src/lib/onboard/machine/core-flow-phases.ts b/src/lib/onboard/machine/core-flow-phases.ts index 61561f7748c..b511baf97bf 100644 --- a/src/lib/onboard/machine/core-flow-phases.ts +++ b/src/lib/onboard/machine/core-flow-phases.ts @@ -23,7 +23,7 @@ import { type ProviderInferenceStateOptions, } from "./handlers/provider-inference"; import { handleSandboxState, type SandboxStateOptions } from "./handlers/sandbox"; -import { UnexpectedLiveOnboardFlowSliceStateError } from "./live-flow-slice"; +import { UnexpectedOnboardFlowSliceStateError } from "./flow-slice-error"; import { type OnboardPrerequisiteRepairEventRecorder, runOnboardPrerequisiteRepair, @@ -32,6 +32,8 @@ import type { OnboardMachineRunnerResult, OnboardMachineRunnerRuntime } from "./ import { runOnboardSequenceWithRunner, type OnboardSequencePhase } from "./sequence-runner"; import type { OnboardMachineState } from "./types"; +export { prepareCoreOnboardFlowContext, prepareFinalOnboardFlowContext } from "./flow-handoff"; + export interface EndpointProvenanceOptions { endpointSource?: InferenceEndpointSource | null; endpointSourceProvider?: string | null; @@ -67,6 +69,7 @@ export interface SandboxOnboardFlowPhaseOptions< gatewayName: string; authoritativeResumeConfig?: boolean; authoritativePolicyTier?: string | null; + recreateJournalTargetIntentFingerprint?: string | null; resumeAgentChanged: boolean; requestedObservabilityEnabled?: boolean | null; @@ -160,6 +163,7 @@ export function createProviderInferenceOnboardFlowPhase< hermesToolGateways: context.hermesToolGateways, preferredInferenceApi: context.preferredInferenceApi, compatibleEndpointReasoning: context.compatibleEndpointReasoning, + compatibleEndpointReasoningEffort: context.compatibleEndpointReasoningEffort, nimContainer: context.nimContainer, webSearchConfig: context.webSearchConfig, @@ -184,6 +188,7 @@ export function createProviderInferenceOnboardFlowPhase< hermesToolGateways: providerInferenceResult.hermesToolGateways, preferredInferenceApi: providerInferenceResult.preferredInferenceApi, compatibleEndpointReasoning: providerInferenceResult.compatibleEndpointReasoning, + compatibleEndpointReasoningEffort: providerInferenceResult.compatibleEndpointReasoningEffort, nimContainer: providerInferenceResult.nimContainer, @@ -215,6 +220,7 @@ export function createSandboxOnboardFlowPhase< gatewayName: options.gatewayName, authoritativeResumeConfig: options.authoritativeResumeConfig, authoritativePolicyTier: options.authoritativePolicyTier, + recreateJournalTargetIntentFingerprint: options.recreateJournalTargetIntentFingerprint, endpointSource: endpointProvenance.endpointSource, resumeAgentChanged: options.resumeAgentChanged, @@ -281,7 +287,7 @@ export async function runCoreOnboardFlowSlice { expect( harness.events.filter((event) => event.type === "state.entered").map((event) => event.state), ).toEqual(["policies", "finalizing", "post_verify", "complete"]); + expect( + harness.events + .filter((event) => event.type === "state.skipped") + .map((event) => `${event.type}:${event.state}`), + ).toEqual(["state.skipped:agent_setup"]); expect(harness.events.some((event) => event.type.startsWith("state.repair."))).toBe(false); }); @@ -186,7 +191,7 @@ describe("final onboard flow runtime boundary", () => { phases: createPhases(branchState, order), recordRepairEvent, }), - ).rejects.toBeInstanceOf(UnexpectedLiveOnboardFlowSliceStateError); + ).rejects.toBeInstanceOf(UnexpectedOnboardFlowSliceStateError); expect(order).toEqual([]); expect(recordRepairEvent).not.toHaveBeenCalled(); @@ -234,6 +239,11 @@ describe("final onboard flow runtime boundary", () => { model: "nvidia/test", machine: { state: "complete" }, }); + expect( + harness.events + .filter((event) => event.type === "state.skipped") + .map((event) => `${event.type}:${event.state}`), + ).toEqual(["state.skipped:openclaw"]); expect(harness.events.some((event) => event.type.startsWith("state.repair."))).toBe(false); }); diff --git a/src/lib/onboard/machine/final-flow-phases.ts b/src/lib/onboard/machine/final-flow-phases.ts index c175ba5b92d..f17880ce764 100644 --- a/src/lib/onboard/machine/final-flow-phases.ts +++ b/src/lib/onboard/machine/final-flow-phases.ts @@ -18,7 +18,7 @@ import { handlePostVerifyState, } from "./handlers/finalization"; import { handlePoliciesState, type PoliciesStateOptions } from "./handlers/policies"; -import { UnexpectedLiveOnboardFlowSliceStateError } from "./live-flow-slice"; +import { UnexpectedOnboardFlowSliceStateError } from "./flow-slice-error"; import { createPhaseProgressReporter } from "./phase-progress"; import type { OnboardStateResult } from "./result"; import type { OnboardMachineRunnerRuntime, OnboardStateHandlerResult } from "./runner"; @@ -336,7 +336,7 @@ export async function runFinalOnboardFlowSlice { + it("constructs core context from the initial result and requested name", () => { + const initialContext = context(); + const persisted = createSession(); + const assertSandboxNameAllowed = vi.fn(); + + const result = prepareCoreOnboardFlowContext({ + initial: { context: initialContext, session: persisted }, + recordedSandboxName: null, + requestedSandboxName: "requested", + checkpointedSandboxName: "checkpointed", + selectedMessagingChannels: ["slack"], + assertSandboxNameAllowed, + }); + + expect(result).toMatchObject({ + session: persisted, + sandboxName: "requested", + selectedMessagingChannels: ["slack"], + gpu: "nvidia", + sandboxGpuConfig: { mode: "cdi" }, + gpuPassthrough: true, + }); + expect(assertSandboxNameAllowed).toHaveBeenCalledWith("requested"); + }); + + it.each([ + { + source: "recorded", + recordedSandboxName: "recorded", + requestedSandboxName: "requested", + checkpointedSandboxName: "checkpointed", + }, + { + source: "requested", + recordedSandboxName: null, + requestedSandboxName: "requested", + checkpointedSandboxName: "checkpointed", + }, + { + source: "checkpointed", + recordedSandboxName: null, + requestedSandboxName: null, + checkpointedSandboxName: "checkpointed", + }, + ])("selects the $source sandbox name by precedence", ({ + source, + recordedSandboxName, + requestedSandboxName, + checkpointedSandboxName, + }) => { + const assertSandboxNameAllowed = vi.fn(); + + const result = prepareCoreOnboardFlowContext({ + initial: { context: context(), session: createSession() }, + recordedSandboxName, + requestedSandboxName, + checkpointedSandboxName, + selectedMessagingChannels: [], + assertSandboxNameAllowed, + }); + + expect(result.sandboxName).toBe(source); + expect(assertSandboxNameAllowed).toHaveBeenCalledWith(source); + }); + + it("rejects a missing preflight GPU configuration", () => { + const initialContext = { ...context(), sandboxGpuConfig: null }; + const persisted = createSession(); + + expect(() => + prepareCoreOnboardFlowContext({ + initial: { context: initialContext, session: persisted }, + recordedSandboxName: null, + requestedSandboxName: null, + checkpointedSandboxName: null, + selectedMessagingChannels: [], + assertSandboxNameAllowed: vi.fn(), + }), + ).toThrow("Preflight did not produce a sandbox GPU configuration."); + }); + + it.each( + handoffResultCases, + )("preserves a $trace runner result at the initial-to-core handoff (#7706)", ({ + trace, + resume, + fresh, + initialState, + status, + }) => { + const failure = + status === "failed" + ? { + step: "gateway", + message: "gateway failed", + recordedAt: TRACE_TIME, + } + : null; + const persisted = runnerSession(initialState, status, failure); + const endpointUrl = `https://${trace}.example.test`; + + const result = prepareCoreOnboardFlowContext({ + initial: { + context: { + ...context(), + resume, + fresh, + endpointUrl, + }, + session: persisted, + }, + recordedSandboxName: null, + requestedSandboxName: null, + checkpointedSandboxName: null, + selectedMessagingChannels: ["slack"], + assertSandboxNameAllowed: vi.fn(), + }); + + expect(result.session).toBe(persisted); + expect(result).toMatchObject({ + resume, + fresh, + endpointUrl, + selectedMessagingChannels: ["slack"], + }); + expect(result.session).toMatchObject({ + status, + resumable: true, + failure, + machine: { + state: initialState, + revision: 7, + }, + }); + }); + + it("constructs final context after sandbox identity and inference are complete", () => { + const persisted = createSession(); + const coreContext = { + ...context(), + sandboxName: "ready", + model: "model", + provider: "provider", + endpointUrl: "https://inference.example.test", + selectedMessagingChannels: ["slack"], + }; + + const result = prepareFinalOnboardFlowContext({ + context: coreContext, + session: persisted, + }); + + expect(result.session).toBe(persisted); + expect(result).toMatchObject({ + sandboxName: "ready", + model: "model", + provider: "provider", + endpointUrl: "https://inference.example.test", + selectedMessagingChannels: ["slack"], + }); + }); + + it.each( + handoffResultCases, + )("preserves a $trace runner result at the core-to-final handoff (#7706)", ({ + trace, + resume, + fresh, + coreState, + status, + }) => { + const failure = + status === "failed" + ? { + step: "sandbox", + message: "sandbox failed", + recordedAt: TRACE_TIME, + } + : null; + const persisted = runnerSession(coreState, status, failure); + const endpointUrl = `https://${trace}.example.test`; + + const result = prepareFinalOnboardFlowContext({ + context: { + ...context(), + resume, + fresh, + sandboxName: "ready", + model: "model", + provider: "provider", + endpointUrl, + selectedMessagingChannels: ["slack"], + }, + session: persisted, + }); + + expect(result.session).toBe(persisted); + expect(result).toMatchObject({ + resume, + fresh, + sandboxName: "ready", + model: "model", + provider: "provider", + endpointUrl, + selectedMessagingChannels: ["slack"], + }); + expect(result.session).toMatchObject({ + status, + resumable: true, + failure, + machine: { + state: coreState, + revision: 7, + }, + }); + }); + + it.each([ + "sandboxName", + "model", + "provider", + ] as const)("rejects final context when $field is missing", (field) => { + const coreContext = { + ...context(), + sandboxName: "ready", + model: "model", + provider: "provider", + [field]: null, + }; + + expect(() => + prepareFinalOnboardFlowContext({ + context: coreContext, + session: createSession(), + }), + ).toThrow("Onboarding state is incomplete after sandbox setup."); + }); +}); diff --git a/src/lib/onboard/machine/flow-handoff.ts b/src/lib/onboard/machine/flow-handoff.ts new file mode 100644 index 00000000000..acd607cf48b --- /dev/null +++ b/src/lib/onboard/machine/flow-handoff.ts @@ -0,0 +1,60 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { OnboardFlowContext } from "./flow-context"; +import type { OnboardMachineRunnerResult } from "./runner"; + +type InitialHandoffContext = OnboardFlowContext & { + readonly gpu: Gpu | null; + readonly sandboxGpuConfig: SandboxGpuConfig | null; + readonly gpuPassthrough: boolean; +}; + +export function prepareCoreOnboardFlowContext< + Context extends InitialHandoffContext, + Gpu, + SandboxGpuConfig, +>(options: { + initial: OnboardMachineRunnerResult; + recordedSandboxName: string | null; + requestedSandboxName: string | null; + checkpointedSandboxName: string | null; + selectedMessagingChannels: string[]; + assertSandboxNameAllowed(sandboxName: string): void; +}): Context & { sandboxGpuConfig: SandboxGpuConfig } { + const context = options.initial.context; + if (!context.sandboxGpuConfig) { + throw new Error("Preflight did not produce a sandbox GPU configuration."); + } + const sandboxName = + options.recordedSandboxName || + options.requestedSandboxName || + options.checkpointedSandboxName || + null; + if (sandboxName) options.assertSandboxNameAllowed(sandboxName); + return { + ...context, + session: options.initial.session, + sandboxName, + selectedMessagingChannels: options.selectedMessagingChannels, + gpu: context.gpu ?? null, + sandboxGpuConfig: context.sandboxGpuConfig, + gpuPassthrough: context.gpuPassthrough, + }; +} + +export function prepareFinalOnboardFlowContext( + core: OnboardMachineRunnerResult, +): Context & { sandboxName: string; model: string; provider: string } { + const context = core.context; + if (!context.sandboxName || !context.model || !context.provider) { + throw new Error("Onboarding state is incomplete after sandbox setup."); + } + return { + ...context, + session: core.session, + sandboxName: context.sandboxName, + model: context.model, + provider: context.provider, + }; +} diff --git a/src/lib/onboard/machine/flow-phases/agent-policy-finalization.test.ts b/src/lib/onboard/machine/flow-phases/agent-policy-finalization.test.ts index f5ad29e38a6..6a858175469 100644 --- a/src/lib/onboard/machine/flow-phases/agent-policy-finalization.test.ts +++ b/src/lib/onboard/machine/flow-phases/agent-policy-finalization.test.ts @@ -10,7 +10,6 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../../state/onboard-session"; import type { OnboardFlowContext } from "../flow-context"; import { advanceTo, completeOnboardMachine } from "../result"; @@ -77,19 +76,13 @@ function createRuntime(initialSession: Session = createSession()) { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: () => cloneSession(session), markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: stepName, message, recordedAt: "now" }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; return current; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/machine/flow-sequence.test.ts b/src/lib/onboard/machine/flow-sequence.test.ts index 9ff28f3baa4..a0e8d6c763e 100644 --- a/src/lib/onboard/machine/flow-sequence.test.ts +++ b/src/lib/onboard/machine/flow-sequence.test.ts @@ -10,7 +10,6 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../state/onboard-session"; import type { OnboardFlowContext, OnboardFlowPhaseResult } from "./flow-context"; import { onboardFlowPhaseResult } from "./flow-context"; @@ -82,19 +81,13 @@ function createRuntime(initialSession: Session = createSession()) { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: () => cloneSession(session), markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: stepName, message, recordedAt: "now" }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; return current; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/machine/flow-slice-error.ts b/src/lib/onboard/machine/flow-slice-error.ts new file mode 100644 index 00000000000..04b2883796f --- /dev/null +++ b/src/lib/onboard/machine/flow-slice-error.ts @@ -0,0 +1,15 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import type { OnboardMachineState } from "./types"; + +export class UnexpectedOnboardFlowSliceStateError extends Error { + constructor( + readonly state: OnboardMachineState, + readonly runStates: readonly OnboardMachineState[], + readonly repairStates: readonly OnboardMachineState[], + ) { + super(`Unexpected onboarding flow state before slice entry: ${state}`); + this.name = "UnexpectedOnboardFlowSliceStateError"; + } +} diff --git a/src/lib/onboard/machine/flow-slices.test.ts b/src/lib/onboard/machine/flow-slices.test.ts index e7e65909d3d..b4b4cafb55a 100644 --- a/src/lib/onboard/machine/flow-slices.test.ts +++ b/src/lib/onboard/machine/flow-slices.test.ts @@ -45,11 +45,8 @@ function runtime(initialSession: Session = createSession()) { markStepStarted: () => cloneSession(session), markStepComplete: (_stepName, updates: SessionUpdates = {}) => updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), markStepSkipped: () => cloneSession(session), markStepFailed: () => cloneSession(session), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: () => cloneSession(session), filterSafeUpdates, emitEvent: () => undefined, diff --git a/src/lib/onboard/machine/initial-flow-phases.test.ts b/src/lib/onboard/machine/initial-flow-phases.test.ts index 768c0a6ecca..4ae83d80b05 100644 --- a/src/lib/onboard/machine/initial-flow-phases.test.ts +++ b/src/lib/onboard/machine/initial-flow-phases.test.ts @@ -43,6 +43,7 @@ function context(overrides: Partial = {}): Context { hermesToolGateways: [], preferredInferenceApi: null, compatibleEndpointReasoning: null, + compatibleEndpointReasoningEffort: null, nimContainer: null, webSearchConfig: null, @@ -593,7 +594,7 @@ describe("initial onboard flow phases", () => { resume: true, recordRepairEvent: repairRecorder(), }), - ).rejects.toThrow("Unexpected onboarding live flow state before slice entry"); + ).rejects.toThrow("Unexpected onboarding flow state before slice entry"); expect(phase.run).not.toHaveBeenCalled(); }); diff --git a/src/lib/onboard/machine/initial-flow-phases.ts b/src/lib/onboard/machine/initial-flow-phases.ts index 4a231be6c57..498ac29e84b 100644 --- a/src/lib/onboard/machine/initial-flow-phases.ts +++ b/src/lib/onboard/machine/initial-flow-phases.ts @@ -13,7 +13,7 @@ import { type PreflightSandboxGpuFlag, type PreflightStateOptions, } from "./handlers/preflight"; -import { UnexpectedLiveOnboardFlowSliceStateError } from "./live-flow-slice"; +import { UnexpectedOnboardFlowSliceStateError } from "./flow-slice-error"; import { type OnboardPrerequisiteRepairEventRecorder, runOnboardPrerequisiteRepair, @@ -211,7 +211,7 @@ export async function runInitialOnboardFlowSlice candidate !== "init" && candidate !== "preflight"), diff --git a/src/lib/onboard/machine/live-flow-slice.test.ts b/src/lib/onboard/machine/live-flow-slice.test.ts deleted file mode 100644 index 08b87da7257..00000000000 --- a/src/lib/onboard/machine/live-flow-slice.test.ts +++ /dev/null @@ -1,406 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { afterEach, describe, expect, it, vi } from "vitest"; - -import { createSession, type Session } from "../../state/onboard-session"; -import { - EmptyLiveOnboardFlowSliceResultError, - runLiveOnboardFlowSlice, - UnexpectedLiveOnboardFlowSliceStateError, -} from "./live-flow-slice"; -import { advanceTo, type OnboardStateResult } from "./result"; -import type { OnboardMachineRunnerRuntime } from "./runner"; -import { DuplicateOnboardSequencePhaseError, type OnboardSequencePhase } from "./sequence-runner"; - -interface Context { - value: number; -} - -function runtime(state: Session["machine"]["state"]): { - runtime: OnboardMachineRunnerRuntime; - applyResult(result: OnboardStateResult): Promise; - session(): Session; -} { - let session = createSession({ - machine: { version: 1, state, stateEnteredAt: null, revision: 1 }, - }); - const runtimeApi: OnboardMachineRunnerRuntime = { - async session() { - return session; - }, - async applyResult(result) { - if (result.type === "transition") { - session = { - ...session, - machine: { - ...session.machine, - state: result.next, - revision: session.machine.revision + 1, - }, - }; - } - return session; - }, - }; - return { - runtime: runtimeApi, - applyResult(result) { - return runtimeApi.applyResult(result); - }, - session() { - return session; - }, - }; -} - -function phase( - state: OnboardSequencePhase["state"], - next: number, - result: OnboardStateResult | readonly OnboardStateResult[] = advanceTo("gateway"), -): OnboardSequencePhase { - return { - state, - run: vi.fn((context) => ({ - context: { value: next }, - result, - })), - }; -} - -function invalidatedRecorder() { - return vi.fn(async () => undefined); -} - -describe("runLiveOnboardFlowSlice", () => { - afterEach(() => { - vi.useRealTimers(); - vi.restoreAllMocks(); - }); - - it("uses the strict slice runner for fresh matching entry states", async () => { - const runSlice = vi.fn(async ({ context }) => ({ - context: { value: context.value + 1 }, - session: createSession(), - })); - const recordStateResult = vi.fn(async () => undefined); - - const result = await runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: runtime("preflight").runtime, - phases: [phase("preflight", 2)], - runWhenState: ["preflight"], - compatibilityWhenState: ["provider_selection"], - runSlice, - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }); - - expect(result.context).toEqual({ value: 2 }); - expect(runSlice).toHaveBeenCalledOnce(); - expect(recordStateResult).not.toHaveBeenCalled(); - }); - - it("applies recomputed results in exact phase order while they match durable state", async () => { - const liveRuntime = runtime("preflight"); - const runSlice = vi.fn(async ({ context }) => ({ context, session: createSession() })); - const results = [ - advanceTo("gateway"), - advanceTo("provider_selection", { metadata: { state: "gateway" } }), - advanceTo("inference", { metadata: { state: "provider_selection" } }), - ]; - const recordStateResult = vi.fn(async (result: OnboardStateResult) => - liveRuntime.applyResult(result), - ); - const recordInvalidatedStateResult = invalidatedRecorder(); - const wrappedStates: string[] = []; - - const result = await runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [ - phase("preflight", 2, results[0]), - { - state: "gateway", - run: vi.fn((context) => ({ - context: { value: context.value + 1 }, - result: [results[1], results[2]], - })), - }, - ], - runWhenState: ["preflight"], - compatibilityWhenState: ["preflight"], - phaseProgress: { - wrap: (candidate) => { - wrappedStates.push(candidate.state); - return candidate; - }, - }, - runSlice, - recordStateResult, - recordInvalidatedStateResult, - }); - - expect(result.context).toEqual({ value: 3 }); - expect(result.session.machine.state).toBe("inference"); - expect(runSlice).not.toHaveBeenCalled(); - expect(wrappedStates).toEqual(["preflight", "gateway"]); - expect(recordStateResult.mock.calls.map(([result]) => result)).toEqual(results); - expect(recordInvalidatedStateResult).not.toHaveBeenCalled(); - }); - - it("keeps resume-at-entry flows on compatibility execution", async () => { - const liveRuntime = runtime("preflight"); - const runSlice = vi.fn(async ({ context }) => ({ context, session: createSession() })); - const recordStateResult = vi.fn(async (result: OnboardStateResult) => - liveRuntime.applyResult(result), - ); - - await runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [phase("preflight", 2)], - runWhenState: ["preflight"], - compatibilityWhenState: ["preflight"], - runSlice, - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }); - - expect(runSlice).not.toHaveBeenCalled(); - expect(recordStateResult).toHaveBeenCalledOnce(); - }); - - it("keeps non-resume ahead-state flows on compatibility execution", async () => { - const liveRuntime = runtime("provider_selection"); - const runSlice = vi.fn(async ({ context }) => ({ context, session: createSession() })); - const recordStateResult = vi.fn(async (result: OnboardStateResult) => - liveRuntime.applyResult(result), - ); - const recordInvalidatedStateResult = invalidatedRecorder(); - // Preflight replays advanceTo('gateway', { state: 'preflight' }) while the - // durable machine already stands at 'provider_selection'. The compatibility - // phase body must still execute (backstop side effects) and the recomputed - // transition must be routed through invalidation, not applied. Assert both: - // stalePhase.run fired once, and invalidation carries the correct source / - // current state ownership for #6227. - const stalePhase = phase("preflight", 2); - - await runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [stalePhase], - runWhenState: ["preflight"], - compatibilityWhenState: ["provider_selection"], - runSlice, - recordStateResult, - recordInvalidatedStateResult, - }); - - expect(runSlice).not.toHaveBeenCalled(); - expect(stalePhase.run).toHaveBeenCalledOnce(); - expect(recordStateResult).not.toHaveBeenCalled(); - expect(recordInvalidatedStateResult).toHaveBeenCalledOnce(); - expect(recordInvalidatedStateResult).toHaveBeenCalledWith( - expect.objectContaining({ next: "gateway" }), - { - reason: "source_state_mismatch", - currentState: "provider_selection", - sourceState: "preflight", - }, - ); - }); - - it("keeps compatibility phases visible through the default heartbeat reporter", async () => { - vi.useFakeTimers(); - vi.setSystemTime(0); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); - let markPhaseStarted!: () => void; - let releasePhase!: () => void; - const phaseStarted = new Promise((resolve) => { - markPhaseStarted = resolve; - }); - const phaseReleased = new Promise((resolve) => { - releasePhase = resolve; - }); - const liveRuntime = runtime("provider_selection"); - const pendingGateway: OnboardSequencePhase = { - state: "gateway", - async run(context) { - markPhaseStarted(); - await phaseReleased; - return { context, result: advanceTo("inference") }; - }, - }; - - const running = runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [pendingGateway], - runWhenState: ["gateway"], - compatibilityWhenState: ["provider_selection"], - runSlice: vi.fn(), - recordStateResult: (result) => liveRuntime.applyResult(result), - recordInvalidatedStateResult: invalidatedRecorder(), - }); - await phaseStarted; - try { - await vi.advanceTimersByTimeAsync(30_000); - expect(log).toHaveBeenCalledWith(" ⏳ Still working on Gateway startup… (30s elapsed)"); - } finally { - releasePhase(); - await running; - } - expect(vi.getTimerCount()).toBe(0); - }); - - it("rejects non-resume states before the slice entry before running side effects", async () => { - const liveRuntime = runtime("init"); - const blocked = phase("provider_selection", 2); - const runSlice = vi.fn(async ({ context }) => ({ context, session: createSession() })); - const recordStateResult = vi.fn(async () => undefined); - - await expect( - runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [blocked], - runWhenState: ["provider_selection"], - compatibilityWhenState: ["inference", "sandbox"], - runSlice, - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }), - ).rejects.toBeInstanceOf(UnexpectedLiveOnboardFlowSliceStateError); - - expect(runSlice).not.toHaveBeenCalled(); - expect(blocked.run).not.toHaveBeenCalled(); - expect(recordStateResult).not.toHaveBeenCalled(); - }); - - it("rejects undeclared resume states before running side effects", async () => { - const liveRuntime = runtime("provider_selection"); - const blocked = phase("preflight", 2); - const runSlice = vi.fn(async ({ context }) => ({ context, session: createSession() })); - const recordStateResult = vi.fn(async () => undefined); - - await expect( - runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [blocked], - - runWhenState: ["preflight"], - compatibilityWhenState: ["sandbox"], - runSlice, - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }), - ).rejects.toBeInstanceOf(UnexpectedLiveOnboardFlowSliceStateError); - - expect(runSlice).not.toHaveBeenCalled(); - expect(blocked.run).not.toHaveBeenCalled(); - expect(recordStateResult).not.toHaveBeenCalled(); - }); - - it("rejects duplicate compatibility phases before running side effects", async () => { - const liveRuntime = runtime("provider_selection"); - const first = phase("preflight", 2); - const second = phase("preflight", 3); - const recordStateResult = vi.fn(async () => undefined); - - await expect( - runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [first, second], - runWhenState: ["preflight"], - compatibilityWhenState: ["provider_selection"], - runSlice: vi.fn(), - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }), - ).rejects.toBeInstanceOf(DuplicateOnboardSequencePhaseError); - - expect(first.run).not.toHaveBeenCalled(); - expect(second.run).not.toHaveBeenCalled(); - expect(recordStateResult).not.toHaveBeenCalled(); - }); - - it("rejects empty compatibility phase results", async () => { - const liveRuntime = runtime("provider_selection"); - const recordStateResult = vi.fn(async () => undefined); - - await expect( - runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [phase("preflight", 2, [])], - runWhenState: ["preflight"], - compatibilityWhenState: ["provider_selection"], - runSlice: vi.fn(), - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }), - ).rejects.toBeInstanceOf(EmptyLiveOnboardFlowSliceResultError); - - expect(recordStateResult).not.toHaveBeenCalled(); - }); - - it("invalidates a recomputed transition when the machine already stands at its target (#6227)", async () => { - const liveRuntime = runtime("gateway"); - const recordStateResult = vi.fn(async (result: OnboardStateResult) => - liveRuntime.applyResult(result), - ); - const recordInvalidatedStateResult = invalidatedRecorder(); - // Recompute of the preflight phase in ahead-state resume: machine is - // already at 'gateway' when preflight replays advanceTo('gateway', - // { state: 'preflight' }). Recording must route through the invalidated - // path with reason 'already_at_target', not the standard apply path. - await runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [phase("preflight", 2)], - runWhenState: ["preflight"], - compatibilityWhenState: ["gateway"], - runSlice: vi.fn(), - recordStateResult, - recordInvalidatedStateResult, - }); - - expect(recordStateResult).not.toHaveBeenCalled(); - expect(recordInvalidatedStateResult).toHaveBeenCalledOnce(); - expect(recordInvalidatedStateResult).toHaveBeenCalledWith( - expect.objectContaining({ next: "gateway" }), - { - reason: "already_at_target", - currentState: "gateway", - sourceState: "preflight", - }, - ); - }); - - it("propagates recomputed result application failures without running later phases", async () => { - const liveRuntime = runtime("preflight"); - const later = phase("gateway", 3); - const recordStateResult = vi.fn(async () => { - throw new Error("compatibility failed"); - }); - - await expect( - runLiveOnboardFlowSlice({ - context: { value: 1 }, - runtime: liveRuntime.runtime, - phases: [phase("preflight", 2), later], - runWhenState: ["preflight"], - compatibilityWhenState: ["preflight"], - runSlice: vi.fn(), - recordStateResult, - recordInvalidatedStateResult: invalidatedRecorder(), - }), - ).rejects.toThrow("compatibility failed"); - - expect(recordStateResult).toHaveBeenCalledOnce(); - expect(later.run).not.toHaveBeenCalled(); - }); -}); diff --git a/src/lib/onboard/machine/live-flow-slice.ts b/src/lib/onboard/machine/live-flow-slice.ts deleted file mode 100644 index 3c6c6134e16..00000000000 --- a/src/lib/onboard/machine/live-flow-slice.ts +++ /dev/null @@ -1,166 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import { createPhaseProgressReporter, type PhaseProgressReporter } from "./phase-progress"; -import type { OnboardStateResult } from "./result"; -import type { - OnboardMachineRunnerResult, - OnboardMachineRunnerRuntime, - OnboardStateHandlerResult, -} from "./runner"; -import { DuplicateOnboardSequencePhaseError, type OnboardSequencePhase } from "./sequence-runner"; -import type { OnboardMachineState } from "./types"; - -export type InvalidatedOnboardStateResultRecorder = ( - result: OnboardStateResult, - options: { - reason: "already_at_target" | "source_state_mismatch"; - currentState: OnboardMachineState; - sourceState?: string | null; - }, -) => Promise; - -export interface LiveOnboardFlowSliceOptions { - context: Context; - runtime: OnboardMachineRunnerRuntime; - phases: readonly OnboardSequencePhase[]; - runWhenState: readonly OnboardMachineState[]; - compatibilityWhenState?: readonly OnboardMachineState[]; - phaseProgress?: PhaseProgressReporter; - runSlice(options: { - context: Context; - runtime: OnboardMachineRunnerRuntime; - phases: readonly OnboardSequencePhase[]; - }): Promise>; - recordStateResult(result: OnboardStateResult): Promise; - recordInvalidatedStateResult: InvalidatedOnboardStateResultRecorder; -} - -export class EmptyLiveOnboardFlowSliceResultError extends Error { - constructor(readonly state: OnboardSequencePhase["state"]) { - super(`Onboarding live flow phase '${state}' returned no results`); - this.name = "EmptyLiveOnboardFlowSliceResultError"; - } -} - -export class UnexpectedLiveOnboardFlowSliceStateError extends Error { - constructor( - readonly state: OnboardMachineState, - readonly runWhenState: readonly OnboardMachineState[], - readonly compatibilityWhenState: readonly OnboardMachineState[], - ) { - super(`Unexpected onboarding live flow state before slice entry: ${state}`); - this.name = "UnexpectedLiveOnboardFlowSliceStateError"; - } -} - -function assertUniquePhases(phases: readonly OnboardSequencePhase[]): void { - const states = new Set["state"]>(); - for (const phase of phases) { - if (states.has(phase.state)) throw new DuplicateOnboardSequencePhaseError(phase.state); - states.add(phase.state); - } -} - -function asResultArray( - result: OnboardStateHandlerResult, - state: OnboardSequencePhase["state"], -): readonly OnboardStateResult[] { - const results = Array.isArray(result) - ? (result as readonly OnboardStateResult[]) - : [result as OnboardStateResult]; - if (results.length === 0) throw new EmptyLiveOnboardFlowSliceResultError(state); - return results; -} - -function resultSourceState(result: OnboardStateResult): string | null { - const source = result.metadata?.state; - return typeof source === "string" ? source : null; -} - -async function recordRecomputedResult( - options: Pick< - LiveOnboardFlowSliceOptions, - "runtime" | "recordStateResult" | "recordInvalidatedStateResult" - > & { phaseState: OnboardSequencePhase["state"]; result: OnboardStateResult }, -): Promise { - if (options.result.type !== "transition") { - await options.recordStateResult(options.result); - return; - } - - const current = await options.runtime.session(); - const sourceState = resultSourceState(options.result) ?? options.phaseState; - if (current.machine.state === options.result.next) { - await options.recordInvalidatedStateResult(options.result, { - reason: "already_at_target", - currentState: current.machine.state, - sourceState, - }); - return; - } - if (sourceState && current.machine.state !== sourceState) { - await options.recordInvalidatedStateResult(options.result, { - reason: "source_state_mismatch", - currentState: current.machine.state, - sourceState, - }); - return; - } - await options.recordStateResult(options.result); -} - -/** - * Run a live onboard flow slice through the strict runner when the current - * machine state is exactly at the slice entry point. Declared compatibility - * states use the recompute path so repair/backstop phase bodies still execute - * during resume or when a saved session has already advanced beyond the slice. - * Recomputed results are applied only when they still match the durable machine - * state; stale transition results are explicitly invalidated with source/target - * diagnostics. Compatibility is limited to caller-declared states so earlier or - * unexpected machine states fail before running slice side effects out of order. - */ -export async function runLiveOnboardFlowSlice({ - context, - runtime, - phases, - runWhenState, - compatibilityWhenState = [], - phaseProgress = createPhaseProgressReporter(), - runSlice, - recordStateResult, - recordInvalidatedStateResult, -}: LiveOnboardFlowSliceOptions): Promise> { - const current = await runtime.session(); - if ( - runWhenState.includes(current.machine.state) && - !compatibilityWhenState.includes(current.machine.state) - ) { - return runSlice({ context, runtime, phases }); - } - if (!compatibilityWhenState.includes(current.machine.state)) { - throw new UnexpectedLiveOnboardFlowSliceStateError( - current.machine.state, - runWhenState, - compatibilityWhenState, - ); - } - - assertUniquePhases(phases); - let nextContext = context; - for (const rawPhase of phases) { - const phase = phaseProgress.wrap(rawPhase); - const phaseResult = await phase.run(nextContext); - for (const result of asResultArray(phaseResult.result, phase.state)) { - await recordRecomputedResult({ - runtime, - recordStateResult, - recordInvalidatedStateResult, - phaseState: phase.state, - result, - }); - } - nextContext = phaseResult.context; - } - return { context: nextContext, session: await runtime.session() }; -} diff --git a/src/lib/onboard/machine/runner-sequence.test.ts b/src/lib/onboard/machine/runner-sequence.test.ts index 031c5fa3bbb..97837bbed3b 100644 --- a/src/lib/onboard/machine/runner-sequence.test.ts +++ b/src/lib/onboard/machine/runner-sequence.test.ts @@ -10,7 +10,6 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../state/onboard-session"; import { advanceTo, branchTo, completeOnboardMachine, failOnboardMachine, retryTo } from "./result"; import { @@ -53,19 +52,13 @@ function createRuntime(initialSession: Session = createSession()) { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: () => cloneSession(session), - markStepFailed: (_stepName, message) => + markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: _stepName, message, recordedAt: "now" }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; return current; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/machine/runner.test.ts b/src/lib/onboard/machine/runner.test.ts index d749b0cc678..f520f480b59 100644 --- a/src/lib/onboard/machine/runner.test.ts +++ b/src/lib/onboard/machine/runner.test.ts @@ -10,7 +10,6 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../state/onboard-session"; import { advanceTo, @@ -58,19 +57,13 @@ function createRuntime(initialSession: Session = createSession()) { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: () => cloneSession(session), - markStepFailed: (_stepName, message) => + markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: _stepName, message, recordedAt: "now" }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; return current; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/machine/runtime.test.ts b/src/lib/onboard/machine/runtime.test.ts index 332ddebe356..84d3add6852 100644 --- a/src/lib/onboard/machine/runtime.test.ts +++ b/src/lib/onboard/machine/runtime.test.ts @@ -9,9 +9,7 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../state/onboard-session"; -import type { StepMutationOptions } from "../../state/onboard-step-mutation"; import type { OnboardMachineEvent } from "./events"; import { advanceTo, @@ -31,7 +29,7 @@ function cloneSession(session: Session): Session { function createHarness(initialSession: Session | null = createSession()) { let session = initialSession ? cloneSession(initialSession) : null; const events: OnboardMachineEvent[] = []; - const stepOptionCalls: Array<{ method: string; options: StepMutationOptions | undefined }> = []; + const stepCalls: string[] = []; let tick = 0; const updateSession = (mutator: (value: Session) => Session | void): Session => { const current = session ? cloneSession(session) : createSession(); @@ -47,8 +45,8 @@ function createHarness(initialSession: Session | null = createSession()) { return cloneSession(session); }, updateSession, - markStepStarted: (stepName, options) => { - stepOptionCalls.push({ method: "markStepStarted", options }); + markStepStarted: (stepName) => { + stepCalls.push("markStepStarted"); return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; @@ -58,8 +56,8 @@ function createHarness(initialSession: Session | null = createSession()) { return current; }); }, - markStepComplete: (stepName, updates: SessionUpdates = {}, options) => { - stepOptionCalls.push({ method: "markStepComplete", options }); + markStepComplete: (stepName, updates: SessionUpdates = {}) => { + stepCalls.push("markStepComplete"); return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; @@ -69,41 +67,30 @@ function createHarness(initialSession: Session | null = createSession()) { return current; }); }, - markStepCompleteRecordOnly: (stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - const step = current.steps[stepName]; - if (!step) return current; - step.status = "complete"; - current.lastCompletedStep = stepName; - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), - markStepSkipped: (stepName) => - updateSession((current) => { - const step = current.steps[stepName]; - if (!step) return current; - step.status = "skipped"; - return current; - }), - markStepFailed: (stepName, message, options) => { - stepOptionCalls.push({ method: "markStepFailed", options }); + markStepSkipped: (stepName) => { + stepCalls.push("markStepSkipped"); return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; - step.status = "failed"; - current.status = "failed"; - current.failure = sanitizeFailure({ step: stepName, message, recordedAt: "now" }); + if (step.status === "complete" || step.status === "failed" || step.status === "skipped") + return current; + step.status = "skipped"; + step.startedAt = null; + step.completedAt = null; + step.error = null; return current; }); }, - markStepFailedRecordOnly: (stepName, message) => - updateSession((current) => { + markStepFailed: (stepName, message) => { + stepCalls.push("markStepFailed"); + return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; step.status = "failed"; step.error = message ?? null; return current; - }), + }); + }, completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); @@ -118,7 +105,7 @@ function createHarness(initialSession: Session | null = createSession()) { return { runtime: new OnboardRuntime(deps), events, - stepOptionCalls, + stepCalls, getSession: () => { if (!session) throw new Error("Expected runtime session"); return cloneSession(session); @@ -150,43 +137,72 @@ describe("OnboardRuntime", () => { expect(events[1]).toMatchObject({ type: "onboard.resumed", state: "init" }); }); - it("defaults step recording dependencies to record-only machine mutations", async () => { - const { runtime, stepOptionCalls } = createHarness(); + it("keeps step dependencies from changing machine or terminal status", async () => { + const { runtime, getSession, stepCalls } = createHarness(); await runtime.markStepStarted("preflight"); await runtime.markStepComplete("preflight", { sandboxName: "my-assistant" }); await runtime.markStepFailed("gateway", "boom"); - expect(stepOptionCalls).toEqual([ - { method: "markStepStarted", options: { updateMachine: false } }, - { method: "markStepComplete", options: { updateMachine: false } }, - { method: "markStepFailed", options: { updateMachine: false } }, - ]); - }); - - it("forwards step mutation options to step recording dependencies", async () => { - const { runtime, getSession, stepOptionCalls } = createHarness(); - const recordOnlyOptions = { updateMachine: false }; - - await runtime.markStepStarted("preflight", recordOnlyOptions); - await runtime.markStepComplete("preflight", { sandboxName: "my-assistant" }, recordOnlyOptions); - await runtime.markStepFailed("gateway", "boom", recordOnlyOptions); - - expect(stepOptionCalls).toEqual([ - { method: "markStepStarted", options: recordOnlyOptions }, - { method: "markStepComplete", options: recordOnlyOptions }, - { method: "markStepFailed", options: recordOnlyOptions }, - ]); + expect(stepCalls).toEqual(["markStepStarted", "markStepComplete", "markStepFailed"]); expect(getSession()).toMatchObject({ sandboxName: "my-assistant", - status: "failed", + status: "in_progress", + failure: null, + machine: { state: "init", revision: 0 }, steps: { preflight: { status: "complete" }, - gateway: { status: "failed" }, + gateway: { status: "failed", error: "boom" }, }, }); }); + it.each([ + "pending", + "in_progress", + ] as const)("emits one mapped skip event when a $status step becomes skipped", async (status) => { + const initial = sessionInState("policies"); + initial.endpointUrl = + "https://alice:super-secret@example.com/v1?token=super-secret&keep=yes#token=super-secret"; + initial.steps.agent_setup.status = status; + const { runtime, events, getSession, stepCalls } = createHarness(initial); + + await runtime.markStepSkipped("agent_setup"); + await runtime.markStepSkipped("agent_setup"); + + expect(stepCalls).toEqual(["markStepSkipped", "markStepSkipped"]); + expect(getSession().steps.agent_setup.status).toBe("skipped"); + expect(events).toHaveLength(1); + expect(events[0]).toMatchObject({ + version: 1, + type: "state.skipped", + sessionId: initial.sessionId, + state: "agent_setup", + step: "agent_setup", + context: { endpointOrigin: "https://example.com" }, + error: null, + metadata: {}, + }); + expect(JSON.stringify(events)).not.toContain("super-secret"); + expect(JSON.stringify(events)).not.toContain("alice"); + }); + + it.each([ + { label: "complete", stepName: "openclaw", status: "complete" as const }, + { label: "failed", stepName: "openclaw", status: "failed" as const }, + { label: "skipped", stepName: "openclaw", status: "skipped" as const }, + { label: "unknown", stepName: "not_a_step", status: null }, + ])("does not emit a skip event for a $label step", async ({ stepName, status }) => { + const initial = sessionInState("policies"); + if (status) initial.steps.openclaw.status = status; + const { runtime, events, stepCalls } = createHarness(initial); + + await runtime.markStepSkipped(stepName); + + expect(stepCalls).toEqual(["markStepSkipped"]); + expect(events).toHaveLength(0); + }); + it("validates and persists explicit transitions", async () => { const { runtime, events, getSession } = createHarness(); @@ -208,6 +224,17 @@ describe("OnboardRuntime", () => { expect(getSession().machine.state).toBe("preflight"); }); + it("rejects a stale result source before changing the machine", async () => { + const { runtime, events, getSession } = createHarness(); + + await expect( + runtime.applyResult(advanceTo("preflight", { metadata: { state: "gateway" } })), + ).rejects.toThrow("Onboarding state result source mismatch: gateway != init"); + + expect(getSession().machine).toMatchObject({ state: "init", revision: 0 }); + expect(events).toHaveLength(0); + }); + it("applies only safe context updates and emits redacted context events", async () => { const { runtime, events, getSession } = createHarness(); diff --git a/src/lib/onboard/machine/runtime.ts b/src/lib/onboard/machine/runtime.ts index 733ae47c590..6f197c9f48f 100644 --- a/src/lib/onboard/machine/runtime.ts +++ b/src/lib/onboard/machine/runtime.ts @@ -4,14 +4,11 @@ import type { JsonObject } from "../../core/json-types"; import type { CompleteSessionOptions, Session, SessionUpdates } from "../../state/onboard-session"; import * as onboardSession from "../../state/onboard-session"; -import { - RECORD_ONLY_STEP_MUTATION_OPTIONS, - type StepMutationOptions, -} from "../../state/onboard-step-mutation"; import type { ResumeConfigConflict } from "../resume-config"; import { createOnboardMachineEvent, emitOnboardMachineEvent, + machineStateFromOnboardSessionStep, type OnboardMachineEvent, } from "./events"; import type { OnboardStateResult } from "./result"; @@ -34,16 +31,10 @@ export interface OnboardRuntimeDeps { createSession(overrides?: Partial): Session; saveSession(session: Session): Session; updateSession(mutator: (session: Session) => Session | void): Session; - markStepStarted(stepName: string, options?: StepMutationOptions): Session; - markStepComplete( - stepName: string, - updates?: SessionUpdates, - options?: StepMutationOptions, - ): Session; - markStepCompleteRecordOnly(stepName: string, updates?: SessionUpdates): Session; + markStepStarted(stepName: string): Session; + markStepComplete(stepName: string, updates?: SessionUpdates): Session; markStepSkipped(stepName: string): Session; - markStepFailed(stepName: string, message?: string | null, options?: StepMutationOptions): Session; - markStepFailedRecordOnly(stepName: string, message?: string | null): Session; + markStepFailed(stepName: string, message?: string | null): Session; completeSession(updates?: SessionUpdates, options?: CompleteSessionOptions): Session; filterSafeUpdates(updates: SessionUpdates): Partial; emitEvent(event: OnboardMachineEvent): void; @@ -84,10 +75,8 @@ function defaultDeps(): OnboardRuntimeDeps { updateSession: onboardSession.updateSession, markStepStarted: onboardSession.markStepStarted, markStepComplete: onboardSession.markStepComplete, - markStepCompleteRecordOnly: onboardSession.markStepCompleteRecordOnly, markStepSkipped: onboardSession.markStepSkipped, markStepFailed: onboardSession.markStepFailed, - markStepFailedRecordOnly: onboardSession.markStepFailedRecordOnly, completeSession: onboardSession.completeSession, filterSafeUpdates: onboardSession.filterSafeUpdates, emitEvent: emitOnboardMachineEvent, @@ -160,45 +149,43 @@ export class OnboardRuntime { return session; } - async markStepStarted( - stepName: string, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, - ): Promise { - return this.deps.markStepStarted(stepName, options); - } - - async markStepComplete( - stepName: string, - updates: SessionUpdates = {}, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, - ): Promise { - return this.deps.markStepComplete(stepName, updates, options); + async markStepStarted(stepName: string): Promise { + return this.deps.markStepStarted(stepName); } - async markStepCompleteRecordOnly( - stepName: string, - updates: SessionUpdates = {}, - ): Promise { - return this.deps.markStepCompleteRecordOnly(stepName, updates); + async markStepComplete(stepName: string, updates: SessionUpdates = {}): Promise { + const safeUpdates = this.deps.filterSafeUpdates(updates); + const fields = Object.keys(safeUpdates); + const updated = this.deps.markStepComplete(stepName, { + ...(safeUpdates as SessionUpdates), + stationExpressModelIdentity: updates.stationExpressModelIdentity, + }); + if (fields.length > 0) { + this.emit("context.updated", updated, { + step: stepName, + metadata: { fields }, + }); + } + return updated; } async markStepSkipped(stepName: string): Promise { - return this.deps.markStepSkipped(stepName); - } - - async markStepFailed( - stepName: string, - message: string | null = null, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, - ): Promise { - return this.deps.markStepFailed(stepName, message, options); + const current = this.ensureSession(); + const state = machineStateFromOnboardSessionStep(stepName); + const previousStatus = state ? current.steps[stepName]?.status : null; + const updated = this.deps.markStepSkipped(stepName); + if ( + state && + (previousStatus === "pending" || previousStatus === "in_progress") && + updated.steps[stepName]?.status === "skipped" + ) { + this.emit("state.skipped", updated, { state, step: stepName }); + } + return updated; } - async markStepFailedRecordOnly( - stepName: string, - message: string | null = null, - ): Promise { - return this.deps.markStepFailedRecordOnly(stepName, message); + async markStepFailed(stepName: string, message: string | null = null): Promise { + return this.deps.markStepFailed(stepName, message); } async completeSession(updates: SessionUpdates = {}): Promise { @@ -291,7 +278,49 @@ export class OnboardRuntime { return updated; } + async assertResultWillApply(result: OnboardStateResult): Promise { + const current = this.ensureSession(); + if (result.type === "failed") { + assertValidOnboardMachineTransition(current.machine.state, "failed"); + return; + } + if (result.type === "complete") { + assertOnboardNotInterrupted(current.machine.state, "complete", current.failure); + assertValidOnboardMachineTransition(current.machine.state, "complete"); + return; + } + const sourceState = + result.metadata && typeof result.metadata.state === "string" ? result.metadata.state : null; + if (result.type === "pause") { + if (sourceState && current.machine.state !== sourceState) { + throw new Error( + `Onboarding state result source mismatch: ${sourceState} != ${current.machine.state}`, + ); + } + return; + } + assertOnboardNotInterrupted(current.machine.state, result.next, current.failure); + if (isTerminalOnboardMachineState(current.machine.state)) { + assertValidOnboardMachineTransition(current.machine.state, result.next); + } + if (current.machine.state === result.next) { + throw new Error(`Onboarding state result already reached target state: ${result.next}`); + } + if (sourceState && current.machine.state !== sourceState) { + throw new Error( + `Onboarding state result source mismatch: ${sourceState} != ${current.machine.state}`, + ); + } + const transition = assertValidOnboardMachineTransition(current.machine.state, result.next); + if (result.transitionKind && transition.kind !== result.transitionKind) { + throw new Error( + `Invalid onboarding machine transition kind: ${current.machine.state} -> ${result.next} expected ${result.transitionKind}, got ${transition.kind}`, + ); + } + } + async applyResult(result: OnboardStateResult): Promise { + await this.assertResultWillApply(result); if (result.type === "pause") { const current = this.ensureSession(); if (isTerminalOnboardMachineState(current.machine.state)) { diff --git a/src/lib/onboard/machine/sequence-runner.test.ts b/src/lib/onboard/machine/sequence-runner.test.ts index a09c52ecd63..579b2158943 100644 --- a/src/lib/onboard/machine/sequence-runner.test.ts +++ b/src/lib/onboard/machine/sequence-runner.test.ts @@ -9,7 +9,6 @@ import { normalizeSession, type Session, type SessionUpdates, - sanitizeFailure, } from "../../state/onboard-session"; import { advanceTo, branchTo, completeOnboardMachine, retryTo } from "./result"; import { OnboardRuntime, type OnboardRuntimeDeps } from "./runtime"; @@ -49,19 +48,13 @@ function createRuntime(initialSession: Session = createSession()) { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: () => cloneSession(session), markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: stepName, message, recordedAt: "now" }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; return current; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/machine/transition-traces.test.ts b/src/lib/onboard/machine/transition-traces.test.ts index e4234ac64af..61d494a2f65 100644 --- a/src/lib/onboard/machine/transition-traces.test.ts +++ b/src/lib/onboard/machine/transition-traces.test.ts @@ -8,8 +8,8 @@ * `runtime.test.ts` pins per-operation event shapes, `runner.test.ts` pins * handler sequencing without observing events). Descriptive, not * aspirational: update a pin in the same PR that changes the ordering. - * Recovery-path semantics (edges leaving terminal `failed`, the legacy - * step-mutation bridge) stay out of scope and are owned by #6227. + * Recovery-path semantics (edges leaving terminal `failed`) stay out of scope + * and are owned by #6227. */ import { describe, expect, it, vi } from "vitest"; @@ -23,7 +23,6 @@ import { type Session, type SessionUpdates, type StepState, - sanitizeFailure, } from "../../state/onboard-session"; import type { OnboardMachineEvent } from "./events"; import { handleSandboxState } from "./handlers/sandbox"; @@ -72,14 +71,12 @@ function createTracedRuntime(initialSession: Session = createSession()) { updateSession, markStepStarted: () => cloneSession(session), markStepComplete: (_stepName, updates) => applySafeUpdates(updates), - markStepCompleteRecordOnly: (_stepName, updates) => applySafeUpdates(updates), markStepSkipped: () => cloneSession(session), markStepFailed: (stepName, message) => updateSession((current) => { - current.status = "failed"; - current.failure = sanitizeFailure({ step: stepName, message, recordedAt: NOW }); + current.steps[stepName].status = "failed"; + current.steps[stepName].error = message ?? null; }), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/src/lib/onboard/resume-machine-repair.test.ts b/src/lib/onboard/resume-machine-repair.test.ts index f2741c40d6c..5c911acc52d 100644 --- a/src/lib/onboard/resume-machine-repair.test.ts +++ b/src/lib/onboard/resume-machine-repair.test.ts @@ -49,7 +49,7 @@ function cloneSession(session: Session): Session { } /** - * Creates a memory-backed runtime boundary with record-only step mutations. + * Creates a memory-backed runtime boundary that persists safe session updates. */ function createBoundaryHarness(initial: Session) { let session = cloneSession(initial); @@ -69,11 +69,8 @@ function createBoundaryHarness(initial: Session) { markStepStarted: () => cloneSession(session), markStepComplete: (_stepName, updates: SessionUpdates = {}) => updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), markStepSkipped: () => cloneSession(session), markStepFailed: () => cloneSession(session), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); @@ -89,7 +86,6 @@ function createBoundaryHarness(initial: Session) { toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, maybeForceE2eStepFailure: () => undefined, createRuntime: () => new OnboardRuntime(deps), - stepMutationOptions: { updateMachine: false }, }); return { boundary, getSession: () => cloneSession(session) }; } @@ -97,7 +93,7 @@ function createBoundaryHarness(initial: Session) { /** * Replays the live resume sequence from failed snapshot repair through completion. */ -async function runRecordOnlyResumeSequence(initial: Session): Promise { +async function runResumeSequence(initial: Session): Promise { applySessionRecovery(initial, "2026-06-01T00:01:00.000Z"); initial.failure = null; initial.status = "in_progress"; @@ -317,7 +313,7 @@ describe("resume machine repair", () => { ["preflight", "preflight", null], ["gateway", "gateway", "preflight"], ["inference", "inference", "provider_selection"], - ] as const)("lets record-only resume complete from failed %s", async (_name, failedStep, completedStep) => { + ] as const)("lets resume complete from failed %s", async (_name, failedStep, completedStep) => { const session = createFailedSession((current) => { current.failure = { step: failedStep, @@ -332,7 +328,7 @@ describe("resume machine repair", () => { } }); - const completed = await runRecordOnlyResumeSequence(session); + const completed = await runResumeSequence(session); expect(completed).toMatchObject({ status: "complete", @@ -344,7 +340,7 @@ describe("resume machine repair", () => { it.each([ "gateway", "policies", - ] as const)("lets record-only resume complete from a reopened complete snapshot after %s", async (completedStep) => { + ] as const)("lets resume complete from a reopened complete snapshot after %s", async (completedStep) => { const session = createSession({ resumable: true, status: "in_progress", @@ -358,7 +354,7 @@ describe("resume machine repair", () => { }); session.steps[completedStep].status = "complete"; - const completed = await runRecordOnlyResumeSequence(session); + const completed = await runResumeSequence(session); expect(completed).toMatchObject({ status: "complete", diff --git a/src/lib/onboard/runtime-boundary-record-only.test.ts b/src/lib/onboard/runtime-boundary-step-result.test.ts similarity index 62% rename from src/lib/onboard/runtime-boundary-record-only.test.ts rename to src/lib/onboard/runtime-boundary-step-result.test.ts index 340ca7e1341..8a68669bc2c 100644 --- a/src/lib/onboard/runtime-boundary-record-only.test.ts +++ b/src/lib/onboard/runtime-boundary-step-result.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { createSession, @@ -45,26 +45,12 @@ function createRuntimeHarness() { Object.assign(current, filterSafeUpdates(updates)); return current; }), - markStepCompleteRecordOnly: (stepName, updates: SessionUpdates = {}) => - updateSession((current) => { - current.steps[stepName].status = "complete"; - Object.assign(current, filterSafeUpdates(updates)); - return current; - }), markStepSkipped: (stepName) => updateSession((current) => { current.steps[stepName].status = "skipped"; return current; }), markStepFailed: (stepName, message) => - updateSession((current) => { - current.steps[stepName].status = "failed"; - current.steps[stepName].error = message ?? null; - current.status = "failed"; - current.failure = { step: stepName, message: message ?? null, recordedAt: "now" }; - return current; - }), - markStepFailedRecordOnly: (stepName, message) => updateSession((current) => { current.steps[stepName].status = "failed"; current.steps[stepName].error = message ?? null; @@ -91,63 +77,55 @@ function createRuntimeHarness() { }; } -describe("OnboardRuntimeBoundary record-only step/result pairing", () => { - it("pairs record-only step completion with an explicit state result", async () => { +describe("OnboardRuntimeBoundary step/result pairing", () => { + it("pairs step completion with an explicit state result", async () => { const { boundary, events } = createRuntimeHarness(); await boundary.recordStateResult(advanceTo("preflight")); const completed = await boundary.recordStepCompleteWithStateResult( "preflight", - { sandboxName: "record-only-sb" }, + { sandboxName: "paired-step-sb" }, advanceTo("gateway", { metadata: { state: "preflight" } }), ); expect(completed).toMatchObject({ - sandboxName: "record-only-sb", + sandboxName: "paired-step-sb", machine: { state: "gateway", revision: 2 }, steps: { preflight: { status: "complete" } }, }); expect(events.map((event) => event.type)).toEqual([ "state.exited", "state.entered", + "context.updated", "state.exited", "state.entered", ]); }); - it("applies validated step completion results directly without compatibility diagnostics", async () => { + it("applies validated step completion results directly", async () => { const { boundary, events } = createRuntimeHarness(); - const compatibilitySpy = vi - .spyOn(boundary, "recordStateResultWithStepCompatibility") - .mockRejectedValue(new Error("compatibility bridge should not be used")); - - try { - await boundary.recordStateResult(advanceTo("preflight")); - const completed = await boundary.recordStepCompleteWithStateResult( - "preflight", - { sandboxName: "strict-path-sb" }, - advanceTo("gateway", { metadata: { state: "preflight" } }), - ); - - expect(completed).toMatchObject({ - sandboxName: "strict-path-sb", - machine: { state: "gateway", revision: 2 }, - steps: { preflight: { status: "complete" } }, - }); - expect(compatibilitySpy).not.toHaveBeenCalled(); - expect(events.map((event) => event.type)).toEqual([ - "state.exited", - "state.entered", - "state.exited", - "state.entered", - ]); - expect(events.some((event) => event.type === "state.result.skipped")).toBe(false); - } finally { - compatibilitySpy.mockRestore(); - } + await boundary.recordStateResult(advanceTo("preflight")); + const completed = await boundary.recordStepCompleteWithStateResult( + "preflight", + { sandboxName: "strict-path-sb" }, + advanceTo("gateway", { metadata: { state: "preflight" } }), + ); + + expect(completed).toMatchObject({ + sandboxName: "strict-path-sb", + machine: { state: "gateway", revision: 2 }, + steps: { preflight: { status: "complete" } }, + }); + expect(events.map((event) => event.type)).toEqual([ + "state.exited", + "state.entered", + "context.updated", + "state.exited", + "state.entered", + ]); }); - it("pairs record-only step failure with an explicit failure result", async () => { + it("pairs step failure with an explicit failure result", async () => { const { boundary, events } = createRuntimeHarness(); await boundary.recordStateResult(advanceTo("preflight")); @@ -171,7 +149,7 @@ describe("OnboardRuntimeBoundary record-only step/result pairing", () => { ]); }); - it("rejects invalid explicit results before persisting record-only step completion", async () => { + it("rejects invalid explicit results before persisting step completion", async () => { const { boundary, getSession } = createRuntimeHarness(); await boundary.recordStateResult(advanceTo("preflight")); @@ -185,25 +163,17 @@ describe("OnboardRuntimeBoundary record-only step/result pairing", () => { }); }); - it("rejects stale state results when record-only steps did not advance the machine", async () => { + it("rejects stale state results when step updates did not advance the machine", async () => { const { boundary, events } = createRuntimeHarness(); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("preflight", { metadata: { state: "init" } }), - ); + await boundary.recordStateResult(advanceTo("preflight", { metadata: { state: "init" } })); await expect( - boundary.recordStateResultWithStepCompatibility( - advanceTo("preflight", { metadata: { state: "init" } }), - ), - ).rejects.toThrow("Record-only step result already reached target state: preflight"); + boundary.recordStateResult(advanceTo("preflight", { metadata: { state: "init" } })), + ).rejects.toThrow("Onboarding state result already reached target state: preflight"); await expect( - boundary.recordStateResultWithStepCompatibility( - advanceTo("gateway", { metadata: { state: "init" } }), - ), - ).rejects.toThrow("Record-only step result source mismatch: init != preflight"); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("gateway", { metadata: { state: "preflight" } }), - ); + boundary.recordStateResult(advanceTo("gateway", { metadata: { state: "init" } })), + ).rejects.toThrow("Onboarding state result source mismatch: init != preflight"); + await boundary.recordStateResult(advanceTo("gateway", { metadata: { state: "preflight" } })); expect(events.map((event) => event.type)).toEqual([ "state.exited", @@ -215,12 +185,12 @@ describe("OnboardRuntimeBoundary record-only step/result pairing", () => { expect(events[3]).toMatchObject({ state: "gateway" }); }); - it("rejects stale default results before compatibility replay", async () => { + it("rejects a stale result source before runtime application", async () => { const { boundary } = createRuntimeHarness(); const result = advanceTo("preflight", { metadata: { state: "missing" } }); - await expect(boundary.recordStateResultWithStepCompatibility(result)).rejects.toThrow( - "Record-only step result source mismatch: missing != init", + await expect(boundary.recordStateResult(result)).rejects.toThrow( + "Onboarding state result source mismatch: missing != init", ); }); }); diff --git a/src/lib/onboard/runtime-boundary.test.ts b/src/lib/onboard/runtime-boundary.test.ts index 160fb372d71..04b0755e232 100644 --- a/src/lib/onboard/runtime-boundary.test.ts +++ b/src/lib/onboard/runtime-boundary.test.ts @@ -10,7 +10,6 @@ import { type Session, type SessionUpdates, } from "../state/onboard-session"; -import type { StepMutationOptions } from "../state/onboard-step-mutation"; import type { OnboardMachineEvent } from "./machine/events"; import { advanceTo, @@ -25,7 +24,6 @@ import { InvalidOnboardMachineTransitionError, OnboardInterruptedError, } from "./machine/transitions"; -import type { OnboardMachineState } from "./machine/types"; import { OnboardRuntimeBoundary } from "./runtime-boundary"; import { applySessionRecovery } from "./session-recovery"; @@ -33,60 +31,10 @@ function cloneSession(session: Session): Session { return normalizeSession(JSON.parse(JSON.stringify(session))) ?? session; } -const STEP_TO_STATE: Record = { - preflight: "preflight", - gateway: "gateway", - provider_selection: "provider_selection", - inference: "inference", - sandbox: "sandbox", - openclaw: "openclaw", - agent_setup: "agent_setup", - policies: "policies", -}; - -function nextStateAfterCompletedStep( - stepName: string, - session: Pick, -): OnboardMachineState | null { - switch (stepName) { - case "preflight": - return "gateway"; - case "gateway": - return "provider_selection"; - case "provider_selection": - return "inference"; - case "inference": - return "sandbox"; - case "sandbox": - return session.agent ? "agent_setup" : "openclaw"; - case "openclaw": - case "agent_setup": - return "policies"; - case "policies": - return "finalizing"; - default: - return null; - } -} - -function transitionMachine(session: Session, state: OnboardMachineState): void { - session.machine = { - version: session.machine.version, - state, - stateEnteredAt: "2026-05-27T00:00:00.000Z", - revision: - session.machine.state === state ? session.machine.revision : session.machine.revision + 1, - }; -} - -function shouldUpdateMachine(options: StepMutationOptions | undefined): boolean { - return options?.updateMachine !== false; -} - function createRuntimeHarness(overrides: Partial = {}) { let session: Session | null = createSession(overrides); const events: OnboardMachineEvent[] = []; - const stepOptionCalls: Array<{ method: string; options: StepMutationOptions | undefined }> = []; + const stepCalls: string[] = []; const updateSession = (mutator: (value: Session) => Session | void): Session => { const current = session ? cloneSession(session) : createSession(); session = cloneSession(mutator(current) ?? current); @@ -100,8 +48,8 @@ function createRuntimeHarness(overrides: Partial = {}) { return cloneSession(session); }, updateSession, - markStepStarted: (stepName, options) => { - stepOptionCalls.push({ method: "markStepStarted", options }); + markStepStarted: (stepName) => { + stepCalls.push("markStepStarted"); return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; @@ -112,13 +60,11 @@ function createRuntimeHarness(overrides: Partial = {}) { current.lastStepStarted = stepName; current.status = "in_progress"; current.failure = null; - const state = STEP_TO_STATE[stepName]; - if (state && shouldUpdateMachine(options)) transitionMachine(current, state); return current; }); }, - markStepComplete: (stepName, updates: SessionUpdates = {}, options) => { - stepOptionCalls.push({ method: "markStepComplete", options }); + markStepComplete: (stepName, updates: SessionUpdates = {}) => { + stepCalls.push("markStepComplete"); return updateSession((current) => { const step = current.steps[stepName]; if (!step) return current; @@ -128,28 +74,22 @@ function createRuntimeHarness(overrides: Partial = {}) { current.lastCompletedStep = stepName; current.failure = null; Object.assign(current, filterSafeUpdates(updates)); - const nextState = nextStateAfterCompletedStep(stepName, current); - if (nextState && shouldUpdateMachine(options)) transitionMachine(current, nextState); return current; }); }, - markStepCompleteRecordOnly: () => cloneSession(session ?? createSession()), markStepSkipped: (stepName) => updateSession((current) => { current.steps[stepName].status = "skipped"; return current; }), - markStepFailed: (stepName, message, options) => { - stepOptionCalls.push({ method: "markStepFailed", options }); + markStepFailed: (stepName, message) => { + stepCalls.push("markStepFailed"); return updateSession((current) => { current.steps[stepName].status = "failed"; - current.failure = shouldUpdateMachine(options) - ? { step: stepName, message: message ?? null, recordedAt: "now" } - : current.failure; + current.steps[stepName].error = message ?? null; return current; }); }, - markStepFailedRecordOnly: () => cloneSession(session ?? createSession()), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); @@ -163,7 +103,7 @@ function createRuntimeHarness(overrides: Partial = {}) { return { createRuntime: () => new OnboardRuntime(deps), events, - stepOptionCalls, + stepCalls, getSession: () => cloneSession(session ?? createSession()), }; } @@ -238,7 +178,7 @@ describe("OnboardRuntimeBoundary", () => { ); }); - it("defaults boundary step recorders to record-only machine mutations", async () => { + it("keeps step status writes separate from runtime transitions", async () => { const harness = createRuntimeHarness(); const boundary = new OnboardRuntimeBoundary({ toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, @@ -250,113 +190,28 @@ describe("OnboardRuntimeBoundary", () => { await boundary.recordStepComplete("preflight"); await boundary.recordStepFailed("gateway", "boom"); - expect(harness.stepOptionCalls).toEqual([ - { method: "markStepStarted", options: { updateMachine: false } }, - { method: "markStepComplete", options: { updateMachine: false } }, - { method: "markStepFailed", options: { updateMachine: false } }, - ]); - }); - - it("keeps default boundary step recorders from advancing the machine", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - }); - - await boundary.startRecordedStep("preflight"); - expect(harness.getSession().machine.state).toBe("init"); - await boundary.recordStepComplete("preflight"); - expect(harness.getSession().machine.state).toBe("init"); - - await boundary.recordStateResultWithStepCompatibility( - advanceTo("preflight", { metadata: { state: "init" } }), - ); - expect(harness.getSession().machine.state).toBe("preflight"); - }); - - it("forwards configured step mutation options through boundary recorders", async () => { - const harness = createRuntimeHarness(); - const legacyOptions = { updateMachine: true }; - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: legacyOptions, - }); - - await boundary.startRecordedStep("preflight"); - await boundary.recordStepComplete("preflight"); - await boundary.recordStepFailed("gateway", "boom"); - - expect(harness.stepOptionCalls).toEqual([ - { method: "markStepStarted", options: legacyOptions }, - { method: "markStepComplete", options: legacyOptions }, - { method: "markStepFailed", options: legacyOptions }, - ]); - expect(harness.getSession().machine.state).toBe("gateway"); - }); - - it("emits diagnostics for legacy-compatible stale state results", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: { updateMachine: true }, - }); - - await boundary.recordStateResultWithStepCompatibility( - advanceTo("gateway", { metadata: { state: "preflight" } }), - ); - - expect(harness.events).toHaveLength(1); - expect(harness.events[0]).toMatchObject({ - type: "state.result.skipped", - state: "init", - metadata: { - reason: "source_state_mismatch", - currentState: "init", - targetState: "gateway", - sourceState: "preflight", - }, + expect(harness.stepCalls).toEqual(["markStepStarted", "markStepComplete", "markStepFailed"]); + expect(harness.getSession()).toMatchObject({ + machine: { state: "init", revision: 0 }, + steps: { preflight: { status: "complete" }, gateway: { status: "failed" } }, }); }); - it("emits diagnostics for explicit repaired resume invalidated results", async () => { + it("applies each explicit transition exactly once", async () => { const harness = createRuntimeHarness(); const boundary = new OnboardRuntimeBoundary({ toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, maybeForceE2eStepFailure: () => undefined, createRuntime: harness.createRuntime, }); - const result = advanceTo("gateway", { metadata: { state: "preflight" } }); - await boundary.recordInvalidatedStateResult(result, { - reason: "source_state_mismatch", - currentState: "init", - sourceState: "preflight", - }); + await boundary.recordStateResult(advanceTo("preflight", { metadata: { state: "init" } })); - expect(harness.events[0]).toMatchObject({ - type: "state.result.invalidated", - metadata: { - reason: "source_state_mismatch", - currentState: "init", - sourceState: "preflight", - targetState: "gateway", - }, - }); + expect(harness.getSession().machine).toMatchObject({ state: "preflight", revision: 1 }); + expect(harness.events.map((event) => event.type)).toEqual(["state.exited", "state.entered"]); }); - it.each([ - { label: "pause", result: () => pauseOnboardMachine() }, - { label: "complete", result: () => completeOnboardMachine() }, - { label: "failed", result: () => failOnboardMachine("boom") }, - ] as const)("rejects non-transition $label results before emitting invalidation (#6227)", async ({ - result, - }) => { + it("rejects a stale explicit transition before mutation", async () => { const harness = createRuntimeHarness(); const boundary = new OnboardRuntimeBoundary({ toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, @@ -365,223 +220,12 @@ describe("OnboardRuntimeBoundary", () => { }); await expect( - boundary.recordInvalidatedStateResult(result(), { - reason: "already_at_target", - currentState: "init", - sourceState: "init", - }), - ).rejects.toThrow(/Cannot invalidate non-transition/); - + boundary.recordStateResult(advanceTo("gateway", { metadata: { state: "preflight" } })), + ).rejects.toThrow("Onboarding state result source mismatch: preflight != init"); + expect(harness.getSession().machine).toMatchObject({ state: "init", revision: 0 }); expect(harness.events).toHaveLength(0); }); - it("emits diagnostics for explicit invalidated replay of stale default results", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - }); - - const result = advanceTo("preflight", { metadata: { state: "missing" } }); - await expect( - boundary.recordInvalidatedStateResult(result, { - reason: "source_state_mismatch", - currentState: "init", - sourceState: "missing", - }), - ).resolves.toMatchObject({ - machine: { state: "init" }, - }); - }); - - it("rejects skipped transition results that carry context updates", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: { updateMachine: true }, - }); - - await expect( - boundary.recordStateResultWithStepCompatibility( - advanceTo("preflight", { metadata: { state: "missing" }, updates: { provider: "nvidia" } }), - ), - ).rejects.toThrow("Cannot skip onboarding state result with context updates"); - }); - - it("allows skipped transition results whose updates are all undefined", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: { updateMachine: true }, - }); - - const session = await boundary.recordStateResultWithStepCompatibility( - advanceTo("preflight", { - metadata: { state: "missing" }, - updates: { provider: undefined, model: undefined }, - }), - ); - - expect(session.machine.state).toBe("init"); - expect(harness.events[0]).toMatchObject({ - type: "state.result.skipped", - state: "init", - metadata: { - reason: "source_state_mismatch", - currentState: "init", - targetState: "preflight", - sourceState: "missing", - }, - }); - }); - - it("records live legacy step/result compatibility through provider retry and finalization", async () => { - const harness = createRuntimeHarness(); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: { updateMachine: true }, - }); - - await boundary.startRecordedStep("preflight"); - await boundary.recordStepComplete("preflight"); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("gateway", { metadata: { state: "preflight" } }), - ); - - await boundary.startRecordedStep("gateway"); - await boundary.recordStepComplete("gateway"); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("provider_selection", { metadata: { state: "gateway" } }), - ); - - await boundary.startRecordedStep("provider_selection"); - await boundary.recordStepComplete("provider_selection", { - provider: "bad", - model: "bad-model", - }); - await boundary.startRecordedStep("inference", { provider: "bad", model: "bad-model" }); - const retryResult = retryTo("provider_selection", { - metadata: { - state: "inference", - provider: "bad", - model: "bad-model", - reason: "selection_retry", - }, - }); - await boundary.startRecordedStep("provider_selection"); - await boundary.recordStepComplete("provider_selection", { - provider: "nvidia", - model: "nemotron", - }); - await boundary.startRecordedStep("inference", { provider: "nvidia", model: "nemotron" }); - await boundary.recordStepComplete("inference", { provider: "nvidia", model: "nemotron" }); - await boundary.recordStateResultWithStepCompatibility(retryResult); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("sandbox", { - metadata: { state: "inference", provider: "nvidia", model: "nemotron" }, - }), - ); - - await boundary.startRecordedStep("sandbox"); - await boundary.recordStepComplete("sandbox", { sandboxName: "openclaw-sb" }); - await boundary.recordStateResultWithStepCompatibility( - branchTo("openclaw", { - metadata: { state: "sandbox", sandboxName: "openclaw-sb", agent: "openclaw" }, - }), - ); - - await boundary.startRecordedStep("openclaw"); - await boundary.recordStepComplete("openclaw"); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("policies", { metadata: { state: "openclaw" } }), - ); - - await boundary.startRecordedStep("policies"); - await boundary.recordStepComplete("policies", { policyPresets: ["github"] }); - await boundary.recordStateResultWithStepCompatibility( - advanceTo("finalizing", { metadata: { state: "policies" } }), - ); - - await boundary.recordStateResultWithStepCompatibility( - advanceTo("post_verify", { metadata: { state: "finalizing" } }), - ); - const completed = await boundary.recordStateResultWithStepCompatibility( - completeOnboardMachine( - { sandboxName: "openclaw-sb", provider: "nvidia", model: "nemotron" }, - { state: "post_verify" }, - ), - ); - - const skipped = harness.events.filter((event) => event.type === "state.result.skipped"); - expect(skipped.map((event) => event.metadata.targetState)).toEqual([ - "gateway", - "provider_selection", - "provider_selection", - "sandbox", - "openclaw", - "policies", - "finalizing", - ]); - expect(skipped[2]).toMatchObject({ - state: "sandbox", - metadata: { - reason: "source_state_mismatch", - currentState: "sandbox", - sourceState: "inference", - targetState: "provider_selection", - }, - }); - expect(completed).toMatchObject({ - status: "complete", - machine: { state: "complete" }, - sandboxName: "openclaw-sb", - provider: "nvidia", - model: "nemotron", - }); - }); - - it("records sandbox branch compatibility for agent setup sandboxes", async () => { - const harness = createRuntimeHarness({ agent: "hermes" }); - const boundary = new OnboardRuntimeBoundary({ - toSessionUpdates: (updates) => filterSafeUpdates(updates as SessionUpdates) as SessionUpdates, - maybeForceE2eStepFailure: () => undefined, - createRuntime: harness.createRuntime, - stepMutationOptions: { updateMachine: true }, - }); - - await boundary.recordStateResult(advanceTo("preflight")); - await boundary.recordStateResult(advanceTo("gateway")); - await boundary.recordStateResult(advanceTo("provider_selection")); - await boundary.recordStateResult(advanceTo("inference")); - await boundary.recordStateResult(advanceTo("sandbox")); - await boundary.startRecordedStep("sandbox"); - await boundary.recordStepComplete("sandbox", { sandboxName: "hermes-sb" }); - - const session = await boundary.recordStateResultWithStepCompatibility( - branchTo("agent_setup", { - metadata: { state: "sandbox", sandboxName: "hermes-sb", agent: "hermes" }, - }), - ); - - expect(session.machine.state).toBe("agent_setup"); - expect(harness.events.at(-1)).toMatchObject({ - type: "state.result.skipped", - state: "agent_setup", - metadata: { - reason: "already_at_target", - currentState: "agent_setup", - targetState: "agent_setup", - }, - }); - }); - it("records resume conflict diagnostics through the runtime", async () => { const harness = createRuntimeHarness(); const boundary = new OnboardRuntimeBoundary({ diff --git a/src/lib/onboard/runtime-boundary.ts b/src/lib/onboard/runtime-boundary.ts index cbd3f9062a3..18c9c8443da 100644 --- a/src/lib/onboard/runtime-boundary.ts +++ b/src/lib/onboard/runtime-boundary.ts @@ -2,17 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 import type { Session, SessionUpdates } from "../state/onboard-session"; -import { - RECORD_ONLY_STEP_MUTATION_OPTIONS, - type StepMutationOptions, - shouldUpdateMachine, -} from "../state/onboard-step-mutation"; import type { OnboardStateFailedResult, OnboardStateResult } from "./machine/result"; import { OnboardRuntime } from "./machine/runtime"; -import { - assertOnboardNotInterrupted, - assertValidOnboardMachineTransition, -} from "./machine/transitions"; import type { OnboardMachineEventType, OnboardMachineState } from "./machine/types"; import type { ResumeConfigConflict } from "./resume-config"; @@ -30,12 +21,6 @@ export interface OnboardRuntimeBoundaryOptions { toSessionUpdates(updates: Record): SessionUpdates; maybeForceE2eStepFailure(stepName: string): void; createRuntime?(): OnboardRuntime; - /** - * Override for legacy/test harnesses. Production boundary writes default to - * status-only step mutations so explicit OnboardStateResult transitions stay - * the durable machine source of truth. - */ - stepMutationOptions?: StepMutationOptions; } export class OnboardRuntimeBoundary { @@ -91,7 +76,7 @@ export class OnboardRuntimeBoundary { } = {}, ): Promise { const runtime = this.getRuntime(); - await runtime.markStepStarted(stepName, this.stepMutationOptions()); + await runtime.markStepStarted(stepName); if (Object.keys(updates).length > 0) { await runtime.updateContext(this.options.toSessionUpdates(updates)); } @@ -99,7 +84,7 @@ export class OnboardRuntimeBoundary { } async recordStepComplete(stepName: string, updates: SessionUpdates = {}): Promise { - return this.getRuntime().markStepComplete(stepName, updates, this.stepMutationOptions()); + return this.getRuntime().markStepComplete(stepName, updates); } async recordStepSkipped(stepName: string): Promise { @@ -107,15 +92,7 @@ export class OnboardRuntimeBoundary { } async recordStepFailed(stepName: string, message: string | null): Promise { - return this.getRuntime().markStepFailed(stepName, message, this.stepMutationOptions()); - } - - private stepMutationOptions(): StepMutationOptions { - return this.options.stepMutationOptions ?? RECORD_ONLY_STEP_MUTATION_OPTIONS; - } - - private usesLegacyMachineStepMutation(): boolean { - return shouldUpdateMachine(this.stepMutationOptions()); + return this.getRuntime().markStepFailed(stepName, message); } async recordStateSkipped( @@ -129,55 +106,13 @@ export class OnboardRuntimeBoundary { return this.getRuntime().applyResult(result); } - private async assertStateResultWillApply(result: OnboardStateResult): Promise { - const current = await this.getRuntime().session(); - if (result.type === "failed") { - assertValidOnboardMachineTransition(current.machine.state, "failed"); - return; - } - if (result.type === "complete") { - assertOnboardNotInterrupted(current.machine.state, "complete", current.failure); - assertValidOnboardMachineTransition(current.machine.state, "complete"); - return; - } - - if (result.type === "pause") { - const sourceState = - result.metadata && typeof result.metadata.state === "string" ? result.metadata.state : null; - if (sourceState && current.machine.state !== sourceState) { - throw new Error( - `Paused onboarding state result source mismatch: ${sourceState} != ${current.machine.state}`, - ); - } - return; - } - - assertOnboardNotInterrupted(current.machine.state, result.next, current.failure); - const sourceState = - result.metadata && typeof result.metadata.state === "string" ? result.metadata.state : null; - if (current.machine.state === result.next) { - throw new Error(`Record-only step result already reached target state: ${result.next}`); - } - if (sourceState && current.machine.state !== sourceState) { - throw new Error( - `Record-only step result source mismatch: ${sourceState} != ${current.machine.state}`, - ); - } - const transition = assertValidOnboardMachineTransition(current.machine.state, result.next); - if (result.transitionKind && transition.kind !== result.transitionKind) { - throw new Error( - `Invalid onboarding machine transition kind: ${current.machine.state} -> ${result.next} expected ${result.transitionKind}, got ${transition.kind}`, - ); - } - } - async recordStepCompleteWithStateResult( stepName: string, updates: SessionUpdates, result: OnboardStateResult, ): Promise { - await this.assertStateResultWillApply(result); - await this.getRuntime().markStepCompleteRecordOnly(stepName, updates); + await this.getRuntime().assertResultWillApply(result); + await this.getRuntime().markStepComplete(stepName, updates); return this.recordStateResult(result); } @@ -186,56 +121,11 @@ export class OnboardRuntimeBoundary { message: string | null, result: OnboardStateFailedResult, ): Promise { - await this.assertStateResultWillApply(result); - await this.getRuntime().markStepFailedRecordOnly(stepName, message); + await this.getRuntime().assertResultWillApply(result); + await this.getRuntime().markStepFailed(stepName, message); return this.recordStateResult(result); } - /** - * Compatibility bridge for legacy/test boundaries explicitly configured with - * `updateMachine === true`. - * - * Default record-only paths reject stale transition results before applying - * them. Skipped legacy results must stay metadata-only so stale results cannot - * become a context source. Live resume replay uses - * `recordInvalidatedStateResult` instead so recomputed-but-stale results are - * explicitly invalidated rather than accepted through this compatibility path. - */ - async recordStateResultWithStepCompatibility(result: OnboardStateResult): Promise { - const runtime = this.getRuntime(); - const current = await runtime.session(); - if (result.type !== "transition") return runtime.applyResult(result); - - if (!this.usesLegacyMachineStepMutation()) { - await this.assertStateResultWillApply(result); - return runtime.applyResult(result); - } - - if (current.machine.state === result.next) { - assertResultHasNoContextUpdates(result, "skip"); - return runtime.emitResultSkipped({ - reason: "already_at_target", - currentState: current.machine.state, - targetState: result.next, - metadata: result.metadata, - }); - } - - const sourceState = - result.metadata && typeof result.metadata.state === "string" ? result.metadata.state : null; - if (sourceState && current.machine.state !== sourceState) { - assertResultHasNoContextUpdates(result, "skip"); - return runtime.emitResultSkipped({ - reason: "source_state_mismatch", - currentState: current.machine.state, - targetState: result.next, - metadata: { ...(result.metadata ?? {}), sourceState }, - }); - } - - return runtime.applyResult(result); - } - async recordInvalidatedStateResult( result: OnboardStateResult, options: { @@ -278,14 +168,10 @@ export class OnboardRuntimeBoundary { async recordSessionComplete(updates: SessionUpdates = {}): Promise { const runtime = this.getRuntime(); const current = await runtime.session(); - assertOnboardNotInterrupted(current.machine.state, "complete", current.failure); if (current.machine.state === "finalizing") { await runtime.transition("post_verify"); return runtime.complete(updates); } - if (current.machine.state === "post_verify") { - return runtime.complete(updates); - } - return runtime.completeSession(updates); + return runtime.complete(updates); } } diff --git a/src/lib/state/onboard-session-reasoning-effort.test.ts b/src/lib/state/onboard-session-reasoning-effort.test.ts index 74d469c510d..62f78b9a431 100644 --- a/src/lib/state/onboard-session-reasoning-effort.test.ts +++ b/src/lib/state/onboard-session-reasoning-effort.test.ts @@ -6,13 +6,9 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { markStepCompleteLegacy } from "../../../test/helpers/onboard-legacy-step-mutation"; - type OnboardSessionModule = typeof import("./onboard-session"); -type OnboardStepMutationModule = typeof import("./onboard-step-mutation"); type LoadedSession = NonNullable>; let session: OnboardSessionModule; -let stepMutation: OnboardStepMutationModule; let tmpDir: string; function requireLoadedSession( @@ -27,7 +23,6 @@ beforeEach(async () => { vi.stubEnv("HOME", tmpDir); vi.resetModules(); session = await import("./onboard-session"); - stepMutation = await import("./onboard-step-mutation"); session.clearSession(); session.releaseOnboardLock(); }); @@ -43,19 +38,19 @@ afterEach(() => { describe("onboard session reasoning effort", () => { it("round-trips only valid compatible-endpoint reasoning effort updates", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { compatibleEndpointReasoningEffort: "high", }); let loaded = requireLoadedSession(session.loadSession()); expect(loaded.compatibleEndpointReasoningEffort).toBe("high"); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { compatibleEndpointReasoningEffort: "extreme", } as never); loaded = requireLoadedSession(session.loadSession()); expect(loaded.compatibleEndpointReasoningEffort).toBe("high"); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { compatibleEndpointReasoningEffort: null, }); expect( diff --git a/src/lib/state/onboard-session.test.ts b/src/lib/state/onboard-session.test.ts index 0de6c8cb5f7..29d976d157a 100644 --- a/src/lib/state/onboard-session.test.ts +++ b/src/lib/state/onboard-session.test.ts @@ -8,21 +8,13 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { - markStepCompleteLegacy, - markStepFailedLegacy, - markStepStartedLegacy, -} from "../../../test/helpers/onboard-legacy-step-mutation"; - const require = createRequire(import.meta.url); const distPath = require.resolve("./onboard-session"); const eventsDistPath = require.resolve("../onboard/machine/events"); -const stepMutationDistPath = require.resolve("./onboard-step-mutation"); const originalHome = process.env.HOME; type OnboardSessionModule = typeof import("./onboard-session"); type OnboardMachineEventsModule = typeof import("../onboard/machine/events"); type OnboardMachineEvent = import("../onboard/machine/events").OnboardMachineEvent; -type OnboardStepMutationModule = typeof import("./onboard-step-mutation"); type LoadedSession = NonNullable>; type DebugSummary = NonNullable>; type NullableSessionUpdateKey = import("./onboard-session").NullableSessionUpdateKey; @@ -30,7 +22,6 @@ type MessagingPlan = NonNullable; type MessagingChannelId = MessagingPlan["channels"][number]["channelId"]; let session: OnboardSessionModule; let machineEvents: OnboardMachineEventsModule; -let stepMutation: OnboardStepMutationModule; let tmpDir: string; const _nullableSessionUpdateKeyAcceptsNullableFields: Record< @@ -117,10 +108,8 @@ beforeEach(() => { process.env.HOME = tmpDir; delete require.cache[distPath]; delete require.cache[eventsDistPath]; - delete require.cache[stepMutationDistPath]; session = require("./onboard-session"); machineEvents = require("../onboard/machine/events"); - stepMutation = require("./onboard-step-mutation"); machineEvents.clearOnboardMachineEventListeners(); session.clearSession(); session.releaseOnboardLock(); @@ -198,7 +187,7 @@ describe("onboard session", () => { it("redacts credential-bearing endpoint URLs before persisting them", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { endpointUrl: "https://alice:secret@example.com/v1/models?token=abc123&sig=def456&X-Amz-Signature=ghi789&keep=yes#token=frag", }); @@ -218,7 +207,7 @@ describe("onboard session", () => { stationExpressIntent: { version: 1, kind: "spark", sandboxName: "my-assistant" }, }), ); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "vllm-local", model: "nvidia/Qwen3.6-35B-A3B-NVFP4", sandboxName: "my-assistant", @@ -229,31 +218,28 @@ describe("onboard session", () => { expect(loaded.provider).toBe("vllm-local"); }); - it("marks steps started, completed, and failed", () => { + it("records step status without changing machine or terminal status", () => { session.saveSession(session.createSession()); - markStepStartedLegacy(session, stepMutation, "gateway"); + session.markStepStarted("gateway"); let loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.gateway.status).toBe("in_progress"); expect(loaded.lastStepStarted).toBe("gateway"); expect(loaded.steps.gateway.completedAt).toBeNull(); - markStepCompleteLegacy(session, stepMutation, "gateway", { sandboxName: "my-assistant" }); + session.markStepComplete("gateway", { sandboxName: "my-assistant" }); loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.gateway.status).toBe("complete"); expect(loaded.sandboxName).toBe("my-assistant"); expect(loaded.steps.gateway.completedAt).toBeTruthy(); - markStepFailedLegacy(session, stepMutation, "sandbox", "Sandbox creation failed"); + session.markStepFailed("sandbox", "Sandbox creation failed"); loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.sandbox.status).toBe("failed"); expect(loaded.steps.sandbox.completedAt).toBeNull(); - expect(loaded.failure).not.toBeNull(); - if (!loaded.failure) { - throw new Error("Expected failure metadata after markStepFailed()"); - } - expect(loaded.failure.step).toBe("sandbox"); - expect(loaded.failure.message).toMatch(/Sandbox creation failed/); - expect(loaded.machine.state).toBe("failed"); + expect(loaded.steps.sandbox.error).toBe("Sandbox creation failed"); + expect(loaded.failure).toBeNull(); + expect(loaded.status).toBe("in_progress"); + expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); }); it("can record step boundaries without mutating the machine snapshot", () => { @@ -261,53 +247,43 @@ describe("onboard session", () => { machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); session.saveSession(session.createSession()); - session.markStepStarted("preflight", { updateMachine: false }); + session.markStepStarted("preflight"); let loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.preflight.status).toBe("in_progress"); expect(loaded.status).toBe("in_progress"); expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - session.markStepComplete( - "preflight", - { sandboxName: "my-assistant" }, - { updateMachine: false }, - ); + session.markStepComplete("preflight", { sandboxName: "my-assistant" }); loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.preflight.status).toBe("complete"); expect(loaded.sandboxName).toBe("my-assistant"); expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - session.markStepFailed("gateway", "Gateway failed", { updateMachine: false }); + session.markStepFailed("gateway", "Gateway failed"); loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.gateway.status).toBe("failed"); expect(loaded.status).toBe("in_progress"); expect(loaded.failure).toBeNull(); expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - expect(emitted.map((event) => event.type)).toEqual(["context.updated"]); + expect(emitted).toEqual([]); }); - it("persists a compact machine snapshot across step boundaries", () => { + it("leaves machine transition ownership outside step helpers", () => { session.saveSession(session.createSession()); let loaded = requireLoadedSession(session.loadSession()); expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - markStepStartedLegacy(session, stepMutation, "preflight"); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "preflight", revision: 1 }); - expect(loaded.machine.stateEnteredAt).toBe(loaded.steps.preflight.startedAt); - - markStepCompleteLegacy(session, stepMutation, "preflight"); + session.markStepStarted("preflight"); loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "gateway", revision: 2 }); - expect(loaded.machine.stateEnteredAt).toBe(loaded.steps.preflight.completedAt); + expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - markStepCompleteLegacy(session, stepMutation, "gateway"); + session.markStepComplete("preflight"); loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "provider_selection", revision: 3 }); + expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - session.completeSession(); + session.markStepComplete("gateway"); loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "complete", revision: 4 }); + expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); expect(requireDebugSummary(session.summarizeForDebug()).machine).toEqual(loaded.machine); }); @@ -388,59 +364,22 @@ describe("onboard session", () => { }); }); - it("emits redacted structured machine events for session step mutations", () => { + it("does not emit machine events for direct step mutations", () => { const emitted: OnboardMachineEvent[] = []; machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); session.saveSession(session.createSession({ sessionId: "session-1" })); - markStepStartedLegacy(session, stepMutation, "gateway"); - markStepCompleteLegacy(session, stepMutation, "gateway", { + session.markStepStarted("gateway"); + session.markStepComplete("gateway", { sandboxName: "my-assistant", endpointUrl: "https://alice:super-secret-token@example.com/v1?token=super-secret-token&keep=yes#token=super-secret-token", credentialEnv: "NVIDIA_INFERENCE_API_KEY", }); session.markStepSkipped("openclaw"); - markStepFailedLegacy( - session, - stepMutation, - "sandbox", - "NVIDIA_INFERENCE_API_KEY=super-secret-token", - ); - session.completeSession({ provider: "ollama-local", credentialEnv: null }); - - expect(emitted.map((event) => event.type)).toEqual([ - "state.entered", - "context.updated", - "state.completed", - "state.skipped", - "state.failed", - "onboard.failed", - "context.updated", - "onboard.completed", - ]); - expect(emitted[0]).toMatchObject({ - version: 1, - sessionId: "session-1", - state: "gateway", - step: "gateway", - error: null, - }); - expect(emitted[1].context).toMatchObject({ - sandboxName: "my-assistant", - credentialEnv: "NVIDIA_INFERENCE_API_KEY", - }); - expect(emitted[1].context.endpointOrigin).toBe("https://example.com"); - expect(emitted[1].metadata.fields).toEqual(["sandboxName", "endpointUrl", "credentialEnv"]); - expect(emitted[4]).toMatchObject({ - type: "state.failed", - state: "sandbox", - step: "sandbox", - error: "NVIDIA_INFERENCE_API_KEY=", - }); - expect(emitted[5]).toMatchObject({ type: "onboard.failed", state: "failed" }); - expect(emitted.at(-1)).toMatchObject({ type: "onboard.completed", state: "complete" }); - expect(JSON.stringify(emitted)).not.toContain("super-secret-token"); + session.markStepFailed("sandbox", "NVIDIA_INFERENCE_API_KEY=super-secret-token"); + + expect(emitted).toEqual([]); const persisted = JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf8")); expect(persisted.events).toBeUndefined(); @@ -452,7 +391,7 @@ describe("onboard session", () => { }); session.saveSession(session.createSession()); - expect(() => markStepStartedLegacy(session, stepMutation, "preflight")).not.toThrow(); + expect(() => session.markStepStarted("preflight")).not.toThrow(); const loaded = requireLoadedSession(session.loadSession()); expect(loaded.steps.preflight.status).toBe("in_progress"); @@ -463,7 +402,7 @@ describe("onboard session", () => { machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); session.saveSession(session.createSession()); - markStepStartedLegacy(session, stepMutation, "not_a_real_step"); + session.markStepStarted("not_a_real_step"); expect(emitted).toEqual([]); }); @@ -478,8 +417,8 @@ describe("onboard session", () => { session.completeSession(); session.completeSession(); - expect(emitted.map((event) => event.type)).toEqual(["state.skipped", "onboard.completed"]); - expect(emitted).toHaveLength(2); + expect(emitted.map((event) => event.type)).toEqual(["onboard.completed"]); + expect(emitted).toHaveLength(1); }); it("persists safe provider metadata without persisting secrets", () => { @@ -503,7 +442,7 @@ describe("onboard session", () => { token: "secret", }, }; - markStepCompleteLegacy(session, stepMutation, "provider_selection", unsafeProviderUpdate); + session.markStepComplete("provider_selection", unsafeProviderUpdate); const loaded = requireLoadedSession(session.loadSession()); expect(loaded.provider).toBe("nvidia-nim"); @@ -535,7 +474,7 @@ describe("onboard session", () => { it("clears credentialEnv when a provider-selection update passes null (#2625)", () => { // Seed with a prior remote-provider onboard state. session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "openai", model: "gpt-4o", endpointUrl: "https://api.openai.com/v1", @@ -548,7 +487,7 @@ describe("onboard session", () => { // User re-runs onboard and picks local Ollama. The wizard emits // credentialEnv=null and nimContainer=null alongside the new provider. - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "ollama-local", model: "qwen3:14b", endpointUrl: "http://host.docker.internal:11434/v1", @@ -568,12 +507,12 @@ describe("onboard session", () => { // Regression guard: undefined must mean "leave unchanged", distinct from // null ("clear"). Partial updates must not accidentally wipe fields. session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "openai", model: "gpt-4o", credentialEnv: "OPENAI_API_KEY", }); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { model: "gpt-4o-mini" }); + session.markStepComplete("provider_selection", { model: "gpt-4o-mini" }); const loaded = requireLoadedSession(session.loadSession()); expect(loaded.model).toBe("gpt-4o-mini"); @@ -595,7 +534,7 @@ describe("onboard session", () => { JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf8")).migratedLegacyValueHashes, ).toBeNull(); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { migratedLegacyValueHashes: { NVIDIA_API_KEY: digest, BROKEN_NUMERIC: 123, @@ -617,10 +556,10 @@ describe("onboard session", () => { session.saveSession(session.createSession()); const unset = JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf8")).credentialEnv; - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { credentialEnv: "NVIDIA_INFERENCE_API_KEY", }); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { credentialEnv: null }); + session.markStepComplete("provider_selection", { credentialEnv: null }); const declined = JSON.parse(fs.readFileSync(session.SESSION_FILE, "utf8")).credentialEnv; expect(unset).toBeNull(); @@ -633,20 +572,20 @@ describe("onboard session", () => { it("only persists known Hermes auth methods", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "hermes-provider", hermesAuthMethod: "oauth", }); let loaded = requireLoadedSession(session.loadSession()); expect(loaded.hermesAuthMethod).toBe("oauth"); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { hermesAuthMethod: "not-a-real-method" as never, }); loaded = requireLoadedSession(session.loadSession()); expect(loaded.hermesAuthMethod).toBe("oauth"); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { hermesAuthMethod: null, }); loaded = requireLoadedSession(session.loadSession()); @@ -701,7 +640,7 @@ describe("onboard session", () => { // support the null-clear contract. If any regresses to the old // string-only guard, the test below catches it. session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { sandboxName: "stale-sandbox", provider: "openai", model: "gpt-4o", @@ -712,7 +651,7 @@ describe("onboard session", () => { nimContainer: "nim-abc", }); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { sandboxName: null, provider: null, model: null, @@ -739,7 +678,7 @@ describe("onboard session", () => { // finalizes the session for a successful run. A local-provider onboard // must not leave a stale credentialEnv on the "complete" record either. session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { provider: "openai", credentialEnv: "OPENAI_API_KEY", }); @@ -870,13 +809,13 @@ describe("onboard session", () => { it("filterSafeUpdates passes through messagingPlan and accepts explicit null clear", () => { session.saveSession(session.createSession()); const plan = makeMessagingPlan("my-assistant", ["discord"]); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { messagingPlan: plan }); + session.markStepComplete("provider_selection", { messagingPlan: plan }); expect(requireLoadedSession(session.loadSession()).messagingPlan).toMatchObject({ sandboxName: "my-assistant", channels: [expect.objectContaining({ channelId: "discord", configured: true })], }); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { messagingPlan: null }); + session.markStepComplete("provider_selection", { messagingPlan: null }); expect(requireLoadedSession(session.loadSession()).messagingPlan).toBeNull(); }); @@ -973,7 +912,7 @@ describe("onboard session", () => { it("persists and clears web search config through safe session updates", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { webSearchConfig: { fetchEnabled: true }, }); @@ -1030,7 +969,7 @@ describe("onboard session", () => { token: "should-not-persist", }, }; - markStepCompleteLegacy(session, stepMutation, "provider_selection", unsafeMetadataUpdate); + session.markStepComplete("provider_selection", unsafeMetadataUpdate); const loaded = requireLoadedSession(session.loadSession()); expect(loaded.metadata.gatewayName).toBe("nemoclaw"); @@ -1284,9 +1223,7 @@ describe("onboard session", () => { it("redacts sensitive values from persisted failure messages", () => { session.saveSession(session.createSession()); - markStepFailedLegacy( - session, - stepMutation, + session.markStepFailed( "inference", "provider auth failed with NVIDIA_INFERENCE_API_KEY=nvapi-secret Bearer topsecret sk-secret-value-that-is-long-enough ghp_1234567890123456789012345", ); @@ -1298,11 +1235,8 @@ describe("onboard session", () => { expect(loaded.steps.inference.error).not.toContain("topsecret"); expect(loaded.steps.inference.error).not.toContain("sk-secret-value-that-is-long-enough"); expect(loaded.steps.inference.error).not.toContain("ghp_1234567890123456789012345"); - expect(loaded.failure).not.toBeNull(); - if (!loaded.failure) { - throw new Error("Expected failure metadata after markStepFailed()"); - } - expect(loaded.failure.message).toBe(loaded.steps.inference.error); + expect(loaded.failure).toBeNull(); + expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); }); it("round-trips null messagingPlan through normalizeSession", () => { @@ -1330,7 +1264,7 @@ describe("onboard session", () => { it("filterSafeUpdates preserves messagingPlan field", () => { session.saveSession(session.createSession()); const plan = makeMessagingPlan("my-assistant", ["slack", "discord"]); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { messagingPlan: plan, }); @@ -1346,7 +1280,7 @@ describe("onboard session", () => { it("filterSafeUpdates ignores malformed messagingPlan values", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { messagingPlan: { sandboxName: "my-assistant" }, } as unknown as Parameters[1]); @@ -1356,7 +1290,7 @@ describe("onboard session", () => { it("routes telegramConfig through markStepComplete in filterSafeUpdates (#1737)", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { telegramConfig: { requireMention: true }, }); @@ -1364,7 +1298,7 @@ describe("onboard session", () => { expect(loaded.telegramConfig).toEqual({ requireMention: true }); // Explicit null (clearing the field) should also round-trip. - markStepCompleteLegacy(session, stepMutation, "provider_selection", { telegramConfig: null }); + session.markStepComplete("provider_selection", { telegramConfig: null }); const cleared = session.loadSession()!; expect(cleared.telegramConfig).toBeNull(); }); @@ -1372,7 +1306,7 @@ describe("onboard session", () => { it("drops malformed telegramConfig values in filterSafeUpdates (#1737)", () => { session.saveSession(session.createSession()); // Non-boolean requireMention — must not leak through. - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { telegramConfig: { requireMention: "yes" } as unknown as { requireMention: boolean }, }); @@ -1382,7 +1316,7 @@ describe("onboard session", () => { it("filterSafeUpdates routes wechatConfig through markStepComplete", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { wechatConfig: { accountId: "primary", baseUrl: "https://x", userId: "u" }, }); @@ -1395,14 +1329,14 @@ describe("onboard session", () => { // Explicit null clears the field (used when WeChat is removed from the // enabled channels on a subsequent onboard). - markStepCompleteLegacy(session, stepMutation, "provider_selection", { wechatConfig: null }); + session.markStepComplete("provider_selection", { wechatConfig: null }); const cleared = session.loadSession()!; expect(cleared.wechatConfig).toBeNull(); }); it("filterSafeUpdates drops malformed wechatConfig values", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { wechatConfig: { accountId: 9000 } as unknown as { accountId: string }, }); @@ -1427,8 +1361,8 @@ describe("onboard session", () => { it("summarizes the session for debug output", () => { session.saveSession(session.createSession({ sandboxName: "my-assistant" })); - markStepStartedLegacy(session, stepMutation, "preflight"); - markStepCompleteLegacy(session, stepMutation, "preflight"); + session.markStepStarted("preflight"); + session.markStepComplete("preflight"); session.completeSession(); const summary = requireDebugSummary(session.summarizeForDebug()); @@ -1440,12 +1374,15 @@ describe("onboard session", () => { }); it("keeps debug summaries redacted when failures were sanitized", () => { - session.saveSession(session.createSession({ sandboxName: "my-assistant" })); - markStepFailedLegacy( - session, - stepMutation, - "provider_selection", - "Bearer abcdefghijklmnopqrstuvwxyz", + session.saveSession( + session.createSession({ + sandboxName: "my-assistant", + failure: { + step: "provider_selection", + message: "Bearer abcdefghijklmnopqrstuvwxyz", + recordedAt: "2026-04-01T00:00:00.000Z", + }, + }), ); const summary = requireDebugSummary(session.summarizeForDebug()); diff --git a/src/lib/state/onboard-session.ts b/src/lib/state/onboard-session.ts index fbc240a5466..84386bc4bb1 100644 --- a/src/lib/state/onboard-session.ts +++ b/src/lib/state/onboard-session.ts @@ -49,11 +49,6 @@ import { preserveInvalidSessionToolDisclosure, type ToolDisclosure, } from "./onboard-session-tool-disclosure"; -import { - RECORD_ONLY_STEP_MUTATION_OPTIONS, - type StepMutationOptions, - shouldUpdateMachine, -} from "./onboard-step-mutation"; import { nextMachineStateAfterCompletedStep } from "./onboard-step-state"; import { nemoclawStateRoot } from "./state-root"; @@ -1382,11 +1377,7 @@ export function updateSession(mutator: (session: Session) => Session | void): Se return saveSession(next); } -function markStepStartedWithOptions( - stepName: string, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { - let shouldEmit = false; +export function markStepStarted(stepName: string): Session { const updatedSession = updateSession((session) => { const step = session.steps[stepName]; if (!step) return session; @@ -1398,28 +1389,14 @@ function markStepStartedWithOptions( session.lastStepStarted = stepName; session.failure = null; session.status = "in_progress"; - const state = machineStateFromOnboardSessionStep(stepName); - shouldEmit = Boolean(state && shouldUpdateMachine(options)); - if (state && shouldEmit) transitionMachineSnapshot(session, state, now); return session; }); - if (shouldEmit) { - emitOnboardMachineEvent( - createOnboardMachineEvent({ type: "state.entered", session: updatedSession, step: stepName }), - ); - } return updatedSession; } -function markStepCompleteWithOptions( - stepName: string, - updates: SessionUpdates = {}, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { +export function markStepComplete(stepName: string, updates: SessionUpdates = {}): Session { const safeUpdates = filterSafeUpdates(updates); - const hasUpdates = Object.keys(safeUpdates).length > 0; - let shouldEmit = false; - const updatedSession = updateSession((session) => { + return updateSession((session) => { const step = session.steps[stepName]; if (!step) return session; // Spark managed-vLLM Express intents (#7231) carry no receipt/served state @@ -1445,62 +1422,12 @@ function markStepCompleteWithOptions( Object.assign(session, safeUpdates); if (stationExpressIntent) session.stationExpressIntent = stationExpressIntent; else if (sparkExpressComplete) session.stationExpressIntent = null; - const nextState = nextMachineStateAfterCompletedStep(stepName, session); - shouldEmit = Boolean(nextState && shouldUpdateMachine(options)); - if (nextState && shouldEmit) transitionMachineSnapshot(session, nextState, now); return session; }); - if (hasUpdates) { - emitOnboardMachineEvent( - createOnboardMachineEvent({ - type: "context.updated", - session: updatedSession, - step: stepName, - metadata: { fields: Object.keys(safeUpdates) }, - }), - ); - } - if (shouldEmit) { - emitOnboardMachineEvent( - createOnboardMachineEvent({ - type: "state.completed", - session: updatedSession, - step: stepName, - }), - ); - } - return updatedSession; -} - -export function markStepStarted( - stepName: string, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { - return markStepStartedWithOptions(stepName, options); -} - -export function markStepStartedRecordOnly(stepName: string): Session { - return markStepStartedWithOptions(stepName, RECORD_ONLY_STEP_MUTATION_OPTIONS); -} - -export function markStepComplete( - stepName: string, - updates: SessionUpdates = {}, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { - return markStepCompleteWithOptions(stepName, updates, options); -} - -export function markStepCompleteRecordOnly( - stepName: string, - updates: SessionUpdates = {}, -): Session { - return markStepCompleteWithOptions(stepName, updates, RECORD_ONLY_STEP_MUTATION_OPTIONS); } export function markStepSkipped(stepName: string): Session { - let shouldEmit = false; - const updatedSession = updateSession((session) => { + return updateSession((session) => { const step = session.steps[stepName]; if (!step) return session; if (step.status === "complete" || step.status === "failed" || step.status === "skipped") @@ -1509,75 +1436,19 @@ export function markStepSkipped(stepName: string): Session { step.startedAt = null; step.completedAt = null; step.error = null; - shouldEmit = true; return session; }); - if (shouldEmit) { - emitOnboardMachineEvent( - createOnboardMachineEvent({ type: "state.skipped", session: updatedSession, step: stepName }), - ); - } - return updatedSession; } -function markStepFailedWithOptions( - stepName: string, - message: string | null = null, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { - let shouldEmit = false; - const updatedSession = updateSession((session) => { +export function markStepFailed(stepName: string, message: string | null = null): Session { + return updateSession((session) => { const step = session.steps[stepName]; if (!step) return session; - const now = new Date().toISOString(); step.status = "failed"; step.completedAt = null; step.error = redactSensitiveText(message); - shouldEmit = shouldUpdateMachine(options); - if (shouldEmit) { - session.failure = sanitizeFailure({ - step: stepName, - message, - recordedAt: now, - interrupted: false, - }); - session.status = "failed"; - transitionMachineSnapshot(session, "failed", now); - } return session; }); - if (shouldEmit) { - emitOnboardMachineEvent( - createOnboardMachineEvent({ - type: "state.failed", - session: updatedSession, - step: stepName, - error: message, - }), - ); - emitOnboardMachineEvent( - createOnboardMachineEvent({ - type: "onboard.failed", - session: updatedSession, - state: "failed", - step: stepName, - error: message, - }), - ); - } - return updatedSession; -} - -export function markStepFailed( - stepName: string, - message: string | null = null, - options: StepMutationOptions = RECORD_ONLY_STEP_MUTATION_OPTIONS, -): Session { - return markStepFailedWithOptions(stepName, message, options); -} - -export function markStepFailedRecordOnly(stepName: string, message: string | null = null): Session { - return markStepFailedWithOptions(stepName, message, RECORD_ONLY_STEP_MUTATION_OPTIONS); } /** diff --git a/src/lib/state/onboard-step-mutation.test.ts b/src/lib/state/onboard-step-mutation.test.ts deleted file mode 100644 index b5593f594c4..00000000000 --- a/src/lib/state/onboard-step-mutation.test.ts +++ /dev/null @@ -1,167 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; - -import type * as eventsModule from "../onboard/machine/events"; -import type * as sessionModule from "./onboard-session"; -import type * as stepMutationModule from "./onboard-step-mutation"; - -const originalHome = process.env.HOME; -let session: typeof sessionModule; -let stepMutation: typeof stepMutationModule; -let machineEvents: typeof eventsModule; -let tmpDir: string; - -beforeEach(async () => { - tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-step-mutation-")); - process.env.HOME = tmpDir; - vi.resetModules(); - stepMutation = await import("./onboard-step-mutation"); - session = await import("./onboard-session"); - machineEvents = await import("../onboard/machine/events"); - machineEvents.clearOnboardMachineEventListeners(); - session.clearSession(); - session.releaseOnboardLock(); -}); - -afterEach(() => { - machineEvents.clearOnboardMachineEventListeners(); - fs.rmSync(tmpDir, { recursive: true, force: true }); - if (originalHome === undefined) { - delete process.env.HOME; - } else { - process.env.HOME = originalHome; - } -}); - -function requireLoadedSession(loaded: ReturnType) { - expect(loaded).not.toBeNull(); - if (!loaded) throw new Error("Expected onboard session to be present"); - return loaded; -} - -describe("record-only onboard step mutation", () => { - it("freezes shared mutation option constants and preserves helper behavior", () => { - expect(Object.isFrozen(stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS)).toBe(true); - expect(Object.isFrozen(stepMutation.RECORD_ONLY_STEP_MUTATION_OPTIONS)).toBe(true); - expect(() => { - ( - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS as { updateMachine?: boolean } - ).updateMachine = false; - }).toThrow(TypeError); - expect(() => { - ( - stepMutation.RECORD_ONLY_STEP_MUTATION_OPTIONS as { updateMachine?: boolean } - ).updateMachine = true; - }).toThrow(TypeError); - - session.saveSession(session.createSession()); - session.markStepStarted("preflight", stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS); - let loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "preflight", revision: 1 }); - - session.markStepStartedRecordOnly("gateway"); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.gateway.status).toBe("in_progress"); - expect(loaded.machine).toMatchObject({ state: "preflight", revision: 1 }); - }); - - it("persists step status and per-step failure errors without mutating the machine snapshot", () => { - const emitted: eventsModule.OnboardMachineEvent[] = []; - machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); - session.saveSession(session.createSession()); - - session.markStepStartedRecordOnly("preflight"); - let loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.preflight.status).toBe("in_progress"); - expect(loaded.status).toBe("in_progress"); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - - session.markStepCompleteRecordOnly("preflight", { sandboxName: "my-assistant" }); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.preflight.status).toBe("complete"); - expect(loaded.sandboxName).toBe("my-assistant"); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - - session.markStepFailedRecordOnly( - "gateway", - "Gateway failed: NVIDIA_INFERENCE_API_KEY=nvapi-secret", - ); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.gateway.status).toBe("failed"); - expect(loaded.steps.gateway.error).toBe("Gateway failed: NVIDIA_INFERENCE_API_KEY="); - expect(loaded.steps.gateway.error).not.toContain("nvapi-secret"); - expect(loaded.status).toBe("in_progress"); - expect(loaded.failure).toBeNull(); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - expect(emitted.map((event) => event.type)).toEqual(["context.updated"]); - }); - - it("keeps explicit legacy step helpers on legacy machine mutation", () => { - const emitted: eventsModule.OnboardMachineEvent[] = []; - machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); - session.saveSession(session.createSession()); - - session.markStepStarted("preflight", stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS); - let loaded = requireLoadedSession(session.loadSession()); - expect(loaded.machine).toMatchObject({ state: "preflight", revision: 1 }); - - session.markStepComplete( - "preflight", - { sandboxName: "my-assistant" }, - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS, - ); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.sandboxName).toBe("my-assistant"); - expect(loaded.machine).toMatchObject({ state: "gateway", revision: 2 }); - - session.markStepFailed( - "gateway", - "Gateway failed: NVIDIA_INFERENCE_API_KEY=nvapi-secret", - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS, - ); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.status).toBe("failed"); - expect(loaded.failure?.message).toBe("Gateway failed: NVIDIA_INFERENCE_API_KEY="); - expect(loaded.failure?.message).not.toContain("nvapi-secret"); - expect(loaded.machine).toMatchObject({ state: "failed", revision: 3 }); - expect(emitted.map((event) => event.type)).toEqual([ - "state.entered", - "context.updated", - "state.completed", - "state.failed", - "onboard.failed", - ]); - }); - - it("defaults no-options step helpers to record-only machine mutation", () => { - const emitted: eventsModule.OnboardMachineEvent[] = []; - machineEvents.addOnboardMachineEventListener((event) => emitted.push(event)); - session.saveSession(session.createSession()); - - session.markStepStarted("preflight"); - let loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.preflight.status).toBe("in_progress"); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - - session.markStepComplete("preflight", { sandboxName: "my-assistant" }); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.preflight.status).toBe("complete"); - expect(loaded.sandboxName).toBe("my-assistant"); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - - session.markStepFailed("gateway", "Gateway failed: NVIDIA_INFERENCE_API_KEY=nvapi-secret"); - loaded = requireLoadedSession(session.loadSession()); - expect(loaded.steps.gateway.status).toBe("failed"); - expect(loaded.steps.gateway.error).toBe("Gateway failed: NVIDIA_INFERENCE_API_KEY="); - expect(loaded.steps.gateway.error).not.toContain("nvapi-secret"); - expect(loaded.status).toBe("in_progress"); - expect(loaded.failure).toBeNull(); - expect(loaded.machine).toMatchObject({ state: "init", revision: 0 }); - expect(emitted.map((event) => event.type)).toEqual(["context.updated"]); - }); -}); diff --git a/src/lib/state/onboard-step-mutation.ts b/src/lib/state/onboard-step-mutation.ts deleted file mode 100644 index 406fb5c98bb..00000000000 --- a/src/lib/state/onboard-step-mutation.ts +++ /dev/null @@ -1,25 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -export interface StepMutationOptions { - /** - * Transitional FSM migration escape hatch for paths where legacy step helpers - * still move session.machine. Runtime-owned onboarding should use record-only - * step writes paired with explicit OnboardStateResult transitions so the - * runtime remains the durable machine source of truth. Remove this option once - * direct step helpers no longer own machine transitions. - */ - updateMachine?: boolean; -} - -export const LEGACY_MACHINE_STEP_MUTATION_OPTIONS: Readonly = Object.freeze({ - updateMachine: true, -}); - -export const RECORD_ONLY_STEP_MUTATION_OPTIONS: Readonly = Object.freeze({ - updateMachine: false, -}); - -export function shouldUpdateMachine(options: StepMutationOptions | undefined): boolean { - return options?.updateMachine !== false; -} diff --git a/test/gateway-state-reconcile-2276.test.ts b/test/gateway-state-reconcile-2276.test.ts index 72adb6ce150..44436b73a74 100644 --- a/test/gateway-state-reconcile-2276.test.ts +++ b/test/gateway-state-reconcile-2276.test.ts @@ -18,7 +18,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, it } from "vitest"; import { testTimeout } from "./helpers/timeouts"; -const TIMEOUT_MS = testTimeout(20_000); +const TIMEOUT_MS = testTimeout(60_000); const SANDBOX_NAME = "my-assistant"; // Output fixtures that mirror real OpenShell CLI output. diff --git a/test/helpers/onboard-final-flow-phases.ts b/test/helpers/onboard-final-flow-phases.ts index 118881eb387..f074bb759e0 100644 --- a/test/helpers/onboard-final-flow-phases.ts +++ b/test/helpers/onboard-final-flow-phases.ts @@ -103,11 +103,19 @@ export function createRuntimeHarness(initialSession: Session) { markStepStarted: () => cloneSession(session), markStepComplete: (_stepName, updates: SessionUpdates = {}) => updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), - markStepCompleteRecordOnly: (_stepName, updates: SessionUpdates = {}) => - updateSession((current) => Object.assign(current, filterSafeUpdates(updates))), - markStepSkipped: () => cloneSession(session), + markStepSkipped: (stepName) => + updateSession((current) => { + const step = current.steps[stepName]; + if (!step) return current; + if (step.status === "complete" || step.status === "failed" || step.status === "skipped") + return current; + step.status = "skipped"; + step.startedAt = null; + step.completedAt = null; + step.error = null; + return current; + }), markStepFailed: () => cloneSession(session), - markStepFailedRecordOnly: () => cloneSession(session), completeSession: (updates: SessionUpdates = {}) => updateSession((current) => { Object.assign(current, filterSafeUpdates(updates)); diff --git a/test/helpers/onboard-legacy-step-mutation.ts b/test/helpers/onboard-legacy-step-mutation.ts deleted file mode 100644 index 164ee3d75d8..00000000000 --- a/test/helpers/onboard-legacy-step-mutation.ts +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -type LegacyStepMutationModule = { - LEGACY_MACHINE_STEP_MUTATION_OPTIONS: object; -}; - -type LegacyStepSessionModule = { - markStepStarted(stepName: string, options?: object): unknown; - markStepComplete(stepName: string, updates?: unknown, options?: object): unknown; - markStepFailed(stepName: string, message?: string | null, options?: object): unknown; -}; - -export function markStepStartedLegacy( - session: SessionModule, - stepMutation: LegacyStepMutationModule, - stepName: string, -): ReturnType { - return session.markStepStarted( - stepName, - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS, - ) as ReturnType; -} - -export function markStepCompleteLegacy( - session: SessionModule, - stepMutation: LegacyStepMutationModule, - stepName: string, - updates?: Parameters[1], -): ReturnType { - return session.markStepComplete( - stepName, - updates, - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS, - ) as ReturnType; -} - -export function markStepFailedLegacy( - session: SessionModule, - stepMutation: LegacyStepMutationModule, - stepName: string, - message: string | null = null, -): ReturnType { - return session.markStepFailed( - stepName, - message, - stepMutation.LEGACY_MACHINE_STEP_MUTATION_OPTIONS, - ) as ReturnType; -} diff --git a/test/repro-1751-extra.test.ts b/test/repro-1751-extra.test.ts index 74912a8b917..9d7fe72e27c 100644 --- a/test/repro-1751-extra.test.ts +++ b/test/repro-1751-extra.test.ts @@ -19,12 +19,6 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import * as session from "../src/lib/state/onboard-session"; -import * as stepMutation from "../src/lib/state/onboard-step-mutation"; -import { - markStepCompleteLegacy, - markStepFailedLegacy, - markStepStartedLegacy, -} from "./helpers/onboard-legacy-step-mutation"; const tmpHomes: string[] = []; @@ -43,7 +37,7 @@ afterEach(() => { describe("GPU passthrough session persistence (#1751)", () => { it("filterSafeUpdates: gpuPassthrough=true is propagated to safe", () => { session.saveSession(session.createSession()); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { gpuPassthrough: true }); + session.markStepComplete("provider_selection", { gpuPassthrough: true }); const loaded = session.loadSession()!; expect(loaded.gpuPassthrough).toBe(true); }); @@ -51,7 +45,7 @@ describe("GPU passthrough session persistence (#1751)", () => { it("filterSafeUpdates: gpuPassthrough=false is propagated to safe", () => { const s = session.createSession({ gpuPassthrough: true }); session.saveSession(s); - markStepCompleteLegacy(session, stepMutation, "provider_selection", { gpuPassthrough: false }); + session.markStepComplete("provider_selection", { gpuPassthrough: false }); const loaded = session.loadSession()!; expect(loaded.gpuPassthrough).toBe(false); }); @@ -68,7 +62,7 @@ describe("GPU passthrough session persistence (#1751)", () => { // Garbage shapes: string, number, null. None should clobber the existing true. const garbageValues: unknown[] = ["yes", 1, null, undefined, "true"]; for (const v of garbageValues) { - markStepCompleteLegacy(session, stepMutation, "provider_selection", { + session.markStepComplete("provider_selection", { gpuPassthrough: v as unknown as boolean, }); const loaded = session.loadSession()!; @@ -93,12 +87,12 @@ describe("GPU passthrough session persistence (#1751)", () => { it("completeSession persists gpuPassthrough via filterSafeUpdates", () => { session.saveSession(session.createSession()); - markStepStartedLegacy(session, stepMutation, "preflight"); - markStepCompleteLegacy(session, stepMutation, "preflight", { gpuPassthrough: true }); - markStepStartedLegacy(session, stepMutation, "gateway"); - markStepCompleteLegacy(session, stepMutation, "gateway"); - markStepStartedLegacy(session, stepMutation, "sandbox"); - markStepCompleteLegacy(session, stepMutation, "sandbox"); + session.markStepStarted("preflight"); + session.markStepComplete("preflight", { gpuPassthrough: true }); + session.markStepStarted("gateway"); + session.markStepComplete("gateway"); + session.markStepStarted("sandbox"); + session.markStepComplete("sandbox"); session.completeSession({ gpuPassthrough: true }); const loaded = session.loadSession()!; expect(loaded.gpuPassthrough).toBe(true); @@ -113,8 +107,8 @@ describe("GPU passthrough session persistence (#1751)", () => { it("markStepFailed preserves gpuPassthrough", () => { session.saveSession(session.createSession({ gpuPassthrough: true })); - markStepStartedLegacy(session, stepMutation, "gateway"); - markStepFailedLegacy(session, stepMutation, "gateway", "test failure"); + session.markStepStarted("gateway"); + session.markStepFailed("gateway", "test failure"); const loaded = session.loadSession()!; expect(loaded.gpuPassthrough).toBe(true); expect(loaded.steps.gateway?.status).toBe("failed"); diff --git a/test/source-architecture.test.ts b/test/source-architecture.test.ts index da17152e5e0..379d3283210 100644 --- a/test/source-architecture.test.ts +++ b/test/source-architecture.test.ts @@ -20,6 +20,19 @@ function writeModule(root: string, file: string, source: string): void { fs.writeFileSync(destination, source); } +function listProductionTypeScriptFiles(directory: string): string[] { + return fs.readdirSync(directory, { withFileTypes: true }).flatMap((entry) => { + const entryPath = path.join(directory, entry.name); + return entry.isDirectory() + ? entry.name === "__test-helpers__" + ? [] + : listProductionTypeScriptFiles(entryPath) + : entry.isFile() && entry.name.endsWith(".ts") && !entry.name.endsWith(".test.ts") + ? [entryPath] + : []; + }); +} + function budget(overrides: Partial = {}): SourceArchitectureBudget { return { fanIn: { defaultMax: 10 }, @@ -42,6 +55,20 @@ describe("source architecture budget (#7692)", () => { expect(evaluateSourceArchitectureBudget(report, parsed)).toEqual([]); }); + // source-shape-contract: compatibility -- Removed step-mutation APIs must stay absent so step helpers cannot persist machine transitions + test("keeps removed step mutation APIs out of production source (#7703)", () => { + const productionSources = ["src/lib/onboard", "src/lib/state"] + .flatMap((directory) => listProductionTypeScriptFiles(path.join(REPO_ROOT, directory))) + .map((file) => fs.readFileSync(file, "utf8")); + + expect(productionSources.join("")).not.toMatch( + /LEGACY_MACHINE_STEP_MUTATION_OPTIONS|RecordOnly|onboard-step-mutation/, + ); + expect(fs.existsSync(path.join(REPO_ROOT, "src/lib/state/onboard-step-mutation.ts"))).toBe( + false, + ); + }); + test("rejects a new runtime cycle and names its files", ({ resources }) => { const root = resources.temporaryDirectory("nemoclaw-architecture-cycle-"); writeModule(root, "src/a.ts", 'import "./b";\n');