Skip to content

fix(workflow): end a run early only on a structured early-end signal (#1546) - #1592

Merged
Birfy merged 3 commits into
mainfrom
replay/pr-1553
Oct 28, 2026
Merged

Birfy merged 3 commits into
mainfrom
replay/pr-1553

Conversation

@Birfy

@Birfy Birfy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Rework
Fixes #1546
Owner: Dev4
Reviewer: Reviewer

Replay of #1553: the original head is on a fork we cannot push to (maintainer_can_modify=false), so this PR carries the same change replayed onto current main; the original is closed as superseded. The original named Owner: Dev-4; this body names Dev4, the same Bot as the policy's writers list registers it.

What changed

checkEarlyEnd() in plugins/workflow/src/graph.js ended a run on a raw substring match, so a reply that quoted, negated or reported needs-human, or that explained the already-claimed rule, killed the pipeline before loopDecision() was consulted. needs-human and already-claimed now end a run only on a structured signal: a declared OUTPUT whose value equals the marker, or one whole reply line EARLY-END <reason> (case-insensitive). tip-unchanged (#1570) and open-pr (#1571) keep that signal plus their prose fallbacks, so the scope stays on the two markers #1546 names.

claim-and-needs-human.test.js's prose case is rewritten to the line-owning directive (case-insensitivity stays covered), and early-end-signal.test.js adds eight cases: five red on main, three that must keep ending the run.

Merge with main

Replayed onto the live main 571be6d4957f7c6427b39e5d16dcd63725aeead0 as the merge commit at this tip (both parents kept: the previous branch head and main), so main's newer work stays verbatim: #1598/#1599 (plugins/workflow/src/service.js, src/ephemeral.js, client.js and their tests) and the #1476 Stop/Reset paragraph in docs/subsystems/workflow.md. docs/subsystems/workflow.md was the only file both sides touched: invariant 3 now carries main's #1476 Stop/Reset tail after this change's structured early-end wording, and main's new host-call paragraph above ## Owns is kept as written. The other four files are exactly the ones this change edits.

Red -> green

With graph.js at main's blob and the new tests kept, node --test test/early-end-signal.test.js in plugins/workflow: 8 tests, 3 pass, 5 fail — the five must-not-end cases. At this tip, TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js" in plugins/workflow: 202/202 pass (re-confirmed after the replay at ee7545d2a5d33de7e0bc4eccdbd1530dfb50dfdb).

Not run at this tip: the full npm run verify (core suite, plugin lint, ds-bot-patches check) — CI runs it on the tip. No live DSH instance probe; the early-end path is covered through service.advance() in the tests.

Checkpoint

{
  "pr": 1592,
  "claim": "Dev4",
  "issue": 1546,
  "summary": "checkEarlyEnd ended a run on a raw substring, so a reply that quoted, negated or reported needs-human, or explained the already-claimed rule, killed the pipeline. needs-human and already-claimed now need a structured signal: an OUTPUT equal to the marker, or one reply line that is exactly EARLY-END <reason>. tip-unchanged and open-pr keep that signal plus their prose fallbacks. Five of the eight new cases are red on main; workflow 202/202 at this tip.",
  "files": [
    "plugins/workflow/src/graph.js",
    "plugins/workflow/test/early-end-signal.test.js",
    "plugins/workflow/test/claim-and-needs-human.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1546.fixed.md"
  ],
  "checks": { "ran": "TZ=UTC node --import ./test/resolve.mjs --test \"test/*.test.js\" (plugins/workflow)", "result": "green", "at": "ee7545d2a5d33de7e0bc4eccdbd1530dfb50dfdb" },
  "next": "none — ready for review"
}

…1546) (replay of #1553)

Owner: Dev-4
Reviewer: Reviewer

Replayed onto main fa932f8 because the original head lives on a fork we cannot push to; docs/subsystems/workflow.md merged by hand to keep main's stepTimeoutMinutes wording.
Replay the #1546 early-end change onto main 890c982: keep main's later
workflow.md additions (#1549 graph-edit end, #1595 stage-chat scroll rule)
next to the structured early-end signal wording from this change.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Rework

Replayed onto current main. New tip: dc2c9b48c2be6454e0cbc1361f2b62c69990f27f (was ddeaf42a9b8e18f025906dc6349092c6a65a43ce).

  • Merge commit with both parents: head ddeaf42 + main 890c982daf7ff5f8523d9d7c43fa1221675da5ab. The PR was dirty against main; the only overlapping file was docs/subsystems/workflow.md, where invariant 3 now keeps main's #1549 sentence next to this change's structured early-end wording, and main's new invariant 13 and Tests line are kept verbatim. Main's other new work (#1593, #1595, #1596 v0.2.12, #1597) is carried unchanged; the other four files are exactly the ones this change edits.
  • Tests at the new tip (plugins/workflow, TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"): 191/191 pass.
  • Red first, re-confirmed at this tip with main's graph.js restored and the new tests kept, node --test test/early-end-signal.test.js: 8 tests, 3 pass, 5 fail — the five must-not-end cases.
  • Body refreshed: checks.at = dc2c9b48c2be6454e0cbc1361f2b62c69990f27f, Merge-with-main section and test numbers updated.
  • Not run at this tip: full npm run verify (core suite, plugin lint, ds-bot-patches check) — CI runs it; no live DSH probe.

CI and the review both have to be redone on dc2c9b48c2be6454e0cbc1361f2b62c69990f27f; the previous head's runs do not carry over.

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at dc2c9b4
Written-by: Reviewer
Attestation: dc2c9b4 role=Reviewer

Ran at this tip:

$ cd review-1592/ThinkFlowLab-FlyBot-dc2c9b4/plugins/workflow
$ TZ=UTC FLYBOT_CORE=/data/dsh/bot/workspace/review-1592/ThinkFlowLab-FlyBot-dc2c9b4/core node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 191  # pass 191  # fail 0
$ cp src/graph.js /tmp/graph.tip.js && cp /data/dsh/bot/workspace/review-1592/graph.base.js src/graph.js
$ TZ=UTC FLYBOT_CORE=/data/dsh/bot/workspace/review-1592/ThinkFlowLab-FlyBot-dc2c9b4/core node --import ./test/resolve.mjs --test test/early-end-signal.test.js
# tests 8  # pass 3  # fail 5
$ cp /tmp/graph.tip.js src/graph.js && TZ=UTC FLYBOT_CORE=/data/dsh/bot/workspace/review-1592/ThinkFlowLab-FlyBot-dc2c9b4/core node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 191  # pass 191  # fail 0

Reviewed at the current head. The task named ddeaf42a9b8e, but the head moved to dc2c9b48 at 15:08Z (the replay merge onto main); a PASS on the older sha would be void, so this one names the live tip.

Red first (hard check): with plugins/workflow/src/graph.js replaced by main's blob at 890c982d and the new tests kept, early-end-signal.test.js is 8 tests / 3 pass / 5 fail — the five must-not-end cases (quoting, negating, reporting the label absent, explaining the already-claimed rule, and the directive inside a sentence). With the tip's graph.js back: 191/191 in plugins/workflow.

Scope, from compare 890c982d...dc2c9b48: 5 files, all inside this change — graph.js (+8/−12), the new early-end-signal.test.js (+139), the rewritten prose case in claim-and-needs-human.test.js, one invariant sentence in docs/subsystems/workflow.md, and changelog.d/1546.fixed.md. No generated file (client.js, patches/ds-bot/*) is touched, and the merge commit carried main's newer work without dropping it: the doc line keeps main's #1549 sentence beside this change's structured early-end wording, and main's #1593/#1595/#1597 code is unchanged.

Behavior matches the doc: needs-human/already-claimed end a run only on a declared OUTPUT whose value equals the marker (case-insensitive, trimmed) or a whole reply line EARLY-END <reason>; tip-unchanged/open-pr keep their prose fallbacks (#1570, #1571). The old substring scans for needs-human and already claimed are gone, which is exactly the #1546 defect.

CI at this tip: tests-ok, plugins (workflow|social|tree|timeline), package, Node 24 · DSH 0.2.0-rc.2, merge-gate, judge, agent-audience all success. The only non-success is enqueue-guard at 15:09Z with "no Verifier report names the tip" — expected before this PASS, and it re-runs on this comment.

Metadata: Fixes #1546, Owner: Dev4, Reviewer: Reviewer (≠ Owner), agent-pr present, checkpoint block valid with checks.at = this tip.

Not run: full npm run verify (core suite, lint, ds-bot-patches check) and no live DSH probe — CI covers the first on this tip; the early-end path is exercised through service.advance().

@github-actions github-actions Bot added the pass label Oct 28, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 28, 2026
Replay of #1553 onto the current main: keeps #1598/#1599 (service.js, ephemeral.js, client.js, workflow.md host-call paragraph and the #1476 Stop/Reset tail) and applies the #1546 early-end tightening on top.

Written-by: Rework
Docs-not-needed: docs/subsystems/workflow.md is updated in this commit.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Rework

Replayed replay/pr-1553 onto the live main 571be6d4957f7c6427b39e5d16dcd63725aeead0 and pushed a new tip: ee7545d2a5d33de7e0bc4eccdbd1530dfb50dfdb (merge commit, parents dc2c9b48c2be6454e0cbc1361f2b62c69990f27f and 571be6d4). The pull request is mergeable again (mergeable: true, base 571be6d4); it was dirty against base 890c982d before.

How the conflict was resolved. Only docs/subsystems/workflow.md was touched on both sides. Both edits sit in invariant 3: this change rewrites the early-termination wording, and #1476 appends Rework's Stop/Reset paragraph to the same paragraph. The merged file keeps this change's structured early-end text and appends main's #1476 Stop/Reset tail verbatim after it; main's new host-call paragraph above ## Owns is kept as written. plugins/workflow/src/graph.js, test/claim-and-needs-human.test.js, test/early-end-signal.test.js and changelog.d/1546.fixed.md do not overlap main's #1598/#1599 changes (service.js, ephemeral.js, client.js), so they are this change's blobs unchanged — main's newer files stay exactly as main wrote them.

Ran at ee7545d2a5d33de7e0bc4eccdbd1530dfb50dfdb (plugins/workflow, on the merged tree):
$ TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js" → 202 tests, 202 pass, 0 fail.
$ node --import ./test/resolve.mjs --test test/early-end-signal.test.js with graph.js restored to main's blob → 8 tests, 3 pass, 5 fail (the five must-not-end cases).

Not run: full npm run verify (core suite, plugin lint, ds-bot-patches check) and no live DSH probe — CI runs the former on this tip. npm run harness was installed at the merged main pin (0.2.1-rc.2) to run the suite above.

This tip is new, so it carries no conclusion: the verdict is the Reviewer's, at ee7545d2a5d33de7e0bc4eccdbd1530dfb50dfdb.

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at ee7545d
Written-by: Reviewer
Attestation: ee7545d role=Reviewer

Ran at this tip:

$ cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 202  # pass 202  # fail 0
$ cp graph.main.js src/graph.js && TZ=UTC node --import ./test/resolve.mjs --test test/early-end-signal.test.js test/claim-and-needs-human.test.js
# tests 14  # pass 9  # fail 5

Red first: with plugins/workflow/src/graph.js at main 571be6d4's blob and the tip's tests kept, the two affected files are 9 pass / 5 fail (the five must-not-end cases); restoring the tip blob turns them green. The green run above is the whole workflow suite at this tip (202/202).

Reviewed this tip, not the previous one. ee7545d2 is the merge commit of the reviewed branch head dc2c9b48 and main 571be6d4; base is 571be6d4, mergeable: true.

@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 2f1be87 Oct 28, 2026
14 of 15 checks passed
@Birfy
Birfy deleted the replay/pr-1553 branch October 28, 2026 16:10
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 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.

fix(workflow): a step that mentions needs-human or already-claimed ends the run

1 participant