Skip to content

fix(workflow): release Bots before Stop/Reset forgets them, and freeze a run whose release failed (#1476) - #1599

Merged
Birfy merged 1 commit into
mainfrom
replay/pr-1498
Oct 28, 2026
Merged

Birfy merged 1 commit into
mainfrom
replay/pr-1498

Conversation

@Birfy

@Birfy Birfy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1476

Owner: Chief
Reviewer: Reviewer

Replay of #1498: 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.

What changed

Stop and Reset released a run's dynamic Bots after the transition had already deleted their ids from the run record, so a release that failed left a live Bot no saved state named and no second attempt could reach. This is the original change, replayed.

  • plugins/workflow/src/ephemeral.js — releaseAll returns the ids whose release threw instead of dropping them silently.
  • plugins/workflow/src/service.js — Stop (stopOne) and Reset (take) split "do the work" from "commit the transition"; the transition runs only after every release succeeded. A partial failure keeps the unfreed ids on the still-live run and freezes it (paused plus the internal releasePending) with a tone:'bad' system line, resume() refuses it, and the next Stop/Reset/dispose()/reload reap retries. A failed whole-workflow Reset freezes every live run it did not reset. The markers go when the run ends.
  • plugins/workflow/test/ephemeral.test.js — the seven #1476 tests.
  • docs/subsystems/workflow.md — invariant 3 gains the release-before-forget rule.
  • changelog.d/1476-workflow-reset-release-order.fixed.md — the fragment.

Replay merge

The original tip 7b723b7d sat on bf5e8780; main moved 28 commits to ae7b19f4 and touched four of these five files. Merged by hand, keeping main's newer content and this change's intent together:

  • service.js — the new stop() keeps main's endRun(item, run, { code: 'stopped' }), historyRow(ended) and its ■ Stopped · {task} line with key/params; reset() keeps main's "no task" refusal before it touches the lane.
  • ephemeral.js, test/ephemeral.test.js — main's #1544/#1466 work kept as is; this change's blocks added, no main test dropped.
  • docs/subsystems/workflow.md — main's invariant 3 kept verbatim (#1537, #1493, #1494, #1549, #1580), this change's clause appended.

Tests at the tip

  • plugins/workflow: 188/188 pass. test/host.test.js is excluded: it needs the DSH harness installed and fails the same way on untouched main.
  • test/ephemeral.test.js: 29/29 — main's 22 plus the seven #1476 tests.
  • node .github/scripts/check-mojibake.mjs --check ok; node .github/scripts/changelog.mjs check 4 fragments well formed.
  • Not run: npm run verify (no node_modules in this checkout), the plugin lint (no oxlint) and the smoke test. CI covers the rest at this tip.

Checkpoint

{
  "pr": 1599,
  "claim": "Chief",
  "issue": 1476,
  "summary": "Stop and Reset released a run's Bots after the record had already dropped their ids, so a failed release stranded a live Bot that no saved state named and no second attempt could reach. This replay releases first and forgets second, keeps the ids a release could not free on the still-live run, and freezes that run as Paused with releasePending so resume is refused and the next Stop retries. A failed whole-workflow Reset freezes every live run it did not reset. Merged by hand onto current main, keeping main's newer stop/reset, connector and history work.",
  "files": [
    "plugins/workflow/src/service.js",
    "plugins/workflow/src/ephemeral.js",
    "plugins/workflow/test/ephemeral.test.js",
    "docs/subsystems/workflow.md",
    "changelog.d/1476-workflow-reset-release-order.fixed.md"
  ],
  "checks": {
    "ran": "cd plugins/workflow && node --import ./test/resolve.mjs --test test/*.test.js (host.test.js excluded: it needs the DSH harness and fails the same way on untouched main)",
    "result": "green",
    "at": "09d844c924d5dad8a0db51e963bc371765ed1a06"
  },
  "next": "Review the replayed tip: the original #1498 intent merged onto current main, with docs invariant 3 and the seven #1476 tests kept beside main's newer work."
}

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 09d844c
Written-by: Reviewer
Attestation: 09d844c role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/rv1599/ThinkFlowLab-FlyBot-09d844c && node --test plugins/workflow/test/ephemeral.test.js
# tests 29  # pass 29  # fail 0
$ cp ../base-ephemeral.js plugins/workflow/src/ephemeral.js && node --test plugins/workflow/test/ephemeral.test.js
# tests 29  # pass 18  # fail 11

base-ephemeral.js is plugins/workflow/src/ephemeral.js as it stands on main, fetched raw; the tip file was restored and the 29/29 run repeated after it (same 29 tests).

Red first: with main's ephemeral.js in place, 11 of the 29 fail — the seven #1476 tests (a failed Stop keeps the id and a second Stop retries it; a failed Stop freezes the run as Paused; resume() refuses it; a reload reaps it and the markers go; dispose() retries; a failed whole-workflow Reset keeps the id; the no-failure path still forgets every id) plus four existing contract tests (5, 15, #1544 20/21) that the new releaseAll return value serves. Nothing else turns red.

Tip checks: tests-ok success (ubuntu-latest · Node 24 · DSH 0.2.0-rc.2, plugins (workflow), plugins (social), plugins (tree), plugins (timeline), package, merge-gate all success; risk tier and reviewed tip report success). enqueue-guard is red only with no Verifier report naming this tip, which this PASS clears.

Reviewed change set: 5 files — plugins/workflow/src/service.js, plugins/workflow/src/ephemeral.js, plugins/workflow/test/ephemeral.test.js, docs/subsystems/workflow.md, changelog.d/1476-workflow-reset-release-order.fixed.md; no core/, no patches/ and no core/client.js, so no regeneration is owed. The body carries Fixes #1476, Owner: Chief, Reviewer: Reviewer and a ## Checkpoint whose pr/claim/issue/at match this pull request and this tip.

Not run: whole-repo npm run verify, the other plugins/workflow test files (no core/dev/harness install here), and the smoke test.

@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
Merged via the queue into main with commit 571be6d Oct 28, 2026
13 of 15 checks passed
@Birfy
Birfy deleted the replay/pr-1498 branch October 28, 2026 15:34
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
Birfy added a commit that referenced this pull request Oct 28, 2026
Resolve the conflict in plugins/workflow/test/client.test.js by keeping both
sides: the workflow page tests #1599 added on main and the #1457/#1458 assign
dialog tests from this branch. The workflow page and its subsystem doc merge
with both sides' changes kept.
Birfy added a commit that referenced this pull request 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.
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): Reset/Stop writes state before releasing run Bots, so a failed release erases the only id record

1 participant