fix: bound the decisions log on disk and on read (#603)

changelog.d/603-decisions-log-bounded.fixed.md

@@ -0,0 +1 @@
+The decision log (`decisions.jsonl`) is now bounded on disk: the host trims it to the newest 2000 entries and at most 1 MB as it appends, and a read parses at most that byte cap instead of the whole file, so the log no longer grows without bound ([#603](https://github.com/ThinkFlowLab/FlyBot/issues/603)).

core/src/host/decisions.js

@@ -8,7 +8,7 @@
 // default install behaves exactly as before. The log is local text, one JSON line per entry; the
 // same entry is fired on the activity feed as `decision` so a consumer (Timeline, an audit view)
 // can show it without reading the file.
-import { appendFile, mkdir, readFile } from 'node:fs/promises'
+import { appendFile, mkdir, open, readFile, rename, stat, writeFile } from 'node:fs/promises'
 import { dirname, join } from 'node:path'
 import { defineTool } from '@deepseek-ai/dsh-tools'
 import { looksSecret } from './memory.js'
@@ -18,6 +18,9 @@ const OPTION_MAX = 200
 const WHY_MAX = 300
 const CHOICES = new Set(['used', 'rejected'])
 const KEPT = 2_000
+// The log is bounded on disk as well as in the answer: next to the line cap there is a byte cap,
+// so a burst of long entries cannot make the file (and every read) expensive.
+const MAX_BYTES = 1_000_000
 const NAME_SAFE = /^[\w-]{1,128}$/
 
 const oneLine = (text, max) => String(text ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, max)
@@ -32,16 +35,63 @@ export function install(rt) {
   // One append at a time, and a failure is a warning, never a turn failure (the log is a side
   // record, not part of the Bot's work).
   let writing = Promise.resolve()
+  // Lines on disk after the last append/trim, so a record does not re-read the file every time
+  // (null until the first record counts once).
+  let linesOnDisk = null
+  const countLines = async () => (await readFile(path, 'utf8')).split('\n').filter(Boolean).length
+  // Keep the file bounded in place: the newest entries under both caps, written through a temp
+  // file so a reader never sees a half-trimmed log. The Bot side stays append-only; this is the
+  // host's own housekeeping.
+  async function trim() {
+    let text
+    try { text = await readFile(path, 'utf8') } catch (error) { if (error?.code === 'ENOENT') return 0; throw error }
+    const lines = text.split('\n').filter(Boolean)
+    const kept = []
+    let bytes = 0
+    for (let i = lines.length - 1; i >= 0 && kept.length < KEPT; i--) {
+      const size = Buffer.byteLength(lines[i], 'utf8') + 1
+      if (kept.length > 0 && bytes + size > MAX_BYTES) break
+      kept.push(lines[i])
+      bytes += size
+    }
+    kept.reverse()
+    const next = kept.length > 0 ? `${kept.join('\n')}\n` : ''
+    if (next !== text) {
+      const tmp = `${path}.tmp`
+      await writeFile(tmp, next, 'utf8')
+      await rename(tmp, path)
+    }
+    return kept.length
+  }
   function record(entry) {
     const line = `${JSON.stringify(entry)}\n`
-    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => appendFile(path, line, 'utf8')))
-      .catch(error => rt.warn('ds-bot: a decision was not written: %s', error))
+    writing = writing.then(async () => {
+      await mkdir(dirname(path), { recursive: true })
+      await appendFile(path, line, 'utf8')
+      linesOnDisk = linesOnDisk === null ? await countLines() : linesOnDisk + 1
+      const { size } = await stat(path)
+      if (linesOnDisk > KEPT || size > MAX_BYTES) linesOnDisk = await trim()
+    }).catch(error => rt.warn('ds-bot: a decision was not written: %s', error))
     return writing
   }
-  // The entries, oldest first, capped to the newest `KEPT` lines. Reads only.
+  // The entries, oldest first, capped to the newest `KEPT` lines and `MAX_BYTES`. A read takes at
+  // most the byte cap from the end, so it does not grow with a file an older build left oversized.
   async function list() {
     try {
-      const text = await readFile(path, 'utf8')
+      let text
+      const { size } = await stat(path)
+      if (size > MAX_BYTES) {
+        const handle = await open(path, 'r')
+        try {
+          const buffer = Buffer.alloc(MAX_BYTES)
+          const { bytesRead } = await handle.read(buffer, 0, MAX_BYTES, size - MAX_BYTES)
+          text = buffer.toString('utf8', 0, bytesRead)
+          const firstNewline = text.indexOf('\n')
+          text = firstNewline === -1 ? '' : text.slice(firstNewline + 1)
+        } finally { await handle.close() }
+      } else {
+        text = await readFile(path, 'utf8')
+      }
       return text.split('\n').filter(Boolean).slice(-KEPT).map(line => { try { return JSON.parse(line) } catch { return undefined } }).filter(Boolean)
     } catch (error) {
       if (error?.code !== 'ENOENT') rt.warn('ds-bot: decisions could not be read: %s', error)

core/test/decisions.test.js

@@ -59,3 +59,28 @@ test('#396: with the flag off the tool is not registered and nothing is written'
   assert.equal(ctx.registeredTools.get('record_decision'), undefined)
   assert.equal(existsSync(join(home, 'decisions.jsonl')), false)
 })
+
+test('#603: record trims the log on disk, keeping the newest entries', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  for (let i = 0; i <= 2_000; i++) await tool('record_decision', { option: `decision ${i}`, choice: 'used' })
+  const text = readFileSync(join(home, 'decisions.jsonl'), 'utf8')
+  const log = text.trim().split('\n').map(line => JSON.parse(line))
+  assert.equal(log.length, 2_000)
+  assert.equal(log[0].option, 'decision 1')
+  assert.equal(log.at(-1).option, 'decision 2000')
+  assert.ok(Buffer.byteLength(text) < 1_000_000, `file is ${Buffer.byteLength(text)} bytes`)
+})
+
+test('#603: a byte cap bounds an oversized log left by an older build', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const big = 'x'.repeat(700_000)
+  writeFileSync(join(home, 'decisions.jsonl'), `${JSON.stringify({ option: big })}\n${JSON.stringify({ option: big })}\n`)
+  await tool('record_decision', { option: 'after the byte cap', choice: 'used' })
+  const text = readFileSync(join(home, 'decisions.jsonl'), 'utf8')
+  assert.ok(Buffer.byteLength(text) <= 1_000_000, `file is ${Buffer.byteLength(text)} bytes`)
+  const log = text.trim().split('\n').map(line => JSON.parse(line))
+  assert.equal(log.at(-1).option, 'after the byte cap')
+  assert.equal(log.length, 2)
+})

docs/subsystems/core-host.md

@@ -83,7 +83,7 @@ external backend or task-specific session mapping; see the
 
 ### Decisions a Bot records (#396)
 
-- `src/host/decisions.js` gives a Bot one small tool, `record_decision({ option, choice, why })` (`choice` is `used` or `rejected`), so a "considered and rejected" choice a reviewer cannot otherwise see is written down. The host appends one JSON line per decision to `<home>/decisions.jsonl` (capped at 2000 lines, one at a time; a write failure is a warning, never a turn failure) and fires the entry on the activity feed as `decision`. A Bot has no tool to edit or delete a past entry.
+- `src/host/decisions.js` gives a Bot one small tool, `record_decision({ option, choice, why })` (`choice` is `used` or `rejected`), so a "considered and rejected" choice a reviewer cannot otherwise see is written down. The host appends one JSON line per decision to `<home>/decisions.jsonl` (bounded on disk at the newest 2000 lines and 1 MB, trimmed by the host as it appends; a read parses at most 1 MB; one write at a time; a write failure is a warning, never a turn failure) and fires the entry on the activity feed as `decision`. A Bot has no tool to edit or delete a past entry.
 - **Opt-in** (`config.decisions`): with the flag off the tool is not registered, nothing is written and no event is fired, so a default install is unchanged. A key, token or password in the text is refused (`looksSecret`), so the log needs no secret. Tests: `core/test/decisions.test.js`.
 
 ### Tasks

patches/ds-bot/files/src/host/decisions.js

@@ -8,7 +8,7 @@
 // default install behaves exactly as before. The log is local text, one JSON line per entry; the
 // same entry is fired on the activity feed as `decision` so a consumer (Timeline, an audit view)
 // can show it without reading the file.
-import { appendFile, mkdir, readFile } from 'node:fs/promises'
+import { appendFile, mkdir, open, readFile, rename, stat, writeFile } from 'node:fs/promises'
 import { dirname, join } from 'node:path'
 import { defineTool } from '@deepseek-ai/dsh-tools'
 import { looksSecret } from './memory.js'
@@ -18,6 +18,9 @@ const OPTION_MAX = 200
 const WHY_MAX = 300
 const CHOICES = new Set(['used', 'rejected'])
 const KEPT = 2_000
+// The log is bounded on disk as well as in the answer: next to the line cap there is a byte cap,
+// so a burst of long entries cannot make the file (and every read) expensive.
+const MAX_BYTES = 1_000_000
 const NAME_SAFE = /^[\w-]{1,128}$/
 
 const oneLine = (text, max) => String(text ?? '').replace(/[\u0000-\u001f\u007f-\u009f\u2028\u2029]/g, ' ').replace(/\s+/g, ' ').trim().slice(0, max)
@@ -32,16 +35,63 @@ export function install(rt) {
   // One append at a time, and a failure is a warning, never a turn failure (the log is a side
   // record, not part of the Bot's work).
   let writing = Promise.resolve()
+  // Lines on disk after the last append/trim, so a record does not re-read the file every time
+  // (null until the first record counts once).
+  let linesOnDisk = null
+  const countLines = async () => (await readFile(path, 'utf8')).split('\n').filter(Boolean).length
+  // Keep the file bounded in place: the newest entries under both caps, written through a temp
+  // file so a reader never sees a half-trimmed log. The Bot side stays append-only; this is the
+  // host's own housekeeping.
+  async function trim() {
+    let text
+    try { text = await readFile(path, 'utf8') } catch (error) { if (error?.code === 'ENOENT') return 0; throw error }
+    const lines = text.split('\n').filter(Boolean)
+    const kept = []
+    let bytes = 0
+    for (let i = lines.length - 1; i >= 0 && kept.length < KEPT; i--) {
+      const size = Buffer.byteLength(lines[i], 'utf8') + 1
+      if (kept.length > 0 && bytes + size > MAX_BYTES) break
+      kept.push(lines[i])
+      bytes += size
+    }
+    kept.reverse()
+    const next = kept.length > 0 ? `${kept.join('\n')}\n` : ''
+    if (next !== text) {
+      const tmp = `${path}.tmp`
+      await writeFile(tmp, next, 'utf8')
+      await rename(tmp, path)
+    }
+    return kept.length
+  }
   function record(entry) {
     const line = `${JSON.stringify(entry)}\n`
-    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => appendFile(path, line, 'utf8')))
-      .catch(error => rt.warn('ds-bot: a decision was not written: %s', error))
+    writing = writing.then(async () => {
+      await mkdir(dirname(path), { recursive: true })
+      await appendFile(path, line, 'utf8')
+      linesOnDisk = linesOnDisk === null ? await countLines() : linesOnDisk + 1
+      const { size } = await stat(path)
+      if (linesOnDisk > KEPT || size > MAX_BYTES) linesOnDisk = await trim()
+    }).catch(error => rt.warn('ds-bot: a decision was not written: %s', error))
     return writing
   }
-  // The entries, oldest first, capped to the newest `KEPT` lines. Reads only.
+  // The entries, oldest first, capped to the newest `KEPT` lines and `MAX_BYTES`. A read takes at
+  // most the byte cap from the end, so it does not grow with a file an older build left oversized.
   async function list() {
     try {
-      const text = await readFile(path, 'utf8')
+      let text
+      const { size } = await stat(path)
+      if (size > MAX_BYTES) {
+        const handle = await open(path, 'r')
+        try {
+          const buffer = Buffer.alloc(MAX_BYTES)
+          const { bytesRead } = await handle.read(buffer, 0, MAX_BYTES, size - MAX_BYTES)
+          text = buffer.toString('utf8', 0, bytesRead)
+          const firstNewline = text.indexOf('\n')
+          text = firstNewline === -1 ? '' : text.slice(firstNewline + 1)
+        } finally { await handle.close() }
+      } else {
+        text = await readFile(path, 'utf8')
+      }
       return text.split('\n').filter(Boolean).slice(-KEPT).map(line => { try { return JSON.parse(line) } catch { return undefined } }).filter(Boolean)
     } catch (error) {
       if (error?.code !== 'ENOENT') rt.warn('ds-bot: decisions could not be read: %s', error)

patches/ds-bot/files/test/decisions.test.js

@@ -59,3 +59,28 @@ test('#396: with the flag off the tool is not registered and nothing is written'
   assert.equal(ctx.registeredTools.get('record_decision'), undefined)
   assert.equal(existsSync(join(home, 'decisions.jsonl')), false)
 })
+
+test('#603: record trims the log on disk, keeping the newest entries', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  for (let i = 0; i <= 2_000; i++) await tool('record_decision', { option: `decision ${i}`, choice: 'used' })
+  const text = readFileSync(join(home, 'decisions.jsonl'), 'utf8')
+  const log = text.trim().split('\n').map(line => JSON.parse(line))
+  assert.equal(log.length, 2_000)
+  assert.equal(log[0].option, 'decision 1')
+  assert.equal(log.at(-1).option, 'decision 2000')
+  assert.ok(Buffer.byteLength(text) < 1_000_000, `file is ${Buffer.byteLength(text)} bytes`)
+})
+
+test('#603: a byte cap bounds an oversized log left by an older build', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const big = 'x'.repeat(700_000)
+  writeFileSync(join(home, 'decisions.jsonl'), `${JSON.stringify({ option: big })}\n${JSON.stringify({ option: big })}\n`)
+  await tool('record_decision', { option: 'after the byte cap', choice: 'used' })
+  const text = readFileSync(join(home, 'decisions.jsonl'), 'utf8')
+  assert.ok(Buffer.byteLength(text) <= 1_000_000, `file is ${Buffer.byteLength(text)} bytes`)
+  const log = text.trim().split('\n').map(line => JSON.parse(line))
+  assert.equal(log.at(-1).option, 'after the byte cap')
+  assert.equal(log.length, 2)
+})