From f7d7a6c2e932912ed48f022d6343fa4f167ccf98 Mon Sep 17 00:00:00 2001
From: Adam Novak <anovak@soe.ucsc.edu>
Date: Thu, 24 Oct 2024 17:12:07 -0400
Subject: [PATCH 1/2] Move condition check into wrapper

---
 src/toil/cwl/cwltoil.py                      |  7 ++++-
 src/toil/test/cwl/cwlTest.py                 |  8 ++++++
 src/toil/test/cwl/not_run_required_input.cwl | 29 ++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 src/toil/test/cwl/not_run_required_input.cwl

diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py
index 65c09ec710..db51f18c10 100644
--- a/src/toil/cwl/cwltoil.py
+++ b/src/toil/cwl/cwltoil.py
@@ -2448,17 +2448,22 @@ def __init__(
     def run(self, file_store: AbstractFileStore) -> Any:
         """Create a child job with the correct resource requirements set."""
         cwljob = resolve_dict_w_promises(self.cwljob, file_store)
+
+        # Check confitional to license full evaluation of job inputs.
+        if self.conditional.is_false(cwljob):
+            return self.conditional.skipped_outputs()
+
         fill_in_defaults(
             self.cwltool.tool["inputs"],
             cwljob,
             self.runtime_context.make_fs_access(self.runtime_context.basedir or ""),
         )
+        # Don't forward the conditional. We checked it already.
         realjob = CWLJob(
             tool=self.cwltool,
             cwljob=cwljob,
             runtime_context=self.runtime_context,
             parent_name=self.parent_name,
-            conditional=self.conditional,
         )
         self.addChild(realjob)
         return realjob.rv()
diff --git a/src/toil/test/cwl/cwlTest.py b/src/toil/test/cwl/cwlTest.py
index 1ff5c1c59b..4711923ae2 100644
--- a/src/toil/test/cwl/cwlTest.py
+++ b/src/toil/test/cwl/cwlTest.py
@@ -453,6 +453,14 @@ def test_glob_dir_bypass_file_store(self) -> None:
             except FileNotFoundError:
                 pass
 
+    def test_required_input_condition_protection(self) -> None:
+        # This doesn't run containerized
+        self._tester(
+            "src/toil/test/cwl/not_run_required_input.cwl",
+            "src/toil/test/cwl/empty.json",
+            {},
+        )
+
     @needs_slurm
     def test_slurm_node_memory(self) -> None:
         pass
diff --git a/src/toil/test/cwl/not_run_required_input.cwl b/src/toil/test/cwl/not_run_required_input.cwl
new file mode 100644
index 0000000000..0cd24f0f0a
--- /dev/null
+++ b/src/toil/test/cwl/not_run_required_input.cwl
@@ -0,0 +1,29 @@
+# This workflow fills in a required int from an optional int, but only when the
+# int is really present. But it also uses the value to compute the conditional
+# task's resource requirements, so Toil can't just schedule the task and then
+# check the condition.
+# See <https://github.com/DataBiosphere/toil/issues/4930#issue-2297563321>
+cwlVersion: v1.2
+class: Workflow
+requirements:
+  InlineJavascriptRequirement: {}
+inputs:
+  optional_input: int?
+steps:
+  the_step:
+    in:
+      required_input:
+        source: optional_input
+    when: $(inputs.required_input != null)
+    run:
+      cwlVersion: v1.2
+      class: CommandLineTool
+      inputs:
+        required_input: int
+      requirements:
+        ResourceRequirement:
+          coresMax: $(inputs.required_input)
+      baseCommand: "nproc"
+      outputs: []
+    out: []
+outputs: []

From 7d895d199d5f26259290b81a8d2f4dde02cd5c7b Mon Sep 17 00:00:00 2001
From: Adam Novak <anovak@soe.ucsc.edu>
Date: Fri, 25 Oct 2024 12:23:17 -0400
Subject: [PATCH 2/2] Fill in empty conditional

---
 src/toil/cwl/cwltoil.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/toil/cwl/cwltoil.py b/src/toil/cwl/cwltoil.py
index db51f18c10..e4eea3844d 100644
--- a/src/toil/cwl/cwltoil.py
+++ b/src/toil/cwl/cwltoil.py
@@ -269,7 +269,7 @@ class Conditional:
     """
     Object holding conditional expression until we are ready to evaluate it.
 
-    Evaluation occurs at the moment the encloses step is ready to run.
+    Evaluation occurs before the enclosing step's inputs are type-checked.
     """
 
     def __init__(
@@ -2442,7 +2442,7 @@ def __init__(
         self.cwltool = tool
         self.cwljob = cwljob
         self.runtime_context = runtime_context
-        self.conditional = conditional
+        self.conditional = conditional or Conditional()
         self.parent_name = parent_name
 
     def run(self, file_store: AbstractFileStore) -> Any: