Skip to content

host: a Clear that cannot remove the rotated log leaves the leaf untouched and silent - #2361

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-p523-clear-rm-guard
Nov 4, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-p523-clear-rm-guard

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2358

Owner: Dev-2
Reviewer: Birfy
Claim-Id: 7d328f20e3084f528c4b2b09cdd1d4c9
Branch: dev/2-p523-clear-rm-guard
Base: 8c58f3a
Tip: e3b27c0
Tree: 30425d27e18c2ce81f5e81edf56e2643edbed913
Diff: 6 files, +182/−9
Tier: T3
why: ["core/test/diagnostics.test.js: /\.(only|skip)\(/"]
basis: core/src/host/diagnostics.js=T2, core/test/diagnostics.test.js=T1, changelog.d/2358-diagnostics-clear-rm.fixed.md=T1, patches/ds-bot/0001-core-fixes.patch=T1, patches/ds-bot/0007-version-and-tooling.patch=T1, docs/subsystems/core-host.md=T0
Docs-changed: docs/subsystems/core-host.md:409 — the Clear sentence now says the removal keeps a try of its own and the truncate runs whether or not it succeeded, so the page's "cleared" matches the leaf again.

What changed

clear() removed the rotated copy and truncated the leaf in one write chain whose only handler was an empty .catch(() => {}). The rm had no try of its own, so a copy the host could not remove — a directory under that name (EISDIR), a folder without its write bit (EACCES), a busy file — rejected the chain on that line and the writeFile one line below never ran. clear() had already set cleared/fresh before the chain, so the window showed "No errors recorded." and "Cleared the error log." while the leaf still held every line, and nothing reached stderr: the leaf guard above had already passed, so refuseLeaf() did not fire.

  • The rm now has a try of its own. A failure writes one console.error line that names the error code and never the folder ([ds-bot] diagnostics log rotated copy not removed: EACCES), and the truncate runs as before, whether or not the removal succeeded. The leftover bytes stay; the log the user asked to clear is empty.
  • The fileKind guard keeps its place, its logic and its order: a leaf that is a link is still refused whole and nothing is truncated. clear() keeps its signature and return value; 0008 and core/client.js are untouched.

After: .1 that cannot be removed is still there, is now named on stderr, and the leaf is empty — a Clear that says it cleared no longer leaves every line on disk. The normal path is unchanged: rm succeeds, one truncation, .1 gone, no stderr line.

Test first

Two deterministic shapes added to core/test/diagnostics.test.js, both with a writable leaf so the only failure is the removal:

  • #2358: clear truncates the leaf even when the rotated copy cannot be removed — logs/diagnostics.jsonl.1 is a directory, so rm without recursive raises ERR_FS_EISDIR.
  • #2358: clear truncates the leaf when the folder refuses the removal — logs/ is 0555 and the leaf is 0644, so the removal raises EACCES; skipped under root with t.skip, where 0555 denies nothing, and EISDIR needs no such guard.

Both assert the leaf reads back '', .1 is still present (the directory; the file with its bytes unchanged), entries() is empty, and stderr carries exactly one line that does not contain the home folder.

On the clean base with only these tests in the tree: node --test core/test/diagnostics.test.js → 30 tests / 28 pass / 2 fail / 0 cancelled; both fail on assert.strictEqual(readFileSync(logPath, 'utf8'), '') — the leaf is truncated although the rotated copy stays and the leaf is truncated although the folder refused the removal, actual 'OLD LINE\n'×4 and the 4-line warning record. The untruncated leaf in shape B also proves the EACCES arms before the truncate, as expected. With the fix: 30 / 30 / 0.

The "no .1 at all" path stays green throughout — rm(force: true) swallows ENOENT, and the existing clear removes the rotated log, not only the leaf covers it.

Generated

patches/ds-bot/0001-core-fixes.patch and 0007-version-and-tooling.patch are the mechanical node scripts/ds-bot-patches.mjs generate output for the two edited files (src/host/diagnostics.js, test/diagnostics.test.js). The product change is core/src/host/diagnostics.js plus the test; nothing else in those patches. generate is a no-op on this tree, check reports 10 patches applying cleanly with core/ equal to ds-bot/ + patches, canonical reports generate is a no-op, and build-client.mjs --check exits 0. 0008 and core/client.js were never in git status on this branch.

The tier is T3 on the content rule \.(only|skip)\( matching the root guard's t.skip( line, not on the product change (diagnostics.js is T2 on its own); the t.skip stays as Node's per-test option form would only be a way around the pattern, not a fix, and CI's runner is not root.

Ran

node --test core/test/diagnostics.test.js
(cd core && TZ=UTC npm test)
node scripts/ds-bot-patches.mjs generate
node scripts/ds-bot-patches.mjs check
node scripts/ds-bot-patches.mjs canonical
node core/scripts/build-client.mjs --check
node .github/scripts/changelog.mjs check
TZ=UTC ARCH_DOCS_BASE=8c58f3a1c7a6511abcd2816aa545dded7ab4b99d npm run verify

verify at the committed tip: core lint 74 warnings / 0 errors, plugin lint 36 / 9 / 2 / 16 warnings and 0 errors, scripts lint 1 / 0, arch-docs: OK (6 changed file(s)), and suites 2767 + 570 + 132 + 140 + 411 + 671 + 37 + 26 = 4754 pass / 0 fail / 0 cancelled.

Not run: the smoke test and the CI package job's pack check, typecheck and actionlint, which npm run verify does not run.

Checkpoint

{
  "pr": 2361,
  "claim": "Dev-2",
  "issue": 2358,
  "summary": "clear() removed the rotated copy inside the write chain but outside a try of its own, so a copy the host could not remove rejected the chain before the truncate ran, and the empty catch kept it quiet: the window said the log was cleared while the leaf held every line. The removal now keeps a try, names the leftover on one stderr line that carries no folder, and the truncate runs whether or not it succeeded. The fileKind guard, clear()'s signature and the removal rule are unchanged.",
  "files": [
    "core/src/host/diagnostics.js",
    "core/test/diagnostics.test.js",
    "changelog.d/2358-diagnostics-clear-rm.fixed.md",
    "docs/subsystems/core-host.md",
    "patches/ds-bot/0001-core-fixes.patch",
    "patches/ds-bot/0007-version-and-tooling.patch"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=8c58f3a1c7a6511abcd2816aa545dded7ab4b99d npm run verify",
    "result": "green",
    "at": "e3b27c0075afe3d96ebf8e6a55f797ab05a2a5ac"
  },
  "next": "none \u2014 ready for review"
}

Clear removed logs/diagnostics.jsonl.1 inside the write chain but outside a try of its own, so a copy the host could not remove - a directory under that name (EISDIR), a folder without its write bit (EACCES) - rejected the chain before the truncate ran, and the trailing empty catch kept it quiet: the window answered "Cleared the error log." while the leaf still held every line. The removal keeps a try of its own now, names the leftover on one stderr line that carries no folder, and the truncate runs whether or not it succeeded.

Fixes ThinkFlowLab#2358.
@HugoXK

HugoXK commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review please — tip e3b27c0075afe3d96ebf8e6a55f797ab05a2a5ac (unchanged since the PR opened, not draft, maintainer_can_modify: true, base main @ 8c58f3a1c7a6511abcd2816aa545dded7ab4b99d).

Tier T3 (changelog.d/, docs/subsystems/core-host.md, core/src/host/diagnostics.js, core/test/diagnostics.test.js, patches/ds-bot/0001, patches/ds-bot/0007 — 6 files +182/−9, 2 generated). So this needs your formal Verdict: PASS at that tip; T1/T0 shortcuts do not apply.

Checks at the tip: 14 success + 1 failure, the only red being enqueue-guard → no Verifier report names the tip e3b27c0075af; tests-ok, merge-gate, judge, policy-sign and risk tier are all green. Mergeable, no unresolved threads, 0 reviews so far.

What it does: clear() now truncates the log leaf even when the rotated copy cannot be removed (directory, or a folder refusing unlink), reporting that removal failure in one stderr line instead of aborting the truncate; red first on the base with the two new cases failing, then 30/30, verify 4754 pass / 0 fail, arch-docs: OK (6 changed file(s)). Deterministic reproduction is in the PR body.

@Birfy

Birfy commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at e3b27c0
Written-by: Reviewer
Attestation: e3b27c0 role=Reviewer

Ran at this tip:

$ cd core && node --import ./test/harness-resolve.mjs --test test/diagnostics.test.js
# tests 30  # pass 30  # fail 0
$ git show 8c58f3a1c7a6511abcd2816aa545dded7ab4b99d:core/src/host/diagnostics.js > core/src/host/diagnostics.js && node --import ./test/harness-resolve.mjs --test test/diagnostics.test.js
# tests 30  # pass 28  # fail 2

Red first: with only core/src/host/diagnostics.js reverted to the base blob (8c58f3a) and the tip tests in place, the two new #2358 tests fail — the leaf is truncated although the rotated copy stays and ... although the folder refused the removal — on strictEqual(readFileSync(logPath, 'utf8'), '') with the old lines still in the leaf. Restoring the tip blob returns 30/30. That is the admission-line test.

Checks at this tip, read from GET /repos/ThinkFlowLab/FlyBot/commits/e3b27c0075afe3d96ebf8e6a55f797ab05a2a5ac/check-runs at 2026-11-04T06:03Z: tests-ok, package, plugins (tree|social|timeline|workflow), Node 24 · DSH 0.2.0-rc.2 and merge-gate all success; enqueue-guard failure is the pre-PASS state of this head (no conclusion names the tip yet), not a red check.

Scope, read on the tarball: 6 files, +182/−9. The only product change is core/src/host/diagnostics.js (+9/−2): the rm(${logPath}.1) call keeps a try of its own, logs one stderr line naming the error code and never the folder, and the truncate is no longer short-circuited by a failed removal. clear()'s signature, the fileKind guard and the removal rule are unchanged; the empty-catch silence is gone. core/test/diagnostics.test.js adds the two EISDIR/EACCES shapes (+80/−1), patches/ds-bot/0001-*/0007-* are the mechanical generate output for those two files, and docs/subsystems/core-host.md:409 now says the removal keeps its own try — the owned-path doc moves with the code, so arch-docs has a real update rather than a trailer.

Tier: T3, on the content rule \.(only|skip)\( matching the test file's root-guard t.skip( — not on the product change. So this tip needs this PASS, and only this one.

Not run here: npm run verify and the CI package job's pack check/typecheck/actionlint, the smoke test. tests-ok green at this exact tip already covers the suites and the package job.

Attestation note: this verdict is at the head the PR names, the PR is not draft, maintainer_can_modify true, no hold, no comment asking to hold this tip.

@github-actions github-actions Bot added the pass label Nov 4, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 4, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 4, 2026
Merged via the queue into ThinkFlowLab:main with commit da7b1a1 Nov 4, 2026
16 checks passed
@Birfy
Birfy deleted the dev/2-p523-clear-rm-guard branch November 4, 2026 06:07
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

host: a Clear that cannot remove the rotated log leaves the leaf untouched and silent

2 participants