From 9a3645a844ec492260dc3fec83654f31680ba50d Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:33:20 +0800 Subject: [PATCH] host: a Clear that did not empty the leaf has no side effect (#2364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 0444 leaf lost its rotated copy to a clear that could not empty anything, kept answering rotated: 'none' while .1 was still on disk, and wiped the readable surface: entries() and logText() advanced `cleared` and `fresh` on entry, so the page read "not cleared" next to "0 recent errors" while every byte was still there. The readable surface now moves only with a leaf that really was truncated (plus the host that keeps no log file); rotated answers what this call did to .1 — kept when one is still there, whether the removal failed or the truncate failed first. The subsystem page and the changelog say the same. --- .../2364-diagnostics-truncate-silent.fixed.md | 1 + core/src/host/diagnostics.js | 58 ++++--- core/test/diagnostics.test.js | 143 +++++++++++++++++ docs/subsystems/core-host.md | 2 +- patches/ds-bot/0001-core-fixes.patch | 64 +++++--- patches/ds-bot/0007-version-and-tooling.patch | 145 +++++++++++++++++- 6 files changed, 371 insertions(+), 42 deletions(-) create mode 100644 changelog.d/2364-diagnostics-truncate-silent.fixed.md diff --git a/changelog.d/2364-diagnostics-truncate-silent.fixed.md b/changelog.d/2364-diagnostics-truncate-silent.fixed.md new file mode 100644 index 000000000..f877fd086 --- /dev/null +++ b/changelog.d/2364-diagnostics-truncate-silent.fixed.md @@ -0,0 +1 @@ +Settings → Feedback's **Clear** now says so when the log file cannot be truncated (a read-only `logs/diagnostics.jsonl`, e.g. after one `sudo` run): it prints one `[ds-bot] diagnostics log not truncated: ` line on stderr instead of leaving the window reading as cleared while every line stays on disk. A Clear that does not empty the leaf has no side effect at all: the truncate runs before the rotated copy is removed, so `logs/diagnostics.jsonl.1` keeps the only copy left of those lines, and the recent errors and the Download still list what is still on disk instead of reading "0 recent errors" next to "not cleared" ([#2364](https://github.com/ThinkFlowLab/FlyBot/issues/2364)). diff --git a/core/src/host/diagnostics.js b/core/src/host/diagnostics.js index be9acba11..a8a0677b7 100644 --- a/core/src/host/diagnostics.js +++ b/core/src/host/diagnostics.js @@ -282,45 +282,65 @@ export function install(rt) { } async function clear() { - cleared = Date.now() - fresh = [] // What the window's sentence is chosen from (#2360): the leaf is only `truncated` once the // truncate ran and the rotated copy only `removed` once its name is gone, so a page cannot say // "cleared" about a file it left alone. const result = { leaf: 'absent', rotated: 'none' } - if (!toFile) return result + if (!toFile) { cleared = Date.now(); fresh = []; return result } + const stamp = Date.now() // Clearing truncates the leaf, so it is the same question as the append: a link is not ours to // truncate, and the old file behind it stays exactly as it was. writing = writing.then(async () => { const kind = await fileKind(logPath) if (kind === 'other') { refuseLeaf(); result.leaf = 'kept'; return } - // Rotation moves the full log to `.1`, and the truncate alone left every rotated line - // on disk after the user asked for the error log to be cleared. That one name belongs to this - // log, so it goes in the same chain and behind the same guard. The removal keeps a try of its - // own: a copy that cannot be removed (a directory under that name, a folder without its write - // bit) must not reject the chain before the truncate runs, or the window says "cleared" while - // the leaf still holds every line. The line names the leftover, never the folder. + // `rotated` says what this call did to `.1`, so the probe runs before the truncate: + // when the leaf cannot be emptied nothing is removed, and a copy that is there answers `kept` + // rather than claiming there was none (#2364). const rotatedKind = await fileKind(`${logPath}.1`) if (rotatedKind !== 'absent') result.rotated = 'kept' - try { - await rm(`${logPath}.1`, { force: true }) - if (rotatedKind !== 'absent') result.rotated = 'removed' - } catch (error) { - console.error('[ds-bot] diagnostics log rotated copy not removed:', error?.code ?? clean(error?.message ?? String(error))) - } - // The truncate runs exactly as it did before #2360, a leaf that is not there yet included (the - // write lands a new empty file, as this line always did): only the answer is new. A folder - // that is not there makes the write fail the way it always did, and nothing is on disk to - // report as kept, so that answer stays `absent` rather than sending the page a refusal. + // The truncate runs first, exactly as it did before #2360, a leaf that is not there yet + // included (the write lands a new empty file, as this line always did): only the answer is new. + // A folder that is not there makes the write fail the way it always did, and nothing is on disk + // to report as kept, so that answer stays `absent` rather than sending the page a refusal. try { await writeFile(logPath, '', 'utf8') result.leaf = 'truncated' } catch (error) { result.leaf = error?.code === 'ENOENT' ? 'absent' : 'kept' + // The answer alone is not enough: it reaches the window, and the host log said nothing about + // a leaf left full. `append` names this same class of failure on one line, so clear does too; + // the line carries the code and never the folder, and the throw stays so the chain ends here. + console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) throw error } + // Rotation moves the full log to `.1`, and the truncate alone left every rotated line + // on disk after the user asked for the error log to be cleared. That one name belongs to this + // log, so it goes in the same chain and behind the same guard — and behind the truncate: a leaf + // that cannot be emptied leaves the copy where it is, because the lines it holds are the only + // copy of those lines that is left. The removal keeps a try of its own: a copy that cannot be + // removed (a directory under that name, a folder without its write bit) must not reject the + // chain, or the window says "cleared" while both names still hold every line. The line names + // the leftover, never the folder. + if (rotatedKind === 'absent') return + try { + await rm(`${logPath}.1`, { force: true }) + result.rotated = 'removed' + } catch (error) { + console.error('[ds-bot] diagnostics log rotated copy not removed:', error?.code ?? clean(error?.message ?? String(error))) + } }).catch(() => {}) await writing + // #2364 (P-530): a Clear that did not empty the leaf has no readable surface to clear either. + // `entries()` reads the old lines through `cleared` and the live ones out of `fresh`, so moving + // either one on a refusal wiped the list and the Download while every byte was still on disk — + // the page read "not cleared" next to "0 recent errors". Only a leaf that really was truncated + // moves them, plus the `!toFile` host above, which keeps no log file to begin with; every other + // answer, an `ENOENT` write included, leaves both exactly as they were. A line noted while the + // chain was running keeps its place, because it is newer than the request. + if (result.leaf === 'truncated') { + cleared = stamp + fresh = fresh.filter(entry => entry.time > stamp) + } return result } diff --git a/core/test/diagnostics.test.js b/core/test/diagnostics.test.js index 14175d315..3aa700fd3 100644 --- a/core/test/diagnostics.test.js +++ b/core/test/diagnostics.test.js @@ -788,3 +788,146 @@ test('#2360: a home with no log folder answers absent and leaves the disk as it assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'absent', rotated: 'none' }, 'nothing was there and nothing was written') assert.equal(existsSync(join(home, 'logs')), false, 'the truncate did not create the folder it never created before') }) + +// #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. #2360 gave the window an answer for that (`leaf: 'kept'`), but the +// host log said nothing at all about a leaf left full, while `append` names the same class of +// failure on one stderr line. Clear now names it too, on one line that carries the code and never +// the folder. Root ignores the mode. +test('#2364: a clear that cannot truncate the leaf says so', async (t) => { + if (process.getuid?.() === 0) { + t.skip('root truncates a leaf without its write bit') + return + } + const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + mkdirSync(join(home, 'logs'), { recursive: true }) + const logPath = join(home, 'logs', 'diagnostics.jsonl') + const rotated = `${logPath}.1` + const old = 'OLD LINE\n'.repeat(4) + writeFileSync(logPath, old) + writeFileSync(rotated, 'ROTATED\n'.repeat(8)) + + const stderr = [] + const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) + const rt = host(home) + chmodSync(logPath, 0o444) + + const result = await rt.diagnostics.clear() + quiet.mock.restore() + + assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'the answer names the leaf it kept and the copy it did not remove') + assert.equal(readFileSync(logPath, 'utf8'), old, 'the leaf keeps every line it had') + assert.equal(readFileSync(rotated, 'utf8'), 'ROTATED\n'.repeat(8), 'the rotated copy is left where it was') + const noted = stderr.filter(line => /not truncated/.test(line)) + assert.equal(noted.length, 1, 'one line says the leaf was not truncated') + assert.match(noted[0], /\[ds-bot\] diagnostics log not truncated: [A-Z]+$/) + assert.equal(noted[0].includes(home), false, 'the line does not print the home folder') + + // The control: a writable leaf truncates with no line at all. + chmodSync(logPath, 0o644) + const control = [] + const quietControl = t.mock.method(console, 'error', (...args) => control.push(args.join(' '))) + writeFileSync(logPath, old) + writeFileSync(rotated, 'ROTATED\n') + await rt.diagnostics.clear() + quietControl.mock.restore() + assert.equal(readFileSync(logPath, 'utf8'), '', 'a writable leaf is truncated') + assert.equal(existsSync(rotated), false, 'and its rotated copy is removed') + assert.deepEqual(control, [], 'a clear that works says nothing') +}) + +// #2364 (P-528): the removal ran first, so the same `0444` leaf lost its rotated copy to a Clear +// that could not empty anything — the lines that had rolled over were the only copy left of them, +// and neither the answer nor the page mentioned it. The truncate now runs first: a leaf that stays +// full leaves `.1` byte for byte where it was, answers `rotated: 'none'`, and the one line it prints +// is the truncate failure. The rotated-copy line belongs to a leaf that was emptied. Root ignores +// the mode. +test('#2364: a clear that cannot truncate the leaf keeps the rotated copy', async (t) => { + if (process.getuid?.() === 0) { + t.skip('root truncates a leaf without its write bit') + return + } + const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + mkdirSync(join(home, 'logs'), { recursive: true }) + const logPath = join(home, 'logs', 'diagnostics.jsonl') + const rotated = `${logPath}.1` + const old = 'OLD LINE\n'.repeat(4) + const rolled = 'ROTATED\n'.repeat(8) + writeFileSync(logPath, old) + writeFileSync(rotated, rolled) + chmodSync(logPath, 0o444) + + const stderr = [] + const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) + const rt = host(home) + const result = await rt.diagnostics.clear() + quiet.mock.restore() + + assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'a leaf that could not be emptied removed nothing') + assert.equal(readFileSync(logPath, 'utf8'), old, 'the leaf keeps every line it had') + assert.equal(readFileSync(rotated, 'utf8'), rolled, 'the rotated copy keeps every byte it had') + assert.equal(existsSync(rotated), true, 'the name is still there to download') + assert.equal(stderr.filter(line => /rotated copy/.test(line)).length, 0, 'nothing was removed, so no line claims a failed removal') + const noted = stderr.filter(line => /not truncated/.test(line)) + assert.equal(noted.length, 1, 'one line says the leaf was not truncated') + assert.match(noted[0], /\[ds-bot\] diagnostics log not truncated: [A-Z]+$/) + + // The control: once the leaf can be written, the same clear empties it and removes the copy. + chmodSync(logPath, 0o644) + writeFileSync(logPath, old) + const control = [] + const quietControl = t.mock.method(console, 'error', (...args) => control.push(args.join(' '))) + assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'truncated', rotated: 'removed' }) + quietControl.mock.restore() + assert.equal(readFileSync(logPath, 'utf8'), '', 'the writable leaf is truncated') + assert.equal(existsSync(rotated), false, 'and its rotated copy is removed') + assert.deepEqual(control, [], 'a clear that works says nothing') +}) + +// #2364 (P-530): a Clear that could not empty the leaf still wiped what the window reads. `entries()` +// is the old lines through `cleared` plus the live ones in `fresh`, and clear() advanced both the +// moment it was called, so a refused Clear answered "not cleared" on one line and "0 recent errors" / +// "No entries." next to it while every byte was still on disk and in the Download. Only a leaf that +// really was emptied, or nothing on disk at all, moves the readable surface now. Root ignores the +// mode. +test('#2364: a clear that cannot truncate leaves the readable surface alone', async (t) => { + if (process.getuid?.() === 0) { + t.skip('root truncates a leaf without its write bit') + return + } + const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + mkdirSync(join(home, 'logs'), { recursive: true }) + const logPath = join(home, 'logs', 'diagnostics.jsonl') + const rotated = `${logPath}.1` + // One line from an earlier run (read through `cleared`) and one from this one (in `fresh`). + writeFileSync(logPath, `${JSON.stringify({ time: Date.now() - 60_000, level: 'error', source: 'host', text: 'an earlier run line' })}\n`) + writeFileSync(rotated, 'ROTATED\n') + const rt = host(home) + rt.warn('ds-bot: written before the clear') + await rt.dispose() + chmodSync(logPath, 0o444) + + const stderr = [] + const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) + const result = await rt.diagnostics.clear() + quiet.mock.restore() + + assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'the answer names the leaf it kept') + const kept = await rt.diagnostics.entries() + assert.equal(kept.length, 2, 'both the earlier line and the live one are still listed') + assert.equal(kept.some(entry => entry.text === 'an earlier run line'), true, 'the line from an earlier run is still there') + assert.equal(kept.some(entry => /written before the clear/.test(entry.text)), true, 'the live line is still there') + const download = await rt.diagnostics.logText() + assert.match(download, /an earlier run line/, 'the Download still carries the earlier line') + assert.match(download, /written before the clear/, 'the Download still carries the live line') + assert.equal(download.includes('No entries.'), false, 'the Download is not the empty page either') + + // The control: a Clear that empties the leaf empties the readable surface too. + chmodSync(logPath, 0o644) + assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'truncated', rotated: 'removed' }) + assert.deepEqual(await rt.diagnostics.entries(), [], 'a real clear lists nothing afterwards') + assert.equal(await rt.diagnostics.logText(), '', 'a real clear leaves nothing to download') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 742fa630e..257adbaa3 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -406,7 +406,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 removal keeps a `try` of its own, and the truncate runs whether or not it succeeded: 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. `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)). A leaf the guard refuses answers `rotated: 'none'`: the removal is behind that guard, so the call never reaches it. This is an answer and nothing else: the truncate, the `fileKind` guard, the removal rule and the `try` have the same 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 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. ### 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 f651d5c4a..dc6fa9bb1 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -5124,7 +5124,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js const list = await entries() const client = sanitizeClient(body.client) const clientEntries = sanitizeClientEntries(body.clientErrors) -@@ -232,7 +232,7 @@ export function install(rt) { +@@ -232,17 +232,72 @@ export function install(rt) { } async function logText() { @@ -5133,49 +5133,71 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js const list = await entries() return list.map(entry => (entry.stack ? `${entryLine(entry)}\n${entry.stack.replace(/^/gm, ' ')}` : entryLine(entry))).join('\n') } -@@ -240,9 +240,44 @@ export function install(rt) { + async function clear() { - cleared = Date.now() - fresh = [] +- cleared = Date.now() +- fresh = [] - if (!toFile) return - writing = writing.then(() => writeFile(logPath, '', 'utf8')).catch(() => {}) + // What the window's sentence is chosen from (#2360): the leaf is only `truncated` once the + // truncate ran and the rotated copy only `removed` once its name is gone, so a page cannot say + // "cleared" about a file it left alone. + const result = { leaf: 'absent', rotated: 'none' } -+ if (!toFile) return result ++ if (!toFile) { cleared = Date.now(); fresh = []; return result } ++ const stamp = Date.now() + // Clearing truncates the leaf, so it is the same question as the append: a link is not ours to + // truncate, and the old file behind it stays exactly as it was. + writing = writing.then(async () => { + const kind = await fileKind(logPath) + if (kind === 'other') { refuseLeaf(); result.leaf = 'kept'; return } -+ // Rotation moves the full log to `.1`, and the truncate alone left every rotated line -+ // on disk after the user asked for the error log to be cleared. That one name belongs to this -+ // log, so it goes in the same chain and behind the same guard. The removal keeps a try of its -+ // own: a copy that cannot be removed (a directory under that name, a folder without its write -+ // bit) must not reject the chain before the truncate runs, or the window says "cleared" while -+ // the leaf still holds every line. The line names the leftover, never the folder. ++ // `rotated` says what this call did to `.1`, so the probe runs before the truncate: ++ // when the leaf cannot be emptied nothing is removed, and a copy that is there answers `kept` ++ // rather than claiming there was none (#2364). + const rotatedKind = await fileKind(`${logPath}.1`) + if (rotatedKind !== 'absent') result.rotated = 'kept' -+ try { -+ await rm(`${logPath}.1`, { force: true }) -+ if (rotatedKind !== 'absent') result.rotated = 'removed' -+ } catch (error) { -+ console.error('[ds-bot] diagnostics log rotated copy not removed:', error?.code ?? clean(error?.message ?? String(error))) -+ } -+ // The truncate runs exactly as it did before #2360, a leaf that is not there yet included (the -+ // write lands a new empty file, as this line always did): only the answer is new. A folder -+ // that is not there makes the write fail the way it always did, and nothing is on disk to -+ // report as kept, so that answer stays `absent` rather than sending the page a refusal. ++ // The truncate runs first, exactly as it did before #2360, a leaf that is not there yet ++ // included (the write lands a new empty file, as this line always did): only the answer is new. ++ // A folder that is not there makes the write fail the way it always did, and nothing is on disk ++ // to report as kept, so that answer stays `absent` rather than sending the page a refusal. + try { + await writeFile(logPath, '', 'utf8') + result.leaf = 'truncated' + } catch (error) { + result.leaf = error?.code === 'ENOENT' ? 'absent' : 'kept' ++ // The answer alone is not enough: it reaches the window, and the host log said nothing about ++ // a leaf left full. `append` names this same class of failure on one line, so clear does too; ++ // the line carries the code and never the folder, and the throw stays so the chain ends here. ++ console.error('[ds-bot] diagnostics log not truncated:', error?.code ?? clean(error?.message ?? String(error))) + throw error + } ++ // Rotation moves the full log to `.1`, and the truncate alone left every rotated line ++ // on disk after the user asked for the error log to be cleared. That one name belongs to this ++ // log, so it goes in the same chain and behind the same guard — and behind the truncate: a leaf ++ // that cannot be emptied leaves the copy where it is, because the lines it holds are the only ++ // copy of those lines that is left. The removal keeps a try of its own: a copy that cannot be ++ // removed (a directory under that name, a folder without its write bit) must not reject the ++ // chain, or the window says "cleared" while both names still hold every line. The line names ++ // the leftover, never the folder. ++ if (rotatedKind === 'absent') return ++ try { ++ await rm(`${logPath}.1`, { force: true }) ++ result.rotated = 'removed' ++ } catch (error) { ++ console.error('[ds-bot] diagnostics log rotated copy not removed:', error?.code ?? clean(error?.message ?? String(error))) ++ } + }).catch(() => {}) await writing ++ // #2364 (P-530): a Clear that did not empty the leaf has no readable surface to clear either. ++ // `entries()` reads the old lines through `cleared` and the live ones out of `fresh`, so moving ++ // either one on a refusal wiped the list and the Download while every byte was still on disk — ++ // the page read "not cleared" next to "0 recent errors". Only a leaf that really was truncated ++ // moves them, plus the `!toFile` host above, which keeps no log file to begin with; every other ++ // answer, an `ENOENT` write included, leaves both exactly as they were. A line noted while the ++ // chain was running keeps its place, because it is newer than the request. ++ if (result.leaf === 'truncated') { ++ cleared = stamp ++ fresh = fresh.filter(entry => entry.time > stamp) ++ } + return result } diff --git a/patches/ds-bot/0007-version-and-tooling.patch b/patches/ds-bot/0007-version-and-tooling.patch index 5d2cfbcb6..2291dc809 100644 --- a/patches/ds-bot/0007-version-and-tooling.patch +++ b/patches/ds-bot/0007-version-and-tooling.patch @@ -1175,7 +1175,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js const short = issueUrl(base, { what: 'Writer failed\nin the group', diagnostics: '| DS Bot | 0.1.0 |' }) assert.equal(short.inline, true) const params = new URL(short.url).searchParams -@@ -253,3 +253,258 @@ test('a browser error is noted once, repeats are counted, and clear empties the +@@ -253,3 +253,401 @@ test('a browser error is noted once, repeats are counted, and clear empties the clearClientErrors() assert.equal(clientErrors().length, 0) }) @@ -1434,3 +1434,146 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js + assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'absent', rotated: 'none' }, 'nothing was there and nothing was written') + assert.equal(existsSync(join(home, 'logs')), false, 'the truncate did not create the folder it never created before') +}) ++ ++// #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. #2360 gave the window an answer for that (`leaf: 'kept'`), but the ++// host log said nothing at all about a leaf left full, while `append` names the same class of ++// failure on one stderr line. Clear now names it too, on one line that carries the code and never ++// the folder. Root ignores the mode. ++test('#2364: a clear that cannot truncate the leaf says so', async (t) => { ++ if (process.getuid?.() === 0) { ++ t.skip('root truncates a leaf without its write bit') ++ return ++ } ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) ++ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) ++ mkdirSync(join(home, 'logs'), { recursive: true }) ++ const logPath = join(home, 'logs', 'diagnostics.jsonl') ++ const rotated = `${logPath}.1` ++ const old = 'OLD LINE\n'.repeat(4) ++ writeFileSync(logPath, old) ++ writeFileSync(rotated, 'ROTATED\n'.repeat(8)) ++ ++ const stderr = [] ++ const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) ++ const rt = host(home) ++ chmodSync(logPath, 0o444) ++ ++ const result = await rt.diagnostics.clear() ++ quiet.mock.restore() ++ ++ assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'the answer names the leaf it kept and the copy it did not remove') ++ assert.equal(readFileSync(logPath, 'utf8'), old, 'the leaf keeps every line it had') ++ assert.equal(readFileSync(rotated, 'utf8'), 'ROTATED\n'.repeat(8), 'the rotated copy is left where it was') ++ const noted = stderr.filter(line => /not truncated/.test(line)) ++ assert.equal(noted.length, 1, 'one line says the leaf was not truncated') ++ assert.match(noted[0], /\[ds-bot\] diagnostics log not truncated: [A-Z]+$/) ++ assert.equal(noted[0].includes(home), false, 'the line does not print the home folder') ++ ++ // The control: a writable leaf truncates with no line at all. ++ chmodSync(logPath, 0o644) ++ const control = [] ++ const quietControl = t.mock.method(console, 'error', (...args) => control.push(args.join(' '))) ++ writeFileSync(logPath, old) ++ writeFileSync(rotated, 'ROTATED\n') ++ await rt.diagnostics.clear() ++ quietControl.mock.restore() ++ assert.equal(readFileSync(logPath, 'utf8'), '', 'a writable leaf is truncated') ++ assert.equal(existsSync(rotated), false, 'and its rotated copy is removed') ++ assert.deepEqual(control, [], 'a clear that works says nothing') ++}) ++ ++// #2364 (P-528): the removal ran first, so the same `0444` leaf lost its rotated copy to a Clear ++// that could not empty anything — the lines that had rolled over were the only copy left of them, ++// and neither the answer nor the page mentioned it. The truncate now runs first: a leaf that stays ++// full leaves `.1` byte for byte where it was, answers `rotated: 'none'`, and the one line it prints ++// is the truncate failure. The rotated-copy line belongs to a leaf that was emptied. Root ignores ++// the mode. ++test('#2364: a clear that cannot truncate the leaf keeps the rotated copy', async (t) => { ++ if (process.getuid?.() === 0) { ++ t.skip('root truncates a leaf without its write bit') ++ return ++ } ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) ++ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) ++ mkdirSync(join(home, 'logs'), { recursive: true }) ++ const logPath = join(home, 'logs', 'diagnostics.jsonl') ++ const rotated = `${logPath}.1` ++ const old = 'OLD LINE\n'.repeat(4) ++ const rolled = 'ROTATED\n'.repeat(8) ++ writeFileSync(logPath, old) ++ writeFileSync(rotated, rolled) ++ chmodSync(logPath, 0o444) ++ ++ const stderr = [] ++ const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) ++ const rt = host(home) ++ const result = await rt.diagnostics.clear() ++ quiet.mock.restore() ++ ++ assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'a leaf that could not be emptied removed nothing') ++ assert.equal(readFileSync(logPath, 'utf8'), old, 'the leaf keeps every line it had') ++ assert.equal(readFileSync(rotated, 'utf8'), rolled, 'the rotated copy keeps every byte it had') ++ assert.equal(existsSync(rotated), true, 'the name is still there to download') ++ assert.equal(stderr.filter(line => /rotated copy/.test(line)).length, 0, 'nothing was removed, so no line claims a failed removal') ++ const noted = stderr.filter(line => /not truncated/.test(line)) ++ assert.equal(noted.length, 1, 'one line says the leaf was not truncated') ++ assert.match(noted[0], /\[ds-bot\] diagnostics log not truncated: [A-Z]+$/) ++ ++ // The control: once the leaf can be written, the same clear empties it and removes the copy. ++ chmodSync(logPath, 0o644) ++ writeFileSync(logPath, old) ++ const control = [] ++ const quietControl = t.mock.method(console, 'error', (...args) => control.push(args.join(' '))) ++ assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'truncated', rotated: 'removed' }) ++ quietControl.mock.restore() ++ assert.equal(readFileSync(logPath, 'utf8'), '', 'the writable leaf is truncated') ++ assert.equal(existsSync(rotated), false, 'and its rotated copy is removed') ++ assert.deepEqual(control, [], 'a clear that works says nothing') ++}) ++ ++// #2364 (P-530): a Clear that could not empty the leaf still wiped what the window reads. `entries()` ++// is the old lines through `cleared` plus the live ones in `fresh`, and clear() advanced both the ++// moment it was called, so a refused Clear answered "not cleared" on one line and "0 recent errors" / ++// "No entries." next to it while every byte was still on disk and in the Download. Only a leaf that ++// really was emptied, or nothing on disk at all, moves the readable surface now. Root ignores the ++// mode. ++test('#2364: a clear that cannot truncate leaves the readable surface alone', async (t) => { ++ if (process.getuid?.() === 0) { ++ t.skip('root truncates a leaf without its write bit') ++ return ++ } ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) ++ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) ++ mkdirSync(join(home, 'logs'), { recursive: true }) ++ const logPath = join(home, 'logs', 'diagnostics.jsonl') ++ const rotated = `${logPath}.1` ++ // One line from an earlier run (read through `cleared`) and one from this one (in `fresh`). ++ writeFileSync(logPath, `${JSON.stringify({ time: Date.now() - 60_000, level: 'error', source: 'host', text: 'an earlier run line' })}\n`) ++ writeFileSync(rotated, 'ROTATED\n') ++ const rt = host(home) ++ rt.warn('ds-bot: written before the clear') ++ await rt.dispose() ++ chmodSync(logPath, 0o444) ++ ++ const stderr = [] ++ const quiet = t.mock.method(console, 'error', (...args) => stderr.push(args.join(' '))) ++ const result = await rt.diagnostics.clear() ++ quiet.mock.restore() ++ ++ assert.deepEqual(result, { leaf: 'kept', rotated: 'kept' }, 'the answer names the leaf it kept') ++ const kept = await rt.diagnostics.entries() ++ assert.equal(kept.length, 2, 'both the earlier line and the live one are still listed') ++ assert.equal(kept.some(entry => entry.text === 'an earlier run line'), true, 'the line from an earlier run is still there') ++ assert.equal(kept.some(entry => /written before the clear/.test(entry.text)), true, 'the live line is still there') ++ const download = await rt.diagnostics.logText() ++ assert.match(download, /an earlier run line/, 'the Download still carries the earlier line') ++ assert.match(download, /written before the clear/, 'the Download still carries the live line') ++ assert.equal(download.includes('No entries.'), false, 'the Download is not the empty page either') ++ ++ // The control: a Clear that empties the leaf empties the readable surface too. ++ chmodSync(logPath, 0o644) ++ assert.deepEqual(await rt.diagnostics.clear(), { leaf: 'truncated', rotated: 'removed' }) ++ assert.deepEqual(await rt.diagnostics.entries(), [], 'a real clear lists nothing afterwards') ++ assert.equal(await rt.diagnostics.logText(), '', 'a real clear leaves nothing to download') ++})