RegisterDynamicWorkflow accepts a WorkflowDefinitionFactory but panics on execution

[datashaman]

Expected Behavior

A dynamic (catch-all) workflow registered with a WorkflowDefinitionFactory should execute via NewWorkflowDefinition(), exactly like a statically-registered factory. worker.RegisterDynamicWorkflow already special-cases and stores a factory, so it should work end-to-end (both on real workers and in testsuite).

Actual Behavior

Registration succeeds, but the first workflow task for a dynamically-dispatched type panics inside the SDK:

reflect: call of reflect.Value.Call on ptr Value      // pointer factory (e.g. roadrunner)
reflect: call of reflect.Value.Call on struct Value   // value factory (see repro below)

On a real worker this surfaces as a WorkflowTaskFailed (cause WORKFLOW_TASK_FAILED_CAUSE_WORKFLOW_WORKER_UNHANDLED_FAILURE) and the execution never progresses; in testsuite it fails the workflow.

Root Cause

registry.RegisterDynamicWorkflow stores a factory as-is (internal/internal_worker.go, ~L709):

if factory, ok := wf.(WorkflowDefinitionFactory); ok {
    r.dynamicWorkflow = factory
    r.dynamicWorkflowOptions = options
    return
}

…but both dispatch paths resolve the "dynamic" sentinel to the stored factory and then wrap it in a reflection-based workflowExecutor without re-checking the WorkflowDefinitionFactory interface:

1. Production — registry.getWorkflowDefinition (internal/internal_worker.go, ~L957):

   var dynamic bool
   if d, ok := wf.(string); ok && d == "dynamic" {
       wf = r.dynamicWorkflow          // the factory…
       dynamic = true
   }
   executor := &workflowExecutor{workflowType: lookup, fn: wf, ...}  // …treated as a func → panic
   return newSyncWorkflowDefinition(executor), nil

2. Test suite — testWorkflowEnvironmentImpl.getWorkflowDefinition (internal/internal_workflow_testsuite.go, ~L681): the same pattern with workflowExecutorWrapper.

The named-workflow path a few lines above does honor the factory (return wdf.NewWorkflowDefinition(), nil); the dynamic path does not. So RegisterDynamicWorkflow and both getWorkflowDefinitions disagree.

Steps to Reproduce the Problem

Runnable with public packages only (no internal access):

package dynamicrepro

import (
	"testing"
	"time"

	"github.com/stretchr/testify/require"
	commonpb "go.temporal.io/api/common/v1"
	"go.temporal.io/sdk/converter"
	"go.temporal.io/sdk/internalbindings"
	"go.temporal.io/sdk/testsuite"
	"go.temporal.io/sdk/workflow"
)

type dynDef struct{ env internalbindings.WorkflowEnvironment }

func (d *dynDef) Execute(env internalbindings.WorkflowEnvironment, _ *commonpb.Header, _ *commonpb.Payloads) {
	d.env = env
}
func (d *dynDef) OnWorkflowTaskStarted(_ time.Duration) {
	name := d.env.WorkflowInfo().WorkflowType.Name
	result, err := converter.GetDefaultDataConverter().ToPayloads(name)
	d.env.Complete(result, err)
}
func (d *dynDef) StackTrace() string { return "" }
func (d *dynDef) Close()             {}

type dynFactory struct{}

func (dynFactory) NewWorkflowDefinition() internalbindings.WorkflowDefinition { return &dynDef{} }

func TestDynamicWorkflowFactory(t *testing.T) {
	var s testsuite.WorkflowTestSuite
	env := s.NewTestWorkflowEnvironment()
	env.RegisterDynamicWorkflow(dynFactory{}, workflow.DynamicRegisterOptions{})

	env.ExecuteWorkflow("pipeline-blog-publish") // any unregistered type

	require.True(t, env.IsWorkflowCompleted())
	require.NoError(t, env.GetWorkflowError())
	var got string
	require.NoError(t, env.GetWorkflowResult(&got))
	require.Equal(t, "pipeline-blog-publish", got)
}

Today this fails with reflect: call of reflect.Value.Call on struct Value. This also reproduces in production through roadrunner-temporal (the PHP SDK host), which registers every workflow as one shared WorkflowDefinitionFactory proxy.

Proposed Fix

In both getWorkflowDefinition implementations, after resolving the dynamic sentinel to the stored factory, re-check the factory interface (mirroring the named path):

if wdf, ok := wf.(WorkflowDefinitionFactory); ok {
    return wdf.NewWorkflowDefinition(), nil
}

Specifications

• Version: go.temporal.io/sdk v1.44.1 (present on main at the referenced lines)
• Platform: SDK-internal; platform-independent (repro'd on darwin/arm64)

Why it matters

This is the one blocker to Dynamic Workflow support in the PHP SDK — PHP and TypeScript are the only SDKs without it. roadrunner-temporal can register its shared factory dynamically, but every dynamic execution panics until this is fixed.