fix(fs/embed): harden runtime pack update lifecycle#354
Merged
Conversation
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).
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.
Follow-up hardening for the fs/embed live install/update path after #353.
What changed:
Verification: