host: the diagnostics report names the team file it counts

changelog.d/2394-state-file.fixed.md

@@ -0,0 +1 @@
+The diagnostics report behind Settings → Feedback's **Report on GitHub** now names the team file itself, right after the table that counts the team: `state file: ~/.dsh/state.json (<size> bytes, last written <ISO UTC>)` for a plain file, `state file: none yet (this computer has not saved the team)` before this computer has one, and `state file: not a plain file (nothing is read or written)` when the path is a directory, a link or a file with a second name. The line is built from `lstat` alone — `state.json` holds the members, the messages and the saved keys, so the report may describe the file and never opens or reads it — and a read-only window draws the same line ([#2394](https://github.com/ThinkFlowLab/FlyBot/issues/2394)).

core/src/host/diagnostics.js

@@ -81,6 +81,9 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries
     row('Read-only', facts.readOnly),
   ].filter(Boolean)
   const recent = entries.slice(-REPORT_ENTRIES)
+  // The team file, right after the table that counts the team (#2394): a report about a team has to
+  // say what that team is saved in, in the shape the two file lines below already have.
+  const stateFile = facts.stateFile === undefined ? [] : ['', `state file: ${facts.stateFile}`]
   // The journal is only mentioned while the host runs one: an off journal is not a fact about
   // this host, and a stopped one is exactly what a report about a bug it was meant to catch misses.
   // Its file follows in the same shape (#2388): the sentence says whether the host records, and
@@ -90,7 +93,7 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries
   // The log's own line, in the journal's shape (#2134): a host whose log is off says nothing, and
   // every other bundle gets the one fact the recent errors cannot show — whether they were written.
   const log = facts.diagnosticsLog === undefined ? [] : ['', `diagnostics log: ${facts.diagnosticsLog}`]
-  const out = [...lines, ...journal, ...log, '']
+  const out = [...lines, ...stateFile, ...journal, ...log, '']
   out.push(recent.length === 0 ? 'No host errors recorded.' : `Recent host errors (${recent.length} of ${entries.length}, oldest first):`)
   if (recent.length > 0) out.push('```', ...recent.map(entryLine), '```')
   const stacks = recent.filter(entry => entry.stack).slice(-3)
@@ -124,7 +127,7 @@ const dshVersion = () => {
 }
 
 export function install(rt) {
-  const { ctx, home, config } = rt
+  const { ctx, home, statePath, config } = rt
   const logPath = join(home, 'logs', 'diagnostics.jsonl')
   const toFile = config.diagnosticsFile !== false
   let fresh = []
@@ -237,20 +240,20 @@ export function install(rt) {
     }))
   }
 
-  // The journal's file, named without opening it (#2388). `lstat` answers what the name is: a
-  // missing file is the next save's to create, a plain file of this host's own has a size and a
-  // last write time worth reporting, and a link or a file with a second name is bytes that are
-  // not this host's to read — so the answer is a sentence in every case, never a throw, and no
-  // read permission is needed for a file that would otherwise have to be opened.
+  // A file named without opening it (#2388): `lstat` answers what the name is — a missing file is
+  // the writer's to create, a plain file of this host's own has a size and a last write time worth
+  // reporting, and a link, a second name or a check that cannot run is bytes this report may not
+  // quote. So the answer is a sentence in every case, never a throw, and no read permission is
+  // needed for a file that would otherwise have to be opened.
   const NOT_PLAIN = 'not a plain file (nothing is read or written)'
   // A path is cleaned with this host's own home, not the process's: a machine shows `~/.dsh/...`
   // and a test's temporary home shows `~/...`, so no report carries the folder a person lives in.
   const cleanPath = text => cleanText(text, { home, redact: rt.redactSecrets })
-  const journalFile = async (path) => {
+  const fileFact = async (path, { missing }) => {
     if (path === undefined) return undefined
     try {
       const kind = await fileKind(path)
-      if (kind === 'absent') return 'none yet'
+      if (kind === 'absent') return missing
       if (kind !== 'real') return NOT_PLAIN
       const info = await lstat(path)
       return `${cleanPath(path)} (${info.size} bytes, last written ${new Date(info.mtimeMs).toISOString()})`
@@ -272,6 +275,10 @@ export function install(rt) {
       os: `${platform()} ${release()} ${arch()}`,
       node: process.version,
       team: `${count(bots.length, 'Bot', 'Bots')}, ${rt.state.mainBotIds.length} Main, ${count(groups, 'group chat', 'group chats')}`,
+      // The file that team is saved in, in the same shape as the two below (#2394): `lstat` only,
+      // because the team file holds members and messages and a report may describe it and never
+      // open it. `none yet` is a computer that has not saved this team — naming it creates nothing.
+      stateFile: await fileFact(statePath, { missing: 'none yet (this computer has not saved the team)' }),
       models: models.length > 0 ? models.join(', ') : 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
@@ -289,7 +296,7 @@ export function install(rt) {
       // The file that sentence is about, in the same shape (#2388): the path with the home as `~`
       // and its size and last write time, `none yet` before the first save creates it, or the one
       // sentence for a leaf this host would not read. A host without the journal says nothing.
-      journalFile: journal === undefined ? undefined : await journalFile(rt.journal.path),
+      journalFile: journal === undefined ? undefined : await fileFact(rt.journal.path, { missing: 'none yet' }),
       // Where those errors are kept is the other half of that sentence, in the same shape: a host
       // with the log off says nothing (#2134), a host whose appends land says so, and a refused
       // leaf or a failed append names its own reason — the words stderr already used, never the

core/test/diagnostics.test.js

@@ -1,5 +1,5 @@
 import assert from 'node:assert/strict'
-import { chmodSync, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
+import { chmodSync, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
 import fsp from 'node:fs/promises'
 import { syncBuiltinESMExports } from 'node:module'
 import { homedir, hostname, tmpdir } from 'node:os'
@@ -612,9 +612,10 @@ test('the report says nothing about the log while it is off, and recording while
 // 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 } = {}) {
+async function realHost(t, { saved, locked = false, prepare } = {}) {
   const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-ro-'))
   const statePath = join(home, 'state.json')
+  prepare?.(statePath)
   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.
@@ -642,9 +643,101 @@ async function realHost(t, { saved, locked = false } = {}) {
     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({}) }
+  return { home, statePath, rt, readOnly: rt.readOnly(), report: await rt.diagnostics.report({}) }
 }
 
+// #2394: the report counts the team — Bots, Main Bots, group chats — and never says what that team
+// is saved in. `rt.statePath` was already on the runtime (store.js), and the file holds the members
+// and the messages themselves, so the line describes it with `lstat` alone: size and last write
+// time, `none yet` before this computer saves the team, or the one sentence for a leaf that is a
+// link, a second name or anything else this report may describe but never read.
+test('the report names the team file, its size and when it was last written (#2394)', async (t) => {
+  const { home, statePath, rt } = await realHost(t)
+  await rt.save()
+  const info = lstatSync(statePath)
+  const report = await rt.diagnostics.report({})
+
+  assert.equal(report.facts.stateFile, `~/state.json (${info.size} bytes, last written ${new Date(info.mtimeMs).toISOString()})`)
+  assert.match(report.markdown, /^state file: ~\/state\.json \(\d+ bytes, last written \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z\)$/m)
+  assert.ok(report.markdown.indexOf('state file: ') > report.markdown.indexOf('| Team |'), 'the line follows the table that counts the team')
+  assert.equal(report.markdown.includes(home), false, 'the line never carries the home folder')
+  assert.match(readFileSync(statePath, 'utf8'), /"version": 2/, 'the file it names is the one the save wrote')
+})
+
+// Naming the file must not create it: a computer that has not saved this team has no team file yet,
+// and the answer says that instead of quoting a size for a file the next save would make. The
+// store's first boot saves the migrated team, so the absent file here is one this computer does not
+// have — deleted since, or a save that never landed — and the report must describe that and nothing
+// else.
+test('a computer that has not saved the team says so, and naming the file creates nothing (#2394)', async (t) => {
+  const { home, statePath, rt } = await realHost(t)
+  rmSync(statePath)
+  const report = await rt.diagnostics.report({})
+  assert.equal(report.facts.stateFile, 'none yet (this computer has not saved the team)')
+  assert.match(report.markdown, /^state file: none yet \(this computer has not saved the team\)$/m)
+  assert.equal(existsSync(statePath), false, 'naming the file creates nothing')
+  assert.equal(readdirSync(home).some(name => name.startsWith('state.json.')), false, 'and writes no temp name beside it')
+})
+
+// The team file's bytes are the members and the messages. A directory, a link and a second name are
+// each a name this report may describe and must not read through — and the bytes behind the link and
+// the other name stay exactly as they were.
+test('a team file that is a directory, a link or a second name is named as not a plain file (#2394)', async (t) => {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-'))
+  t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
+  const elsewhere = mkdtempSync(join(tmpdir(), 'ds-bot-diag-target-'))
+  t.after(() => rmSync(elsewhere, { recursive: true, force: true, maxRetries: 5 }))
+  const kept = '{"version":2,"bots":{}}\n'
+  const quiet = { config: { diagnosticsFile: false } }
+
+  const asDir = join(home, 'dir-home')
+  mkdirSync(join(asDir, 'state.json'), { recursive: true })
+  const dirReport = await host(asDir, { ...quiet, statePath: join(asDir, 'state.json') }).diagnostics.report({})
+  assert.equal(dirReport.facts.stateFile, 'not a plain file (nothing is read or written)')
+  assert.match(dirReport.markdown, /^state file: not a plain file \(nothing is read or written\)$/m)
+
+  const target = join(elsewhere, 'someone-else.json')
+  writeFileSync(target, kept)
+  const linked = join(home, 'link-home')
+  mkdirSync(linked, { recursive: true })
+  symlinkSync(target, join(linked, 'state.json'))
+  const linkedReport = await host(linked, { ...quiet, statePath: join(linked, 'state.json') }).diagnostics.report({})
+  assert.equal(linkedReport.facts.stateFile, 'not a plain file (nothing is read or written)')
+  assert.match(linkedReport.markdown, /^state file: not a plain file \(nothing is read or written\)$/m)
+  assert.equal(readFileSync(target, 'utf8'), kept, 'the file behind the link keeps its bytes')
+
+  const plain = join(elsewhere, 'plain.json')
+  writeFileSync(plain, kept)
+  const shared = join(home, 'hard-home')
+  mkdirSync(shared, { recursive: true })
+  linkSync(plain, join(shared, 'state.json'))
+  const hardReport = await host(shared, { ...quiet, statePath: join(shared, 'state.json') }).diagnostics.report({})
+  assert.equal(lstatSync(plain).nlink, 2, 'this is a file with a second name')
+  assert.equal(hardReport.facts.stateFile, 'not a plain file (nothing is read or written)')
+  assert.match(hardReport.markdown, /^state file: not a plain file \(nothing is read or written\)$/m)
+  assert.equal(readFileSync(plain, 'utf8'), kept, 'the other name keeps its bytes')
+  assert.equal(hardReport.markdown.includes(home), false, 'the line never carries the home folder')
+})
+
+// The line is read information, not a write: a window another dsh keeps read-only still names the
+// team file, the way it still counts the team it shows.
+test('a read-only window names the team file too (#2394)', async (t) => {
+  const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-'))
+  t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
+  const statePath = join(home, 'state.json')
+  writeFileSync(statePath, '{"version":2}\n')
+  const info = lstatSync(statePath)
+  const report = await host(home, {
+    config: { diagnosticsFile: false },
+    statePath,
+    readOnly: () => 'Another dsh (pid 1 on host) holds the team',
+  }).diagnostics.report({})
+
+  assert.match(report.markdown, /^\| Read-only \| /m, 'this window says it may not write')
+  assert.equal(report.facts.stateFile, `~/state.json (${info.size} bytes, last written ${new Date(info.mtimeMs).toISOString()})`)
+  assert.match(report.markdown, /^state file: ~\/state\.json \(\d+ bytes, last written .+\)$/m)
+})
+
 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/)