Skip to content

Move terminal bytes out of panel state to stop the JSON OOM crash - #603

Merged
parsakhaz merged 1 commit into
mainfrom
state-no-terminal-bytes
Sep 11, 2026
Merged

Move terminal bytes out of panel state to stop the JSON OOM crash#603
parsakhaz merged 1 commit into
mainfrom
state-no-terminal-bytes

Conversation

@parsakhaz

Copy link
Copy Markdown
Member

Description

Pane crashed twice (2026-09-10 21:41, 2026-09-11 11:11) opening the archived pane "PR 2164: durable run attention and device alerts". Both crashes are a V8 heap-limit OOM inside one JSON operation: JSON.stringify in updatePanel on the first, JSON.parse of the same row on the second. The row was the Codex terminal panel 26044081-3829-4eb1-a6e6-f86b37b4c010, whose tool_panels.state had grown to 428 MB. Root cause report: ~/.pane/reports/pane-oom-crash-2026-09-11.md.

The defect: the pty output "command detection" heuristic in terminalPanelManager.ts appended every output chunk without a newline to currentCommand. Full-screen TUIs never emit a newline in the alternate screen, so the accumulator grew for as long as the agent ran, and every state save persisted it as lastActiveCommand (plus commandHistory). updatePanel then parsed and re-serialized the whole blob on every write. Nothing ever read either key.

This PR fixes the root cause rather than the symptom: panel state carries no terminal bytes, ever.

  1. State carries no terminal bytes. lastActiveCommand and commandHistory are gone from TerminalPanelState and both persist sites. The in-memory currentCommand is kept for file-operation detection, but no longer accumulates while the alternate screen is active, resets past 4 KB, and history entries are capped at 1 KB / 100 entries.
  2. Bytes get their own bounded store. New table panel_buffers(panel_id, scrollback, serialized, alternate, bytes, updated_at). scrollbackBuffer, serializedBuffer and alternateScreenBuffer are split out of every state write at the database layer (splitPanelBufferState) and stored there under a hard 4 MB per-panel cap across the three columns; the oldest scrollback is trimmed first, ANSI-safe, with one logged warning per panel per process. Readers of persisted bytes (restoreTerminalState, the lazy scrollback fallback in ipc/panels.ts, panels screen/panels output in ipc/runpane.ts) read the table. The retention sweep deletes rows there instead of json_remove on state.
  3. State writes are partial and ceilinged. updatePanel no longer parses or re-serializes the blob in JS. Keys are merged inside SQLite with json_set / json_remove (same one-level customState merge semantics as before: a key set to undefined is removed). Any write whose resulting state exceeds 256 KB is rolled back, logged with the panel id, the size and the largest key, and updatePanel returns false; createPanel throws. The panelManager cache and the panel:updated payload also stop carrying bytes.
  4. Migration and repair, idempotent via PRAGMA user_version, runs in DatabaseService.initialize() at module load, before any pane opens. It clones sessions.db to sessions.db.pre-panel-buffers-<timestamp>.bak first (APFS clone, instant), unwraps legacy string-wrapped rows, moves the three buffer keys into panel_buffers under the cap, json_removes all five keys, then VACUUMs. A fresh database is versioned without a backup or rewrite. The bootstrap logger reports before/after sizes.
  5. Tests, see below.

Migration proven on a copy of the live database

Run against an APFS clone of ~/.pane/sessions.db (never the live file), using the compiled main/dist code:

Before After
sessions.db file 1,816.3 MB 175.7 MB
tool_panels.state total 1,648.3 MB 0.9 MB
largest state row 428.5 MB (panel 26044081…) 11.1 KB
panel 26044081… state 428.5 MB 1,003 bytes
rows with lastActiveCommand / commandHistory 940 / 941 0 / 0
rows with any of the three buffer keys 941 0
panel_buffers rows / total / largest 941 / 34.8 MB / 568 KB
PRAGMA user_version 0 1

Migration plus VACUUM 16.5 s wall clock including the stats queries, peak RSS 1.4 GB (SQLite parsing the 428 MB row in its own heap, not V8's), PRAGMA integrity_check ok, a partial updatePanel on the repaired big panel accepted, second initialize() on the same copy a no-op with no second backup.

The migration was not run against the live database. Pane must be restarted for the startup migration to apply; that is the operator's action after merge. Expect roughly 15 s of extra startup time on that first launch plus a 1.8 GB .bak next to the database.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Performance improvement

Testing

New Vitest coverage (pnpm --filter main test):

  • terminalPanelManager.persistence.test.ts — (a) streams 50 MB of newline-free alternate-screen frames through the real pty.onData handler via a fake ptyHost handle; persisted state stays under the ceiling and currentCommand under 4 KB; a normal-screen variant proves the 4 KB reset. (d) persists a terminal, restarts the manager, and asserts the replayed bytes equal what the old path replayed from the state JSON, in both normal and alternate screen.
  • panelBuffers.test.ts — (b) a 300 KB state write is refused and logged with the panel, size and customState.initialInput (… bytes); a 200 KB write succeeds; nothing from a refused write lands. (c) 5 MB of scrollback persists as 4 MB, oldest trimmed, newest kept, one warning; a second oversize write stays quiet. Plus per-key merge semantics, legacy array scrollback, delete cascades and the retention sweep.
  • panelBufferMigration.test.ts — (e) a fixture with a 30 MB lastActiveCommand row (plus a string-wrapped legacy row, a logs row and a NULL-state row) migrates through the production initialize() entry point: state under 10 KB, buffers intact, backup written once, file shrinks below 1 MB, second run a no-op.
  • database.panel-loading.test.ts and two runpane.test.ts cases updated to the new contract (bytes come from the store, not state).

main/src/test/setup.ts now points PANE_DIR at a scratch directory for every test run: importing services/database opens ${PANE_DIR}/sessions.db and runs the startup migrations at import time, so a local vitest could otherwise touch the developer's live ~/.pane.

Gates: pnpm lint (oxlint, eslint, advisory, boundary conformance, knip) clean; pnpm typecheck clean; pnpm --filter main test 958 passed, 2 skipped; frontend terminalRestore.test.ts passes.

Not done: no manual run of the packaged app against the live database, by design.

Critical Areas Modified

  • State management/IPC events (panel state persistence; panel:updated no longer carries terminal bytes; renderer restore still goes through terminal:getState, unchanged)

Additional Notes

  • Out of scope, named for the next brief: PTY out of the main process; initialize only the active tab's CLI when a pane becomes visible; a startup sweep that reports oversized rows.
  • restoreTerminalState has no callers in the repo (unchanged since the initial release); it is updated and covered because the brief names it as a restore path.
  • getPanelsForSession(sessionId, includeScrollback) lost its second parameter: with bytes out of the state row, one query serves summaries and full reads.
  • panel_buffers.panel_id references tool_panels(id) ON DELETE CASCADE; this SQLite build enforces foreign keys, so rows go with their panel and no manual cleanup is needed.
  • Reviewed with the repo's four-angle cleanup pass; deliberately kept: the split at the database layer (it holds the invariant for every writer, including panels:update from the renderer), updatePanel returning false instead of throwing (a refused write must not abort panels:initialize), the synchronous VACUUM (as briefed), and the write-only in-memory commandHistory (as briefed; it could be deleted in a follow-up).

https://claude.ai/code/session_0138qD9Q32QSDNbJUBi7pRiZ

Both Pane crashes on 2026-09-10 and 2026-09-11 were V8 heap OOMs inside
JSON.stringify / JSON.parse of one tool_panels.state row that had grown
to 428 MB: the pty output command-detection heuristic appended every
newline-free chunk to lastActiveCommand, and updatePanel re-serialized
the whole blob on every write.

State now carries no terminal bytes. lastActiveCommand and commandHistory
are dropped from TerminalPanelState; the in-memory accumulator is frozen
in the alternate screen and reset past 4 KB. scrollbackBuffer,
serializedBuffer and alternateScreenBuffer are split out of every state
write into a new panel_buffers table capped at 4 MB per panel (oldest
scrollback trimmed first, one logged warning). updatePanel merges keys
inside SQLite with json_set / json_remove and refuses any state over
256 KB with a logged error naming the largest key. An idempotent startup
migration backs up sessions.db, moves the buffers, removes the five keys
and vacuums; on a copy of the live database it went from 1,816 MB to
176 MB in 16.5 s.

Claude-Session: https://claude.ai/code/session_0138qD9Q32QSDNbJUBi7pRiZ
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T19:34:23.049837Z c772bf0 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c772bf0d23

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +4685 to +4686
this.mergePanelState(panelId, state);
this.assertPanelStateWithinCeiling(panelId);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep logs state writable after reaching the new ceiling

When a run-script's last 1,000 output chunks exceed 256 KB, logsManager.handleOutput continues submitting the entire outputBuffer, so this assertion rolls back that write and every later state update, including the process-exit status. Since PanelManager.updatePanel absorbs the false result and the logs manager mutates its cached array before saving, the UI can continue while the database remains frozen with stale output and potentially isRunning: true; either move/bound the logs output by bytes or avoid applying this terminal-oriented ceiling to that state.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant