Skip to content

fix(aws-healthomics-mcp-server): add workflow_type parameter to start_run for Ready2Run workflows - #4350

Open
steditt wants to merge 10 commits into
awslabs:mainfrom
steditt:fix/start-run-workflow-type
Open

fix(aws-healthomics-mcp-server): add workflow_type parameter to start_run for Ready2Run workflows#4350
steditt wants to merge 10 commits into
awslabs:mainfrom
steditt:fix/start-run-workflow-type

Conversation

@steditt

@steditt steditt commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Changes

The start_run tool in the HealthOmics MCP server is missing the workflowType parameter that the AWS HealthOmics StartRun API requires when executing Ready2Run workflows. Without it, the API defaults to PRIVATE and returns "Workflow not found" for Ready2Run workflow IDs — even though ListWorkflows and GetWorkflow discover them correctly.

This PR adds:

  • An optional workflow_type parameter (PRIVATE or READY2RUN) to the start_run tool
  • Validation using the shared validate_workflow_type() utility from validation_utils.py (same pattern as list_workflows and get_workflow)
  • Passes workflowType to client.start_run() only when provided (backward compatible — omitting it preserves existing PRIVATE behavior)
  • Three new unit tests covering READY2RUN, None (omitted), and invalid value scenarios

User experience

Before: An agent discovers a Ready2Run workflow via ListWorkflows(workflow_type="READY2RUN"), retrieves its details via GetWorkflow, but StartRun fails with:

ResourceNotFoundException: Workflow 2174942 not found

After: The agent can pass workflow_type="READY2RUN" to StartRun and the run starts successfully:

{
  "workflow_id": "2174942",
  "workflow_type": "READY2RUN",
  "role_arn": "arn:aws:iam::123456789012:role/OmicsRole",
  "name": "scrnaseq-starsolo",
  "output_uri": "s3://my-bucket/outputs/",
  "parameters": {"samplename": "test", "protocol": "10XV3", "input": [...]},
  "storage_type": "STATIC",
  "storage_capacity": 1200
}

Checklist

If your change doesn't seem to apply, please leave them unchecked.

  • I have reviewed the contributing guidelines
  • I have performed a self-review of this change
  • Changes have been tested
  • Changes are documented

Is this a breaking change? N

RFC issue number: N/A (bug fix — extends existing parameter pattern to a missing tool)

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

… for Ready2Run workflows

The start_run tool was missing the workflowType parameter that the AWS
HealthOmics StartRun API requires when running Ready2Run workflows.
Without it, the API defaults to PRIVATE and returns 'Workflow not found'
for Ready2Run workflow IDs.

Changes:
- Add optional workflow_type parameter (PRIVATE|READY2RUN) to start_run
- Pass workflowType to client.start_run() when provided
- Validate workflow_type against allowed values
- Add tests for READY2RUN, None (omitted), and invalid values
@steditt
steditt force-pushed the fix/start-run-workflow-type branch from 1e96f01 to 209e6bb Compare July 27, 2026 12:05
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.10%. Comparing base (f984f58) to head (bf6af93).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4350      +/-   ##
==========================================
+ Coverage   93.04%   93.10%   +0.05%     
==========================================
  Files        1031     1014      -17     
  Lines       86905    85879    -1026     
  Branches    14019    13835     -184     
==========================================
- Hits        80865    79959     -906     
+ Misses       3658     3581      -77     
+ Partials     2382     2339      -43     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

steditt and others added 5 commits July 28, 2026 08:32
…2RUN validation

- Front-load 'REQUIRED for Ready2Run workflows' in workflow_type Field
  description to improve LLM tool-calling accuracy
- Add pre-validation rejecting storage_type/storage_capacity when
  workflow_type is READY2RUN (fail fast with clear error instead of
  confusing downstream API error)
- Remove redundant 'not applicable' prose from storage field descriptions
  since the validation now enforces it

Addresses agent struggling with 5 retries before discovering that
workflow_type='READY2RUN' is needed for AWS-provided workflows.
The HealthOmics API returns a misleading ResourceNotFoundException
when workflow_type is omitted for Ready2Run workflows.

Part of: awslabs#4350
- Sort imports alphabetically (isort/I001)
- Assert on result in test_start_run_invalid_workflow_type to fix
  unused variable warning (F841)
…ombo

The test_start_run_with_workflow_type_ready2run test passed
storage_type='STATIC' and storage_capacity=1200 alongside
workflow_type='READY2RUN'. The new pre-validation correctly rejects
this combination before reaching the API client, so the test never
invoked mock_client.start_run (causing NoneType subscript error).

Fix: use storage_type='DYNAMIC' and storage_capacity=None, which
is the valid parameter set for Ready2Run workflows.
Use single-line plain ASCII description without em-dash or embedded
quotes. This avoids potential serialization issues across tool schema
boundaries (AgentCore Gateway indexes tool schemas at connect time).
@steditt
steditt force-pushed the fix/start-run-workflow-type branch from cc7e75c to b579dd4 Compare July 28, 2026 16:05
…d code

- Add tests for READY2RUN storage parameter validation
- Add tests for VPC networking mode validation paths
- Add tests for list_runs date filter pagination and truncation
- Add test for list_run_tasks with pagination token
- Add test for filter_runs_by_creation_time with malformed datetime
- Refactor S3 URI test to exercise real try/except path
- Remove unreachable else branch in list_runs (dead code)

Coverage: 90% -> 100% for workflow_execution.py
@steditt
steditt force-pushed the fix/start-run-workflow-type branch from d8c1d66 to 63f3613 Compare July 29, 2026 05:09

@markjschreiber markjschreiber left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@alxawan alxawan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +238 to +244
# Validate workflow type (using shared utility from validation_utils)
if workflow_type is not None and isinstance(workflow_type, str):
validation_result = await validate_workflow_type(ctx, workflow_type)
if isinstance(validation_result, dict):
return validation_result
# Normalize workflow_type: only pass to API if it's a valid string
effective_workflow_type = workflow_type if isinstance(workflow_type, str) else None

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Consider using Literal typing for workflow_type instead of manual validation.


@pytest.mark.asyncio
async def test_start_run_ready2run_with_storage_capacity_rejected():
"""Test READY2RUN workflow rejects storage_capacity parameter (line 249)."""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Remove these line number labels in the docstring in case original file is modified. There are several of these in this test file.

)

# Should return an error dict from handle_tool_error
assert 'error' in str(result).lower() or isinstance(result, dict)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This assertion always passes: the right operand is always true since start_run returns Dict[str, Any].

@markjschreiber
markjschreiber enabled auto-merge July 29, 2026 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To triage

Development

Successfully merging this pull request may close these issues.

4 participants