Warn when a post job action cannot run on a step with no job

client/src/components/Workflow/Editor/Forms/FormPickValue.test.ts

@@ -9,6 +9,7 @@ import type { Step } from "@/stores/workflowStepStore";
 
 import FormPickValue from "./FormPickValue.vue";
 import FormElement from "@/components/Form/FormElement.vue";
+import FormSection from "@/components/Workflow/Editor/Forms/FormSection.vue";
 
 const localVue = getLocalVue();
 localVue.use(PiniaVuePlugin);
@@ -33,10 +34,11 @@ function makeStep(overrides: Partial<Step> = {}): Step {
     } as Step;
 }
 
-function mountPickValue(step?: Step): Wrapper<Vue> {
+function mountPickValue(step?: Step, datatypes?: unknown[]): Wrapper<Vue> {
     return shallowMount(FormPickValue as any, {
         propsData: {
             step: step ?? makeStep(),
+            datatypes,
         },
         localVue,
         pinia: createTestingPinia({ createSpy: vi.fn }),
@@ -85,6 +87,12 @@ describe("FormPickValue", () => {
         });
     });
 
+    it("hides actions that require a job", () => {
+        const wrapper = mountPickValue(undefined, []);
+
+        expect(wrapper.findComponent(FormSection).props("supportsJobBasedActions")).toBe(false);
+    });
+
     describe("mode changes", () => {
         it("emits onChange with updated mode", () => {
             const wrapper = mountPickValue();

client/src/components/Workflow/Editor/Forms/FormPickValue.vue

@@ -135,6 +135,7 @@ if (connections) {
                 :step="step"
                 :datatypes="datatypes"
                 :post-job-actions="postJobActions ?? {}"
+                :supports-job-based-actions="false"
                 @onChange="onChangePostJobActions" />
         </div>
     </div>

client/src/components/Workflow/Editor/Forms/FormSection.test.ts

@@ -0,0 +1,37 @@
+import { getLocalVue } from "@tests/vitest/helpers";
+import { shallowMount } from "@vue/test-utils";
+import { describe, expect, it } from "vitest";
+
+import FormSection from "./FormSection.vue";
+import FormElement from "@/components/Form/FormElement.vue";
+
+const localVue = getLocalVue();
+
+function mountFormSection(supportsJobBasedActions?: boolean) {
+    return shallowMount(FormSection as any, {
+        localVue,
+        propsData: {
+            id: 0,
+            nodeInputs: [],
+            nodeOutputs: [{ name: "output" }],
+            step: { id: 0, type: "pick_value" },
+            datatypes: [],
+            postJobActions: {},
+            supportsJobBasedActions,
+        },
+    });
+}
+
+describe("FormSection", () => {
+    it("shows job-based actions by default", () => {
+        const wrapper = mountFormSection();
+
+        expect(wrapper.findAllComponents(FormElement)).toHaveLength(2);
+    });
+
+    it("hides job-based actions when the step cannot execute them", () => {
+        const wrapper = mountFormSection(false);
+
+        expect(wrapper.findAllComponents(FormElement)).toHaveLength(0);
+    });
+});

client/src/components/Workflow/Editor/Forms/FormSection.vue

@@ -1,13 +1,15 @@
 <template>
     <div v-if="firstOutput">
         <FormElement
+            v-if="supportsJobBasedActions"
             :id="emailActionKey"
             :value="emailActionValue"
             title="Email notification"
             type="boolean"
             help="An email notification will be sent when the job has completed."
             @input="onInput" />
         <FormElement
+            v-if="supportsJobBasedActions"
             :id="deleteActionKey"
             :value="deleteActionValue"
             title="Output cleanup"
@@ -62,6 +64,10 @@ export default {
             type: Object,
             required: true,
         },
+        supportsJobBasedActions: {
+            type: Boolean,
+            default: true,
+        },
     },
     data() {
         return {

client/src/utils/navigation/navigation.yml

@@ -1122,6 +1122,14 @@ workflow_editor:
       type: xpath
       selector: >
         //div[@data-label='Remove Tags']//input
+    email_notification:
+      type: xpath
+      selector: >
+        //div[@data-label='Email notification']
+    output_cleanup:
+      type: xpath
+      selector: >
+        //div[@data-label='Output cleanup']
     tool_version_button: ".tool-versions"
     connector_for: "#connection-${sink_id}-${source_id}"
     connector_invalid_for: "#connection-${sink_id}-${source_id} .connection.invalid"

lib/galaxy/job_execution/actions/post.py

@@ -21,6 +21,10 @@ class DefaultJobAction:
 
     name = "DefaultJobAction"
     verbose_name = "Default Job"
+    # Whether execute_on_mapped_over below is actually implemented. Steps that
+    # apply actions without a job to hang them off of can only run actions that
+    # set this, so ActionBox warns rather than silently dropping the others.
+    supports_mapped_over = False
 
     @classmethod
     def execute(cls, app, sa_session, action, job, replacement_dict=None, final_job_state=None):
@@ -105,6 +109,7 @@ def get_short_str(cls, pja):
 
 class ChangeDatatypeAction(DefaultJobAction):
     name = "ChangeDatatypeAction"
+    supports_mapped_over = True
     verbose_name = "Change Datatype"
 
     @classmethod
@@ -151,6 +156,7 @@ def get_short_str(cls, pja):
 
 class RenameDatasetAction(DefaultJobAction):
     name = "RenameDatasetAction"
+    supports_mapped_over = True
     verbose_name = "Rename Dataset"
 
     @classmethod
@@ -294,6 +300,7 @@ def get_short_str(cls, pja):
 
 class HideDatasetAction(DefaultJobAction):
     name = "HideDatasetAction"
+    supports_mapped_over = True
     verbose_name = "Hide Dataset"
 
     @classmethod
@@ -319,6 +326,7 @@ def get_short_str(cls, pja):
 class DeleteDatasetAction(DefaultJobAction):
     # This is disabled for right now.  Deleting a dataset in the middle of a workflow causes errors (obviously) for the subsequent steps using the data.
     name = "DeleteDatasetAction"
+    supports_mapped_over = True
     verbose_name = "Delete Dataset"
 
     @classmethod
@@ -342,6 +350,7 @@ def get_short_str(cls, pja):
 
 class ColumnSetAction(DefaultJobAction):
     name = "ColumnSetAction"
+    supports_mapped_over = True
     verbose_name = "Assign Columns"
 
     @classmethod
@@ -473,6 +482,7 @@ def get_short_str(cls, pja):
 
 class TagDatasetAction(DefaultJobAction):
     name = "TagDatasetAction"
+    supports_mapped_over = True
     verbose_name = "Add tag to dataset"
     action = "Add"
     direction = "to"
@@ -597,10 +607,21 @@ def handle_incoming(cls, incoming):
     def execute_on_mapped_over(
         cls, trans, sa_session, pja, step_inputs, step_outputs, replacement_dict=None, final_job_state=None
     ):
-        if pja.action_type in cls.actions:
-            cls.actions[pja.action_type].execute_on_mapped_over(
-                trans, sa_session, pja, step_inputs, step_outputs, replacement_dict, final_job_state=final_job_state
+        action = cls.actions.get(pja.action_type)
+        if action is None:
+            return
+        if not action.supports_mapped_over:
+            log.warning(
+                "Ignoring post job action %s on workflow step %s output '%s', it cannot be applied to a step "
+                "output that has no job attached to it.",
+                pja.action_type,
+                pja.workflow_step_id,
+                pja.output_name,
             )
+            return
+        action.execute_on_mapped_over(
+            trans, sa_session, pja, step_inputs, step_outputs, replacement_dict, final_job_state=final_job_state
+        )
 
     @classmethod
     def execute(cls, app, sa_session, pja, job, replacement_dict=None, final_job_state=None):

lib/galaxy_test/selenium/test_workflow_editor.py

@@ -1216,6 +1216,38 @@ def test_pick_value_add_tags_pja(self):
         assert "TagDatasetActionoutput" in pjas
         assert "picktag" in pjas["TagDatasetActionoutput"]["action_arguments"]["tags"]
 
+    @selenium_test
+    def test_pick_value_hides_actions_needing_a_job(self):
+        self.open_in_workflow_editor("""
+class: GalaxyWorkflow
+inputs:
+  input_data: data
+steps:
+  branch_a:
+    tool_id: cat
+    in:
+      input1: input_data
+  pick:
+    type: pick_value
+    state:
+      mode: first_non_null
+    in:
+      input_0: branch_a/out_file1
+""")
+        editor = self.components.workflow_editor
+        # A pick_value step has no job for these to act on, so they are not offered.
+        # Select it first - the panel a selected node opens covers the nodes to its right.
+        editor.node._(label="pick").wait_for_and_click()
+        # The rest of the section still renders, otherwise the assertions below would
+        # pass on a form that simply had not loaded.
+        editor.configure_output(output="output").wait_for_visible()
+        editor.email_notification.assert_absent()
+        editor.output_cleanup.assert_absent()
+        # A tool step does run a job, so it still gets both.
+        editor.node._(label="branch_a").wait_for_and_click()
+        editor.email_notification.wait_for_visible()
+        editor.output_cleanup.wait_for_visible()
+
     @selenium_test
     def test_pick_value_compact_on_disconnect(self):
         self.open_in_workflow_editor("""

test/unit/job_execution/test_action_box.py

@@ -0,0 +1,78 @@
+"""Tests for dispatching post job actions to step outputs with no job."""
+
+import inspect
+import logging
+
+from galaxy.job_execution.actions.post import (
+    ActionBox,
+    DefaultJobAction,
+)
+from galaxy.model import (
+    HistoryDatasetAssociation,
+    PostJobAction,
+)
+
+
+def _all_action_classes():
+    def descend(action_class):
+        yield action_class
+        for subclass in action_class.__subclasses__():
+            yield from descend(subclass)
+
+    return list(descend(DefaultJobAction))
+
+
+def _implements_mapped_over(action_class):
+    # getattr_static rather than a plain attribute read - it hands back the classmethod
+    # object itself, so __func__ reaches the underlying function to compare identity.
+    def underlying(cls):
+        return inspect.getattr_static(cls, "execute_on_mapped_over").__func__
+
+    return underlying(action_class) is not underlying(DefaultJobAction)
+
+
+def test_supports_mapped_over_matches_implementation():
+    """supports_mapped_over has to agree with what the class actually overrides.
+
+    ActionBox skips and warns about actions that do not set it, so a stale flag
+    either drops a working action or lets an unimplemented one through as a
+    no-op - which is what the flag exists to make noisy.
+    """
+    for action_class in _all_action_classes():
+        assert action_class.supports_mapped_over == _implements_mapped_over(action_class), action_class.name
+
+
+def test_mapped_over_output_actions_are_supported():
+    """Every action ToolModule applies to mapped over outputs must implement it."""
+    for action_type in ActionBox.mapped_over_output_actions:
+        assert ActionBox.actions[action_type].supports_mapped_over, action_type
+
+
+def _dispatch(action_type, step_outputs):
+    # No trans or session - a step output with no job is already in the history, so
+    # the actions that run here only touch the output they are handed.
+    pja = PostJobAction(action_type, output_name="", action_arguments={"newname": "renamed"})
+    ActionBox.execute_on_mapped_over(None, None, pja, {}, step_outputs)
+
+
+def _warnings(caplog):
+    return [record for record in caplog.records if record.levelno >= logging.WARNING]
+
+
+def test_unsupported_action_warns_instead_of_running(caplog):
+    """EmailAction is configurable on a pick_value step but has no job to report on."""
+    _dispatch("EmailAction", {})
+    assert [record for record in _warnings(caplog) if "EmailAction" in record.getMessage()]
+
+
+def test_supported_action_still_runs(caplog):
+    """The rename has to actually reach the output, not just avoid the warning."""
+    output = HistoryDatasetAssociation(name="original")
+    _dispatch("RenameDatasetAction", {"output": output})
+    assert output.name == "renamed"
+    assert _warnings(caplog) == []
+
+
+def test_unknown_action_is_ignored(caplog):
+    _dispatch("NoSuchAction", {})
+    assert _warnings(caplog) == []