Commit Graph

12 Commits

Author SHA1 Message Date
YeonGyu-Kim
cf40ca5553 fix(hooks): ensure notification fallback on terminal-notifier failure 2026-03-03 00:38:17 +09:00
1noilimrev
fbe3b5423d refactor(test): extract shared mock helper and add try-finally for env cleanup 2026-02-27 15:30:13 +09:00
1noilimrev
88bf8268f5 test(hooks): add darwin notification backend selection tests 2026-02-27 14:48:34 +09:00
YeonGyu-Kim
99f4c7e222 fix(hooks): stabilize session notification checks in parallel tests
Use sender-module indirection and an optional main-session filter guard to keep notification assertions deterministic across concurrent test execution.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-02-24 21:43:47 +09:00
YeonGyu-Kim
f146aeff0f refactor: major codebase cleanup - BDD comments, file splitting, bug fixes (#1350)
* style(tests): normalize BDD comments from '// #given' to '// given'

- Replace 4,668 Python-style BDD comments across 107 test files
- Patterns changed: // #given -> // given, // #when -> // when, // #then -> // then
- Also handles no-space variants: //#given -> // given

* fix(rules-injector): prefer output.metadata.filePath over output.title

- Extract file path resolution to dedicated output-path.ts module
- Prefer metadata.filePath which contains actual file path
- Fall back to output.title only when metadata unavailable
- Fixes issue where rules weren't injected when tool output title was a label

* feat(slashcommand): add optional user_message parameter

- Add user_message optional parameter for command arguments
- Model can now call: command='publish' user_message='patch'
- Improves error messages with clearer format guidance
- Helps LLMs understand correct parameter usage

* feat(hooks): restore compaction-context-injector hook

- Restore hook deleted in cbbc7bd0 for session compaction context
- Injects 7 mandatory sections: User Requests, Final Goal, Work Completed,
  Remaining Tasks, Active Working Context, MUST NOT Do, Agent Verification State
- Re-register in hooks/index.ts and main plugin entry

* refactor(background-agent): split manager.ts into focused modules

- Extract constants.ts for TTL values and internal types (52 lines)
- Extract state.ts for TaskStateManager class (204 lines)
- Extract spawner.ts for task creation logic (244 lines)
- Extract result-handler.ts for completion handling (265 lines)
- Reduce manager.ts from 1377 to 755 lines (45% reduction)
- Maintain backward compatible exports

* refactor(agents): split prometheus-prompt.ts into subdirectory

- Move 1196-line prometheus-prompt.ts to prometheus/ subdirectory
- Organize prompt sections into separate files for maintainability
- Update agents/index.ts exports

* refactor(delegate-task): split tools.ts into focused modules

- Extract categories.ts for category definitions and routing
- Extract executor.ts for task execution logic
- Extract helpers.ts for utility functions
- Extract prompt-builder.ts for prompt construction
- Reduce tools.ts complexity with cleaner separation of concerns

* refactor(builtin-skills): split skills.ts into individual skill files

- Move each skill to dedicated file in skills/ subdirectory
- Create barrel export for backward compatibility
- Improve maintainability with focused skill modules

* chore: update import paths and lockfile

- Update prometheus import path after refactor
- Update bun.lock

* fix(tests): complete BDD comment normalization

- Fix remaining #when/#then patterns missed by initial sed
- Affected: state.test.ts, events.test.ts

---------

Co-authored-by: justsisyphus <justsisyphus@users.noreply.github.com>
2026-02-01 16:47:50 +09:00
YeonGyu-Kim
96e7b39a83 fix: use _resetForTesting() consistently to prevent flaky tests (#1318)
- Replace setMainSession(undefined) with _resetForTesting() in keyword-detector tests
- Add _resetForTesting() to afterEach hooks for proper cleanup
- Un-skip the previously flaky mainSessionID test in state.test.ts

Fixes #848

Co-authored-by: 배지훈 <new0126@naver.com>
2026-01-31 16:34:07 +09:00
justsisyphus
fa9bf4590c fix(test): add _resetForTesting to all session state tests 2026-01-17 17:04:40 +09:00
João Carlos Magalhães de Castro
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
YeonGyu-Kim
2064568124 fix: correct spawn mock type in session-notification test 2026-01-07 01:43:03 +09:00
João Carlos Magalhães de Castro
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
Sisyphus
0cee39dafb fix: properly mock utility functions in session-notification tests (#274)
The test mock for ctx.$ was not handling tagged template literals correctly,
causing it to ignore interpolated values. Additionally, utility functions that
check for command availability (osascript, notify-send, etc.) were returning
null in test environments, causing sendNotification to exit early.

Changes:
- Fixed template literal reconstruction in mock $ function
- Added spyOn mocks for all utility path functions
- All session-notification tests now passing (11/11)

Fixes #273

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
2025-12-27 23:22:17 +09:00
YeonGyu-Kim
a6ee5a7553 fix: Notification hook works weirdly for subagent sessions (#189)
* fix: Notification hook works weirdly for subagent sessions

- Added mainSessionID check to prevent notifications in subagent sessions
- Only trigger notifications for main session when waiting for user input
- Added comprehensive tests to validate the fix

Issue: https://github.com/code-yeongyu/oh-my-opencode/issues/92

* chore: changes by sisyphus-dev-ai

---------

Co-authored-by: codingsh <codingsh@pm.me>
Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
2025-12-25 06:58:03 +09:00