From 189d718e96b4e88ad6f477bbef11e835c32e824a Mon Sep 17 00:00:00 2001
From: Nathan Glass <nagl@stripe.com>
Date: Wed, 27 Oct 2021 11:26:04 -0700
Subject: [PATCH 1/4] Add signal arguments to start_workflow (to support
 signal_with_start)

---
 examples/bin/worker                           |  1 +
 .../integration/signal_with_start_spec.rb     | 67 +++++++++++++++++
 .../workflows/signal_with_start_workflow.rb   | 16 ++++
 lib/temporal/client.rb                        | 65 +++++++++++++----
 lib/temporal/connection/grpc.rb               | 48 +++++++++++-
 .../testing/local_workflow_context.rb         |  6 +-
 lib/temporal/testing/temporal_override.rb     |  4 +-
 spec/unit/lib/temporal/client_spec.rb         | 73 +++++++++++++++++++
 spec/unit/lib/temporal/grpc_client_spec.rb    | 37 ++++++++++
 .../testing/temporal_override_spec.rb         |  8 ++
 10 files changed, 304 insertions(+), 21 deletions(-)
 create mode 100644 examples/spec/integration/signal_with_start_spec.rb
 create mode 100644 examples/workflows/signal_with_start_workflow.rb

diff --git a/examples/bin/worker b/examples/bin/worker
index 65828dfa..497eb9fc 100755
--- a/examples/bin/worker
+++ b/examples/bin/worker
@@ -39,6 +39,7 @@ worker.register_workflow(ReleaseWorkflow)
 worker.register_workflow(ResultWorkflow)
 worker.register_workflow(SerialHelloWorldWorkflow)
 worker.register_workflow(SideEffectWorkflow)
+worker.register_workflow(SignalWithStartWorkflow)
 worker.register_workflow(SimpleTimerWorkflow)
 worker.register_workflow(TimeoutWorkflow)
 worker.register_workflow(TripBookingWorkflow)
diff --git a/examples/spec/integration/signal_with_start_spec.rb b/examples/spec/integration/signal_with_start_spec.rb
new file mode 100644
index 00000000..46799996
--- /dev/null
+++ b/examples/spec/integration/signal_with_start_spec.rb
@@ -0,0 +1,67 @@
+require 'workflows/signal_with_start_workflow'
+
+describe 'signal with start' do
+
+  it 'signals at workflow start time' do
+    workflow_id = SecureRandom.uuid
+    run_id = Temporal.start_workflow(
+      SignalWithStartWorkflow,
+      'signal_name',
+      0.1,
+      signal_name: 'signal_name',
+      signal_input: 'expected value',
+      options: { workflow_id: workflow_id }
+    )
+
+    result = Temporal.await_workflow_result(
+      SignalWithStartWorkflow,
+      workflow_id: workflow_id,
+      run_id: run_id,
+    )
+
+    expect(result).to eq('expected value') # the workflow should return the signal value
+  end
+
+  it 'signals at workflow start time with name only' do
+    workflow_id = SecureRandom.uuid
+    run_id = Temporal.start_workflow(
+      SignalWithStartWorkflow,
+      'signal_name',
+      0.1,
+      signal_name: 'signal_name',
+      options: { workflow_id: workflow_id }
+    )
+
+    result = Temporal.await_workflow_result(
+      SignalWithStartWorkflow,
+      workflow_id: workflow_id,
+      run_id: run_id,
+    )
+
+    expect(result).to eq(nil) # the workflow should return the signal value
+  end
+
+  it 'does not launch a new workflow when signaling a running workflow through signal_with_start' do
+    workflow_id = SecureRandom.uuid
+    run_id = Temporal.start_workflow(
+      SignalWithStartWorkflow,
+      'signal_name',
+      10,
+      signal_name: 'signal_name',
+      signal_input: 'expected value',
+      options: { workflow_id: workflow_id }
+    )
+
+    second_run_id = Temporal.start_workflow(
+      SignalWithStartWorkflow,
+      'signal_name',
+      0.1,
+      signal_name: 'signal_name',
+      signal_input: 'expected value',
+      options: { workflow_id: workflow_id }
+    )
+
+    # If the run ids are the same, then we didn't start a new workflow
+    expect(second_run_id).to eq(run_id)
+  end
+end
diff --git a/examples/workflows/signal_with_start_workflow.rb b/examples/workflows/signal_with_start_workflow.rb
new file mode 100644
index 00000000..dbcb186a
--- /dev/null
+++ b/examples/workflows/signal_with_start_workflow.rb
@@ -0,0 +1,16 @@
+class SignalWithStartWorkflow < Temporal::Workflow
+
+  def execute(expected_signal, sleep_for)
+    received = 'no signal received'
+
+    workflow.on_signal do |signal, input|
+      if signal == expected_signal
+        received = input
+      end
+    end
+
+    # Do something to get descheduled so the signal handler has a chance to run
+    workflow.sleep(sleep_for)
+    received
+  end
+end
diff --git a/lib/temporal/client.rb b/lib/temporal/client.rb
index d2380855..df1dcf84 100644
--- a/lib/temporal/client.rb
+++ b/lib/temporal/client.rb
@@ -13,26 +13,57 @@ def initialize(config)
       @config = config
     end
 
-    def start_workflow(workflow, *input, **args)
+    # Starts a workflow with an optional signal.
+    #
+    # signal_name and signal_input are optional parameters that control Temporal's signal_with_start
+    # behavior.  signal_name corresponds to the 'signal' argument to signal_workflow, and signal_input
+    # corresponds to the 'input' argument to signal_workflow
+    #
+    # If signal_name is specified, Temporal will atomically do one of the following two things:
+    # A) start a new workflow and signal it
+    # B) if workflow_id is specified and the workflow already exists, signal the existing workflow.
+    #
+    # It is legal to specify signal_name without signal_input, but it is illegal to specify signal_input
+    # without signal_name.
+    def start_workflow(workflow, *input, signal_name: nil, signal_input: nil, **args)
       options = args.delete(:options) || {}
       input << args unless args.empty?
 
       execution_options = ExecutionOptions.new(workflow, options, config.default_execution_options)
       workflow_id = options[:workflow_id] || SecureRandom.uuid
 
-      response = connection.start_workflow_execution(
-        namespace: execution_options.namespace,
-        workflow_id: workflow_id,
-        workflow_name: execution_options.name,
-        task_queue: execution_options.task_queue,
-        input: input,
-        execution_timeout: execution_options.timeouts[:execution],
-        # If unspecified, individual runs should have the full time for the execution (which includes retries).
-        run_timeout: execution_options.timeouts[:run] || execution_options.timeouts[:execution],
-        task_timeout: execution_options.timeouts[:task],
-        workflow_id_reuse_policy: options[:workflow_id_reuse_policy],
-        headers: execution_options.headers
-      )
+      if signal_name.nil? && signal_input.nil?
+        response = connection.start_workflow_execution(
+          namespace: execution_options.namespace,
+          workflow_id: workflow_id,
+          workflow_name: execution_options.name,
+          task_queue: execution_options.task_queue,
+          input: input,
+          execution_timeout: execution_options.timeouts[:execution],
+          # If unspecified, individual runs should have the full time for the execution (which includes retries).
+          run_timeout: compute_run_timeout(execution_options),
+          task_timeout: execution_options.timeouts[:task],
+          workflow_id_reuse_policy: options[:workflow_id_reuse_policy],
+          headers: execution_options.headers
+        )
+      else
+        raise ArgumentError, 'If signal_input is provided, you must also provide signal_name' if signal_name.nil?
+
+        response = connection.signal_with_start_workflow_execution(
+          namespace: execution_options.namespace,
+          workflow_id: workflow_id,
+          workflow_name: execution_options.name,
+          task_queue: execution_options.task_queue,
+          input: input,
+          execution_timeout: execution_options.timeouts[:execution],
+          run_timeout: compute_run_timeout(execution_options),
+          task_timeout: execution_options.timeouts[:task],
+          workflow_id_reuse_policy: options[:workflow_id_reuse_policy],
+          headers: execution_options.headers,
+          signal_name: signal_name,
+          signal_input: signal_input
+        )
+      end
 
       response.run_id
     end
@@ -54,7 +85,7 @@ def schedule_workflow(workflow, cron_schedule, *input, **args)
         # Execution timeout is across all scheduled jobs, whereas run is for an individual run.
         # This default is here for backward compatibility.  Certainly, the run timeout shouldn't be higher
         # than the execution timeout.
-        run_timeout: execution_options.timeouts[:run] || execution_options.timeouts[:execution],
+        run_timeout: compute_run_timeout(execution_options),
         task_timeout: execution_options.timeouts[:task],
         workflow_id_reuse_policy: options[:workflow_id_reuse_policy],
         headers: execution_options.headers,
@@ -226,6 +257,10 @@ def connection
       @connection ||= Temporal::Connection.generate(config.for_connection)
     end
 
+    def compute_run_timeout(execution_options)
+      execution_options.timeouts[:run] || execution_options.timeouts[:execution]
+    end
+
     def find_workflow_task(namespace, workflow_id, run_id, strategy)
       history = get_workflow_history(
         namespace: namespace,
diff --git a/lib/temporal/connection/grpc.rb b/lib/temporal/connection/grpc.rb
index 848d2de0..5b7e553b 100644
--- a/lib/temporal/connection/grpc.rb
+++ b/lib/temporal/connection/grpc.rb
@@ -292,8 +292,52 @@ def signal_workflow_execution(namespace:, workflow_id:, run_id:, signal:, input:
         client.signal_workflow_execution(request)
       end
 
-      def signal_with_start_workflow_execution
-        raise NotImplementedError
+      def signal_with_start_workflow_execution(
+        namespace:,
+        workflow_id:,
+        workflow_name:,
+        task_queue:,
+        input: nil,
+        execution_timeout:,
+        run_timeout:,
+        task_timeout:,
+        workflow_id_reuse_policy: nil,
+        headers: nil,
+        cron_schedule: nil,
+        signal_name:,
+        signal_input:
+      )
+        request = Temporal::Api::WorkflowService::V1::SignalWithStartWorkflowExecutionRequest.new(
+          identity: identity,
+          namespace: namespace,
+          workflow_type: Temporal::Api::Common::V1::WorkflowType.new(
+            name: workflow_name
+          ),
+          workflow_id: workflow_id,
+          task_queue: Temporal::Api::TaskQueue::V1::TaskQueue.new(
+            name: task_queue
+          ),
+          input: to_payloads(input),
+          workflow_execution_timeout: execution_timeout,
+          workflow_run_timeout: run_timeout,
+          workflow_task_timeout: task_timeout,
+          request_id: SecureRandom.uuid,
+          header: Temporal::Api::Common::V1::Header.new(
+            fields: headers
+          ),
+          cron_schedule: cron_schedule,
+          signal_name: signal_name,
+          signal_input: to_signal_payloads(signal_input)
+        )
+
+        if workflow_id_reuse_policy
+          policy = WORKFLOW_ID_REUSE_POLICY[workflow_id_reuse_policy]
+          raise Client::ArgumentError, 'Unknown workflow_id_reuse_policy specified' unless policy
+
+          request.workflow_id_reuse_policy = policy
+        end
+
+        client.signal_with_start_workflow_execution(request)
       end
 
       def reset_workflow_execution(namespace:, workflow_id:, run_id:, reason:, workflow_task_event_id:)
diff --git a/lib/temporal/testing/local_workflow_context.rb b/lib/temporal/testing/local_workflow_context.rb
index f9422af8..e4ded199 100644
--- a/lib/temporal/testing/local_workflow_context.rb
+++ b/lib/temporal/testing/local_workflow_context.rb
@@ -175,15 +175,15 @@ def now
       end
 
       def on_signal(&block)
-        raise NotImplementedError, 'not yet available for testing'
+        raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on'
       end
 
       def cancel_activity(activity_id)
-        raise NotImplementedError, 'not yet available for testing'
+        raise NotImplementedError, 'Cancel is not available when Temporal::Testing.local! is on'
       end
 
       def cancel(target, cancelation_id)
-        raise NotImplementedError, 'not yet available for testing'
+        raise NotImplementedError, 'Cancel is not available when Temporal::Testing.local! is on'
       end
 
       private
diff --git a/lib/temporal/testing/temporal_override.rb b/lib/temporal/testing/temporal_override.rb
index d4c58310..d786cf4b 100644
--- a/lib/temporal/testing/temporal_override.rb
+++ b/lib/temporal/testing/temporal_override.rb
@@ -8,10 +8,12 @@ module Temporal
   module Testing
     module TemporalOverride
 
-      def start_workflow(workflow, *input, **args)
+      def start_workflow(workflow, *input, signal_name: nil, signal_input: nil, **args)
         return super if Temporal::Testing.disabled?
 
         if Temporal::Testing.local?
+          # signals aren't supported at all, so let's prohibit start_workflow calls that try to signal
+          raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on' unless signal_name.nil? && signal_input.nil?
           start_locally(workflow, nil, *input, **args)
         end
       end
diff --git a/spec/unit/lib/temporal/client_spec.rb b/spec/unit/lib/temporal/client_spec.rb
index 92ec094b..589ad90d 100644
--- a/spec/unit/lib/temporal/client_spec.rb
+++ b/spec/unit/lib/temporal/client_spec.rb
@@ -185,6 +185,79 @@ class TestStartWorkflow < Temporal::Workflow
     end
   end
 
+  describe '#start_workflow with a signal' do
+    let(:temporal_response) do
+      Temporal::Api::WorkflowService::V1::SignalWithStartWorkflowExecutionResponse.new(run_id: 'xxx')
+    end
+
+    before { allow(connection).to receive(:signal_with_start_workflow_execution).and_return(temporal_response) }
+
+    def expect_signal_with_start(expected_arguments, expected_signal_argument)
+      expect(connection)
+        .to have_received(:signal_with_start_workflow_execution)
+        .with(
+          namespace: 'default-test-namespace',
+          workflow_id: an_instance_of(String),
+          workflow_name: 'TestStartWorkflow',
+          task_queue: 'default-test-task-queue',
+          input: expected_arguments,
+          task_timeout: Temporal.configuration.timeouts[:task],
+          run_timeout: Temporal.configuration.timeouts[:run],
+          execution_timeout: Temporal.configuration.timeouts[:execution],
+          workflow_id_reuse_policy: nil,
+          headers: {},
+          signal_name: 'the question',
+          signal_input: expected_signal_argument,
+        )
+    end
+
+    it 'starts a workflow with a signal and no arguments' do
+      subject.start_workflow(
+        TestStartWorkflow,
+        signal_name: 'the question'
+      )
+
+      expect_signal_with_start([], nil)
+    end
+
+    it 'starts a workflow with a signal and one scalar argument' do
+      signal_input = 'what do you get if you multiply six by nine?'
+      subject.start_workflow(
+        TestStartWorkflow,
+        42,
+        signal_name: 'the question',
+        signal_input: signal_input
+      )
+
+      expect_signal_with_start([42], signal_input)
+    end
+
+    it 'starts a workflow with a signal and multiple arguments and signal_inputs' do
+      signal_input = ['what do you get', 'if you multiply six by nine?']
+      subject.start_workflow(
+        TestStartWorkflow,
+        42,
+        43,
+        signal_name: 'the question',
+        # signals can't have multiple scalar args, but you can pass an array
+        signal_input: signal_input
+      )
+
+      expect_signal_with_start([42, 43], signal_input)
+    end
+
+    it 'raises when signal_input is given but signal_name is not' do
+      expect do
+        subject.start_workflow(
+          TestStartWorkflow, 
+          [42, 54],
+          [43, 55],
+          signal_input: 'what do you get if you multiply six by nine?',
+        )
+      end.to raise_error(ArgumentError)
+    end
+  end
+
   describe '#schedule_workflow' do
     let(:temporal_response) do
       Temporal::Api::WorkflowService::V1::StartWorkflowExecutionResponse.new(run_id: 'xxx')
diff --git a/spec/unit/lib/temporal/grpc_client_spec.rb b/spec/unit/lib/temporal/grpc_client_spec.rb
index bcc0458d..8349cc1e 100644
--- a/spec/unit/lib/temporal/grpc_client_spec.rb
+++ b/spec/unit/lib/temporal/grpc_client_spec.rb
@@ -34,6 +34,43 @@
       end
     end
   end
+  
+  describe '#signal_with_start_workflow' do
+    let(:temporal_response) do
+      Temporal::Api::WorkflowService::V1::SignalWithStartWorkflowExecutionResponse.new(run_id: 'xxx')
+    end
+
+    before { allow(grpc_stub).to receive(:signal_with_start_workflow_execution).and_return(temporal_response) }
+
+    it 'starts a workflow with a signal with scalar arguments' do
+      subject.signal_with_start_workflow_execution(
+        namespace: namespace,
+        workflow_id: workflow_id,
+        workflow_name: 'workflow_name',
+        task_queue: 'task_queue',
+        input: ['foo'],
+        execution_timeout: 1,
+        run_timeout: 2,
+        task_timeout: 3,
+        signal_name: 'the question',
+        signal_input: 'what do you get if you multiply six by nine?'
+      )
+
+      expect(grpc_stub).to have_received(:signal_with_start_workflow_execution) do |request|
+        expect(request).to be_an_instance_of(Temporal::Api::WorkflowService::V1::SignalWithStartWorkflowExecutionRequest)
+        expect(request.namespace).to eq(namespace)
+        expect(request.workflow_id).to eq(workflow_id)
+        expect(request.workflow_type.name).to eq('workflow_name')
+        expect(request.task_queue.name).to eq('task_queue')
+        expect(request.input.payloads[0].data).to eq('"foo"')
+        expect(request.workflow_execution_timeout.seconds).to eq(1)
+        expect(request.workflow_run_timeout.seconds).to eq(2)
+        expect(request.workflow_task_timeout.seconds).to eq(3)
+        expect(request.signal_name).to eq('the question')
+        expect(request.signal_input.payloads[0].data).to eq('"what do you get if you multiply six by nine?"')
+      end
+    end
+  end
 
   describe '#get_workflow_execution_history' do
     let(:response) do
diff --git a/spec/unit/lib/temporal/testing/temporal_override_spec.rb b/spec/unit/lib/temporal/testing/temporal_override_spec.rb
index 09e4eeee..cf70a25e 100644
--- a/spec/unit/lib/temporal/testing/temporal_override_spec.rb
+++ b/spec/unit/lib/temporal/testing/temporal_override_spec.rb
@@ -136,6 +136,14 @@ def execute
           .with(an_instance_of(Temporal::Testing::LocalWorkflowContext))
       end
 
+      it 'explicitly does not support staring a workflow with a signal' do
+        expect {
+          client.start_workflow(TestTemporalOverrideWorkflow, signal_name: 'breakme')
+        }.to raise_error(NotImplementedError) do |e| 
+          expect(e.message).to eql("Signals are not available when Temporal::Testing.local! is on")
+        end
+      end
+
       describe 'execution control' do
         subject do
           client.start_workflow(

From 09d8ccb3e5fedfe68ca2d0e03916b8d8e1db40bd Mon Sep 17 00:00:00 2001
From: Nathan Glass <nagl@stripe.com>
Date: Wed, 3 Nov 2021 17:50:28 -0700
Subject: [PATCH 2/4] Move signal arguments to the options hash

---
 .../integration/signal_with_start_spec.rb     | 30 ++++++++++++-------
 lib/temporal/client.rb                        | 17 ++++++-----
 lib/temporal/testing/temporal_override.rb     |  9 ++++--
 spec/unit/lib/temporal/client_spec.rb         | 18 ++++++-----
 .../testing/temporal_override_spec.rb         |  2 +-
 5 files changed, 47 insertions(+), 29 deletions(-)

diff --git a/examples/spec/integration/signal_with_start_spec.rb b/examples/spec/integration/signal_with_start_spec.rb
index 46799996..4af4f6f1 100644
--- a/examples/spec/integration/signal_with_start_spec.rb
+++ b/examples/spec/integration/signal_with_start_spec.rb
@@ -8,9 +8,11 @@
       SignalWithStartWorkflow,
       'signal_name',
       0.1,
-      signal_name: 'signal_name',
-      signal_input: 'expected value',
-      options: { workflow_id: workflow_id }
+      options: {
+        workflow_id: workflow_id,
+        signal_name: 'signal_name',
+        signal_input: 'expected value',
+      }
     )
 
     result = Temporal.await_workflow_result(
@@ -28,8 +30,10 @@
       SignalWithStartWorkflow,
       'signal_name',
       0.1,
-      signal_name: 'signal_name',
-      options: { workflow_id: workflow_id }
+      options: {
+        workflow_id: workflow_id,
+        signal_name: 'signal_name',
+      }
     )
 
     result = Temporal.await_workflow_result(
@@ -47,18 +51,22 @@
       SignalWithStartWorkflow,
       'signal_name',
       10,
-      signal_name: 'signal_name',
-      signal_input: 'expected value',
-      options: { workflow_id: workflow_id }
+      options: {
+        workflow_id: workflow_id,
+        signal_name: 'signal_name',
+        signal_input: 'expected value',
+      }
     )
 
     second_run_id = Temporal.start_workflow(
       SignalWithStartWorkflow,
       'signal_name',
       0.1,
-      signal_name: 'signal_name',
-      signal_input: 'expected value',
-      options: { workflow_id: workflow_id }
+      options: {
+        workflow_id: workflow_id,
+        signal_name: 'signal_name',
+        signal_input: 'expected value',
+      }
     )
 
     # If the run ids are the same, then we didn't start a new workflow
diff --git a/lib/temporal/client.rb b/lib/temporal/client.rb
index df1dcf84..ad216d3d 100644
--- a/lib/temporal/client.rb
+++ b/lib/temporal/client.rb
@@ -15,20 +15,23 @@ def initialize(config)
 
     # Starts a workflow with an optional signal.
     #
-    # signal_name and signal_input are optional parameters that control Temporal's signal_with_start
-    # behavior.  signal_name corresponds to the 'signal' argument to signal_workflow, and signal_input
-    # corresponds to the 'input' argument to signal_workflow
+    # Set options[:signal_name] to use Temporal's signal_with_start behavior. options[:signal_name]
+    # corresponds to the 'signal' argument to signal_workflow, and options[:signal_input] corresponds
+    # to the 'input' argument to signal_workflow.
     #
-    # If signal_name is specified, Temporal will atomically do one of the following two things:
+    # If options[:signal_name] is specified, Temporal will atomically do one of the following two things:
     # A) start a new workflow and signal it
     # B) if workflow_id is specified and the workflow already exists, signal the existing workflow.
     #
-    # It is legal to specify signal_name without signal_input, but it is illegal to specify signal_input
-    # without signal_name.
-    def start_workflow(workflow, *input, signal_name: nil, signal_input: nil, **args)
+    # It is legal to specify options[:signal_name] without options[:signal_input], but it is illegal
+    # to specify options[:signal_input] without options[:signal_name].
+    def start_workflow(workflow, *input, **args)
       options = args.delete(:options) || {}
       input << args unless args.empty?
 
+      signal_name = options.delete(:signal_name)
+      signal_input = options.delete(:signal_input)
+
       execution_options = ExecutionOptions.new(workflow, options, config.default_execution_options)
       workflow_id = options[:workflow_id] || SecureRandom.uuid
 
diff --git a/lib/temporal/testing/temporal_override.rb b/lib/temporal/testing/temporal_override.rb
index d786cf4b..37dc7914 100644
--- a/lib/temporal/testing/temporal_override.rb
+++ b/lib/temporal/testing/temporal_override.rb
@@ -8,12 +8,10 @@ module Temporal
   module Testing
     module TemporalOverride
 
-      def start_workflow(workflow, *input, signal_name: nil, signal_input: nil, **args)
+      def start_workflow(workflow, *input, **args)
         return super if Temporal::Testing.disabled?
 
         if Temporal::Testing.local?
-          # signals aren't supported at all, so let's prohibit start_workflow calls that try to signal
-          raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on' unless signal_name.nil? && signal_input.nil?
           start_locally(workflow, nil, *input, **args)
         end
       end
@@ -75,6 +73,11 @@ def start_locally(workflow, schedule, *input, **args)
         options = args.delete(:options) || {}
         input << args unless args.empty?
 
+        # signals aren't supported at all, so let's prohibit start_workflow calls that try to signal
+        signal_name = options.delete(:signal_name)
+        signal_input = options.delete(:signal_input)
+        raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on' unless signal_name.nil? && signal_input.nil?
+
         reuse_policy = options[:workflow_id_reuse_policy] || :allow_failed
         workflow_id = options[:workflow_id] || SecureRandom.uuid
         run_id = SecureRandom.uuid
diff --git a/spec/unit/lib/temporal/client_spec.rb b/spec/unit/lib/temporal/client_spec.rb
index 589ad90d..a0260f43 100644
--- a/spec/unit/lib/temporal/client_spec.rb
+++ b/spec/unit/lib/temporal/client_spec.rb
@@ -214,7 +214,7 @@ def expect_signal_with_start(expected_arguments, expected_signal_argument)
     it 'starts a workflow with a signal and no arguments' do
       subject.start_workflow(
         TestStartWorkflow,
-        signal_name: 'the question'
+        options: { signal_name: 'the question' }
       )
 
       expect_signal_with_start([], nil)
@@ -225,8 +225,10 @@ def expect_signal_with_start(expected_arguments, expected_signal_argument)
       subject.start_workflow(
         TestStartWorkflow,
         42,
-        signal_name: 'the question',
-        signal_input: signal_input
+        options: {
+          signal_name: 'the question',
+          signal_input: signal_input,
+        }
       )
 
       expect_signal_with_start([42], signal_input)
@@ -238,9 +240,11 @@ def expect_signal_with_start(expected_arguments, expected_signal_argument)
         TestStartWorkflow,
         42,
         43,
-        signal_name: 'the question',
-        # signals can't have multiple scalar args, but you can pass an array
-        signal_input: signal_input
+        options: {
+          signal_name: 'the question',
+          # signals can't have multiple scalar args, but you can pass an array
+          signal_input: signal_input
+        }
       )
 
       expect_signal_with_start([42, 43], signal_input)
@@ -252,7 +256,7 @@ def expect_signal_with_start(expected_arguments, expected_signal_argument)
           TestStartWorkflow, 
           [42, 54],
           [43, 55],
-          signal_input: 'what do you get if you multiply six by nine?',
+          options: { signal_input: 'what do you get if you multiply six by nine?', }
         )
       end.to raise_error(ArgumentError)
     end
diff --git a/spec/unit/lib/temporal/testing/temporal_override_spec.rb b/spec/unit/lib/temporal/testing/temporal_override_spec.rb
index cf70a25e..42d72a9c 100644
--- a/spec/unit/lib/temporal/testing/temporal_override_spec.rb
+++ b/spec/unit/lib/temporal/testing/temporal_override_spec.rb
@@ -138,7 +138,7 @@ def execute
 
       it 'explicitly does not support staring a workflow with a signal' do
         expect {
-          client.start_workflow(TestTemporalOverrideWorkflow, signal_name: 'breakme')
+          client.start_workflow(TestTemporalOverrideWorkflow, options: { signal_name: 'breakme' })
         }.to raise_error(NotImplementedError) do |e| 
           expect(e.message).to eql("Signals are not available when Temporal::Testing.local! is on")
         end

From 30b7e8ca9114dd398a8fd2b06bf9b01ba072c51f Mon Sep 17 00:00:00 2001
From: Nathan Glass <nagl@stripe.com>
Date: Mon, 8 Nov 2021 09:13:03 -0800
Subject: [PATCH 3/4] PR feedback

---
 lib/temporal/testing/temporal_override.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/temporal/testing/temporal_override.rb b/lib/temporal/testing/temporal_override.rb
index 37dc7914..825241be 100644
--- a/lib/temporal/testing/temporal_override.rb
+++ b/lib/temporal/testing/temporal_override.rb
@@ -76,7 +76,7 @@ def start_locally(workflow, schedule, *input, **args)
         # signals aren't supported at all, so let's prohibit start_workflow calls that try to signal
         signal_name = options.delete(:signal_name)
         signal_input = options.delete(:signal_input)
-        raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on' unless signal_name.nil? && signal_input.nil?
+        raise NotImplementedError, 'Signals are not available when Temporal::Testing.local! is on' if signal_name || signal_input
 
         reuse_policy = options[:workflow_id_reuse_policy] || :allow_failed
         workflow_id = options[:workflow_id] || SecureRandom.uuid

From f796e71b4f3a868bdbe8bceae813b73099e1283e Mon Sep 17 00:00:00 2001
From: Nathan Glass <nagl@stripe.com>
Date: Mon, 8 Nov 2021 09:15:12 -0800
Subject: [PATCH 4/4] Fix merge error

---
 lib/temporal/client.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/temporal/client.rb b/lib/temporal/client.rb
index f7430cd5..e7e3c26c 100644
--- a/lib/temporal/client.rb
+++ b/lib/temporal/client.rb
@@ -13,6 +13,8 @@ def initialize(config)
       @config = config
     end
 
+    # Start a workflow with an optional signal
+    #
     # If options[:signal_name] is specified, Temporal will atomically do one of:
     # A) start a new workflow and signal it
     # B) if workflow_id is specified and the workflow already exists, signal the existing workflow.