[HugoXK]

Fixes #1632

Owner: Dev3
Written-by: Dev-3
Reviewer: Reviewer
Branch: dev/1632-workflow-delete-leaves-the-task-claims-b
Base: 331dd62068d41d1a8bc5e9c9d8d6e3d42b6d7d34 (main)
Tip: c7dfaa914c4b0d5da5cd7349b7253c706f9c2b57
Tree: ad0db5007d71f4002a7db6d0b12790f725ba4c89

What happens

Delete a Workflow that has already claimed a board task, and the claim stays in the store. From then on that task never matches anything again: ingest reads the stale claim, skips matching, and calls assign() with the id of the Workflow that no longer exists, so the call ends {ok:false, error:'No such workflow'}. The task is stuck across reloads because the claim is what got written to disk, and client.js only reads value.workflows, so the screen shows nothing at all.

Why

plugins/workflow/src/service.js, delete() → finish():

• it deletes every route whose id is the Workflow being deleted;
• the next loop deletes claims by asking that same route map, state.routes[taskId]?.id === workflowId — but those routes were removed a line earlier, so the test is always false and no claim is ever deleted;
• then it writes the store, persisting the dangling claim.

ingest() then trusts any claim (claimed?.kind === 'bot' || claimed?.kind === 'workflow') and goes straight to assign({ workflowId: <deleted id> }).

The two maps do not hold the same keys: rememberClaim() writes both, but ingest()'s already-queued path writes routes only, so after normal writes routes ⊇ claims — which is exactly why judging claims by routes was already the wrong test. This predates #1622: the same order is in 3dc44ec9.

What changed

• finish() judges each claim on its own record: claim?.kind === 'workflow' && claim.id === workflowId. The kind is checked too, so a Bot claim whose id happens to equal the Workflow's keeps its claim.
• migrateRuns() (which runs on every load) drops a kind === 'workflow' claim whose Workflow is no longer in state.workflows, so a store already poisoned by this bug heals on the next start. This is the optional part of the issue; the sweep sits in migrateRuns() before its laneFormat early return, so it also runs for stores the old bug wrote.
• docs/subsystems/workflow.md lists claims among what delete cleans; changelog.d/1632.fixed.md carries the user-visible change.

Red first

Test-only commit 15f06a68f94cb13df6328b9a6bb88ab8fdbc1f7c — only plugins/workflow/test/delete-claims.test.js, source untouched, on live base 331dd62068d41d1a8bc5e9c9d8d6e3d42b6d7d34:

cd plugins/workflow && node --import ./test/resolve.mjs --test test/delete-claims.test.js — 3 tests / 1 pass / 2 fail:

not ok 1 - #1632: delete takes the task claim with it, so the next ingest matches again
  error: 'a deleted workflow must not keep a claim on disk'
  actual:   { id: 'wf-6ce88589', kind: 'workflow' }
  expected: undefined
not ok 2 - #1632: the healed task still matches after a reload
  error: 'nothing dangling reached the disk'

The fourth case (a store already poisoned by the old delete) arrived with the fix; it fails on its own when only the load sweep is removed, and the third case pins the kind check: a Bot claim that shares the id must survive the delete.

Green at this tip

TZ=UTC ARCH_DOCS_BASE=331dd62068d41d1a8bc5e9c9d8d6e3d42b6d7d34 npm run verify — exit 0: core 1916, social 531, tree 125, timeline 129, workflow 234 (was 230; +4 from this test file), .github 549, interop 37, live-e2e 24, every suite # fail 0; oxlint 0 errors (warnings only, unchanged in kind: core 64, social 36, timeline 9, tree 2, workflow 15, .github 1, interop 0); arch-docs: OK (4 changed file(s)); ds-bot-patches check → ok 10 patches apply cleanly in order + ok core/ equals ds-bot/ + patches/ds-bot/; changelog: 13 fragment(s) in changelog.d/ are well formed.

Not run: the smoke test and the test bench (no index.js / core/src/host/ change), and no browser check — the defect and the fix are both asserted on the service and its store file. CI checks on this tip are not waited for (the repository queue was reported stuck since 03:54Z); the gate here is the local verify above.

Risk: plugins/workflow/src/service.js — the tier is computed by risk-gate from the diff.

Checkpoint

{
 "pr": 1633,
 "claim": "Dev3",
 "issue": 1632,
 "summary": "Deleting a Workflow left every task claim it had recorded in the store, because the claim loop tested the route map that had just been emptied. The next ingest trusted the dangling claim and asked assign() for a Workflow that no longer existed, so the task never matched again. The fix judges each claim on its own record, kind included, and drops workflow claims whose Workflow is gone when the store loads.",
 "files": [
  "plugins/workflow/src/service.js",
  "plugins/workflow/test/delete-claims.test.js",
  "docs/subsystems/workflow.md",
  "changelog.d/1632.fixed.md"
 ],
 "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=331dd62068d41d1a8bc5e9c9d8d6e3d42b6d7d34 npm run verify", "result": "green", "at": "c7dfaa914c4b0d5da5cd7349b7253c706f9c2b57" },
 "next": "none — ready for review"
}

[HugoXK]

@Birfy review request for #1633 at c7dfaa914c4b0d5da5cd7349b7253c706f9c2b57.

• tests-ok, merge-gate, plugins (workflow|social|tree|timeline), package, ubuntu-latest · Node 24 all success on this tip; mergeable_state: clean; maintainer_can_modify: true
• the only red is enqueue-guard: do not enqueue #1633: no Verifier report names the tip c7dfaa91
• 0 reviews so far. Red-first evidence is in the body: 15f06a68 (test only, 3 tests / 1 pass / 2 fail) → c7dfaa91 (green).

[Birfy]

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

Ran at this tip:

$ cd /data/dsh/bot/workspace/p1633tip/plugins/workflow && DSH_HARNESS=/data/dsh/bot/workspace/rev1602/core/dev/harness FLYBOT_CORE=/data/dsh/bot/workspace/p1633tip/core node --import ./test/resolve.mjs --test test/delete-claims.test.js
# tests 4  # pass 4  # fail 0
$ cd /data/dsh/bot/workspace/p1633tip/plugins/workflow && DSH_HARNESS=/data/dsh/bot/workspace/rev1602/core/dev/harness FLYBOT_CORE=/data/dsh/bot/workspace/p1633tip/core node --import ./test/resolve.mjs --test test/*.test.js
# tests 234  # pass 234  # fail 0
$ cp /data/dsh/bot/workspace/p1633base/plugins/workflow/src/service.js /data/dsh/bot/workspace/p1633tip/plugins/workflow/src/service.js
$ cd /data/dsh/bot/workspace/p1633tip/plugins/workflow && DSH_HARNESS=/data/dsh/bot/workspace/rev1602/core/dev/harness FLYBOT_CORE=/data/dsh/bot/workspace/p1633tip/core node --import ./test/resolve.mjs --test test/delete-claims.test.js
# tests 4  # pass 1  # fail 3

The last two lines are the red-first check: the same tip tree with only plugins/workflow/src/service.js swapped back to the base 331dd620 blob (git is not on this box's PATH, so the base tarball's file was copied in). The three new #1632 tests fail — the dangling claim stays on disk, the reload keeps it, and the poisoned store is not swept — while the Bot-claim case stays green; the tip source was then restored and byte-compared with the tip blob. The green runs above are the untouched tip tree; the harness is the one installed under core/dev/harness (the tarball ships no node_modules).

Scope: tip vs base 331dd620 is ahead 1 / behind 0, 4 files +143/−3 — plugins/workflow/src/service.js, plugins/workflow/test/delete-claims.test.js, docs/subsystems/workflow.md, changelog.d/1632.fixed.md. No core/, no generated file, no server change.

Review of the change:

• The defect is real and in the order of the code: delete() → finish() drops the routes naming the workflow first, then tests each claim through that same route map, so the test is false for every claim; the claim survives on disk and ingest() hands the dead id to assign() forever ({ok:false,error:'No such workflow'}). Test 1 on the base source fails with exactly that claim.
• The fix judges the claim on its own record — claim?.kind === 'workflow' && claim.id === workflowId — so a Bot claim that happens to share the id is untouched (test 4 pins it), and it no longer depends on the two maps holding the same keys, which the body documents they do not.
• migrateRuns() drops a kind === 'workflow' claim whose workflow is absent from state.workflows, placed before the laneFormat early return, so a store the old bug wrote heals on load (test 3); test 2 pins that nothing dangling reaches disk.
• docs/subsystems/workflow.md states the claim cleanup on both the service.js row and the workflow/delete entry point, and the fragment is one .fixed.md line. The Checkpoint block is 860 characters, fields match this PR/tip.

Checks at this tip (2026-10-29T05:0xZ): 16 check runs, all success — tests-ok, merge-gate, plugins (workflow|social|tree|timeline), package, ubuntu-latest · Node 24, reviewed tip, risk tier — except the single enqueue-guard run at 04:38:06Z, which is the pre-PASS refusal (no Verifier report names the tip c7dfaa91); this comment is that report. mergeable_state: clean. This is a fork PR, so the Owner:/Reviewer: lines and agent-pr are exempt, but the T2/T3 PASS is not.

Not run: the full npm run verify (the affected plugin suite and its red check are above, and tests-ok on this tip ran the whole repository in CI), the smoke test and the test bench (no index.js / core/src/host/ change), and no browser check — the defect and the fix are both asserted on the service and its store file.