Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/2134-journal-status.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
With `stateJournal` on, the diagnostics report now says whether the state journal is recording or why it stopped, and `journal.status()` answers the same question to a host ([#2134](https://github.com/ThinkFlowLab/FlyBot/issues/2134)).
7 changes: 6 additions & 1 deletion core/src/host/diagnostics.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries
row('Read-only', facts.readOnly),
].filter(Boolean)
const recent = entries.slice(-REPORT_ENTRIES)
const out = [...lines, '']
// 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.
const journal = facts.stateJournal === undefined ? [] : ['', `state journal: ${facts.stateJournal}`]
const out = [...lines, ...journal, '']
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)
Expand Down Expand Up @@ -221,6 +224,7 @@ export function install(rt) {
const models = [...new Set(bots.map(bot => bot.model ?? bot.appliedModel).filter(Boolean))]
const count = (n, one, many) => `${n} ${n === 1 ? one : many}`
const groups = Object.keys(rt.state.rooms).length
const journal = rt.journal?.status()
return {
botVersion: ownVersion(),
dshVersion: dshVersion(),
Expand All @@ -229,6 +233,7 @@ export function install(rt) {
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,
stateJournal: journal === undefined ? undefined : journal.recording ? 'recording' : `stopped (${clean(journal.because)})`,
}
}

Expand Down
10 changes: 8 additions & 2 deletions core/src/host/journal.js
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,19 @@ export function pruneEntries(entries, { now, maxBytes = JOURNAL_MAX_BYTES, maxAg
// and leaves the file as it is; it never stops the team from saving.
export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX_BYTES, maxAgeMs = JOURNAL_MAX_AGE_MS }) {
let stopped = false
// Why recording stopped, in the words `stop()` already had; `status()` hands it on.
let because
let lastText
let seq = 0
let bytes = 0
let oldest
let queue = Promise.resolve()

const stop = (error) => {
if (stopped) return
stopped = true
warn('ds-bot: state journal %s is not recording: %s', path, error)
because = `${path} is not recording: ${error}`
warn('ds-bot: state journal %s', because)
}

const open = async () => {
Expand Down Expand Up @@ -322,5 +326,7 @@ export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX
queue = queue.then(() => write(snapshot, actor)).catch(stop)
return queue
}
return { record, path }
// Whether this writer still records, and why not — the host can ask instead of reading stderr.
const status = () => (stopped ? { recording: false, because } : { recording: true })
return { record, path, status }
}
2 changes: 1 addition & 1 deletion core/src/host/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ export function install(rt) {
const isAdminOf = (room, id) => id !== undefined && room.admin === id

Object.assign(rt, {
state, load, save, refresh, readOnly, mutate, savesQueued, savesSettled,
state, load, save, refresh, readOnly, mutate, savesQueued, savesSettled, journal,
botOf, roomOf, isMain, chatOf, selfOf, isBotSession,
chatOwners, groupOwners, pastOwners,
groupSessionsOf, dropGroupSession, findBot, findRooms, isAdminOf,
Expand Down
27 changes: 27 additions & 0 deletions core/test/diagnostics.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { homedir, 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 { 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'
Expand Down Expand Up @@ -273,6 +274,32 @@ test('diagnostics append fallback redacts an error message', async (t) => {
assert.match(stderr.join('\n'), /api_key=\[hidden\]/)
})

// #2134: the report is where a user looks after a bug the journal was turned on for, so it says
// whether the journal is still recording or why it stopped, and says nothing while it is off.
test('the diagnostics report says whether the state journal is recording or why it stopped (#2134)', async (t) => {
const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-'))
t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
const off = await host(home, { config: { diagnosticsFile: false } }).diagnostics.report({})
assert.equal(off.facts.stateJournal, undefined, 'a host without the journal says nothing about it')
assert.doesNotMatch(off.markdown, /state journal:/)

const target = join(home, 'someone-else.txt')
writeFileSync(target, 'PRIVATE LINE\n')
mkdirSync(join(home, 'journal'), { recursive: true })
const path = join(home, 'journal', 'state.jsonl')
symlinkSync(target, path)
const journal = openJournal({ path, warn: () => {} })
await journal.record(stateText({ version: 2, revision: 1 }))
const stopped = await host(home, { journal, config: { diagnosticsFile: false } }).diagnostics.report({})
assert.match(stopped.markdown, /^state journal: stopped \(.*not a plain file.*\)$/m)
assert.equal(stopped.facts.stateJournal.startsWith('stopped ('), true)

const recording = openJournal({ path: join(home, 'live', 'state.jsonl'), warn: () => {} })
const live = await host(home, { journal: recording, config: { diagnosticsFile: false } }).diagnostics.report({})
assert.match(live.markdown, /^state journal: recording$/m)
assert.equal(live.facts.stateJournal, 'recording')
})

test('reportMarkdown says when nothing went wrong', () => {
const text = reportMarkdown({ facts: { botVersion: '0.1.0', os: 'linux' } })
assert.match(text, /No host errors recorded\./)
Expand Down
28 changes: 28 additions & 0 deletions core/test/state-journal.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,31 @@ test('a journal that is still its own folder and file keeps recording (#2074 con
if (posix) assert.equal(statSync(path).mode & 0o777, 0o600)
rmSync(home, { recursive: true, force: true })
})

// #2134: the journal can stop for good — a link where its file belongs, a folder the host does not
// own, a write that failed — and until now only a stderr warning said so. `status()` answers the
// question a host can ask, with the reason `stop()` already received and the journal path in it.
test('the journal says whether it is still recording, and why it stopped, path included (#2134)', async () => {
const home = tempHome()
const outside = mkdtempSync(join(tmpdir(), 'ds-bot-journal-outside-'))
const target = join(outside, 'someone-else.txt')
writeFileSync(target, 'PRIVATE LINE\n')
mkdirSync(join(home, 'journal'))
const path = join(home, 'journal', 'state.jsonl')
symlinkSync(target, path)
const stopped = openJournal({ path, warn: () => {} })
await stopped.record(stateText({ version: 2, revision: 1 }))
const answer = stopped.status()
assert.equal(answer.recording, false)
assert.ok(answer.because.includes(path), `the reason names the journal: ${answer.because}`)
assert.match(answer.because, /not a plain file/)
assert.equal(readFileSync(target, 'utf8'), 'PRIVATE LINE\n', 'nothing is written through the link')

// The control: a journal that is still its own file answers recording, before and after a write.
const own = openJournal({ path: join(home, 'own', 'state.jsonl'), warn: () => {} })
assert.deepEqual(own.status(), { recording: true })
await own.record(stateText({ version: 2, revision: 1 }))
assert.deepEqual(own.status(), { recording: true })
rmSync(home, { recursive: true, force: true })
rmSync(outside, { recursive: true, force: true })
})
2 changes: 1 addition & 1 deletion docs/subsystems/core-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ A local, replayable record of every change to the saved team, for multi-Bot bugs
- **On.** `stateJournal: true` in the `bot` config. Off (the default) nothing is opened or written, so every other test runs as before.
- **What.** `src/host/journal.js`. After each successful `state.json` write, `store.js` hands the exact snapshot text to the journal, which appends the difference from the last recorded snapshot to `<home>/journal/state.jsonl` (file mode 0600, folder 0700). Lines are `{ seq, t, op, key, prev, next, botId?, actor }`; `op` is `base` (the whole team: the first line, and any change only a key reorder explains), `set`, `delete`, or `append` (`prevLength`, `drop`, the new items: a group log or other list that grew or slid). Paths go at most three keys deep (`bots.<id>.<field>`); `botId` is set on a `bots.<id>` path; `actor` is `host` for a save and `retention` for a folded base (per-call attribution is not recorded). Only the host that holds the state lock writes, so a read-only window records nothing.
- **Retention.** When a write would pass 20 MB, or the oldest change is older than 7 days, the oldest lines are dropped down to 90% of each cap and folded into the base line (atomic rewrite), so the kept lines still replay. A team that alone exceeds the cap stops recording.
- **Fail closed.** Replay (`replayTo(text, seq)`) refuses the whole journal on an unreadable or incomplete line, an unknown op, a gap in `seq`, a first line that is not a base, or a `prev` that does not match what the earlier lines built; a `seq` outside the file is refused, not approximated. On start the writer replays the existing file the same way; a journal that does not replay, a path that is not a plain file, or a failed write stops recording for that run with a warning and leaves the file as it is. Saving the team never depends on the journal. The folder above the journal must be a real directory too (#2056): a link, a plain file or a check that cannot run refuses before the `mkdir`/`chmod` that would follow it, so neither the permissions nor a write reach the folder a link names; a folder that does not exist yet is still this writer's to create, as before. Since [#2074](https://github.com/ThinkFlowLab/FlyBot/issues/2074) the same question is asked again when the write runs: a journal folder swapped for a link is refused with one warning and recording stops, and a `state.jsonl` swapped for a link is never written through. A hard link reads as a plain file to `lstat` while its bytes are also a name outside this home, so both the start-up read and the append also require the file to carry only this one name (`nlink === 1`): a shared inode is refused exactly like a link — one warning, nothing read, chmod-ed or appended — and a journal with one name records as before. The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder, so nothing is written through it.
- **Fail closed.** Replay (`replayTo(text, seq)`) refuses the whole journal on an unreadable or incomplete line, an unknown op, a gap in `seq`, a first line that is not a base, or a `prev` that does not match what the earlier lines built; a `seq` outside the file is refused, not approximated. On start the writer replays the existing file the same way; a journal that does not replay, a path that is not a plain file, or a failed write stops recording for that run with a warning and leaves the file as it is. Saving the team never depends on the journal. The folder above the journal must be a real directory too (#2056): a link, a plain file or a check that cannot run refuses before the `mkdir`/`chmod` that would follow it, so neither the permissions nor a write reach the folder a link names; a folder that does not exist yet is still this writer's to create, as before. Since [#2074](https://github.com/ThinkFlowLab/FlyBot/issues/2074) the same question is asked again when the write runs: a journal folder swapped for a link is refused with one warning and recording stops, and a `state.jsonl` swapped for a link is never written through. A hard link reads as a plain file to `lstat` while its bytes are also a name outside this home, so both the start-up read and the append also require the file to carry only this one name (`nlink === 1`): a shared inode is refused exactly like a link — one warning, nothing read, chmod-ed or appended — and a journal with one name records as before. The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder, so nothing is written through it. A writer that stopped says so off the host too: `rt.journal.status()` answers `{ recording, because }`, and the diagnostics report adds one line — `state journal: recording` or `state journal: stopped (<reason>)` — while a host without the journal adds none ([#2134](https://github.com/ThinkFlowLab/FlyBot/issues/2134)).
- **Replay.** `core/scripts/replay.mjs` (dev-only, not in the package, see [install](install.md)) prints or writes (`--out`, new file only, 0600) the team after a given `--to <seq>`; it makes no network or process calls. Test: `core/test/state-journal.test.js` (a 3-Bot round replays byte-identical at every step).

### Standing tool approvals (#540)
Expand Down
25 changes: 23 additions & 2 deletions patches/ds-bot/0001-core-fixes.patch
Original file line number Diff line number Diff line change
Expand Up @@ -4783,7 +4783,15 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js
row('DSH', facts.dshVersion),
row('Surface', client.surface),
row('OS', facts.os),
@@ -82,7 +82,7 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries
@@ -78,11 +78,14 @@ export function reportMarkdown({ facts, client = {}, entries = [], clientEntries
row('Read-only', facts.readOnly),
].filter(Boolean)
const recent = entries.slice(-REPORT_ENTRIES)
- const out = [...lines, '']
+ // 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.
+ const journal = facts.stateJournal === undefined ? [] : ['', `state journal: ${facts.stateJournal}`]
+ const out = [...lines, ...journal, '']
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)
Expand Down Expand Up @@ -4856,7 +4864,20 @@ 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',
@@ -204,7 +204,7 @@ export function install(rt) {
@@ -192,6 +192,7 @@ export function install(rt) {
const models = [...new Set(bots.map(bot => bot.model ?? bot.appliedModel).filter(Boolean))]
const count = (n, one, many) => `${n} ${n === 1 ? one : many}`
const groups = Object.keys(rt.state.rooms).length
+ const journal = rt.journal?.status()
return {
botVersion: ownVersion(),
dshVersion: dshVersion(),
@@ -200,11 +200,12 @@ export function install(rt) {
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,
+ stateJournal: journal === undefined ? undefined : journal.recording ? 'recording' : `stopped (${clean(journal.because)})`,
}
}

const sanitizeClient = (client) => {
Expand Down
2 changes: 1 addition & 1 deletion patches/ds-bot/0002-flybot-api-host.patch
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,7 @@ diff --git a/src/host/store.js b/src/host/store.js

Object.assign(rt, {
- state, load, save, refresh, readOnly,
+ state, load, save, refresh, readOnly, mutate, savesQueued, savesSettled,
+ state, load, save, refresh, readOnly, mutate, savesQueued, savesSettled, journal,
botOf, roomOf, isMain, chatOf, selfOf, isBotSession,
chatOwners, groupOwners, pastOwners,
groupSessionsOf, dropGroupSession, findBot, findRooms, isAdminOf,
Expand Down
36 changes: 35 additions & 1 deletion patches/ds-bot/0007-version-and-tooling.patch
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ 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,11 @@
@@ -1,11 +1,12 @@
import assert from 'node:assert/strict'
-import { mkdtempSync, readFileSync, rmSync } from 'node:fs'
+import { lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
Expand All @@ -806,6 +806,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js
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 { 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'
Expand Down Expand Up @@ -1002,6 +1003,39 @@ 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,32 @@ test('diagnostics append fallback redacts an error message', async (t) => {
assert.match(stderr.join('\n'), /api_key=\[hidden\]/)
})

+// #2134: the report is where a user looks after a bug the journal was turned on for, so it says
+// whether the journal is still recording or why it stopped, and says nothing while it is off.
+test('the diagnostics report says whether the state journal is recording or why it stopped (#2134)', async (t) => {
+ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-'))
+ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 }))
+ const off = await host(home, { config: { diagnosticsFile: false } }).diagnostics.report({})
+ assert.equal(off.facts.stateJournal, undefined, 'a host without the journal says nothing about it')
+ assert.doesNotMatch(off.markdown, /state journal:/)
+
+ const target = join(home, 'someone-else.txt')
+ writeFileSync(target, 'PRIVATE LINE\n')
+ mkdirSync(join(home, 'journal'), { recursive: true })
+ const path = join(home, 'journal', 'state.jsonl')
+ symlinkSync(target, path)
+ const journal = openJournal({ path, warn: () => {} })
+ await journal.record(stateText({ version: 2, revision: 1 }))
+ const stopped = await host(home, { journal, config: { diagnosticsFile: false } }).diagnostics.report({})
+ assert.match(stopped.markdown, /^state journal: stopped \(.*not a plain file.*\)$/m)
+ assert.equal(stopped.facts.stateJournal.startsWith('stopped ('), true)
+
+ const recording = openJournal({ path: join(home, 'live', 'state.jsonl'), warn: () => {} })
+ const live = await host(home, { journal: recording, config: { diagnosticsFile: false } }).diagnostics.report({})
+ assert.match(live.markdown, /^state journal: recording$/m)
+ assert.equal(live.facts.stateJournal, 'recording')
+})
+
test('reportMarkdown says when nothing went wrong', () => {
const text = reportMarkdown({ facts: { botVersion: '0.1.0', os: 'linux' } })
assert.match(text, /No host errors recorded\./)
@@ -155,6 +155,29 @@ test('turnFailure drops AUTH messages and keeps the facts', () => {
assert.equal(failureLine({ code: 'SERVER', status: 503, message: 'busy' }), 'SERVER 503: busy')
})
Expand Down
Loading
Loading