fix(editor): correct URI predicate + relocate close cleanup to App level

[brennanb2025]

Summary

Two follow-up bugs from the merged fix in #1353 (commit ffaca13b):

Bug 1 — predicate compared against the wrong URI form

findLeakedDiffModelUris (in editor-model-leak.ts) compared monaco.Uri.toString() against literal : prefixes. Monaco's URI.toString() percent-encodes : to %3A in the path segment, so real-world URIs like diff:modified%3A/Users/me/file.md%3A%3Ascope never matched the predicate's diff:modified:/Users/me/file.md:: prefix. The unit test passed because it used synthetic tab ids like tab-abc that contained no encoded characters — the regression class real production tabs (file paths with :) silently fell into.

Fix: the helper now operates on the URI's scheme and path (which preserve literal :) instead of toString(). Renamed findLeakedDiffModelUris(uris: string[], …) → findLeakedDiffModelPaths(models: { scheme; path }[], …). Predicate prefixes drop the diff: scheme (now checked separately via scheme === 'diff') and match against path. Added an 'absolute file path' test that exercises the exact path shape Monaco produces for file-path tab ids.

Bug 2 — cleanup effect lived on a component that unmounts at tab close

The close-cleanup useEffect was inside EditorPanel. But EditorPanel is conditionally mounted on the active tab — when the user closes the active tab, the panel unmounts before its effect can observe the change. Confirmed empirically: after closeFile, no [debug-effect-fire] log printed because the panel had already unmounted (debug-unmount-fire fired first). This means the existing case 'edit', case 'diff', case 'markdown-preview' cleanup branches all silently failed for the active tab. Other panels could pick up the slack in multi-pane setups, but a single-pane close leaked everything.

Fix: extracted the entire cleanup into useEditorTabCloseCleanup and mount it at App level alongside useGitStatusPolling — App is always mounted, so the effect always observes openFiles changes regardless of which panel is active.

Files changed

• src/renderer/src/App.tsx — import + call useEditorTabCloseCleanup(). Comment cross-references the hook file for the architectural why.
• src/renderer/src/components/editor/EditorPanel.tsx — removed the entire close-cleanup block (~85 lines) and the now-unused prevOpenFilesRef, deleteCacheEntriesByPrefix. Dropped imports that were only used by the removed code: monaco, scrollTopCache/cursorPositionCache/diffViewStateCache, findLeakedDiffModelPaths. Left a short comment pointing readers to the hook.
• src/renderer/src/components/editor/editor-model-leak.ts — predicate now keyed on {scheme, path}; function renamed.
• src/renderer/src/components/editor/editor-model-leak.test.ts — tests updated to the new shape; added an absolute-file-path regression test that locks in the encoding-mismatch fix.
• src/renderer/src/components/editor/use-editor-tab-close-cleanup.ts — new file; owns prevOpenFilesRef, the switch over prevFile.mode, and a module-level deleteCacheEntriesByPrefix helper. Cleanup logic is byte-identical to what was in EditorPanel — same switch, same disposal calls, same cache evictions; only the location changed.

The cleanup effect was reviewed by parallel Claude /review-code, Claude /review-algorithm-architecture, Claude deletion-impact, and Codex /review-code agents — all four reported zero must-fix issues.

Test plan

• pnpm exec vitest run src/renderer/src/components/editor/editor-model-leak.test.ts — 10/10 pass

• pnpm exec tsc --noEmit -p config/tsconfig.web.json — clean (the two pre-existing worktree-logic/wsl.ts errors are unrelated)

• Manual verification in running Electron build (port 9333) via playwright-cli eval:

  Bug 1 — encoding fix: Drove a real monaco.Uri.parse() against a real-world Changes-mode path (/Users/me/repo/file.md::scope:original:hash):

  • uri.toString() → modified%3A/Users/me/repo/file.md%3A%3Ascope-id-1 (percent-encoded). Old toString()-based predicate matched 0 of 2 paths (reproduces the production bug).
  • uri.path → modified:/Users/me/repo/file.md::scope-id-1 (literal :). New predicate matched 2 of 2 paths.

  Bug 2 — App-level hook fires for non-active tabs: Created 4 kept-alive Monaco diff models (single-pane modified, split-pane modified, rotated original, split-pane rotated original) for a synthetic edit-mode tab whose id is a file path with embedded :. Inserted into openFiles, removed without ever activating it (so no EditorPanel was ever mounted for it — the exact failure mode the old code had). Result: all 4 models disposed (isDisposed() === true) and no leftover diff models for the tab id.

  Diff-mode + collision sanity: Repeated with a production-shape diff tab id (fake-wt::diff::unstaged::synthetic.md). All 4 target models disposed. A near-prefix unrelated tab (fake-wt::diff::unstaged::synthetic.md.txt) was correctly not disposed — confirms the :: boundary in the predicate prevents prefix-collision false-positives.

• User session integrity: 8 user-open tabs unchanged before/after every test step.

Made with Orca 🐋