Repository navigation
feat(workflow): DAG conditions and composable sub-workflows - #1129
Conversation
|
@SaulMoro Thanks for all your recent contributions to TeamAI. We really appreciate the consistent, thoughtful work you've been doing. I've upgraded your repository access to write—well deserved! |
|
@jeff-r2026 Thank you, I appreciate the trust. I use TeamAI both with my team and on my own, and it's very useful in both cases. In my personal projects, it gives me a single source of truth for skills, rules and hooks, and every AI tool I use pulls from it, so I dogfood it a lot. The new workflows are very powerful. I've enjoyed contributing, and I have more ideas to improve it, so I'll keep sending them as issues and PRs. |
Testing: The description provides sufficient build and representative real-CLI evidence. Deferred provider/agent coverage is acceptable. No PR code was run, built, or installed. |
A read of steps.<inline>.outputs.<name> or a comparison of steps.<inline>.end with a name the inline workflow does not declare passed pull and then always read null. Both now report undeclared_output / undeclared_end, as they already do for a workflow file.
Bundle the parse helpers' shared arguments into one context, walk referenced workflows through workflowsRunBy at start as at pull, name the cache-drop rule and the settled-workflow result once, share one isRecord, and type the outcome an expression reads. No behaviour change.
Pull and the engine now read the templated texts of a step (with, prompt, env) from one templatedTexts() in template.ts, and the step actions from one STEP_ACTIONS map with hasAction(). A new templated field or action is added in one place. Pull now also checks `with` templates under keys YAML reads as numbers or booleans, which the engine already rendered.
Pull kept the last valid version of a file without checking it again. A kept parent could then read an output its child had renamed and silently skip the branch, or close a cycle with a child cached in the same sync, so both starts failed with workflow_recursion. A kept copy that runs a workflow file is now checked again, from the source the cache now stores, against what each child settles to in the same sync. A copy that no longer fits is dropped, and the file has no runnable version. Settling follows the versions that would actually run, so a cycle through a kept copy is caught at pull.
4975af4 to
cc799fb
Compare
Earlier findings: Both previously reported failure scenarios are resolved in this diff. Testing: The description supplies sufficient build and representative real-CLI evidence. Deferred provider/agent coverage is acceptable. No PR code was run, built, or installed. |
Pull checked a kept copy for cycles against a child's new revision even when that revision was then rejected, so a valid parent lost its last valid version. Every version is now checked only against the versions the other files are at, and a failing one falls back until none fails; a cycle falls back on the new revisions in it first. A file that cannot be read keeps its last valid version under the same checks, and the cache keeps only what the sync settled.
|
No new findings in the specified diff. Earlier findings: All three previously reported failure scenarios are resolved. Retained parents are revalidated against settled child contracts, and cycle checks use the effective versions, rejecting cyclic revisions before removing valid fallback copies. Testing: The PR description supplies sufficient build and representative real-CLI evidence, including verification on No PR code was run, built, or installed. |
Tencent#1129 merged before Tencent#1126, so by the agreement in Tencent#1126 (comment) this branch carries the adaptation, prepared by @SaulMoro (records-on-graph-runs fd5b709) and re-verified here: - the end record publishes from leaseNext's settleRun (the one settlement point), still after the run lock is released; the reportAttempt publication path is gone; - one timestamp per operation, so a settled run's end record and its run file agree; - definitions looked up by path via the exported locateStep; stepCounts gains skipped; an end-only step that succeeded gets outcome 'success', a skipped step none; - the e2e fixture gains needs (required after the first step now) and passes the driver via startRun's options, not inputs. Three sub-workflow tests from the pre-squash branch layout were stale against main's final engine (they predated the saveCachedCopy/restore mechanics); this restores main's versions of those tests verbatim.
… e2e tests Per review: the two Tencent#1129 e2e tests compare the done envelope with toEqual, so the new record field broke the strict match. Both fixtures end with the end record still pending, so pin record: 'pending' to keep the match strict and document the field.
Closes #1078. Independent of #1126 (run records): whichever merges second adapts run records to graph runs (step lookup by path, a
skippedcount,end). If #1126 lands first without that, I follow up with the adaptation. One commit per slice; the plan is in #1078 (comment).Summary
Workflow steps form a graph:
needs,if, namedends,inputs, and sub-workflows (from a file or inline).Pull checks each file, then checks a step that runs another workflow against that workflow's runnable version: missing child, recursion,
withkeys against the child'sinputs, and reads of outputs or ends the child (file or inline) does not declare. Pull and the engine take the templated fields (with,prompt,env) and the step actions from one list, so they cannot disagree on what is a template.docs/dev/workflow-engine.mdlists the engine rules that span more than one file, for the next change tosrc/workflow/.Pull settles every file's version before any check reads it, so no file is checked against a version that ends up rejected:
Decisions worth a look:
needsis required after the first step;needs: []declares another root. Order alone never implies a dependency.skippedis a step status, not a report outcome: drivers still report onlysuccess | failed | released.command;${{ … }}stays literal elsewhere (GitHub Actions text), but is flagged insidecommand.needsmakesnextreturnrun_unreadableinstead of running later steps as roots.Evidence
Before (
main):needs,if,end,inputs,env,withandworkfloware not step keys (STEP_KEYS = id, command, prompt, prompt_file, agent, output), so the file below fails pull and a failed step fails the whole run.After, real CLI on 62d004f (
npm run build, sandboxedHOME,teamai init --self, the two files above withcheck.yamlclassifying the title):And a typo in an inline workflow read is caught at pull:
Kept copies, real CLI on cc799fb (same setup). A parent whose new revision is invalid no longer keeps a copy that reads an output its child renamed:
Kept copies and cycles, real CLI on f251447 (same setup). Cache
ship -> checkwith a command-onlycheck, then makeship.yamlinvalid andcheck.yamlrunship.Before (cc799fb): the cycle check followed
check's rejected revision and droppedship:After (f251447):
check's new revision is rejected for the cycle, and both keep their last valid version:Tests on f251447, on main 3865561:
npx tsc --noEmit,npm run lint,npx vitest run(9968 passed;gitlab-clone-realspawntimes out here, and also on main), fullnpm run test:e2e(1246 passed; the 6 workflow files pass;dashboard-unifiedtimes out here and on main,submodule-checkout-1041passes when run alone,source-freshness-recallis the macOS/private/varpath also failing on main). A table test runs a parent and its child through accept, keep, delete, unreadable and symlink revisions, a renamed output and a cycle, in both name orders. It starts every workflow each sync caches or keeps and checks the cache holds nothing else. The conformance suite gains scenarios for skipped steps, renderedenv, sub-workflow paths and recovery after a failure.Not checked locally: providers other than
gitand agents other than Claude; left to CI.Merge Danger
Door: two-way
Workflows are new in the 0.28 beta; no stable release carries them. An older teamai reading a cache written by this version runs the graph as a linear list (it fails open), so a downgrade during the beta should be followed by
teamai pull.Blast Radius: workflows
A step after the first now needs
needs, so existing beta workflow files without it fail pull until updated. A workflow file that is a symlink, or a symlinkedworkflows/directory, now loses its cached version at the next pull, like a deleted file. Nothing outsidesrc/workflow/,workflow start --inputs, and the workflow guide changes.Follow-ups noted on the issues they affect: lazy child creation and inline readiness need revisiting for parallel leases (#1079);
envkeys may shadow declared secrets (#1082).