Fix: decisions.jsonl never trims (#603)

changelog.d/603.fixed.md

@@ -0,0 +1 @@
+Decisions a Bot records no longer grow without bound: `decisions.jsonl` is trimmed on every write to the newest 2000 lines and at most 1 MB, so a home that records for a long time keeps a bounded file and a read that no longer parses the whole history (#603). The Bot side stays append-only; the trim is the host's.

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, readFile, rename, 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
+// One line can be ~600 bytes, so a line cap alone would still let the file grow; the byte cap
+// bounds it even when every entry is the longest allowed (#603).
+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)
@@ -29,16 +32,41 @@ export function install(rt) {
   }
   const { home } = rt
   const path = join(home, 'decisions.jsonl')
+
+  // The file is bounded on disk, not only in the answer (#603): trim before each append, so a
+  // reader never pays for a file that only ever grew. The rewrite is atomic (temp file + rename),
+  // so a crash cannot leave half a log, and it is one more append step, never a Bot-side action.
+  async function trim() {
+    let text
+    try {
+      text = await readFile(path, 'utf8')
+    } catch (error) {
+      if (error?.code === 'ENOENT') return
+      throw error
+    }
+    const lines = text.split('\n').filter(Boolean)
+    if (lines.length < KEPT && Buffer.byteLength(text) <= MAX_BYTES) return
+    const asText = list => (list.length === 0 ? '' : `${list.join('\n')}\n`)
+    // One slot is left for the line about to be appended, so the file ends at KEPT, not KEPT + 1.
+    let kept = lines.slice(-(KEPT - 1))
+    // An entry too big to keep at all is dropped rather than kept and over the cap: the cap wins.
+    while (kept.length > 0 && Buffer.byteLength(asText(kept)) > MAX_BYTES) kept = kept.slice(1)
+    const tmp = `${path}.trim`
+    await writeFile(tmp, asText(kept), 'utf8')
+    await rename(tmp, path)
+  }
+
   // 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()
   function record(entry) {
     const line = `${JSON.stringify(entry)}\n`
-    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => appendFile(path, line, 'utf8')))
+    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => trim()).then(() => appendFile(path, line, 'utf8')))
       .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. Reads only; with the file trimmed
+  // on write this is O(KEPT), not O(file).
   async function list() {
     try {
       const text = await readFile(path, 'utf8')

core/test/decisions.test.js

@@ -1,5 +1,5 @@
 import assert from 'node:assert/strict'
-import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
@@ -59,3 +59,33 @@ 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)
 })
+
+// The cap `src/host/decisions.js` documents. Kept as literals so this test fails when the
+// implementation drops the bound, rather than following it wherever it goes.
+const KEPT = 2_000
+const MAX_BYTES = 1_000_000
+
+const seeded = (count) => Array.from({ length: count }, (_, index) => JSON.stringify({
+  time: index, bot: 'a', botName: 'Chief', option: `old ${index}`, choice: 'used',
+})).join('\n')
+
+test('#603: the log is trimmed on disk, so it cannot grow without bound', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const path = join(home, 'decisions.jsonl')
+  writeFileSync(path, `${seeded(KEPT)}\n`)
+  await tool('record_decision', { option: 'newest', choice: 'rejected' })
+  const lines = readFileSync(path, 'utf8').trim().split('\n')
+  assert.ok(lines.length <= KEPT, `decisions.jsonl holds ${lines.length} lines; the cap is ${KEPT}`)
+  assert.equal(JSON.parse(lines.at(-1)).option, 'newest')
+  assert.notEqual(JSON.parse(lines[0]).option, 'old 0', 'the oldest entry should have been trimmed')
+})
+
+test('#603: one huge entry cannot blow the byte cap', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const path = join(home, 'decisions.jsonl')
+  writeFileSync(path, `${JSON.stringify({ time: 1, bot: 'a', botName: 'Chief', option: 'x'.repeat(1_100_000), choice: 'used' })}\n`)
+  await tool('record_decision', { option: 'after the huge entry', choice: 'used' })
+  assert.ok(statSync(path).size <= MAX_BYTES, `decisions.jsonl is ${statSync(path).size} bytes; the cap is ${MAX_BYTES}`)
+})

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, readFile, rename, 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
+// One line can be ~600 bytes, so a line cap alone would still let the file grow; the byte cap
+// bounds it even when every entry is the longest allowed (#603).
+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)
@@ -29,16 +32,41 @@ export function install(rt) {
   }
   const { home } = rt
   const path = join(home, 'decisions.jsonl')
+
+  // The file is bounded on disk, not only in the answer (#603): trim before each append, so a
+  // reader never pays for a file that only ever grew. The rewrite is atomic (temp file + rename),
+  // so a crash cannot leave half a log, and it is one more append step, never a Bot-side action.
+  async function trim() {
+    let text
+    try {
+      text = await readFile(path, 'utf8')
+    } catch (error) {
+      if (error?.code === 'ENOENT') return
+      throw error
+    }
+    const lines = text.split('\n').filter(Boolean)
+    if (lines.length < KEPT && Buffer.byteLength(text) <= MAX_BYTES) return
+    const asText = list => (list.length === 0 ? '' : `${list.join('\n')}\n`)
+    // One slot is left for the line about to be appended, so the file ends at KEPT, not KEPT + 1.
+    let kept = lines.slice(-(KEPT - 1))
+    // An entry too big to keep at all is dropped rather than kept and over the cap: the cap wins.
+    while (kept.length > 0 && Buffer.byteLength(asText(kept)) > MAX_BYTES) kept = kept.slice(1)
+    const tmp = `${path}.trim`
+    await writeFile(tmp, asText(kept), 'utf8')
+    await rename(tmp, path)
+  }
+
   // 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()
   function record(entry) {
     const line = `${JSON.stringify(entry)}\n`
-    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => appendFile(path, line, 'utf8')))
+    writing = writing.then(() => mkdir(dirname(path), { recursive: true }).then(() => trim()).then(() => appendFile(path, line, 'utf8')))
       .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. Reads only; with the file trimmed
+  // on write this is O(KEPT), not O(file).
   async function list() {
     try {
       const text = await readFile(path, 'utf8')

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

@@ -1,5 +1,5 @@
 import assert from 'node:assert/strict'
-import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
@@ -59,3 +59,33 @@ 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)
 })
+
+// The cap `src/host/decisions.js` documents. Kept as literals so this test fails when the
+// implementation drops the bound, rather than following it wherever it goes.
+const KEPT = 2_000
+const MAX_BYTES = 1_000_000
+
+const seeded = (count) => Array.from({ length: count }, (_, index) => JSON.stringify({
+  time: index, bot: 'a', botName: 'Chief', option: `old ${index}`, choice: 'used',
+})).join('\n')
+
+test('#603: the log is trimmed on disk, so it cannot grow without bound', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const path = join(home, 'decisions.jsonl')
+  writeFileSync(path, `${seeded(KEPT)}\n`)
+  await tool('record_decision', { option: 'newest', choice: 'rejected' })
+  const lines = readFileSync(path, 'utf8').trim().split('\n')
+  assert.ok(lines.length <= KEPT, `decisions.jsonl holds ${lines.length} lines; the cap is ${KEPT}`)
+  assert.equal(JSON.parse(lines.at(-1)).option, 'newest')
+  assert.notEqual(JSON.parse(lines[0]).option, 'old 0', 'the oldest entry should have been trimmed')
+})
+
+test('#603: one huge entry cannot blow the byte cap', async (t) => {
+  const { home, tool, stop } = await boot({ decisions: true })
+  t.after(stop)
+  const path = join(home, 'decisions.jsonl')
+  writeFileSync(path, `${JSON.stringify({ time: 1, bot: 'a', botName: 'Chief', option: 'x'.repeat(1_100_000), choice: 'used' })}\n`)
+  await tool('record_decision', { option: 'after the huge entry', choice: 'used' })
+  assert.ok(statSync(path).size <= MAX_BYTES, `decisions.jsonl is ${statSync(path).size} bytes; the cap is ${MAX_BYTES}`)
+})