fix: propagate CANCELED instead of FAILED from JOIN when a forked branch is canceled

[nikhiln64]

Summary

Fixes #1422.

When a sub-workflow inside a FORK_JOIN is manually terminated, the SUB_WORKFLOW task in the parent becomes CANCELED, but Join.execute converted that into an unconditional FAILED on the JOIN task. DeciderService already maps a CANCELED terminal task to a TERMINATED workflow, but it never got the chance because the JOIN rewrote the status before the decider saw it. The parent workflow therefore reported FAILED instead of TERMINATED.

The failure branch in Join.execute now derives the JOIN status from the forked tasks it joins on. If every non-optional, non-successful terminal forked task is CANCELED the join surfaces CANCELED, and the existing decider mapping takes the parent workflow to TERMINATED. If any forked task genuinely failed, FAILED still wins exactly as before. As a small tidy up in the same lines, null incompletion reasons from canceled tasks are no longer joined into the reason string as the literal text null.

Testing

Two regression tests in TestJoin: a completed task plus a canceled task must produce a CANCELED join, and a canceled task alongside a genuinely failed task must still produce a FAILED join. Ran TestJoin, JoinTest and TestDeciderService with ./gradlew :conductor-core:test, all green. The repro definitions posted by @nthmost-orkes on the issue drive exactly the first scenario end to end.

Behavior note

Only the JOIN status derivation changes. Workflows where a forked branch fails for real keep reporting FAILED, and optional or permissive semantics are untouched.

[mp-orkes]

@nikhiln64 thanks for picking this up — diagnosis seems to be right, and the fix works.

Findings:

1. The repro steps on bug: manually aborting sub-workflow causes parent workflow to show FAILED instead of TERMINATED #1422 don't actually reproduce. Ran them verbatim, parent came back TERMINATED. Without permissive, sub_branch_a's CANCELED terminates the parent at DeciderService.java:217 before the JOIN is ever evaluated.

2. It does reproduce with "permissive": true on both branches (and aborting both children — a permissive JOIN waits for all branches to be terminal). Then you get parent FAILED, global_join FAILED, exactly as reported.

3. I ran your tests against main: test 1 fails (expected:<CANCELED> but was:<FAILED>), test 2 passes. So test 1 is the real regression test, test 2 is a guardrail. Both fine.

4. But test 1 behaves the same regardless of permissive — it calls Join.execute() directly, and that method writes FAILED for any non-successful non-optional branch. permissive only changes when the failure branch fires (Join.java:107), not the status it writes. It matters end-to-end only because it's what stops the decider from terminating the parent before the JOIN's status is read. So the unit tests can't tell us the user-visible bug is fixed — can you add a test-harness test with permissive: true asserting the parent hits TERMINATED?

Could you also fix the repro steps on the issue? As written they'll send the next person chasing something they can't trigger.

Unrelated to this PR: same symptom happens with a permissive task and no JOIN at all (DeciderService.java:311 defaults to FAILED). Reproduced that too — probably a follow-up.

[nikhiln64]

Thanks for the thorough verification @mp-orkes, especially for pinning down the permissive requirement in the end to end path. The test harness test you asked for is pushed. It forks two permissive SUB_WORKFLOW branches, terminates both children through workflowExecutor.terminateWorkflow, then evaluates the join and asserts the parent lands on TERMINATED with the JOIN task CANCELED. Against main the same test fails with the parent in FAILED, so it captures the user visible regression rather than just the unit level status write. The full ForkJoinSpec passes locally with the fix and the branch is merged up to the latest main.

I also posted corrected repro steps on #1422 so the next person starts from the permissive configuration instead of the plain fork. Good spot on the permissive task with no JOIN defaulting to FAILED at DeciderService.java:311, happy to pick that up as a follow up issue once this lands. The one red check on this run is the Trigger Playwright E2E job failing to set a commit status with the fork read only token, the same 403 pattern as other external PRs.

[nikhiln64]

A note on the red e2e check before anyone burns time on it. The failing test is WorkflowRerunTests, Rerun from inner task of SUB_WORKFLOW in FORK branch, and it died on a 404 from updateTaskSync while completing a task in the freshly spawned child after the rerun, a step that runs while the JOIN is still mid flight so the code this PR changes is never on that path. In that test the failing branch ends FAILED rather than CANCELED, which means the new hasNonCanceledFailure check writes FAILED exactly as main does.

I ran the full WorkflowRerunTests class locally against the dockerized redis es8 stack from run_tests-es8.sh with this branch and all 37 tests pass, including that one. The same CI job also flaked on main this week, the Aug 5 run failed the jitter window test in RetryPolicyTests and the Aug 6 run failed the postgres e2e variant, each time a single test out of 200 with everything else green. Since fork PRs cannot re trigger the job, could you re run the failed e2e job when you get a chance? The other jobs on this run, unit-test, test-harness and build, are already green.

[nthmost-orkes]

Since fork PRs cannot re trigger the job, could you re run the failed e2e job when you get a chance? The other jobs on this run, unit-test, test-harness and build, are already green.

Yep - back on this now, let's try and get this over the line this week.

[nikhiln64]

Appreciate it, thank you. Nothing else is pending on my side. The diff is unchanged and green on unit-test, test-harness and build, so a maintainer triggered run of the e2e job should take it the rest of the way.

[nthmost-orkes]

My own testing has shown this fix is sound -- hoping for second approval on this one.

[nthmost-orkes]

Fix works locally, matches repro conditions -- with architectural approval from Dale we're good to go here