Commit Graph

16 Commits

Author SHA1 Message Date
SpencerJung ec9997b7a6 fix(background-agent): keep cleanup error listener active
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-05-22 17:12:14 +09:00
YeonGyu-Kim 0e49767a3e Merge pull request #4026 from dihak/fix/graceful-shutdown-exit
fix(process-cleanup): call process.exit() after SIGTERM cleanup
2026-05-21 00:43:32 +09:00
YeonGyu-Kim 4d417a33b6 fix(process-cleanup): stop force-exiting opencode on transient unhandled errors
Root cause of the user-visible `/init-deep ulw` hang (session
`ses_1cb9c3013ffesUOy5H3QOIya4K`): the plugin's `unhandledRejection` and
`uncaughtException` listeners were calling
`scheduleForcedExit(handler(error), 1, true)`, which both ran the entire
`cleanupAll()` chain (BackgroundManager shutdown, tmux pane closure,
team-mode teardown) and then `process.exit(1)`'d the host. Under heavy
slash commands like `/init-deep ultrafucking deep`, a single mid-stream
error (e.g. opencode's own `session.processor` Aborted-process condition,
or a transient socket reset) would:

  1. trigger the listener,
  2. abort the in-flight background tasks (`session.error
     MessageAbortedError` for both child sessions in the log),
  3. close the tmux panes the user was watching,
  4. immediately kill the host via `process.exit(1)`.

From the user's seat that looked like a frozen TUI, which is what they
reported as "ulw 여전히 멈추는데". The error blob also logged as `{}`
because `JSON.stringify(new Error(...))` strips non-enumerable Error
fields, so the previous log line carried no diagnostic value.

This change makes the global `uncaughtException` / `unhandledRejection`
listeners log-only:

* New `describeProcessCleanupError()` extracts `{name, message, stack}`
  from Error instances, falls back to a structured `{raw: ...}` payload
  for plain objects / primitives, so the log now actually says what
  failed.
* `registerErrorEvent()` no longer runs cleanup and no longer calls
  `scheduleForcedExit`. It detaches itself, logs a single explanatory
  line, and returns. Bun's default crash behaviour is already suppressed
  for these events when a listener is present, so the host now genuinely
  survives transient streaming errors instead of being killed by our
  own helper.
* Signal handlers (`SIGINT` / `SIGTERM` / `SIGBREAK` / `beforeExit` /
  `exit`) keep their existing behaviour and still run `cleanupAll()`
  before the host terminates — that is now the only path that tears
  down background tasks and tmux panes.

Tests are updated to lock in the new contract:

* New regression `#given scheduleForcedExit enabled AND unhandledRejection
  fires #when the listener runs #then process.exit is NOT called AND
  process.exitCode stays 0 AND no cleanup runs` (and the
  uncaughtException twin) re-enables `scheduleForcedExit`, spies on
  `process.exit` plus `globalThis.setTimeout`, and asserts none of them
  are touched. Without the fix this test failed exactly like the
  observed hang (exit called once, exitCode set to 1).
* The existing "manager shuts down before process exits" tests are
  rewritten to assert the opposite: cleanup is NOT invoked from the
  error path.
* A complementary `'exit'` listener test pins the real shutdown
  contract (`exit` event still triggers `cleanupAll`).
* A new `#given describeProcessCleanupError` block covers the four
  shapes (Error, plain object with own fields, empty object, primitive).
* The unregister assertion is tightened to check the listener count
  drops back to baseline (previously it relied on a side effect of the
  old cleanup-on-error path).

Manual QA: `bun /tmp/process-cleanup-smoke-test.ts` (out-of-tree smoke
driver) emits five back-to-back unhandledRejection/uncaughtException
events with various payload shapes and prints
`SMOKE_TEST_OK survived 5 emissions; exitCode=0; shutdownInvocations=0`,
confirming the host survives and no spurious cleanup runs.

`bun test` runs green for the affected modules:
  - src/features/background-agent (528 tests)
  - src/create-managers + src/plugin (218 tests)
  - src/hooks/{unstable-agent-babysitter,ralph-loop,keyword-detector}
    (225 tests)
2026-05-17 15:11:44 +09:00
Dihak ec49d6af3f fix(process-cleanup): call process.exit() after SIGTERM cleanup
scheduleForcedExit was called without exitAfterCleanup=true for SIGTERM/SIGINT.
After cleanup completed, timeout cleared but process.exit() never called.
Process stayed alive waiting for event loop to drain.

Benchmark (systemctl stop):
- Before: 10s+ timeout (systemd had to SIGKILL)
- After: ~30ms instant shutdown

Test results (3 runs):
- Test 1: 30ms
- Test 2: 28ms
- Test 3: 27ms
2026-05-15 00:56:05 +07:00
MoerAI dd68f324d0 fix(background-agent): add OMO_DISABLE_PROCESS_CLEANUP env opt-out for global handlers (fixes #3856)
Currently `registerManagerForCleanup` unconditionally installs global `uncaughtException` and `unhandledRejection` listeners that call `process.exit(1)` after cleanup. For users who load the plugin but never run background-agent tasks, these handlers turn transient streaming errors (e.g. undici `UND_ERR_SOCKET` mid-stream resets from `api.githubcopilot.com`) into a full process kill — opencode dies after every flaky response.

Add an `OMO_DISABLE_PROCESS_CLEANUP` env var (accepts 1/true/yes/on, case-insensitive) that skips just the error-event registration. Signal handlers (SIGINT/SIGTERM/SIGBREAK/beforeExit/exit) remain installed so graceful shutdown of any in-flight cleanup targets still runs. This is the lowest-risk near-term mitigation suggested in the issue (option #2): users opting in pay the cost of unhandled rejections themselves, but no longer lose their session to a transient socket reset.

Verification: 6 new test cases cover env-var precedence (set/unset, truthy/falsy values), signal-handler preservation, and behavior under `uncaughtException`. All 20 tests in process-cleanup.test.ts pass. Typecheck clean. Manual QA confirms env-var detection works end-to-end.
2026-05-11 18:43:40 +09:00
YeonGyu-Kim 32fab88d68 fix(process-cleanup): add test seam to prevent process.exitCode contaminating bun runner
scheduleForcedExit() sets process.exitCode which taints the bun test runner's
own exit code for the entire suite. This caused CI to fail even though all
tests passed individually.

Fix:
- Add __disableScheduledForcedExitForTesting / __enableScheduledForcedExitForTesting
  seams to skip scheduleForcedExit() during tests
- beforeEach disables forced exit; afterEach re-enables
- The 'fallback exit timer' test explicitly re-enables to verify setTimeout/clearTimeout
- Remove process.exitCode and exitSpy assertions that required forced exit to be active
  (shutdown call counts are sufficient to verify behavior)
2026-05-05 19:04:40 +09:00
YeonGyu-Kim 45452a039c Merge pull request #3731 from yizhifengye/fix/process-cleanup-infinite-loop-epipe
fix(background-agent): detach error listener before running body to s…
2026-05-05 04:24:01 +09:00
auyua9 875fffb7d1 fix(background): exit after fatal cleanup 2026-05-01 16:04:03 +08:00
yizhifengye 49c2a40251 fix(background-agent): detach error listener before running body to stop re-entrant log loop
When shutdown() itself emitted uncaughtException (e.g. EPIPE while closing
a broken pipe), the error listener re-entered itself, re-logged, re-ran
cleanup, and threw EPIPE again. The 6 s forced-exit timer could not fire
because every re-entry stalled the event loop with fresh synchronous work.

Users hit this after v3.17.5 and observed 100+ GB of log lines written to
disk within minutes, with one confirmed report of a 157 GB log file filling
the filesystem.

Detaching the listener with process.off() before running log() + handler()
breaks the loop at the first re-emit: the second event has no listener to
invoke, and the first invocation's scheduleForcedExit() proceeds normally.
2026-04-30 14:47:09 +08:00
YeonGyu-Kim b9d2acdcf9 fix(background-agent): run manager cleanup on uncaughtException and unhandledRejection
Signal handlers covered SIGINT/SIGTERM/SIGBREAK/beforeExit/exit, but a
synchronous throw or a top-level rejected promise terminated the process
without letting TmuxSessionManager (or any other registered manager) run
its shutdown hook. That reliably left orphan tmux panes after an opencode
crash.

Added registration for uncaughtException and unhandledRejection that fan
out through the existing cleanupAll() path, set process.exitCode = 1,
and arm the same 6 second forced-exit guard we use for signals. Test
helpers hold process-level spies so the new tests do not leak listeners
between runs.
2026-04-18 19:32:04 +09:00
YeonGyu-Kim fabbcaa4b7 refactor(runtime): replace unicode dashes in prompt strings
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
2026-04-04 01:27:51 +09:00
YeonGyu-Kim 7d6f47bf3d fix: resolve process-cleanup signal delay and hashline_edit tool name mismatch 2026-03-31 16:59:04 -07:00
YeonGyu-Kim 127626a122 fix(#2822): properly cleanup tmux sessions on process shutdown
Two issues fixed:
1. process-cleanup.ts used fire-and-forget void Promise for shutdown
   handlers — now properly collects and awaits all cleanup promises
   via Promise.allSettled, with dedup guard to prevent double cleanup
2. TmuxSessionManager was never registered for process cleanup —
   now registered in create-managers.ts via registerManagerForCleanup

Also fixed setTimeout().unref() which could let the process exit
before cleanup completes.
2026-03-27 15:23:48 +09:00
YeonGyu-Kim cf276322a3 fix(background-agent): handle async shutdown in process-cleanup signal handlers 2026-03-12 01:37:03 +09:00
YeonGyu-Kim d1e5bd63c1 fix: address Oracle + Cubic review feedback for background-agent refactoring
- Revert getMessageDir to original join(MESSAGE_STORAGE, sessionID) behavior
- Fix dead subagentSessions.delete by capturing previousSessionID before tryFallbackRetry
- Add .unref() to process cleanup setTimeout to prevent 6s hang on Ctrl-C
- Add missing isUnstableAgent to fallback retry input mapping
- Fix process-cleanup tests to use exit listener instead of SIGINT at index 0
- Swap test filenames in compaction-aware-message-resolver to exercise skip logic correctly
2026-02-22 12:14:26 +09:00
YeonGyu-Kim d53bcfbced refactor(background-agent): extract inline logic from manager.ts into focused modules
Extract 5 concerns from BackgroundManager into dedicated modules:
- error-classifier.ts: enhance with extractErrorName, extractErrorMessage, getSessionErrorMessage, isRecord
- fallback-retry-handler.ts: standalone tryFallbackRetry with full retry logic
- process-cleanup.ts: registerManagerForCleanup/unregisterManagerForCleanup
- compaction-aware-message-resolver.ts: isCompactionAgent/findNearestMessageExcludingCompaction
- Delete notification-builder.ts (duplicate of background-task-notification-template.ts)

Manager.ts method bodies now delegate to extracted modules.
Wire duration-formatter.ts and task-poller.ts (existing but unused).

manager.ts: 2036 -> 1647 LOC (19% reduction).
All 161 existing tests pass unchanged.
2026-02-22 11:58:57 +09:00