From 1010896b00b9dc7cfa65109cf0108e0f3a0ab395 Mon Sep 17 00:00:00 2001
From: Day Dreamer <87640074+HugoXK@users.noreply.github.com>
Date: Mon, 2 Nov 2026 21:25:28 +0800
Subject: [PATCH] diagnostics: repeat the host's own read-only sentence in the
 report (#2167)

The Read-only row answered one constant for every window that may not write:
"another dsh holds the team". The store answers three ways - writable, a live
state.lock, and stuck (a team file that could not be read) - so a newer team
file or a directory at state.json was reported as a second dsh holding the
lock, and the repair sentence the sidebar shows was dropped. facts() now shows
the store's own sentence, cut to 200 and cleaned like the rest of the report.

Fixes #2167
---
 .../2167-diagnostics-readonly-reason.fixed.md |  1 +
 core/src/host/diagnostics.js                  |  5 +-
 core/test/diagnostics.test.js                 | 69 ++++++++++++++++-
 docs/subsystems/core-host.md                  |  1 +
 patches/ds-bot/0001-core-fixes.patch          |  7 +-
 patches/ds-bot/0007-version-and-tooling.patch | 74 ++++++++++++++++++-
 6 files changed, 148 insertions(+), 9 deletions(-)
 create mode 100644 changelog.d/2167-diagnostics-readonly-reason.fixed.md

diff --git a/changelog.d/2167-diagnostics-readonly-reason.fixed.md b/changelog.d/2167-diagnostics-readonly-reason.fixed.md
new file mode 100644
index 000000000..f202f453d
--- /dev/null
+++ b/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)).
diff --git a/core/src/host/diagnostics.js b/core/src/host/diagnostics.js
index 013cc9b17..19558217b 100644
--- a/core/src/host/diagnostics.js
+++ b/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
diff --git a/core/test/diagnostics.test.js b/core/test/diagnostics.test.js
index 9561654d1..e2d0d0021 100644
--- a/core/test/diagnostics.test.js
+++ b/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\./)
diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md
index 7c2ca6630..3e9aab6df 100644
--- a/docs/subsystems/core-host.md
+++ b/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
diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch
index 3636360c1..26b1b4d7d 100644
--- a/patches/ds-bot/0001-core-fixes.patch
+++ b/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
diff --git a/patches/ds-bot/0007-version-and-tooling.patch b/patches/ds-bot/0007-version-and-tooling.patch
index 924848f2a..ccf18aebb 100644
--- a/patches/ds-bot/0007-version-and-tooling.patch
+++ b/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' } })