Skip to content

Fail the workflow task when the test server refuses a command instead of stranding it - #3097

Open
sangkyoonnam wants to merge 2 commits into
temporalio:mainfrom
sangkyoonnam:fix/3088-refused-completion-stays-started
Open

sangkyoonnam wants to merge 2 commits into
temporalio:mainfrom
sangkyoonnam:fix/3088-refused-completion-stays-started

Conversation

@sangkyoonnam

@sangkyoonnam sangkyoonnam commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

What was changed

  • TestWorkflowMutableStateImpl#completeWorkflowTask restores the state machines and the id maps when command processing throws. CompletionSnapshot captures the timer, activity, child, Nexus, external signal/cancel and update maps plus every state machine's transition count before the completion, and puts them back on failure (new StateMachine#transitionCount / rollbackTo). The in-memory task then matches the history that still shows it as STARTED, and a timer or activity created by a command ahead of the refused one is gone.
  • If the refusal was a caller error (INVALID_ARGUMENT or FAILED_PRECONDITION) for a command that has a failure cause, a second update records WorkflowTaskFailed with that cause and schedules a new task. From the second attempt on the failure is dropped and the task times out, as the search-attribute path already does. The worker gets INVALID_ARGUMENT with <Cause>: <reason>. RefusedCommand maps command types to the real server's causes: BAD_*_ATTRIBUTES, and START_TIMER_DUPLICATE_ID / SCHEDULE_ACTIVITY_DUPLICATE_ID for the duplicate-id refusals the test server reports as FAILED_PRECONDITION.
  • processRequestCancelActivityTask accepts a cancel for an activity that finished while the task was in flight. It records ActivityTaskCancelRequested and nothing else; the buffered finish event follows in history, as on the real server.
  • External signal and cancellation dispatch, search-attribute upserts and memo changes run in ctx.onCommit, so a completion refused by a later command never applies them. A ProtocolMessage command that is refused (or references a missing message) fails the task with BAD_UPDATE_WORKFLOW_EXECUTION_MESSAGE, as the real server does. The update that records the failure checks it is still the refused task (same scheduledEventId, still STARTED) under the workflow lock before it changes anything.
  • RefusedCommandTest covers: cancel of an unknown timer, cancel of an unknown activity, ScheduleActivityTask without a task queue, a duplicate timer id, a cancel of an activity that completed during the task, and a refused completion whose earlier StartTimer must not break the worker's replay.

Why?

completeWorkflowTask moved the workflow task state machine to NONE and only then processed commands. When a handler threw, the RequestContext was discarded but the state machine stayed at NONE, so timeoutWorkflowTask returned early. Nothing failed the task and nothing redelivered it; the run stayed RUNNING forever. #3088 hits this with CancelTimer from the TypeScript SDK and a second user with RequestCancelActivityTask from the Python SDK. Every handler that throws after the transition has the same effect.

The real server validates each command and fails the workflow task with the command's cause (handleCommandCancelTimer and the rest in workflow_task_completed_handler.go, through failWorkflowTaskOnInvalidArgument), then returns InvalidArgument with the cause message. This change gives the test server that contract without touching each handler.

Two things I found on the way. Rolling back only the workflow task state machine is not enough: with a valid StartTimer ahead of the refused command the timer stayed in memory, and the worker's replay of that StartTimer was rejected as a duplicate on every attempt, hence the snapshot. And the activity race is real: an activity that completes while a task is running leaves the in-memory map at once, so the worker's cancel for it was refused and stranded the task. The timer side of that race (a TimerFired still buffered) is #3060; with this PR it fails the task with BAD_CANCEL_TIMER_ATTRIBUTES instead of hanging, and #3060 makes it succeed, so the two land in either order.

Checklist

  1. Closes Test server: refused CancelTimer leaves the workflow task complete in memory but STARTED in history, so it is never timed out or redelivered #3088

  2. How was this tested:
    ./gradlew :temporal-test-server:test --tests "io.temporal.testserver.functional.RefusedCommandTest". I ran the new tests on main without the change: the unknown-timer case leaves history at [WORKFLOW_EXECUTION_STARTED, WORKFLOW_TASK_SCHEDULED, WORKFLOW_TASK_STARTED] with no failure or redelivery, the unknown-activity case returns FAILED_PRECONDITION and strands the task the same way, and the finished-activity case is refused with FAILED_PRECONDITION: ACTIVITY_UNKNOWN for scheduledEventId=5. With the change all six pass. The full temporal-test-server suite passes (99 passed, 1 skipped), and the temporal-sdk activity, timer, cancellation, child-workflow, signal and update tests run against this server pass (273 passed, 9 skipped). After the second commit: the three added tests (search attributes and memo, external signal, unknown protocol message) fail on the first commit's source and pass with it, RefusedCommandTest is 9/9, the full temporal-test-server suite is 102 passed, 1 skipped, and the temporal-sdk signal, cancel-external, search-attribute, memo and update tests are 112 passed, 8 skipped.

Not covered: this is not a complete rollback of mutable state. Fields written by transition callbacks are not copied, and other completion effects such as continue-as-new and completion callbacks are outside this change. Messages processed without a protocol command and malformed messages are not covered by the failure mapping. The tests resubmit commands over raw RPC rather than running SDK replay. The buffered timer/cancel race is #3060, and the reporter's second observation (time skipping slowing after terminate with a task and activity in flight) is a separate path. There is no regression test for a refused external cancellation or for a message refused inside processMessage; the follow-up tests cover a refused external signal, search attributes and memo, and a message with an unknown id.

  1. Any docs updates needed?
    No.

@sangkyoonnam
sangkyoonnam requested a review from a team as a code owner September 24, 2026 12:01
@sangkyoonnam

Copy link
Copy Markdown
Contributor Author

Pushed f206a60 for three gaps I found while re-checking the rollback. External signals and cancellations, search-attribute upserts and memo changes were applied before the completion committed, so a command refused later in the same completion rolled back the state machines but not those effects. They now run in ctx.onCommit. Refused update messages had no failure cause and fell back to the timeout path. They now fail the task with BAD_UPDATE_WORKFLOW_EXECUTION_MESSAGE, including a command that references a missing message. And the follow-up update that fails the task now checks it is still the refused task before touching anything.

Tests cover the search-attribute/memo case, the external signal case and the unknown-message case. I don't have a deterministic test for the stale-update race. The body now spells out what the rollback does not cover.

This branch has not been deployed

No deployments
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.

Test server: refused CancelTimer leaves the workflow task complete in memory but STARTED in history, so it is never timed out or redelivered

1 participant