2 Commits
Author SHA1 Message Date
1570e292fb fix(session-notification): revert PR #543 and add proper notification plugin conflict detection (#575)
* revert: undo PR #543 changes (bun shell GC crash was misdiagnosed)

This reverts commit 4a38e70 (PR #543) and 2064568 (follow-up fix).

## Why This Revert

The original diagnosis was incorrect. PR #543 assumed Bun's
ShellInterpreter GC bug was causing Windows crashes, but further
investigation revealed the actual root cause:

**The crash occurs when oh-my-opencode's session-notification runs
alongside external notification plugins (e.g., @mohak34/opencode-notifier).**

Evidence:
- User removed opencode-notifier plugin → crashes stopped
- Release version (with original ctx.$ code) works fine when used alone
- No widespread crash reports from users without external notifiers
- Both plugins listen to session.idle and send concurrent notifications

The real issue is a conflict between two notification systems:
1. oh-my-opencode: ctx.$ → PowerShell → Windows.UI.Notifications
2. opencode-notifier: node-notifier → SnoreToast.exe

A proper fix will detect and handle this conflict gracefully.

Refs: #543, oven-sh/bun#23177, oven-sh/bun#24368
See: docs/CRASH_INVESTIGATION_TIMELINE.md (in follow-up commit)

* fix(session-notification): detect and avoid conflict with external notification plugins

When oh-my-opencode's session-notification runs alongside external
notification plugins like opencode-notifier, both listen to session.idle
and send concurrent notifications. This can cause crashes on Windows
due to resource contention between different notification mechanisms:
- oh-my-opencode: ctx.$ → PowerShell → Windows.UI.Notifications
- opencode-notifier: node-notifier → SnoreToast.exe

This commit adds:
1. External plugin detection (checks opencode.json for known notifiers)
2. Auto-disable of session-notification when conflict detected
3. Console warning explaining the situation
4. Config option 'notification.force_enable' to override

Known notification plugins detected:
- opencode-notifier
- @mohak34/opencode-notifier
- mohak34/opencode-notifier

This is the actual fix for the Windows crash issue previously
misdiagnosed as a Bun.spawn GC bug (PR #543).

Refs: #543

* docs: add crash investigation timeline explaining the real root cause

Documents the investigation journey from initial misdiagnosis (Bun GC bug)
to discovering the actual root cause (notification plugin conflict).

Key findings:
- PR #543 was based on incorrect assumption
- The real issue is concurrent notification plugins
- oh-my-opencode + opencode-notifier = crash on Windows
- Either plugin alone works fine

* fix: address review feedback - add PowerShell escaping and use existing JSONC parser

- Add back single-quote escaping for PowerShell soundPath to prevent command failures
- Replace custom stripJsonComments with existing parseJsoncSafe from jsonc-parser
- All 655 tests pass

---------

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
2026-01-07 23:44:03 +09:00
João Carlos Magalhães de CastroandGitHub 4a38e70fa8 fix(session-notification): use node:child_process to avoid Bun shell GC crash (#543)
Replace Bun shell template literals (ctx.$) with node:child_process.spawn
to work around Bun's ShellInterpreter garbage collection bug on Windows.

This bug causes segmentation faults in deinitFromFinalizer during heap
sweeping when shell operations are used repeatedly over time.

Bug references:
- oven-sh/bun#23177 (closed incomplete)
- oven-sh/bun#24368 (still open)
- Pending fix: oven-sh/bun#24093

The fix applies to all platforms for consistency and safety.
2026-01-07 01:37:15 +09:00