Skip to content

Commit 0dbcd79

Browse files
authored
fix(workflow): an early-ended failed run logs neither the early end nor why, and a silent reply claims it stopped there (#1615)
Fixes #1577 Owner: Dev2 Reviewer: Reviewer Written-by: Dev-2 Base: bde09d1 (live `main` when this branch was cut) Tip: 5fc75fb (the fork head; tree `588d5fec…`) Tree: 588d5fe ## What was wrong `#1572` made a failure able to end the run early (`needs-human`, `already-claimed`, `tip-unchanged`, `open-pr`): `failNode` (`plugins/workflow/src/graph.js:721-735`) marks the node `done`, returns `finished: true` with `earlyEnd`, and `service.js:448` finishes the run with `run.earlyEnd`. `linesFor` receives `earlyEnd` at `service.js:497`, but neither fail branch in `narrate.js` read it — only the complete branch checks `finished`. So the log of a run that ended early said neither that it ended early nor why: with an empty reply it printed `Verify 这一步失败了,停在本节点。` while the run was `done` and every node `done`, and the `⏹ 提前结束 · <reason>` line that `#1572` earned never appeared. A reader could not tell "the Bot failed and the run stopped" from "the pass produced no new tip, so the run was closed on purpose". ## What changed `plugins/workflow/src/narrate.js` only — both fail branches (live `:102`, non-live `:154`) ask one helper first: - `earlyEndLines(workflow, event)` returns `null` unless `event.earlyEnd != null`. When it is set, it emits the Bot reply if `event.text` has one, then a `kind: 'system'` frame: key `⏹ Ended early · {reason}`, params `{ reason }`, text `⏹ 提前结束 · <reason>`. No `tone`, so it never renders in the failure colour. - With no reply there is no bot line at all, rather than a line that says the step failed: an empty bubble would be noise, and the sentence it used to carry was false. - The complete branch is untouched: a complete step that ends the run early still closes on `✓ 完成 · …` and does **not** add a `⏹` line, so the two paths never double up. - Plain failures still say `停在本节点`, and `recovered` still says `↺ 失败恢复 → …`; the regression tests pin both on this tip. `graph.js` and `service.js` are untouched: `nodeStatus`, `finished`, `doneCount` and `run.earlyEnd` keep exactly the semantics `#1572` landed. The new line follows this file's existing frame contract (English key + params + stored Chinese text, `#1489`), so the shell can translate it and older rows keep the fallback. ## Red → green, honestly Red first on the live base `bde09d1c`, with only `plugins/workflow/test/narrate-early-end.test.js` added and `narrate.js` at `main`: `TZ=UTC node --import ./test/resolve.mjs --test test/narrate-early-end.test.js` — **7 tests, 3 pass, 4 fail**, exactly the four early-end tests: - `live fail with earlyEnd and no reply says early end, never failure` - `live fail with earlyEnd keeps the Bot reply and adds the early-end line` - `non-live fail with earlyEnd says early end, never failure` - `service: a Verify reply that marks tip-unchanged narrates the early end` The three that pass on the base are the regression locks (complete path still `✓ 完成` with no `⏹`, plain failure still `停在本节点`, recovered still `↺ 失败恢复`). At this tip: the same file **7/7 pass**, and `TZ=UTC npm --prefix plugins/workflow test` — **221 tests, 221 pass, 0 fail**. ## Files ``` 3 files changed, 119 insertions(+) crucially unchanged: plugins/workflow/src/graph.js, plugins/workflow/src/service.js plugins/workflow/src/narrate.js | 15 ++++ plugins/workflow/test/narrate-early-end.test.js | 103 ++++++++++++++++++++++++ changelog.d/1577.fixed.md | 1 + ``` ## Checks `TZ=UTC ARCH_DOCS_BASE=bde09d1c npm run verify` at tree `588d5fec…` — **exit 0**: core **1903**, social **528**, tree **125**, timeline **129**, workflow **221**, `.github` **549**, interop **37**, live-e2e **24**; oxlint **0 errors** (only the repository's pre-existing warnings); `build-client --check` clean; `arch-docs: OK (3 changed file(s))`; `ds-bot-patches 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 .github/scripts/changelog.mjs check` — fragments well formed, exit 0. Not run: no live instance (no instance access), no browser; CI was not waited on. Docs-not-needed: narration copy only — `graph.js` and `service.js` keep the state semantics `docs/subsystems/workflow.md` already describes, so the page needs no new rule. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1615, "claim": "Dev2", "issue": 1577, "summary": "A run that ended early (needs-human, already-claimed, tip-unchanged, open-pr) was narrated as a failed step: the log said the node failed and stopped there, and the reason never appeared. Both fail branches in narrate.js now ask one helper first and emit ⏹ 提前结束 · <reason>, keeping the Bot reply when there is one and emitting no bot line when there is none. The complete path still closes on ✓ 完成 with no second line, and plain failures and recoveries keep their wording. graph.js and service.js are untouched.", "files": [ "plugins/workflow/src/narrate.js", "plugins/workflow/test/narrate-early-end.test.js", "changelog.d/1577.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=bde09d1c npm run verify, exit 0 (core 1903, social 528, tree 125, timeline 129, workflow 221, .github 549, interop 37, live-e2e 24, oxlint 0 errors); narrate-early-end.test.js red first on bde09d1 3/7, green 7/7 here", "result": "green", "at": "5fc75fb29893f7fc5fae83f3de2bd6d361427583" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint --> Co-authored-by: HugoXK <hugoxk@users.noreply.github.com>
1 parent a6755fa commit 0dbcd79

3 files changed

Lines changed: 119 additions & 0 deletions

File tree

‎changelog.d/1577.fixed.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
A run that ended early on `needs-human`, `already-claimed`, `tip-unchanged` or `open-pr` no longer reads as a failed step: the run log ends with `⏹ 提前结束 · <reason>` and still quotes the Bot's reply, instead of saying the node failed and stopped there ([#1577](https://github.com/ThinkFlowLab/FlyBot/issues/1577)).

‎plugins/workflow/src/narrate.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,17 @@ function botReply(workflow, nodeId, text) {
8989
return { kind: 'bot', botId: nodeId, step: stepLabel(workflow, nodeId), text }
9090
}
9191

92+
// A step can end the run early without failing (#1537, #1538): the row names why it
93+
// stopped instead of the failure copy, and a Bot reply is still quoted when there is one.
94+
function earlyEndLines(workflow, event) {
95+
if (event.earlyEnd == null) return null
96+
const reply = quoted(event.text)
97+
return [
98+
...(reply ? [botReply(workflow, event.nodeId, reply)] : []),
99+
frame('system', '⏹ Ended early · {reason}', { reason: event.earlyEnd }, `⏹ 提前结束 · ${event.earlyEnd}`),
100+
]
101+
}
102+
92103
// A live run quotes the Bot when event.text is set; otherwise a frame fills in.
93104
function liveLines(workflow, task, event) {
94105
const label = summarizeTask(task)
@@ -100,6 +111,8 @@ function liveLines(workflow, task, event) {
100111
return [frame('system', 'Started · {task} · {nodes}', { task: label, nodes }, `开始 · ${label} · ${nodes}`, { tone: 'accent' })]
101112
}
102113
if (event.type === 'fail') {
114+
const ended = earlyEndLines(workflow, event)
115+
if (ended) return ended
103116
if (event.recovered === true && event.started.length > 0) {
104117
const target = names(workflow, event.started)
105118
return [
@@ -147,6 +160,8 @@ export function linesFor(workflow, task, event) {
147160
return [frame('system', 'Task {task} entered the workflow; {nodes} claimed it', { task: label, nodes }, `任务 ${label} 已进入 Workflow,${nodes} 认领`, { tone: 'accent' })]
148161
}
149162
if (event.type === 'fail') {
163+
const ended = earlyEndLines(workflow, event)
164+
if (ended) return ended
150165
if (event.recovered === true && event.started.length > 0) {
151166
const target = names(workflow, event.started)
152167
return [
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// A run that ended early (needs-human / already-claimed / tip-unchanged / open-pr)
2+
// is not a failure: the log says why it stopped, not that it stopped on the node.
3+
import assert from 'node:assert/strict'
4+
import { test } from 'node:test'
5+
import { linesFor } from '../src/narrate.js'
6+
import { normalizeGraph } from '../src/graph.js'
7+
import { createWorkflowService } from '../src/service.js'
8+
9+
const roster = [
10+
{ id: 'bot-claim', name: 'Claim', role: 'Claim', remote: false },
11+
{ id: 'bot-verify', name: 'Verify', role: 'Verify', remote: false },
12+
{ id: 'bot-land', name: 'Land', role: 'Land', remote: false },
13+
]
14+
15+
function graph() {
16+
return normalizeGraph({
17+
id: 'claim-verify-land',
18+
name: 'Claim Verify Land',
19+
nodes: [
20+
{ id: 'claim', name: 'Claim', role: 'Claim', outputs: [{ name: 'claimed', type: 'text' }] },
21+
{ id: 'verify', name: 'Verify', role: 'Verify', inputs: [{ name: 'claimed', type: 'text' }], outputs: [{ name: 'status', type: 'text' }] },
22+
{ id: 'land', name: 'Land', role: 'Land', inputs: [{ name: 'status', type: 'text' }] },
23+
],
24+
edges: [
25+
{ from: 'claim', to: 'verify' },
26+
{ from: 'verify', to: 'land' },
27+
],
28+
})
29+
}
30+
31+
const task = { id: '#1577', title: 'Early end' }
32+
33+
function failEvent(extra = {}) {
34+
return { type: 'fail', nodeId: 'verify', started: [], joined: false, finished: true, layerSize: 0, live: true, text: '', recovered: false, ...extra }
35+
}
36+
37+
test('live fail with earlyEnd and no reply says early end, never failure', () => {
38+
const lines = linesFor(graph(), task, failEvent({ earlyEnd: 'tip-unchanged' }))
39+
assert.equal(lines.some(line => String(line.text).includes('失败')), false, JSON.stringify(lines))
40+
const end = lines.find(line => String(line.text).includes('提前结束'))
41+
assert.ok(end, JSON.stringify(lines))
42+
assert.equal(end.kind, 'system')
43+
assert.match(end.text, /tip-unchanged/)
44+
assert.equal(end.key, '⏹ Ended early · {reason}', 'the shell translates the key, the stored text is the fallback')
45+
assert.deepEqual(end.params, { reason: 'tip-unchanged' })
46+
assert.notEqual(end.tone, 'bad')
47+
})
48+
49+
test('live fail with earlyEnd keeps the Bot reply and adds the early-end line', () => {
50+
const lines = linesFor(graph(), task, failEvent({ earlyEnd: 'open-pr', text: 'PR #1533 is already open for this issue.' }))
51+
const bot = lines.find(line => line.kind === 'bot')
52+
assert.equal(bot?.text, 'PR #1533 is already open for this issue.')
53+
assert.equal(lines.some(line => String(line.text).includes('失败')), false, JSON.stringify(lines))
54+
assert.ok(lines.some(line => String(line.text).includes('提前结束') && String(line.text).includes('open-pr')))
55+
})
56+
57+
test('complete with earlyEnd still ends on the ok finish line and adds no early-end line', () => {
58+
const lines = linesFor(graph(), task, {
59+
type: 'complete', nodeId: 'land', started: [], joined: false, finished: true, layerSize: 1, live: true, text: '', earlyEnd: 'already-claimed',
60+
})
61+
assert.ok(lines.some(line => String(line.text).startsWith('✓ 完成')))
62+
assert.equal(lines.some(line => String(line.text).includes('提前结束')), false, JSON.stringify(lines))
63+
})
64+
65+
test('a plain live failure still says the run stopped on this node', () => {
66+
const lines = linesFor(graph(), task, failEvent())
67+
assert.ok(lines.some(line => String(line.text).includes('停在本节点')), JSON.stringify(lines))
68+
assert.equal(lines.some(line => String(line.text).includes('提前结束')), false)
69+
})
70+
71+
test('a recovered live failure still says it recovered', () => {
72+
const lines = linesFor(graph(), task, failEvent({ recovered: true, started: ['claim'] }))
73+
assert.ok(lines.some(line => String(line.text).includes('失败恢复')))
74+
assert.equal(lines.some(line => String(line.text).includes('提前结束')), false)
75+
})
76+
77+
test('non-live fail with earlyEnd says early end, never failure', () => {
78+
const lines = linesFor(graph(), task, { ...failEvent({ earlyEnd: 'needs-human' }), live: false })
79+
assert.equal(lines.some(line => String(line.text).includes('失败')), false, JSON.stringify(lines))
80+
assert.ok(lines.some(line => String(line.text).includes('提前结束') && String(line.text).includes('needs-human')))
81+
})
82+
83+
test('service: a Verify reply that marks tip-unchanged narrates the early end', () => {
84+
const svc = createWorkflowService({ persist: false, now: () => 1_700_000_000_000, bots: () => roster })
85+
const base = graph()
86+
const created = svc.create({ name: base.name, nodes: base.nodes, edges: base.edges })
87+
const workflowId = created.workflows.find(workflow => workflow.name === base.name).id
88+
svc.assign({ workflowId, taskId: '#1577', title: 'Early end' })
89+
svc.advance({ workflowId, nodeId: 'claim', outcome: 'done', outputs: { claimed: 'ok' }, text: 'Claimed.' })
90+
91+
const result = svc.advance({
92+
workflowId,
93+
nodeId: 'verify',
94+
outcome: 'failed',
95+
text: 'Suite green at this tip; tip unchanged from the previous pass.',
96+
})
97+
const workflow = result.workflows.find(item => item.id === workflowId)
98+
const texts = workflow.messages.map(line => String(line.text))
99+
assert.equal(workflow.run.status, 'done')
100+
assert.equal(workflow.run.earlyEnd, 'tip-unchanged')
101+
assert.equal(texts.some(text => text.includes('失败')), false, texts.join(' | '))
102+
assert.ok(texts.some(text => text.includes('提前结束') && text.includes('tip-unchanged')), texts.join(' | '))
103+
})

0 commit comments

Comments
 (0)