tab type "dsh-better-sidebar:files" already registered on sidebarRightTabs re-prove (+ React #130) #770

Activity

1. Mlte0907 commented on Sep 26, 2026

   

   Mlte0907

   on Sep 26, 2026

   More actions

   Reproduced and root-caused this on DSH 0.1.7-rc.2 + dsh-better-sidebar@0.21.1 (profile web, pnpm patched install). Same red strip, and it fires on every plugin-market update — not only on session switches.

   I think there is a second, more direct cause than the inject re-prove race, and it is inside a single sync() run rather than between two runs.

   Observation that pins it down

   I instrumented SidebarRightTabRegistry.ids (a Set) in the live page and forced a client-side replacement of the dsh-better-sidebar entry. The recorded order, for the failing run, was:

   ids.delete("dsh-better-sidebar:files")   <- had: true
   ids.add("dsh-better-sidebar:files")      <- register OK
   ids.add(...) -> throw "already registered" (x N)
   

   The stack of that first delete is not the body disposer at the end of the injected callback. It is:

   SidebarRightTabRegistry.register   (the effect body that runs this.ids.delete(id))
     <- Object.dispose                (registration.dispose())
     <- syncOnce                      (src/client/native/index.ts)
   

   syncOnce disposing the id it is about to re-register, in the same pass, is the bug.

   Root cause

   live is a single map that holds two different kinds of entries, but the cleanup loop only understands one of them:

   • per-descriptor native types, keyed by descriptor id (editor, git, subagent, ...), which come from service.getTabs() and therefore appear in wanted;
   • the files takeover, stored under FILES_KIND ('files'), which is created by registerFilesKind() and is never in service.getTabs(), so it is never in wanted.

   So in stock 0.21.1 (src/client/native/index.ts, cleanup loop at ~line 320):

   for (const [descriptorId, registration] of live) {
     if (wanted.has(descriptorId)) continue   // 'files' is never in wanted
     registration.dispose()                    // <- disposes the files takeover
     live.delete(descriptorId)
   }

   wanted is then rebuilt, and further down the same function:

   const wantsFiles = service.isTabEnabled(EDITOR_KIND)
   const hasFiles = live.has(FILES_KIND)       // false again, we just deleted it
   if (wantsFiles && !hasFiles) {
     live.set(FILES_KIND, { dispose: registerFilesKind(...) })   // re-registers the same fixed id
   }

   registerFilesKind() registers a fixed id (dsh-better-sidebar:files, registerFilesKind at line 286, tabs.register({ id, kind: FILES_KIND, ... }) at ~288). SidebarRightTabRegistry.register refuses a duplicate id by design, so any sync() that runs while the takeover is live tears it down and immediately trips over itself.

   This also explains the intermittency without invoking an inject race: BetterSidebarService.notify() and the store are synchronous (for (const fn of [...listeners]) fn()), and tabs.register / registration.dispose() both publish. A notification landing between the dispose and the re-register observes the window and throws. Whether you hit it depends only on whether such a notification arrives in that window — which is why it looks like "sometimes".

   Fix (verified)

   Skip the takeover in the descriptor cleanup loop — it is managed entirely by the wantsFiles/hasFiles block below it:

   for (const [descriptorId, registration] of live) {
     // `files` is a native takeover managed below, not a service tab descriptor.
     if (descriptorId === FILES_KIND || wanted.has(descriptorId)) continue
     registration.dispose()
     live.delete(descriptorId)
   }

   I applied this plus two defensive changes, since both underlying hazards are real and worth closing regardless:

   1. a disposal latch, with subscriptions removed before live registrations are disposed (releasing a registration publishes, so a late sync must be a no-op);
   2. re-entrancy coalescing around sync, because notify() is synchronous:

   let disposed = false
   let syncing = false
   let syncPending = false
   
   const syncOnce = (): void => { /* previous sync body, plus `if (disposed) return` */ }
   
   const sync = (): void => {
     if (disposed) return
     if (syncing) { syncPending = true; return }
     syncing = true
     try {
       do { syncPending = false; syncOnce() } while (syncPending && !disposed)
     } finally { syncing = false }
   }
   
   return () => {
     disposed = true
     for (const dispose of disposeSubscriptions.reverse()) dispose()
     for (const registration of live.values()) registration.dispose()
     live.clear()
   }

   Note on ordering: with only the FILES_KIND skip, the failure disappears. The latch and the coalescer are belt-and-braces for the dispose-publishes window and are not what carries the fix — I mention it only so the intent of each hunk is clear.

   Verification

   Applied through a pnpm patch on dsh-better-sidebar@0.21.1 (so src and both lib bundles move together), then dsh-restart:

   • served bundle contains the fix (checked the live /plugins/... response, not just disk);
   • 3 consecutive forced client-entry replacements of dsh-better-sidebar: console errors [], no [dsh-better-sidebar] native register files error strip, and exactly one dsh-better-sidebar:files in the registry at all times;
   • independent Playwright pass with system Chromium: pageerror, console errors and failed requests all 0, dsh-better-sidebar:files present in the registry;
   • web.log clean.

   Before the fix the same forced replacement reliably produced the red strip plus a burst of already registered; after it, nothing.

   About the re-prove theory in this issue

   I did not manage to observe the "previous body disposer did not finish" path — in my runs the entry is fully torn down and re-created (one fiber at a time, old gone before the new appears), and the duplicate still fires inside the new instance's own first sync. So I would treat the FILES_KIND bookkeeping as a sufficient cause on its own, independent of the re-prove ordering. If both are real, the WeakMap<tabs, liveMap> idea in this issue is still a reasonable belt for the re-prove half, but it is not what fixes the update-time failure I am seeing.

   Environment: DSH 0.1.7-rc.2, dsh-better-sidebar@0.21.1, pnpm 12.3.4, profile web. Happy to open a PR with the three hunks if useful.
2. Mlte0907 commented on Sep 26, 2026

   

   Mlte0907

   on Sep 26, 2026

   More actions

   Opened a separate issue with the root cause I landed on, in case it is easier to track there: #771.

   Short version: the ids.delete I instrumented traces to the cleanup loop inside syncOnce, not to the injected body disposer at the end of the callback. live holds both descriptor-derived types (which appear in wanted) and the files takeover under FILES_KIND (which never does), so the loop disposes the takeover and the wantsFiles / hasFiles block below re-registers the same fixed id in the same invocation. I could not reproduce the re-prove path you describe — the old entry is fully gone before the new one appears in my runs, and the duplicate still fires inside the new instance's first sync().

   If it turns out to be the same defect, feel free to close either one. I will not push a PR unless asked.
3. Menghuan1918 commented on Sep 28, 2026

   

   Menghuan1918

   on Sep 28, 2026

   Collaborator

   More actions

   同一串报错：真机日志确认了两条不同的栈，根因已定位并修复（v0.22.1 待发）

   本 issue 归因到「inject re-prove 竞态」，而 #771 归因到「同一次 sync() 内自撞」。官方桌面壳日志（desktop.frontdesk.log，2026-09-27 15:51）显示两条栈都真实存在，但它们是同一根因链的两段：

   • 15:51:39.338（第一条，泄漏点）：cannot create effect on inactive context，栈是 registerSlots ← registerFilesKind ← sync ← notify——teardown 期间 service.notify() 驱动 sync()，清理循环把 files 接管释放后在同一轮重建；tabs.register 建在宿主 ctx 上照样成功取走 id，而 ctx.slots.inject 建在插件 ctx 上抛 INACTIVE_EFFECT，于是 disposer 丢失、id 被永久占用。
   • 15:51:52.02/.40（re-prove 段）：栈是 register ← registerFilesKind ← sync ← _reload——新激活（_reload）尝试注册同一个仍被占用的 id。

   所以「新激活先于旧激活释放」确实是一条真实可达路径，但它只是把已经被泄漏的 id 再撞一次；把泄漏本身修掉（清理循环不再拆建接管 + 注册失败回滚 id）之后，这条路径就退化成一次自愈的日志噪音。

   修复：#777（已合并）+ #785（守护断言与部署级回归门）。回归门在 scratch profile 上用 npm 上的 0.22.0 实测 3/3 红（栈与上面两条逐字一致）、修复版 3/3 绿。事故记录：docs/plans/2026-09-28-native-files-takeover-reload-leak.md。

   未做：跨激活竞态的「吸收 + 补注册」机制（#766 的方向）——它需要在账本里记下插件并不拥有的注册，旧持有者随后释放时无人补注册，files 接管可能永久缺位，风险大于收益。此取舍已写进设计文档，如果你有反例请直接贴在这里。
4. Menghuan1918 commented on Sep 28, 2026

   

   Menghuan1918

   on Sep 28, 2026

   Collaborator

   More actions

   v0.22.1 已发布（npm latest）——本 issue 的缺陷已修复

   修复随 v0.22.1 上线：dsh plugin --profile web add dsh-better-sidebar@latest（或 @0.22.1）。发布说明：https://github.com/omdsh-dev/DSH-better-sidebar/releases/tag/v0.22.1

   本次修了什么

   • tab type "dsh-better-sidebar:files" already registered on sidebarRightTabs re-prove (+ React #130) #770 / [Bug] sync() 清理循环误删 FILES_KIND takeover 后立即重注册，触发 dsh-better-sidebar:files already registered #771（同串 tab type id "dsh-better-sidebar:files" is already registered）：清理循环不再把 files 接管当描述符拆建（跳过 FILES_KIND），并且任何在宿主取走 id 之后失败的注册都会回滚释放该 type 与已建槽位——id 不再可能被孤儿化。附带把守护断言改写到注册表事件日志、补上部分回滚覆盖，并新增部署级回归门 tests/e2e/native-reload.e2e.ts（在 npm 0.22.0 上连续 3 次运行全红、修复版连续 3 次全绿）。
   • [macOS Desktop] Window drag works once then stops: [data-dsh-panel-host] inherits -webkit-app-region: no-drag #772（macOS 窗口拖拽 / 双击标题栏缩放失效）：[data-dsh-better-sidebar] / [data-dsh-panel-host]（> * 保持 no-drag）与放大视图 .mermaidModal 用中性值 -webkit-app-region: initial !important 退出 app-region 计算。

   验证方式（如果你想复核）：升级到 0.22.1 后触发一次客户端条目替换（Plugins 页禁用→启用该插件，或插件市场更新），确认左下的红色诊断条不再出现、文件树仍然可用；macOS 上连续拖拽窗口标题区 5–6 次、并双击标题栏看缩放是否恢复。

   如果升级后仍能复现，请把 DSH 版本 + 插件版本 + Console 里的完整栈贴回来，我按新证据重开。
5. Menghuan1918

   closed this as completedon Sep 28, 2026