opencode >=1.14 freezes output.args via Immer before plugin hooks run.
Direct property assignment or Object.assign on a frozen object throws
TypeError. This helper replaces output.args with a shallow clone
containing the patch, avoiding mutation of the frozen original.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Refs #3772 (the rotation half — EPIPE shutdown-noise suppression
remains a separate follow-up).
`src/shared/logger.ts` appends every entry to `os.tmpdir()/oh-my-opencode.log`
via `fs.appendFileSync` with no size cap. On long-running or busy projects
the file grows into the multi-GB range — a real-world reproduction on one
machine showed a 4.5 GB `oh-my-opencode.log.1` accumulated from per-shutdown
noise across many sessions. Eats `%TEMP%` on Windows and `/tmp` on Unix.
Add size-based rotation inside the existing batched `flush()` path:
oh-my-opencode.log → oh-my-opencode.log.1
oh-my-opencode.log.1 → oh-my-opencode.log.2 (oldest dropped)
Cap is 50 MB per file; worst-case on-disk footprint is therefore ~150 MB.
The check runs only inside `flush()`, so the cost is amortized over
`BUFFER_SIZE_LIMIT` (50 entries) or the 500 ms flush timer. All filesystem
ops stay wrapped in try/catch — logging must never throw — and a failed
rotation leaves existing on-disk state intact rather than crashing the
agent. Pattern mirrors `src/openclaw/reply-listener-log.ts`, but with two
backup slots instead of one to keep a usable history window for debugging.
No config knobs in this iteration. The issue proposes `logs.max_size_mb`
/ `logs.max_files`, but the defaults are reasonable and adding schema is
more surface area than the bug warrants. Easy to promote later (the
existing test seams already let callers override the cap).
Tests:
- `src/shared/logger.test.ts` (new): under-threshold no-rotate, over-
threshold rotates to `.1`, repeated rotation evicts oldest, rotation-
failure-doesn't-throw, default path lives under `os.tmpdir()`. Uses a
`mock.module(...)` substring marker so `script/run-ci-tests.ts` routes
the file to its own bun process — the logger module's singleton state
otherwise gets contaminated by sibling tests that mock `./shared`.
Out of scope: suppressing specific shutdown-noise messages (EPIPE,
`unhandledRejection received during shutdown cleanup`). The rotation
cap bounds the disk impact regardless of which noise pattern is
generating volume; per-message suppression can stand on its own
merits in a follow-up.
When a background subagent emits [ALL BACKGROUND TASKS COMPLETE], the
plugin queues a parent-wake that ultimately calls
dispatchInternalPrompt against the parent session. If the user submits
a new prompt inside the ~250 ms post-dispatch hold window, both writes
land on the same OpenCode session-storage file at the same instant.
OpenCode's @parcel/watcher (which the plugin itself does not depend on,
but does indirectly trigger) batches those events into a TSFN callback
and dispatches them into a JS env that the renderer has just torn down
because the session view re-mounted around the user's new message ->
napi_fatal_error / SIGABRT on macOS arm64. Removing the plugin removes
the parent-wake, which is why removing OmO eliminates the crash.
Mitigation:
- Before flushPendingParentWake calls dispatchInternalPrompt, inspect
the parent session's message tail. If the most recent message is a
user message added inside PARENT_WAKE_USER_MESSAGE_IN_PROGRESS_WINDOW_MS
(default 2_000 ms), reschedule instead of dispatching. The user's own
prompt will drive the model; queued notifications will be re-flushed
on the next idle.
- Best-effort unref() of the long-lived pending-retry and dispatched-
wake bookkeeping setTimeouts. They previously pinned the host event
loop and prolonged the teardown window during which the watcher race
can fire.
The new option userMessageInProgressWindowMs is wired through
BackgroundManager via a module-level constant and is independently
testable.
Regression test parent-wake-user-message-race.test.ts covers:
- fresh user message -> dispatch deferred
- latest message is assistant -> dispatch proceeds
- user message older than window -> dispatch proceeds
- window=0 disables the guard
This is a surface-level mitigation of the most-likely root cause from
the audit; a deeper fix (singleton guard against plugin
double-instantiation under @opencode-ai/plugin@local reload, dispose
lifecycle for OpenCode plugin reload) is out of scope here.
Addresses cubic-dev-ai P1 + P2 findings on #4115.
The original PR relaxed `exitCode > 1` to `> 2` based on the (wrong)
claim that ripgrep exits 2 only on non-fatal I/O issues. ripgrep
actually uses exit code 2 for BOTH fatal errors (pattern syntax,
invalid args) AND non-fatal I/O issues; GNU grep (the fallback backend
in grep/cli.ts) likewise uses 2 for fatal errors. So `> 2` would
silently suppress fatal errors.
The correct fix is just `--no-messages`, which suppresses ripgrep's
stderr only for soft I/O issues (broken symlinks, permission denied)
while leaving fatal-error messages intact. With the gate kept at
`exitCode > 1 && stderr.trim()`:
- Broken symlink: ripgrep exits 2, stderr is empty (suppressed) →
`stderr.trim()` is falsy → gate fails → partial results survive.
- Fatal error: ripgrep exits 2, stderr has the real error message
(not suppressed by --no-messages) → gate triggers → error returned.
Reverting both `exitCode > 1` → `> 2` changes; keeping the
`--no-messages` flag additions and the regression test (test comment
updated to describe the cleaner architecture).
Verification: bun test src/tools/glob/ src/tools/grep/ → 30 pass / 0 fail.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses cubic-dev-ai P1 finding on #4113 (#4113 review).
The original chain `extractErrorName(error)?.toLowerCase().replace(...)`
is semantically safe — JavaScript optional chaining short-circuits the
ENTIRE access chain when the head returns null/undefined, so when
`extractErrorName` returns undefined the whole expression evaluates to
undefined without ever reaching `.replace()`. Verified empirically via
`const x = undefined; x?.toLowerCase().replace(/_/g, "")` returns
undefined with no crash.
Applying the suggested defensive `?.` before `.replace` anyway, since
it is semantically a no-op and explicit chaining at each hop is easier
for static analyzers to reason about.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3726.
## Root cause
Two coupled bugs in the ripgrep integration caused the glob and grep
tools to fail outright when the search path contained a broken
(dangling) symlink:
1. **Missing `--no-messages`** in `RG_FILES_FLAGS` (glob) and
`RG_SAFETY_FLAGS` (grep). ripgrep prints a stderr warning for every
broken symlink:
`rg: /path/to/broken-link: No such file or directory (os error 2)`
2. **Overly strict exit-code gate** in both `glob/cli.ts` and
`grep/cli.ts`: `if (exitCode > 1 && stderr.trim())` treated exit
code 2 as fatal and discarded the stdout, even though ripgrep exits
2 on *non-fatal* I/O issues (broken symlinks, permission denied)
while still printing valid results to stdout.
Combined effect: a single broken symlink anywhere in the search path
turned all subsequent file matches into an empty result with an error.
## Fix
- Append `--no-messages` to `RG_FILES_FLAGS` (`src/tools/glob/constants.ts`)
and to `RG_SAFETY_FLAGS` (`src/tools/grep/constants.ts`). This silences
ripgrep's non-fatal stderr warnings without changing match behavior.
- Relax the exit-code gate in both `glob/cli.ts` and `grep/cli.ts` from
`> 1` to `> 2`, matching ripgrep's documented contract:
- 0 = matches found
- 1 = no matches (success, just nothing matched)
- 2 = non-fatal I/O issues (partial success — stdout is still valid)
- >2 = fatal error
## Test changes
- New regression assertion in `src/tools/glob/cli.test.ts` for
`buildRgArgs` confirms `--no-messages` is included in the args.
- The grep change is symmetric (same flag, same rationale, same
exit-code constant) and rides on the parallel structure.
## Verification
- `bun test src/tools/glob/` → all pass (19 tests in cli.test.ts)
- `bun test src/tools/grep/` → all pre-existing tests still pass
- `bunx tsc --noEmit` → clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3607.
## Root cause
On Windows, `detectCommandShellType()` fell through to `detectShellType()`
for two common environments and incorrectly returned `"unix"`:
1. **SHELL points at a Unix-shaped path** (e.g. Git Bash sets
`SHELL=/usr/bin/bash` on a fresh Windows install).
The `detectWindowsShellType(process.env.SHELL)` probe didn't recognize
`bash` as a Windows shell, so the function fell through and
`detectShellType()` returned `"unix"`.
2. **MSYSTEM is set but SHELL is not** (Git Bash leaves MSYSTEM permanently
set system-wide even when the active shell is PowerShell).
The fall-through path returned `"unix"` via the MSYSTEM check.
In both cases, the hook then prepended `export KEY=val;` to git commands,
which PowerShell rejects with:
`export : 无法将"export"项识别为 cmdlet...`
OpenCode on Windows runs the bash tool through a Windows shell
(PowerShell by default, cmd as the user-overridable fallback), regardless
of MSYSTEM or a Unix-shaped SHELL set by Git Bash — so the env prefix
must use Windows-compatible syntax.
## Fix
`detectCommandShellType()` now short-circuits on `process.platform === "win32"`:
- If `SHELL` points at a recognized Windows shell (`cmd.exe`, `powershell.exe`,
`pwsh.exe`), return that.
- If `SHELL` and `MSYSTEM` are both unset, fall back to `ComSpec` then to cmd.
- Otherwise, default to PowerShell — matching what OpenCode actually spawns.
`detectShellType()` is unchanged; other callers (including non-Windows
platforms) are unaffected.
## Test changes
Three pre-existing tests encoded the buggy behavior as expected behavior
and have been updated to assert the new PowerShell syntax with a
`(#3607)` marker and a comment explaining why a Unix-shaped SHELL on
win32 must still resolve to PowerShell. WSL is not affected because in
WSL `process.platform === "linux"`, not `"win32"`.
- `src/hooks/non-interactive-env/`: 24 tests pass / 0 fail
- `bunx tsc --noEmit`: clean
## Note on issue thread
The sisyphus-bot triage comment on #3607 framed this as a policy choice
between (A) forcing Windows env-prefix syntax and (B) resolving against
the OpenCode-configured shell. This PR implements option (A) as the
minimal surgical fix; option (B) remains a follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses cubic-dev-ai bot review on #4113 (P2): the RESOURCE_EXHAUSTED
and snake_case insufficient_quota fixtures contained quota-shaped
messages that already matched pre-existing message regexes, so the tests
passed even without the new errorName allow-list entry and the
underscore normalization respectively.
Replace both fixture messages with a generic "Request failed." so the
only path to a `quota_exceeded` classification is via the new code:
- RESOURCE_EXHAUSTED: only the new `errorName?.includes("resourceexhausted")`
match on the normalized name can fire.
- insufficient_quota (snake_case): only the new underscore-stripping
normalization can route the name to `insufficientquota` and match the
existing allow-list entry.
The third new test (Google ResourceExhausted message-only) is unchanged
because its message uniquely matches only the new
`/resource.?exhausted/i` pattern and not any existing quota regex.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3937.
Adds three small classification gaps to `classifyErrorType` so that
quota-exhaustion errors from a wider range of providers trigger
configured fallback chains instead of looping retry attempts:
- Normalize error names by stripping `_` and `-` so snake_case /
SCREAMING_SNAKE_CASE provider names (`insufficient_quota`,
`RESOURCE_EXHAUSTED`, `rate_limit_exceeded`) match the existing
alphanumeric `.includes()` checks.
- Add `resourceexhausted` to the quota error-name allow-list to cover
Google Generative AI's gRPC code 8 / `ResourceExhausted` surface.
- Add `/resource.?exhausted/i` to the quota message-pattern list so the
same error surface is caught when the provider only sets a generic
error name but puts the signal in the message.
Three new regression tests in
`quota-error-classifier.regression.test.ts` cover:
- Google `RESOURCE_EXHAUSTED` (gRPC error name + quota-shaped message)
- Google `ResourceExhausted` message form without HTTP status
- OpenAI snake_case `insufficient_quota` error name
No existing tests were touched; the underscore normalization preserves
all existing `.includes()` matches by rewriting the one underscore-bearing
literal (`ai_loadapikeyerror` → `ailoadapikeyerror`) so previously
matched names still resolve.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Guard command.execute.before against injecting when parts already
contain auto-slash-command tags, preventing duplication when both
chat.message and command.execute.before fire for the same slash command.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use one dispatchInternalPrompt surface with mode: async | sync so source, settle, hold, timeout, status checks, reservations, and release semantics stay in one runner. Keep the old helper names temporarily so caller migration can land atomically in follow-up commits.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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)
Root cause: `formatTaskResult` and `formatFullSession` both call
`client.session.messages({ path: { id: task.sessionId } })` with no
timeout. The OpenCode SDK explicitly disables fetch timeout
(`req.timeout = false` in `packages/sdk/js/src/client.ts:37`), so when
`session.processor` enters an "Aborted process" loop (same condition
patched in #4086 for the Read hot path) every subsequent
`session.messages` call hangs forever.
User-visible impact: the parent agent running a heavy slash command
such as `/init-deep` fires many background `task` agents in parallel
and then calls `background_output(task_id, block=true)` for each. The
existing `timeoutMs` (max 10min) caps only the polling loop that waits
for the child task to transition out of `running`. Once the child is
`completed`, the loop exits and the code falls through to
`formatTaskResult` / `formatFullSession`. The 10min cap does not apply
to that post-completion fetch, so a wedged `session.messages` leaves
the tool call hanging indefinitely. The parent session appears stuck
to the user (the symptom they report on `/init-deep ultrafucking
deep`).
Fix: race the underlying `session.messages` call against a 5s timeout
through a new `withSdkCallTimeout` helper local to
`src/tools/background-task/` (mirrors `withFetchTimeout` in
`src/shared/dynamic-truncator.ts` and `withDispatchTimeout` in
`src/shared/prompt-async-gate.ts`). On timeout the caller returns a
clearly-marked `"Error fetching messages: ... timed out after 5000ms"`
fallback so the agent can recognise the failure and continue instead
of waiting forever.
Tests:
- New `sdk-call-timeout.test.ts` pins the fix with three BDD cases
using a never-settling mock and the new
`_setBackgroundOutputFetchTimeoutMsForTesting(50)` override:
(1) `formatTaskResult` resolves to the timeout fallback under the
fetch budget, (2) same for `formatFullSession`, (3) two parallel
callers against the wedged client both resolve cleanly.
- All three failed (timed out) before the fix and pass in 51ms each
after.
Verification:
- `bun test src/tools/background-task/` -- 33 pass.
- `bun test` (full suite) -- 7014 pass, 1 skip, 0 fail across 723
files.
- `bun run typecheck` -- clean (tsgo --noEmit).
- LSP diagnostics on the four changed files -- 0 errors.
- Manual QA harness `.local-ignore/init-deep-hang-qa.ts` (gitignored)
drove production default 5s timeout against a wedged client:
serial `formatTaskResult` resolved in 5001ms, serial
`formatFullSession` in 5002ms, 10 parallel `formatTaskResult` calls
all resolved within 5002ms with the timeout fallback. Without the
fix every call hangs indefinitely.
Refs: builds on #4086 (dynamic-truncator) which patched the Read hot
path of the same SDK hang.
Detect OpenCode promptAsync calls that return before a child session has any durable message, and surface a prompt acceptance error before the generic five-minute sync poll timeout.
Add a failing-first regression for the idle zero-message case and keep the existing durable-message completion path covered.
Debugging-Journal: .debugging
Unresolved git merge conflict markers (<<<<<<<, =======, >>>>>>>) in
TypeScript source files break parsing and can cause the plugin to fail
at runtime or tests to hang with cryptic errors. This guard scans all
.ts/.tsx/.json files under src/ and fails the test suite if any
conflict markers are found.
Closes #debugging-hang-investigation
YeonGyu-Kim