fix(turn-origin): fail closed on a source kind it does not recognise (#620)

changelog.d/620-turn-origin-fail-closed.fixed.md

@@ -0,0 +1 @@
+`turn-origin` now treats a message source kind it does not recognise as `other` instead of "nobody's request", so an unknown or future source arriving in a Bot's own chat no longer inherits the owner's standing approval: only DSH's own notice kinds and the documented mention/handoff cases carry no verdict ([#620](https://github.com/ThinkFlowLab/FlyBot/issues/620)).

core/src/host/remote-chat.js

@@ -10,6 +10,7 @@
 // Remote chat Bots are not team members: findBot, the team tools, flybot.bot.list() and
 // memory skip them, and the tree never adopts them.
 import { randomUUID } from 'node:crypto'
+import { CLOCK_KINDS } from './time-context.js'
 import { createUserMessage, LlmAdapter } from '@deepseek-ai/dsh-llm'
 import { SOURCE_KIND } from './constants.js'
 import { cleanName, fenceUntrusted } from './delivery.js'
@@ -73,7 +74,7 @@ export function notSentLine(error) {
 // time context), the AGENTS.md baseline (form 'instructions') and notices (plan mode, tool
 // reminders). Matched by form, not by a whitelist of senders, so a new notice kind is skipped too.
 const CONTEXT_FORMS = new Set(['snapshot', 'instructions', 'notice'])
-const isContextSnapshot = source => CONTEXT_FORMS.has(source?.form) || source?.kind === 'agent-instructions' || source?.kind === 'ds-bot-time-context' || source?.kind === 'time-context'
+const isContextSnapshot = source => CONTEXT_FORMS.has(source?.form) || source?.kind === 'agent-instructions' || CLOCK_KINDS.includes(source?.kind)
 
 export function remoteTurn(messages) {
   // Context the host appends as user messages (the time context, refreshed every few minutes;

core/src/host/time-context.js

@@ -1,7 +1,9 @@
 import { createUserMessage } from '@deepseek-ai/dsh-llm'
 
-const SOURCE = 'ds-bot-time-context'
-const isClock = source => source?.kind === SOURCE || source?.kind === 'time-context'
+export const SOURCE = 'ds-bot-time-context'
+// The host injects `SOURCE`; DSH injects 'time-context'. One home for both (#626).
+export const CLOCK_KINDS = [SOURCE, 'time-context']
+const isClock = source => CLOCK_KINDS.includes(source?.kind)
 
 export function install(rt) {
   const { ctx, config } = rt

core/src/host/turn-origin.js

@@ -11,18 +11,58 @@
 //   'other'      everything else that carries someone's words: a friend's message, a line in a room
 //                with other people in it, a Bot of another owner, a relay nobody vouched for;
 //   (nothing)    a message that carries no one's request (DSH's own notices, a handoff note, a
-//                receipt): it neither grants nor takes away.
+//                receipt): it neither grants nor takes away. A source kind this version does not
+//                recognise is 'other', not nothing (#620).
 //
 // A turn's verdict is the strictest of its messages ('other' beats everything and sticks for the
 // rest of the turn), and a turn that took in no message with a verdict has none. The caller treats
 // "none" as 'other' (fail closed).
-import { REPLY_KIND, SOURCE_KIND } from './constants.js'
+import { REFERENCE_KIND, REPLY_KIND, SOURCE_KIND } from './constants.js'
+import { CLOCK_KINDS } from './time-context.js'
 
 export const OWNER = 'owner'
 export const OTHER = 'other'
 export const SCHEDULED = 'scheduled'
 
-// The verdict of one message `source` (DSH's `user/message` event) to a Bot.
+// The messages that carry nobody's request, by source kind — one list, because everything else
+// fails closed as 'other' (#620): a kind missing here takes the connector tools away in the middle
+// of a turn the owner started (#626), and a kind wrongly added here would hand them out.
+// Derived from `MessageSourceMap` in @deepseek-ai/dsh-llm (0.2.0-rc.2, the version
+// core/dev/harness/package.json pins). A kind is not recognisable by its `form` alone: the kinds
+// below that declare no `form` at all (`working-directory`, `ptc-mode`, `cordis-host-runner`) would
+// read as opaque content, and `webhook` declares `form: 'notice'` while being an outside delivery
+// (see `originOfSource`). Bumping the DSH pin means re-reading that map: the test
+// `#626: the no-request kind list is derived from the DSH version the tests run against` fails
+// until this list has been looked at again.
+const NOTICE_KINDS = new Set([
+  // Sources the host writes for itself, never a user turn.
+  'tool',
+  'system-prompt',
+  'plan-mode', // @deepseek-ai/dsh-plan-mode, ContextFormed
+  'user-approval', // @deepseek-ai/dsh-user-approval, ContextFormed
+  // Context appended as role 'user' while a turn runs: the clock this host injects
+  // (`ds-bot-time-context`, time-context.js `SOURCE`) and DSH injects (`time-context`); the
+  // AGENTS.md baseline and changes (`agent-instructions`, @deepseek-ai/dsh-agent-instructions); a
+  // cited conversation's snapshot (`session-reference`, constants.js `REFERENCE_KIND`); the model
+  // and working directory a Session switched to (@deepseek-ai/dsh-agent `model-selection`,
+  // `working-directory`); the PTC toggle (`ptc-mode`); the plugin host runner
+  // (`cordis-host-runner`).
+  ...CLOCK_KINDS,
+  REFERENCE_KIND,
+  'agent-instructions',
+  'model-selection',
+  'working-directory',
+  'ptc-mode',
+  'cordis-host-runner',
+  // One-off accounts of what just happened, `form: 'notice'`: a subagent settling, which steers
+  // the turn that is running (@deepseek-ai/dsh-subagent); a compaction checkpoint, the summary
+  // that replaces the log (@deepseek-ai/dsh-compaction); and a skill's own instructions, injected
+  // with `form: 'instructions'` when it is invoked (@deepseek-ai/dsh-skill).
+  'subagent-settled',
+  'compact-checkpoint',
+  'skill-invocation',
+])
+
 export function originOfSource(source, bot) {
   if (source === undefined || source === null || typeof source !== 'object') return OTHER
   switch (source.kind) {
@@ -34,14 +74,26 @@ export function originOfSource(source, bot) {
       return OWNER
     case 'schedule':
       return SCHEDULED
+    case 'goal':
+      // A goal round continues a goal that was created in this Session: by the owner, or by the Bot
+      // on a request that reached it. It is not a request arriving now, so it reads like a due task
+      // — the owner's while the Bot is not shared (`settled` turns it into 'other' when it is).
+      return SCHEDULED
     case SOURCE_KIND:
       if (source.remote !== undefined || source.form === 'remote-chat') return OTHER
       // A host notice about what the user did, and the note a Bot's own part switch hands on.
       if (source.role === 'mention') return undefined
       if (source.handoff === true && source.resume !== true) return undefined
       return source.origin === OWNER || source.origin === SCHEDULED ? source.origin : OTHER
     default:
-      return undefined
+      if (NOTICE_KINDS.has(source.kind)) return undefined
+      // A kind this version does not recognise is not "nobody's request": the host cannot vouch for
+      // who is behind it, so it fails closed as 'other' (#620). Only the two documented no-request
+      // cases above may return undefined. Three DSH kinds stay here on purpose: `webhook` carries
+      // an outside delivery (`form: 'notice'`, but a provider's event is somebody's request, so
+      // reading `form` alone would let it through); `agent-message` (`form: 'relay'`) and
+      // `team-message` carry another agent's words.
+      return OTHER
   }
 }
 

core/test/group-status.test.js

@@ -363,7 +363,7 @@ test('#608: an own-chat turn that carries no verdict still honours the Bot grant
   assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'allowed-once')
   host.emit('b', 'turn/end', { reason: { kind: 'completed' } })
   host.emit('b', 'turn/start', { turn: 3 })
-  host.emit('b', 'user/message', { content: [{ type: 'text', text: 'note' }], source: { kind: 'ds-bot', role: 'mention' } })
+  host.emit('b', 'user/message', { content: [{ type: 'text', text: 'note' }], source: { kind: 'bot', form: 'relay', role: 'mention' } })
   assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'allowed-once')
   // Someone else's turn in that chat still asks, and says once why the grant was not used.
   host.emit('b', 'turn/end', { reason: { kind: 'completed' } })
@@ -372,6 +372,18 @@ test('#608: an own-chat turn that carries no verdict still honours the Bot grant
   assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'from-dsh')
 })
 
+// #620: the own-chat widening the own-chat widening in standingCovers is for a turn nobody's request is in, not for a
+// source kind the host cannot read. An unrecognised kind fails closed, so the grant does not apply.
+test('#620: an unrecognised source kind in the Bot’s own chat still asks', async (t) => {
+  const host = await start()
+  t.after(host.stop)
+  await openTurn(t, host)
+  await grantFromGroup(host, 'always-bot')
+  host.emit('b', 'turn/start', { turn: 2 })
+  host.emit('b', 'user/message', { content: [{ type: 'text', text: 'from a future source' }], source: { kind: 'some-future-kind' } })
+  assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'from-dsh')
+})
+
 test('#608: a grant survives a restart and answers the same ask again', async (t) => {
   const first = await start()
   t.after(() => rmSync(first.home, { recursive: true, force: true, maxRetries: 5 }))
@@ -434,3 +446,34 @@ test('a running Bot\'s work names its current step, in one short line, until the
   host.emit('gb', 'turn/end', { reason: { kind: 'error', error: new Error('boom') } })
   assert.equal((await bots()).b, undefined)
 })
+
+// #626: the shape a Bot actually hits — the owner asks for work, the turn calls `subagent`, and the
+// child settles while that same turn is still running. The notice is an account of work the owner
+// already asked for, not a new request, so the connector tools stay.
+test('#626: a subagent settling mid-turn does not take the connection away', async (t) => {
+  const host = await start()
+  t.after(host.stop)
+  await openTurn(t, host)
+  await grantFromGroup(host, 'always-bot')
+  host.emit('b', 'turn/start', { turn: 2 })
+  host.emit('b', 'user/message', { content: [{ type: 'text', text: 'read the repo and report' }], source: { kind: 'user' } })
+  host.emit('b', 'tool/call', { callId: 's1', name: 'subagent', arguments: JSON.stringify({ prompt: 'summarise the README' }) })
+  host.emit('b', 'user/message', { content: [{ type: 'text', text: 'Subagent finished: summarised the README.' }], source: { kind: 'subagent-settled', form: 'notice', summary: 'summarised the README', senderSessionId: 'child-1' } })
+  assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'allowed-once')
+})
+
+// #626: the host and DSH inject context into ordinary owner turns — this host's clock snapshot
+// (`ds-bot-time-context`), DSH's own clock and cited-conversation snapshots, and the AGENTS.md
+// baseline. None of them carries a request, so a turn that took one in still honours the owner's
+// standing grant; treating one as `other` is what made the #608 card come back.
+for (const kind of ['ds-bot-time-context', 'time-context', 'session-reference', 'agent-instructions']) {
+  test(`#626: a turn that took in a ${kind} message still honours the Bot grant`, async (t) => {
+    const host = await start()
+    t.after(host.stop)
+    await openTurn(t, host)
+    await grantFromGroup(host, 'always-bot')
+    host.emit('b', 'turn/start', { turn: 2 })
+    host.emit('b', 'user/message', { content: [{ type: 'text', text: 'context' }], source: { kind, form: 'snapshot' } })
+    assert.equal(await host.askApproval('b', { toolName: 'bash', reason: ESCALATE }, () => 'from-dsh'), 'allowed-once')
+  })
+}

core/test/turn-origin.test.js

@@ -1,4 +1,5 @@
 import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
 import { test } from 'node:test'
 import { OTHER, OWNER, SCHEDULED, combine, originOfSource, settled } from '../src/host/turn-origin.js'
 import { othersInRoom, sharedWith } from '../src/host/reach.js'
@@ -29,6 +30,40 @@ test('each kind of message has one verdict, and a stamp the host did not write i
   for (const source of [{ kind: 'tool' }, { kind: 'system-prompt' }, { kind: 'plan-mode' }, { kind: 'user-approval' }, relay({ role: 'mention' }), relay({ role: 'handoff', handoff: true })]) {
     assert.equal(originOfSource(source, bot), undefined, JSON.stringify(source))
   }
+  // #626: the contexts the host and DSH inject into ordinary owner turns carry nobody's request
+  // either, so they must not flip a turn to `other` (that dropped the standing grant of #608).
+  for (const kind of [
+    'ds-bot-time-context',
+    'time-context',
+    'session-reference',
+    'agent-instructions',
+    'model-selection',
+    'working-directory',
+    'ptc-mode',
+    'cordis-host-runner',
+    'subagent-settled',
+    'compact-checkpoint',
+    'skill-invocation',
+  ]) {
+    assert.equal(originOfSource({ kind }, bot), undefined, kind)
+    assert.equal(originOfSource({ kind, form: 'snapshot' }, bot), undefined, `${kind} snapshot`)
+  }
+  // #626: `form` alone cannot decide this. A webhook declares `form: 'notice'` but is an outside
+  // delivery, and an agent's relay or a team message carries somebody's words.
+  for (const source of [
+    { kind: 'webhook', form: 'notice', summary: 'push to main' },
+    { kind: 'agent-message', form: 'relay' },
+    { kind: 'team-message' },
+  ]) {
+    assert.equal(originOfSource(source, bot), OTHER, JSON.stringify(source))
+  }
+  // #626: a goal round continues a goal this Session already has, like a due task.
+  assert.equal(originOfSource({ kind: 'goal', goalId: 'g1', revision: 1, round: 2 }, bot), SCHEDULED)
+  // #620: a source kind this version does not recognise is not a no-request message: the host
+  // cannot vouch for who is behind it, so it fails closed.
+  assert.equal(originOfSource({ kind: 'some-future-kind' }, bot), OTHER, 'an unrecognised kind is other')
+  assert.equal(originOfSource({ kind: 'some-future-kind', authorId: 'me' }, bot), OTHER, 'even one that claims an author')
+  assert.equal(originOfSource({ kind: '' }, bot), OTHER)
   // A handoff that goes on with a stopped turn is that turn's request, so it carries the turn's stamp.
   assert.equal(originOfSource(relay({ role: 'handoff', handoff: true, resume: true, origin: 'owner' }), bot), OWNER)
   assert.equal(originOfSource(relay({ role: 'handoff', handoff: true, resume: true }), bot), OTHER)
@@ -93,3 +128,12 @@ test('settled: the verdict that holds now, from the team as it is now', () => {
   assert.equal(settled(rt, 'room-session'), OTHER)
   assert.equal(settled({ ...rt, roomOf: () => undefined }, 'room-session'), OTHER, 'a room that cannot be found')
 })
+
+// #626: the no-request list is derived from DSH's `MessageSourceMap`. A new DSH version can add a
+// kind, and a kind nobody listed silently takes the connector tools away mid-turn — so a bump fails
+// here, where the fix is to re-read the map, instead of showing up months later as a lost tool.
+test('#626: the no-request kind list is derived from the DSH version the tests run against', () => {
+  const harness = new URL('../dev/harness/package.json', import.meta.url)
+  const pinned = JSON.parse(readFileSync(harness, 'utf8')).dependencies['@deepseek-ai/dsh']
+  assert.equal(pinned, '0.2.0-rc.2', 'DSH moved: re-read MessageSourceMap and update NOTICE_KINDS in src/host/turn-origin.js')
+})