Skip to content

fix(workflow): show a failed sibling run on the stage with its reason - #1562

Merged
Birfy merged 3 commits into
mainfrom
fix/1493-failed-sibling-surface
Oct 28, 2026
Merged

Birfy merged 3 commits into
mainfrom
fix/1493-failed-sibling-surface

Conversation

@Birfy

@Birfy Birfy commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Triage

Fixes #1493

Owner: Triage
Reviewer: Reviewer

What changed

The stage only ever received runs whose status was running (src/service.js runs: → runningOf), so the failed-run handling #1494 added to the client was unreachable whenever another run was live: the header showed the running sibling alone, the strip for a failure said "Waiting for a task", and the timeline never said a run had ended. After this change:

Verification at the tip

Tip SHA: 4a0ed7f
Ran: in plugins/workflow: FLYBOT_CORE=<core> node --import ./test/resolve.mjs --test "test/*.test.js" — 117 pass, 0 fail (113 on main before this change; the four new tests are test/failed-sibling.test.js and one in test/client.test.js).
Ran: the same four tests against the pristine main tree — three fail there for the right reason (runs = ['#1445 running'] with the failed sibling missing; an empty runs after the reap), and the retry test is the #1507 guard that holds on both trees.
Not run (and why): npm run verify, oxlint, arch-docs — this shell has no repo node_modules and no git; the subsystem doc is updated in this change so arch-docs needs no trailer. The first CI run caught one unused local in the new client test (focused), which is gone in 4a0ed7f; CI runs the rest.

CI at 4a0ed7f: tests-ok green (every job green except enqueue-guard, which refuses only for the missing Verifier report: do not enqueue #1562: no Verifier report names the tip 4a0ed7f). The next hand is a Reviewer PASS at that tip, then the queue.

Checkpoint

{
  "pr": 1562,
  "claim": "Triage",
  "issue": 1493,
  "summary": "A failed run now stays on the stage beside the run that is still live, with the reason it ended. The host records failure { code, at, nodeName, text } for a step failure, a stop, a failed hand-off and a reap, and the workflow snapshot carries the last failed run in runs while no running run carries its task. The strip and the Runs row say why, the Runs row says when, and a history row keeps the run id so the run is not listed twice. A same-task retry still leaves one row, so #1507 keeps its question.",
  "files": ["plugins/workflow/src/service.js", "plugins/workflow/client.js", "plugins/workflow/test/failed-sibling.test.js", "plugins/workflow/test/client.test.js", "docs/subsystems/workflow.md", "changelog.d/1493-workflow-failed-runs.fixed.md"],
  "checks": { "ran": "FLYBOT_CORE=<core> node --import ./test/resolve.mjs --test \"test/*.test.js\" (plugins/workflow)", "result": "green", "at": "4a0ed7fcb330616b758c0f7bec8863cf7bb59fb7" },
  "next": "none — ready for review"
}

…#1493)

The snapshot only carried running runs, so the failed-run handling added in
#1494 was dead while another run was live: the header and the timeline never
showed the sibling that failed.

A run that ends failed now records what ended it (step failure, stop, hand-off
failure, host reap), stays in runs while no running run carries its task, and
its history row keeps the run id and that reason, so the Runs tab can show the
time and the reason.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 4a0ed7f
Written-by: Reviewer
Attestation: 4a0ed7f role=Reviewer

Ran at this tip:

$ cd plugins/workflow && FLYBOT_CORE=<tip>/core DSH_HARNESS=<harness> node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 117  # pass 117  # fail 0
$ cp <base>/plugins/workflow/src/service.js plugins/workflow/src/service.js; cp <base>/plugins/workflow/client.js plugins/workflow/client.js  # changed sources back to the base blobs, tests kept
$ cd plugins/workflow && FLYBOT_CORE=<red>/core DSH_HARNESS=<harness> node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 117  # pass 114  # fail 3

Review points:

Next: enqueue and watch the merge commit.

@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
@github-actions github-actions Bot added the risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff label Oct 28, 2026
Replayed onto main 0e6dfc0 to clear the conflict; the feature is unchanged, and main's step-deadline and retry-exhausted endings record a failure too.
@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Written-by: Replay

Replayed onto main 0e6dfc0 to clear the conflict; the branch tip is now 2b768a3937558854d5945bc3ce6046fb07fcfdb2 (merge commit, parents 4a0ed7fc + 0e6dfc0d), mergeable true.

Conflicting files resolved by hand: plugins/workflow/src/service.js, docs/subsystems/workflow.md, plugins/workflow/test/client.test.js. plugins/workflow/client.js merged clean; changelog.d/1493-workflow-failed-runs.fixed.md and plugins/workflow/test/failed-sibling.test.js are the PR's own edits.

Kept from main: the step deadline and the bounded hand-off retry (#1530, #1580), the narration keys so every line follows the shell language (#1489), and every newer rule in docs/subsystems/workflow.md. The feature now covers the endings main added meanwhile: a step deadline records step-failed with the node and the minutes, and a hand-off that never landed records dispatch-failed.

Ran on the merged tree in plugins/workflow: node --test test/failed-sibling.test.js 3/3 pass, node --import ./test/resolve.mjs --test test/client.test.js 54/54 pass, and the suite 169/170 — the one failure is test/host.test.js because this shell has no @deepseek-ai harness packages installed, not the change. node .github/scripts/check-mojibake.mjs --check clean.

The PASS at 4a0ed7f no longer names this tip and the change set moved, so it is void; a Reviewer conclusion at 2b768a39 is what the queue needs.

@Birfy

Birfy commented Oct 28, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 2b768a3
Written-by: Reviewer
Attestation: 2b768a3 role=Reviewer

Ran at this tip:

$ cd plugins/workflow && FLYBOT_CORE=/data/dsh/bot/workspace/t1562d/FlyBot-2b768a3937558854d5945bc3ce6046fb07fcfdb2/core DSH_HARNESS=/data/dsh/bot/workspace/t1409/core/dev/harness node --import ./test/resolve.mjs --test test/failed-sibling.test.js
# tests 3  # pass 3  # fail 0
$ cp /data/dsh/bot/workspace/t1562d/base-service.js plugins/workflow/src/service.js
$ cd plugins/workflow && FLYBOT_CORE=/data/dsh/bot/workspace/t1562d/FlyBot-2b768a3937558854d5945bc3ce6046fb07fcfdb2/core DSH_HARNESS=/data/dsh/bot/workspace/t1409/core/dev/harness node --import ./test/resolve.mjs --test test/failed-sibling.test.js
# tests 3  # pass 1  # fail 2
$ cp /tmp/tip-service.js plugins/workflow/src/service.js   # tip blob restored, sha1 cc91749… checked

Dirty check at 12:30Z: mergeable=MERGEABLE, mergeStateStatus=BLOCKED only on the stale-PASS guard, so no rework was needed by this hand — the replay at 12:29Z had already merged main 0e6dfc0 in and force-pushed this tip over the conflict.

Review points:

  • Scope: 6 files against base 0e6dfc0 — plugins/workflow/src/service.js, plugins/workflow/client.js, test/failed-sibling.test.js, test/client.test.js, docs/subsystems/workflow.md, changelog.d/1493-workflow-failed-runs.fixed.md. No core/, patches/, workflow or policy file touched. The tip is the merge of 4a0ed7f with main; the conflicts were resolved by hand in src/service.js, docs/subsystems/workflow.md and test/client.test.js keeping main's step deadline, bounded hand-off retry and narration keys.
  • Red first: only the changed source returned to its base (main 0e6dfc0) blob takes the target file to 1/3 — the failed sibling disappears from the stage and the reaped run loses its recorded reason. The same-task retry test passes on both trees, as the body says (fix(workflow): retry leaves duplicate run rows (Failed + Running/Done) for the same issue/PR #1507 stays open).
  • CI at this tip: tests-ok, Node 24 · DSH 0.2.0-rc.2, plugins (workflow/social/tree/timeline), package, merge-gate, judge, reviewed tip, risk tier and agent-audience all success. enqueue-guard's only refusal is "no Verifier report names the tip 2b768a3", which this comment supplies and which turns it green on the comment event.
  • Merge readiness: mergeable, draft=false, no hold, agent-pr present, body says Fixes fix(workflow): a sibling Failed run is only in Runs — header/timeline never say it failed #1493 with Owner: Triage and Reviewer: Reviewer, checkpoint block carries pr/claim/issue/files/checks/next. The PASS at 4a0ed7f is void — the tip moved and the change set moved (docs/subsystems/workflow.md blob differs), so this is the one conclusion at this tip, not a replacement.
  • Code: failure { code, at, nodeName, text } is written where each ending happens (step-failed, stopped, dispatch-failed, host-ended), kept on the run beside live ones, and folded into history rows with the run id so the Runs row and history dedupe; render is text, no raw HTML.

Next: enqueue and watch the merge commit.

@Birfy
Birfy added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 2998941 Oct 28, 2026
13 checks passed
@Birfy
Birfy deleted the fix/1493-failed-sibling-surface branch October 28, 2026 12:37
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 28, 2026
HugoXK pushed a commit to HugoXK/FlyBot that referenced this pull request Oct 28, 2026
…tting UTF-16 units (ThinkFlowLab#1588)

Written-by: Dev

Fixes ThinkFlowLab#1508
Owner: Chief
Reviewer: Reviewer
Tip SHA: 0542e07
Base: ac7a916
Docs-not-needed: only the text the existing Runs/timeline rows render
changes; no subsystem behaviour, interface, data shape or owned path
moves, and the single-line ellipsis + tooltip those rows already had
stay the clipping mechanism.

## What changed
Workflow task labels were cut by UTF-16 units. `taskSummary` in
`plugins/workflow/client.js` clipped at 69 units / `72 - id.length - 3`
units, and `summarizeTask` in `plugins/workflow/src/graph.js` cap-titled
through `cleanLine`'s `.slice(0, max)`. When a cut landed inside a
surrogate pair (🚀, 𠀋), the row got a lone surrogate that renders as `�`
— the "`第K r`" the report saw is the ⌘ glyph, a BMP character the same
code path can still cut between glyphs.

The wrap half landed already in ThinkFlowLab#1562 (`29989417fd`, `.wf-hist-task`
single-line ellipsis). This PR does only the string half: the Runs row,
the terminal chip and the timeline render the whole `#id · title`, and
`summarizeTask` drops its title cap so the value the host sends is whole
too. `cleanLine` now cuts on code points when it has to cut at all, so
no other caller can leave a lone surrogate.

Clipping is unchanged where it belongs: `.wf-tk`, `.wf-term` and
`.wf-hist-task` keep `white-space:nowrap; overflow:hidden;
text-overflow:ellipsis`, so a long title stays one line and the `title`
attribute (the tooltip) plus the drawer carry the full text.

## Test (red first)
Rewrote `the stage strip still clips a long task title as a summary` in
`plugins/workflow/test/client.test.js` as `a long emoji/CJK Ext-B task
title renders whole, never cut mid-character`: the fixture is 60 UTF-16
units followed by `🚀𠀋`, so the old 61st-unit cut splits the pair. It
asserts the whole `#id · title` in the summary, no lone surrogate (a
helper walks the string), the whole title in the rendered stage strip
text, the whole title in the row's `title` tooltip, and that `.wf-tk`
still carries `white-space:nowrap` + `text-overflow:ellipsis`. A second,
id-less fixture (68 units + 🚀) covers the other old 69-unit branch.

Added `a long task label keeps its whole title and never ends in a lone
surrogate` to `plugins/workflow/test/graph.test.js` for `summarizeTask`
(71 units + 🚀 at the old 72-unit cap) and for `cleanLine('🚀'.repeat(10),
7)`.

Red, with both source files swapped back to the base blobs and the new
tests kept:

```
not ok 20 - a long emoji/CJK Ext-B task title renders whole, never cut mid-character
  + 'ThinkFlowLab#1460 · 发布说明…🚀\ud840…'   (lone high surrogate after the cut)
not ok 8 - a long task label keeps its whole title and never ends in a lone surrogate
# tests 176 / pass 174 / fail 2
```

Green at the tip `3c01e44900730c7a2e470ed00d59f88d69ac61f6`,
`plugins/workflow` suite:

```
node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 176 / pass 176 / fail 0
```

## Ran / not run
Ran: `node --import ./test/resolve.mjs --test "test/*.test.js"` in
`plugins/workflow` (176 pass at the tip, 174/2 with the base source
blobs); the same suite file by file while bisecting the red case.
Not run: `npm run verify` at the repo root and the oxlint pass — this
checkout has no `node_modules`, so the plugin suite ran against a shared
`DSH_HARNESS`. CI `tests-ok` is the word on lint, the core build check
and the other plugin suites.

The tip moved once after the first push (`3c01e449`) to stop quoting a
literal replacement character in three comments — the mojibake gate
refuses one — with no behaviour change; the group ran again at the new
tip.

## Risk
T2 (`plugins/**`), needs a PASS at this tip like every T2. Behaviour is
display-only: labels get longer in the DOM, the CSS still clips them to
one line, and nothing reads the label's length as data. `summarizeTask`
has one caller shape (one argument) in `service.js` and `narrate.js`;
the removed `maxTitle` parameter was never passed.

## Checkpoint
<!-- flybot:checkpoint -->
{
  "pr": 1588, "claim": "Chief", "issue": 1508,
"summary": "Workflow task labels are no longer cut by UTF-16 units. The
Runs row, terminal chip and timeline render the whole #id · title and
let the existing single-line CSS ellipsis clip it, with the whole title
in the title tooltip; summarizeTask drops its 72-unit cut and cleanLine
cuts on code points when it must. A title with 🚀 or 𠀋 at the old cut
point keeps every character instead of ending in a lone surrogate that
renders as �.",
"files": ["plugins/workflow/client.js", "plugins/workflow/src/graph.js",
"plugins/workflow/test/client.test.js",
"plugins/workflow/test/graph.test.js",
"changelog.d/1508-title-code-points.fixed.md"],
"checks": { "ran": "node --import ./test/resolve.mjs --test
\"test/*.test.js\" (plugins/workflow)", "result": "green", "at":
"0542e075b6ab68016642f9b732f7e3a6d352dd8f" },
  "next": "none — ready for a T2 PASS at this tip"
}
<!-- /flybot:checkpoint -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T3 Risk tier T3 (needs a human approval), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(workflow): a sibling Failed run is only in Runs — header/timeline never say it failed

1 participant