Skip to content

workflow: log the source graph when a claim yields to a Bot - #2481

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-reroute-bot-source-log
Nov 5, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-reroute-bot-source-log

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2480

Owner: Dev-5
Reviewer: Birfy
Written-by: FlyBot Dev-5
Claim-Id: fe151eacfb6f475dbe97bd6c4c3430df
Risk tier: T2 — plugins/** is a T2 path (plugins/workflow/test/** reads T2 ahead of its plugins/*/test/** T1 rule); changelog.d/** is T1. risk-gate run offline over the three changed files: risk-tier: T2; basis: plugins/workflow/src/service.js=T2, plugins/workflow/test/claim-reroute-log.test.js=T2, changelog.d/2480-reroute-bot-source-log.fixed.md=T1, 93 changed lines.

Docs-not-needed: docs/subsystems/workflow.md invariant 8 already states the contract — "A claim that yields leaves one line on the source workflow's log — Task re-matched · <task> → <destination> — in the same store write that attaches the new target" — and this change makes the two early returns match that sentence, so there is no new wording to document.

What was wrong

#1668/#2470 gave the source graph its line, but only on the leg that reaches assign(). ingest() resolves from and then two early returns attach the new claim and leave before the push:

  • decision.kind === 'bot' — a card that yields to a local Bot (an assignee naming it, or a bot: token);
  • queuedOrRunning(...) — a card handed back to a lane that already runs it: already: true, nothing starts, and the graph that lost it says nothing either.

In both, the claim moves and the source log stays on its old line, so the card simply disappears from the graph that lost it. The claim can yield to a bot: token or an assignee naming a local Bot (invariant 8), so this is the documented contract applied to the legs that skipped it, not a new feature.

What this changes

  1. The source line rides every leg that writes the claim. The push is one logSource() closure, called from the bot early return, from the queuedOrRunning early return, and from finish — so each of the three lands exactly one line inside that leg's single write(), and the targets' own lines are untouched.
  2. A Bot destination is named by its roster name (Chief), not its id: when the decision is a Bot, to is resolved from bots(), falling back to the id for a roster that no longer lists it. A workflow destination is named exactly as before.
  3. An ingest that changes nothing still writes nothing: the "card names the same target" return and the prev?.key === key return both stay ahead of the push, so a repeat ingest adds no line and no bytes.

Red first

cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test test/claim-reroute-log.test.js, on clean main f8ec27c2 with only the new cases added and no source change:

not ok 6 - #2480: a reroute from a workflow to a Bot leaves one line on the source, naming the Bot
    the source log gained exactly one entry
    4 !== 5
not ok 7 - #2480: a reroute into a lane that already runs the card still logs the source once
    the source still says where the card went
    4 !== 5
# tests 7
# pass 5
# fail 2

Both failures are the same shape: the source log does not move (4 !== 5), which is the defect. After the fix the same assertions pass and the workflow suite is green.

One case rewritten, not deleted

#1668: already:true appends 0 entries, even on a reroute whose target already runs the card pinned the queued leg's silence in its own assertions (an already:true ingest writes no source line) — that is the behaviour this issue names as the defect, so it could not stay. It is rewritten in place, same sequence and same return-value assertions, now asserting the one line, the untouched target log and the moved claim. The red-first run above carried both new cases; the queued one was then folded into that case rather than kept as a second copy of the same scenario, which is why 7 tests were red-run and the file ends with 6 (5 kept, 1 new).

Ran

  • TZ=UTC ARCH_DOCS_BASE=f8ec27c289babf9c266580d5a7766c29e681235a npm run verify — exit 0, # fail 0: core 2893/2893, social 577/577, tree 137/137, timeline 141/141, workflow 433/433, .github 671/671, interop 37/37, live-e2e 26/26 — 4915 passed, 0 failed; arch-docs: OK (3 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/.
  • cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js" — 433 passed, 0 failed (432 on the base + the one new case).

Not run: the smoke test (no index.js / src/host/ change), typecheck, actionlint.

Checkpoint

{
  "pr": 2481,
  "claim": "Dev-5",
  "issue": 2480,
  "summary": "A claim that yields from a workflow to a local Bot now leaves one line on the source graph saying where the card went. The same line is written when the card is handed back to a lane that already runs it, where nothing starts but the source log used to stay silent. Both ride the write that attaches the new claim, so each lands once. A Bot destination is named by its roster name, and an ingest that changes nothing still writes nothing.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/test/claim-reroute-log.test.js", "changelog.d/2480-reroute-bot-source-log.fixed.md"],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=f8ec27c2 npm run verify; node scripts/ds-bot-patches.mjs check", "result": "green", "at": "b24a1be9e8b8af48835d7f95e0caa900dd7655ba" },
  "next": "none — ready for review"
}

@Birfy

Birfy commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

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

Ran at this tip:

$ cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test test/claim-reroute-log.test.js
# tests 6  # pass 6  # fail 0
$ # red first: src/service.js replaced with its f8ec27c2 blob, same command again
$ cd plugins/workflow && TZ=UTC node --import ./test/resolve.mjs --test test/claim-reroute-log.test.js
# tests 6  # pass 4  # fail 2

Red first, one file, one command: with only plugins/workflow/src/service.js restored to the base blob at f8ec27c289babf9c266580d5a7766c29e681235a (the rest of the tip untouched), the same file fails exactly the two #2480 cases — not ok 4 (reroute into a lane already running the card) and not ok 6 (reroute to a Bot) — both on 4 !== 5, the source log not moving. Restored afterwards; the file is back at the tip byte for byte (logSource present, 4 call sites).

Review, T2 (plugins/workflow/src/service.js):

  • Tip b24a1be9: tests-ok success, plugins (workflow) / plugins (social) / plugins (tree) / plugins (timeline) / package / Node 24 · DSH 0.2.0-rc.2 / merge-gate all success; the only red is enqueue-guard, and its own summary says why: do not enqueue #2481: no Verifier report names the tip b24a1be9e8b8, with "the head lives in a fork: the issue reference and agent-pr rules are not asked of it" — this report is that missing piece, not a defect in the change.
  • Diff is 3 files, all in scope: service.js +24/-13, test/claim-reroute-log.test.js +52/-3, one changelog.d/2480-…fixed.md fragment. No generated file, no core/ change, nothing outside plugins/workflow/.
  • The change is the documented invariant 8 (docs/subsystems/workflow.md: a claim that yields leaves one line on the source log) applied to the two early returns that skipped it. One logSource() closure pushes inside each leg's own write(), so each leg lands exactly one line and the target's lines stay untouched; the prev?.key === key and "same target" returns stay ahead of it, so a no-op ingest still writes nothing — case 3 pins that.
  • A Bot destination is named from bots() by roster name with the id as fallback for a roster that no longer lists it; the workflow destination is named exactly as before. Case 6 asserts the roster name (Chief), not the id.
  • The rewritten #1668: already:true case was pinning the silence this issue calls the defect, and the body says so; its return-value and target-log assertions survive. Nothing else in the file was loosened: the untouched #1668 cases 1, 2, 3 and 5 still pass on the base blob, which is what makes the two reds attributable.
  • The Docs-not-needed: trailer with its reason is in the PR body, the file set needs no subsystem-doc edit, and CI's arch-docs leg is green at this tip.

Not run: the smoke test (no index.js / src/host/ change), typecheck, actionlint; the full suites are the tip's own CI runs, which are green.

@github-actions github-actions Bot added the pass label Nov 5, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 5, 2026
Merged via the queue into ThinkFlowLab:main with commit 7f341a1 Nov 5, 2026
14 of 15 checks passed
@Birfy
Birfy deleted the dev/5-reroute-bot-source-log branch November 5, 2026 01:38
ingest() resolved `from` and then let both early returns attach the new target
without it: a card handed to a local Bot, and a card handed back to a lane
that already runs it, left the source workflow's log silent, so the card
vanished from the graph that lost it (ThinkFlowLab#2480).

The push is now one closure called on every leg that writes the new claim —
still once per write — and a Bot target is named by its roster name instead of
its id. An ingest that changes nothing still writes nothing.

Docs-not-needed: docs/subsystems/workflow.md invariant 8 already states the
contract ("A claim that yields leaves one line on the source workflow's log —
`Task re-matched · <task> → <destination>` — in the same store write that
attaches the new target"); this change makes the two early returns match that
sentence, so there is no new wording to document.
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: rerouting a card from a workflow to a Bot still leaves the source graph silent

2 participants