-
Notifications
You must be signed in to change notification settings - Fork 18
[APO-2170] Default to entrypoint trigger when no inputs provided #3097
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
devin-ai-integration
wants to merge
1
commit into
main
Choose a base branch
from
devin/APO-2170-1763147069
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
98 changes: 98 additions & 0 deletions
98
tests/workflows/integration_trigger_execution/tests/test_default_entrypoint.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| """Tests for default entrypoint behavior when no trigger is provided.""" | ||
|
|
||
| from tests.workflows.integration_trigger_execution.nodes.slack_message_trigger import SlackMessageTrigger | ||
| from tests.workflows.integration_trigger_execution.workflows.multi_trigger_workflow import MultiTriggerWorkflow | ||
| from tests.workflows.integration_trigger_execution.workflows.routing_only_workflow import RoutingOnlyWorkflow | ||
| from tests.workflows.integration_trigger_execution.workflows.simple_workflow import SimpleSlackWorkflow | ||
|
|
||
|
|
||
| def test_single_trigger_no_inputs_defaults_to_entrypoint(): | ||
| """ | ||
| Tests that workflow with single IntegrationTrigger and no inputs defaults to entrypoint. | ||
| """ | ||
|
|
||
| workflow = RoutingOnlyWorkflow() | ||
|
|
||
| # WHEN we run the workflow without trigger and without inputs | ||
| result = workflow.run() | ||
|
|
||
| # THEN it should execute successfully | ||
| assert result.name == "workflow.execution.fulfilled" | ||
|
|
||
| assert result.outputs.result == "Workflow executed successfully" | ||
|
|
||
|
|
||
| def test_single_trigger_with_attribute_references_still_fails(): | ||
| """ | ||
| Tests that workflow referencing trigger attributes still fails without trigger data. | ||
| """ | ||
|
|
||
| # GIVEN a workflow with SlackMessageTrigger that references trigger attributes | ||
| workflow = SimpleSlackWorkflow() | ||
|
|
||
| # WHEN we run the workflow without trigger and without inputs | ||
| result = workflow.run() | ||
|
|
||
| assert result.name == "workflow.execution.rejected" | ||
|
|
||
| assert "Missing trigger attribute" in result.body.error.message | ||
|
|
||
|
|
||
| def test_multiple_triggers_no_inputs_uses_manual_path(): | ||
| """ | ||
| Tests that workflow with multiple triggers and no inputs uses ManualTrigger path. | ||
| """ | ||
|
|
||
| # GIVEN a workflow with both ManualTrigger and IntegrationTrigger | ||
| workflow = MultiTriggerWorkflow() | ||
|
|
||
| # WHEN we run the workflow without trigger and without inputs | ||
| result = workflow.run() | ||
|
|
||
| # THEN it should execute successfully via ManualTrigger path (existing behavior) | ||
| assert result.name == "workflow.execution.fulfilled" | ||
|
|
||
| assert result.outputs.manual_result == "Manual execution" | ||
|
|
||
|
|
||
| def test_explicit_trigger_param_works_unchanged(): | ||
| """ | ||
| Tests that providing explicit trigger parameter still works as before. | ||
| """ | ||
|
|
||
| workflow = SimpleSlackWorkflow() | ||
|
|
||
| # AND a valid Slack trigger instance | ||
| trigger = SlackMessageTrigger( | ||
| message="Explicit trigger test", | ||
| channel="C123456", | ||
| user="U789012", | ||
| ) | ||
|
|
||
| # WHEN we run the workflow with the trigger | ||
| result = workflow.run(trigger=trigger) | ||
|
|
||
| # THEN it should execute successfully | ||
| assert result.name == "workflow.execution.fulfilled" | ||
|
|
||
| # AND the node should have access to trigger outputs | ||
| assert result.outputs.result == "Received 'Explicit trigger test' from channel C123456" | ||
|
|
||
|
|
||
| def test_single_trigger_no_inputs_stream_works(): | ||
| """ | ||
| Tests that workflow.stream() with single IntegrationTrigger and no inputs works. | ||
| """ | ||
|
|
||
| workflow = RoutingOnlyWorkflow() | ||
|
|
||
| events = list(workflow.stream()) | ||
|
|
||
| # THEN we should get workflow events | ||
| assert len(events) > 0 | ||
|
|
||
| # AND the final event should be fulfilled | ||
| last_event = events[-1] | ||
| assert last_event.name == "workflow.execution.fulfilled" | ||
|
|
||
| assert last_event.outputs.result == "Workflow executed successfully" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
tests/workflows/integration_trigger_execution/workflows/routing_only_workflow.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| """Workflow with IntegrationTrigger that doesn't reference trigger attributes.""" | ||
|
|
||
| from vellum.workflows import BaseWorkflow | ||
| from vellum.workflows.nodes.bases import BaseNode | ||
|
|
||
| from tests.workflows.integration_trigger_execution.nodes.slack_message_trigger import SlackMessageTrigger | ||
|
|
||
|
|
||
| class ConstantOutputNode(BaseNode): | ||
| """Node that returns a constant output without referencing trigger attributes.""" | ||
|
|
||
| class Outputs(BaseNode.Outputs): | ||
| result: str | ||
|
|
||
| def run(self) -> Outputs: | ||
| return self.Outputs(result="Workflow executed successfully") | ||
|
|
||
|
|
||
| class RoutingOnlyWorkflow(BaseWorkflow): | ||
| """Workflow with IntegrationTrigger used only for routing, not for data access.""" | ||
|
|
||
| graph = SlackMessageTrigger >> ConstantOutputNode | ||
|
|
||
| class Outputs(BaseWorkflow.Outputs): | ||
| result = ConstantOutputNode.Outputs.result |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto-trigger path instantiates a blank
IntegrationTriggerand immediately calls_validate_and_bind_triggeron it. Because the instance has no attributes,BaseTrigger.bind_to_stateiterates the class descriptors and writes thoseTriggerAttributeReferenceobjects directly intostate.meta.trigger_attributes. As a result, workflows that actually reference trigger attributes (e.g.,SimpleSlackWorkflow.message) will read those descriptors and happily format strings like"SlackMessageTrigger.message"instead of raising the expected"Missing trigger attribute"error, so the workflow is marked fulfilled when it should be rejected. The new tests added in this change (test_single_trigger_with_attribute_references_still_fails) will therefore fail, and the original bug remains. Consider skipping_validate_and_bind_triggerfor the default trigger or detecting missing required attributes before binding so that attribute access still raises aNodeException.Useful? React with 👍 / 👎.