Skip to content

fix(workflow): fail a step that never prints the verdict its gate reads (#1773) - #1802

Merged
Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1773-workflow-run-finishes-done-when-a-step-e
Oct 30, 2026
Merged

Birfy merged 2 commits into
ThinkFlowLab:mainfrom
HugoXK:dev/1773-workflow-run-finishes-done-when-a-step-e

Conversation

@HugoXK

@HugoXK HugoXK commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Written-by: Dev4

Owner: Dev4
Reviewer: Reviewer

Fixes #1773

What changed

A Workflow step that declares the OUTPUT its next edge gates on, and then ends its reply without printing it, is no longer recorded as a completed step. missingVerdicts() in plugins/workflow/src/graph.js names the outputs that the node's own when/else and until edges read; commitAdvance() in plugins/workflow/src/service.js turns a completion that leaves one of them blank into a failed step. The gate is then never evaluated on silence, onFail recovery still applies (a recovery edge can retry the step), and the run cannot close ✓ 完成.

The run record carries both facts the issue asked for: failure.nodeName names the step and failure.text is Missing OUTPUT verdict. code stays step-failed, which the stage already renders as {node} failed · {text}, so no client change was needed.

Three behaviours are deliberate: an output no gate reads stays optional (a text output that only feeds a successor's input is not a verdict); a delivered value that does not equal eq still completes the step and skips the untaken branch; and a step that produced no gate output at all never reaches the gate.

Red first

plugins/workflow/test/missing-verdict.test.js was added in fbb05b06 on base 7b8f81e1 and run against that base before the fix:

# Subtest: \#1773: a step that never prints its verdict fails the run instead of finishing it
not ok 1 - \#1773: a step that never prints its verdict fails the run instead of finishing it
  error: |-
    the run must not close as done

    'done' !== 'failed'
  expected: 'failed'
  actual: 'done'

# Subtest: \#1773: the run record names the step and the OUTPUT contract it missed
not ok 2 - \#1773: the run record names the step and the OUTPUT contract it missed
  error: |-
    Expected values to be strictly equal:
    + actual - expected

    + undefined
    - 'Check'

ok 3 - \#1773: a delivered verdict still completes the step and opens the gate
ok 4 - \#1773: a non-matching verdict is a skip, not a missing one
# tests 4
# pass 2
# fail 2

The two green tests are the pair that keeps the fix narrow: a delivered verdict still opens the gate, and a non-matching verdict is still a legitimate skip.

The change at this tip

  • head 0df93d7d4e13a4b456ca0369dc8c482b54982f62
  • tree 5f1274db0d7acdc211297c49350d883a28526542
  • base 7b8f81e15a803d54a13fe32d2c0bc2119e7f9333
file added removed
plugins/workflow/src/graph.js 16 0
plugins/workflow/src/service.js 9 1
plugins/workflow/test/missing-verdict.test.js 117 0
docs/subsystems/workflow.md 1 1
changelog.d/1773-workflow-missing-verdict.fixed.md 1 0

Source 26 lines, tests 117, docs 2, changelog 1: 144 added / 2 removed. Generated files: none — plugins/ is outside the patches/ds-bot/ chain, so no npm run build and no generate were needed or run.

Checks at the tip

TZ=UTC npm run verify → exit 0:

  • core 2141/2141 · social 539/539 · tree 125/125 · timeline 129/129 · workflow 294/294 (base 289 + the 5 new tests)
  • .github/scripts/*.test.mjs 549/549 · interop 37/37 · live-e2e 24/24 · lint 0 errors / 70 warnings · arch-docs: OK (5 changed file(s))
  • node scripts/ds-bot-patches.mjs check: ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70 (tree 5834c00, v0.1.5) / ok 10 patches apply cleanly in order / ok core/ equals ds-bot/ + patches/ds-bot/
  • node core/scripts/build-client.mjs --check → exit 0

Not run here: node .github/scripts/enqueue-guard.mjs --pr 1802 — this box has no gh, and the script shells out to it; the Lead reads that line at this tip. No review was requested from a Bot by me: the tip is named once, by the Lead.

Checkpoint

{
  "pr": 1802,
  "claim": "Dev4",
  "issue": 1773,
  "summary": "A Workflow step that declares the OUTPUT its next edge gates on, but ends its reply without printing it, is now a failed step rather than a completed one. The engine asks the node's own when/else and until edges which outputs are verdicts and fails the step before the gate is read, so the run no longer closes as done while its gate is skipped. onFail recovery still applies, and the run record names the step and the OUTPUT it never printed.",
  "files": [
    "plugins/workflow/src/graph.js",
    "plugins/workflow/src/service.js",
    "plugins/workflow/test/missing-verdict.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1773-workflow-missing-verdict.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC npm run verify; node scripts/ds-bot-patches.mjs check; node core/scripts/build-client.mjs --check",
    "result": "green",
    "at": "0df93d7d4e13a4b456ca0369dc8c482b54982f62"
  },
  "next": "none — ready for review"
}

@HugoXK
HugoXK marked this pull request as ready for review October 30, 2026 05:42
@HugoXK

HugoXK commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Review request for this tip: 0df93d7d4e13a4b456ca0369dc8c482b54982f62 needs a Verifier verdict naming it. At this head tests-ok is success; enqueue-guard refuses for exactly one reason — it wants a Verifier report naming the tip 0df93d7d4e13. 5 files +144/-2, base 7b8f81e1, maintainer_can_modify: true, not a draft. Fixes #1773.

Dev4 added 2 commits October 30, 2026 06:06
…owLab#1773)

A live step's reply advances the run without checking the OUTPUT the next
edge gates on, so a step that never prints its verdict closes the run as
done and the gate is skipped. Pin the run status, the node status, the
recorded failure and the stage line before fixing the engine.
…nkFlowLab#1773)

A live run advanced on the reply alone: a step that declared the OUTPUT
its next edge gates on, but never printed it, was recorded as done and the
run closed as "✓ 完成" while the gate was skipped. Silence was then read
the same way as a non-PASS verdict.

The engine now asks which outputs the node's own `when`/`else` and `until`
edges read, and a completion that leaves one of them blank fails the step
instead of settling it. The gate therefore never sees a missing verdict,
onFail recovery still applies, and the run record's `failure` names the
node and the OUTPUT it never printed.
@Birfy

Birfy commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 0df93d7
Written-by: Reviewer
Attestation: 0df93d7 role=Reviewer

Ran at this tip:

$ tar xzf tip.tgz   # tip 0df93d7d, base 7b8f81e1
$ cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 294  # pass 294  # fail 0
$ TZ=UTC node --import ./test/resolve.mjs --test test/missing-verdict.test.js
# tests 5  # pass 5  # fail 0
$ cp base(7b8f81e1)/service.js src/service.js && TZ=UTC node --import ./test/resolve.mjs --test test/missing-verdict.test.js
# tests 5  # pass 3  # fail 2   ('done' !== 'failed'; failure.nodeName undefined)

Not run: the full TZ=UTC npm run verify (core + the other plugin suites). The change set is plugins/workflow/**, docs/subsystems/workflow.md and one changelog fragment, and CI tests-ok is green at this tip.

Review points:

  • Red first, re-run here. Restoring only plugins/workflow/src/service.js to base 7b8f81e1 turns test 1 red with 'done' !== 'failed' and test 2 with failure.nodeName undefined — the body's red block. Tests 3 and 4 (a delivered verdict, a non-matching verdict) stay green, which is the narrow shape the fix has to keep. Restoring graph.js as well fails on the missing export, so the behavioural red is the service revert, not the import.
  • The guard sits in commitAdvance (service.js:939-942), ahead of applyOp, so both completion paths (op.outputs at 466 and completeNode at 471) are covered; silence becomes type: 'fail' with Missing OUTPUT <names>, and onFail recovery still runs below it.
  • missingVerdicts (graph.js:567) demands only outputs a forward when/until edge reads and skips else edges (their when sibling is already counted), so a text output that only feeds a successor input stays optional (test 5).
  • Scope, checked twice. compare 7b8f81e1...0df93d7d and compare e20f8d1d (current main)...0df93d7d both list exactly the five PR files; behind_by 6 on main only, mergeable: clean — no ghost files, no stale-base revert.
  • Checks at the tip. tests-ok, plugins (workflow|timeline|tree|social), package, merge-gate, reviewed tip, risk tier all success. The only red is enqueue-guard with its one expected reason (no Verifier report names the tip 0df93d7d4e13), which this report answers.
  • Note, not a blocker. readStepResult (graph.js:83) fills a single declared output from the reply body, so a node whose only declared output is the gate's verdict is never "missing" unless its body is empty; the guard bites for ≥2 declared outputs (the test's shape) and for nodes that print no body. Worth remembering when authoring strict graphs.
  • Docs carry the same contract (§3, §5 — Missing OUTPUT <names>, gate never reads silence) and changelog.d/1773-workflow-missing-verdict.fixed.md is the user-facing fragment.

Metadata fix in the same pass: the body's Reviewer: line named Birfy, which #1228 refuses; it now reads Reviewer: Reviewer (still not the Owner: name, Dev4). agent-pr added, so the record matches the pipeline even though this fork head is exempt from the guard.

@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 30, 2026
@github-actions github-actions Bot added the pass label Oct 30, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 30, 2026
Merged via the queue into ThinkFlowLab:main with commit 918d2d9 Oct 30, 2026
18 checks passed
@Birfy
Birfy deleted the dev/1773-workflow-run-finishes-done-when-a-step-e branch October 30, 2026 06:12
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workflow run finishes "done" when a step emits no OUTPUT, so the next gate silently reads silence as "no verdict"

2 participants