host: the diagnostics report says another dsh holds the team even when the team file needs updating or cannot be read

changelog.d/2167-diagnostics-readonly-reason.fixed.md

@@ -0,0 +1 @@
+The `Read-only` row of a host diagnostics report no longer says another dsh holds the team when it does not: a team file saved by a newer FlyBot reads `update FlyBot` and an unreadable one reads `could not be read` with its cause, exactly the sentence the sidebar shows, while a window that really lost the team to a live `state.lock` keeps the other-dsh wording ([#2167](https://github.com/ThinkFlowLab/FlyBot/issues/2167)).

core/src/host/diagnostics.js

@@ -233,7 +233,10 @@ export function install(rt) {
       node: process.version,
       team: `${count(bots.length, 'Bot', 'Bots')}, ${rt.state.mainBotIds.length} Main, ${count(groups, 'group chat', 'group chats')}`,
       models: models.length > 0 ? models.join(', ') : undefined,
-      readOnly: readOnly ? 'yes, another dsh holds the team' : undefined,
+      // The store's own sentence (#2153): `stuck` (a team file that could not be read) and a
+      // live state.lock are different faults, and the sidebar shows this same sentence, so the
+      // report must not compress both into "another dsh holds the team".
+      readOnly: readOnly ? clean(cut(readOnly, 200).text) : undefined,
       // The writer is opened before the lock is asked for, so in a window another dsh keeps
       // read-only `save()` returns before `journal.record()` and no line is written at all;
       // claiming `recording` there promises a line the file will not get. A journal that

core/test/diagnostics.test.js

@@ -2,11 +2,12 @@ import assert from 'node:assert/strict'
 import { linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
 import fsp from 'node:fs/promises'
 import { syncBuiltinESMExports } from 'node:module'
-import { homedir, tmpdir } from 'node:os'
+import { homedir, hostname, tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
 import { ISSUES_URL, cleanText, install as installDiagnostics, reportMarkdown } from '../src/host/diagnostics.js'
 import { openJournal, stateText } from '../src/host/journal.js'
+import { install as installStore } from '../src/host/store.js'
 import { failureLine, install as installTurns, turnFailure } from '../src/host/turns.js'
 import { errorCode } from '../src/host/channel.js'
 import { SOURCE_KIND } from '../src/host/constants.js'
@@ -309,7 +310,7 @@ test('a read-only window does not say the state journal is recording (#2153)', a
   const readOnly = () => 'Another dsh (pid 1 on host) holds the team'
   const journal = openJournal({ path: join(home, 'journal', 'state.jsonl'), warn: () => {} })
   const held = await host(home, { journal, readOnly, config: { diagnosticsFile: false } }).diagnostics.report({})
-  assert.match(held.markdown, /^\| Read-only \| yes, another dsh holds the team \|$/m)
+  assert.match(held.markdown, /^\| Read-only \| Another dsh \(pid 1 on host\) holds the team \|$/m)
   assert.doesNotMatch(held.markdown, /state journal: recording/)
   assert.match(held.markdown, /^state journal: paused \(this window is read-only\)$/m)
   assert.equal(held.facts.stateJournal, 'paused (this window is read-only)')
@@ -326,6 +327,70 @@ test('a read-only window does not say the state journal is recording (#2153)', a
   assert.match(stoppedHeld.markdown, /^state journal: stopped \(.*not a plain file.*\)$/m)
 })
 
+// The Read-only row answered one constant for every window that may not write, but the store
+// answers three ways (#2153): `stuck` (the team file cannot be read) is not a live lock at all,
+// and a report that calls a newer or unreadable team file "another dsh holds the team" sends the
+// reader to close a host that is not running. These four build the real store and the real
+// diagnostics on one runtime, so the sentence is the one the host actually produces.
+async function realHost(t, { saved, locked = false } = {}) {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-ro-'))
+  const statePath = join(home, 'state.json')
+  if (saved === 'dir') mkdirSync(statePath)
+  else if (saved !== undefined) writeFileSync(statePath, JSON.stringify(saved))
+  // A lock file naming a live process is exactly what a second dsh leaves behind.
+  if (locked) writeFileSync(join(home, 'state.lock'), JSON.stringify({ pid: 1, host: hostname(), token: 'the-other-dsh' }))
+  const disposers = []
+  const rt = {
+    ctx: {
+      logger: { warn: () => {} },
+      effect(execute) {
+        const dispose = execute()
+        if (typeof dispose === 'function') disposers.push(dispose)
+      },
+    },
+    home,
+    statePath,
+    config: { diagnosticsFile: false },
+    autoRoomName: () => 'Room',
+    retitleRoom: async () => {},
+    warn: () => {},
+  }
+  installStore(rt)
+  await rt.load()
+  installDiagnostics(rt)
+  t.after(async () => {
+    await Promise.all(disposers.splice(0).map(dispose => dispose()))
+    rmSync(home, { recursive: true, force: true, maxRetries: 5 })
+  })
+  return { home, readOnly: rt.readOnly(), report: await rt.diagnostics.report({}) }
+}
+
+test('the Read-only row says a newer team file needs an update, not that another dsh holds the team (#2167)', async (t) => {
+  const { home, readOnly, report } = await realHost(t, { saved: { version: 99 } })
+  assert.match(readOnly, /update FlyBot/)
+  assert.ok(report.markdown.includes(`| Read-only | The Bot team file ${join(home, 'state.json')} could not be read (saved by a newer FlyBot; update FlyBot). FlyBot does not change it; fix or move the file and reload. |`), report.markdown)
+  assert.doesNotMatch(report.markdown, /another dsh holds the team/)
+})
+
+test('the Read-only row says an unreadable team file could not be read, not that another dsh holds the team (#2167)', async (t) => {
+  const { report } = await realHost(t, { saved: 'dir' })
+  assert.match(report.markdown, /^\| Read-only \| The Bot team file .* could not be read \(EISDIR\)\. FlyBot does not change it; fix or move the file and reload\. \|$/m)
+  assert.doesNotMatch(report.markdown, /another dsh holds the team/)
+})
+
+test('the Read-only row keeps the other-dsh wording when a live lock names it (#2167)', async (t) => {
+  const { report } = await realHost(t, { locked: true })
+  assert.ok(report.markdown.includes('open in another dsh (Web or Desktop, pid 1 on '), report.markdown)
+  assert.match(report.markdown, /This window shows the team but cannot change it; close the other one and reload\./)
+  assert.doesNotMatch(report.markdown, /could not be read/)
+})
+
+test('a writable window has no Read-only row', async (t) => {
+  const { report } = await realHost(t)
+  assert.equal(report.facts.readOnly, undefined)
+  assert.doesNotMatch(report.markdown, /^\| Read-only \|/m)
+})
+
 test('reportMarkdown says when nothing went wrong', () => {
   const text = reportMarkdown({ facts: { botVersion: '0.1.0', os: 'linux' } })
   assert.match(text, /No host errors recorded\./)

docs/subsystems/core-host.md

@@ -405,6 +405,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi
 ### Diagnostics
 
 - Host diagnostics never depend on `ctx.logger`: DSH may export no logger, and a test or plugin-local host may carry one without `warn`. `rt.warn` still writes stderr and the diagnostics log in that case (`src/host/diagnostics.js`). Plugins reach the same log through `flybot.warn` (#68); see [plugin-runtime](plugin-runtime.md).
+- The `Read-only` row of a report repeats this host's own answer rather than one constant: `rt.readOnly()` is `undefined` while the window writes, a sentence about a live `state.lock` when another dsh holds the team, and a sentence about the team file when it is `stuck` (a newer version, a file that will not parse, an unreadable path). The report shows that sentence — `clean(cut(answer, 200))`, so saved keys, the home folder and `|` are handled like any other report text — the same one the sidebar shows, so a newer team file reads `update FlyBot` and an unreadable one reads `could not be read`, never `another dsh holds the team`; a writable window still adds no row ([#2167](https://github.com/ThinkFlowLab/FlyBot/issues/2167)).
 - The log itself is one leaf file under the home, and a write asks it before it appends ([#2089](https://github.com/ThinkFlowLab/FlyBot/issues/2089)): when `logs/diagnostics.jsonl` is a link, a hard link or anything other than a plain file of its own — the line is dropped with one line on stderr that carries no home folder, the entries stay readable in memory, and `clear` leaves a file behind such a link untouched rather than truncating it.
 
 ### Untrusted fences in every shared room

patches/ds-bot/0001-core-fixes.patch

@@ -4884,12 +4884,15 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js
      return {
        botVersion: ownVersion(),
        dshVersion: dshVersion(),
-@@ -199,12 +199,19 @@ export function install(rt) {
+@@ -199,12 +199,22 @@ export function install(rt) {
        node: process.version,
        team: `${count(bots.length, 'Bot', 'Bots')}, ${rt.state.mainBotIds.length} Main, ${count(groups, 'group chat', 'group chats')}`,
        models: models.length > 0 ? models.join(', ') : undefined,
 -      readOnly: rt.readOnly?.() ? 'yes, another dsh holds the team' : undefined,
-+      readOnly: readOnly ? 'yes, another dsh holds the team' : undefined,
++      // The store's own sentence (#2153): `stuck` (a team file that could not be read) and a
++      // live state.lock are different faults, and the sidebar shows this same sentence, so the
++      // report must not compress both into "another dsh holds the team".
++      readOnly: readOnly ? clean(cut(readOnly, 200).text) : undefined,
 +      // The writer is opened before the lock is asked for, so in a window another dsh keeps
 +      // read-only `save()` returns before `journal.record()` and no line is written at all;
 +      // claiming `recording` there promises a line the file will not get. A journal that

patches/ds-bot/0007-version-and-tooling.patch

@@ -795,18 +795,20 @@ diff --git a/scripts/lint.mjs b/scripts/lint.mjs
 diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js
 --- a/test/diagnostics.test.js
 +++ b/test/diagnostics.test.js
-@@ -1,11 +1,12 @@
+@@ -1,11 +1,13 @@
  import assert from 'node:assert/strict'
 -import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
 +import { linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
  import fsp from 'node:fs/promises'
  import { syncBuiltinESMExports } from 'node:module'
- import { homedir, tmpdir } from 'node:os'
+-import { homedir, tmpdir } from 'node:os'
++import { homedir, hostname, tmpdir } from 'node:os'
  import { join } from 'node:path'
  import { test } from 'node:test'
 -import { cleanText, install as installDiagnostics, reportMarkdown } from '../src/host/diagnostics.js'
 +import { ISSUES_URL, cleanText, install as installDiagnostics, reportMarkdown } from '../src/host/diagnostics.js'
 +import { openJournal, stateText } from '../src/host/journal.js'
++import { install as installStore } from '../src/host/store.js'
  import { failureLine, install as installTurns, turnFailure } from '../src/host/turns.js'
  import { errorCode } from '../src/host/channel.js'
  import { SOURCE_KIND } from '../src/host/constants.js'
@@ -1003,7 +1005,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js
  test('diagnostics append fallback redacts an error message', async (t) => {
    const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-'))
    t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
-@@ -133,6 +133,58 @@ test('diagnostics append fallback redacts an error message', async (t) => {
+@@ -133,6 +133,122 @@ test('diagnostics append fallback redacts an error message', async (t) => {
    assert.match(stderr.join('\n'), /api_key=\[hidden\]/)
  })
 
@@ -1042,7 +1044,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js
 +  const readOnly = () => 'Another dsh (pid 1 on host) holds the team'
 +  const journal = openJournal({ path: join(home, 'journal', 'state.jsonl'), warn: () => {} })
 +  const held = await host(home, { journal, readOnly, config: { diagnosticsFile: false } }).diagnostics.report({})
-+  assert.match(held.markdown, /^\| Read-only \| yes, another dsh holds the team \|$/m)
++  assert.match(held.markdown, /^\| Read-only \| Another dsh \(pid 1 on host\) holds the team \|$/m)
 +  assert.doesNotMatch(held.markdown, /state journal: recording/)
 +  assert.match(held.markdown, /^state journal: paused \(this window is read-only\)$/m)
 +  assert.equal(held.facts.stateJournal, 'paused (this window is read-only)')
@@ -1058,6 +1060,70 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js
 +  const stoppedHeld = await host(home, { journal: stoppedJournal, readOnly, config: { diagnosticsFile: false } }).diagnostics.report({})
 +  assert.match(stoppedHeld.markdown, /^state journal: stopped \(.*not a plain file.*\)$/m)
 +})
++
++// The Read-only row answered one constant for every window that may not write, but the store
++// answers three ways (#2153): `stuck` (the team file cannot be read) is not a live lock at all,
++// and a report that calls a newer or unreadable team file "another dsh holds the team" sends the
++// reader to close a host that is not running. These four build the real store and the real
++// diagnostics on one runtime, so the sentence is the one the host actually produces.
++async function realHost(t, { saved, locked = false } = {}) {
++  const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-ro-'))
++  const statePath = join(home, 'state.json')
++  if (saved === 'dir') mkdirSync(statePath)
++  else if (saved !== undefined) writeFileSync(statePath, JSON.stringify(saved))
++  // A lock file naming a live process is exactly what a second dsh leaves behind.
++  if (locked) writeFileSync(join(home, 'state.lock'), JSON.stringify({ pid: 1, host: hostname(), token: 'the-other-dsh' }))
++  const disposers = []
++  const rt = {
++    ctx: {
++      logger: { warn: () => {} },
++      effect(execute) {
++        const dispose = execute()
++        if (typeof dispose === 'function') disposers.push(dispose)
++      },
++    },
++    home,
++    statePath,
++    config: { diagnosticsFile: false },
++    autoRoomName: () => 'Room',
++    retitleRoom: async () => {},
++    warn: () => {},
++  }
++  installStore(rt)
++  await rt.load()
++  installDiagnostics(rt)
++  t.after(async () => {
++    await Promise.all(disposers.splice(0).map(dispose => dispose()))
++    rmSync(home, { recursive: true, force: true, maxRetries: 5 })
++  })
++  return { home, readOnly: rt.readOnly(), report: await rt.diagnostics.report({}) }
++}
++
++test('the Read-only row says a newer team file needs an update, not that another dsh holds the team (#2167)', async (t) => {
++  const { home, readOnly, report } = await realHost(t, { saved: { version: 99 } })
++  assert.match(readOnly, /update FlyBot/)
++  assert.ok(report.markdown.includes(`| Read-only | The Bot team file ${join(home, 'state.json')} could not be read (saved by a newer FlyBot; update FlyBot). FlyBot does not change it; fix or move the file and reload. |`), report.markdown)
++  assert.doesNotMatch(report.markdown, /another dsh holds the team/)
++})
++
++test('the Read-only row says an unreadable team file could not be read, not that another dsh holds the team (#2167)', async (t) => {
++  const { report } = await realHost(t, { saved: 'dir' })
++  assert.match(report.markdown, /^\| Read-only \| The Bot team file .* could not be read \(EISDIR\)\. FlyBot does not change it; fix or move the file and reload\. \|$/m)
++  assert.doesNotMatch(report.markdown, /another dsh holds the team/)
++})
++
++test('the Read-only row keeps the other-dsh wording when a live lock names it (#2167)', async (t) => {
++  const { report } = await realHost(t, { locked: true })
++  assert.ok(report.markdown.includes('open in another dsh (Web or Desktop, pid 1 on '), report.markdown)
++  assert.match(report.markdown, /This window shows the team but cannot change it; close the other one and reload\./)
++  assert.doesNotMatch(report.markdown, /could not be read/)
++})
++
++test('a writable window has no Read-only row', async (t) => {
++  const { report } = await realHost(t)
++  assert.equal(report.facts.readOnly, undefined)
++  assert.doesNotMatch(report.markdown, /^\| Read-only \|/m)
++})
 +
  test('reportMarkdown says when nothing went wrong', () => {
    const text = reportMarkdown({ facts: { botVersion: '0.1.0', os: 'linux' } })