Fix all_non_null single source element

[LanderOtto]

No description provided.

[LanderOtto]

Resolve #2219

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.09%. Comparing base (4c8e609) to head (9271e77).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2220      +/-   ##
==========================================
- Coverage   85.10%   85.09%   -0.02%     
==========================================
  Files          46       46              
  Lines        8522     8525       +3     
  Branches     1988     1989       +1     
==========================================
+ Hits         7253     7254       +1     
- Misses        802      805       +3     
+ Partials      467      466       -1     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jmchilton]

LGTM - Claude's assessment:

PR #2220 Review: Fix all_non_null single source element

Issue Summary

Issue #2219 reports that pickValue: all_non_null with a single-element source list fails validation, because cwltool doesn't apply linkMerge (array-wrapping) when there's only one source. Per the spec, all_non_null always produces an array, so the source values must always be collected into a list first (via merge_nested) before pickValue filters nulls.

Does it fix the issue? Yes.

The root cause: when source has only one element, linkMerge defaults to None (line 391: "merge_nested" if len(...) > 1 else None). Without linkMerge, match_types sets the input as a scalar rather than wrapping it in an array. Then at line 429, pickValue processing is guarded by isinstance(inputobj.get(iid), MutableSequence) — a scalar fails this check, so all_non_null is never applied.

The fix forces linkMerge = "merge_nested" when pickValue is present and linkMerge would otherwise be None, in both:

1. checker.py (static validation) — ensures type-checking passes
2. workflow_job.py (runtime) — ensures values are collected into an array

Correctness Analysis

The fix is correct per the spec. The CWL v1.2 spec says:

• pickValue is evaluated after linkMerge
• all_non_null: "Gathers all non-null values from the first list level, producing a list"
• This means the input to all_non_null must always be a list, regardless of how many sources there are

checker.py change (lines 397-398):

elif pickValue == "all_non_null":
    linkMerge = linkMerge or "merge_nested"

This is correct. It only overrides when linkMerge is None (single source without explicit linkMerge). It preserves any explicit linkMerge setting. It correctly doesn't apply to first_non_null/the_only_non_null which produce scalars.

workflow_job.py change (lines 402-412):

link_merge = cast(...)  # existing logic
if inp.get("pickValue") and link_merge is None:
    link_merge = "merge_nested"

This mirrors the checker logic at runtime. One note: this applies for any pickValue value, not just all_non_null. For first_non_null/the_only_non_null with a single source, this would wrap the value in an array, then pickValue would unwrap it back to a scalar — which is functionally correct (the spec says linkMerge runs first, then pickValue selects from the resulting list).

Potential Concerns

1. workflow_job.py is broader than checker.py: The checker only forces merge_nested for all_non_null, but the runtime forces it for any pickValue. This asymmetry is not a bug — the checker already sets linkMerge = None for first_non_null/the_only_non_null (line 395-396), which is correct for type-checking (the result type is scalar). At runtime, wrapping then unwrapping is harmless and arguably more spec-compliant.

2. Test coverage looks good: Tests cover:

   • all_non_null with single source + non-null input → [1] (array output)
   • all_non_null with single source + null input → [0] (empty array filtered, step gets empty array strs, length=0)
   • Error case: single source + all_non_null where step expects scalar string (not string[]) → should fail validation (exit code 1)

3. Minor nit: The _err test file (single_all_non_null_err.cwl) has strs: string (scalar) as input while source is a list with pickValue: all_non_null. This correctly tests that type validation still catches mismatches — all_non_null produces an array, which shouldn't match a scalar string input.

Verdict

The PR correctly fixes issue #2219. The approach is sound — it ensures linkMerge is always applied when pickValue is present so that pickValue always operates on an array as the spec requires. The fix is minimal, well-tested, and handles both the validation and runtime paths.