Skip to content

server-zd and SDKs: acknowledge SubFlow starts on completion; reject SubFlows from dashed Step types - #1574

Merged
longquanzheng merged 6 commits into
mainfrom
fix/subflow-start-identity
Nov 11, 2026
Merged

longquanzheng merged 6 commits into
mainfrom
fix/subflow-start-identity

Conversation

@longquanzheng

Copy link
Copy Markdown
Contributor

Summary

Two SubFlow start fixes in server-zd, plus the matching SDK checks.

  • Child start acknowledged on completion. When the parent applies a child's completion, it marks the parent's matching unacknowledged ChildStart outbox entry (same ParentFlowReference, including run attempt) as acknowledged in the same commit (cross_flow/outbox.rs, inbox.rs). No persisted format changes.
  • SubFlows from a Step type that contains - are rejected. The child Flow ID is SubFlow:{parent}-{StepType}-{n}-{i}, so a - in the Step type makes the ID ambiguous across parents. The server rejects a WaitFor result whose SubFlow condition comes from such a Step type as an invalid Worker response (non-retryable attempt failure; cross_flow/outbox.rs:209, called from steps/step_results.rs:879). All five SDKs raise the error locally before sending the result.

Why

Found while auditing the ID reuse policies after #1551.

  • Without the acknowledgment, this sequence starts a second child run: the child commits its start; the parent's leader stops before OutboxAck; the new leader's redelivery keeps getting Unavailable from the child's partition while the child completes and reports; retention purges the child; the redelivery finally lands and starts a new child. After this change, every path that re-reads the outbox after the completion (rebuild, re-prepare, a later leader) skips the entry. A redelivery already retrying in memory with a prebuilt command is not closed; the owner decided not to fix that remaining window.
  • The collision is reachable: parent a-b with Step type C, and parent a with Step type b-C, both derive SubFlow:a-b-C-1-0, and the second attaches to the first's child. Step types with - need a custom name (GetStepType / get_step_type and equivalents); the server allows them. The owner chose to reject this case rather than change the published child ID format.

Compatibility

Breaking, authorized by the owner: a Step type whose name contains - can no longer start a SubFlow. Migration: rename the Step type. Results that older servers already accepted and recorded replay unchanged. Recorded in the server changelog (unreleased 0.1.26) and the SDK 1.44 change log entries, with "Changed in 1.44.0" in the SDK API docs. The rule is stable, so the SubFlow concept page (English and Simplified Chinese) and skills/dex-sdk/references/core/subflow.md state it; skills PATCH bump to 1.0.12.

StepAttemptFailed's MismatchedResult kind was renamed to the more general InvalidResult inside apply to keep step_results.rs under the R1 size limit (internal name only).

Testing

  • server-zd/crates/dexzd-apply/tests/sub_flow_id_collision.rs: a dashed Step type starting a SubFlow is rejected (error text asserted); a dashed Step type waiting only on a Timer works; plain Step types with dashed parent Flow IDs get distinct children.
  • child_completion_acknowledges_the_parent_child_start_before_the_child_can_be_purged: fails without the acknowledgment, passes with it. always_restart_old_child_result_does_not_complete_new_attempt_wait setup copies attempt 1's attempt record to attempt 2 (the new ack writes timer rows and triggers a checkpoint); assertions unchanged.
  • dexzd-apply (all targets, --no-fail-fast), dexzd-simulation (pins unchanged), dexzd-engine: pass. cargo fmt --all --check, cargo clippy -p dexzd-apply --all-targets -- -D warnings, make rust-rules-check: pass.
  • SDK unit tests for the local rejection: Go (make unitTests, go vet), Python (710 unit tests), Java (gradlew test checkstyle javadoc), TypeScript (typecheck, 222 tests), Rust (clippy, 144 tests): pass. SDK integration tests that need a server: CI.
  • cd docs && npm run check: pass. Pages read in a browser, English and Simplified Chinese: the SubFlow concept page (changed section).

A child purge deletes the child's inbox deduplication rows and its Flow
directory entry. If the parent's ChildStart outbox entry is still
unacknowledged at that point (its OutboxAck was lost and redelivery is
delayed), a late redelivery starts a second child run, or attaches to a
newer one.

A child reports only to parents whose start it committed, and its purge
waits until that report is acknowledged. The parent now acknowledges
its pending ChildStart entries carrying the reporting parent reference
in the same commit that applies the ChildCompleted message, so
redelivery stops before the child can be purged. The per-message
acknowledgment is shared with OutboxAck.

Also add sub_flow_id_collision.rs, a known-failing reproduction:
parents `a-b` (Step `C`) and `a` (Step `b-C`) both derive the child
Flow ID `SubFlow:a-b-C-1-0`, and the second parent attaches to the
first parent's child. It stays failing until a fix is chosen, because
the ID format is persisted and computed by every SDK.
A child Flow ID is SubFlow:<parent Flow ID>-<Step execution ID>-<index>,
and a Step execution ID is <Step type>-<n>. A dash inside the Step type
makes the ID ambiguous: parent a-b at Step type C and parent a at Step
type b-C both produce SubFlow:a-b-C-1-0, so the second parent attached
to the first parent's child and received its result.

The owner chose to forbid the input instead of changing the persisted
and published ID format. This is an authorized breaking change:

- server-zd apply turns a WaitFor result that registers a SubFlow
  condition from a dashed Step type into a non-retryable InvalidResponse
  attempt failure that names the Step type and the rule. Recorded
  results replay as recorded. The MismatchedResult outcome becomes the
  more general InvalidResult.
- The Go, Python, Java, TypeScript, and Rust SDKs fail the WaitFor
  result locally before sending it, and document the rule on the
  SubFlow API and the custom Step type name API (Changed in 1.44.0).
- sub_flow_id_collision.rs now asserts the rejection; Step types with a
  dash may still wait on Timers.
- Docs: server and SDK Developer reference sections, Breaking change
  log entries, the SubFlow concept page in both locales, and the
  dex-sdk SubFlow skill rule (skills 1.0.12).
@codecov

codecov Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.32394% with 9 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
sdk-typescript/src/worker-dispatcher.ts 60.00% 6 Missing ⚠️
sdk-python/dex/_worker_dispatcher.py 60.00% 1 Missing and 1 partial ⚠️
sdk-rust/crates/dex-sdk/src/sub_flow.rs 91.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Review of the dashed Step type rule found two problems.

The SDKs rejected the WaitFor result locally. That surfaced as a Worker
error, which the engine retries under the Step's retry policy, so the
attempt never reached the server's non-retryable InvalidResponse path
that the docs promise. The five SDKs now send the result unchanged and
log a warning that names the Step type; server-zd apply stays the one
authority. Their tests now assert the result is still sent.

SDK 1.44.0 shipped without the change, so its doc markers move to
1.45.0 and its change log entries to new 1.45 sections.

Tests: the dashed Step test asserts that no retry is scheduled under
the default retry policy. The AlwaysRestart fixture builds a real
attempt 2 record and asserts that a child report leaves another
attempt's start and another child's start pending.
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.

1 participant