You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat: add specify workflow definition for run-scoped definitions - #4912
Adds specify workflow definition <run_id> --json, a read-only command that
returns the workflow definition a run was started with. It reads only the
persisted run snapshot and frozen definitions for workflow calls that were
actually bound during execution, so output does not drift after installed
workflows are edited, overlaid, or removed.
definition is the persisted, overlay-composed root definition, passed
through without filling defaults or restructuring fields.
workflow_scopes is always present and contains each bound workflow call's scope_path, workflow ID, and frozen definition. Its paths and workflow IDs
match workflow status --json.
RunState.definition_path, load_definition(), and load_workflow_scopes() make the snapshot read surface explicit; resume and
definition share the same persisted snapshot path.
--json is required. Successful output is one JSON object on stdout; errors
are one {"error": ...} object on stderr.
Strict JSON output stringifies YAML-native values, non-string mapping keys,
and non-finite floats, and rejects recursive YAML aliases with the same
strict JSON error contract.
Decisions that diverge from the original issue
Output mirrors the persisted workflow structure rather than defining a new,
normalized definition model. schema_version therefore versions the exposed
definition shape; the JSON envelope is additive-only. This avoids partially
applying engine defaults and preserves the definition that was executed.
The command is JSON-only for now. It requires --json and exits 2 without it;
a human text renderer is intentionally deferred rather than introducing a
second output contract.
A composed run includes only definitions that are already bound in its
execution tree. Unreached calls are not resolved from current installed
workflows, because doing so would reintroduce definition drift.
The root definition remains authored: call steps are not expanded inline.
Frozen callees appear separately in workflow_scopes, which also supports a
call invoked more than once through loops or fan-out.
Errors follow the established artifact strict-JSON contract, including
project-resolution and unsafe-workflow-storage failures. workflow status --json intentionally retains its existing Rich error rendering.
Dynamically generated steps are out of scope. Steps that a custom step type
produces at run time through StepResult.next_steps are persisted in the
execution state but are not exported; definition and workflow_scopes
cover only the authored definition and bound workflow calls. The
documentation states this limit.
Follow-ups
[Bug]: Harden persisted workflow run artifact access against symlinks #4914 will harden persisted workflow-run artifact access at the shared
storage boundary. The symlink concern predates this command: resume already
reads the same persisted snapshot and run-state loading reads sibling files.
A command-local check would leave those consumers inconsistent.
Ran focused workflow, overlay, preset, extension, and engine tests: 1996 passed, 1 skipped.
Ran uvx ruff@0.15.0 check src tests.
Ran git diff --check.
Ran the primary workflow suites again: 850 passed.
Ran tests/specify_cli/workflows/test_command_definition.py: 36 passed,
including recursive YAML alias rejection.
Manually tested the working-tree CLI in /tmp/test-workflow-run-definition:
initialized a project, ran root and composed paused workflows, verified the
persisted snapshot and JSON envelope, checked workflow_scopes parity with status --json, exercised required-flag and unknown-run errors, and confirmed
unchanged output after editing then removing the installed child workflow.
The full suite was attempted twice. Both runs showed the known pre-existing
failure tests/integration/test_preset_update_workflow.py::test_preset_update_cli_contract,
which expects an older Typer missing-argument string; the isolated test still
fails unchanged. The second full run continued cleanly through 83% before the
available execution timeout.
AI Disclosure
I did not use AI assistance for this contribution
I did use AI assistance (fill in the disclosure below)
AI disclosure: Implemented and updated with OpenCode using gpt-5.6-terra
(github-copilot/gpt-5.6-terra) in autonomous mode with default reasoning.
The agent generated implementation, tests, documentation, follow-up issues, and
review responses from the reviewed issue plan and review decisions, and ran the
listed automated and manual checks. No claim of human line-by-line review is
made.
Add a strict JSON command for inspecting the definition persisted with a workflow run, including frozen definitions for bound workflow calls.\n\nCloses github#4792.\n\nAssisted-by: OpenCode (model: gpt-5.6-terra, autonomous)
Manual validation completed after opening this PR. Using the working-tree CLI in a disposable initialized project, I verified root and composed paused runs, persisted snapshots, definition/status scope parity, required-flag and unknown-run errors, and stable definition output after editing and removing the installed child workflow.
AI disclosure: Posted by Markus Wondrak with OpenCode (github-copilot/gpt-5.6-terra), default reasoning, autonomous mode; the agent performed and summarized the manual CLI validation.
Fixed the recursive YAML-alias finding. Strict workflow JSON conversion now rejects cyclic mappings, lists, and tuples, while allowing non-recursive aliases reused across separate branches. workflow definition --json reports this through its single JSON stderr error envelope.
Added a command-level regression test covering snapshot persistence of a recursive YAML alias and the strict error contract.
Validation: 36 passed for tests/specify_cli/workflows/test_command_definition.py; 133 passed across definition, run, resume, and status command suites; Ruff and git diff --check passed.
AI disclosure: Posted by Markus Wondrak with OpenCode (github-copilot/gpt-5.6-terra), default reasoning, autonomous mode. The agent implemented the fix, tests, follow-up issues, and this review summary, and ran the stated automated checks; no claim of human line-by-line review is made.
Stringified mapping keys can silently overwrite existing keys
src/specify_cli/workflows/_commands.py:1082
Stringifying a non-string mapping key inside this comprehension can silently overwrite an existing string key. For example, YAML containing both an unquoted 2026-01-01 key (loaded as date) and a quoted "2026-01-01" key has two entries before this code and only the latter afterward, so the advertised persisted definition is returned with data missing. Normalize keys explicitly and reject collisions (with a negative test) rather than allowing the comprehension to drop an entry.
Scope rendering adds avoidable traversal and per-node memory
src/specify_cli/workflows/_execution.py:368
This refactor turns scope rendering from one tree walk into two and retains an additional entry for every execution node in nodes_by_path. Since workflow trees grow with loop iterations and fan-out items and status is commonly polled, this adds avoidable O(nodes) memory and a full traversal even when there are few or no bound scopes. Iterate the nodes directly (as before), or make the shared iterator yield the node together with the path and binding.
Canonicalize every mapping key to the JSON property name json.dumps emits
before inserting it, and raise when two keys collapse to the same name.
Previously the definition command could emit duplicate JSON properties for
keys such as 1 and "1", and json.loads then silently dropped an entry.
Assisted-by: OpenCode (model: deepseek-v4.1-flash, autonomous)
Addressed the duplicate-canonical-key finding in a08f50e.
_emit_workflow_json now canonicalizes every mapping key to the JSON property name json.dumps emits before insertion, and raises ValueError when two keys collapse to the same name (e.g. 1/"1", true/"true", or a date and its string form). That routes through the existing strict-JSON error path (stdout empty, exit 1) instead of emitting duplicate properties that json.loads silently truncates. Added a parametrized regression test for int/string, bool/string and date/string collisions, and documented the behavior in docs/reference/workflows.md.
AI disclosure: posted by OpenCode (model: deepseek-v4.1-flash, autonomous) on behalf of @markuswondrak. The human selected and directed the fix and reviewed the plan; the code was not line-by-line human-reviewed before commit.
YAML alias DAG expansion causes exponential resource usage
src/specify_cli/workflows/_commands.py:1109
active_containers rejects cycles but forgets each container after returning, so a non-recursive YAML alias DAG is expanded independently at every reference. A linear-size definition such as xN: &xN [*xN-1, *xN-1] therefore causes exponential CPU, memory, and stdout growth; the repository already treats this exact pattern as a regression hazard in tests/test_resolve_template_python_parity.py:1088-1095. Reject repeated container identities or enforce a bounded expansion/output policy, and add a negative test for a deep non-cyclic alias graph.
`_emit_workflow_json` rejected recursive YAML aliases but re-expanded a
non-recursive alias DAG independently at every reference, so a linear-size
definition such as `xN: &xN [*xN-1, *xN-1]` produced exponential CPU,
memory, and stdout. Add a node budget that raises the existing strict-JSON
error instead of rejecting every reused alias, and cover both a bounded
shared alias and a deep non-cyclic DAG.
Assisted-by: opencode (model: deepseek-v4.1-flash, autonomous)
Addresses the alias-expansion finding from review 5494817862.
_emit_workflow_json now enforces a node budget in plain_json, so a non-recursive YAML alias DAG raises the strict-JSON error instead of expanding exponentially. The budget bounds expansion rather than rejecting every reused alias, so legitimate shared anchors still emit.
Added a negative test (deep non-cyclic DAG) and a positive test (bounded shared alias).
Regression evidence: the negative test fails on the parent commit (exit_code 0, ~31s emitting the exponential payload at depth 20) and passes after.
Updated docs/reference/workflows.md (Plain JSON and Errors).
100000 still leaves >1000x headroom over the largest shipped workflow
definition (~67 nodes) while capping worst-case memory and stdout.
Assisted-by: opencode (model: deepseek-v4.1-flash, autonomous)
Python 3.14's JSON decoder reports truncated deeply nested input as a
decode error rather than RecursionError, so the message differs.
Assisted-by: GitHub Copilot (model: Claude Sonnet 5.5, supervised)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The documented and tested gate lookup can select the wrong step when fan-out templates reuse an ID.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Resolve colliding fan-out and root step IDs by scope
docs/reference/workflows.md:146
Fan-out templates have separate ID namespaces (engine.py:592-603), so step IDs are not unique across the entire definition. A root shell g and a fan-out gate g are valid, but this rule—and _gate_scope() in test_command_definition.py—selects the shell for fan:g:0. Integrations can therefore render the wrong gate configuration. Use the remaining gate.scope_path to select the enclosing container or template before matching the authored ID. Update the test helper and add a regression with these colliding IDs.
AI disclosure: GitHub Copilot (model unknown, autonomous), reviewing for the requester; analysis, comment, and suggested fix are AI-generated.
…rtion
Fan-out templates are validated in their own ID namespace, so a root step
and a template step may share an ID. The documented gate join claimed IDs
are unique per definition and the test helper picked the first depth-first
match, so `fan:g:0` resolved to a root shell named `g`. The rule now looks
only inside the fan-out template that `gate.scope_path` runs through, and
ignores templates otherwise. Adds a regression test that failed before.
test_deeply_nested_state_returns_json_error asserted decoder-specific text
behind a Python version gate; whether RecursionError or a decode error
surfaces depends on the platform, so it now asserts the run-scoped message.
Assisted-by: pi coding agent (model: opencode-go/deepseek-v4.1-flash, autonomous)
CI failure, pytest (windows-latest, 3.14):test_deeply_nested_state_returns_json_error gated its expected text on sys.version_info >= (3, 14). Which error the decoder raises for deeply nested input depends on the platform, not the version (3.14 on Linux also raises RecursionError), so the test now asserts only the run-scoped message Invalid run state for <run_id>. No production change.
Fan-out ID collision (Copilot review, docs/reference/workflows.md): Confirmed with the CLI: with a root shell g and a fan-out template gate g, the documented join resolved fan:g:0 to the shell. Step 2 of "Matching a gate to its definition" no longer claims IDs are unique across the whole definition. It now says to look only inside the fan-out template that gate.scope_path runs through, and to ignore fan-out templates otherwise. The _gate_scope() test helper implements the same rule, and a new regression test fails on the previous helper (returns the shell) and passes now. The command's output is unchanged; only the documented consumer rule and its helper were wrong.
Validation: tests/specify_cli/workflows and tests/test_workflows.py 1443 passed, 1 skipped; uvx ruff@0.15.0 check src tests clean; markdownlint-cli2 docs/reference/workflows.md clean. The Windows 3.14 job has not been re-run locally; CI will confirm it.
AI disclosure: posted on behalf of @markuswondrak by the pi coding agent (model: opencode-go/deepseek-v4.1-flash, reasoning high), autonomous mode in a session directed by the contributor, who chose the scope of the fix. The agent investigated the findings, wrote the change and the comment, and ran the checks above; no claim of human line-by-line review is made.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The documented gate-matching contract must account for omitted scope paths to prevent integration failures.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Document optional scope_path for root workflow gates
docs/reference/workflows.md:143
For a gate directly in the root workflow, _gate_outcome() omits scope_path (src/specify_cli/workflows/_commands.py:1030–1031). A consumer that reads this field unconditionally before matching prefixes will fail on that common case. Document that it is optional and that consumers must treat a missing value as [], as the test helper already does.
AI disclosure: GitHub Copilot (model unknown), automated review for the requester; analysis and suggested documentation change are AI-generated.
`status --json` only emits `gate.scope_path` when it is non-empty, so a
consumer reading it unconditionally fails on a gate directly in the root
workflow. The gate-matching procedure now says to treat a missing value as
`[]`, as the test helper already does.
Assisted-by: pi coding agent (model: opencode-go/deepseek-v4.1-flash, autonomous)
Root gate has no scope_path (Copilot review, docs/reference/workflows.md): Confirmed: _gate_outcome() emits gate.scope_path only when it is non-empty, so a gate directly in the root workflow has no such field. Step 1 of "Matching a gate to its definition" now says to read a missing gate.scope_path as []. Docs only; no behavior change. markdownlint-cli2 is clean and test_command_definition.py still passes.
AI disclosure: posted on behalf of @markuswondrak by the pi coding agent (model: opencode-go/deepseek-v4.1-flash, reasoning high), autonomous mode in a session directed by the contributor. The agent verified the finding, wrote the change and this comment; no claim of human line-by-line review is made.
The byte-budget fix still misses YAML-native values converted through default=str. A !!binary value reused through aliases can remain well below the node limit while exceeding the output cap.
Please account for the serialized representation of these values on every occurrence, including alias references, and enforce the limit before materializing the complete oversized output. Add a regression test using !!binary with aliases that produces the existing JSON error with no stdout when over budget, plus a smaller case that succeeds. This should remain scoped to definition output; existing uncapped callers should retain their behavior.
`default=str` converted YAML-native values that the byte budget never saw.
A `!!binary` value reused through aliases stayed under the node limit while
its repr string was repeated into oversized output, and a `!!set` had the
same gap and also serialized in hash-randomized order.
Binary values now appear as a `<binary: N bytes>` placeholder; the content
stays in the run snapshot. Sets become lists sorted by JSON form, with each
member charged to the budget on every occurrence. Other non-JSON scalars are
stringified and charged. `default=str` is dropped because every non-native
value is converted explicitly. The budget stays opt-in, so uncapped callers
are unchanged.
Adds a CLI test for aliased `!!binary` and for set ordering, unit tests for
the set byte budget and the opt-in behavior, and documents the output in
docs/reference/workflows.md. The binary and set tests fail on the parent
commit and pass now.
Assisted-by: GitHub Copilot CLI (model: Claude Sonnet 5.5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Binary values (!!binary) are no longer emitted. Each occurrence appears as "<binary: N bytes>", so a value reused through aliases cannot grow the output. The content remains in the run snapshot.
Sets (!!set) are emitted as lists sorted by JSON form, and each member is charged to the byte budget on every occurrence. This also removes the hash-randomized ordering that default=str produced.
Other non-JSON scalars (dates) are stringified and charged. default=str is removed because every non-native value is now converted explicitly. The budget is still opt-in through max_nodes, so status, run and resume are unchanged.
docs/reference/workflows.md describes the placeholder, the sorted sets and the output-size budget.
Deviation from the requested test: the !!binary regression test was specified to fail with the JSON error and no stdout when over budget. With a placeholder a binary value can no longer exceed the budget, so the test asserts a small successful output for 64 KiB reused through 30 aliases (about 10 MB as a repr string, which the parent commit rejected). The over-budget error case is covered for sets, which are charged per member. If binary values should instead be emitted and capped, say so and I will change it.
Validation: the new binary, set-order and set-budget tests fail on 12b6512 and pass on 7a81012. The 50 tests in test_command_definition.py that need only project_dir and capsys pass locally. They ran through a stand-in runner because pytest could not be installed in this environment. The full suite, ruff and markdownlint were not run locally, so CI will confirm them.
AI disclosure: posted on behalf of @markuswondrak by GitHub Copilot CLI (model: Claude Sonnet 5.5), autonomous mode in a session directed by the contributor, who chose the approach (placeholder for binary, sorted list for sets). The agent wrote the change, tests, docs and this comment; no claim of human line-by-line review is made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
author-awaitingWaiting on author responsetriage-can-waitVerdict: valid and in-scope but deprioritized; held behind the evidence gate
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Closes #4792.
Adds
specify workflow definition <run_id> --json, a read-only command thatreturns the workflow definition a run was started with. It reads only the
persisted run snapshot and frozen definitions for workflow calls that were
actually bound during execution, so output does not drift after installed
workflows are edited, overlaid, or removed.
definitionis the persisted, overlay-composed root definition, passedthrough without filling defaults or restructuring fields.
workflow_scopesis always present and contains each bound workflow call'sscope_path, workflow ID, and frozen definition. Its paths and workflow IDsmatch
workflow status --json.RunState.definition_path,load_definition(), andload_workflow_scopes()make the snapshot read surface explicit; resume anddefinition share the same persisted snapshot path.
--jsonis required. Successful output is one JSON object on stdout; errorsare one
{"error": ...}object on stderr.and non-finite floats, and rejects recursive YAML aliases with the same
strict JSON error contract.
Decisions that diverge from the original issue
Output mirrors the persisted workflow structure rather than defining a new,
normalized definition model.
schema_versiontherefore versions the exposeddefinition shape; the JSON envelope is additive-only. This avoids partially
applying engine defaults and preserves the definition that was executed.
The command is JSON-only for now. It requires
--jsonand exits 2 without it;a human text renderer is intentionally deferred rather than introducing a
second output contract.
A composed run includes only definitions that are already bound in its
execution tree. Unreached calls are not resolved from current installed
workflows, because doing so would reintroduce definition drift.
The root definition remains authored: call steps are not expanded inline.
Frozen callees appear separately in
workflow_scopes, which also supports acall invoked more than once through loops or fan-out.
Errors follow the established artifact strict-JSON contract, including
project-resolution and unsafe-workflow-storage failures.
workflow status --jsonintentionally retains its existing Rich error rendering.Dynamically generated steps are out of scope. Steps that a custom step type
produces at run time through
StepResult.next_stepsare persisted in theexecution state but are not exported;
definitionandworkflow_scopescover only the authored definition and bound workflow calls. The
documentation states this limit.
Follow-ups
storage boundary. The symlink concern predates this command: resume already
reads the same persisted snapshot and run-state loading reads sibling files.
A command-local check would leave those consumers inconsistent.
coverage. A
workflow.definition-only disposition would leave every existingworkflow leaf unaccounted for.
Testing
1996 passed, 1 skipped.uvx ruff@0.15.0 check src tests.git diff --check.850 passed.tests/specify_cli/workflows/test_command_definition.py:36 passed,including recursive YAML alias rejection.
/tmp/test-workflow-run-definition:initialized a project, ran root and composed paused workflows, verified the
persisted snapshot and JSON envelope, checked
workflow_scopesparity withstatus --json, exercised required-flag and unknown-run errors, and confirmedunchanged output after editing then removing the installed child workflow.
The full suite was attempted twice. Both runs showed the known pre-existing
failure
tests/integration/test_preset_update_workflow.py::test_preset_update_cli_contract,which expects an older Typer missing-argument string; the isolated test still
fails unchanged. The second full run continued cleanly through 83% before the
available execution timeout.
AI Disclosure
AI disclosure: Implemented and updated with OpenCode using
gpt-5.6-terra(
github-copilot/gpt-5.6-terra) in autonomous mode with default reasoning.The agent generated implementation, tests, documentation, follow-up issues, and
review responses from the reviewed issue plan and review decisions, and ran the
listed automated and manual checks. No claim of human line-by-line review is
made.