Repository navigation
fix(inbox): a failed workflow run reaches the Inbox - #1717
Conversation
|
Verdict: PASS at f34b145 Ran at this tip: Red first: with The one red in Scope and gates:
|
…reasoning line runs wide (ThinkFlowLab#1721) Fixes ThinkFlowLab#1718 Written-by: Dev-1 Owner: Dev-1 Reviewer: Reviewer Branch: dev/1-think-row-clip Base: f0159ea (live main; the issue named aafb8d7, then ThinkFlowLab#1708 moved main to a932752, ThinkFlowLab#1717/ThinkFlowLab#1716 to f8e4943, and ThinkFlowLab#1715 to f0159ea. `styles.js` and this row's DOM are byte-identical on all of them; ThinkFlowLab#1715 re-rendered `core/client.js` and re-encoded `patches/ds-bot/0008`, which made this branch conflict, so it is rebased once onto f0159ea and the mirrors are regenerated) Tip: acb38f4 Tree: 95d9c33 Docs-not-needed: ThinkFlowLab#1718 bounds three CSS rules of the thinking row; no architecture, interface or contract changes. ## What changed The folded thinking row is a `<details class="bt-think">` whose `<summary>` is one `inline-flex` line. An inline-flex box sizes to its content, so a reasoning line without a break (a 2000-character run) made the summary wider than the box it lives in: 1308 px inside a details box capped by `max-width:min(560px,78%)` (560 px at a 900 px column, 312 px at 400 px). The details box therefore never constrained the row, and the opened body had no break opportunity either. Four declarations, all in `core/src/client/styles.js`: - `.bt-think summary` gains `max-width:100%;min-width:0;white-space:nowrap`. `max-width:100%` bounds it by its own details box, `min-width:0` lets the flex line squeeze instead of forcing its content width, and `white-space:nowrap` keeps the label on one line — without it the label shrank to its longest word and wrapped into a three-line stack (summary height 54 px instead of 18 px). - `.bt-think-peek` gains `max-width:100%`; it already had `min-width:0;overflow:hidden;text-overflow:ellipsis;white-space:nowrap`, so now the ellipsis fills whatever the summary leaves instead of the content width. - `.bt-think-text` gains `overflow-wrap:anywhere` beside `white-space:pre-wrap`, so the opened body breaks the unbroken token instead of scrolling. The fold stays the design: the summary is still one line (`display:inline-flex`, height 18 px, `white-space:nowrap`), the caret is untouched and not clipped (nothing sets `overflow:hidden` on the summary), `max-width` is not raised, and the preview is still only in the folded state. `ThinkFlowLab#1636` (opened row hides the peek), `ThinkFlowLab#1642` (fence-only gets “{n} line(s) of reasoning”), `ThinkFlowLab#1624` (no reasoning, no row) keep passing. `core/client.js` is regenerated (`node core/scripts/build-client.mjs`), and `patches/ds-bot/{0003,0008}` + `files/test/thinking-row.test.js` come from `node scripts/ds-bot-patches.mjs generate` — no hunk is written by hand. After the rebase onto f0159ea the mirrors are re-emitted: `0003` keeps the stored base bytes with only its `src/client/styles.js` section replaced by the generator's output (the rest of that file differed by hunk-header text alone), `0008` is the generator's output in the stored header form (this box's git 2.34.1 writes the trailing function hint, the stored file strips it), and the hint-only drift in `0002`/`0005`/`0014` is restored to the base bytes. `patches check` and `build-client --check` are green. ## Evidence: geometry, base → fix The unit suite runs on jsdom, which lays nothing out — every element reports `scrollWidth === clientWidth === 0` — and CI runs no browser, so CI carries the containment assertions below. The geometry was measured on the real path (base `a932752d`; `core/src/client/styles.js` and `core/src/client/cells.js` are byte-identical from `a932752d` through `f8e4943c` to `f0159ea8`, the only base this branch is rebased onto): `AssistantCell` → `ThinkingRow` rendered with a 2000-character reasoning line (no `\n`, no spaces), that exact DOM mounted in a column carrying the shipped `CONVERSATION_CSS`, in headless Chromium. Base `a932752d`: | container | details box | summary width | peek client / line | opened body | | --- | --- | --- | --- | --- | | 900 px | 560 | **1308** (escapes by 748) | 1174 / 1236 | 560 / 15397 scrolls sideways | | 400 px | 312 | **1308** (escapes by 996) | 1174 / 1236 | 312 / 15397 scrolls sideways | At this tip: | container | details box | summary width | peek client / line | opened body | | --- | --- | --- | --- | --- | | 900 px | 560 | 560 | 420 / 1236, ellipsis | 560 / 560, wraps | | 400 px | 312 | 312 | 172 / 1236, ellipsis | 312 / 312, wraps | (The peek's box is 420 / 172 px here rather than 488 / 240: the summary's `white-space:nowrap` keeps the whole `Thought it through` label on its line, so 120 px of the row is the label instead of the label wrapping into three lines.) Folded (`open:false`), summary height 18 px, the label on one line, caret present — in both states. **Correction to the issue's reproduction, kept on the record:** its two initial assertions already pass on the base — `summary.scrollWidth <= summary.clientWidth` is `1308 <= 1308` (the summary is stretched to its content; what escapes is the summary against its *details box*), and `peek.scrollWidth > peek.clientWidth` is `1236 > 1174` (the ellipsis already fires). The defect that actually fails on the base is `summary.clientWidth > details.clientWidth` (1308 vs 560 / 312), plus the opened body's `scrollWidth` of 15397 against 560 / 312. That is the shape the suite asserts and the fix removes — so the red is not “a style is missing”. ## Tests `core/test/thinking-row.test.js` already mounts this row through `AssistantCell`; it gains three cases: - the 2000-character line mounted at 900 px **and** 400 px in the shipped stylesheet: summary `max-width:100%`, `min-width:0`, `white-space:nowrap`; peek `max-width:100%` with `overflow:hidden;text-overflow:ellipsis;white-space:nowrap`; body `overflow-wrap:anywhere` with `white-space:pre-wrap`; the details ceiling still `min(560px,78%)`; the row folded with its caret and `Thought it through`. - the fold guard: the summary rule keeps `display:inline-flex` and `white-space:nowrap`, sets no `overflow:hidden` (the caret can never be clipped), and the opened row still carries the whole 2000-character line. - the bundle guard: the app loads `core/client.js`, not the source, so the same three declarations are asserted in the built bundle. Red first at the base `f0159ea8`, with only this test file present and the two generated/source files taken from that base (`git checkout f0159ea -- core/src/client/styles.js core/client.js`): ```text not ok 10 - \ThinkFlowLab#1718: a 2000-character line cannot widen the folded row past its box (900 px and 400 px) error: |- summary is bounded by its box at 900px expected: '100%' actual: 'none' not ok 11 - \ThinkFlowLab#1718: the fold stays the design — one line, no block summary, no hidden caret error: 'the label cannot wrap into a stacked block' actual: '.bt-think summary{cursor:pointer;list-style:none;display:inline-flex;align-items:center;gap:6px}' not ok 12 - \ThinkFlowLab#1718: the bundle the app loads carries the same containment rules # tests 12 / # pass 9 / # fail 3 ``` At this tip the same file is 12/12, and the pre-existing `ThinkFlowLab#1624` / `ThinkFlowLab#1636` / `ThinkFlowLab#1642` cases stay green. ## What was run Ran at this tip: `TZ=UTC ARCH_DOCS_BASE=f0159ea8 npm run verify` → exit 0 (core 2028/2028, social 539/539, tree 125, timeline 129, workflow 279, `.github/scripts` 549, interop 37, live-e2e 24 — zero `not ok`; `arch-docs: OK (7 changed file(s))`; `ok 10 patches apply cleanly in order`, `ok core/ equals ds-bot/ + patches/ds-bot/`). `node scripts/ds-bot-patches.mjs check` printed its three green lines, and `node core/scripts/build-client.mjs --check` is silent-green. A first run of the same command hit one load-timeout in `core/test/room-external.test.js` (ThinkFlowLab#198, `timed out waiting for the child`); that file passes alone (13/13) and the rerun is the run quoted here. Not run: the smoke test — this is a client CSS change with no `core/index.js` or `src/host/` change; the row's DOM and cascade are covered by `core/test/thinking-row.test.js`. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 1721, "claim": "Dev-1", "issue": 1718, "summary": "The folded thinking row no longer runs past its column. The inline-flex summary was sizing itself to a long unbroken reasoning line (1308 px inside a 560 px details box); it is now bounded by its box with max-width:100% and min-width:0, and white-space:nowrap keeps the label on one line. The preview fills what is left and ellipsizes, and the opened body breaks the same token instead of scrolling sideways. Fold, caret, the peek and the ThinkFlowLab#1624/ThinkFlowLab#1636/ThinkFlowLab#1642 behaviour are unchanged.", "files": ["core/src/client/styles.js", "core/client.js", "core/test/thinking-row.test.js", "changelog.d/1718-think-row-clip.fixed.md", "patches/ds-bot/0003-flybot-api-client.patch", "patches/ds-bot/0008-client-bundle.patch", "patches/ds-bot/files/test/thinking-row.test.js"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=f0159ea8 npm run verify", "result": "green", "at": "acb38f44cd8265c819e52ae0f61fef6ded8477bf" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
Fixes #1706
Written-by: Dev-1
Owner: Dev-1
Reviewer: Birfy
Branch: dev/1-workflow-failure-inbox
Base: aafb8d7 (live main)
Tip: f34b145
Tree: 371e6d8679614fef77fda465bf2a78c7a26bc632
What changed
A workflow run that fails wrote its failure only into the plugin's own state, so the Inbox — the one place the owner looks — showed nothing. Now each graph's newest failed run is one Needs you card, on the existing
InboxCardpath withSTATUSES.failed:plugins/workflow/src/service.js:publicRungains exactly two fields.finishedAtisnullwhile the run is live, else the moment it ended (run.updatedAttoday;run.finishedAtif it ever exists).currentStepis{id, name}fromorderedIds(workflow, run.nodeStatus, 'running')[0]— the walk the stage already uses — andnullwhen the run is notstatus:'running'. A paused run is stillstatus:'running' + paused:true(read from the engine, not assumed), so it is not finished and still names its node. No field renamed, no old field's meaning changed;plugins/workflow/test/run-progress.test.jspins the key set and the four states (running / paused / done / failed).core/src/client/inbox.js:inboxModeltakes theworkflow/statepack'sworkflows. Per graph it mergesrunsandhistoryby run id (runswins), keeps failed runs inside the samefresh()7-day window Done uses, and emits one card — that graph's newest failure; older ones stay on the graph's Runs tab. Title is<graph> · <failure.nodeName>(graph alone when no node is named), reason isfailure.text || failure.code,atis the end time. A failed run whose task card is gone still reaches the list viahistory.data-run(neverdata-task), nochatand noplace; a Bot is attached only whenrun.task.callerBotIdis a string, and only for attribution. Open goes toactions.openSettings('workflow')— the Workflow graphs page — because a failed run has no chat. There is no per-graph deep link today (WorkflowSettings({call})takes no target), so this PR promises only the page, not a jump to that graph.inboxAnswerreturnsundefinedforkind:'workflow', so the row has exactly one control, Open.core/src/client/index.js: one action,workflowState: () => readWorkflows(call), so the Inbox reuses the Workflows page's 8s settle and missing-plugin mapping instead of a bare call. Onlyphase === 'ready'yields graphs; a missing plugin / timeout / host error is zero rows and no error line.core/src/client/i18n.js,docs/subsystems/core-client.md,docs/subsystems/workflow.md,changelog.d/1706-workflow-failure-inbox.fixed.mdfollow the change.patches/ds-bot/**regenerated withnode scripts/ds-bot-patches.mjs generateagainst this base:0001(i18n),0003(index.js),0008(client.js) andfiles/src/client/inbox.js+files/test/inbox-workflow-failure.test.js. The 0008 patch shows hunk-header churn from this box's git (2.34.1) re-diffing the generated bundle; the payload is the new bundle, and bothchecklines are green.Verification at the tip
Red first at this base
aafb8d74(also re-checked on2213b4cabefore it moved), with only the new test file added and the three sources taken from the base (git checkout aafb8d74 -- core/src/client/inbox.js core/src/client/index.js plugins/workflow/src/service.js),cd core && DSH_HARNESS=… node --import ./test/harness-resolve.mjs --test test/inbox-workflow-failure.test.js→ exit 1, 6 failed / 2 passed:At the tip the same file is 8/8, and
plugins/workflow/test/run-progress.test.jsis 4/4.Ran at this tip:
TZ=UTC ARCH_DOCS_BASE=aafb8d74 npm run verify→ exit 0 (core 2004/2004, social 539, tree 125, timeline 129, workflow 279,.github/scripts549, interop 37, live-e2e 24 — zeronot ok;arch-docs: OK (15 changed file(s));ok 10 patches apply cleanly in order,ok core/ equals ds-bot/ + patches/ds-bot/).Not run: the smoke test — it needs a bench run on a free port with a DSH checkout; this change is client-side only (no
core/index.jsorsrc/host/change), and the Inbox model, click target, andpublicRunshape are covered by the two test files above.Checkpoint
{ "pr": 1717, "claim": "Dev-1", "issue": 1706, "summary": "A failed workflow run now reaches the Inbox as one Needs you card per graph: its newest failure inside the same 7-day window Done uses, titled graph · failed node, with the failure text as its reason and data-run on the row. Open goes to Settings → Workflow graphs because the run has no chat and there is no per-graph deep link yet; the row has no answer control and is attributed only when the run names a callerBotId. publicRun gains finishedAt and currentStep (pure additions) so the Inbox can time the run and name its node the same way the stage does.", "files": ["core/src/client/inbox.js", "core/src/client/index.js", "core/src/client/i18n.js", "core/client.js", "core/test/inbox-workflow-failure.test.js", "plugins/workflow/src/service.js", "plugins/workflow/test/run-progress.test.js", "docs/subsystems/core-client.md", "docs/subsystems/workflow.md", "changelog.d/1706-workflow-failure-inbox.fixed.md", "patches/ds-bot/files/src/client/inbox.js"], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=2213b4ca npm run verify", "result": "green", "at": "e8db8b30f519bb3bc112c54a8965c16167705101" }, "next": "none — ready for review" }