Skip to content

decisions.jsonl never trims: the 2000-line cap is read-only, the file grows without bound #603

Closed

Description

@Birfy

Summary

core/src/host/decisions.js (merged in abaf938b via #566) caps at 2000 lines only in list(): it reads the whole file, splits it, and slices the last KEPT lines. record() appends with appendFile and never trims, rotates or checks size. So <home>/decisions.jsonl grows without bound, and every read parses the whole file before discarding all but the newest 2000 lines.

Evidence (main abaf938b)

  • KEPT = 2_000 is used only in list(): readFile → split → slice(-KEPT) → parse.
  • record() appends with appendFile, with no size guard, trimming or rotation.
  • The natural caller (a Timeline view that renders the log on refresh) pays O(file) per read, and that cost grows with every recorded decision.

Expected behavior

  1. The file is bounded on disk, not only in the answer: trim in record() (keep the newest N lines) or rotate to decisions.jsonl.1, done by the host, while the Bot side stays append-only (a Bot still cannot edit or delete an entry).
  2. A read is O(KEPT), not O(file).
  3. A byte cap as well as a line cap, so one huge entry cannot blow the bound.
  4. list() keeps returning the newest entries, existing tests stay green, and a new test appends more than KEPT entries then asserts the on-disk file stays bounded and the newest entries survive.

Change scope

core/src/host/decisions.js, core/test/decisions.test.js, the mirror patches/ds-bot/files/src/host/decisions.js, docs/subsystems/core-host.md (state the cap), one changelog fragment.

Note

If the user chooses to revert #566, note this defect in the revert PR instead of fixing it here.

Why

A per-Bot file that only grows is a real resource leak in merged code: reproducible, not a design proposal.

Activity

  1. added
    bugSomething isn't working
    area:automationScheduled tasks and automation
    on Oct 21, 2026
  2. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Triage (2026-10-21): dedupe done — no existing issue tracks the unbounded decisions.jsonl (#396 is the feature scope; the growth note there is a comment, not a defect issue). Kept bug + area:automation.

    Blocking: the user's keep/revert decision on #566/#396, through the Chief. If keep → this becomes agent-ready for Dev (acceptance and scope are in the body: trim/rotate in record(), byte cap, a test appending more than KEPT entries while asserting the file stays bounded). If revert → note the defect in the revert PR instead. needs-human until that answer.

  3. added
    needs-humanThe agent loop stopped; a person must decide (reason in the last comment)
    on Oct 21, 2026
  4. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Blocking resolved: the Chief accepted #566 (no revert), so the decision log stays and this defect gets fixed rather than noted in a revert PR. agent-ready for Dev — acceptance and scope are in the body (trim/rotate in record(), byte cap, a test appending more than KEPT entries with the on-disk file staying bounded).

  5. added
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    and removed
    needs-humanThe agent loop stopped; a person must decide (reason in the last comment)
    on Oct 21, 2026
  6. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Dev claim (2026-10-21, 10:3xZ): taking #603 on branch fix/603-decisions-trim, no shell available, so the red/green is produced by CI instead of locally.

    Plan, as the Lead ordered:

    1. First commit: test only — core/test/decisions.test.js gets the minimal reproduction (pre-seed KEPT lines, record one more, assert the on-disk file stays bounded; and a 1.1 MB single entry, assert the byte cap). CI must go red on that commit; that run is the reproduction evidence.
    2. Second commit: the fix in core/src/host/decisions.js (trim on write, newest 2000 lines and a 1 MB byte cap, read stays O(KEPT)), its mirror patches/ds-bot/files/src/host/decisions.js, docs/subsystems/core-host.md, one changelog fragment. CI must go green on the new tip.

    patches/ is a whole-file mirror of core/src/host/decisions.js, so the missing generate step is a byte-for-byte copy, stated in the PR. Everything goes through the GitHub API; nothing was run locally.

  7. self-assigned this
    on Oct 21, 2026
  8. added
    agent-prPull request opened by an agent
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    and removed
    agent-readyMaintainer-approved spec: an agent may take it. Only maintainers set this; never in a template
    on Oct 21, 2026
  9. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Dev claim: branch fix/603-decisions-log-bounded, PR #606 (tip 2a161c7d). Fix: record() trims the log to the newest 2000 lines / 1 MB (temp file + rename), list() reads at most the last 1 MB; tests append 2001 entries and use an oversized log. Red without the source change: 3 pass / 2 fail (the two #603 tests).

  10. removed
    agent-prPull request opened by an agent
    on Oct 21, 2026
  11. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Triage (2026-10-21): two open PRs now Fixes #603 — #605 (fix/603-decisions-trim) and #606 (fix/603-decisions-log-bounded). One issue = one PR: keep one branch and close or fold the other (they overlap on the same trim/byte-cap change); the dropped one should say "superseded by #N" rather than stay open. Also, none of the three new PRs (#604/#605/#606) carries a risk:T* yet — the controller adds it from the diff, so I am not hand-labeling; it should appear before review.

  12. Birfy commented on Oct 21, 2026

    @Birfy
    CollaboratorAuthor

    Dev dedupe (2026-10-21): keeping #606 (fix/603-decisions-log-bounded, tip 2a161c7d) — red-without-fix 3 pass / 2 fail with main's decisions.js, 5/5 with the fix, the core-host.md doc update, and green tests-ok/clean merge state. #605 (fix/603-decisions-trim) is closed as superseded by #606: its tests-ok is red and its body has no repro/red/not-run sections. Reviewer: review #606 only.

    Note for the controller: #606 is green at 2a161c7d and still has no risk:T*; the risk tier (report only) job succeeded without labeling, so per the current rule that is a separate defect.

  13. removed
    agent-workingAn agent has claimed this issue (the claim comment names the branch)
    on Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:automationScheduled tasks and automationbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions