[nwparker]

Part of a full-codebase memory-leak audit — final PR (2 leaks, same subsystem/mechanism).

Description

The native-chat composer keeps two module-level caches so a typed-but-unsent message survives the composer unmounting on a TUI/GUI toggle:

• draftCache (native-chat-draft-cache.ts) — the in-progress draft text.
• attachmentCache (use-native-chat-composer-attachments.ts) — pending image-attachment paths.

Both are keyed by targetPtyId ?? terminalTabId. When a pane/tab is permanently removed while holding an unsent draft/attachment, its scope key is never revisited, so the entry lingered for the renderer's whole session (keys are never reused).

Fix: a shared setBoundedScopeCacheEntry helper (delete-then-set LRU, cap 128) that both write paths route through.

Evidence

• native-chat-draft-cache.ts: writeNativeChatDraftCache only .deleted on empty draft, else .set() — no bound.
• use-native-chat-composer-attachments.ts: writeNativeChatAttachmentCache same shape.
• Scope key targetPtyId ?? terminalTabId is a fresh, never-reused id, so a removed pane's entry is unreachable and permanent.

Proof of fix

• New native-chat-composer-scope-cache.test.ts: the shared helper caps at the limit, evicts the oldest untouched key, keeps re-set keys, and moves a re-set key to most-recent.
• New draft-cache test: flooding past the cap evicts the oldest draft while the actively-edited and most-recent drafts survive.

Test Files  3 passed (3)   ← incl. existing attachment-cache suite
      Tests  9 passed (9)

Typecheck (web) + oxlint clean.

ELI5

When you start typing a message to an agent but flip back to the terminal, the app stashes your half-written note so it's still there when you flip back. If you closed that pane for good, the stashed note was kept forever. Now the stash holds at most ~128 notes and forgets the oldest untouched ones — and the note you're actively typing is always kept.

Trade-offs

• User-facing behavior: none in practice. The cap (128) is vastly larger than the handful of panes anyone has unsent drafts in at once, and delete-then-set keeps the pane you're actively typing in most-recent so it's never the one evicted.
• Why a cap instead of purge-on-removal: a precise purge would require the store to import native-chat component modules — a coupling this codebase deliberately avoids (no store → components/native-chat imports exist). The bound is self-contained in the native-chat layer and matches the codebase's existing FIFO/LRU convention (rememberUnwatchableRoot).
• Pathological case: >128 panes each holding a distinct unsent draft/attachment would drop the least-recently-touched draft — astronomically unlikely, and the dropped value is convenience state only. Target: 0 regression.

Made with Orca 🐋

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

This PR introduces a shared setBoundedScopeCacheEntry helper and NATIVE_CHAT_COMPOSER_SCOPE_CACHE_MAX constant in a new native-chat-composer-scope-cache.ts module, implementing LRU-style bounded Map updates. The draft cache (native-chat-draft-cache.ts) and attachment cache (use-native-chat-composer-attachments.ts) modules are updated to use this helper instead of direct Map.set calls when writing cache entries. New and updated test files verify eviction behavior and cache size bounding for both draft and generic scope caches.

Sequence Diagram(s)

Not applicable; changes involve synchronous Map operations without cross-component request/response flows.

Estimated code review effort: 2/5 (Low-Medium)

Related PRs: None specified.

Suggested labels: enhancement, performance, tests

Suggested reviewers: None specified.

🐰 A cache that once grew without a bound,
now trims its edges, keeps the ground,
LRU whispers, oldest away,
newest and kept ones here to stay.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the fix and evidence, but it omits several required template sections like Screenshots, Testing checklist, AI Review Report, Security Audit, and Notes.	Add the missing template sections with concrete details, including screenshots or no visual change, the test checklist, AI review, security audit, and notes.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title concisely captures the main change: LRU-bounding the native-chat composer scope caches.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/renderer/src/components/native-chat/native-chat-composer-scope-cache.ts (1)

1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Trim the header comment to comply with the brevity guideline.

This comment spans 7 lines; the guideline for **/*.{ts,tsx,js,jsx} requires "why" comments to stay to one or two lines.

✏️ Suggested trim

-// Shared LRU bound for the native-chat composer's per-scope caches (draft text
-// and image attachments), both keyed by `targetPtyId ?? terminalTabId`. The
-// caches exist so an in-progress message survives the composer unmounting on a
-// TUI/GUI toggle, but a scope key for a permanently-removed pane is never
-// revisited, so without a bound its unsent entry would linger for the renderer's
-// whole session. delete-then-set keeps the actively-edited scope most-recent so
-// eviction only sheds the oldest untouched scopes.
+// Bounds per-scope caches (keyed by targetPtyId ?? terminalTabId) so unsent
+// state for permanently-removed panes doesn't linger for the whole session.

As per coding guidelines, "Keep comments short — one or two lines, capturing only the non-obvious reason."

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9a11e626-a01d-4ee3-8da6-28cb065cbf2c

📥 Commits

Reviewing files that changed from the base of the PR and between b41cab1 and a605080.

📒 Files selected for processing (5)

• src/renderer/src/components/native-chat/native-chat-composer-scope-cache.test.ts
• src/renderer/src/components/native-chat/native-chat-composer-scope-cache.ts
• src/renderer/src/components/native-chat/native-chat-draft-cache.test.ts
• src/renderer/src/components/native-chat/native-chat-draft-cache.ts
• src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts