From f140ec52ec1eae8cfd5a71386ba35b256c1611fa Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 01:57:25 +0200 Subject: [PATCH 1/6] =?UTF-8?q?feat(cef):=20crash-reporting=20smoke=20test?= =?UTF-8?q?=20=E2=80=94=20real=20Crashpad=20path,=20verified=20against=20C?= =?UTF-8?q?EF=20source?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wave 2 competency-gate item (CEF-RUST-COMPETENCY-MATRIX.md): "at least one crash-reporting/symbolization path proven". Unlike the earlier accessibility attempt (PR #391), every API and mechanism here was verified against the pinned CEF branch's actual source (chromiumembedded/cef branch 7922) before writing any code, not assumed from generic/older CEF knowledge: - crash_reporter.cfg next to the executable (include/cef_crash_util.h), copied via a new CMakeLists.txt configure_file step. - CefCrashReportingEnabled() logged in main.cpp after CefInitialize. - WorldScriptHandler now also implements CefRequestHandler:: OnRenderProcessTerminated (real method — confirmed present in include/cef_client.h, unlike GetAccessibilityHandler) to observe a deliberately induced renderer crash (chrome://crash, the same URL CEF's own cefclient sample uses for this). - The harness's new runCrashReportingProofCycle launches with chrome://crash, overrides BREAKPAD_DUMP_LOCATION (verified in libcef/common/crash_reporter_client.cc — still the real override var name even though this CEF version uses Crashpad on Linux, not Breakpad as CEF's own docs page currently claims) to a fresh temp dir, and asserts a dump file actually appears there after the crash. Full symbolization (dump_syms/minidump_stackwalk) requires building from a complete Chromium source checkout — out of reach of this project's minimal-CEF-SDK CI setup, documented as a known limit rather than attempted and faked. Co-Authored-By: Claude Sonnet 5 --- apps/desktop-cef/CMakeLists.txt | 7 + apps/desktop-cef/resources/crash_reporter.cfg | 15 ++ apps/desktop-cef/src/main.cpp | 6 + apps/desktop-cef/src/worldscript_handler.cpp | 26 ++++ apps/desktop-cef/src/worldscript_handler.h | 12 +- scripts/cef/run-launch-cycle-proof.mjs | 142 +++++++++++++++++- 6 files changed, 198 insertions(+), 10 deletions(-) create mode 100644 apps/desktop-cef/resources/crash_reporter.cfg diff --git a/apps/desktop-cef/CMakeLists.txt b/apps/desktop-cef/CMakeLists.txt index 9e601d706..5f70e5799 100644 --- a/apps/desktop-cef/CMakeLists.txt +++ b/apps/desktop-cef/CMakeLists.txt @@ -54,6 +54,13 @@ SET_EXECUTABLE_TARGET_PROPERTIES(worldscript_host) COPY_FILES("worldscript_host" "${CEF_BINARY_FILES}" "${CEF_BINARY_DIR}" "${CMAKE_CURRENT_BINARY_DIR}") COPY_FILES("worldscript_host" "${CEF_RESOURCE_FILES}" "${CEF_RESOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}") +# QNBS-v3: our own file, not a CEF SDK one, so COPY_FILES (which only knows CEF_BINARY_DIR/CEF_RESOURCE_DIR) doesn't apply — CEF requires crash_reporter.cfg next to the executable on Linux (include/cef_crash_util.h). +configure_file( + "${CMAKE_CURRENT_SOURCE_DIR}/resources/crash_reporter.cfg" + "${CMAKE_CURRENT_BINARY_DIR}/crash_reporter.cfg" + COPYONLY +) + if(OS_LINUX) FIND_LINUX_LIBRARIES("x11") endif() diff --git a/apps/desktop-cef/resources/crash_reporter.cfg b/apps/desktop-cef/resources/crash_reporter.cfg new file mode 100644 index 000000000..18772b987 --- /dev/null +++ b/apps/desktop-cef/resources/crash_reporter.cfg @@ -0,0 +1,15 @@ +# CEF crash-reporting config (docs/cef/knowledge/cef-architecture-primer.md, +# roadmap Appendix A.1 "Accessibility/crash smoke" items). Format and every +# key here come from include/cef_crash_util.h in the pinned CEF branch +# (https://github.com/chromiumembedded/cef/blob/7922/include/cef_crash_util.h) +# — verified against source, not assumed. Copied next to worldscript_host by +# CMakeLists.txt; CEF requires this exact filename and location on Linux. +# +# No ServerURL is set: reports stay local-only, nothing is ever uploaded — +# deliberate for a CI/dev proof, safe by default for anyone building this host. +# RateLimitEnabled / MaxUploadsPerDay / MaxDatabaseSizeInMb / MaxDatabaseAgeInDays +# are intentionally omitted — the same doc states they are not supported on Linux. + +[Config] +ProductName=WorldScriptStudioDesktopHost +ProductVersion=0.0.0-cef-wave2-smoke diff --git a/apps/desktop-cef/src/main.cpp b/apps/desktop-cef/src/main.cpp index 2ff6123bf..d5040619a 100644 --- a/apps/desktop-cef/src/main.cpp +++ b/apps/desktop-cef/src/main.cpp @@ -2,6 +2,7 @@ #include #include "include/cef_app.h" +#include "include/cef_crash_util.h" #include "shutdown_signal.h" #include "worldscript_app.h" @@ -45,6 +46,11 @@ int main(int argc, char* argv[]) { return 1; } + // QNBS-v3: CefCrashReportingEnabled() reflects whether crash_reporter.cfg (copied next to this binary by CMakeLists.txt) was found and parsed — the CI harness asserts on this line, not just on the .cfg file existing on disk. + printf("[worldscript_host] crash_reporting_enabled = %s\n", + CefCrashReportingEnabled() ? "true" : "false"); + fflush(stdout); + CefRunMessageLoop(); CefShutdown(); diff --git a/apps/desktop-cef/src/worldscript_handler.cpp b/apps/desktop-cef/src/worldscript_handler.cpp index aedebffb2..b3a06cc7c 100644 --- a/apps/desktop-cef/src/worldscript_handler.cpp +++ b/apps/desktop-cef/src/worldscript_handler.cpp @@ -1,6 +1,7 @@ #include "worldscript_handler.h" #include +#include #include "include/cef_app.h" #include "include/cef_task.h" @@ -15,6 +16,20 @@ namespace { constexpr int kShutdownPollIntervalMs = 100; +// QNBS-v3: lookup table (repo convention) over an if/else chain — six real, verified enum values from include/internal/cef_types.h in the pinned CEF branch. +const char* TerminationStatusToString(cef_termination_status_t status) { + static const std::unordered_map kNames = { + {TS_ABNORMAL_TERMINATION, "TS_ABNORMAL_TERMINATION"}, + {TS_PROCESS_WAS_KILLED, "TS_PROCESS_WAS_KILLED"}, + {TS_PROCESS_CRASHED, "TS_PROCESS_CRASHED"}, + {TS_PROCESS_OOM, "TS_PROCESS_OOM"}, + {TS_LAUNCH_FAILED, "TS_LAUNCH_FAILED"}, + {TS_INTEGRITY_FAILURE, "TS_INTEGRITY_FAILURE"}, + }; + const auto it = kNames.find(status); + return it != kNames.end() ? it->second : "TS_UNKNOWN"; +} + // QNBS-v3: plain CefTask subclass instead of base::BindOnce — CEF's own ref-counting scheme hit real base::Bind template/header issues (caught by CI, not locally); this is simpler and avoids that machinery entirely. class PollShutdownTask : public CefTask { public: @@ -63,6 +78,17 @@ void WorldScriptHandler::PollShutdownFlag() { } } +void WorldScriptHandler::OnRenderProcessTerminated(CefRefPtr browser, + TerminationStatus status, + int error_code, + const CefString& error_string) { + CEF_REQUIRE_UI_THREAD(); + // QNBS-v3: proof line for the CI harness's crash cycle — the browser process reaching this line at all is itself the "renderer termination observed and handled" evidence (CEF-RUST-COMPETENCY-MATRIX.md), since only the renderer subprocess died. + printf("[worldscript_host] renderer_terminated status=%s error_code=%d\n", + TerminationStatusToString(status), error_code); + fflush(stdout); +} + bool WorldScriptHandler::DoClose(CefRefPtr browser) { CEF_REQUIRE_UI_THREAD(); // QNBS-v3: no save-coordinator/state to flush yet (Wave 5+ scope, docs/cef/knowledge/subprocess-and-shutdown.md) — allow the close to proceed. diff --git a/apps/desktop-cef/src/worldscript_handler.h b/apps/desktop-cef/src/worldscript_handler.h index d6213fa9b..794deaf9d 100644 --- a/apps/desktop-cef/src/worldscript_handler.h +++ b/apps/desktop-cef/src/worldscript_handler.h @@ -5,15 +5,17 @@ #include "include/cef_client.h" -// QNBS-v3: browser-process lifecycle/display callbacks only (ADR-0020 scorecard) — renderer-process-specific handlers (CefRenderProcessHandler) are explicitly out of scope for this proof. +// QNBS-v3: browser-process lifecycle/display/request callbacks only (ADR-0020 scorecard) — renderer-process-specific handlers (CefRenderProcessHandler) are explicitly out of scope for this proof. class WorldScriptHandler : public CefClient, public CefLifeSpanHandler, - public CefDisplayHandler { + public CefDisplayHandler, + public CefRequestHandler { public: WorldScriptHandler(); CefRefPtr GetLifeSpanHandler() override { return this; } CefRefPtr GetDisplayHandler() override { return this; } + CefRefPtr GetRequestHandler() override { return this; } void OnTitleChange(CefRefPtr browser, const CefString& title) override; @@ -21,6 +23,12 @@ class WorldScriptHandler : public CefClient, bool DoClose(CefRefPtr browser) override; void OnBeforeClose(CefRefPtr browser) override; + // QNBS-v3: real, verified CefRequestHandler method (unlike the reverted GetAccessibilityHandler attempt) — fires in the browser process when a renderer subprocess dies; the browser process itself and CefRunMessageLoop() keep running. + void OnRenderProcessTerminated(CefRefPtr browser, + TerminationStatus status, + int error_code, + const CefString& error_string) override; + // QNBS-v3: public (not private) — called from PollShutdownTask::Execute(), an unrelated class in worldscript_handler.cpp's anonymous namespace; polls g_worldscript_shutdown_requested from the UI thread and requests a graceful close via TryCloseBrowser when set. void PollShutdownFlag(); diff --git a/scripts/cef/run-launch-cycle-proof.mjs b/scripts/cef/run-launch-cycle-proof.mjs index f29a8822d..494b7350f 100644 --- a/scripts/cef/run-launch-cycle-proof.mjs +++ b/scripts/cef/run-launch-cycle-proof.mjs @@ -25,9 +25,20 @@ * callback-based signal this harness would have checked for does not exist; see * docs/cef/knowledge/cef-architecture-primer.md for the real, honestly-documented blocker. * + * After the repeated cycles, one additional crash-reporting proof runs + * (runCrashReportingProofCycle): launches with chrome://crash to deliberately crash + * the renderer subprocess, verifies CefCrashReportingEnabled() was true, verifies the + * browser process survived (CefRequestHandler::OnRenderProcessTerminated fired instead + * of the whole process dying), and verifies a real dump file was written to a + * BREAKPAD_DUMP_LOCATION-overridden directory. Full symbolization (dump_syms / + * minidump_stackwalk) needs a complete Chromium source checkout and is genuinely out of + * reach of this project's minimal-CEF-SDK-only CI setup — see the primer doc. + * * Run: node scripts/cef/run-launch-cycle-proof.mjs [--cycles N] */ import { execFileSync, spawn } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; import path from 'node:path'; const [binaryPath, url] = process.argv.slice(2); @@ -45,6 +56,11 @@ const ORPHAN_CHECK_GRACE_MS = 3000; const FFI_PROOF_LINE = 'rust_core ping = 424242'; const EXPECTED_TITLE_LINE = 'title = WorldScript Studio'; +// QNBS-v3: crash-reporting/symbolization competency-gate proof (CEF-RUST-COMPETENCY-MATRIX.md) — mechanism verified against real CEF 151 source (libcef/common/crash_reporting.cc, crash_reporter_client.cc), not assumed; see docs/cef/knowledge/cef-architecture-primer.md. +const CRASH_REPORTING_ENABLED_LINE = 'crash_reporting_enabled = true'; +const RENDERER_TERMINATED_PROOF_PREFIX = 'renderer_terminated status='; +const CRASH_URL = 'chrome://crash'; + if (!binaryPath || !url) { console.error( '[launch-cycle-proof] Usage: node scripts/cef/run-launch-cycle-proof.mjs [--cycles N]', @@ -81,8 +97,22 @@ function processTreeAlive() { } } -function logStderr(index, stderr) { - if (stderr) console.error(`[launch-cycle-proof] Cycle ${index + 1} stderr:\n${stderr}`); +function logStderr(label, stderr) { + if (stderr) console.error(`[launch-cycle-proof] ${label} stderr:\n${stderr}`); +} + +// QNBS-v3: recursive, not a flat readdir — Crashpad's on-disk database nests reports under subdirectories (e.g. completed/, pending/, attachments/) whose exact layout isn't asserted on here; presence of any file is the proof. +function findFilesRecursive(dir) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return []; + } + return entries.flatMap((entry) => { + const full = path.join(dir, entry.name); + return entry.isDirectory() ? findFilesRecursive(full) : [full]; + }); } async function runCycle(index) { @@ -110,7 +140,7 @@ async function runCycle(index) { // QNBS-v3: races against the startup grace period so an immediate crash is caught here, distinct from a deliberate SIGTERM-driven exit later — Qodo review finding on PR #388 ("crashed cycles count clean"). const earlyExit = await Promise.race([exited, sleep(STARTUP_GRACE_MS).then(() => null)]); if (earlyExit) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error( `Cycle ${index + 1}: exited during startup (code=${earlyExit.code}, signal=${earlyExit.signal}) instead of staying up — likely a crash, not a deliberate shutdown.`, ); @@ -119,14 +149,14 @@ async function runCycle(index) { child.kill('SIGTERM'); const shutdownResult = await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS).then(() => null)]); if (!shutdownResult) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error(`Cycle ${index + 1}: process did not exit within the shutdown grace period.`); } // QNBS-v3: accepts either "died from the SIGTERM we sent" or "exited 0 on its own" as clean — anything else (e.g. SIGSEGV) is a real crash during shutdown, not evidence this proof should accept. const cleanShutdown = shutdownResult.signal === 'SIGTERM' || shutdownResult.code === 0; if (!cleanShutdown) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error( `Cycle ${index + 1}: abnormal exit during shutdown (code=${shutdownResult.code}, signal=${shutdownResult.signal}).`, ); @@ -134,17 +164,17 @@ async function runCycle(index) { await sleep(ORPHAN_CHECK_GRACE_MS); if (processTreeAlive()) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error(`Cycle ${index + 1}: orphaned worldscript_host process(es) still running.`); } // QNBS-v3: required per cycle, not aggregated across the whole run — Qodo review finding on PR #388 ("FFI proof is not repeated"); one cycle's success must never mask another cycle's failure. if (!stdout.includes(FFI_PROOF_LINE)) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error(`Cycle ${index + 1}: no FFI boundary proof ("${FFI_PROOF_LINE}") observed.`); } if (!stdout.includes(EXPECTED_TITLE_LINE)) { - logStderr(index, stderr); + logStderr(`Cycle ${index + 1}`, stderr); throw new Error( `Cycle ${index + 1}: expected "${EXPECTED_TITLE_LINE}" not observed — the production bundle may not have rendered (a CEF error page would not produce this specific title).`, ); @@ -154,6 +184,100 @@ async function runCycle(index) { ); } +// QNBS-v3: separate function, not a mode flag on runCycle — keeps the already-proven repeated-cycle proof completely untouched (the accessibility-attempt regression on PR #391 was caused by exactly this kind of shared-code coupling). +async function runCrashReportingProofCycle() { + console.log( + `[launch-cycle-proof] Crash-reporting proof: launching with ${CRASH_URL} to deliberately crash the renderer…`, + ); + // QNBS-v3: fresh, empty-at-start temp dir — BREAKPAD_DUMP_LOCATION (verified in libcef/common/crash_reporter_client.cc) overrides where CEF/Crashpad writes dumps on Linux/POSIX, so "any file appears here" is unambiguous evidence, no need to guess CEF's default directory layout. + const dumpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'worldscript-crash-dumps-')); + + const child = spawn(binaryPath, [`--url=${CRASH_URL}`, '--enable-logging=stderr', '--v=1'], { + cwd: path.dirname(binaryPath), + stdio: ['ignore', 'pipe', 'pipe'], + env: { ...process.env, BREAKPAD_DUMP_LOCATION: dumpDir }, + }); + + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { + stdout += chunk.toString(); + }); + child.stderr.on('data', (chunk) => { + stderr += chunk.toString(); + }); + + const exited = new Promise((resolve) => + child.once('exit', (code, signal) => resolve({ code, signal })), + ); + + const rendererTerminated = await Promise.race([ + (async () => { + while (!stdout.includes(RENDERER_TERMINATED_PROOF_PREFIX)) { + await sleep(200); + } + return true; + })(), + exited.then(() => false), + sleep(STARTUP_GRACE_MS).then(() => false), + ]); + + if (!rendererTerminated) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + `Crash-reporting proof: "${RENDERER_TERMINATED_PROOF_PREFIX}" not observed within ${STARTUP_GRACE_MS}ms (or the process exited early) — stdout so far:\n${stdout}`, + ); + } + + // QNBS-v3: the actual "renderer termination observed and handled" evidence (CEF-RUST-COMPETENCY-MATRIX.md) — only the renderer subprocess should have died; the browser process and its message loop must still be running. + if (!processTreeAlive()) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + 'Crash-reporting proof: browser process is not alive after the renderer crash — process isolation did not hold.', + ); + } + + if (!stdout.includes(CRASH_REPORTING_ENABLED_LINE)) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + `Crash-reporting proof: "${CRASH_REPORTING_ENABLED_LINE}" not observed — crash_reporter.cfg (apps/desktop-cef/resources/crash_reporter.cfg) was not found/parsed next to the binary.`, + ); + } + + child.kill('SIGTERM'); + const shutdownResult = await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS).then(() => null)]); + if (!shutdownResult) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + 'Crash-reporting proof: process did not exit within the shutdown grace period after the renderer crash.', + ); + } + + await sleep(ORPHAN_CHECK_GRACE_MS); + if (processTreeAlive()) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + 'Crash-reporting proof: orphaned worldscript_host process(es) still running after shutdown.', + ); + } + + const dumpFiles = findFilesRecursive(dumpDir); + console.log( + `[launch-cycle-proof] Crash-reporting proof: dump location (${dumpDir}) after the crash: ${dumpFiles.length > 0 ? dumpFiles.join(', ') : '(empty)'}`, + ); + if (dumpFiles.length === 0) { + logStderr('Crash-reporting proof', stderr); + throw new Error( + `Crash-reporting proof: no file was written under BREAKPAD_DUMP_LOCATION (${dumpDir}) despite crash_reporting_enabled=true and an observed renderer crash.`, + ); + } + + console.log( + '[launch-cycle-proof] Crash-reporting proof: OK — crash reporting enabled, renderer crash observed and handled (browser process survived), ' + + `${dumpFiles.length} file(s) written to the crash dump location. Full symbolization (dump_syms/minidump_stackwalk) requires a complete Chromium source checkout and is out of scope — see docs/cef/knowledge/cef-architecture-primer.md.`, + ); +} + async function main() { for (let i = 0; i < cycles; i++) { await runCycle(i); @@ -162,6 +286,8 @@ async function main() { console.log( `[launch-cycle-proof] OK — ${cycles}/${cycles} repeated start/close cycles clean, FFI boundary and real rendering both proven in every cycle.`, ); + + await runCrashReportingProofCycle(); } main().catch((err) => { From 4afff0d5ffa5c635a5bd0972b662c157c64cc968 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:06:15 +0200 Subject: [PATCH 2/6] docs(cef): record crash-reporting proof evidence in competency matrix + primer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flips renderer_crash_ci to true (chrome://crash + OnRenderProcessTerminated + browser-process survival, CI-proven) and checks the two "renderer termination"/"crash-reporting path" checklist items with linked evidence (PR #392). crash_symbolization_smoke stays false — only the crash-reporting half was proven; decoding a dump needs dump_syms/minidump_stackwalk built from a full Chromium source checkout, out of reach of this project's minimal-CEF-SDK CI setup. Adds a "Crash reporting" section to cef-architecture-primer.md, including a correction to CEF's own docs/crash_reporting.md (claims Breakpad on Linux; this CEF version's actual source uses Crashpad everywhere on POSIX except macOS-specific branches) — confirmed by reading libcef/common/crash_reporting.cc directly rather than trusting the doc page. Adds a matching PASS row (reporting half only) to native-readiness.md and refreshes the two OWNERSHIP.yaml notes. Co-Authored-By: Claude Sonnet 5 --- docs/architecture/native-readiness.md | 3 ++- docs/cef/CEF-RUST-COMPETENCY-MATRIX.md | 18 +++++++++--------- docs/cef/OWNERSHIP.yaml | 4 ++-- docs/cef/knowledge/cef-architecture-primer.md | 14 +++++++++++++- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/docs/architecture/native-readiness.md b/docs/architecture/native-readiness.md index 9f80050e9..666d76d8d 100644 --- a/docs/architecture/native-readiness.md +++ b/docs/architecture/native-readiness.md @@ -61,9 +61,10 @@ Wave 2's first deliverable — the CEF binding/C++ decision — is now backed by | CEF lifecycle assumptions documented | **PASS** | cef-runtime | `docs/cef/knowledge/subprocess-and-shutdown.md`'s core Wave 2 claim (SIGTERM → graceful `TryCloseBrowser`/`OnBeforeClose`/`CefQuitMessageLoop`/`CefShutdown`, repeated clean start/close cycles) now has a real linked chain: test (`scripts/cef/run-launch-cycle-proof.mjs`) → CI job (`🧪 CEF Learning Harness`) → doc, exactly what §61.1.4 requires. Save-coordinator/window-state persistence remain explicitly Wave 5+ scope (not a Wave 2 gap); Windows/macOS and a real packaged layout remain open, tracked in the doc's own "Outline" section. | | Early Accessibility Gate | Not yet attempted (real blocker found) | cef-runtime, Wave 2 | PR #391 attempted `CefAccessibilityHandler` — does not compile against CEF 151.3.18 (`CefClient::GetAccessibilityHandler()` doesn't exist in this version). Reverted rather than left half-working, after a fallback attempt (enable-only, no observability) regressed the previously-reliable FFI/rendering proofs. Real CEF-151 API research needed before the next attempt — see `docs/cef/knowledge/cef-architecture-primer.md`. | | Sandbox posture | Not yet attempted | desktop-security, Wave 2/3 (roadmap §12) | Every run so far used `no_sandbox=true`; zero evidence either way on this row. | +| Crash reporting / renderer-crash resilience | **PASS** — crash-reporting half only | cef-runtime | PR #392: `crash_reporter.cfg` + `CefCrashReportingEnabled()` verified true, `chrome://crash` deliberately crashes the renderer, `CefRequestHandler::OnRenderProcessTerminated` fires (`TS_PROCESS_CRASHED`), the browser process/message loop survive, and a real Crashpad dump (`.dmp`/`.meta`/`settings.dat`) is produced under an overridden `BREAKPAD_DUMP_LOCATION` — all CI-run, not a doc claim. Symbolization (decoding the dump into a stack trace via `dump_syms`/`minidump_stackwalk`) needs a full Chromium source checkout and was not attempted — see `docs/cef/knowledge/cef-architecture-primer.md`. | | CEF SDK fetch/verify + version diagnostics automated | **PASS** | cef-runtime | `🧪 CEF Learning Harness` CI job (`.github/workflows/cef-learning-harness.yml`) fetches the pinned CEF SDK, verifies its checksum, and parses real version macros out of the extracted `include/cef_version.h` — a genuine CI-run check, not a doc claim. | | Linux dependency inventory — clean-machine data point | DEBT — partial | cef-runtime | Same CI job runs the package-presence check against a stock `ubuntu-latest` runner before any `apt-get`, adding a real second data point beyond the spike's one already-configured dev machine. Still narrow: `dpkg` package-presence only (not `ldd` against the actual shipped `.so` files), one distro/runner image. | | CEF host build + repeated launch/close cycle proof, in CI | **PASS** | cef-runtime | PR #388: `apps/desktop-cef/`'s `worldscript_host` (real, repo-committed C++/Rust source, not spike code) builds against the fetched CEF SDK and runs 3 independently-verified clean start/close cycles under Xvfb in CI — the roadmap's literal "isolated learning harness" / "safe repeated startup/shutdown" deliverables (§3142), not just the fetch/diagnostics increment. | | Rust FFI boundary proven inside the real host | **PASS** | cef-runtime, rust-core | `worldscript_rust_ping()` (rust-core, linked via Corrosion) is called from `OnAfterCreated` on every cycle and its exact sentinel value observed in CI output — stronger than the ADR-0020 spike's decoupled isolation test, since this proves the boundary works inside the actual multi-process CEF host, not a standalone C++ program. | -**Overall for this snapshot**: 5 PASS, 2 explicit DEBT-in-progress rows (each with a concrete exit condition, not open-ended), 2 not-yet-attempted rows correctly left blank rather than assumed. No row is marked PASS without the evidence cited above. +**Overall for this snapshot**: 6 PASS (one — crash reporting — explicitly PASS for its reporting half only, not symbolization), 2 explicit DEBT-in-progress rows (each with a concrete exit condition, not open-ended), 2 not-yet-attempted rows correctly left blank rather than assumed. No row is marked PASS without the evidence cited above. diff --git a/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md b/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md index 23db6ce15..160cb1383 100644 --- a/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md +++ b/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md @@ -1,7 +1,7 @@ # CEF/Rust Competency Matrix **Companion to:** [`ROADMAP-CEF-DESKTOP-MIGRATION.md`](ROADMAP-CEF-DESKTOP-MIGRATION.md) §4.11, §61.1, Appendix A.1 · [ADR-0019](../adr/0019-cef-desktop-runtime-strategy.md) -**Established:** Wave 0, 2026-08-18. **Baseline was: nothing done yet.** Updated in place, 2026-08-18/19 (Wave 2, ADR-0020 spike + PR #386/#387/#388), per this doc's own "Update discipline" below — items flip to `true` only with a linked evidence commit, in the same commit as the flip. This file exists so future waves have a live, gradeable target instead of re-deriving the checklist from the roadmap prose each time. +**Established:** Wave 0, 2026-08-18. **Baseline was: nothing done yet.** Updated in place, 2026-08-18/19 (Wave 2, ADR-0020 spike + PR #386/#387/#388/#391/#392), per this doc's own "Update discipline" below — items flip to `true` only with a linked evidence commit, in the same commit as the flip. This file exists so future waves have a live, gradeable target instead of re-deriving the checklist from the roadmap prose each time. This is an engineering gate (roadmap §4.11.6), not a training checklist. `WS-CEF-IPC` (Wave 4) and any production storage capability exposing privileged native operations may not proceed until the relevant items below are `true` with linked evidence. @@ -12,10 +12,10 @@ cef_competency: binding_model_documented: true # docs/adr/0020-cef-binding-choice-thin-cpp-host.md lifetime_model_reviewed: false repeated_shutdown_ci: true # scripts/cef/run-launch-cycle-proof.mjs, cef-learning-harness CI job (PR #388) - renderer_crash_ci: false + renderer_crash_ci: true # chrome://crash + OnRenderProcessTerminated + browser-process survival, cef-learning-harness CI job (PR #392) sandbox_smoke: false accessibility_smoke: false - crash_symbolization_smoke: false + crash_symbolization_smoke: false # crash REPORTING is proven (PR #392) — this field is specifically about decoding a dump (dump_syms/minidump_stackwalk), which needs a full Chromium source checkout and was not attempted ``` CI validation of this block ("fail CI when a required item for the active program phase is absent or false") is not yet implemented — this manifest is hand-maintained for now, matching every `driftCheckTool: "planned — not implemented"` entry in `OWNERSHIP.yaml`. @@ -28,7 +28,7 @@ CI validation of this block ("fail CI when a required item for the active progra | CEF threading & lifetime rules (UI-thread callbacks, IO thread, ref-counted objects, callback lifetime, async cancellation, shutdown races) | Partial | `CEF_REQUIRE_UI_THREAD()` used throughout; `IMPLEMENT_REFCOUNTING`/`CefRefPtr` applied correctly; a real callback-lifetime lesson learned and fixed (`base::Unretained` vs. a plain `CefTask` — see `apps/desktop-cef/src/worldscript_handler.cpp`). No dedicated review doc yet (`docs/cef/knowledge/threading-and-lifetimes.md` still skeleton); IO thread and async-cancellation patterns untouched. | | Rust binding layer (crate/version, unsafe/FFI boundary, wrapper ownership, API coverage gaps, upgrade procedure) | Partial | `apps/desktop-cef/rust-core/` (`worldscript_rust_core`, Corrosion-linked) — FFI boundary proven inside the real CEF host in CI (PR #388), not just an isolated test. No upgrade procedure written yet (`docs/cef/knowledge/binding-upgrade-playbook.md` still skeleton); API coverage is currently one trivial function, not representative of real surface area. | | Cross-platform native host (Linux loader/resource layout, Windows process/installer/sandbox, macOS bundle/signing, window lifecycle, high-DPI, IME/a11y) | Partial (Linux only) | Linux loader/resource layout confirmed via a real filesystem listing in CI (`docs/cef/knowledge/linux-runtime-notes.md`); a real cwd-relative-path startup bug found and fixed. Zero Windows/macOS evidence. Window lifecycle proven for open/close only — high-DPI and IME/a11y untouched. | -| Operational CEF (crash reporting, symbol handling, version-update automation, sandbox verification, packaging deps, runtime diagnostics) | Partial | Packaging deps: `scripts/cef/check-linux-runtime-deps.mjs` (CI-run). Runtime diagnostics: `scripts/cef/print-cef-version-diagnostics.mjs` + verbose CEF logging (`--enable-logging=stderr --v=1`) added mid-debugging this wave. Crash reporting, symbol handling, version-update automation, and sandbox verification all remain not started. | +| Operational CEF (crash reporting, symbol handling, version-update automation, sandbox verification, packaging deps, runtime diagnostics) | Partial | Packaging deps: `scripts/cef/check-linux-runtime-deps.mjs` (CI-run). Runtime diagnostics: `scripts/cef/print-cef-version-diagnostics.mjs` + verbose CEF logging (`--enable-logging=stderr --v=1`) added mid-debugging this wave. Crash reporting: proven in CI (PR #392) — `crash_reporter.cfg` + `CefCrashReportingEnabled()` + a deliberately induced renderer crash (`chrome://crash`) produced a real Crashpad dump (`.dmp`/`.meta`/`settings.dat`) under an overridden `BREAKPAD_DUMP_LOCATION`; the browser process survived. Symbol handling (decoding a dump into a stack trace) needs `dump_syms`/`minidump_stackwalk` built from a full Chromium source checkout — out of reach of this project's minimal-CEF-SDK CI setup, not attempted. Version-update automation and sandbox verification remain not started. | ## Appendix A.1 checklist (live) @@ -41,9 +41,9 @@ CI validation of this block ("fail CI when a required item for the active progra [ ] Binding API gaps catalogued [x] Unpackaged CEF resource layout proven — PR #388, real filesystem listing in CI (real shipped/installer packaging remains separate, unproven, later scope) [x] Repeated startup/shutdown harness green — PR #388, 3/3 cycles, cef-learning-harness CI job -[ ] Renderer crash observation green +[x] Renderer crash observation green — PR #392, chrome://crash + OnRenderProcessTerminated (TS_PROCESS_CRASHED), browser process survived, cef-learning-harness CI job [ ] Accessibility smoke green (attempted, real blocker — see cef-architecture-primer.md's "Accessibility API" section) -[ ] Crash-reporting/symbolization smoke green +[ ] Crash-reporting/symbolization smoke green (crash-reporting half proven — PR #392, real Crashpad dump produced in CI; symbolization/decoding the dump not attempted, needs a full Chromium source checkout — see cef-architecture-primer.md) [ ] Linux dependency inventory complete (inventoried, not yet proven sufficient — see native-readiness.md) [ ] X11/Wayland initial smoke complete (X11 only; Wayland zero evidence) [ ] Upgrade playbook written @@ -59,15 +59,15 @@ CI validation of this block ("fail CI when a required item for the active progra [x] unsafe/FFI boundary identified — apps/desktop-cef/rust-core/, proven in CI (PR #388) [ ] threading/lifetime map reviewed (no dedicated doc yet — see domains table) [x] clean repeated startup/shutdown proven — PR #388, 3/3 cycles, cef-learning-harness CI job -[ ] renderer termination observed and handled (not proven — PR #388 exercised normal shutdown only; no test deliberately terminates/crashes a renderer, per Qodo review finding on PR #389) +[x] renderer termination observed and handled — PR #392, chrome://crash deliberately crashes the renderer, OnRenderProcessTerminated fires, browser process/message loop survive, cef-learning-harness CI job [ ] sandbox development plan validated [ ] Linux runtime dependencies inventoried (inventoried but not yet proven sufficient — see native-readiness.md) [ ] at least one accessibility smoke test performed (attempted, real blocker — see cef-architecture-primer.md's "Accessibility API" section) -[ ] at least one crash-reporting/symbolization path proven +[x] at least one crash-reporting/symbolization path proven — PR #392: crash-reporting path proven end-to-end (real Crashpad dump produced in CI); full symbolization (decoding the dump) is a separate, unattempted step needing a full Chromium source checkout [ ] upgrade playbook exists ``` -This gate is **not** satisfied yet — 4 of 12 items checked, several with explicit caveats above. `WS-CEF-IPC` (Wave 4) remains blocked. +This gate is **not** satisfied yet — 6 of 12 items checked, several with explicit caveats above. `WS-CEF-IPC` (Wave 4) remains blocked. ## What this snapshot (Wave 2, 2026-08-18/19) does NOT claim diff --git a/docs/cef/OWNERSHIP.yaml b/docs/cef/OWNERSHIP.yaml index 22a5444d6..a922bdd90 100644 --- a/docs/cef/OWNERSHIP.yaml +++ b/docs/cef/OWNERSHIP.yaml @@ -56,7 +56,7 @@ documents: - cef-learning-harness # cef-competency-gate: no such CI workflow/job exists yet — planned, not implemented (CodeRabbit review finding on PR #389). Re-add once it's a real job. driftCheckTool: "planned — not implemented, see Wave 1" - note: "Updated in place for Wave 2 (PR #386/#387/#388) — 2 of 7 cef_competency items now true with linked evidence; competency gate still not satisfied (5/12)." + note: "Updated in place for Wave 2 (PR #386/#387/#388/#391/#392) — 3 of 7 cef_competency items now true with linked evidence (renderer_crash_ci added, PR #392); competency gate still not satisfied (6/12)." - path: docs/cef/TAURI-COUPLING-INVENTORY.md tier: B @@ -104,7 +104,7 @@ documents: related_ci: - cef-learning-harness driftCheckTool: "planned — not implemented, see Wave 1" - note: "Real evidence from PR #388 for process model, message loop, subprocess packaging. Sandbox config and a directly-observed process-tree snapshot remain open." + note: "Real evidence from PR #388 for process model, message loop, subprocess packaging; crash reporting proven in CI (PR #392). Sandbox config, symbolization, and a directly-observed process-tree snapshot remain open." - path: docs/cef/knowledge/cef-rust-binding-cookbook.md tier: A diff --git a/docs/cef/knowledge/cef-architecture-primer.md b/docs/cef/knowledge/cef-architecture-primer.md index d8bd66a8f..b321ff09b 100644 --- a/docs/cef/knowledge/cef-architecture-primer.md +++ b/docs/cef/knowledge/cef-architecture-primer.md @@ -1,6 +1,6 @@ # CEF Architecture Primer -**Status:** Real evidence from `apps/desktop-cef/` (PR #388) for process model, message loop, and subprocess packaging. Sandbox configuration and a directly-observed full process-tree snapshot remain open. +**Status:** Real evidence from `apps/desktop-cef/` (PR #388) for process model, message loop, and subprocess packaging; crash reporting and renderer-crash resilience proven in CI (PR #392). Sandbox configuration, dump symbolization, and a directly-observed full process-tree snapshot remain open. **Scope:** How CEF's multi-process architecture (browser process, renderer process, GPU/utility processes; browser/frame/client ownership; message-loop integration; subprocess launch and packaging; sandbox model) maps onto WorldScript Studio's specific host and build, written from our actual integration — not a generic CEF tutorial. **Tier:** A (release/security-critical) — see [`../OWNERSHIP.yaml`](../OWNERSHIP.yaml). **Roadmap context:** [`../ROADMAP-CEF-DESKTOP-MIGRATION.md`](../ROADMAP-CEF-DESKTOP-MIGRATION.md) §4.11.1 ("CEF architecture" domain), §4.11.2, Wave 2. @@ -37,6 +37,18 @@ The attempt was fully reverted rather than left half-working: `browser->GetHost( **What this means for the next attempt**: the correct modern CEF 151 mechanism for accessibility tree observation is genuinely unknown as of this doc's writing — it needs real API research (current CEF source/docs, not assumptions carried from older versions or generic Chromium knowledge) before writing any more code against it. `accessibility_smoke` stays `false` in the competency manifest; the Early Accessibility Gate remains unattempted-with-a-working-mechanism, not "attempted and passed." +## Crash reporting — a real, working proof (with an honest limit) + +Unlike the accessibility attempt above, every mechanism here was verified against the pinned CEF branch's actual source (`chromiumembedded/cef` branch `7922`, matching `151.0.7922.138`) before any code was written — the same discipline the "what this means for the next attempt" note above called for. + +**A real, and initially surprising, correction to CEF's own docs**: `docs/crash_reporting.md` in the CEF repo states crash reporting is "implemented using Crashpad on Windows and macOS, and Breakpad on Linux." That is stale relative to this exact branch's source. `libcef/common/crash_reporting.cc`'s `InitCrashReporter()` calls `crash_reporter::InitializeCrashpad(...)` unconditionally for every non-Mac POSIX process (Linux included) — Linux uses **Crashpad** too in this CEF version, not Breakpad. This was confirmed, not assumed, before relying on it: reading `libcef/common/crash_reporter_client.cc`'s `GetCrashDumpLocation()` showed the `BREAKPAD_DUMP_LOCATION` environment variable (a legacy name, kept for compatibility) still overrides the dump directory on POSIX, and CI evidence (below) confirmed a real Crashpad database layout (`pending/`, `.meta`, `settings.dat`), not a Breakpad one. + +**What's implemented** (`apps/desktop-cef/resources/crash_reporter.cfg`, `CMakeLists.txt`, `main.cpp`, `worldscript_handler.{h,cpp}`): `crash_reporter.cfg` (format from `include/cef_crash_util.h`) is copied next to the built executable via a `configure_file` step; `main.cpp` logs `CefCrashReportingEnabled()` after `CefInitialize`; `WorldScriptHandler` now also implements `CefRequestHandler` and overrides `OnRenderProcessTerminated` — a real method (confirmed present in `include/cef_client.h`'s `GetRequestHandler()`, unlike the accessibility handler) that fires in the browser process when a renderer subprocess dies, without the browser process itself going down. + +**Directly observed evidence, PR #392, `🧪 CEF Learning Harness` CI job**: the harness launches `worldscript_host --url=chrome://crash` (the same debug URL CEF's own `cefclient` reference app uses to test this exact path) with `BREAKPAD_DUMP_LOCATION` pointed at a fresh, empty temp directory. The CI log shows `crash_reporting_enabled = true`, then `renderer_terminated status=TS_PROCESS_CRASHED error_code=...`, then — after the harness's usual graceful-shutdown sequence — three real files in that directory: `pending/.dmp`, `pending/.meta`, and `settings.dat`. The browser process's own clean-shutdown proof (same mechanism as the repeated start/close cycles) passed too, confirming process isolation held: only the renderer subprocess died. + +**What this does NOT prove**: symbolization — decoding the `.dmp` file into a human-readable stack trace — needs `dump_syms` and `minidump_stackwalk`, which CEF's own docs say must be built from a *complete Chromium source checkout* (`gn`/`ninja`, hours of build time, tens of GB of disk). That is out of reach of this project's minimal-CEF-SDK-only CI setup (and of the local dev machine's own constrained RAM/disk, per this repo's own low-end-hardware guidance) and was not attempted. `crash_symbolization_smoke` in `docs/cef/CEF-RUST-COMPETENCY-MATRIX.md` stays `false` for that reason — the crash-*reporting* half is proven; symbolization is a separate, still-open item. + ## Sandbox configuration, as shipped `chrome-sandbox` is present in the output directory (copied automatically as part of `CEF_BINARY_FILES`) but is **not used** — `main.cpp` sets `CefSettings.no_sandbox = true` unconditionally. Zero evidence exists on real sandbox posture; this is explicitly tracked as "Not yet attempted" in `docs/architecture/native-readiness.md` and `false` in the competency manifest. From 0b3885dbd1d7945852c1acc84168d463cfef2bf8 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:32:02 +0200 Subject: [PATCH 3/6] =?UTF-8?q?fix(cef):=20harden=20crash-reporting=20proo?= =?UTF-8?q?f=20=E2=80=94=20CodeAnt/Qodo=20review=20findings=20on=20PR=20#3?= =?UTF-8?q?92?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5 real findings across CodeAnt and Qodo, all valid, all fixed: - Only TS_PROCESS_CRASHED now satisfies the proof, not any termination status (TS_LAUNCH_FAILED/TS_PROCESS_WAS_KILLED/TS_ABNORMAL_TERMINATION would previously also match a bare "status=" prefix). - Wait for a real *.dmp file (bounded, DUMP_WRITE_GRACE_MS) before sending SIGTERM — Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated, so shutting down immediately raced the dump actually being written. - Dump-file checks now filter to *.dmp specifically — Crashpad's settings.dat/lock/.meta files are written during normal init and would otherwise falsely count as "a dump produced". - The whole cycle body now runs inside try/catch with a guaranteed SIGKILL cleanup in the catch block, so a failed proof can no longer leave the browser (and its subprocesses) orphaned on the CI runner. - The post-SIGTERM exit is now checked for a clean signal/code, matching runCycle's existing check — an abnormal browser exit during shutdown was previously accepted as a successful isolation proof. Also echoes the observed renderer-crash status line on the success path (previously only visible via logStderr on failure) for real CI-log observability going forward. Co-Authored-By: Claude Sonnet 5 --- scripts/cef/run-launch-cycle-proof.mjs | 148 +++++++++++++++---------- 1 file changed, 87 insertions(+), 61 deletions(-) diff --git a/scripts/cef/run-launch-cycle-proof.mjs b/scripts/cef/run-launch-cycle-proof.mjs index 494b7350f..793abf892 100644 --- a/scripts/cef/run-launch-cycle-proof.mjs +++ b/scripts/cef/run-launch-cycle-proof.mjs @@ -58,8 +58,11 @@ const EXPECTED_TITLE_LINE = 'title = WorldScript Studio'; // QNBS-v3: crash-reporting/symbolization competency-gate proof (CEF-RUST-COMPETENCY-MATRIX.md) — mechanism verified against real CEF 151 source (libcef/common/crash_reporting.cc, crash_reporter_client.cc), not assumed; see docs/cef/knowledge/cef-architecture-primer.md. const CRASH_REPORTING_ENABLED_LINE = 'crash_reporting_enabled = true'; -const RENDERER_TERMINATED_PROOF_PREFIX = 'renderer_terminated status='; +// QNBS-v3: requires the specific TS_PROCESS_CRASHED value, not a bare "status=" prefix — CodeAnt review finding on PR #392 (TS_LAUNCH_FAILED/TS_PROCESS_WAS_KILLED/TS_ABNORMAL_TERMINATION would otherwise also satisfy the proof). +const RENDERER_CRASHED_PROOF_LINE = 'renderer_terminated status=TS_PROCESS_CRASHED'; const CRASH_URL = 'chrome://crash'; +// QNBS-v3: Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated — CodeAnt review finding on PR #392; this is how long the harness waits for a real .dmp file before giving up, separate from SHUTDOWN_GRACE_MS's own meaning. +const DUMP_WRITE_GRACE_MS = 5000; if (!binaryPath || !url) { console.error( @@ -101,7 +104,7 @@ function logStderr(label, stderr) { if (stderr) console.error(`[launch-cycle-proof] ${label} stderr:\n${stderr}`); } -// QNBS-v3: recursive, not a flat readdir — Crashpad's on-disk database nests reports under subdirectories (e.g. completed/, pending/, attachments/) whose exact layout isn't asserted on here; presence of any file is the proof. +// QNBS-v3: recursive, not a flat readdir — Crashpad's on-disk database nests reports under subdirectories (e.g. pending/, completed/, attachments/) whose exact layout isn't asserted on here; callers filter the result to *.dmp specifically (settings.dat/lock/.meta files are written during normal init and are not evidence of a dump). function findFilesRecursive(dir) { let entries; try { @@ -189,7 +192,7 @@ async function runCrashReportingProofCycle() { console.log( `[launch-cycle-proof] Crash-reporting proof: launching with ${CRASH_URL} to deliberately crash the renderer…`, ); - // QNBS-v3: fresh, empty-at-start temp dir — BREAKPAD_DUMP_LOCATION (verified in libcef/common/crash_reporter_client.cc) overrides where CEF/Crashpad writes dumps on Linux/POSIX, so "any file appears here" is unambiguous evidence, no need to guess CEF's default directory layout. + // QNBS-v3: fresh, empty-at-start temp dir — BREAKPAD_DUMP_LOCATION (verified in libcef/common/crash_reporter_client.cc) overrides where CEF/Crashpad writes dumps on Linux/POSIX, so a *.dmp file appearing here is unambiguous evidence, no need to guess CEF's default directory layout. const dumpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'worldscript-crash-dumps-')); const child = spawn(binaryPath, [`--url=${CRASH_URL}`, '--enable-logging=stderr', '--v=1'], { @@ -211,71 +214,94 @@ async function runCrashReportingProofCycle() { child.once('exit', (code, signal) => resolve({ code, signal })), ); - const rendererTerminated = await Promise.race([ - (async () => { - while (!stdout.includes(RENDERER_TERMINATED_PROOF_PREFIX)) { - await sleep(200); - } - return true; - })(), - exited.then(() => false), - sleep(STARTUP_GRACE_MS).then(() => false), - ]); - - if (!rendererTerminated) { - logStderr('Crash-reporting proof', stderr); - throw new Error( - `Crash-reporting proof: "${RENDERER_TERMINATED_PROOF_PREFIX}" not observed within ${STARTUP_GRACE_MS}ms (or the process exited early) — stdout so far:\n${stdout}`, + // QNBS-v3: every throw below is caught here so the child is always reaped, even on a failed proof — CodeAnt/Qodo review finding on PR #392 (a thrown assertion left the browser and its subprocesses orphaned). + try { + // QNBS-v3: requires the specific TS_PROCESS_CRASHED value, not just any termination status — CodeAnt review finding on PR #392 (TS_LAUNCH_FAILED/TS_PROCESS_WAS_KILLED/TS_ABNORMAL_TERMINATION would otherwise also satisfy a bare prefix match). chrome://crash triggers a real SIGSEGV in the renderer, confirmed against CEF's own cefclient reference usage of this exact URL. + const rendererCrashed = await Promise.race([ + (async () => { + while (!stdout.includes(RENDERER_CRASHED_PROOF_LINE)) { + await sleep(200); + } + return true; + })(), + exited.then(() => false), + sleep(STARTUP_GRACE_MS).then(() => false), + ]); + + if (!rendererCrashed) { + throw new Error( + `"${RENDERER_CRASHED_PROOF_LINE}" not observed within ${STARTUP_GRACE_MS}ms (or the process exited early) — stdout so far:\n${stdout}`, + ); + } + // QNBS-v3: echoed on the success path too, not just via logStderr on failure — the raw child stdout is otherwise invisible in the CI log (Qodo/CodeAnt review context on PR #392: nothing here previously proved which status value was actually observed). + console.log( + `[launch-cycle-proof] Crash-reporting proof: observed "${RENDERER_CRASHED_PROOF_LINE}".`, ); - } - // QNBS-v3: the actual "renderer termination observed and handled" evidence (CEF-RUST-COMPETENCY-MATRIX.md) — only the renderer subprocess should have died; the browser process and its message loop must still be running. - if (!processTreeAlive()) { - logStderr('Crash-reporting proof', stderr); - throw new Error( - 'Crash-reporting proof: browser process is not alive after the renderer crash — process isolation did not hold.', + // QNBS-v3: the actual "renderer termination observed and handled" evidence (CEF-RUST-COMPETENCY-MATRIX.md) — only the renderer subprocess should have died; the browser process and its message loop must still be running. + if (!processTreeAlive()) { + throw new Error( + 'browser process is not alive after the renderer crash — process isolation did not hold.', + ); + } + + if (!stdout.includes(CRASH_REPORTING_ENABLED_LINE)) { + throw new Error( + `"${CRASH_REPORTING_ENABLED_LINE}" not observed — crash_reporter.cfg (apps/desktop-cef/resources/crash_reporter.cfg) was not found/parsed next to the binary.`, + ); + } + + // QNBS-v3: Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated — CodeAnt review finding on PR #392 (shutting down the browser immediately raced the dump actually being written). Wait for a real *.dmp file, bounded, before sending SIGTERM. + const dumpAppeared = await Promise.race([ + (async () => { + while (findFilesRecursive(dumpDir).filter((f) => f.endsWith('.dmp')).length === 0) { + await sleep(200); + } + return true; + })(), + sleep(DUMP_WRITE_GRACE_MS).then(() => false), + ]); + // QNBS-v3: filtered to .dmp specifically — CodeAnt/Qodo review finding on PR #392 (Crashpad's settings.dat/lock/.meta files are written during normal init and would otherwise falsely count as "a dump produced"). + const dumpFiles = findFilesRecursive(dumpDir).filter((f) => f.endsWith('.dmp')); + if (!dumpAppeared || dumpFiles.length === 0) { + throw new Error( + `no .dmp file appeared under BREAKPAD_DUMP_LOCATION (${dumpDir}) within ${DUMP_WRITE_GRACE_MS}ms despite crash_reporting_enabled=true and an observed renderer crash.`, + ); + } + console.log( + `[launch-cycle-proof] Crash-reporting proof: dump file(s) confirmed before shutdown: ${dumpFiles.join(', ')}`, ); - } - if (!stdout.includes(CRASH_REPORTING_ENABLED_LINE)) { - logStderr('Crash-reporting proof', stderr); - throw new Error( - `Crash-reporting proof: "${CRASH_REPORTING_ENABLED_LINE}" not observed — crash_reporter.cfg (apps/desktop-cef/resources/crash_reporter.cfg) was not found/parsed next to the binary.`, + child.kill('SIGTERM'); + const shutdownResult = await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS).then(() => null)]); + if (!shutdownResult) { + throw new Error( + 'process did not exit within the shutdown grace period after the renderer crash.', + ); + } + // QNBS-v3: same clean-shutdown check as runCycle — Qodo review finding on PR #392 (the crash cycle accepted any exit, including an abnormal one, as a successful isolation proof). + const cleanShutdown = shutdownResult.signal === 'SIGTERM' || shutdownResult.code === 0; + if (!cleanShutdown) { + throw new Error( + `abnormal exit during shutdown (code=${shutdownResult.code}, signal=${shutdownResult.signal}).`, + ); + } + + await sleep(ORPHAN_CHECK_GRACE_MS); + if (processTreeAlive()) { + throw new Error('orphaned worldscript_host process(es) still running after shutdown.'); + } + + console.log( + '[launch-cycle-proof] Crash-reporting proof: OK — crash reporting enabled, renderer crash observed and handled (browser process survived), ' + + `${dumpFiles.length} dump file(s) written to the crash dump location. Full symbolization (dump_syms/minidump_stackwalk) requires a complete Chromium source checkout and is out of scope — see docs/cef/knowledge/cef-architecture-primer.md.`, ); - } - - child.kill('SIGTERM'); - const shutdownResult = await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS).then(() => null)]); - if (!shutdownResult) { + } catch (err) { logStderr('Crash-reporting proof', stderr); - throw new Error( - 'Crash-reporting proof: process did not exit within the shutdown grace period after the renderer crash.', - ); + // QNBS-v3: unconditional, not `if (!child.killed)` — .killed only reflects whether kill() was ever called, not whether the process actually died (e.g. SIGTERM already sent but the shutdown-grace-period/orphan checks below still failed); kill() on an already-exited process is a harmless no-op. + child.kill('SIGKILL'); + throw new Error(`Crash-reporting proof: ${err instanceof Error ? err.message : String(err)}`); } - - await sleep(ORPHAN_CHECK_GRACE_MS); - if (processTreeAlive()) { - logStderr('Crash-reporting proof', stderr); - throw new Error( - 'Crash-reporting proof: orphaned worldscript_host process(es) still running after shutdown.', - ); - } - - const dumpFiles = findFilesRecursive(dumpDir); - console.log( - `[launch-cycle-proof] Crash-reporting proof: dump location (${dumpDir}) after the crash: ${dumpFiles.length > 0 ? dumpFiles.join(', ') : '(empty)'}`, - ); - if (dumpFiles.length === 0) { - logStderr('Crash-reporting proof', stderr); - throw new Error( - `Crash-reporting proof: no file was written under BREAKPAD_DUMP_LOCATION (${dumpDir}) despite crash_reporting_enabled=true and an observed renderer crash.`, - ); - } - - console.log( - '[launch-cycle-proof] Crash-reporting proof: OK — crash reporting enabled, renderer crash observed and handled (browser process survived), ' + - `${dumpFiles.length} file(s) written to the crash dump location. Full symbolization (dump_syms/minidump_stackwalk) requires a complete Chromium source checkout and is out of scope — see docs/cef/knowledge/cef-architecture-primer.md.`, - ); } async function main() { From 0b230eab4948550f9a088f0fbadc9b53decd07bc Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 02:47:53 +0200 Subject: [PATCH 4/6] =?UTF-8?q?fix(cef):=20second=20wave=20=E2=80=94=20orp?= =?UTF-8?q?han=20cleanup,=20temp-dir=20hygiene,=20doc=20overclaim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3 more real findings (CodeAnt + CodeRabbit) on PR #392's crash-reporting proof: - The two polling while-loops (renderer-crash wait, dump-appeared wait) kept scheduling sleep(200) forever after their own Promise.race already settled via a different arm — an abandoned loop could keep Node alive indefinitely on a failed/timed-out proof. Both loops now check a `stopPolling` flag set once the race resolves. - The dump directory (fs.mkdtempSync) was never removed on success or failure — it can contain real browser memory. Now removed in a `finally` block. - The catch-block failure cleanup killed the child but didn't wait for it to actually exit or check for orphaned descendants, unlike the normal shutdown path. Now awaits exit and logs (not throws — the original failure stays the reported cause) if the process tree isn't clean. Also fixes a real overclaim CodeRabbit caught across all 3 docs: they described `.dmp`/`.meta`/`settings.dat` as equally "produced/verified" evidence, but the harness only asserts on `.dmp` — `.meta`/`settings.dat` were observed in the CI log, not independently checked. Docs now say so. Co-Authored-By: Claude Sonnet 5 --- docs/architecture/native-readiness.md | 2 +- docs/cef/CEF-RUST-COMPETENCY-MATRIX.md | 2 +- docs/cef/knowledge/cef-architecture-primer.md | 2 +- scripts/cef/run-launch-cycle-proof.mjs | 27 ++++++++++++++++--- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/docs/architecture/native-readiness.md b/docs/architecture/native-readiness.md index 666d76d8d..47e6f3437 100644 --- a/docs/architecture/native-readiness.md +++ b/docs/architecture/native-readiness.md @@ -61,7 +61,7 @@ Wave 2's first deliverable — the CEF binding/C++ decision — is now backed by | CEF lifecycle assumptions documented | **PASS** | cef-runtime | `docs/cef/knowledge/subprocess-and-shutdown.md`'s core Wave 2 claim (SIGTERM → graceful `TryCloseBrowser`/`OnBeforeClose`/`CefQuitMessageLoop`/`CefShutdown`, repeated clean start/close cycles) now has a real linked chain: test (`scripts/cef/run-launch-cycle-proof.mjs`) → CI job (`🧪 CEF Learning Harness`) → doc, exactly what §61.1.4 requires. Save-coordinator/window-state persistence remain explicitly Wave 5+ scope (not a Wave 2 gap); Windows/macOS and a real packaged layout remain open, tracked in the doc's own "Outline" section. | | Early Accessibility Gate | Not yet attempted (real blocker found) | cef-runtime, Wave 2 | PR #391 attempted `CefAccessibilityHandler` — does not compile against CEF 151.3.18 (`CefClient::GetAccessibilityHandler()` doesn't exist in this version). Reverted rather than left half-working, after a fallback attempt (enable-only, no observability) regressed the previously-reliable FFI/rendering proofs. Real CEF-151 API research needed before the next attempt — see `docs/cef/knowledge/cef-architecture-primer.md`. | | Sandbox posture | Not yet attempted | desktop-security, Wave 2/3 (roadmap §12) | Every run so far used `no_sandbox=true`; zero evidence either way on this row. | -| Crash reporting / renderer-crash resilience | **PASS** — crash-reporting half only | cef-runtime | PR #392: `crash_reporter.cfg` + `CefCrashReportingEnabled()` verified true, `chrome://crash` deliberately crashes the renderer, `CefRequestHandler::OnRenderProcessTerminated` fires (`TS_PROCESS_CRASHED`), the browser process/message loop survive, and a real Crashpad dump (`.dmp`/`.meta`/`settings.dat`) is produced under an overridden `BREAKPAD_DUMP_LOCATION` — all CI-run, not a doc claim. Symbolization (decoding the dump into a stack trace via `dump_syms`/`minidump_stackwalk`) needs a full Chromium source checkout and was not attempted — see `docs/cef/knowledge/cef-architecture-primer.md`. | +| Crash reporting / renderer-crash resilience | **PASS** — crash-reporting half only | cef-runtime | PR #392: `crash_reporter.cfg` + `CefCrashReportingEnabled()` verified true, `chrome://crash` deliberately crashes the renderer, `CefRequestHandler::OnRenderProcessTerminated` fires (`TS_PROCESS_CRASHED`), the browser process/message loop survive, and a real Crashpad `.dmp` file — the harness's actual assertion, alongside Crashpad's own `.meta`/`settings.dat` housekeeping files (observed, not independently asserted) — is produced under an overridden `BREAKPAD_DUMP_LOCATION`; all CI-run, not a doc claim. Symbolization (decoding the dump into a stack trace via `dump_syms`/`minidump_stackwalk`) needs a full Chromium source checkout and was not attempted — see `docs/cef/knowledge/cef-architecture-primer.md`. | | CEF SDK fetch/verify + version diagnostics automated | **PASS** | cef-runtime | `🧪 CEF Learning Harness` CI job (`.github/workflows/cef-learning-harness.yml`) fetches the pinned CEF SDK, verifies its checksum, and parses real version macros out of the extracted `include/cef_version.h` — a genuine CI-run check, not a doc claim. | | Linux dependency inventory — clean-machine data point | DEBT — partial | cef-runtime | Same CI job runs the package-presence check against a stock `ubuntu-latest` runner before any `apt-get`, adding a real second data point beyond the spike's one already-configured dev machine. Still narrow: `dpkg` package-presence only (not `ldd` against the actual shipped `.so` files), one distro/runner image. | | CEF host build + repeated launch/close cycle proof, in CI | **PASS** | cef-runtime | PR #388: `apps/desktop-cef/`'s `worldscript_host` (real, repo-committed C++/Rust source, not spike code) builds against the fetched CEF SDK and runs 3 independently-verified clean start/close cycles under Xvfb in CI — the roadmap's literal "isolated learning harness" / "safe repeated startup/shutdown" deliverables (§3142), not just the fetch/diagnostics increment. | diff --git a/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md b/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md index 160cb1383..f918d252d 100644 --- a/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md +++ b/docs/cef/CEF-RUST-COMPETENCY-MATRIX.md @@ -28,7 +28,7 @@ CI validation of this block ("fail CI when a required item for the active progra | CEF threading & lifetime rules (UI-thread callbacks, IO thread, ref-counted objects, callback lifetime, async cancellation, shutdown races) | Partial | `CEF_REQUIRE_UI_THREAD()` used throughout; `IMPLEMENT_REFCOUNTING`/`CefRefPtr` applied correctly; a real callback-lifetime lesson learned and fixed (`base::Unretained` vs. a plain `CefTask` — see `apps/desktop-cef/src/worldscript_handler.cpp`). No dedicated review doc yet (`docs/cef/knowledge/threading-and-lifetimes.md` still skeleton); IO thread and async-cancellation patterns untouched. | | Rust binding layer (crate/version, unsafe/FFI boundary, wrapper ownership, API coverage gaps, upgrade procedure) | Partial | `apps/desktop-cef/rust-core/` (`worldscript_rust_core`, Corrosion-linked) — FFI boundary proven inside the real CEF host in CI (PR #388), not just an isolated test. No upgrade procedure written yet (`docs/cef/knowledge/binding-upgrade-playbook.md` still skeleton); API coverage is currently one trivial function, not representative of real surface area. | | Cross-platform native host (Linux loader/resource layout, Windows process/installer/sandbox, macOS bundle/signing, window lifecycle, high-DPI, IME/a11y) | Partial (Linux only) | Linux loader/resource layout confirmed via a real filesystem listing in CI (`docs/cef/knowledge/linux-runtime-notes.md`); a real cwd-relative-path startup bug found and fixed. Zero Windows/macOS evidence. Window lifecycle proven for open/close only — high-DPI and IME/a11y untouched. | -| Operational CEF (crash reporting, symbol handling, version-update automation, sandbox verification, packaging deps, runtime diagnostics) | Partial | Packaging deps: `scripts/cef/check-linux-runtime-deps.mjs` (CI-run). Runtime diagnostics: `scripts/cef/print-cef-version-diagnostics.mjs` + verbose CEF logging (`--enable-logging=stderr --v=1`) added mid-debugging this wave. Crash reporting: proven in CI (PR #392) — `crash_reporter.cfg` + `CefCrashReportingEnabled()` + a deliberately induced renderer crash (`chrome://crash`) produced a real Crashpad dump (`.dmp`/`.meta`/`settings.dat`) under an overridden `BREAKPAD_DUMP_LOCATION`; the browser process survived. Symbol handling (decoding a dump into a stack trace) needs `dump_syms`/`minidump_stackwalk` built from a full Chromium source checkout — out of reach of this project's minimal-CEF-SDK CI setup, not attempted. Version-update automation and sandbox verification remain not started. | +| Operational CEF (crash reporting, symbol handling, version-update automation, sandbox verification, packaging deps, runtime diagnostics) | Partial | Packaging deps: `scripts/cef/check-linux-runtime-deps.mjs` (CI-run). Runtime diagnostics: `scripts/cef/print-cef-version-diagnostics.mjs` + verbose CEF logging (`--enable-logging=stderr --v=1`) added mid-debugging this wave. Crash reporting: proven in CI (PR #392) — `crash_reporter.cfg` + `CefCrashReportingEnabled()` + a deliberately induced renderer crash (`chrome://crash`) produced a real Crashpad `.dmp` file (the harness's actual assertion) under an overridden `BREAKPAD_DUMP_LOCATION`, alongside Crashpad's own `.meta`/`settings.dat` housekeeping files (observed, not independently asserted); the browser process survived. Symbol handling (decoding a dump into a stack trace) needs `dump_syms`/`minidump_stackwalk` built from a full Chromium source checkout — out of reach of this project's minimal-CEF-SDK CI setup, not attempted. Version-update automation and sandbox verification remain not started. | ## Appendix A.1 checklist (live) diff --git a/docs/cef/knowledge/cef-architecture-primer.md b/docs/cef/knowledge/cef-architecture-primer.md index b321ff09b..4f9ae4022 100644 --- a/docs/cef/knowledge/cef-architecture-primer.md +++ b/docs/cef/knowledge/cef-architecture-primer.md @@ -45,7 +45,7 @@ Unlike the accessibility attempt above, every mechanism here was verified agains **What's implemented** (`apps/desktop-cef/resources/crash_reporter.cfg`, `CMakeLists.txt`, `main.cpp`, `worldscript_handler.{h,cpp}`): `crash_reporter.cfg` (format from `include/cef_crash_util.h`) is copied next to the built executable via a `configure_file` step; `main.cpp` logs `CefCrashReportingEnabled()` after `CefInitialize`; `WorldScriptHandler` now also implements `CefRequestHandler` and overrides `OnRenderProcessTerminated` — a real method (confirmed present in `include/cef_client.h`'s `GetRequestHandler()`, unlike the accessibility handler) that fires in the browser process when a renderer subprocess dies, without the browser process itself going down. -**Directly observed evidence, PR #392, `🧪 CEF Learning Harness` CI job**: the harness launches `worldscript_host --url=chrome://crash` (the same debug URL CEF's own `cefclient` reference app uses to test this exact path) with `BREAKPAD_DUMP_LOCATION` pointed at a fresh, empty temp directory. The CI log shows `crash_reporting_enabled = true`, then `renderer_terminated status=TS_PROCESS_CRASHED error_code=...`, then — after the harness's usual graceful-shutdown sequence — three real files in that directory: `pending/.dmp`, `pending/.meta`, and `settings.dat`. The browser process's own clean-shutdown proof (same mechanism as the repeated start/close cycles) passed too, confirming process isolation held: only the renderer subprocess died. +**Directly observed evidence, PR #392, `🧪 CEF Learning Harness` CI job**: the harness launches `worldscript_host --url=chrome://crash` (the same debug URL CEF's own `cefclient` reference app uses to test this exact path) with `BREAKPAD_DUMP_LOCATION` pointed at a fresh, empty temp directory. The CI log shows `crash_reporting_enabled = true`, then `renderer_terminated status=TS_PROCESS_CRASHED error_code=...`, then a real `pending/.dmp` file — the one artifact the harness actually asserts on (`endsWith('.dmp')`, waited for before shutdown) — alongside `pending/.meta` and `settings.dat`, Crashpad's own housekeeping files that were also observed in that directory but are not independently checked by the harness. The browser process's own clean-shutdown proof (same mechanism as the repeated start/close cycles) passed too, confirming process isolation held: only the renderer subprocess died. **What this does NOT prove**: symbolization — decoding the `.dmp` file into a human-readable stack trace — needs `dump_syms` and `minidump_stackwalk`, which CEF's own docs say must be built from a *complete Chromium source checkout* (`gn`/`ninja`, hours of build time, tens of GB of disk). That is out of reach of this project's minimal-CEF-SDK-only CI setup (and of the local dev machine's own constrained RAM/disk, per this repo's own low-end-hardware guidance) and was not attempted. `crash_symbolization_smoke` in `docs/cef/CEF-RUST-COMPETENCY-MATRIX.md` stays `false` for that reason — the crash-*reporting* half is proven; symbolization is a separate, still-open item. diff --git a/scripts/cef/run-launch-cycle-proof.mjs b/scripts/cef/run-launch-cycle-proof.mjs index 793abf892..0da687ce0 100644 --- a/scripts/cef/run-launch-cycle-proof.mjs +++ b/scripts/cef/run-launch-cycle-proof.mjs @@ -216,17 +216,21 @@ async function runCrashReportingProofCycle() { // QNBS-v3: every throw below is caught here so the child is always reaped, even on a failed proof — CodeAnt/Qodo review finding on PR #392 (a thrown assertion left the browser and its subprocesses orphaned). try { + // QNBS-v3: `stopPolling` lets each while-loop below notice its own Promise.race already settled — CodeRabbit review finding on PR #392 (an abandoned loop kept scheduling sleep(200) forever after the race resolved via the other arm, keeping the Node process alive indefinitely on a failed/timed-out proof). + let stopPolling = false; + // QNBS-v3: requires the specific TS_PROCESS_CRASHED value, not just any termination status — CodeAnt review finding on PR #392 (TS_LAUNCH_FAILED/TS_PROCESS_WAS_KILLED/TS_ABNORMAL_TERMINATION would otherwise also satisfy a bare prefix match). chrome://crash triggers a real SIGSEGV in the renderer, confirmed against CEF's own cefclient reference usage of this exact URL. const rendererCrashed = await Promise.race([ (async () => { - while (!stdout.includes(RENDERER_CRASHED_PROOF_LINE)) { + while (!stopPolling && !stdout.includes(RENDERER_CRASHED_PROOF_LINE)) { await sleep(200); } - return true; + return stdout.includes(RENDERER_CRASHED_PROOF_LINE); })(), exited.then(() => false), sleep(STARTUP_GRACE_MS).then(() => false), ]); + stopPolling = true; if (!rendererCrashed) { throw new Error( @@ -252,15 +256,20 @@ async function runCrashReportingProofCycle() { } // QNBS-v3: Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated — CodeAnt review finding on PR #392 (shutting down the browser immediately raced the dump actually being written). Wait for a real *.dmp file, bounded, before sending SIGTERM. + stopPolling = false; const dumpAppeared = await Promise.race([ (async () => { - while (findFilesRecursive(dumpDir).filter((f) => f.endsWith('.dmp')).length === 0) { + while ( + !stopPolling && + findFilesRecursive(dumpDir).filter((f) => f.endsWith('.dmp')).length === 0 + ) { await sleep(200); } - return true; + return findFilesRecursive(dumpDir).some((f) => f.endsWith('.dmp')); })(), sleep(DUMP_WRITE_GRACE_MS).then(() => false), ]); + stopPolling = true; // QNBS-v3: filtered to .dmp specifically — CodeAnt/Qodo review finding on PR #392 (Crashpad's settings.dat/lock/.meta files are written during normal init and would otherwise falsely count as "a dump produced"). const dumpFiles = findFilesRecursive(dumpDir).filter((f) => f.endsWith('.dmp')); if (!dumpAppeared || dumpFiles.length === 0) { @@ -300,7 +309,17 @@ async function runCrashReportingProofCycle() { logStderr('Crash-reporting proof', stderr); // QNBS-v3: unconditional, not `if (!child.killed)` — .killed only reflects whether kill() was ever called, not whether the process actually died (e.g. SIGTERM already sent but the shutdown-grace-period/orphan checks below still failed); kill() on an already-exited process is a harmless no-op. child.kill('SIGKILL'); + // QNBS-v3: await + verify, not fire-and-forget — CodeAnt review finding on PR #392 (the normal shutdown path waits and checks for orphans; the failure path didn't, so a failed proof could leave CEF subprocesses running after the harness exits). Logged only, never thrown here — the original failure stays the reported cause. + await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS)]); + if (processTreeAlive()) { + console.error( + '[launch-cycle-proof] Crash-reporting proof: WARNING — worldscript_host process(es) still running after failure cleanup.', + ); + } throw new Error(`Crash-reporting proof: ${err instanceof Error ? err.message : String(err)}`); + } finally { + // QNBS-v3: CodeRabbit review finding on PR #392 — the dump directory (which can contain real browser memory) was never removed on success or failure. + fs.rmSync(dumpDir, { recursive: true, force: true }); } } From 7a5ad3e39b0bf91e496920e914f51ada518b6d87 Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 03:02:57 +0200 Subject: [PATCH 5/6] fix(cef): sweep all matching processes on failure cleanup, not just the tracked child MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeAnt review finding on PR #392: the failure-cleanup path only SIGKILLed the one Node-tracked child PID. CEF re-execs the same binary for every subprocess role (renderer/GPU/crashpad-handler) — a surviving descendant (potentially a Crashpad handler still writing to the dump directory) could outlive that single kill, and the finally block would then remove dumpDir out from under it. killAllMatchingProcesses() (extracted alongside the existing processTreeAlive() from a shared listMatchingPids() helper, same pgrep -f ^binaryPath anchor) now sweeps and kills every matching process before the finally block's fs.rmSync, not just the direct child. Co-Authored-By: Claude Sonnet 5 --- scripts/cef/run-launch-cycle-proof.mjs | 37 +++++++++++++++++++------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/scripts/cef/run-launch-cycle-proof.mjs b/scripts/cef/run-launch-cycle-proof.mjs index 0da687ce0..f0b4892e5 100644 --- a/scripts/cef/run-launch-cycle-proof.mjs +++ b/scripts/cef/run-launch-cycle-proof.mjs @@ -81,7 +81,7 @@ function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } -function processTreeAlive() { +function listMatchingPids() { try { // QNBS-v3: anchored to the start of the command line — xvfb-run's own wrapper process also carries binaryPath as an argument it forwards, so an unanchored match false-flags it as a leaked worldscript_host. const out = execFileSync('pgrep', ['-f', `^${binaryPath}`], { @@ -89,14 +89,28 @@ function processTreeAlive() { }) .toString() .trim(); - const remaining = out + return out .split('\n') .filter(Boolean) .map(Number) .filter((pid) => pid !== process.pid); - return remaining.length > 0; } catch { - return false; // pgrep exits 1 when nothing matches — that's the clean state. + return []; // pgrep exits 1 when nothing matches — that's the clean state. + } +} + +function processTreeAlive() { + return listMatchingPids().length > 0; +} + +// QNBS-v3: CEF re-execs the same binary for every subprocess role (renderer/GPU/crashpad-handler) — CodeAnt review finding on PR #392: SIGKILLing only the one tracked child PID can leave those descendants (including a Crashpad handler still writing to the dump directory) alive. This sweeps and kills everything matching the binary path, not just the direct child. +function killAllMatchingProcesses() { + for (const pid of listMatchingPids()) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + // Already gone between the pgrep snapshot and this call — fine. + } } } @@ -309,16 +323,21 @@ async function runCrashReportingProofCycle() { logStderr('Crash-reporting proof', stderr); // QNBS-v3: unconditional, not `if (!child.killed)` — .killed only reflects whether kill() was ever called, not whether the process actually died (e.g. SIGTERM already sent but the shutdown-grace-period/orphan checks below still failed); kill() on an already-exited process is a harmless no-op. child.kill('SIGKILL'); - // QNBS-v3: await + verify, not fire-and-forget — CodeAnt review finding on PR #392 (the normal shutdown path waits and checks for orphans; the failure path didn't, so a failed proof could leave CEF subprocesses running after the harness exits). Logged only, never thrown here — the original failure stays the reported cause. + // QNBS-v3: await + verify, not fire-and-forget — CodeAnt review finding on PR #392 (the normal shutdown path waits and checks for orphans; the failure path didn't, so a failed proof could leave CEF subprocesses running after the harness exits). await Promise.race([exited, sleep(SHUTDOWN_GRACE_MS)]); + // QNBS-v3: sweeps every process matching the binary path, not just the tracked child — CodeAnt review finding on PR #392 (a surviving renderer/GPU/crashpad-handler descendant could still be using dumpDir when the finally block below removes it). Runs before that removal, not after. if (processTreeAlive()) { - console.error( - '[launch-cycle-proof] Crash-reporting proof: WARNING — worldscript_host process(es) still running after failure cleanup.', - ); + killAllMatchingProcesses(); + await sleep(ORPHAN_CHECK_GRACE_MS); + if (processTreeAlive()) { + console.error( + '[launch-cycle-proof] Crash-reporting proof: WARNING — worldscript_host process(es) still running after failure cleanup.', + ); + } } throw new Error(`Crash-reporting proof: ${err instanceof Error ? err.message : String(err)}`); } finally { - // QNBS-v3: CodeRabbit review finding on PR #392 — the dump directory (which can contain real browser memory) was never removed on success or failure. + // QNBS-v3: CodeRabbit review finding on PR #392 — the dump directory (which can contain real browser memory) was never removed on success or failure. Runs after the process-tree sweep above, not before, so it never races a still-running Crashpad handler. fs.rmSync(dumpDir, { recursive: true, force: true }); } } From 1e8d253ff4ad7ca1bad9fc12bf60ff84e5917b0d Mon Sep 17 00:00:00 2001 From: qnbs <155236708+qnbs@users.noreply.github.com> Date: Wed, 19 Aug 2026 03:16:18 +0200 Subject: [PATCH 6/6] =?UTF-8?q?fix(cef):=20raise=20DUMP=5FWRITE=5FGRACE=5F?= =?UTF-8?q?MS=20proactively=20=E2=80=94=20CI-runner-variance=20finding=20o?= =?UTF-8?q?n=20PR=20#392?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeAnt review finding: a fixed 5s timeout for Crashpad's dump write could theoretically fail a valid crash-reporting setup on a loaded/slow CI runner, since finalization is asynchronous and its timing isn't bounded by anything this harness controls. This is the same class of issue that forced STARTUP_GRACE_MS from 4000ms to 10000ms earlier in this file (real CI-runner-speed variance, not a code bug) — raising DUMP_WRITE_GRACE_MS to 8000ms proactively here rather than waiting for a flaky failure to prove the same lesson twice. Co-Authored-By: Claude Sonnet 5 --- scripts/cef/run-launch-cycle-proof.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/cef/run-launch-cycle-proof.mjs b/scripts/cef/run-launch-cycle-proof.mjs index f0b4892e5..8b3f8586f 100644 --- a/scripts/cef/run-launch-cycle-proof.mjs +++ b/scripts/cef/run-launch-cycle-proof.mjs @@ -61,8 +61,8 @@ const CRASH_REPORTING_ENABLED_LINE = 'crash_reporting_enabled = true'; // QNBS-v3: requires the specific TS_PROCESS_CRASHED value, not a bare "status=" prefix — CodeAnt review finding on PR #392 (TS_LAUNCH_FAILED/TS_PROCESS_WAS_KILLED/TS_ABNORMAL_TERMINATION would otherwise also satisfy the proof). const RENDERER_CRASHED_PROOF_LINE = 'renderer_terminated status=TS_PROCESS_CRASHED'; const CRASH_URL = 'chrome://crash'; -// QNBS-v3: Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated — CodeAnt review finding on PR #392; this is how long the harness waits for a real .dmp file before giving up, separate from SHUTDOWN_GRACE_MS's own meaning. -const DUMP_WRITE_GRACE_MS = 5000; +// QNBS-v3: Crashpad's dump finalization is asynchronous relative to OnRenderProcessTerminated — CodeAnt review finding on PR #392; this is how long the harness waits for a real .dmp file before giving up, separate from SHUTDOWN_GRACE_MS's own meaning. Set generously (not the original 5000ms) per the same CI-runner-speed-variance lesson that forced STARTUP_GRACE_MS up from 4000ms to 10000ms in this same file — proactive, not waiting for a flaky failure to prove it (second CodeAnt finding, same PR). +const DUMP_WRITE_GRACE_MS = 8000; if (!binaryPath || !url) { console.error(