diff --git a/changelog.d/1068-remote-dm-failure.fixed.md b/changelog.d/1068-remote-dm-failure.fixed.md new file mode 100644 index 000000000..c8a200001 --- /dev/null +++ b/changelog.d/1068-remote-dm-failure.fixed.md @@ -0,0 +1 @@ +A friend DM shows when the other home's Bot could not reply (for example it has no API key) instead of staying on "Weighing the options…" ([#1068](https://github.com/ThinkFlowLab/FlyBot/issues/1068)). diff --git a/core/src/host/hooks.js b/core/src/host/hooks.js index bb2b6226a..9287890d4 100644 --- a/core/src/host/hooks.js +++ b/core/src/host/hooks.js @@ -1185,9 +1185,17 @@ export function install(rt) { // A local Bot's answer to a friend's Bot (turns.js): back through bot.send, so the // plugin's middleware still decides (auto-accept, or the approval hold). // `replyIn` names the chat and turn whose text this is, so a held reply is drawn on that turn (#418). - const replyRemote = ({ from, handle, home, text, replyTo, replyIn }) => send({ - from, to: { kind: 'remote', handle, ...(home ? { home } : {}) }, text, role: 'reply', via: 'auto-reply', - meta: { replyTo, ...(typeof replyIn?.sessionId === 'string' && (Number.isInteger(replyIn.turn) || (typeof replyIn.turn === 'string' && replyIn.turn !== '')) ? { replyIn: { sessionId: replyIn.sessionId, turn: replyIn.turn } } : {}) }, + // `failure` is the host's own sentence for a turn that died (#1068): via `turn-failed`, not an + // auto-reply, so the model-reply hold does not keep the sender weighing. The plugin allows it + // only when the text is one of the two fixed sentences. + const replyRemote = ({ from, handle, home, text, replyTo, replyIn, failure }) => send({ + from, to: { kind: 'remote', handle, ...(home ? { home } : {}) }, text, role: 'reply', + via: failure === true ? 'turn-failed' : 'auto-reply', + meta: { + replyTo, + ...(failure === true ? { notice: 'turn-failed' } : {}), + ...(typeof replyIn?.sessionId === 'string' && (Number.isInteger(replyIn.turn) || (typeof replyIn.turn === 'string' && replyIn.turn !== '')) ? { replyIn: { sessionId: replyIn.sessionId, turn: replyIn.turn } } : {}), + }, }) rt.flybotGrantsChanged = botId => { const bot = rt.botOf(botId); if (bot !== undefined && bot.remote === undefined) fire('bot.grants', view(bot)) } Object.assign(rt, { flybot, flybotReplyRemote: replyRemote, flybotEndpoint: endpoint, flybotRemoteMembers: remoteMembers.list, flybotSend: send, flybotIsRead: isReadEndpoint, flybotHideTools: hidePluginTools }) diff --git a/core/src/host/turns.js b/core/src/host/turns.js index 21c0adaa3..896e07074 100644 --- a/core/src/host/turns.js +++ b/core/src/host/turns.js @@ -3,10 +3,32 @@ // // Turn tracking: which turns were triggered by another Bot, and what they said. import { REFERENCE_KIND, REPLY_KIND, SOURCE_KIND } from './constants.js' +import { unwrapUntrusted } from './delivery.js' import { textOf } from './text.js' import { reminderText } from './schedules.js' import { OTHER, combine, originOfSource, settled } from './turn-origin.js' +// What a friend's DM is told when the turn that should have answered died (#1068). Two fixed +// sentences, never the provider's text: a missing-key failure can echo part of a key. The social +// plugin allows only these, so a lookalike stays a normal held reply. Kept equal by its test. +export const NO_MODEL_NOTICE = 'Could not reply: this Bot has no model set up yet.' +export const COULD_NOT_NOTICE = 'Could not reply: this Bot could not answer right now.' +export const REMOTE_FAILURE_NOTICES = Object.freeze([NO_MODEL_NOTICE, COULD_NOT_NOTICE]) +// The client's missingModelKey (model-menu.js) reads the same provider line. AUTH is not this: +// a rejected key is a different failure, and its message is already dropped by turnFailure. +const MISSING_KEY = /\bno api key\b|\bmissing (?:an? )?api key\b|\bapi key (?:is )?(?:missing|not set|not configured)\b/i + +export function remoteFailureNotice(failure) { + const code = typeof failure?.code === 'string' ? failure.code : '' + const message = typeof failure?.message === 'string' ? failure.message : '' + if (code === 'MISSING_CREDENTIAL' || code === 'NO_KEY' || (code !== 'AUTH' && MISSING_KEY.test(message))) return NO_MODEL_NOTICE + return COULD_NOT_NOTICE +} + +export function isRemoteFailureNotice(text) { + return REMOTE_FAILURE_NOTICES.includes(String(text ?? '').trim()) +} + // A failed turn's `LlmFailure`, kept to what can be shown and logged. Like DSH's own // error row, an AUTH failure loses its message: providers may echo part of the key. export function turnFailure(error) { @@ -49,6 +71,13 @@ const RETRY_TEXT = 'Your last reply wrote a tool call as plain text ( export function install(rt) { const { ctx, state } = rt + // A friend's line whose body is already a failure notice. Answering it with another would + // bounce the same sentence between two homes that cannot run a model. + const failureEchoes = new WeakSet() + const noticeBody = (content) => { + const raw = textOf(content).replace(/^\[[^\]\n]*\]\n?/, '').trim() + return isRemoteFailureNotice(unwrapUntrusted(raw)) || isRemoteFailureNotice(raw) + } // The retry goes where the turn's trigger came from: a group round's waiter or a // Bot's request keeps its exchange; anything else is a nudge in the Bot's own chat. @@ -449,6 +478,7 @@ export function install(rt) { ...(typeof source.origin === 'string' ? { origin: source.origin } : {}), }) } else if (source?.kind === SOURCE_KIND) { + if (fromRemote(source) && noticeBody(event.data?.content)) failureEchoes.add(source) if (entry.turn === undefined) entry.pending = source else if (!opened) claimTrigger(id, entry, source) else if (expectsReply(source) && !answersUser(source) @@ -542,8 +572,17 @@ export function install(rt) { // the turn's text (or a stopped-before-answer line), or nothing. const answer = (source, reply) => { if (fromRemote(source)) { - if (failure === undefined && reply !== '') { - void rt.flybotReplyRemote?.({ from: rt.selfOf(id), handle: source.remote.handle, home: source.remote.home, text: reply, replyTo: source.exchangeId, replyIn: { sessionId: id, turn: answeredTurn } }) + // A hard failure owes the sender a sentence (#1068). Without it their DM stays on + // "Weighing the options…" until the wait expires, and a missing credential never expires + // into an explanation. The sentence is fixed; the provider text stays here. + const notice = failure !== undefined && !failureEchoes.has(source) + if (notice || (failure === undefined && reply !== '')) { + void rt.flybotReplyRemote?.({ + from: rt.selfOf(id), handle: source.remote.handle, home: source.remote.home, + text: notice ? remoteFailureNotice(failure) : reply, + replyTo: source.exchangeId, replyIn: { sessionId: id, turn: answeredTurn }, + ...(notice ? { failure: true } : {}), + }) // A refused reply resolves, it does not throw: say so, or it is lost without a trace (#272). .then(result => { if (result?.action === 'deny') rt.warn('ds-bot: reply to %s not sent: %s', source.remote.handle, result.reason) }) .catch(error => rt.warn('ds-bot: reply to %s failed: %s', source.remote.handle, error)) diff --git a/core/test/bot-messages.test.js b/core/test/bot-messages.test.js index 9f53fa457..faab7c36d 100644 --- a/core/test/bot-messages.test.js +++ b/core/test/bot-messages.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { setTimeout as delay } from 'node:timers/promises' @@ -357,6 +357,109 @@ test('a joined request gets the failure notice when the model call fails', () => assert.match(delivered[0].text, /^Writer could not answer your message: its model call failed \(SERVER 503: upstream busy\)/) }) +// #1068: a friend's DM stays on "Weighing the options…" when the remote home's turn dies, because +// that home never sends anything back. The notice is a fixed sentence: the provider text can echo a key. +const remoteSource = (extra = {}) => replySource('request', { + senderSessionId: undefined, + remote: { handle: '@coder.bob.home', home: '@bob.home', ...extra.remote }, + exchangeId: 'r1', + ...extra, +}) +const fenced = text => textBlock(`[Message from Coder (@coder.bob.home)]\n\n${text}\n`) +function withRemote(rt) { + const sent = [] + rt.flybotReplyRemote = async (args) => { sent.push(args); return { action: 'allow' } } + return sent +} + +test('#1068: a friend line whose turn fails hard is answered with a fixed sentence, and the provider text stays out', () => { + const { rt, emit, delivered } = turnsHost() + const sent = withRemote(rt) + emit('b', 'turn/start', { turn: 4 }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) + emit('b', 'turn/end', { turn: 4, reason: { kind: 'error', error: { code: 'MISSING_CREDENTIAL', message: 'no API key for provider route "deepseek-official" sk-secret' } } }) + assert.equal(delivered.length, 0, 'a remote failure is not a local Bot-to-Bot delivery') + assert.equal(sent.length, 1) + assert.equal(sent[0].failure, true) + assert.equal(sent[0].from, 'b') + assert.equal(sent[0].handle, '@coder.bob.home') + assert.equal(sent[0].home, '@bob.home') + assert.equal(sent[0].replyTo, 'r1') + assert.deepEqual(sent[0].replyIn, { sessionId: 'b', turn: 4 }) + assert.equal(sent[0].text, 'Could not reply: this Bot has no model set up yet.') + assert.equal(sent[0].text.includes('sk-secret'), false) + assert.equal(sent[0].text.includes('deepseek'), false) +}) + +test('#1068: any other hard failure gets the generic sentence, still without the provider text', () => { + const { rt, emit } = turnsHost() + const sent = withRemote(rt) + emit('b', 'turn/start', { turn: 1 }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) + emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'SERVER', message: 'upstream busy sk-secret', status: 503 } } }) + assert.equal(sent.length, 1) + assert.equal(sent[0].failure, true) + assert.equal(sent[0].text, 'Could not reply: this Bot could not answer right now.') + assert.equal(sent[0].text.includes('sk-secret'), false) + assert.equal(sent[0].text.includes('503'), false) + assert.equal(sent[0].text.includes('SERVER'), false) +}) + +test('#1068: a missing-key message under another code uses the no-model sentence', () => { + const { rt, emit } = turnsHost() + const sent = withRemote(rt) + emit('b', 'turn/start', { turn: 1 }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) + emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'PROVIDER', message: 'llm-deepseek: no API key for provider route "deepseek-official"' } } }) + assert.equal(sent[0].text, 'Could not reply: this Bot has no model set up yet.') + assert.equal(sent[0].text.includes('llm-deepseek'), false) +}) + +test('#1068: a finished remote reply is the reply itself; a stopped turn and an echoed notice send nothing', () => { + const { rt, emit } = turnsHost() + const sent = withRemote(rt) + emit('b', 'turn/start', { turn: 1 }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'ok' }) }) + emit('b', 'assistant/message', assistantText('pong')) + emit('b', 'turn/end', { turn: 1, reason: { kind: 'completed' } }) + assert.equal(sent.length, 1) + assert.equal(sent[0].failure, undefined) + assert.equal(sent[0].text, 'pong') + assert.equal(sent[0].replyTo, 'ok') + + emit('b', 'turn/start', { turn: 2 }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'stop' }) }) + emit('b', 'turn/end', { turn: 2, reason: { kind: 'aborted', reason: { kind: 'hook', reason: 'ds-bot/stopped' } } }) + assert.equal(sent.length, 1, 'stopping the turn is not a hard failure') + + const notice = 'Could not reply: this Bot has no model set up yet.' + emit('b', 'turn/start', { turn: 3 }) + emit('b', 'user/message', { content: fenced(notice), source: remoteSource({ exchangeId: 'echo' }) }) + emit('b', 'turn/end', { turn: 3, reason: { kind: 'error', error: { code: 'MISSING_CREDENTIAL', message: 'no API key' } } }) + assert.equal(sent.length, 1, 'a notice that came back does not earn another') +}) + +test('#1068: a remote request that joined a failed turn is told the same way', () => { + const { rt, emit, delivered } = turnsHost() + const sent = withRemote(rt) + emit('b', 'turn/start', { turn: 2 }) + emit('b', 'user/message', { content: textBlock('write the letter'), source: { kind: 'user' } }) + emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'join' }) }) + emit('b', 'turn/end', { turn: 2, reason: { kind: 'error', error: { code: 'AUTH', message: 'sk-secret rejected' } } }) + assert.equal(delivered.length, 0) + assert.equal(sent.length, 1) + assert.equal(sent[0].replyTo, 'join') + assert.equal(sent[0].failure, true) + assert.equal(sent[0].text, 'Could not reply: this Bot could not answer right now.') + assert.equal(sent[0].text.includes('sk-secret'), false) +}) + +test('#1068: the failure notice is not an auto-reply, so the hold on model replies does not keep it', () => { + const src = readFileSync(new URL('../src/host/hooks.js', import.meta.url), 'utf8') + assert.match(src, /failure === true \? 'turn-failed' : 'auto-reply'/) + assert.match(src, /notice: 'turn-failed'/) +}) + test('a joined request never claims the next turn', () => { const { rt, emit, delivered } = turnsHost() emit('b', 'turn/start', { turn: 1 }) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 5e269c92c..ebd14a18d 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -270,6 +270,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - Channel `live()` also sends `pluginTools`: the names of plugin tools a per-Bot mask may take away (`rt.pluginToolNames`, from `hidePluginTools`' gated set), so the client hides their `Tool added` / `Tool removed` rows (#419). A plugin reload changes them without a new revision, so they ride every `state` answer, an `unchanged` one too. - Channel `state` notes when a window on screen last read the team (#497): the client sends `watching: !document.hidden` with every poll (a visible window asks every 1.5 s, a hidden one every 10 s), and `watching: false` does not count; an older client that sends nothing counts as on screen. `rt.watched()` is true for 8 s after such a read and reaches plugins as `flybot.watched()` ([plugin-runtime](plugin-runtime.md)). Process-local, never saved. Covered by `core/test/flybot.test.js` #497, which also locks in that a friend's first line and its held reply move the revision, so a window that keeps polling reads `friendLineAt`, `pendingCount` and `heldReplies` with no reload. - A face-to-face room's presented Bot participants carry `label` = `remoteBotTitle`, ` ()`, for remote and local seats alike (`room-membership.js`, #384); `faceAliases` still answers to the older ` · 's Bot` / `'s Chief` labels. +- A friend's DM that is waiting on a reply (`remoteAwaiting`, #443) is told when the other home's turn dies (#1068). `turns.js` `remoteFailureNotice` picks one of two fixed sentences — `Could not reply: this Bot has no model set up yet.` for `MISSING_CREDENTIAL`, `NO_KEY`, or a provider line that says the route has no API key, and `Could not reply: this Bot could not answer right now.` for every other `reason.kind === 'error'`. The provider's message is never copied (it may echo part of a key; `AUTH` is the generic sentence even when its text mentions a key). `flybotReplyRemote` carries `failure: true`. A stopped turn sends nothing. A line whose body is already one of those sentences does not earn another, so two homes that cannot run a model do not trade it. The sender's chat shows it as that Bot's reply and drops the weighing row, the same way any reply does (`remote-chat.js` `deliver`). - `remoteTurn` (`src/host/remote-chat.js`) finds what the user typed on a friend's remote chat by skipping host-appended `role:'user'` context: forms `snapshot`, `instructions` (the AGENTS.md baseline on a chat's first turn) and `notice`, and the time-context kinds. Match by form, not by a whitelist of senders, or the first message of a new chat is dropped (#66). - `remoteChats.sync(plugin, list)` (`src/host/remote-chat.js`) records that a chat's Bot left the published list as `bot.remote.gone` and never as a `bot.hidden` of its own (#956). The two are separate: `gone` is the platform's fact, `hidden` is the user's **Hide from sidebar** (`roster.setFlags`) and nothing else writes it. So a share that ends keeps its row in the sidebar (`isShownRemoteChat`, marked *no longer shared*) with its history and its way back, a hide the user set keeps the row out whether the Bot is published or not, and a Bot published again is an ordinary chat again with no leftover marker. It remembers which chats were `gone` before `open()` clears the marker, and takes back a `hidden` that a version before #956 wrote beside its `remote.hiddenByGone` marker on the next sync — and deletes the marker with it, because taking that hide back is its only use and one left behind would make a later **Hide from sidebar** read as ours and be deleted by the next sync — so an existing state file shows the row again without the user doing anything, and the user’s own hide works from then on. Covered by the `#956` cases in `core/test/remote-chat.test.js` and `core/test/new-chat.test.js`. - `notSentLine` (`src/host/remote-chat.js`) is the one short line for a failed send to a friend's Bot. It names only steps a FlyBot owner can take (#404): a paused chat says to approve a waiting message or remove and re-add the friend (or, when the plugin's error says `resend`, to send the message again, which starts a new chat; there is no Extend and no turn limit), and the fallbacks end in *Try again.*; it never points to the Bot Social console, which a FlyBot owner cannot sign in to. A friend's Bot Bot Social no longer knows (`bot_not_found` / "no such remote bot") gets its own sentence — *the Bot is no longer shared with you, so messages to it cannot be delivered. Ask its owner for a new invite.* — which the social plugin also writes and the host reuses whole instead of translating again; the social plugin's "not shared with your Bots" sentence from a revoked share (`has not granted chat.public`, #971) is passed through whole the same way, and a raw `chat.public` refusal that arrives before that mapping still gets that one sentence rather than the server's words. A bare `capability_not_granted`, or the same code naming another capability, is not that fact (#988). No line carries *Try again.* twice (#954). diff --git a/docs/subsystems/plugin-runtime.md b/docs/subsystems/plugin-runtime.md index 928731980..b30b6ea16 100644 --- a/docs/subsystems/plugin-runtime.md +++ b/docs/subsystems/plugin-runtime.md @@ -55,7 +55,7 @@ FlyBot's plugin API surface on top of the patched core: host hooks, client slots A tool whose `visibleTo` says no is taken out of that Bot's list with `agent.ctx.tools.restrict({ deny })` (`hidePluginTools`). DSH's `tools.schemas(agent)` lists only what the Agent sees, so the tools the current mask hides count as known when the mask is recomputed; otherwise every other resolve lifts the mask and the chat shows `Tool added` / `Tool removed` on each turn (#276). The mask is recomputed when the Agent is created, on every `resolveAgent` (delivery), and when a turn starts in the Session (a `session/event` `turn/start`), so a Bot a plugin adopts only after its Agent was created — the tree taking a Main Bot made after the server started — gets its tools on the next turn, with no restart (#594). The names of these gated tools go to the client as the roster's `pluginTools` (`rt.pluginToolNames`), and the client's `ContextCell` hides `Tool added` / `Tool removed` rows for them in every chat, so the first mask leaves no line either (#419). 5. Reach (#90): the core's owner policy (`reach.js`) is not a middleware. `hooks.js` runs it before the `bot.send` chain, on approval release and on every deliver that carries a sender or a remote origin; a plugin's `allow` or `bypass` never skips it, and plugins see its denial as `{ action: 'deny', by: 'owner-policy' }`. Plugins read grants through `flybot.bot.hasGrant` / `hasAnyGrant`, `BotView.granted`, `flybot.grants` and the `bot.grants` event. A group's mention receipt (`mentionReceipt`) turns a not-shared refusal (`cannot message their Bots`, #273) into its short invite-or-add hint. 6. `flybot.roomApprovals.setProvider` lets a plugin (social) attach pending outbound holds to a shared-room view. It does not enable team mode. - An `allow` verdict may say `autoSent: true` (with `recipient`) when it skips a hold only because the owner turned Auto-send on; the host then marks an auto-reply's turn Auto-sent (#417). It confers nothing. A `hold` verdict may carry `recipient`, who gets the message as the owner knows them (a person's name, trimmed, at most 120 characters); the record keeps it as `recipientName` and the cards name it (#418). A Bot's held sends are also visible to Bot coordination: `list_bots` tags the waiting Bot with `N to approve` (a count only, `src/host/tools.js`, #587). A `flybot.approvals` entry with `inline: true` and a `policy` is not an overlay card: it draws the decision on a held reply's own bubble in the chat (`held-reply.js`, props `{ held, decide, call, actions }`), and the overlay skips it. Approval claims (`claimApprovals`, `chatOpened`) live in `approval-claims.js` and are re-exported by `flybot-slots.js`. + An `allow` verdict may say `autoSent: true` (with `recipient`) when it skips a hold only because the owner turned Auto-send on; the host then marks an auto-reply's turn Auto-sent (#417). It confers nothing. A remote turn that died is not that hold (#1068): `replyRemote` sends the host's fixed sentence with `via: 'turn-failed'` and `meta.notice: 'turn-failed'` (`failure: true`), and the social plugin allows only those two sentences. A model reply stays `auto-reply`. A `hold` verdict may carry `recipient`, who gets the message as the owner knows them (a person's name, trimmed, at most 120 characters); the record keeps it as `recipientName` and the cards name it (#418). A Bot's held sends are also visible to Bot coordination: `list_bots` tags the waiting Bot with `N to approve` (a count only, `src/host/tools.js`, #587). A `flybot.approvals` entry with `inline: true` and a `policy` is not an overlay card: it draws the decision on a held reply's own bubble in the chat (`held-reply.js`, props `{ held, decide, call, actions }`), and the overlay skips it. Approval claims (`claimApprovals`, `chatOpened`) live in `approval-claims.js` and are re-exported by `flybot-slots.js`. 7. `cleanRemote` is the record boundary for a friend's granted Bot: `name`, `homeName` and `owner` strip `@` (so they cannot look like a mention of another Bot in `nameMentions` chips/previews) and control characters/newlines, then cap at 60 characters. Handles stay on the `handle` field. Optional look fields (`color`, `avatar` object, `avatarUpdatedAt`) pass through when present so remote chats, group lists, and headers can render a peer's chosen look; missing look stays handle-seeded. Those labels then flow into delivery `fromName` (still `cleanName`'d and the body fenced) and into `list_bots` friends lines via the remote-chat roster. Group room logs use local team names, not peer labels. 8. A user's @ of a friend's Bot in a group is answered in that group (#825): the line reaches the Bot as a room line and its reply comes back as one, which is why `mentionReceipt` (`hooks.js`, relay role `mention`) posts only what went wrong — why the send was denied, held, or failed — and no "sent" line at all. The receipt's role stays outside the room round trigger (`groups.js` `opens`), so it never opens a round; peer names go through `cleanName`. A human line that names a friend's Bot already in that room is **not** 1:1-relayed as the group's admin (Chief): the room message is the delivery, so Chief's missing grant does not block it (#143). A Chief-originated `bot.send` to an ungranted friend Bot is still deny. A remaining grant deny on a mention that is *not* in the room says to invite the Bot or add it to the group. 9. `flybot.rooms` hosts a Bot Social face-to-face matching room as a native room (`upsertFace` / `applyFaceLine` / `ofFace` / `dropFace` / `onFaceLine` (`fn(room, line, { mentions })`: for the owner's typed line, the handles its `@` names, #293) / `postNotice` / `postHuman` (quiet; it reads the friend's line for `@` of a local Bot by label or handle and passes `callIds`, so only those Bots answer, #287; an optional `mentions` handle list from an attested line is resolved first, #293; `attested: true`, passed only for a line the server stamps `typed_by_owner`, lets a plain or `@all` line start this home's Bots in one capped pass, #295; a plain line is answered by the Bots that judge it theirs, `@all` is not routed, #299) / `postRemote`; `postHuman` draws another member's typed line as a chat bubble from that person, #180). Slot `flybot.room.overview` is a block at the top of a group's overview: while joining, a tile grid (avatar above name + owner) with **Leave** and **Enter chat** on one row; once open, compact **Leave** and Auto-send — not Open/Start/Leave together. Slot `flybot.room.members` adds rows at the foot of a local group's Members card (not a face or shared room), with `host(endpoint, payload)` for core endpoints (#876). Slot `flybot.new-chat.action` adds a row to the sidebar **+** short action menu (after New group chat; `label` / `hint` go through the same `t()` map). Still not team mode. `upsertFace` also takes `lead` (`{ handle, picked, canPick, supported }`, #476) and a member `role` label. diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 9b10da360..e6187ef3f 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -55,6 +55,10 @@ Everything in `plugins/social/` (entry `plugins/social/index.js` and any helpers 11. A message right after an invite join is never silently lost (#442; owner decision 2026-10-19, option b; Bot Social #30). Bot Social writes a join's grants before it confirms the join and queues `relationship.changed` (`status: "active"`, with `grants_ready` and `grants`) before the friend's first message. `pollOnce` runs `syncOutgoingGrants({ only })` for that Bot when it reads that event (or `grant.changed` `granted`), so the first message is judged with the grant. Fallback for a server without #30 or a lost event: an inbound message the owner policy refuses (`by: 'owner-policy'`, no grant for that sender) syncs that Bot's grants once and is judged again; still refused, it is not delivered and the plugin calls `report_not_delivered { message_id, reason: 'not_shared_yet' }` (a server without the tool only logs it). On the sender's home, a `message.not_delivered` event for a Bot the owner's own chat goes through writes *"Not sent: Coder (Bob) was not ready to talk with you yet, so it did not get your message. Send it again."* into that chat, where a failed send shows its Not sent line; for a Bot's own send (`message_friend_bot`) it is only logged. Grants stay per Bot; nothing is published, approved or granted by this path. 12. A face room has one room lead (总负责人, #476; Bot Social #32). `viewFaceLead` reads `effective_lead_bot_handle` / `lead_bot_handle` / `created_by_me` from the room snapshot; a server without them gets the first member (by join order: the creator's first Bot) on every home, with no way to pick. `social/face-lead` (the Room lead button) is refused unless this home made the room on a server that stores a lead, then `POST /v1/face-rooms/{id}/lead` (Bot Social: creator only, owner token only, no MCP tool). `room.lead_changed` renders as a platform notice rebuilt from the lead's fields. On a server listing `member_roles`, `publishFaceRoles` sends my Bots' role labels (`PUT /v1/face-rooms/{id}/roles`) when the room shows another value, once per value; a value is recorded as sent only after the room takes it, so a failed call is retried on the next snapshot, and a room that is forgotten clears its per-seat marks; a friend's `members[].role` is cleaned (`cleanFaceRole`, 80 chars, no markup or `@`) and treated as data. +### A remote turn that died (#1068) + +A local Bot's turn that is answering a friend's line and ends in error does not stay silent. The host sends one of two fixed sentences (`Could not reply: this Bot has no model set up yet.` / `Could not reply: this Bot could not answer right now.`, `turns.js` `remoteFailureNotice`) with `via: 'turn-failed'` and `meta.notice: 'turn-failed'`. `isTurnFailureNotice` allows only those exact strings, so the reply hold does not apply and the sender's DM is not left on "Weighing the options…". The same words on `auto-reply`, or any other text on `turn-failed`, are a normal held reply. Bot Social's own blanket hold on that send is cleared the way an Auto-sent line is (`autoSendRemoteLine`, `holdIsBlanket` only); a finding of its own keeps its card. The sender's home shows the sentence in that friend's chat, which ends the weighing row. A notice that comes back as the next line does not earn another. + ### Paused friend chat (#404; no turn limit since 2026-10-19) Owner decision 2026-10-19 18:05: friend chats have no turn limit and never need Extend. Bot Social (#28) dropped the turn budget, and its loop detection checks only Bot-written lines: a line the owner typed (`typed_by_owner`) is never refused for a loop. A send Bot Social still refuses with `conversation_paused` (a Bot repeating itself, an emergency stop, or a server that predates #28) marks that friend's chat paused (in memory, by the friend Bot's handle; `social/paused`). The friend's chat then shows a quiet **Chat paused** line in the session dock (`social-paused`, `PausedChat`): *Bot Social paused the last chat with this Bot (reason). Send your message again: it starts a new chat.* It has no buttons and no turn count. It clears on the next successful send to that Bot. FlyBot never calls `extend_conversation`, and there is no model tool or endpoint for it. The earlier Extend card was removed: re-opening a loop-paused chat let the owner's next identical line trip loop detection again at once, so Extend reported success and the send still failed. diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index 8602bd4b5..7e305182b 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -4128,11 +4128,35 @@ diff --git a/src/host/time-context.js b/src/host/time-context.js diff --git a/src/host/turns.js b/src/host/turns.js --- a/src/host/turns.js +++ b/src/host/turns.js -@@ -5,6 +5,7 @@ +@@ -3,8 +3,31 @@ + // + // Turn tracking: which turns were triggered by another Bot, and what they said. import { REFERENCE_KIND, REPLY_KIND, SOURCE_KIND } from './constants.js' ++import { unwrapUntrusted } from './delivery.js' import { textOf } from './text.js' import { reminderText } from './schedules.js' +import { OTHER, combine, originOfSource, settled } from './turn-origin.js' ++ ++// What a friend's DM is told when the turn that should have answered died (#1068). Two fixed ++// sentences, never the provider's text: a missing-key failure can echo part of a key. The social ++// plugin allows only these, so a lookalike stays a normal held reply. Kept equal by its test. ++export const NO_MODEL_NOTICE = 'Could not reply: this Bot has no model set up yet.' ++export const COULD_NOT_NOTICE = 'Could not reply: this Bot could not answer right now.' ++export const REMOTE_FAILURE_NOTICES = Object.freeze([NO_MODEL_NOTICE, COULD_NOT_NOTICE]) ++// The client's missingModelKey (model-menu.js) reads the same provider line. AUTH is not this: ++// a rejected key is a different failure, and its message is already dropped by turnFailure. ++const MISSING_KEY = /\bno api key\b|\bmissing (?:an? )?api key\b|\bapi key (?:is )?(?:missing|not set|not configured)\b/i ++ ++export function remoteFailureNotice(failure) { ++ const code = typeof failure?.code === 'string' ? failure.code : '' ++ const message = typeof failure?.message === 'string' ? failure.message : '' ++ if (code === 'MISSING_CREDENTIAL' || code === 'NO_KEY' || (code !== 'AUTH' && MISSING_KEY.test(message))) return NO_MODEL_NOTICE ++ return COULD_NOT_NOTICE ++} ++ ++export function isRemoteFailureNotice(text) { ++ return REMOTE_FAILURE_NOTICES.includes(String(text ?? '').trim()) ++} // A failed turn's `LlmFailure`, kept to what can be shown and logged. Like DSH's own // error row, an AUTH failure loses its message: providers may echo part of the key. @@ -4145,7 +4169,17 @@ diff --git a/src/host/turns.js b/src/host/turns.js if (post === undefined || text === '') return const labels = (Array.isArray(source?.references) ? source.references : []) .map(reference => reference?.label).filter(label => typeof label === 'string' && label !== '') -@@ -51,9 +51,10 @@ export function install(rt) { +@@ -48,12 +48,20 @@ const RETRY_TEXT = 'Your last reply wrote a tool call as plain text ( + + export function install(rt) { + const { ctx, state } = rt ++ // A friend's line whose body is already a failure notice. Answering it with another would ++ // bounce the same sentence between two homes that cannot run a model. ++ const failureEchoes = new WeakSet() ++ const noticeBody = (content) => { ++ const raw = textOf(content).replace(/^\[[^\]\n]*\]\n?/, '').trim() ++ return isRemoteFailureNotice(unwrapUntrusted(raw)) || isRemoteFailureNotice(raw) ++ } // The retry goes where the turn's trigger came from: a group round's waiter or a // Bot's request keeps its exchange; anything else is a nudge in the Bot's own chat. @@ -4346,7 +4380,7 @@ diff --git a/src/host/turns.js b/src/host/turns.js // A message that shares a step with a bare handoff package is what the turn // answers, so the turn is not a hidden acknowledgement. const bare = info => info?.handoff === true && info.resume !== true -@@ -331,14 +331,25 @@ export function install(rt) { +@@ -331,15 +331,27 @@ export function install(rt) { hop: source.hop ?? 0, ...(source.late ? { late: true } : {}), ...(source.quiet ? { quiet: true } : {}), @@ -4372,8 +4406,10 @@ diff --git a/src/host/turns.js b/src/host/turns.js - } else if (source?.kind === SOURCE_KIND && source.role === 'kickoff' && rt.roomOf(id)) { - queueRoomMessage(id, { text: '', kickoff: true }) } else if (source?.kind === SOURCE_KIND) { ++ if (fromRemote(source) && noticeBody(event.data?.content)) failureEchoes.add(source) if (entry.turn === undefined) entry.pending = source else if (!opened) claimTrigger(id, entry, source) + else if (expectsReply(source) && !answersUser(source) @@ -363,12 +363,15 @@ export function install(rt) { if (rt.roomOf(id) && source.kind === 'user') { const images = (Array.isArray(event.data.content) ? event.data.content : []) @@ -4438,13 +4474,22 @@ diff --git a/src/host/turns.js b/src/host/turns.js const replyText = () => { const reply = (wroteCall ? texts.map(stripWrittenCalls) : texts).join('\n\n').trim() return wroteCall && reply === '' ? 'Tried to run a tool but wrote the call as text, so it did not run.' : reply -@@ -419,6 +419,15 @@ export function install(rt) { +@@ -419,6 +419,24 @@ export function install(rt) { // Whoever the turn answers gets the same three-way rule: a failure notice, // the turn's text (or a stopped-before-answer line), or nothing. const answer = (source, reply) => { + if (fromRemote(source)) { -+ if (failure === undefined && reply !== '') { -+ void rt.flybotReplyRemote?.({ from: rt.selfOf(id), handle: source.remote.handle, home: source.remote.home, text: reply, replyTo: source.exchangeId, replyIn: { sessionId: id, turn: answeredTurn } }) ++ // A hard failure owes the sender a sentence (#1068). Without it their DM stays on ++ // "Weighing the options…" until the wait expires, and a missing credential never expires ++ // into an explanation. The sentence is fixed; the provider text stays here. ++ const notice = failure !== undefined && !failureEchoes.has(source) ++ if (notice || (failure === undefined && reply !== '')) { ++ void rt.flybotReplyRemote?.({ ++ from: rt.selfOf(id), handle: source.remote.handle, home: source.remote.home, ++ text: notice ? remoteFailureNotice(failure) : reply, ++ replyTo: source.exchangeId, replyIn: { sessionId: id, turn: answeredTurn }, ++ ...(notice ? { failure: true } : {}), ++ }) + // A refused reply resolves, it does not throw: say so, or it is lost without a trace (#272). + .then(result => { if (result?.action === 'deny') rt.warn('ds-bot: reply to %s not sent: %s', source.remote.handle, result.reason) }) + .catch(error => rt.warn('ds-bot: reply to %s failed: %s', source.remote.handle, error)) @@ -5123,7 +5168,11 @@ diff --git a/test/approval-notices.test.js b/test/approval-notices.test.js diff --git a/test/bot-messages.test.js b/test/bot-messages.test.js --- a/test/bot-messages.test.js +++ b/test/bot-messages.test.js -@@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' +@@ -1,10 +1,10 @@ + import assert from 'node:assert/strict' +-import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' ++import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' + import { tmpdir } from 'node:os' import { join } from 'node:path' import { setTimeout as delay } from 'node:timers/promises' import { test } from 'node:test' @@ -5160,6 +5209,116 @@ diff --git a/test/bot-messages.test.js b/test/bot-messages.test.js } installTurns(rt) let seq = 0 +@@ -356,6 +356,109 @@ test('a joined request gets the failure notice when the model call fails', () => + assert.match(delivered[0].text, /^Writer could not answer your message: its model call failed \(SERVER 503: upstream busy\)/) + }) + ++// #1068: a friend's DM stays on "Weighing the options…" when the remote home's turn dies, because ++// that home never sends anything back. The notice is a fixed sentence: the provider text can echo a key. ++const remoteSource = (extra = {}) => replySource('request', { ++ senderSessionId: undefined, ++ remote: { handle: '@coder.bob.home', home: '@bob.home', ...extra.remote }, ++ exchangeId: 'r1', ++ ...extra, ++}) ++const fenced = text => textBlock(`[Message from Coder (@coder.bob.home)]\n\n${text}\n`) ++function withRemote(rt) { ++ const sent = [] ++ rt.flybotReplyRemote = async (args) => { sent.push(args); return { action: 'allow' } } ++ return sent ++} ++ ++test('#1068: a friend line whose turn fails hard is answered with a fixed sentence, and the provider text stays out', () => { ++ const { rt, emit, delivered } = turnsHost() ++ const sent = withRemote(rt) ++ emit('b', 'turn/start', { turn: 4 }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) ++ emit('b', 'turn/end', { turn: 4, reason: { kind: 'error', error: { code: 'MISSING_CREDENTIAL', message: 'no API key for provider route "deepseek-official" sk-secret' } } }) ++ assert.equal(delivered.length, 0, 'a remote failure is not a local Bot-to-Bot delivery') ++ assert.equal(sent.length, 1) ++ assert.equal(sent[0].failure, true) ++ assert.equal(sent[0].from, 'b') ++ assert.equal(sent[0].handle, '@coder.bob.home') ++ assert.equal(sent[0].home, '@bob.home') ++ assert.equal(sent[0].replyTo, 'r1') ++ assert.deepEqual(sent[0].replyIn, { sessionId: 'b', turn: 4 }) ++ assert.equal(sent[0].text, 'Could not reply: this Bot has no model set up yet.') ++ assert.equal(sent[0].text.includes('sk-secret'), false) ++ assert.equal(sent[0].text.includes('deepseek'), false) ++}) ++ ++test('#1068: any other hard failure gets the generic sentence, still without the provider text', () => { ++ const { rt, emit } = turnsHost() ++ const sent = withRemote(rt) ++ emit('b', 'turn/start', { turn: 1 }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) ++ emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'SERVER', message: 'upstream busy sk-secret', status: 503 } } }) ++ assert.equal(sent.length, 1) ++ assert.equal(sent[0].failure, true) ++ assert.equal(sent[0].text, 'Could not reply: this Bot could not answer right now.') ++ assert.equal(sent[0].text.includes('sk-secret'), false) ++ assert.equal(sent[0].text.includes('503'), false) ++ assert.equal(sent[0].text.includes('SERVER'), false) ++}) ++ ++test('#1068: a missing-key message under another code uses the no-model sentence', () => { ++ const { rt, emit } = turnsHost() ++ const sent = withRemote(rt) ++ emit('b', 'turn/start', { turn: 1 }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource() }) ++ emit('b', 'turn/end', { turn: 1, reason: { kind: 'error', error: { code: 'PROVIDER', message: 'llm-deepseek: no API key for provider route "deepseek-official"' } } }) ++ assert.equal(sent[0].text, 'Could not reply: this Bot has no model set up yet.') ++ assert.equal(sent[0].text.includes('llm-deepseek'), false) ++}) ++ ++test('#1068: a finished remote reply is the reply itself; a stopped turn and an echoed notice send nothing', () => { ++ const { rt, emit } = turnsHost() ++ const sent = withRemote(rt) ++ emit('b', 'turn/start', { turn: 1 }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'ok' }) }) ++ emit('b', 'assistant/message', assistantText('pong')) ++ emit('b', 'turn/end', { turn: 1, reason: { kind: 'completed' } }) ++ assert.equal(sent.length, 1) ++ assert.equal(sent[0].failure, undefined) ++ assert.equal(sent[0].text, 'pong') ++ assert.equal(sent[0].replyTo, 'ok') ++ ++ emit('b', 'turn/start', { turn: 2 }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'stop' }) }) ++ emit('b', 'turn/end', { turn: 2, reason: { kind: 'aborted', reason: { kind: 'hook', reason: 'ds-bot/stopped' } } }) ++ assert.equal(sent.length, 1, 'stopping the turn is not a hard failure') ++ ++ const notice = 'Could not reply: this Bot has no model set up yet.' ++ emit('b', 'turn/start', { turn: 3 }) ++ emit('b', 'user/message', { content: fenced(notice), source: remoteSource({ exchangeId: 'echo' }) }) ++ emit('b', 'turn/end', { turn: 3, reason: { kind: 'error', error: { code: 'MISSING_CREDENTIAL', message: 'no API key' } } }) ++ assert.equal(sent.length, 1, 'a notice that came back does not earn another') ++}) ++ ++test('#1068: a remote request that joined a failed turn is told the same way', () => { ++ const { rt, emit, delivered } = turnsHost() ++ const sent = withRemote(rt) ++ emit('b', 'turn/start', { turn: 2 }) ++ emit('b', 'user/message', { content: textBlock('write the letter'), source: { kind: 'user' } }) ++ emit('b', 'user/message', { content: fenced('hi'), source: remoteSource({ exchangeId: 'join' }) }) ++ emit('b', 'turn/end', { turn: 2, reason: { kind: 'error', error: { code: 'AUTH', message: 'sk-secret rejected' } } }) ++ assert.equal(delivered.length, 0) ++ assert.equal(sent.length, 1) ++ assert.equal(sent[0].replyTo, 'join') ++ assert.equal(sent[0].failure, true) ++ assert.equal(sent[0].text, 'Could not reply: this Bot could not answer right now.') ++ assert.equal(sent[0].text.includes('sk-secret'), false) ++}) ++ ++test('#1068: the failure notice is not an auto-reply, so the hold on model replies does not keep it', () => { ++ const src = readFileSync(new URL('../src/host/hooks.js', import.meta.url), 'utf8') ++ assert.match(src, /failure === true \? 'turn-failed' : 'auto-reply'/) ++ assert.match(src, /notice: 'turn-failed'/) ++}) ++ + test('a joined request never claims the next turn', () => { + const { rt, emit, delivered } = turnsHost() + emit('b', 'turn/start', { turn: 1 }) @@ -510,3 +510,64 @@ test('a request the model keeps turning down moves the Bot on, without its lates await delay(30) assert.equal(made, 2) @@ -6049,33 +6208,19 @@ diff --git a/test/follow-through.test.js b/test/follow-through.test.js started.set(home, [...(started.get(home) ?? []), host]) return host } -@@ -69,21 +85,52 @@ - const freshHome = (t) => { - const home = mkdtempSync(join(tmpdir(), 'ds-bot-follow-')) - writeFileSync(join(home, 'state.json'), JSON.stringify(seed())) -- t.after(async () => { -- await Promise.all((started.get(home) ?? []).map(host => host.stop())) -- started.delete(home) -- rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 20 }) -- }) -- return home --} +@@ -72,15 +72,46 @@ const freshHome = (t) => { + t.after(async () => { + await Promise.all((started.get(home) ?? []).map(host => host.stop())) + started.delete(home) ++ durable.delete(home) + rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 20 }) + }) + return home + } -const settle = async (check) => { - const until = Date.now() + 3000 - while (!check() && Date.now() < until) await new Promise(resolve => setImmediate(resolve)) - assert.ok(check(), 'condition not reached') --} --const text = message => message.content.map(block => block.text ?? '').join('\n') --const notes = (home) => { -- try { return JSON.parse(readFileSync(join(home, 'inflight.json'), 'utf8')) } catch { return { turns: {}, queued: {} } } -+ t.after(async () => { -+ await Promise.all((started.get(home) ?? []).map(host => host.stop())) -+ started.delete(home) -+ durable.delete(home) -+ rmSync(home, { recursive: true, force: true, maxRetries: 5, retryDelay: 20 }) -+ }) -+ return home -+} +// The timers a case mocked, if any. A wait must advance that clock: `setImmediate` rounds alone can +// never fire a mocked `setTimeout`, so a wall-clock deadline would decide a race the test controls (#821). +let mockedTimers @@ -6105,18 +6250,15 @@ diff --git a/test/follow-through.test.js b/test/follow-through.test.js + await new Promise(resolve => setImmediate(resolve)) + } + return check() -+} + } +const settle = async (check) => { assert.ok(await waitFor(check), 'condition not reached') } +// Writes to `inflight.json` land on a later tick; after a mock-timer tick and after `stop()` the file +// can still hold the previous write. Wait for them rather than reading early (the note of a failed +// pickup is only proved kept once the clearing write has landed and been replaced). +const flushWrites = async (rounds = 20) => { for (let i = 0; i < rounds; i++) await new Promise(resolve => setImmediate(resolve)) } -+const text = message => message.content.map(block => block.text ?? '').join('\n') -+const notes = (home) => { -+ try { return JSON.parse(readFileSync(join(home, 'inflight.json'), 'utf8')) } catch { return { turns: {}, queued: {} } } - } - const inflight = home => notes(home).turns ?? {} - // A message as DSH would replay it: the turn starts and takes it up. + const text = message => message.content.map(block => block.text ?? '').join('\n') + const notes = (home) => { + try { return JSON.parse(readFileSync(join(home, 'inflight.json'), 'utf8')) } catch { return { turns: {}, queued: {} } } @@ -104,7 +104,7 @@ async function openRoomTurn(t, host) { } @@ -6335,7 +6477,8 @@ diff --git a/test/follow-through.test.js b/test/follow-through.test.js const WRITTEN = 'Checking now.\n\n\n\nGet-ChildItem E:\\\n\n\n' test('a tool call written as text gets one more turn on the same round slot', async (t) => { -@@ -284,5 +284,17 @@ +@@ -283,6 +283,18 @@ test('in its own chat, a Bot that wrote a tool call as text is asked to make it' + assert.match(text(retry), /^\[Your last reply wrote a tool call as text, so nothing ran\]/) }) +test('a real-clock wait outlives its round budget (#1012)', async () => { @@ -6353,7 +6496,6 @@ diff --git a/test/follow-through.test.js b/test/follow-through.test.js test('a written tool call is told apart from text about one', () => { assert.equal(writesToolCall(WRITTEN), true) assert.equal(writesToolCall('\na.md'), true) - diff --git a/test/group-status.test.js b/test/group-status.test.js --- a/test/group-status.test.js +++ b/test/group-status.test.js @@ -6705,7 +6847,7 @@ diff --git a/test/history.test.js b/test/history.test.js diff --git a/test/home-lock.test.js b/test/home-lock.test.js --- a/test/home-lock.test.js +++ b/test/home-lock.test.js -@@ -5,7 +5,7 @@ +@@ -5,7 +5,7 @@ import { syncBuiltinESMExports } from 'node:module' import { hostname, tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' @@ -6714,7 +6856,7 @@ diff --git a/test/home-lock.test.js b/test/home-lock.test.js import { homeLock } from '../src/host/home-lock.js' import { fakeContext } from './fake-ctx.js' -@@ -118,10 +118,14 @@ +@@ -118,10 +118,14 @@ function start(t, home) { route = options return register(options) } @@ -6814,7 +6956,6 @@ diff --git a/test/home-lock.test.js b/test/home-lock.test.js + fb.remote.publish([{ handle: '@a.b.home', name: 'A' }]) + assert.deepEqual(fb.remote.list(), []) +}) - diff --git a/test/images.test.js b/test/images.test.js --- a/test/images.test.js +++ b/test/images.test.js diff --git a/patches/ds-bot/files/src/host/hooks.js b/patches/ds-bot/files/src/host/hooks.js index bb2b6226a..9287890d4 100644 --- a/patches/ds-bot/files/src/host/hooks.js +++ b/patches/ds-bot/files/src/host/hooks.js @@ -1185,9 +1185,17 @@ export function install(rt) { // A local Bot's answer to a friend's Bot (turns.js): back through bot.send, so the // plugin's middleware still decides (auto-accept, or the approval hold). // `replyIn` names the chat and turn whose text this is, so a held reply is drawn on that turn (#418). - const replyRemote = ({ from, handle, home, text, replyTo, replyIn }) => send({ - from, to: { kind: 'remote', handle, ...(home ? { home } : {}) }, text, role: 'reply', via: 'auto-reply', - meta: { replyTo, ...(typeof replyIn?.sessionId === 'string' && (Number.isInteger(replyIn.turn) || (typeof replyIn.turn === 'string' && replyIn.turn !== '')) ? { replyIn: { sessionId: replyIn.sessionId, turn: replyIn.turn } } : {}) }, + // `failure` is the host's own sentence for a turn that died (#1068): via `turn-failed`, not an + // auto-reply, so the model-reply hold does not keep the sender weighing. The plugin allows it + // only when the text is one of the two fixed sentences. + const replyRemote = ({ from, handle, home, text, replyTo, replyIn, failure }) => send({ + from, to: { kind: 'remote', handle, ...(home ? { home } : {}) }, text, role: 'reply', + via: failure === true ? 'turn-failed' : 'auto-reply', + meta: { + replyTo, + ...(failure === true ? { notice: 'turn-failed' } : {}), + ...(typeof replyIn?.sessionId === 'string' && (Number.isInteger(replyIn.turn) || (typeof replyIn.turn === 'string' && replyIn.turn !== '')) ? { replyIn: { sessionId: replyIn.sessionId, turn: replyIn.turn } } : {}), + }, }) rt.flybotGrantsChanged = botId => { const bot = rt.botOf(botId); if (bot !== undefined && bot.remote === undefined) fire('bot.grants', view(bot)) } Object.assign(rt, { flybot, flybotReplyRemote: replyRemote, flybotEndpoint: endpoint, flybotRemoteMembers: remoteMembers.list, flybotSend: send, flybotIsRead: isReadEndpoint, flybotHideTools: hidePluginTools }) diff --git a/plugins/social/src/social.js b/plugins/social/src/social.js index ad03ace06..c61feb9ca 100644 --- a/plugins/social/src/social.js +++ b/plugins/social/src/social.js @@ -131,6 +131,18 @@ export function holdReasons(item) { // DLP category or a risk flag still holds the line (#766); an absent field is not "not blanket", or // every hold on an older server would strand. const BLANKET_HOLD = 'mvp.all_outbound_requires_approval' +// The two sentences turns.js `remoteFailureNotice` sends (#1068). A shipped plugin cannot import +// core, so they are repeated here; the social test asserts the lists stay equal. Only this exact +// text on `via: 'turn-failed'` skips the reply hold. A lookalike is a normal reply. +export const TURN_FAILURE_NOTICES = Object.freeze([ + 'Could not reply: this Bot has no model set up yet.', + 'Could not reply: this Bot could not answer right now.', +]) +export function isTurnFailureNotice(msg) { + if (msg?.via !== 'turn-failed' || msg?.meta?.notice !== 'turn-failed') return false + return TURN_FAILURE_NOTICES.includes(String(msg.text ?? '').trim()) +} + export function holdIsBlanket(item) { const reason = typeof item?.hold_reason === 'string' ? item.hold_reason : '' if (reason) return reason === 'blanket_outbound' @@ -1484,6 +1496,10 @@ export function createSocial(service, config = {}, { client: given, api: givenAp // The friendship may be gone since the roster was cached (#666): deny now, never ask the owner // to approve a line that can no longer leave, and let the row go with it. if (!roomId && !(await friendshipActive(bot.home, msg.from))) return { action: 'deny', reason: `${msg.target.handle} is not a Bot a friend shared with you, so your Bots cannot message it. Its owner can share it with you in that Bot's settings → Sharing.` } + // A hard failure of the turn that answered this friend (#1068). The text is a host sentence, + // not the model's, so it does not wait on the reply card: the sender's DM would otherwise stay + // on "Weighing the options…". The same words on any other via are a normal reply, held below. + if (isTurnFailureNotice(msg)) return { action: 'allow' } // Auto-send on for this pair (#417): it leaves without a card, and the chat marks it Auto-sent. if (approvalFor(msg.from, bot.home) === 'auto') return { action: 'allow', autoSent: true, ...named } return { action: 'hold', reason: `messages to ${bot.home}'s Bots need your approval`, ...named } @@ -1684,6 +1700,9 @@ export function createSocial(service, config = {}, { client: given, api: givenAp // different (the friend's owner decides those), so this branch leaves them alone; a blanket // hold on a room's own envelope is cleared below by autoSendRoomEnvelope (#847). if (msg.approved === true && msg.via !== 'room' && msg.via !== 'room-share') await approveTypedLine(sent) + // A failure notice has no FlyBot card (#1068). Bot Social's blanket hold would park it the + // same way, so it is cleared like an Auto-sent line; a finding of its own keeps its card. + else if (isTurnFailureNotice(msg)) await autoSendRemoteLine(sent) // Auto-send (#417) let this line out without a FlyBot card, so Bot Social's own hold would // park it with nobody left to decide it: the owner's switch already answered the same // question (#737). A hold with a finding of its own keeps its card. Only Auto-send on this diff --git a/plugins/social/test/social.test.js b/plugins/social/test/social.test.js index b9332c05f..948fc63a5 100644 --- a/plugins/social/test/social.test.js +++ b/plugins/social/test/social.test.js @@ -8,12 +8,13 @@ import { test } from 'node:test' import { apply as applyDsBot } from 'ds-bot' import { fakeContext } from './fake-ctx.js' import { apply as applySocial, inject, name } from '../index.js' -import { approvalDraftText, approvalView, createSocial, draftText, holdAllowsFaceAutoSend, holdIsBlanket, homeHandleOf, onboard, onboardHome, nextOnboardDisplayName, mapJoinInviteError, parseInviteInput, inviteLink, registerUnavailable, startSocial, unavailableMessage } from '../src/social.js' +import { approvalDraftText, approvalView, createSocial, draftText, holdAllowsFaceAutoSend, holdIsBlanket, homeHandleOf, isTurnFailureNotice, onboard, onboardHome, nextOnboardDisplayName, mapJoinInviteError, parseInviteInput, inviteLink, registerUnavailable, startSocial, TURN_FAILURE_NOTICES, unavailableMessage } from '../src/social.js' import { createMcpClient, createSocialApi, untrustedText, mapPool, socialHttpAgent, fetchOptions, grantPersonView, memberHandleOf, personHomeOf, normalizeGrant, isUnknownTool, isGrantNotFound, isMissingEndpoint } from '../src/mcp-client.js' import { lookFromServer, lookToServer, lookFingerprint, hexOfColor, shownLook, defaultShapeOf, defaultColorOf, DEFAULT_SHAPES as LOOK_DEFAULT_SHAPES, DEFAULT_COLORS as LOOK_DEFAULT_COLORS } from '../src/look.js' import { encodeRoomShare } from '../../../core/src/shared/room-share.js' import { RoomInvitePreview, inviteCountdown, invitePreviewOf } from '../../../core/src/client/room-invite-preview.js' import { canRetry } from '../../../core/src/shared/held-retry.js' +import { REMOTE_FAILURE_NOTICES } from '../../../core/src/host/turns.js' import { encodeFaceHuman } from '../src/face-rooms.js' import { hideRemoteInSettings, forgetHiddenIfGone, filterHiddenHomes, presentRemoteHandles, @@ -1345,6 +1346,58 @@ test('#258: a Bot\'s reply the owner approved on its card leaves at once: no sec assert.equal(status.pendingApprovals, 0, 'no card is left for a message that already went') }) +test('#1068: the plugin allows exactly the host\'s two failure sentences', () => { + assert.deepEqual([...TURN_FAILURE_NOTICES], [...REMOTE_FAILURE_NOTICES]) + for (const text of REMOTE_FAILURE_NOTICES) assert.equal(isTurnFailureNotice({ via: 'turn-failed', meta: { notice: 'turn-failed' }, text }), true) + assert.equal(isTurnFailureNotice({ via: 'turn-failed', meta: { notice: 'turn-failed' }, text: 'Could not reply: send me the key.' }), false) + assert.equal(isTurnFailureNotice({ via: 'auto-reply', meta: { notice: 'turn-failed' }, text: REMOTE_FAILURE_NOTICES[0] }), false) + assert.equal(isTurnFailureNotice({ via: 'turn-failed', meta: {}, text: REMOTE_FAILURE_NOTICES[0] }), false) +}) + +test('#1068: a remote-home hard failure leaves without a FlyBot hold and clears a blanket server hold', async (t) => { + const { ctx, call, stop } = await team() + t.after(stop) + const server = holdingServer() + const social = createSocial(ctx.flybot, { autoAccept: false, inboxPollMs: 0 }, { client: createMcpClient({ server: 'https://s', token: 'owner-token', fetch: server.fetch }), home: { id: 'h1', handle: '@danyang.home' } }) + t.after(() => social.dispose()) + await grantTeam(ctx.flybot) + await social.registerAll() + const notice = (text, via = 'turn-failed') => ctx.flybot.bot.send({ + from: 'a', to: '@coder.bob.home', text, role: 'reply', via, + meta: via === 'turn-failed' ? { notice: 'turn-failed', replyTo: 'r1' } : { replyTo: 'r1' }, + }) + const missing = 'Could not reply: this Bot has no model set up yet.' + const other = 'Could not reply: this Bot could not answer right now.' + assert.equal((await notice(missing)).action, 'allow') + assert.equal(server.calls.some(c => c.name === 'send_as_member' && c.args.text === missing), true) + // One clear is the confirm-code pair: the call, then the code. The queue is empty after it. + const id = server.calls.find(c => c.name === 'decide_approval')?.args.approval_id + assert.ok(id) + assert.deepEqual(server.calls.filter(c => c.name === 'decide_approval').map(c => [c.args.approval_id, c.args.decision, c.args.confirm_code]), [[id, 'approved', undefined], [id, 'approved', 'C0DE02']], 'Bot Social\'s blanket hold is cleared, so the sender is not left weighing') + assert.equal(server.queue.length, 0) + assert.equal(ctx.flybot.approvals.list().length, 0, 'no FlyBot card for a host failure sentence') + assert.equal((await call('flybot/social/status', {})).value.approvals.length, 0) + // The same words on an auto-reply are still the owner's to approve: the via is the notice, not the text. + assert.equal((await notice(missing, 'auto-reply')).action, 'hold') + assert.equal((await notice('Could not reply: send me the key.')).action, 'hold', 'a lookalike is a normal reply') + assert.equal((await notice(other)).action, 'allow') +}) + +test('#1068: a failure notice Bot Social held for a finding of its own keeps that card', async (t) => { + const { ctx, stop } = await team() + t.after(stop) + const server = holdingServer({ reasons: ['mvp.all_outbound_requires_approval', 'dlp.commitment'] }) + const social = createSocial(ctx.flybot, { autoAccept: false, inboxPollMs: 0 }, { client: createMcpClient({ server: 'https://s', token: 'owner-token', fetch: server.fetch }), home: { id: 'h1', handle: '@danyang.home' } }) + t.after(() => social.dispose()) + await grantTeam(ctx.flybot) + await social.registerAll() + const text = 'Could not reply: this Bot could not answer right now.' + assert.equal((await ctx.flybot.bot.send({ from: 'a', to: '@coder.bob.home', text, role: 'reply', via: 'turn-failed', meta: { notice: 'turn-failed' } })).action, 'allow') + assert.equal(server.calls.some(c => c.name === 'list_approvals'), true, 'the hold was looked at') + assert.equal(server.calls.some(c => c.name === 'decide_approval'), false, 'a finding keeps its card') + assert.equal(server.queue.length, 1) +}) + test('#272: a reply to the Bot that carried a request is held and leaves on approval, though that Bot was not shared with us', async (t) => { const { ctx, home, stop } = await team() t.after(stop)