host: a linked team file is described as read through the link, not as nothing being read

changelog.d/2404-state-file-link-wording.fixed.md

@@ -0,0 +1 @@
+The diagnostics report behind Settings → Feedback's **Report on GitHub** no longer describes a linked `state.json` as `not a plain file (nothing is read or written)`. The store reads the team file through that link and its next save replaces the name, so the `state file:` row now says `not a plain file (described only; the store reads the team file through the link and the next save replaces the name)` instead of contradicting the `Read-only` row right below it, which can only say the file could not be read because the read happened. A directory or a check that cannot run keeps the earlier sentence, and `journal file:`, `state journal:`, `diagnostics log:` and the `Read-only` row are unchanged ([#2404](https://github.com/ThinkFlowLab/FlyBot/issues/2404)).

core/src/host/diagnostics.js

@@ -246,15 +246,28 @@ export function install(rt) {
   // 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)'
+  // The team file is not a journal leaf (#2404): the store reads it through the link
+  // (`readFile(statePath)` follows the last name) and its next save renames a temp name over this
+  // one (`rename(temp, statePath)`), so a link is read and replaced, not ignored. A directory or a
+  // check that cannot run keeps the leaf sentence above: no read of those lands either.
+  const LINKED_STATE = 'not a plain file (described only; the store reads the team file through the link and the next save replaces the name)'
+  const stateNotPlain = async path => {
+    try {
+      const info = await lstat(path)
+      return info.isSymbolicLink() || (info.isFile() && info.nlink > 1) ? LINKED_STATE : NOT_PLAIN
+    } catch {
+      return NOT_PLAIN
+    }
+  }
   // 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 fileFact = async (path, { missing }) => {
+  const fileFact = async (path, { missing, notPlain }) => {
     if (path === undefined) return undefined
     try {
       const kind = await fileKind(path)
       if (kind === 'absent') return missing
-      if (kind !== 'real') return NOT_PLAIN
+      if (kind !== 'real') return notPlain === undefined ? NOT_PLAIN : await notPlain(path)
       const info = await lstat(path)
       return `${cleanPath(path)} (${info.size} bytes, last written ${new Date(info.mtimeMs).toISOString()})`
     } catch {
@@ -278,7 +291,8 @@ export function install(rt) {
       // 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)' }),
+      // A link is described by the store's own shape (#2404), not by the journal leaf's sentence.
+      stateFile: await fileFact(statePath, { missing: 'none yet (this computer has not saved the team)', notPlain: stateNotPlain }),
       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

core/test/diagnostics.test.js

@@ -646,6 +646,10 @@ async function realHost(t, { saved, locked = false, prepare } = {}) {
   return { home, statePath, rt, readOnly: rt.readOnly(), report: await rt.diagnostics.report({}) }
 }
 
+// The state line's own sentence for a link or a second name (#2404). The journal leaf keeps
+// `nothing is read or written`; the store reads this file through the link and replaces the name.
+const LINKED_STATE = 'not a plain file (described only; the store reads the team file through the link and the next save replaces the name)'
+
 // #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
@@ -681,7 +685,8 @@ test('a computer that has not saved the team says so, and naming the file create
 
 // 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.
+// the other name stay exactly as they were. A directory keeps the journal leaf's sentence, which is
+// true of it; a link says what the store does with that shape instead (#2404).
 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 }))
@@ -702,8 +707,8 @@ test('a team file that is a directory, a link or a second name is named as not a
   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(linkedReport.facts.stateFile, LINKED_STATE)
+  assert.match(linkedReport.markdown, /^state file: not a plain file \(described only; the store reads the team file through the link and the next save replaces the name\)$/m)
   assert.equal(readFileSync(target, 'utf8'), kept, 'the file behind the link keeps its bytes')
 
   const plain = join(elsewhere, 'plain.json')
@@ -713,12 +718,45 @@ test('a team file that is a directory, a link or a second name is named as not a
   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(hardReport.facts.stateFile, LINKED_STATE)
+  assert.match(hardReport.markdown, /^state file: not a plain file \(described only; the store reads the team file through the link and the next save replaces the name\)$/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')
 })
 
+// #2404: the state line borrowed the journal leaf's sentence, `nothing is read or written`, but the
+// store does both with the team file: `readFile(statePath)` follows the link and `rename(temp,
+// statePath)` replaces the name on the next save. A linked `state.json` therefore said nothing had
+// been read in the line above a `Read-only` row that could only exist because the read happened and
+// failed. The state line describes its own file: described only, read through the link, name
+// replaced by the next save. A directory or a check that cannot run keeps the leaf sentence, which
+// is true of it.
+test('a linked team file is described as read through the link, not as nothing being read (#2404)', async (t) => {
+  const elsewhere = mkdtempSync(join(tmpdir(), 'ds-bot-diag-target-'))
+  t.after(() => rmSync(elsewhere, { recursive: true, force: true, maxRetries: 5 }))
+  const newer = '{"version":99}\n'
+  const target = join(elsewhere, 'newer.json')
+  writeFileSync(target, newer)
+
+  const linked = await realHost(t, { prepare: statePath => symlinkSync(target, statePath) })
+  const line = linked.report.markdown.match(/^state file: (.*)$/m)?.[1]
+  assert.ok(line !== undefined, 'the report still names the team file')
+  assert.doesNotMatch(line, /nothing is read or written/, 'the line must not deny the read the row below it proves')
+  assert.match(line, /the store reads the team file through the link and the next save replaces the name/)
+  assert.match(linked.report.markdown, /^\| Read-only \| The Bot team file .* could not be read \(saved by a newer FlyBot; update FlyBot\)/m,
+    'the row that only exists because the file was read stays as it is')
+  assert.equal(readFileSync(target, 'utf8'), newer, 'the file behind the link keeps its bytes')
+
+  const plain = join(elsewhere, 'plain.json')
+  writeFileSync(plain, newer)
+  const shared = await realHost(t, { prepare: statePath => linkSync(plain, statePath) })
+  const hardLine = shared.report.markdown.match(/^state file: (.*)$/m)?.[1]
+  assert.equal(lstatSync(plain).nlink, 2, 'this is a file with a second name')
+  assert.doesNotMatch(hardLine, /nothing is read or written/, 'a second name is read and replaced the same way')
+  assert.match(hardLine, /the store reads the team file through the link and the next save replaces the name/)
+  assert.equal(readFileSync(plain, 'utf8'), newer, 'the other name keeps its bytes')
+})
+
 // 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) => {

docs/subsystems/core-host.md

@@ -407,7 +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: <code>`, 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 (<code>)` 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: <path, home as ~> (<size> bytes, last written <ISO UTC>)` 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 also names the team file itself, right after the table that counts the team ([#2394](https://github.com/ThinkFlowLab/FlyBot/issues/2394)): `state file: <path, home as ~> (<size> bytes, last written <ISO UTC>)` 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 (described only; the store reads the team file through the link and the next save replaces the name)` when the path is a link or a file with a second name (`nlink > 1`) — the store's own shape, which reads the team file through the name and renames a temp name over it on the next save, so the line must not claim nothing is read or written ([#2404](https://github.com/ThinkFlowLab/FlyBot/issues/2404)). A directory or a check that cannot run keeps the journal leaf's `state file: not a plain file (nothing is read or written)`, which is true of it: no read of those lands. 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: <path, home as ~> (<size> bytes, last written <ISO UTC>)` 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