From b33e1fb8aa9ec26882e087bf2df40f78f26e6399 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Wed, 4 Nov 2026 20:52:16 +0800 Subject: [PATCH] host: the diagnostics report names the team file it counts (#2394) The report behind Report on GitHub counts the team and then says nothing about the file that team is saved in. It now prints `state file:` right after the table, built from lstat alone: size and last write for a plain file, a named absence before the first save, and one refusal line for a directory, a link or a file with a second name. state.json holds the members, the messages and the saved keys, so the report describes it and never opens it. --- changelog.d/2394-state-file.fixed.md | 1 + core/src/host/diagnostics.js | 27 +++-- core/test/diagnostics.test.js | 99 ++++++++++++++++- docs/subsystems/core-host.md | 1 + patches/ds-bot/0001-core-fixes.patch | 38 ++++--- patches/ds-bot/0007-version-and-tooling.patch | 101 +++++++++++++++++- 6 files changed, 236 insertions(+), 31 deletions(-) create mode 100644 changelog.d/2394-state-file.fixed.md diff --git a/changelog.d/2394-state-file.fixed.md b/changelog.d/2394-state-file.fixed.md new file mode 100644 index 000000000..527f5341b --- /dev/null +++ b/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 ( bytes, last written )` 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)). diff --git a/core/src/host/diagnostics.js b/core/src/host/diagnostics.js index a6f45a8d5..d1c76b804 100644 --- a/core/src/host/diagnostics.js +++ b/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 diff --git a/core/test/diagnostics.test.js b/core/test/diagnostics.test.js index 5a1b40b61..8a325a5d7 100644 --- a/core/test/diagnostics.test.js +++ b/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/) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 2e077366c..ca78a8042 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -407,6 +407,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - 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. Past 1 MB the leaf moves aside to `diagnostics.jsonl.1`, and `clear` removes that name as well — in the same write chain, behind the same leaf guard — so a cleared log leaves no rotated lines behind. The truncate runs first and the removal only after it: a leaf that cannot be emptied keeps the rotated copy where it is, because those lines are then the only copy left of them. The removal keeps a `try` of its own: a copy the host cannot remove (a directory under that name, a folder without its write bit) keeps its bytes, is named on one stderr line that carries no folder, and never turns a Clear that emptied the leaf into one that leaves every line on disk. The truncate keeps a `try` of its own for the same reason ([#2364](https://github.com/ThinkFlowLab/FlyBot/issues/2364)): the leaf guard asks the file's kind, not its mode, so a plain `0444` leaf passes it and the write fails with `EACCES` — that failure is named on one stderr line, `[ds-bot] diagnostics log not truncated: `, which carries no folder, the same way `append` names its own; the answer it leaves behind is `leaf: 'kept'`. `clear` answers with what it did — `{ leaf: 'truncated' | 'kept' | 'absent', rotated: 'removed' | 'kept' | 'none' }` — and `diagnostics-clear` passes that result through instead of a constant, so the Feedback page's sentence matches it: the unchanged one, one that names the leftover `logs/diagnostics.jsonl.1`, or one that does not claim a cleared log ([#2360](https://github.com/ThinkFlowLab/FlyBot/issues/2360)). `rotated` says what this call did to that name: `removed` when it deleted one, `kept` when one was there and still is — the removal failed, or the truncate failed first and nothing was removed — and `none` when there was none; a leaf the guard refuses never reaches the probe, so it answers `none`. What the window reads moves only with an emptied leaf: `entries()` answers the old lines through `cleared` plus the live ones in `fresh`, and `logText()` behind the Download reads the same, so a Clear that leaves the leaf full changes neither of them — the page can no longer show "not cleared" next to "0 recent errors" while every byte is still on disk. Only a leaf that really was truncated clears both — plus a host that keeps no log file at all (`diagnosticsFile: false`), which has no file and no list to keep; an `ENOENT` write leaves both alone like every other refusal ([#2364](https://github.com/ThinkFlowLab/FlyBot/issues/2364)). That is the answer, one stderr line and nothing else that moves: the truncate, the `fileKind` guard, the removal rule and the `try` keep the behaviour they had, and a leaf that is not there yet still gets the empty file the write always landed there. `absent` therefore means nothing was on disk to clear — this host keeps no log file (`diagnosticsFile: false`), or the folder is not there so the write found nothing to truncate — not a leaf the call refused. The report says whether that same file is being written, in the shape the journal line already has: `diagnostics log: recording` while the appends land, `not written ()` after an append the host could not make, and `not written (not a plain file of its own)` when the leaf guard refused the write, so a bundle that lists recent errors also says when the file holding them is missing or short and why. The value follows the last append attempt, not the first refusal: a host that puts a plain file of its own back writes the next entry into it and says `recording` again, and a leaf that has become a plain `0444` file says `not written (EACCES)` on the next attempt, while the guard's own stderr line stays one per host ([#2089](https://github.com/ThinkFlowLab/FlyBot/issues/2089)); a host with the log off (`diagnosticsFile: false`) adds no line at all, the rule the journal line keeps ([#2383](https://github.com/ThinkFlowLab/FlyBot/issues/2383)). +- The report also names the team file itself, right after the table that counts the team ([#2394](https://github.com/ThinkFlowLab/FlyBot/issues/2394)): `state file: ( bytes, last written )` for a plain file with this one name, `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, a file with a second name (`nlink > 1`) or a check that cannot run. It is built from `lstat` alone like the journal's line, and that is the point: `state.json` holds the members, the messages and the saved keys, so the report may describe the file and must never open it — a report can say how large the team file is and when it was last written without reading a byte of it. The path goes through the same home redaction, the line is read information rather than a write action so a read-only window draws it too, and the wording of `state journal:`, `journal file:` and the `Read-only` row is unchanged. - The report names the file the journal records into, in the journal's own shape and without opening it ([#2388](https://github.com/ThinkFlowLab/FlyBot/issues/2388)): `journal file: ( bytes, last written )` for a plain file with this one name, `journal file: none yet` before the first save creates it, and `journal file: not a plain file (nothing is read or written)` when the path is a link, a file with a second name (`nlink > 1`) or the check cannot run. `lstat` is the only call — never `open`, `readFile` or `createReadStream` — so the line needs no read permission on the file and touches nothing another name holds; the path goes through the same home redaction the rest of the report uses, and a host without a journal adds no line, like the `state journal:` sentence it follows. The line says what the file is, not that a write happened: `state journal:` keeps answering whether the host is recording. ### 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 4cba375c0..8da1e1442 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -4996,11 +4996,14 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js row('DSH', facts.dshVersion), row('Surface', client.surface), row('OS', facts.os), -@@ -78,11 +78,20 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries +@@ -78,11 +78,23 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries row('Read-only', facts.readOnly), ].filter(Boolean) const recent = entries.slice(-REPORT_ENTRIES) - const out = [...lines, ''] ++ // 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 @@ -5010,7 +5013,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js + // 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) @@ -5019,7 +5022,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js if (clientEntries.length > 0) { out.push('', `Recent browser errors (${Math.min(clientEntries.length, 10)}):`, '```', ...clientEntries.slice(-10).map(entryLine), '```') } -@@ -105,9 +105,9 @@ const dshVersion = () => { +@@ -105,19 +105,22 @@ const dshVersion = () => { try { const manifest = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8')) if (manifest.name === '@deepseek-ai/dsh-llm') return manifest.version @@ -5031,7 +5034,10 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js return undefined } -@@ -117,7 +117,10 @@ export function install(rt) { + 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 = [] let writing = Promise.resolve() @@ -5108,25 +5114,25 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js console.error(`[ds-bot] ${line}${frames.length > 0 ? `\n${clean(frames.join('\n'))}` : ''}`) note({ level: 'warn', -@@ -187,11 +187,35 @@ export function install(rt) { +@@ -187,24 +187,77 @@ export function install(rt) { })) } - const facts = () => { -+ // 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()})` @@ -5145,9 +5151,13 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js return { botVersion: ownVersion(), dshVersion: dshVersion(), -@@ -199,12 +199,37 @@ 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, - readOnly: rt.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 @@ -5166,7 +5176,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js + // 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 diff --git a/patches/ds-bot/0007-version-and-tooling.patch b/patches/ds-bot/0007-version-and-tooling.patch index da2d280fb..f8a06a268 100644 --- a/patches/ds-bot/0007-version-and-tooling.patch +++ b/patches/ds-bot/0007-version-and-tooling.patch @@ -798,7 +798,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js @@ -1,11 +1,13 @@ import assert from 'node:assert/strict' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' -+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, tmpdir } from 'node:os' @@ -1005,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,402 @@ test('diagnostics append fallback redacts an error message', async (t) => { +@@ -133,6 +133,495 @@ test('diagnostics append fallback redacts an error message', async (t) => { assert.match(stderr.join('\n'), /api_key=\[hidden\]/) }) @@ -1346,9 +1346,10 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js +// 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. @@ -1376,9 +1377,101 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js + 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/)