From 16f743d93802e8b2a1c86d37f73324f6a979ab11 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 5 Nov 2026 18:02:05 +0800 Subject: [PATCH] client: open a failed workflow run on the Workflows page (#2515) The Inbox row asked Settings for the page 'workflow', but the page key is 'workflows', so the dialog silently fell back to General: the Open action landed on General instead of Workflow graphs. The test now renders the Settings dialog and reads the current page off the nav, instead of asserting the string the call carried, which could not tell the two keys apart. --- .../2515-inbox-workflow-open-page.fixed.md | 1 + core/client.js | 2 +- core/src/client/inbox.js | 3 +- core/test/inbox-workflow-failure.test.js | 80 ++++++++++++++++++- docs/subsystems/core-client.md | 2 +- patches/ds-bot/0008-client-bundle.patch | 2 +- patches/ds-bot/files/src/client/inbox.js | 3 +- .../files/test/inbox-workflow-failure.test.js | 80 ++++++++++++++++++- 8 files changed, 164 insertions(+), 9 deletions(-) create mode 100644 changelog.d/2515-inbox-workflow-open-page.fixed.md diff --git a/changelog.d/2515-inbox-workflow-open-page.fixed.md b/changelog.d/2515-inbox-workflow-open-page.fixed.md new file mode 100644 index 00000000..38e39796 --- /dev/null +++ b/changelog.d/2515-inbox-workflow-open-page.fixed.md @@ -0,0 +1 @@ +A failed workflow run's Open in the Inbox now lands on Settings → Workflows: the row asked for a page named `workflow`, which is not a Settings key, so the dialog quietly fell back to General. The test renders the Settings dialog and reads the page it shows instead of trusting the name the call carried. diff --git a/core/client.js b/core/client.js index eb69f506..0302a97e 100644 --- a/core/client.js +++ b/core/client.js @@ -9325,7 +9325,7 @@ function useNow2(ms) { function openCard(actions, card) { actions.closeInbox(); if (card?.kind === "workflow") { - actions.openSettings("workflow"); + actions.openSettings("workflows"); return; } actions.openSession(card.chat); diff --git a/core/src/client/inbox.js b/core/src/client/inbox.js index 43fe5b98..eea25aeb 100644 --- a/core/src/client/inbox.js +++ b/core/src/client/inbox.js @@ -598,10 +598,11 @@ function useNow(ms) { // Open on a card. A failed workflow run has no chat to land in, so it opens the // Workflow graphs page in settings instead (#1706); every other card keeps the chat. +// The page key is `workflows` — Settings falls back to General for any key it does not list. export function openCard(actions, card) { actions.closeInbox() if (card?.kind === 'workflow') { - actions.openSettings('workflow') + actions.openSettings('workflows') return } actions.openSession(card.chat) diff --git a/core/test/inbox-workflow-failure.test.js b/core/test/inbox-workflow-failure.test.js index 3047e348..2140d01a 100644 --- a/core/test/inbox-workflow-failure.test.js +++ b/core/test/inbox-workflow-failure.test.js @@ -49,6 +49,75 @@ const workflowPage = await load(await bundleOf('../src/client/workflows.js')) const { inboxModel, needsCount, InboxCard, InboxDialog, openCard, workflowsOf, inboxAnswer, GROUPS } = inbox const { readWorkflows } = workflowPage +// The page a workflow row opens is only real once Settings renders it: `openSettings('workflow')` +// and `openSettings('workflows')` record the same shape of call, and Settings quietly falls back +// to General for a key it does not know. This second harness renders the dialog the way +// settings-readonly-pages.test.js does, so the page is read off the nav (`aria-current="page"`) +// instead of off the string the call carried. +const { outputFiles: settingsFiles } = await build({ + stdin: { + contents: "export { SettingsDialog } from './src/client/settings.js'", + resolveDir: fileURLToPath(new URL('..', import.meta.url)), + }, + bundle: true, format: 'cjs', platform: 'browser', write: false, logLevel: 'silent', + external: ['react', 'react-dom', '@deepseek-ai/dsh-client-ui-primitives'], +}) +const hChild = (type, props, ...children) => ({ type, props: { ...props, children: children.flat(Infinity) } }) +const slots = [] +let hookIndex = 0 +const settingsReact = { + createElement: hChild, Component: class {}, Fragment: Symbol('F'), + useMemo: (fn) => { const at = hookIndex++; if (!(at in slots)) slots[at] = fn(); return slots[at] }, + useState: (initial) => { + const at = hookIndex++ + if (!(at in slots)) slots[at] = typeof initial === 'function' ? initial() : initial + return [slots[at], (value) => { slots[at] = typeof value === 'function' ? value(slots[at]) : value }] + }, + useCallback: (fn) => { const at = hookIndex++; if (!(at in slots)) slots[at] = fn; return slots[at] }, + useRef: (value) => { const at = hookIndex++; if (!(at in slots)) slots[at] = { current: value }; return slots[at] }, + useEffect: () => { hookIndex++ }, + useLayoutEffect: () => { hookIndex++ }, + useId: () => { hookIndex++; return 'id' }, + useSyncExternalStore: (_subscribe, get) => { hookIndex++; return get() }, +} +const settingsModule = { exports: {} } +runInNewContext(settingsFiles[0].text, { + module: settingsModule, exports: settingsModule.exports, console, setTimeout, clearTimeout, + requestAnimationFrame: () => 0, cancelAnimationFrame() {}, + document: { hidden: false, addEventListener() {}, removeEventListener() {}, createElement: () => ({ getContext: () => ({}) }) }, + navigator: { language: 'en' }, + localStorage: { getItem: () => null, setItem() {} }, + window: { location: { hash: '' }, addEventListener() {}, removeEventListener() {}, matchMedia: () => ({ matches: false, addEventListener() {}, removeEventListener() {} }) }, + require: (name) => { + if (name === 'react') return settingsReact + if (name === 'react-dom') return { createPortal: node => node } + if (name === '@deepseek-ai/dsh-client-ui-primitives') return {} + throw new Error(`no platform module ${name}`) + }, +}) +const { SettingsDialog } = settingsModule.exports +const settingsRoster = { + bots: [], byId: {}, rooms: [], roomsById: {}, prefs: { theme: 'deepseek', motion: 'quiet' }, + models: [], connectors: [], capabilities: [], secrets: [], humans: [], remoteBots: [], mainBotIds: [], memory: false, +} +// The nav buttons carry `aria-current="page"` on the page the dialog shows; an unknown key +// lands on General, which is exactly the defect this reads. +const currentSettingsPages = (page) => { + slots.length = 0 + hookIndex = 0 + const found = [] + const walk = (node) => { + if (Array.isArray(node)) { node.forEach(walk); return } + if (!node || typeof node !== 'object') return + if (node.type === 'button' && node.props?.['aria-current'] === 'page') { + found.push(node.props.children.filter(child => typeof child === 'string').join('')) + } + walk(node.props?.children) + } + walk(SettingsDialog({ page, roster: settingsRoster, actions: { closeSettings() {}, openSettings() {}, notify() {} }, useUpdate: () => ({}) })) + return found +} + const plain = value => JSON.parse(JSON.stringify(value)) const OWN = new Set(['InboxCard', 'Status', 'AnswerFields', 'ResultActions']) const expand = (node) => { @@ -218,13 +287,20 @@ test('#1706: a workflow row has no chat, opens the Workflow graphs page, and ans assert.equal(card.bot?.name, 'Writer') const log = [] + let opened = null const actions = { closeInbox: () => log.push('close'), - openSettings: page => log.push(`settings ${page}`), + openSettings: page => { opened = page; log.push(`settings ${page}`) }, openSession: id => log.push(`session ${id}`), } openCard(actions, card) - assert.deepEqual(log, ['close', 'settings workflow']) + assert.equal(log[0], 'close', 'the Inbox closes behind the row') + assert.ok(opened, 'a workflow row opens Settings') + // Where the row lands is checked by rendering the page it asks for: the Settings key is + // "workflows", and a key the dialog does not know falls back to General without a word. + const shown = currentSettingsPages(opened) + assert.deepEqual(shown, ['Workflows'], `the row shows the Workflows page, not ${shown.join(', ') || 'nothing'}`) + assert.ok(!shown.includes('General'), 'a failed workflow row never lands the reader on General') assert.equal(inboxAnswer(actions, card, 'allow'), undefined) const rendered = expand(InboxCard({ card, roster, now: NOW, busy: false, error: '', onAnswer: () => { throw new Error('a workflow row has no answer field') }, onOpen: () => openCard(actions, card) })) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index 29ee0973..2bb2c579 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -49,7 +49,7 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl | `core/src/shared/approval-waiting.js` | The one rule for "this hold still waits for the owner's decision" (#734, #754): `isApprovalWaiting(status)` is true for `pending` and `pending_approval` and false for every settled spelling (`approved`, `auto`, `failed`, `rejected`, `stale`, missing). The host writes `pending`; Bot Social's own outbound hold arrives as `pending_approval`. Host and client both import this one file — the held-reply card, the approvals overlay and the chat bubble on this side, `hooks.js` (`decide()`, `heldOf()`, the cap, the in-flight filter, the cleanup) and `channel.js` (a row's wait badge) on the other — and the overlay asks the host for `[...WAITING_STATUSES, 'failed']`, so the two spellings cannot drift apart again on one side only. Tests: `core/test/approval-waiting.test.js`, `core/test/host-approval-status.test.js`. | | `core/src/shared/held-retry.js` | The one rule for "this failure can still be retried" (#968): `canRetry(record)` is false for `retry: false`, for `noRetry: true`, and for a record whose stored `error`/`code` names a terminal reason (`no such remote bot` / `is not shared with your Bots` / `is no longer shared with you` / `has not granted chat.public` / `bot_not_shared` / `bot_not_found`; a bare `capability_not_granted` is not one, #988), true otherwise. The host states the same answer in two spellings (`noRetry` on an approval, `retry: false` on a held reply), so both cards and Bot Social's own dock ask this one function instead of keeping a copy each; a hold written before the flag existed is read from its stored sentence at render time, with no state migration. Tests: `core/test/held-retry.test.js`. | | `core/src/client/schedules.js` | Settings → Scheduled tasks, a Bot's card and the details line. Each row has **Edit** (pencil; the row turns into the same form that creates a task, filled in, with **Save changes**; a new task opens with no target chosen — **Send to** reads "Pick a Bot or group chat" and **Create task** stays disabled until one is picked, so one click cannot create the task for the first Bot (#1334); an edited task keeps its time zone, a cron rule it cannot show is kept), **Copy** (`CopyRow`, #1287: the row turns into the create form's target picker, minus the chat the task already runs in, and **Copy** posts `schedule-create` with `copyFrom: { sessionId, id }` — the host writes the row, so the copy keeps the source's next run) and **Pause** / **Resume**; a paused row reads **Paused** (#354). A row with a recorded run adds one line under it: "Running since …", "Last run …", "Last run … · Stopped" or, in red, "Last run … · Failed: "; a failed run of a task that is not paused gets **Retry**, and any other task the host will run on demand — active, with no run in flight — gets **Run now**; both call the same `actions.scheduleRetry` and run the task once now without moving DSH's own next time (#473, #2507). With Automation tasks off, the hint reads "Switch on Automation tasks on the Plugins page." and the button **Open Plugins** closes the Settings dialog and opens DSH's Plugins panel (`actions.selectPanel('plugins')`); the Harness settings dialog has no such switch (#378). The page opens with the team's **Quiet hours** card (#1166): **Off** with a **Turn on** button, or **On, 22:00–07:00** with the window's two time fields and **Save window** (the settings write refuses an invalid window, and the host's named refusal appears on the form's error line, as a `role="alert"` so it is announced and not only printed (#2232)). While quiet hours are on and an `every`/`cron` task exists, the view's `quietUnchecked` flag draws one line saying such a task can still run inside the window: that flag is the only place the rule is computed — the page does not scan the task list itself (#1199) — and it asks for the view again right after the settings write, so the line does not wait for the 30-second poll. Never a silent claim of silence, and no suppressed count. A repeat whose interval is not a whole number of minutes is stated as seconds (`Every 90 seconds`, #1651): the edit form opens holding that exact value in the **seconds** unit, and saving a task whose repeat was not touched sends the stored interval back unchanged instead of a rounded number of minutes. The details panel's line (`ScheduleSummary`) lists every task of that Bot, paused and finished ones included, because a Bot whose tasks are all paused has tasks while a Bot with none is the only empty state; each row answers when — `Next: