From f34b5e37abd2d917519d2d22af0f58bc2cf1ec27 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 13 Dec 2025 09:10:54 +0100 Subject: [PATCH 1/6] Add failing test --- lib/galaxy_test/api/test_workflows.py | 92 +++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 14d41a2f74b..1f04d2becda 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -5648,6 +5648,98 @@ test_data: messages = subworkflow_invocation.get("messages", []) assert len(messages) == 0, f"Expected no error messages, got: {messages}" + def test_run_subworkflow_with_boolean_parameter_in_when_condition(self): + """Test boolean false parameter passed to subworkflow with when condition. + + This test verifies that boolean parameters (especially false) are properly + passed from parent to subworkflow when the subworkflow has: + 1. Delayed scheduling (via $link) + 2. A when condition that uses the boolean parameter + + Previously, false values were converted to None in the when expression evaluation, + causing "when_not_boolean" errors. + """ + with self.dataset_populator.test_history() as history_id: + workflow = """ +class: GalaxyWorkflow +inputs: + should_run: + type: boolean + some_file: + type: data +steps: + nested_workflow: + in: + subworkflow_should_run: should_run + subworkflow_file: some_file + run: + class: GalaxyWorkflow + inputs: + subworkflow_should_run: boolean + subworkflow_file: data + steps: + expression: + tool_id: expression_forty_two + state: {} + conditional_step: + tool_id: cheetah_casting + in: + subworkflow_should_run: subworkflow_should_run + state: + floattest: 3.14 + inttest: + $link: expression/out1 + when: $(inputs.subworkflow_should_run) +test_data: + some_file: + value: 1.bed + type: File + should_run: + value: false + type: raw +""" + summary = self._run_workflow(workflow, history_id=history_id, wait=True, assert_ok=True) + + # Verify parent workflow executed successfully + parent_invocation = self.workflow_populator.get_invocation(summary.invocation_id, step_details=True) + assert parent_invocation["state"] == "scheduled" + + # Find the subworkflow step and get its invocation + subworkflow_step = None + for step in parent_invocation["steps"]: + if step.get("subworkflow_invocation_id"): + subworkflow_step = step + break + + assert subworkflow_step is not None, "No subworkflow step found" + subworkflow_invocation_id = subworkflow_step["subworkflow_invocation_id"] + subworkflow_invocation = self.workflow_populator.get_invocation( + subworkflow_invocation_id, step_details=True + ) + + # The subworkflow should have succeeded + assert ( + subworkflow_invocation["state"] == "scheduled" + ), f"Expected subworkflow to succeed, got state: {subworkflow_invocation['state']}" + + # Should not have error messages (previously failed with "when_not_boolean") + messages = subworkflow_invocation.get("messages", []) + assert len(messages) == 0, f"Expected no error messages, got: {messages}" + + # Find the conditional step in the subworkflow and verify it was skipped + # (when condition was false, so step should not execute) + conditional_step = None + for step in subworkflow_invocation["steps"]: + if step.get("workflow_step_label") == "conditional_step": + conditional_step = step + break + + assert conditional_step is not None, "Conditional step not found in subworkflow" + # The step should have been skipped because should_run=false + assert len(conditional_step["jobs"]) == 0 or all( + j["state"] == "skipped" for j in conditional_step["jobs"] + ), "Expected conditional step to be skipped when should_run=false" + def test_run_with_non_optional_data_unspecified_fails_invocation(self): with self.dataset_populator.test_history() as history_id: error = self._run_jobs( From 1e48e15ffb65b15e570ffce4f2a9dcd2ce330432 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 13 Dec 2025 12:14:22 +0100 Subject: [PATCH 2/6] Record input parameter invocation inputs as they become available. Fixes the test in in the previous commit and also means that inputs are properly recorded, something that's always been unavailable for subworkflow invocations. --- lib/galaxy/workflow/modules.py | 7 ------- lib/galaxy/workflow/run.py | 6 ++++++ test/unit/workflows/test_workflow_progress.py | 5 +++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index d0428ae0ee7..3ea170baeee 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -1029,13 +1029,6 @@ class InputModule(WorkflowModule): step_outputs["input_ds_copy"] = new_hdca else: raise Exception("Unknown history content encountered") - # If coming from UI - we haven't registered invocation inputs yet, - # so do that now so dependent steps can be recalculated. In the future - # everything should come in from the API and this can be eliminated. - if not invocation.has_input_for_step(step.id): - content = next(iter(step_outputs.values())) - if content and content is not NO_REPLACEMENT: - invocation.add_input(content, step.id) progress.set_outputs_for_input(invocation_step, step_outputs) return None diff --git a/lib/galaxy/workflow/run.py b/lib/galaxy/workflow/run.py index 6d4bd6d1eb0..84594022bf6 100644 --- a/lib/galaxy/workflow/run.py +++ b/lib/galaxy/workflow/run.py @@ -623,6 +623,12 @@ class WorkflowProgress: if step.label and step.type == "parameter_input" and "output" in outputs: self.runtime_replacements[step.label] = str(outputs["output"]) + invocation = invocation_step.workflow_invocation + if not invocation.has_input_for_step(step.id): + content = outputs.get("output", NO_REPLACEMENT) + if content is not NO_REPLACEMENT: + log.info("ADDING INPUT FOR STEP %s: %s", step.id, content, exc_info=True) + invocation.add_input(content, step.id) self.set_step_outputs(invocation_step, outputs, already_persisted=already_persisted) def effective_replacement_dict(self): diff --git a/test/unit/workflows/test_workflow_progress.py b/test/unit/workflows/test_workflow_progress.py index 1a6147c95ca..08cc8c057f4 100644 --- a/test/unit/workflows/test_workflow_progress.py +++ b/test/unit/workflows/test_workflow_progress.py @@ -94,8 +94,7 @@ class TestWorkflowProgress(TestCase): workflow_invocation_step.state = "scheduled" workflow_invocation_step.workflow_step = self._step(i) assert step_id == self._step(i).id - # workflow_invocation_step.workflow_invocation = self.invocation - self.invocation.steps.append(workflow_invocation_step) + workflow_invocation_step.workflow_invocation = self.invocation workflow_invocation_step_state = model.WorkflowRequestStepState() workflow_invocation_step_state.workflow_step_id = step_id @@ -111,6 +110,7 @@ class TestWorkflowProgress(TestCase): else: workflow_invocation_step = model.WorkflowInvocationStep() workflow_invocation_step.workflow_step = self._step(index) + workflow_invocation_step.workflow_invocation = self.invocation return workflow_invocation_step def test_connect_data_input(self): @@ -211,6 +211,7 @@ class TestWorkflowProgress(TestCase): subworkflow_invocation_step.workflow_step_id = subworkflow_input_step.id subworkflow_invocation_step.state = "new" subworkflow_invocation_step.workflow_step = subworkflow_input_step + subworkflow_invocation_step.workflow_invocation = subworkflow_invocation subworkflow_progress.set_outputs_for_input(subworkflow_invocation_step) From 42183a6cf7f4c6acb44cb6a7a81022912629c0c8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 16 Dec 2025 11:00:07 +0100 Subject: [PATCH 3/6] Maintain column definitions on map over --- lib/galaxy/managers/collections.py | 2 ++ .../model/dataset_collections/structure.py | 28 +++++++++++++++---- .../api/test_dataset_collections.py | 1 + .../data/dataset_collections/test_matching.py | 1 + 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/managers/collections.py b/lib/galaxy/managers/collections.py index c8539c319ea..4f0a834fd9a 100644 --- a/lib/galaxy/managers/collections.py +++ b/lib/galaxy/managers/collections.py @@ -137,6 +137,8 @@ class DatasetCollectionManager: collection_type_description = structure.collection_type_description dataset_collection = DatasetCollection(populated=False) dataset_collection.collection_type = collection_type_description.collection_type + # Preserve column_definitions from input structure for sample sheets + dataset_collection.column_definitions = structure.column_definitions elements = [] for index, (identifier, substructure) in enumerate(structure.children): # TODO: Open question - populate these now or later? diff --git a/lib/galaxy/model/dataset_collections/structure.py b/lib/galaxy/model/dataset_collections/structure.py index 97d38319703..37ef1471f0b 100644 --- a/lib/galaxy/model/dataset_collections/structure.py +++ b/lib/galaxy/model/dataset_collections/structure.py @@ -64,12 +64,15 @@ class UninitializedTree(BaseTree): class Tree(BaseTree): children_known = True - def __init__(self, children, collection_type_description, when_values=None, columns_metadata=None): + def __init__( + self, children, collection_type_description, when_values=None, columns_metadata=None, column_definitions=None + ): super().__init__(collection_type_description) self.children = children self.when_values = when_values # columns_metadata is a dict mapping element_identifier to columns data self.columns_metadata = columns_metadata or {} + self.column_definitions = column_definitions @staticmethod def for_dataset_collection(dataset_collection, collection_type_description): @@ -90,7 +93,12 @@ class Tree(BaseTree): # Capture columns metadata from sample sheet collections if element.columns is not None: columns_metadata[element.element_identifier] = element.columns - return Tree(children, collection_type_description, columns_metadata=columns_metadata) + return Tree( + children, + collection_type_description, + columns_metadata=columns_metadata, + column_definitions=dataset_collection.column_definitions, + ) def walk_collections(self, hdca_dict): return self._walk_collections(dict_map(lambda hdca: hdca.collection, hdca_dict)) @@ -148,12 +156,22 @@ class Tree(BaseTree): for identifier, structure in self.children: new_children.append((identifier, structure.multiply(other_structure))) - # Preserve columns_metadata when multiplying - return Tree(new_children, new_collection_type, columns_metadata=self.columns_metadata.copy()) + # Preserve columns_metadata and column_definitions when multiplying + return Tree( + new_children, + new_collection_type, + columns_metadata=self.columns_metadata.copy(), + column_definitions=self.column_definitions, + ) def clone(self): cloned_children = [(_[0], _[1].clone()) for _ in self.children] - return Tree(cloned_children, self.collection_type_description, columns_metadata=self.columns_metadata.copy()) + return Tree( + cloned_children, + self.collection_type_description, + columns_metadata=self.columns_metadata.copy(), + column_definitions=self.column_definitions, + ) def __str__(self): return f"Tree[collection_type={self.collection_type_description},children=({','.join(f'{identifier_and_element[0]}={identifier_and_element[1]}' for identifier_and_element in self.children)})]" diff --git a/lib/galaxy_test/api/test_dataset_collections.py b/lib/galaxy_test/api/test_dataset_collections.py index ce9b02c64e6..2eef937346f 100644 --- a/lib/galaxy_test/api/test_dataset_collections.py +++ b/lib/galaxy_test/api/test_dataset_collections.py @@ -481,6 +481,7 @@ class TestDatasetCollectionsApi(ApiTestCase): collection_details = self.dataset_populator.get_history_collection_details( history_id, content_id=output_collection["id"] ) + assert collection_details["column_definitions"] == sample_sheet["column_definitions"] # Verify that the output collection preserved the columns metadata output_elements = collection_details["elements"] diff --git a/test/unit/data/dataset_collections/test_matching.py b/test/unit/data/dataset_collections/test_matching.py index 50cb6af35d4..0528166f1ee 100644 --- a/test/unit/data/dataset_collections/test_matching.py +++ b/test/unit/data/dataset_collections/test_matching.py @@ -247,6 +247,7 @@ class MockCollection: self.collection_type = collection_type self.elements = elements self.populated = True + self.column_definitions = None class MockCollectionElement: From 74c3b635c1d12dbcb1fed5fd80cc1e6bb0433561 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 16 Dec 2025 13:59:14 +0500 Subject: [PATCH 4/6] [25.1] Remove ref from release-drafter workflow We realized that the release drafter checks out whatever version is on dev instead of the release branch that we want to target. --- .github/workflows/release-drafter.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/release-drafter.yml b/.github/workflows/release-drafter.yml index 27ab7b43bb1..9d0e1d581da 100644 --- a/.github/workflows/release-drafter.yml +++ b/.github/workflows/release-drafter.yml @@ -22,7 +22,6 @@ jobs: steps: - uses: actions/checkout@v4 with: - ref: dev sparse-checkout: | lib/galaxy/version.py From a05abd0ff235d6008708f5aca6c2c802cf9e49e6 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 16 Dec 2025 16:23:27 +0500 Subject: [PATCH 5/6] add an optional workflow_dispatch trigger to release-drafter --- .github/workflows/release-drafter.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/release-drafter.yml b/.github/workflows/release-drafter.yml index 9d0e1d581da..d47a59e4f0a 100644 --- a/.github/workflows/release-drafter.yml +++ b/.github/workflows/release-drafter.yml @@ -8,6 +8,7 @@ on: - 'release_*' pull_request_target: types: [opened, reopened, synchronize] + workflow_dispatch: permissions: contents: read From 19486b79fe4e60075e2d826fd360ab1db95d468f Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Tue, 16 Dec 2025 19:12:31 +0500 Subject: [PATCH 6/6] only allow one draft at a time, and update it if it is the target version A limitation of the release-drafter (as I understand it) is that it only updates the latest draft, whether it is the dev draft at that point or the release draft, hence, resulting in duplicate release drafts if a PR against a release branch is merged. Here, we've added a step that checks and expects only 1 draft release at a time. This is cleaner because: while the only draft is `dev`, we only update that draft. Once, during the release process, `dev` becomes a `release_` branch, we do not create yet another draft for dev and instead just update the latest release branch draft, until the release will be published and then dev will start getting its own draft notes. --- .github/workflows/release-drafter.yml | 29 +++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/.github/workflows/release-drafter.yml b/.github/workflows/release-drafter.yml index d47a59e4f0a..b552e2d2a72 100644 --- a/.github/workflows/release-drafter.yml +++ b/.github/workflows/release-drafter.yml @@ -35,7 +35,36 @@ jobs: echo "version=${RELEASE_VERSION}" >> $GITHUB_OUTPUT echo "Galaxy release version: ${RELEASE_VERSION}" + - name: Check if we should update draft + id: check-draft + run: | + VERSION="${{ inputs.version || steps.galaxy-version.outputs.version }}" + + # Check if this version is already published + PUBLISHED=$(gh release view "v${VERSION}" --json isDraft --jq '.isDraft' 2>/dev/null || echo "not_found") + + if [ "$PUBLISHED" = "false" ]; then + echo "Published release v${VERSION} already exists. Skipping." + echo "skip=true" >> $GITHUB_OUTPUT + exit 0 + fi + + # Check if a draft exists for a DIFFERENT version (only one draft at a time) + EXISTING_DRAFT=$(gh release list --limit 50 --json tagName,isDraft --jq '.[] | select(.isDraft == true) | .tagName' | head -1) + + if [ -n "$EXISTING_DRAFT" ] && [ "$EXISTING_DRAFT" != "v${VERSION}" ]; then + echo "Draft release $EXISTING_DRAFT already exists (for different version). Skipping v${VERSION}." + echo "Only one draft release at a time is supported." + echo "skip=true" >> $GITHUB_OUTPUT + else + echo "Proceeding with draft for v${VERSION}." + echo "skip=false" >> $GITHUB_OUTPUT + fi + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - uses: release-drafter/release-drafter@v6 + if: steps.check-draft.outputs.skip == 'false' with: config-name: release-drafter.yml version: ${{ steps.galaxy-version.outputs.version }}