Skip to content

Fix interceptor contract inconsistency for start_update_with_start_workflow#1588

Merged
tconley1428 merged 3 commits into
temporalio:mainfrom
brucearctor:fix/interceptor-rpc-metadata-1582
Jun 11, 2026
Merged

Fix interceptor contract inconsistency for start_update_with_start_workflow#1588
tconley1428 merged 3 commits into
temporalio:mainfrom
brucearctor:fix/interceptor-rpc-metadata-1582

Conversation

@brucearctor

@brucearctor brucearctor commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Fixes #1582.

StartWorkflowUpdateWithStartInput was the only OutboundInterceptor input dataclass missing top-level rpc_metadata and rpc_timeout fields, breaking interceptors that generically set metadata on every outbound call.

Additionally, _ClientImpl._start_workflow_update_with_start() in temporalio/client/_impl.py called execute_multi_operation() without passing metadata= or timeout=, silently dropping any RPC metadata.

Changes

temporalio/client/_interceptor.py

  • Added rpc_metadata: Mapping[str, str | bytes] and rpc_timeout: timedelta | None fields to StartWorkflowUpdateWithStartInput
  • Expanded the docstring to document that these top-level fields are authoritative for the gRPC call, while the sub-input fields exist for interceptor introspection only

temporalio/client/_client.py

  • Populated the new fields at the construction site in Client._start_update_with_start()

temporalio/client/_impl.py

tests/worker/test_update_with_start.py

  • Added test_update_with_start_rpc_metadata_and_timeout_forwarded which verifies:
    • An interceptor can set rpc_metadata via the new top-level field
    • Caller-provided rpc_metadata is preserved
    • rpc_timeout is forwarded to the gRPC call

Backward Compatibility

  • StartWorkflowUpdateWithStartInput is only constructed internally (in Client._start_update_with_start), not by external callers
  • Existing interceptors (OpenTelemetry, LangSmith) don't touch rpc_metadata on this method — they manipulate headers. No changes needed to them
  • The sub-input classes retain their own rpc_metadata fields for introspection

Testing

  • All 33 tests in tests/worker/test_update_with_start.py pass
  • pyright: 0 errors, 0 warnings
  • ruff + pydocstyle: clean

@brucearctor brucearctor requested a review from a team as a code owner June 8, 2026 01:00
@brucearctor brucearctor force-pushed the fix/interceptor-rpc-metadata-1582 branch 2 times, most recently from 572c339 to 91e26f1 Compare June 8, 2026 01:08
@brucearctor
Fix interceptor contract inconsistency for start_update_with_start_wo…
a66bda4 
…rkflow

Add top-level rpc_metadata and rpc_timeout fields to
StartWorkflowUpdateWithStartInput, making it consistent with every other
OutboundInterceptor input dataclass. Previously this composite input
lacked these fields, forcing interceptors to special-case it.

Also fix the _ClientImpl to actually pass rpc_metadata and rpc_timeout
to the execute_multi_operation gRPC call, which were previously silently
dropped.

Add a test verifying that rpc_metadata set by an interceptor on
StartWorkflowUpdateWithStartInput is forwarded to the gRPC call.

Fixes temporalio#1582
@brucearctor brucearctor force-pushed the fix/interceptor-rpc-metadata-1582 branch from 91e26f1 to a66bda4 Compare June 8, 2026 01:14
@brucearctor brucearctor mentioned this pull request Jun 8, 2026
Closed
@tconley1428

tconley1428 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

I think we should also remove the rpc metadata from the child interceptor objects, since they aren't actually used when it gets to the underlying multi_operation RPC. This will be a breaking change for a tiny number of folks we'll have to call out in release notes. If you'd like to address that, you can, otherwise I will end up making the change.

@brucearctor
Merge branch 'main' into fix/interceptor-rpc-metadata-1582
c70d68c
@brucearctor
Remove unused rpc_metadata/rpc_timeout from child interceptor inputs
2cfe041 
Remove rpc_metadata and rpc_timeout fields from
UpdateWithStartUpdateWorkflowInput and UpdateWithStartStartWorkflowInput.
These fields were never forwarded to the underlying execute_multi_operation
gRPC call — only the top-level StartWorkflowUpdateWithStartInput fields
are authoritative.

Also remove the corresponding parameters from WithStartWorkflowOperation
since they only served to populate the (now-removed) child input fields.

This is a breaking change for interceptors that accessed rpc_metadata or
rpc_timeout on the child input objects.
@brucearctor

brucearctor commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

I think we should also remove the rpc metadata from the child interceptor objects, since they aren't actually used when it gets to the underlying multi_operation RPC. This will be a breaking change for a tiny number of folks we'll have to call out in release notes. If you'd like to address that, you can, otherwise I will end up making the change.

Was 2cfe041 what you had in mind @tconley1428 ?

@tconley1428

tconley1428 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Looks like it to me, thanks!

@tconley1428 tconley1428 merged commit b9b2cc3 into temporalio:main Jun 11, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tconley1428 tconley1428 tconley1428 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Interceptor contract inconsistency for OutboundInterceptor.start_update_with_start_workflow

2 participants

@brucearctor @tconley1428