diff --git a/changelog.d/2354-reach-ready-only.fixed.md b/changelog.d/2354-reach-ready-only.fixed.md new file mode 100644 index 000000000..3c3c37c9a --- /dev/null +++ b/changelog.d/2354-reach-ready-only.fixed.md @@ -0,0 +1 @@ +A schedule row no longer names a connector whose connection failed, or one still connecting, as carried by its unattended run: the host unmounts a failed connection before it writes `error` and a `connecting` one has no tools yet, so only a `ready` connector appears in the row's `Runs with …` sentence, while the Connectors page keeps the switch that reconnects it ([#2354](https://github.com/ThinkFlowLab/FlyBot/issues/2354)). diff --git a/core/client.js b/core/client.js index 442f75a1c..d6914c58f 100644 --- a/core/client.js +++ b/core/client.js @@ -5573,7 +5573,7 @@ function reachText(task, roster) { if (!task?.botId) return ""; const bot = roster?.byId?.[task.botId] ?? (roster?.bots ?? []).find((entry) => entry.id === task.botId); if (bot === void 0) return ""; - const names = connectedConnectors(roster).filter((connector) => isOn(bot, connector.id)).map((connector) => connector.name); + const names = connectedConnectors(roster).filter((connector) => connector.state === "ready" && isOn(bot, connector.id)).map((connector) => connector.name); return names.length === 0 ? t("Runs with no connectors") : t("Runs with {names}", { names: names.join(", ") }); } var lockedWindow = (roster) => typeof roster?.readOnly === "string" && roster.readOnly.trim() !== ""; diff --git a/core/src/client/schedules.js b/core/src/client/schedules.js index 9c5d4c70c..6682042a1 100644 --- a/core/src/client/schedules.js +++ b/core/src/client/schedules.js @@ -122,12 +122,17 @@ export function lastRunText(run) { // stored with the task, so turning a connector on today changes what a task written weeks ago will // use. A room task (no `botId`) has no single Bot's switch, and a Bot the roster no longer carries // says nothing rather than guessing. `connectedConnectors` drops a sender and a webhook-only -// channel, which have neither tools nor a way in; `isOn` keeps only what this Bot may use. +// channel, which have neither tools nor a way in; `isOn` keeps only what this Bot may use. Only a +// `ready` connection gives the run tools (#2354): the host unmounts a failed connection before it +// writes `error`, and a `connecting` one has none yet — `connectedConnectors` still names both so +// the Connectors page can draw the switch that reconnects them. export function reachText(task, roster) { if (!task?.botId) return '' const bot = roster?.byId?.[task.botId] ?? (roster?.bots ?? []).find(entry => entry.id === task.botId) if (bot === undefined) return '' - const names = connectedConnectors(roster).filter(connector => isOn(bot, connector.id)).map(connector => connector.name) + const names = connectedConnectors(roster) + .filter(connector => connector.state === 'ready' && isOn(bot, connector.id)) + .map(connector => connector.name) return names.length === 0 ? t('Runs with no connectors') : t('Runs with {names}', { names: names.join(', ') }) } diff --git a/core/test/schedules-client.test.js b/core/test/schedules-client.test.js index 12c4abccd..dfe44c4bf 100644 --- a/core/test/schedules-client.test.js +++ b/core/test/schedules-client.test.js @@ -846,9 +846,13 @@ test('P-482: a locked Bot card with tasks keeps the heading and the rows', () => // with the task, so its row says which connectors an unattended run would carry. A room task has // no single Bot's switch, so it says nothing: drawing one would state a falsehood. const CONNECTORS = [ - { id: 'github', name: 'GitHub', state: 'connected' }, - { id: 'composio', name: 'Composio', state: 'connected' }, + { id: 'github', name: 'GitHub', state: 'ready' }, + { id: 'composio', name: 'Composio', state: 'ready' }, ] +// The host writes exactly four connector states — off, connecting, ready, error — and the row +// names only what can give the run tools (#2354). The fixture below used to say 'connected', a +// value the host never writes, which is why any predicate let it through. +const ERRORED = { id: 'composio', name: 'Composio', state: 'error' } const reachRoster = bot => ({ bots: [bot], rooms: [], byId: { [bot.id]: bot }, roomsById: {}, connectors: CONNECTORS }) const reachLines = roster => { seeded.push({ available: true, tasks: [ACTIVE] }, null) @@ -872,7 +876,7 @@ test('#2323 c: every connected connector off reads as no connectors', () => { }) test('#2323 d: a sender connector never appears in the sentence', () => { - const roster = { ...reachRoster({ id: 'b1', name: 'B' }), connectors: [...CONNECTORS, { id: 'feishu', name: 'Feishu', state: 'connected', kind: 'sender' }] } + const roster = { ...reachRoster({ id: 'b1', name: 'B' }), connectors: [...CONNECTORS, { id: 'feishu', name: 'Feishu', state: 'ready', kind: 'sender' }] } const lines = reachLines(roster) assert.equal(reachOf(lines), 'Runs with GitHub, Composio') assert.ok(!lines.join('|').includes('Feishu'), 'a sender has neither tools nor a way in') @@ -885,3 +889,26 @@ test('#2323 e: a room task says nothing about reach', () => { const lines = text(render(RoomSchedules({ room: { id: 'room-1', name: 'Team room', origin: 'group' }, actions: spy() }))) assert.ok(!lines.some(line => line.startsWith('Runs with')), lines.join('|')) }) + +// #2354: reconcile() unmounts a connection before it writes `error` +// (core/src/host/connectors.js:750-757), so a failed connector's tools are gone and its row must +// not promise them; `connecting` has no tools yet either. The Connectors page still draws a switch +// for both, which is how the owner reconnects, so `connectedConnectors` keeps its own predicate. +test('#2354 a: a failed connection is not named as carried, a ready one still is', () => { + const roster = { ...reachRoster({ id: 'b1', name: 'B' }), connectors: [CONNECTORS[0], ERRORED] } + const lines = reachLines(roster) + assert.equal(reachOf(lines), 'Runs with GitHub') + assert.ok(!lines.join('|').includes('Composio'), 'a connector whose connection failed carries nothing') +}) + +test('#2354 b: nothing ready left reads as no connectors, not as the failed ones', () => { + const roster = { ...reachRoster({ id: 'b1', name: 'B' }), connectors: [{ id: 'github', name: 'GitHub', state: 'error' }, ERRORED] } + assert.equal(reachOf(reachLines(roster)), 'Runs with no connectors') +}) + +test('#2354 c: a connector still connecting is not named', () => { + const roster = { ...reachRoster({ id: 'b1', name: 'B' }), connectors: [CONNECTORS[0], { id: 'composio', name: 'Composio', state: 'connecting' }] } + const lines = reachLines(roster) + assert.equal(reachOf(lines), 'Runs with GitHub') + assert.ok(!lines.join('|').includes('Composio'), 'a connecting connector has no tools yet') +}) diff --git a/docs/subsystems/core-client.md b/docs/subsystems/core-client.md index 8538057ac..abdd45286 100644 --- a/docs/subsystems/core-client.md +++ b/docs/subsystems/core-client.md @@ -46,7 +46,7 @@ The browser side of the patched `ds-bot` core (`core/`): sources in `core/src/cl | `core/src/client/panes.js` | `PANE_MIN_WIDTH_CSS` (#705): every pane the multi-pane row flexes gets `min-width:0`, the conversation column is matched by its DSH class fragment, and the panel's long values (About text, `4.1M tok · Cache hit …`) wrap with `overflow-wrap:anywhere`, so a value that cannot wrap never widens the panel past the space reserved for it. The same sheet clips horizontal overflow on the document, the conversation column and the app frame (`overflow-x:clip`, #1086), and `watchShellLevel` / `levelShell` put a leftover `scrollLeft` back, so an error toast or dialog cannot leave the window shifted. Tests: `core/test/shell-level.test.js`. | | `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**, which runs the task once now (`actions.scheduleRetry`, #473). 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: