Skip to content

chore(deps): Update xstate to v5.32.4#333

Open
X-Guardian wants to merge 2 commits into
open-constructs:mainfrom
X-Guardian:chore/bump-xstate
Open

chore(deps): Update xstate to v5.32.4#333
X-Guardian wants to merge 2 commits into
open-constructs:mainfrom
X-Guardian:chore/bump-xstate

Conversation

@X-Guardian

Copy link
Copy Markdown
Contributor

Description

Bumps xstate from 4.38.3 to 5.32.4 (latest) in @cdktn/cli-core, the only package that declares it.

This is a major version bump (4.x → 5.x) and required migrating the deploy state machine, its consumer, and its tests to the reworked v5 API:

  • createMachine<Ctx, Evt, State>(…)setup({ types, actors }).createMachine(…). The generic-parameter form was replaced by the setup() builder with a types block, in deploy-machine.ts. The predictableActionArguments flag was dropped (predictable ordering is the default in v5) and services became actors.
  • Invoked callback service → fromCallback. The v4 (context, event) => (send, onReceive) => {…} factory became fromCallback(({ sendBack, receive, input }) => …). Since v5 invoked actors receive input rather than the triggering event, the spawn config now flows in through invoke.input. The actor is built by makeTerraformPtyService(spawn) so tests can inject a mock pty.
  • send(…, { to })sendTo(…); assign<Ctx, Evt>assign. Action creators were split and the assign implementation signature changed to a single argument.
  • raise(…)sendTo(({ self }) => self, …). The two internally re-emitted events (EXITED on a missing variable, and the OVERRIDE_REJECTED_EXTERNALLY re-label of an external discard) were sent to self rather than raised. Raised events are internal micro-steps and do not surface through the inspection API, so the consumer would otherwise stop observing them.
  • interpret(…)createActor(…, { inspect }). The actor is now started explicitly before events are sent.

The consumer in terraform-cli.ts needed the most care, because v5 snapshots no longer expose the triggering event (state.event):

  • Transitions now come from the actor's own service.subscribe(…), which yields a fully-typed snapshot for the root actor only — replacing service.onTransition(…).
  • Events come from the inspect callback (the only source that still carries the triggering event), replacing service.onEvent(…). The root actor is identified by reference so the invoked pty child's events are ignored, and events are narrowed through the existing isDeployEvent guard.
  • The last EXITED exit code is now remembered from inspection to decide whether the run failed, since it is no longer readable off the final snapshot.
  • waitFor is imported from xstate and its predicate uses snapshot.status === "done"; service.send("X") calls became service.send({ type: "X" }).

The tests in deploy-machine.test.ts were migrated to match: interpret().onTransitioncreateActor().subscribe, withConfig({ services })provide({ actors }), machine.transitiongetNextSnapshot, and the state.event assertions rebuilt on the inspection API.

Checklist

  • I have updated the PR title to match CDKTN's style guide
  • I have run the linter on my code locally
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if applicable
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works if applicable
  • New and existing unit tests pass locally with my changes

@X-Guardian
X-Guardian marked this pull request as ready for review July 14, 2026 10:52
@X-Guardian
X-Guardian requested a review from a team as a code owner July 14, 2026 10:52
@sakul-learning

sakul-learning commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

I validated a non-blocking ordering regression in TerraformCli.handleService(): createAndStartDeployService() starts the XState actor and sends START before handleService() attaches service.subscribe(). A silent process that exits immediately can therefore miss the running callback ([] rather than ["running"]); ordinary Terraform output produces a later snapshot that reports it, so I would not block this XState update on that behavior.

What the repro shows

createAndStartDeployService() starts the actor and sends START before handleService() subscribes to snapshots. A silent process that exits immediately therefore never reports the initial running callback:

jest.mocked(spawnInteractive).mockReturnValue({
  actions: {
    write: jest.fn(),
    writeLine: jest.fn(),
    stop: jest.fn(),
  },
  exitCode: Promise.resolve(0),
});

const states: string[] = [];
await cli.deploy({}, (state) => states.push(state.type));

expect(states).toEqual(["running"]);

On this PR head, the assertion fails with received: [].

This is technically an observable behavior regression from v4, but normal Terraform apply/destroy invocations emit stdout/stderr. Those events produce a later snapshot while the machine is still in running.processing; because previousState is still idle, that later snapshot triggers the running callback and masks the missed initial transition. I also verified that output emitted during actor startup is observed, and that an immediate non-zero exit is still propagated correctly.

So I see this as a non-blocking lifecycle hardening opportunity, rather than evidence of a current customer-facing failure. The practical difference for a real Terraform run is that running is reported on its first output instead of immediately on entry. The silent/immediate process is useful for exposing the ordering, but is not representative enough to justify blocking the dependency update by itself.

Minimal fix validated locally

Processing the actor's current snapshot once after subscribing restores the initial callback without changing the factory API:

const handleSnapshot = (
  snapshot: ReturnType<typeof service.getSnapshot>,
) => {
  // existing transition handling
};

service.subscribe(handleSnapshot);

// The actor was already started by createService, so consume the
// snapshot whose transition may have preceded the subscription.
handleSnapshot(service.getSnapshot());

I pushed the TDD sequence as two commits:

  1. RED — repro test: sakul-learning@8bcd827
  2. GREEN — minimal fix: sakul-learning@e4f72c3

Branch: https://github.com/sakul-learning/cdk-terrain/tree/review/pr-333-xstate-running-repro

Validation:

  • RED commit: focused suite fails exactly one test, reports running before a silent process exits (1 failed, 5 passed).
  • GREEN commit: focused suite passes (6/6).
  • Full @cdktn/cli-core suite with the fix: 163 passed; 30 existing dist-dependent tests skipped.
  • Prettier and git diff --check: pass.

Given the realistic execution path and green existing CI, this is a suggestion rather than a requested change.

@so0k so0k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Up to you if you want to address the non-block ordering concern (which may not be a real failure mode) - still need to rebase for conflict resolution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants