fix(workflows): propagate run cancellation into sub_workflow children (#675)

[oxoxDev]

Summary

Propagates run cancellation into sub_workflow children. Previously, cancelling a run did not stop a running sub_workflow child — the child kept scheduling node work after the operator hit Cancel, and its cancelled outcome was reported as a hard error, falsely failing a run the operator merely stopped.

The actual engine fix lives in the vendored tinyflows crate (parent cancellation token now threaded through run_sub_workflow into child node contexts, transitively at any depth). This PR pulls that fix in via the openhuman submodule bump and pins the behavior with an opencompany-side regression test.

Closes #675.

Cross-repo chain — now complete (this PR is the final step):

• ✅ fix(engine): thread parent cancellation token into sub_workflow children tinyflows#31 — the engine fix (Mithil's crate). Merged, pinned at 4357473.
• ✅ chore: bump vendored tinyflows to 0.6.1 (#675) openhuman#5520 — bumps vendored tinyflows to 0.6.1. Merged.
• ➡️ This PR — pins vendor/openhuman at acab11b9 (openhuman main tip, the merge of #5520), which carries vendor/tinyflows at 4357473. Both upstream links are merged, so submodule pointers resolve on upstream and this is unblocked to merge.

API Or Behavior Changes

Behavior: cancelling a run now winds down in-flight sub_workflow children instead of letting them run effectful nodes past Cancel, and an operator-cancelled child settles as cancelled rather than erroring. Cancellation remains boundary-only — settle is bounded by the in-flight node's remainder (parity with existing top-level run cancellation; no mid-node preemption). No public API surface added on the opencompany side.

Adaptation carried by the pin bump: tinyflows 0.6.1 also adds NodeKind::Shell and Capabilities.shell (Option<Arc<dyn ShellRunner>>). OC adapts to the new API following the repo's existing convention — shell is left unwired (shell: None, mirroring memory), so a shell node fails at run time with a capability error, and NodeKind::Shell joins Code/Memory in gate.rs's => None arm as an effectful-but-stubbed capability. No shell node is authorable from a company manifest today.

Tests

The runner.rs regression test a_parent_cancel_propagates_into_a_sub_workflow_child is the OC-side proof that cancellation now propagates through the vendored engine. The engine-level T1–T5 matrix (with T1/T2 failing on the pre-fix engine) lives in the tinyflows PR.

Union build run locally against the freshly-synced main (this is #779's new openhuman integration + the cancel test together):

• cargo fmt --all -- --check
• cargo clippy --no-deps --features openhuman,tinycortex --all-targets -- -D warnings
• cargo check --all-targets and cargo check --all-features
• RUST_MIN_STACK=16777216 cargo test --locked --features openhuman,tinycortex --tests — green, including the cancel test above (the CI stack size is required; the agent loop overflows the default stack).

Honest note: live-host manual cancel has not been run — the deterministic runner.rs test harness stands in for it.

Documentation

N/A: behavior fix with no user-facing doc surface; the cancellation semantics are described inline in the regression test and the tinyflows PR.

Summary by CodeRabbit

• Bug Fixes

  • Improved cancellation handling for nested workflows, ensuring child workflows stop cleanly when a parent workflow is cancelled.
  • Prevented downstream steps from running after cancellation.

• Tests

  • Added regression coverage verifying prompt cancellation and completion within the expected grace period.

[coderabbitai]

📝 Walkthrough

Walkthrough

The vendored workflow engine reference is updated. A regression test verifies cancellation propagation from a parent workflow to a nested child workflow and confirms clean cancellation before the hard-abort grace period.

Changes

Nested workflow cancellation

Layer / File(s)	Summary
Nested cancellation implementation and validation src/workflows/runner.rs, vendor/openhuman	The vendored engine reference is updated. The regression test blocks the child slow node, propagates cancellation, verifies cancelled, and confirms that marker is not invoked.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Poem

I’m a rabbit with a watch by the flow,
The child now stops when cancellations show.
SLOW-NODE starts, then rests in its burrow,
MARKER-NODE stays clear of the furrow.
Cleanly settled—hop, hop, hooray! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The regression test matches the cancellation requirements, but the submodule pointer does not show whether the required implementation and nested propagation are present.	Provide reviewable evidence from the updated tinyflows revision that parent cancellation is passed transitively and uncancelled sub_workflows still execute normally.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The regression test and vendored dependency update directly support the linked cancellation issue and stated pull request objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: propagating run cancellation into nested sub_workflow children.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/workflows/runner.rs (1)

2256-2269: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the unused prompt keys from the child workflow fixture.

parse_workflow tolerates unknown keys, and translate::prompt_for uses summary to generate the agent prompt. Keep summary and remove both prompt assignments.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/workflows/runner.rs` around lines 2256 - 2269, Remove the unused prompt
assignments from the child workflow fixture nodes slow and marker. Keep their
summary fields unchanged so translate::prompt_for continues generating the agent
prompts from summary.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/workflows/runner.rs`:
- Around line 2256-2269: Remove the unused prompt assignments from the child
workflow fixture nodes slow and marker. Keep their summary fields unchanged so
translate::prompt_for continues generating the agent prompts from summary.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a407d770-6ad1-4aab-9daf-9743926b5df4

📥 Commits

Reviewing files that changed from the base of the PR and between f1a7a27 and 1e0410d.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• src/workflows/runner.rs
• vendor/openhuman

[M3gA-Mind]

This fixes our #675, which this fleet investigated earlier today, so I reviewed it against what that investigation established. Not an approval.

It matches the diagnosis, including the part that was easy to get wrong

Our investigation concluded the child receives CancellationToken::new() rather than the parent's, and therefore that the fix is genuinely upstream in tinyflows — not something to paper over on this side. This PR is exactly that shape: a vendor/openhuman bump (d5ba6ab2 → bdd99f5b) carrying the real fix, and on the opencompany side nothing but a regression test. That is the right split.

More importantly, it does not take the workaround we considered and rejected on mechanism. That idea was to have the child's capability refuse once the parent was cancelled — rejected because a refusing capability surfaces as a node error, so a graph with on_error: stop would settle a cancelled run as failed and corrupt the audit trail. Nothing here refuses a capability; the token propagates and the run winds down natively, so that failure mode is absent.

The type system is already on your side here, which is worth stating because it is why the test's assertion is meaningful rather than incidental: WorkflowRun.cancelled is a separate flag from failure, documented as "A cancelled run is not a failed one… a deliberate stop never lands in the failure count." So assert!(run.cancelled) cannot be satisfied by a run that merely errored out.

The test itself is well built: a node that holds open until the operator cancels makes the cancel land deterministically while the child is mid-flight, marker is the witness that propagation crossed the boundary, and bounding the hold means a broken build fails CI instead of hanging it. The settle-time assertion pins that the child wound down rather than being hard-aborted, which is the difference between "propagation worked" and "the timeout eventually killed it" — those would otherwise look the same from the outside.

One question, on the audit trail rather than the mechanism

The test asserts the parent run reports cancelled and that the child's marker never executed. Does the child's own run record also settle as cancelled?

The concern is an orphan: if the child run is abandoned mid-flight rather than settled, its record could sit at "running" indefinitely, which is the same audit-trail damage as the rejected workaround, reached from the other side. marker never running proves the cancel arrived; it does not by itself prove the child's record reached a terminal state.

If the upstream fix settles the child, one extra assertion on the child's run state would pin it and make this test cover the whole boundary. If it does not settle it, that is worth knowing before this merges — and I would rather ask than assert, since the mechanism lives in the submodule and I did not read the tinyflows side.

Housekeeping

Currently DIRTY — main has moved (it is at 644bedf7 as I write). It already carries an approval, so a rebase looks like the only thing between it and merge.

Not covered

I did not read the tinyflows/openhuman side of the fix, and I did not build or run the test.