You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #922 added a releaseTerminal(terminalId) API in packages/client/src/comments/useComments.ts (commit 69557ad2) to close the code-police no-unbounded-growth finding on the per-terminal comment-store Map. The API exists but isn't called anywhere — wiring it to the terminal-deletion path is the follow-up.
What needs to happen
When a terminal is deleted, call releaseTerminal(terminalId) so its storesByKey Map entry is freed.
import{releaseTerminal}from"./comments/useComments";// in the terminal-deletion handler, after the terminal is removed from the// active listreleaseTerminal(deletedTerminalId);
Find the deletion path — search for client.terminal.delete or wherever the client reacts to a terminal-removed snapshot from the server stream.
What the API deliberately does NOT do
releaseTerminal only drops the in-memory Map entry. It does not touch localStorage. That's intentional — comments survive terminal recreation if the same terminalId is reused (e.g. session restore on app reload, where kolu replays saved terminals with stable ids). If session restore brings the terminal back, the next useComments(tid) call rehydrates the store from localStorage transparently.
Magnitude
Each unfreed Map entry is a couple of closures + a SolidJS signal — maybe a few hundred bytes. Realistic upper bound for a heavy user is ~100 terminals over a session, so the leak is "annoying but not catastrophic" (the police finding is correct, but the urgency is low). Worth doing for hygiene; not blocking anything.
Police finding: no-unbounded-growth — storesByKey accumulates one entry per useComments(tid) call
Verified makePersisted does NOT write to localStorage on signal creation — only on signal change — so the localStorage half is already bounded to "terminals where comments were saved." Just the in-memory Map needs eviction.
PR #922 added a
releaseTerminal(terminalId)API inpackages/client/src/comments/useComments.ts(commit69557ad2) to close the code-policeno-unbounded-growthfinding on the per-terminal comment-store Map. The API exists but isn't called anywhere — wiring it to the terminal-deletion path is the follow-up.What needs to happen
When a terminal is deleted, call
releaseTerminal(terminalId)so itsstoresByKeyMap entry is freed.Find the deletion path — search for
client.terminal.deleteor wherever the client reacts to a terminal-removed snapshot from the server stream.What the API deliberately does NOT do
releaseTerminalonly drops the in-memory Map entry. It does not touch localStorage. That's intentional — comments survive terminal recreation if the sameterminalIdis reused (e.g. session restore on app reload, where kolu replays saved terminals with stable ids). If session restore brings the terminal back, the nextuseComments(tid)call rehydrates the store from localStorage transparently.Magnitude
Each unfreed Map entry is a couple of closures + a SolidJS signal — maybe a few hundred bytes. Realistic upper bound for a heavy user is ~100 terminals over a session, so the leak is "annoying but not catastrophic" (the police finding is correct, but the urgency is low). Worth doing for hygiene; not blocking anything.
Reference
69557ad2(PR Comments on any file — select text, queue, paste into the agent #922)no-unbounded-growth—storesByKeyaccumulates one entry peruseComments(tid)callmakePersisteddoes NOT write to localStorage on signal creation — only on signal change — so the localStorage half is already bounded to "terminals where comments were saved." Just the in-memory Map needs eviction.