mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
SidandGitHub
1bd1f3a661
* fix(daemon): surface OpenCode error frames + treat empty-output runs as failed Closes #691. OpenCode runs would silently complete in ~3 seconds without producing any visible chat output and still be rendered as a successful turn — three independent bugs along the structured-stream path conspired to produce this silent-failure shape. ## Bug 1 — `apps/daemon/src/json-event-stream.ts:85-91` OpenCode emits structured error frames on stdout (e.g. provider auth failures, network errors, schema mismatches) and still exits 0. The parser was downgrading these to `{type: 'raw', line: ...}`, which the chat UI does not render as an assistant message. The error string was discarded as "no-op output." Fix: emit a proper `{type: 'error', message, raw}` event matching the qoder-stream contract that the daemon's existing error-handling path already recognises. ## Bug 2 — `apps/daemon/src/server.ts:4199-4205` Even after Bug 1 was fixed, the json-event-stream branch wired the parser to a bare `(ev) => send('agent', ev)` lambda — bypassing the `sendAgentEvent` wrapper that interprets `type:'error'` events and sets the `agentStreamError` flag the close handler reads to flip the run to `failed`. So an emitted `error` event would just be forwarded as a no-op `agent` SSE event with no lifecycle effect. Fix: route json-event-stream through `sendAgentEvent`, mirroring the qoder-stream-json wiring at line 4175. ## Bug 3 — `apps/daemon/src/server.ts:4220-4234` Even after Bugs 1 and 2 are fixed, there's still a class of runs where OpenCode never emits any error frame, never emits any substantive event, and exits 0. Pre-fix this was marked `succeeded` and the user saw a blank chat with no diagnostic. Fix: track `agentProducedOutput` inside `sendAgentEvent` (set on `text_delta`, `thinking_delta`, `tool_use`, `tool_result`, `artifact` — deliberately NOT on `status` / `usage`, since a model can emit token-usage numbers for an empty completion). When the close handler sees `code === 0 && trackingSubstantiveOutput && !agentProducedOutput` the run is marked `failed` with an explicit AGENT_EXECUTION_FAILED SSE error so the chat shows a clear reason instead of a silent empty turn. The check is gated by `trackingSubstantiveOutput` so it only fires on streams that actually contribute to the output flag (currently qoder-stream-json and json-event-stream). ACP sessions and plain stdout streams keep their existing success/failure determination. ## Tests - 3 new unit tests in `apps/daemon/tests/json-event-stream.test.ts` pin the OpenCode error event shape: full repro (`error.data.message`), `error.name` fallback, and the generic-fallback shape when `error` is empty. - All 60 daemon test files (851 tests) pass on `pnpm --filter @open-design/daemon test`. All 42 web test files (309 tests) pass on `pnpm --filter @open-design/web test`. - Full repo `pnpm typecheck` clean. ## Live verification Verified end-to-end via a stub `opencode` binary that mimics each of the failure shapes against `pnpm tools-dev run web`: 1. Stub emits `{"type":"error",...}` then `exit 0` — run now ends as `failed` with the OpenCode error message surfaced as an SSE `error` event. Pre-fix this was `succeeded` with an empty chat. 2. Stub emits nothing then `exit 0` — run now ends as `failed` with "Agent completed without producing any output…" diagnostic. Pre-fix this was `succeeded` with an empty chat. 3. Stub emits a normal `step_start` / `text` / `step_finish` sequence then `exit 0` — run still succeeds. (Regression check.) ## Out of scope (mentioned for the next person) - `claude-stream-json` and `copilot-stream-json` still wire to a bare `(ev) => send('agent', ev)` and don't currently parse `type:'error'` frames. If their CLIs ever start emitting structured error events the same pattern (route through `sendAgentEvent` + emit proper `type:'error'`) applies. Not in scope here because we have no evidence those CLIs do this today, and changing the wiring without a confirmed failure mode risks regressing currently-working flows. - ACP sessions (`pi-rpc`, `acp-json-rpc`) own their own success / failure determination via `acpSession?.hasFatalError()` and the empty-output guard explicitly skips them via `trackingSubstantiveOutput`. - Plain stdout streams have no event-level tracking, so the empty- output guard skips them too. Diagnosing a no-output plain-stream agent is a separate problem that needs different signals. * chore: retrigger CI on top of green main (post #697 i18n backfill)