Skip to content

Resident-string blob backing (#1726/#1729): spill pins memory, BigInt path crashes, idle tokens reach session.messages, blob dir growth and cross-process id collisions #1746

Description

@code-yeongyu

Summary

PR #1726 adds a recoverable on-disk backing for the resident string store and releases materialized views at agent_idle; PR #1729 (stacked) spills resident strings to that backing across compaction. Reading both diffs end to end surfaced six defects/risks that must be fixed before the pair can merge. This issue tracks completing the design to its ideal state and landing both PRs.

Actual (at #1726 head 8359873f, #1729 head 32618a6c)

  • A. spillResident() never releases memory. session-resident-store.ts:66-77 deletes from strings but not from the idsByText reverse index; the Map keys are the spilled strings themselves, so every spilled string stays pinned. changes.md:5973 claims the index is "deleted on eviction/spill/clear" — false for spill. feat(coding-agent): spill resident strings to the blob backing across compaction #1729's stated goal (empty the map, keep the backing) is therefore not achieved.
  • B. BigInt path now crashes with the wrong error. The explicit TypeError("Do not know how to serialize a BigInt") was removed from transformJsonValue, but a bigint then reaches seen.add(value) on a WeakSetTypeError: WeakSet values must be objects (verified with Bun 1.4). The PR body's "serializes BigInt values instead of throwing" is not what the code does.
  • C. Runtime tokens escape to consumers. Between agent_idle and the next turn, agent.state.messages holds \0senpi-resident-string:v1:<id> tokens (agent-session.ts:1998-2002). The public getter get messages() (agent-session.ts:3445) returns that array unchanged, so any idle-time reader (TUI message selector, interactive-mode.ts:9400 JSON export, extensions, RPC) sees tokens. test/suite/harness.ts had to be changed to materialize through the store — proof that the consumer contract changed despite the PR body's "every existing consumer contract unchanged". This is the same failure class as _getCompactEntries() leaks evicted resident-string refs into resumed image data #1631, introduced into runtime state.
  • D. Blob directory growth is unbounded. Ids are a per-process counter; a string hydrated from a blob and re-externalized at the next idle gets a NEW id → a NEW .blob file, and old files are never deleted. The dir is only removed on newSession/branch inside the same process — never on process exit or on reopening the session. feat(coding-agent): spill resident strings to the blob backing across compaction #1729's "bounded by the resident budget" does not hold.
  • E. Cross-process id collision. Two processes on one session id share <sessionDir>/resident-blobs/<sessionId>/; both start ids at 0, so N.blob is overwritten with a different text and the other process hydrates the wrong content silently (the envelope is valid JSON).
  • F. Per-turn full re-materialization. Caches are dropped on EVERY idle regardless of budget, so any settled-time getEntries() consumer re-materializes the full history each turn — the cost bug(coding-agent): post-compaction entry counting re-materializes the full JSONL history on the settled path #1699 (57a7b5441f) just removed — even though releasing the views frees nothing unless the store has evicted something (resident strings are shared objects).

Expected (ideal state)

  • Blob files are content-addressed (id = hash of the text): re-externalizing hydrated text maps to the same blob, no per-turn churn, and two processes writing the same session dir can only ever write identical bytes under a given name. The reverse index becomes unnecessary, which fixes A structurally; spillResident() empties the map and releases the strings.
  • BigInt keeps JSON semantics: externalize/materialize throw TypeError matching /BigInt/.
  • session.messages and every out-of-turn reader of runtime messages (compaction admission estimates, log-token estimates) always observe real strings; test/suite/harness.ts returns to its pre-PR form.
  • The idle release (view drop + in-place tokenization) runs only when the store has evicted something.
  • The blob directory is released when the session's writer is disposed and cleared when a persisted session is reopened; newSession/branch removal stays.
  • Corrupt blobs are deleted on read so a later eviction rewrites them.

Acceptance criteria

  • test/session-resident-store.test.ts: (a) re-externalizing a spilled string makes it resident again and yields the pre-spill token; (b) two stores over one blobsDir with different texts each hydrate their own text; (c) evict→hydrate→re-externalize adds no .blob file; (d) a corrupt blob is removed on read; (e) externalize({ n: 1n }) throws TypeError matching /BigInt/.
  • Harness tests: after agent_idle on a persisted over-budget session, session.messages text equals the original; an under-budget session keeps getEntries() identity across idle, an over-budget one does not.
  • SessionManager tests: blob dir removed on writer disposal; stale dir cleared on open(); existing newSession/branch removal tests stay green.
  • Focused suites + root bun run check green; PR CI green at the final head; changes.md and CHANGELOG.md entries updated.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions