Repository navigation
feat(incident): always-on App Run Journal (Phases 1–8 + run-identity unification) - #363
Merged
Merged
Conversation
Adds a main-owned, always-on diagnostic spine (not gated by AGENT_CODE_PERF)
that gives crashes, restarts, and unclean shutdowns durable on-disk evidence in
normal user runs. This is Phase 1 of the plan in
docs/plans_and_ideas/2026-06-25-performance-incident-journal-plan.md: the file
backbone only (manifest + heartbeat + events + clean-shutdown marker +
retention). Crash/freeze hooks, prior-run classification, and cross-process
correlation (Phases 2-8) are intentionally NOT in this PR.
What lands:
- src/main/incident/{AppRunJournal,journalTypes,appRunIds}.ts — the engine:
one appRunId per process, overwrite-only heartbeat.json (5s), append-only
bounded events.jsonl (drop-oldest at 2000), synchronous clean-shutdown marker.
- index.ts — construct the journal right after the state lock, instrument the
brittle startup sequence (toolchain / tmux detect+recovery / mcp_host /
window create) and the quit path.
- storage/paths.ts — INCIDENT_RUNS_DIR storage root (~/.config/agent-code/incidents/runs).
- storage/debugRetention.ts — an `incidents` budget bucket that protects the 50
newest run dirs while letting older ones age out of the disk budget.
Hardening from multi-agent review (the reasons the slice is shippable, not just
present):
- Quit-time durability: events queued at shutdown (app.before_quit /
app.will_quit) were routed through the async flush queue, which never drains
before Electron exits — they were guaranteed-lost on every clean quit. stop()
and markCleanShutdown() now drain `pending` SYNCHRONOUSLY (flushSync).
- Write-failure no longer silently drops the newest batch: a failed async append
re-queues the batch (bounded) instead of discarding it during exactly the
disk-trouble the journal exists to record.
- Startup lock-leak guard: a throw out of startApp (toolchain/mcp_host failure)
used to become an unhandledRejection that left the app hung with the state
lock held, blocking the NEXT launch. whenReady().then(startApp) now has a
.catch that journals the error, releases the lock, and quits.
- Retention guarantee fixed: the per-bucket cap pass now honors the `protected`
flag, so the 4% incidents cap can no longer evict a protected recent run
(the TTL and global passes already did).
- Security: the always-on manifest no longer persists the process-lock token
(a removal secret) — only the lock path is recorded.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…model)
Builds the incident layer on top of the Phase-1 backbone. This is the highest-
value next slice and is ordered ahead of the prior-run classifier (Phase 2) on
purpose: the classifier needs the signal these hooks produce, and these hooks
are the entire reason the journal exists ("durable evidence when the flag is
off, the renderer is gone, main is about to exit").
Data model — the missing incidents.jsonl:
- journalTypes.ts: AppRunIncident / AppRunIncidentInput / kind + severity unions.
- appRunIds.ts: createIncidentId (run-scoped, so timestamp + short random).
- AppRunJournal.recordIncident(): writes incidents.jsonl SYNCHRONOUSLY and
flushSync()s the pending events first, so the breadcrumb timeline leading up
to an incident survives even if the process dies milliseconds later (the 1s
async flush would otherwise lose the final pre-crash second). The incident is
also mirrored as an event so a single events.jsonl scan shows it in context;
incidents.jsonl stays a clean triage index. Error normalization shared with
recordError().
Hooks:
- installCrashHooks.ts (process): uncaughtException -> sync fatal incident +
synchronous lock release + exit (recursion-guarded; deliberately NOT a clean
shutdown); unhandledRejection coalesced by message and capped; process warning.
- installWindowIncidentHooks.ts (app/window): render-process-gone,
child-process-gone, per-window unresponsive/responsive (with frozen duration),
preload-error, did-fail-load (ERR_ABORTED filtered). Attached via
browser-window-created so every current and future window is covered and main
reports renderer death on its own.
- index.ts: install both early in startApp; start electron crashReporter with
uploadToServer:false to capture NATIVE crashes (V8 aborts, native-addon
SIGSEGV, GPU death) that never reach JS. Local-only, no telemetry.
tsc --noEmit: clean for all changed files.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On startup, classify how the PREVIOUS run ended and attribute it on the current run. The most valuable moment to classify a crash is the next launch: the dead process can't report itself, but its on-disk evidence (missing clean-shutdown marker, crash incidents, heartbeat presence) still can. - previousRunClassifier.ts: read-only scan of the most recent prior run dir (lexical sort of ISO-timestamped names == chronological). Classifies as clean / unclean_shutdown / main_crash_suspected / renderer_crash_suspected / force_quit_or_power_loss / unknown using the clean-shutdown marker + incidents.jsonl tail + heartbeat presence. Tolerates a torn final JSONL line (expected after an unclean exit). Never mutates prior-run evidence. - journalTypes.ts: add the app.prior_unclean_shutdown incident kind. - index.ts: after the journal + crash hooks are up, classify the prior run and record an app.prior_unclean_shutdown incident (error severity for crash-like classifications, warn otherwise); guarded so classification can never block startup. This deliberately follows the crash hooks (Phase 3): without their signal the classifier could only ever say "unclean" vs "unknown". Native-crash correlation (matching a Crashpad minidump) and stale-lock-owner evidence are left as enrichments for a later pass. tsc --noEmit: clean for all changed files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Juliusolsson05
changed the title
…#1) The architecture review's #1 finding: AppRunJournal and PerformanceService each minted their OWN unrelated run id (appRunId vs `${Date.now()}-${pid}`), which re-creates the exact "match subsystems by timestamp" problem the journal exists to kill, and would force Phase 7 to retro-stamp ids onto perf runs/bundles. Fix the identity at the source instead of stamping it on later: - appRunIds.ts: getAppRunId() — a lazily-minted, process-wide singleton. The first diagnostic to start mints it (the always-on journal in normal runs; PerformanceService at module load when AGENT_CODE_PERF is on); everyone else gets the same value. One id for the whole run. - AppRunJournal: consume getAppRunId() instead of minting its own. - PerformanceService: runId now = getAppRunId() when enabled (still null when disabled, so every `runId === null` gate behaves exactly as before). When perf is on, its run id is byte-identical to the journal's — incidents and perf runs finally correlate by one key. Heap snapshots / debug bundles (Phases 4/7) can now adopt the same getter for free. Deliberately NOT done (documented in startProbes): collapsing the two 5s event-loop/memory samplers. That overlap exists ONLY when AGENT_CODE_PERF is enabled (normal runs never start the perf service), the two feed different sinks (OTel metrics vs. always-on heartbeat.json), and unifying them would couple the heavy OTel pipeline to the always-on spine for one extra dev-only timer. Identity was the real correlation risk; that's fixed. Sampler dedup is left as a noted, low-value follow-up rather than a reckless rewire. tsc --noEmit: clean for all changed files. runId is treated opaquely everywhere (no code parses its format), so the value-shape change is safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The heap watchdog already writes a near-OOM snapshot, but nothing told the rest
of the app it happened — a silent artifact. Now it's a durable incident.
- heapWatchdog.ts: on trip, fire an onHeapPressure callback with heapUsed /
heapLimit / rss / uptimeMs / snapshotPath. The callback (not a direct journal
import) keeps the watchdog self-contained. The snapshot filename is now
stamped with the canonical appRunId, so the snapshot, the heap.pressure
incident, and the run's other diagnostics correlate by one key (this is the
payoff of the run-identity unification). Single-shot behavior unchanged;
emitting the incident is fully defensive so it can never destabilize the
near-OOM capture path.
- journalTypes.ts: add the heap.pressure incident kind.
- index.ts: wire the callback to recordIncident({ kind: 'heap.pressure', ... }).
tsc --noEmit: clean for all changed files.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Renderer-side breadcrumbs that flow into the main-owned journal as non-fatal incidents. Enrichment only — main's window hooks remain the source of truth for renderer death; this adds the JS-level detail main can't see. - preload/api/incident.ts: one-way `reportIncident` sender (send, not invoke — fire-and-forget; never blocks the renderer). Wired into the flat api surface. - main/ipc/incident.ts: registerIncidentIpc(journal) — defensively parses the cross-boundary payload, routes renderer.error / renderer.unhandledrejection to recordIncident() and renderer.bootstrap to a routine event. - ipc/index.ts + index.ts: thread appRunJournal through IpcDeps so the handler reaches the journal (first time the journal is exposed to the IPC layer). - renderer/app/main.tsx: window 'error' + 'unhandledrejection' listeners, attached before React mounts, rate-limited (coalesce by message over 5s) and redacted (message<=200 chars, stack<=6 lines) at the boundary so an error storm can't flood the journal or IPC. - renderer/app/AppErrorBoundary.tsx: catches React subtree throws, reports them, and shows a minimal fallback instead of a white screen. Wraps <App/>. - journalTypes.ts: add renderer.error / renderer.unhandledrejection kinds. tsc --noEmit: clean on tsconfig.node.json AND tsconfig.web.json for all changed files (window.api.reportIncident typing flows through the preload Api type). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Manual debug bundles now carry the run's incident context, so a bundle is
self-contained for triage instead of requiring timestamp-matching against the
incidents dir. Done in main's saveDebugBundle() — the single bottleneck every
bundle passes through, and the side that owns getAppRunId() + the filesystem.
- Stamp the canonical appRunId into the bundle's manifest.json (the renderer
builds the manifest without one; main amends it post-write).
- Copy a bounded tail of events.jsonl (128 KiB) + incidents.jsonl (64 KiB) and
the whole heartbeat.json into the bundle as incident-*.{jsonl,json}.
- readJsonlTailBytes() seeks to the file tail and drops the first partial line
so the copy stays valid JSONL and cheap even for a long run's events.jsonl.
Manual-only (autosave returns earlier — it's a high-frequency cache). Entirely
best-effort: nothing here can fail the save the user asked for.
tsc --noEmit: clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Instruments the session lifecycle's single highest-value failure: a renderer pane writing input to a session id main has no backend for. This is the durable signal for the "restored agents are null until reload" bug — previously a silent `return false` in SessionManager.write() that was brutal to diagnose from the renderer. - SessionManager: optional AppRunJournal constructor param (null-guarded). On a write miss, record a session.input_write_failed incident (severity error, reason backend_session_not_found, sessionId + dataLength in context), coalesced to the FIRST miss per session id so a broken renderer can't flood incidents.jsonl. - index.ts: pass appRunJournal into the SessionManager constructor. - journalTypes.ts: add the session.input_write_failed kind. Scope note: this covers the explicitly-named Phase-5 failure. Broader routine session lifecycle EVENTS (spawn requested/started, pty first byte, normal exit) are deferred to a follow-up rather than sprawling many edits across this 1000-line file in one commit — the failure incident is the diagnostic payoff. tsc --noEmit: clean for changed files (the lone error is the pre-existing claude-code-headless unbuilt-package import at line 8, present on main). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Instruments the orchestration/MCP path — the exact path observed flaking with "prompt delivery failed / not ready" — with the failures that matter most. - BuiltInMcpHttpHost: appRunJournal added to BuiltInMcpDependencies; start() records a fatal mcp.host_start_failed incident on bind error / invalid bind address (if the host can't bind, every tool is dead for the run). - createBuiltInMcpServer: orchestration.prompt_delivery_failed incident at both submitPrompt failure points — create_agent bootstrap and send_prompt — with the session id + provider message (this is the durable record of the exact delivery failures seen during this work). `dependencies` was already in scope. - OrchestrationBridge: setJournal() injection (it's built at module-eval before the journal exists); records orchestration.request_timeout when the renderer never answers within 30s — a renderer hang the renderer itself can't report. - index.ts: pass appRunJournal into builtInMcpHost.setDependencies(...) and call orchestrationBridge.setJournal(appRunJournal). - journalTypes.ts: add orchestration.request_timeout / orchestration.prompt_delivery_failed / mcp.host_start_failed kinds. Scope note: high-volume routine MCP events (mcp.request.received, per-tool start/complete) are intentionally NOT recorded — they'd need sampling/coalescing to avoid flooding; the failure incidents are the diagnostic payoff. tsc --noEmit: clean for all changed files (only pre-existing unbuilt-package env noise remains, present on main). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Juliusolsson05
changed the title
Backstop against the historical "persistent logging ate 500 GB" failure mode. heartbeat.json is overwrite-only (constant size) and retention prunes old run dirs, but events.jsonl/incidents.jsonl are append-only and the newest 50 runs are protected from pruning — so a pathological record() loop in one long-lived run could still grow unbounded. Add MAX_RUN_JOURNAL_BYTES (50 MiB) tracked across events.jsonl + incidents.jsonl per run via reserveJournalBytes(); once hit, further events/incidents are dropped (logged once). Each run's journal is now provably bounded; with the incidents retention bucket on top, the whole incidents tree cannot run away. 50 MiB is ~10000x a normal run (~5 KB observed), so it only trips on a genuine fault — and when it does, the journal refusing to grow is exactly right: a diagnostic must never be the thing that fills the disk. tsc --noEmit: clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses the findings from a 4-lens review (engine, integration, disk-safety,
adversarial).
Blockers / High:
- start() no longer gates app launch: mkdir/writeFile wrapped in try/catch; on a
full/read-only ~/.config it degrades to a no-op journal and the app boots.
(Adversarial HIGH-1 — the disk-safety feature could itself brick boot.)
- Fatal-startup catch no longer hides a crashed boot: it now records an
app.startup_failed incident (sync) and NULLs appRunJournal before app.quit(),
so will-quit's markCleanShutdown can't write the clean-shutdown marker and
misclassify the next launch as clean. (Integration HIGH.)
Major / Medium:
- mcp.host_start_failed is no longer dead code: BuiltInMcpHttpHost gets a
setJournal() called BEFORE start() (setDependencies, which carries the journal,
only runs after start()). (Integration MED.)
- Renderer-report IPC is rate-limited main-side (token bucket 10/s, burst 20,
excess summarized) — main no longer trusts the renderer's limiter, and the
React error boundary path (which bypassed it) is covered. (Disk/Adversarial MED.)
- process.on('warning') is now coalesced+capped like unhandledRejection, so a
warning storm can't drive unbounded synchronous disk writes. (Engine MAJOR.)
- JSON.stringify can no longer throw out of the journal: safeStringify() guards
flush/flushSync/recordIncident, so circular data can't leak the lock at quit.
(Engine MAJOR.)
- session.input_write_failed only fires for an id main NEVER owned (tracked via a
bounded everKnownSessionIds set), not the benign exit-race. (Adversarial MED-3.)
- heartbeat.json is now written atomically (temp+rename) with a single-write-in-
flight guard, so the crash-moment sample can't be torn. (Adversarial MED-5.)
Low / nits:
- Per-run byte cap counts UTF-8 bytes (Buffer.byteLength), not UTF-16 code units;
projects total before admitting (no cap+one-batch overshoot); un-reserves bytes
on write-failure re-queue so the counter tracks real on-disk size.
- previousRunClassifier reads only the trailing 256 KiB of incidents.jsonl (was a
whole-file sync read on the startup path).
- render_process_gone skips clean-exit entirely and treats 'killed' as non-fatal
(quit-teardown noise). orchestration request_timeout extracts the 30s constant.
- renderer dedup evicts expired entries instead of clearing the whole window.
Phase 5 process-death instrumentation (session.exit/spawn_failed) remains
deferred — see the PR description, relabeled honestly per the adversarial review.
tsc --noEmit: clean for all changed files (node + web).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 1, 2026
Merged
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
1 participant
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.
What this is
A complete, always-on, main-owned incident + performance journal for Agent Code (plan:
docs/plans_and_ideas/2026-06-25-performance-incident-journal-plan.md).AGENT_CODE_PERFtracing can't help with the failures that matter most — crashes, freezes, near-OOM, unclean shutdowns, prompt-delivery hangs — because those happen with the flag off or the renderer already gone. This is the always-on spine that captures them, carries the verdict into the next launch, and correlates every artifact under one run id.11 commits. Verified working at runtime (see below) and reviewed by a 4-lens agent pass whose findings are resolved.
Commits / phases
INCIDENT_RUNS_DIR· retention bucket (50 protected) · hard 50 MiB/run capincidents.jsonl· uncaught/rejection/warning (all coalesced) · render/child-process-gone · unresponsive · preload-error · did-fail-load · native Crashpadapp.prior_unclean_shutdownappRunIdshared with PerformanceServiceheap.pressure+ snapshot stamped with the shared idsession.input_write_failed(fires only for an id main never owned)prompt_delivery_failed·request_timeout·mcp.host_start_failedRuntime verification (live, perf-enabled run)
Backbone files written; heartbeat live;
crashReporterarmed; a realrenderer.unhandledrejectioncaptured end-to-end; perfrunId== journalappRunId(unification confirmed); manifest has no lock token. Incident tree was 16 KB vs the historical 848 MB feed-debug hog.Honest scope (relabeled per the adversarial review)
session.input_write_failedonly. Process-death instrumentation (session.exiton unexpected exit,session.spawn_failed) is deferred — the plan's "unexpected provider exit" acceptance criterion is not yet met.mcp.request.received, per-tool start/complete, routine request queued/dispatched) are deferred pending a sampling policy.Disk safety (the 500 GB question)
Structurally prevented: heartbeat overwrite-only; events/incidents hard-capped at 50 MiB/run (enforced at all three append sites, UTF-8 bytes); retention bucket + 50-protected. Provable ceiling ≈ 2.5 GiB for the incidents tree; normal case ~KB/run.
Remaining follow-ups (not in this PR)
🤖 Generated with Claude Code