fix(temporal): resolve boundary type names as strings (no leading colon for namespace-less)#352
Merged
Merged
Conversation
skhaz
changed the title
Why: Spawning a workflow whose registry ID has no namespace (e.g. an external, PHP-hosted workflow) recorded the Temporal WorkflowType as ":PopulateItemRelation" instead of "PopulateItemRelation". registry.ID.String() renders a namespace-less ID in its canonical ":name" form, and the Temporal layer used that string directly as the external workflow/activity type name. What: Add registry.ID.Qualified(), which returns "name" when the namespace is empty and "ns:name" otherwise (never a leading colon), mirroring MarshalJSON. Switch every Temporal type-name derivation -- both dispatch (start) and registration -- from String() to Qualified(), so namespaced names are unchanged while namespace-less names lose the spurious colon symmetrically on both sides.
…name option Why: Temporal workflow/activity type names are plain strings that only sometimes coincide with a registry ID. Deriving them by stringifying registry.ID ad-hoc across the dispatch paths coupled the boundary to ID semantics (the source of the leading-colon bug) and offered no way to name an external workflow whose type string is not a registry entry. What: Add a single boundary-name resolver (service/temporal/options/name.go): WorkflowName/ActivityName prefer an explicit "workflow.name"/"activity.name" option (with temporal.* legacy alias) and fall back to id.Qualified(); WorkflowNameFromID/ActivityNameFromID cover sites with no options bag. Route every dispatch site (host spawn, child spawn/exec, workflow exec, activity dispatch, continue-as-new) through the resolver, demoting registry.ID to a naming fallback while keeping it as the local-path key on Start.Source. Add the symmetric MetaActivityName registration override mirroring MetaWorkflowName, so an activity definition can declare a string name; default stays Qualified().
Why: The colon-free type-name derivation was added as a registry.ID.Qualified() method, putting Temporal-boundary concerns on the generic registry value type. What: Remove registry.ID.Qualified() (revert api/registry/id.go and id_test.go to their original form) and move the logic into an unexported qualify() in service/temporal/options/name.go, used by the WorkflowName/ActivityName resolvers. The workflow and activity listeners now derive their fallback name via temporaloptions.WorkflowNameFromID/ActivityNameFromID. Behaviour is identical (bare name when the namespace is empty, "ns:name" otherwise).
Why: Five temporal app-test fixtures passed a Message? value where payload_data expects a non-optional Message, a latent Luau type error. It never surfaced because CI always sets SKIP_TEMPORAL_TESTS, but it blocks the whole temporal app suite from type-checking once the suite is enabled. What: Narrow the value at the 13 call sites with `msg as Message`, the cast idiom already used elsewhere in these same files. Type-check only; no behaviour change.
Why: The workflow type-name behaviour had only Go-level coverage; the real Lua application surface (process:spawn through a Temporal worker) was untested. What: Add a named_hello_workflow registered under an explicit string name (meta.temporal.workflow.name) and a workflow_custom_name test that, via spawn_monitored against the live worker, proves (a) a bare custom name dispatches without a leading colon and (b) the workflow.name option overrides the source id. Verified green against a real Temporal dev server (temporal suite 41/41, app suite 423/423).
skhaz
force-pushed
the
fix/temporal-workflow-type-name
branch
from
3547d9e to
a4d5a1d
Compare
Contributor
|
looks good |
wolfy-j
approved these changes
Jun 16, 2026
wolfy-j
added a commit
that referenced
this pull request
Jun 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Spawning a workflow whose registry ID has no namespace (e.g. an external, PHP-hosted workflow) recorded the Temporal
WorkflowTypeas":PopulateItemRelation"instead of"PopulateItemRelation". Temporal type names are plain strings that only sometimes coincide with a registry ID; the runtime derived them by stringifyingregistry.IDad-hoc across the dispatch paths, so a namespace-less ID rendered in its canonical:nameform leaked into the type name. There was also no first-class way to name an external workflow.What
service/temporal/options/name.go) owns the rule: prefer an explicit name, else the qualified id (namewhen the namespace is empty,ns:nameotherwise — never a leading colon). The qualify logic lives entirely in the temporal options package;registry.IDis untouched.workflow.name/activity.namespawn options (withtemporal.*legacy aliases) and aMetaActivityNameregistration override mirroring the existingMetaWorkflowName.registry.IDstays the carrier onStart.Source(the local process path needs it as a key) but is demoted to a naming fallback.tests/appexercise the real Lua surface: a workflow registered under an explicit string name, spawned by a bare custom name and via theworkflow.nameoption override.Message?->Messageat 13 call sites in 5 pre-existing temporal app fixtures (latent Luau type error that blocked the temporal app suite from loading; unrelated to naming).Testing
make build-wippy+go build ./...green;golangci-lint0 issues;go test -race ./service/temporal/... ./api/registry/green.workflow.nameoverride and namespace-less default ->PopulateItemRelation); workflow + activity listener register/unregister symmetry with a custom name. gremlins on the resolver: 100% killed.workflow_custom_nametest (bare-name dispatch + option override end-to-end).