fix(desktop): drop orphaned FK outbox entries and cap sidecar heap - #1006
Merged
Merged
Conversation
Two fixes for the crash-then-data-loss scenario reported in #996: 1. persistence-outbox: a FOREIGN KEY constraint failure means the parent row (session or turn) no longer exists — typically after a sidecar crash. Retrying is futile and blocks the entire queue, causing all subsequent transcript writes to be lost. Treat FK errors the same as duplicate-id and poison-message: drop the orphaned entry and keep draining. In the reported case this would have prevented 27 689 flush-paused cycles and the 1024-entry outbox saturation. 2. agent-sidecar: add --max-old-space-size=2048 to the sidecar spawn args. The default V8 limit (~4 GB on 64-bit) lets a runaway session consume nearly all physical RAM before OOM-killing the process. A 2 GB cap triggers GC pressure earlier and leaves headroom for the main process and renderer on 16 GB machines. closes #996
This was referenced Sep 26, 2026
vastsa
pushed a commit
that referenced
this pull request
Sep 26, 2026
A sidecar that dies mid-turn settled its owning turn through `settleCrashedSession` with one fixed Plan-family code, `PLAN_APPROVAL_INTERRUPTED`, for every exit reason. Two consequences (issue #1077): - A crash inside an unrelated Agent conversation was recorded in the durable transcript as a plan-approval interruption, which misleads both the user and any triage reading the turn rows. - A heap-exhaustion death — the deterministic crash loop behind #1077, where the 2GB `--max-old-space-size` cap from #1006 turns an over-large session into an endless edit -> OOM -> restart cycle — looked identical to any other failure, hiding the one signal that points at the fix (the context is too large). Classify the exit once, from the stderr tail the transport already snapshots, and carry the verdict into every settlement: - `packages/agent-runtime/src/sidecar-crash.ts`: `classifySidecarCrash` recognizes the V8 fatal-error banner (`Reached heap limit`, `heap out of memory`, `CALL_AND_RETRY_LAST Allocation failed`, `Last few GCs`) and maps the kind onto two new shared codes, `AGENT_SIDECAR_OOM` and `AGENT_SIDECAR_CRASHED`. The codes live in the shared registry, so an unregistered code is a compile error. - Electron main `runtime/sidecar.ts`: both the ownership-kept settlement and the `releaseCrashedTurn` drop path take the classified code; the crash log additionally carries `crashKind` and the matching marker. The Plan-family code no longer appears anywhere in the file. - Headless `runtime-service.ts`: its sidecar link's exit info carries no stderr tail, so it cannot detect OOM; it still settles with the honest generic `AGENT_SIDECAR_CRASHED` instead of borrowing approval vocabulary. Specs: `08-error-codes.md` registers both codes in the turn-terminal table; `07-process-model.md` §4 crash policy names the classification. Both zh-CN mirrors updated. Validation (commit aa8e792, on top of ccf6672): - `pnpm check:pr-base` PASS - Full `node --test apps/desktop/test/*.test.mjs`: 2912 pass, 3 fail — `browser-cdp`, `bundled-plugins`, `plugin-work-panel-views`, which also fail on a clean upstream/main checkout (environment issues unrelated to this change) - New `sidecar-crash.test.ts` 5/5; `runtime-service.test.ts` 16/16 with the new `AGENT_SIDECAR_CRASHED` assertion; the `rpc-lifecycle-contract` source-contract test now pins the classification wiring and the absence of the old code - `pnpm docs:check` (81 en/zh pairs, 516 pages), `lint:biome`, typecheck for shared/agent-runtime/host-runtime/desktop: PASS - Not run: E2E suites that drive a live Electron app (no-local-E2E policy); the crash path is covered by the contract and unit tests above. Closes #1077's diagnostic half: the OOM itself needs a separate root-cause fix in how an over-large session grows the sidecar heap, and this change makes every occurrence visible and attributable in the transcript and logs.
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the crash-then-data-loss scenario in #996: sidecar V8 heap OOM → crash → orphaned outbox entries block the flush queue → all subsequent transcript writes lost.
Changes
1. persistence-outbox: treat FOREIGN KEY errors as non-retriable
A
FOREIGN KEY constraint failedmeans the parent row is gone (session/turn deleted or never committed after crash). Retrying is futile and blocks the entire queue. Now dropped like duplicate-id and poison-message errors.In the reported case this would have prevented 27,689 flush-paused cycles and the 1024-entry outbox saturation.
2. agent-sidecar: cap V8 heap at 2 GB
Add
--max-old-space-size=2048to the sidecar spawn args. The default ~4 GB limit lets a runaway session consume nearly all physical RAM before OOM-killing. A 2 GB cap triggers GC pressure earlier and leaves headroom for the main process and renderer on 16 GB machines.Tested
node --test apps/desktop/test/persistence-outbox.test.mjs— 10/10 pass (was 9, added FK test)tsc --noEmit— cleancloses #996