[eshulman2]

Summary

• Add reusable Subworkflow definitions and same-project or built-in workflow includes, resolved into a flat graph before execution.
• Pin expanded graphs and dependency identities for running instances, and validate active consumers before activating a shared revision.
• Extract the common GitHub PR, CI, fix, and review response path into a built-in subworkflow used by the feature, bug, and task workflows.
• Update the workflow CLI, reference docs, architecture docs, and authoring skill for composition and publication.

Verification

• 91 relevant workflow tests passed; one existing transition-budget test was deselected because LangGraph.ainvoke hangs in this environment even for a minimal graph.
• Ruff checks and formatting checks passed.
• Strict Zensical documentation build passed with no issues.

[eshulman2]

Demo: change a shared GitHub flow once and see consumers pick it up

Run these commands from the PR branch with Forge's Redis available. Use a scratch Jira project key for DEMO_PROJECT (or another project where you can create test tickets).

1. Inspect the built-in composition. The first render shows the reusable subworkflow and its named exits; the second shows the expanded Feature graph.

DEMO_PROJECT=MYPROJ
ROOT=src/forge/workflow/declarative/definitions
uv run forge workflow render "$ROOT/github_pr_review.json"
uv run forge workflow render "$ROOT/feature.json"

2. Publish and activate a project-owned copy of the shared flow. Publication stores an immutable revision; activation makes it available to new runs.

uv run forge workflow publish "$DEMO_PROJECT" "$ROOT/github_pr_review.json" --actor demo --reason "shared flow demo"
uv run forge workflow activate "$DEMO_PROJECT" github_pr_review 1 --actor demo --reason "activate shared flow"

3. Make a Feature workflow consume that project definition. This copies the complete built-in Feature process, gives it a separate name, and changes its include from builtin to project. Its six shared PR/CI/review steps are still defined only in the reusable flow.

jq '.metadata.name = "demo-feature" | .spec.includes[0].source = "project"' "$ROOT/feature.json" > /tmp/demo-feature.json
uv run forge workflow validate /tmp/demo-feature.json --project-key "$DEMO_PROJECT"
uv run forge workflow publish "$DEMO_PROJECT" /tmp/demo-feature.json --actor demo --reason "consume shared flow"
uv run forge workflow activate "$DEMO_PROJECT" demo-feature 5 --actor demo --reason "activate demo workflow"

Record the digest printed by validate. To exercise a real run, add forge:workflow:demo-feature to a new Feature or Story ticket in that project.

4. Change the shared flow without republishing the consumer. This example changes the CI fix retry bound from 5 to 4. The expected digest is the active revision 1 source digest.

OLD_DIGEST=$(uv run python -c 'from forge.workflow.declarative.loader import load_workflow_file; print(load_workflow_file("src/forge/workflow/declarative/definitions/github_pr_review.json").digest)')
jq '.metadata.revision = 2 | .spec.steps.attempt_ci_fix.retryBound = 4' "$ROOT/github_pr_review.json" > /tmp/demo-shared-v2.json
uv run forge workflow publish "$DEMO_PROJECT" /tmp/demo-shared-v2.json --actor demo --reason "CI retry update"
uv run forge workflow activate "$DEMO_PROJECT" github_pr_review 2 --expected-active-digest "$OLD_DIGEST" --actor demo --reason "roll out shared update"
uv run forge workflow validate /tmp/demo-feature.json --project-key "$DEMO_PROJECT"

The final validation prints a different expanded digest while demo-feature remains at source revision 5; its expanded attempt_ci_fix.retryBound is now 4. New tickets use the new shared revision. A ticket started before step 4 retains its pinned graph and digest; compare definition.digest in the operator execution view/API for a run started before and after activation.

[JoshSalomon]

Just a question (I did not try this and I did not ask the AI).
If I have process P, which includes sub-process S, at what point in time is the process expanded?
Example - I create S1 and P1
I start process P1 -> S1 has not yet started -> I change S1 to S2 (and publish) --> will I use S1 or S2?
I start process P1 -> S1 started but not completed -> I change S1 to S2 (and publish) --> will I use S1 or S2 (or combo)?
based on the answers on this I may have more questions.

[JoshSalomon]

Code review — feat: compose reusable workflow definitions

Reviewed the new composition.py resolver, the models.py include/subworkflow schema, publication.py validation/activation, builtins.py synchronous expansion, resolver.py pinning, the three rewritten built-in definitions, and the Jira/plan-approval fixes. Findings 1 and 2 were reproduced against a checkout of feature/reusable-workflows.

---

🔴 High

1. Full-Workflow includes rewrite the wrong terminals — src/forge/workflow/declarative/composition.py:109

terminals only matches steps with next == "__end__". But in this codebase normal completion is almost always a routed branch (branches: {"__end__": "__end__"} — see feature.json prd_approval_gate / decompose_epics, github_pr_review.json review_response_gate), while next: "__end__" is used by the failure terminal escalate_blocked.

Reproduced with a child derived from feature: after resolve_definition,

• escalate_blocked.next == "update_documentation" (the caller's returnTo), and
• prd_approval_gate.branches["__end__"] stayed "__end__".

So the included workflow's error path silently falls through into the caller as if it had completed successfully, while its real completion hard-ends the whole parent run. A child that completes only via branches fails outright with the misleading workflow 'X' has no normal completion.

Suggested fix: apply _replace_targets to every child step with {"__end__": include.return_to}, and make terminal detection consider branches values as well as next.

docs/reference/declarative-workflows.md:133 documents the intended semantics. tests/unit/workflow/test_composition.py::test_full_workflow_normal_completion_returns_to_caller only covers the next-based case.

---

🟠 Medium

2. Changing kind for an existing name bypasses the in-flight compatibility gate — publication.py:187 (and :456)

The impact check is now guarded by target.kind == previous.kind == "Workflow". Publishing revision N+1 of an already-active Workflow name as a Subworkflow therefore sets impact = None, _compatible_impact never runs, and activation succeeds.

Reproduced: after activating a Subworkflow over an active Workflow named shared, every subsequent load_project_workflow(...) raises WorkflowValidationError: resolve includes before compiling a workflow — the workflow is bricked.

activate (and ideally publish) should reject a kind change for a name that already has revisions.

3. Dropping the if existing: return short-circuit can duplicate Jira epics — src/forge/workflow/gates/plan_approval.py:137

Reconciliation is now keyed on exact issue.summary[:255] equality. On a resume where plan_draft was regenerated or edited (/forge update ID summary=..., or an LLM re-run that rewords a title), the existing epic no longer matches and a second epic is created under the same forge:parent:<KEY> label.

Additionally, search_issues here still uses the default max_results=50, so a parent with more than 50 existing epics will re-create the unseen ones.

Matching on a stable key — e.g. a per-item idempotency label like the _creation_marker scheme already used in src/forge/effects/jira.py:184 — would be safe. Summary text is not.

4. mypy fails — the repo sets strict = true and uv run mypy src/forge/ is a documented command

composition.py:77: error: Incompatible types in assignment (expression has type "WorkflowDefinition | None", variable has type "WorkflowDefinition")
resolver.py:117:   error: Function is missing a return type annotation   (the `dependency` closure)
cli.py:23:         error: Function is missing a return type annotation   (`_load_resolved`)

Ruff on src/ is clean; only type checking is broken. (tests/unit/workflow/gates/test_plan_draft_state.py is also not ruff format-clean, though tests don't appear to be linted in CI.)

---

🟡 Low

5. Hand-rolled synchronous expansion diverges from composition.expand and fails unsafely — builtins.py:33

Unlike composition.py, this path does no exact exits-set check and no MAX_STEPS check, and it handles unbound exits inconsistently:

• include["exits"][imported["next"][6:]] raises a bare KeyError for an unbound next exit;
• include["exits"].get(target[6:], target) silently leaves @exit/blocked as a literal branch target, surfacing much later as an opaque compiler "undeclared target" error.

It also assumes value["spec"]["steps"] exists, so a built-in whose body is includes-only raises KeyError instead of a RuntimeError. Worth routing through shared asyncio-free logic, or at least mirroring the exact-exit validation.

6. Everything except spec.steps of an included definition is silently discarded — composition.py:52

expand merges only steps. An included definition's spec.entry, observationPolicy, mandatoryPolicies, extensionPoints, and resume.fromRevisions are dropped with no error, and validate_subworkflow does not reject them either — so a subworkflow author who sets them gets no warning and no effect. Either reject those fields on Subworkflow at publication time, or merge them.

7. _validate_consumers re-compiles every active workflow in the project — publication.py:284

It filters only on consumer_name == name and kind != "Workflow", not on whether the consumer actually includes name. This costs a Redis SCAN plus a full resolve + validate_for_publication() per active workflow on every activate/rollback, and any single unrelated active workflow that no longer resolves will block activations of everything else with a confusing error naming the wrong definition. Filtering on include.name in {…} first would scope it correctly. It also skips Subworkflow consumers, which are only covered transitively via a Workflow root.

8. Diamond dependencies report a misleading error — composition.py:118

If W includes A and B and both include C, C's nodes are merged twice and the second pass fails with included node 'X' collides with caller — pointing at a caller collision that does not exist. Tracking already-expanded identities across siblings (or at least emitting a distinct "included twice transitively" message) would make this diagnosable.

9. includes are unusable for projects that author workflows via the Jira project property — resolver.py:112

The dependency() closure only consults definition_reader.active(name); it never falls back to jira.get_project_property, even though the definition itself may have come from that property two branches above. A project that has not adopted Redis publication gets project dependency 'X' is not active with no hint that publication is a prerequisite. If that's intended, the error message should say so.

---

🤖 Generated with Claude Code

[eshulman2]

@JoshSalomon Thanks for the detailed review. I addressed all nine findings in 918e062b:

Finding	Change
1. Full-workflow terminals	Added explicit returnFrom edge selectors. A caller binds only intended completion edges (step or step:outcome) to returnTo; pause and blocked terminals remain __end__. The regression test covers routed completion, wait, and blocked paths. Rewriting every __end__ would also redirect waits and failures.
2. Kind changes	Publishing or activating a different kind for an existing name now fails in both publication stores.
3. Epic retries	Epics now carry a stable per-draft-item label. Retries search all matching Jira issues and reuse marked Epics even after a title edit; ambiguous legacy matches stop rather than create duplicates.
4. Type/format checks	Fixed the three reported annotations/type errors and formatted the affected tests. Targeted mypy, Ruff, and formatting checks pass. The repository still has unrelated full-tree mypy errors.
5. Built-in expansion	Added exact exit binding, step-count, and missing-steps validation, with malformed-artifact tests.
6. Ignored subworkflow fields	Publication validation rejects fields that inclusion cannot apply to the caller. entry remains required for the subworkflow's own validation and rendering; inclusion flattens steps, so the caller must explicitly link to the intended entry node by name.
7. Consumer validation	Activation now validates only direct and transitive active workflow consumers of the changed definition.
8. Diamond dependencies	The collision error now identifies both include paths.
9. Jira property dependencies	The error now says project dependencies must be published and activated; the authoring docs state that prerequisite.

On the expansion timing question: Forge resolves and pins the expanded graph when a new run starts. If P starts with S1 and S2 is activated before P reaches S, that run still uses S1. If S2 is activated while P is inside S, the run also stays on S1. A later new run uses S2. Publishing S2 alone does not change new runs until S2 is activated. Checkpoint resumes load the pinned expanded artifact, not today's active dependency.

The focused regression suite passed (29 tests). The new PR CI run is still in progress; lint, contract tests, Helm lint, and integration tests have passed so far.