Skip to content

feat(packaged): attach crash-scene evidence to packaged_runtime_failed - #5224

Merged
lefarcen merged 3 commits into
mainfrom
feat/packaged-startup-crash-evidence
Jul 6, 2026
Merged

feat(packaged): attach crash-scene evidence to packaged_runtime_failed#5224
lefarcen merged 3 commits into
mainfrom
feat/packaged-startup-crash-evidence

Conversation

@lefarcen

@lefarcen lefarcen commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Why

Use case: I was triaging the packaged_runtime_failed event (PostHog project 420348) to explain the 0.13.0 startup-crash population and hit a wall β€” the event tells me that startup failed but not why on a given machine.

Pain: Two of the biggest slices are un-actionable with today's fields:

  • The macOS daemon-start failures are better-sqlite3 ERR_MODULE_NOT_FOUND, yet I verified the shipped 0.13.0 mac-arm DMG's better_sqlite3.node is present, arm64, Developer-ID signed, notarized, and resolvable from the daemon sidecar location. So the field crash is a machine-side subset (~a couple dozen devices), not a build defect β€” and we currently have zero per-machine signal to explain the subset.
  • The Windows unknown bucket (the single largest slice, ~334/7d) carries no exit code and no daemon log to parse, so error_code/missing_module are both null and the row is a dead end.

This PR adds the on-machine crash evidence needed to separate those hypotheses in the dashboard instead of guessing.

What users will see

Nothing in the UI or CLI. This is internal crash telemetry emitted automatically by the packaged main process on the fatal-exit path (the daemon that normally hosts PostHog never came up, which is the whole reason this path exists). The only observable change is that the packaged_runtime_failed event now carries more diagnostic fields:

  • error_message / error_stack β€” the top-level thrown error, scrubbed of the user's home dir (scrubUserPaths) and length-capped. For the unknown bucket this is the only signal there is.
  • native_module_present / native_module_size / native_module_path β€” a best-effort statSync probe of the daemon's better-sqlite3 binding on that machine, which distinguishes "the .node is missing" from "the .node is present but unloadable" (arch mismatch / Gatekeeper quarantine / AV).

Startup errors are module-resolution / daemon-exit strings, not user content, and paths are scrubbed + text capped, so the exposure is bounded to build/OS strings.

Surface area

  • API / contract β€” extended PackagedRuntimeFailedProps in packages/contracts with the new optional telemetry fields.
  • None

Capability dual-track (UI + CLI) note: not applicable. This is an automatic internal crash-telemetry event, not a user-facing capability β€” there is no user action to expose through od or the web UI.

Bug fix verification

This is diagnostic instrumentation rather than a behavior bug fix, but I still led with a falsifiable spec per the bug-follow-up workflow:

  • Test path: `apps/packaged/tests/startup-telemetry.test.ts` (two new cases: scrubbed message/stack + native-module probe present/missing via an injected `statNativeModule` dep).
  • Red on the unmodified source, green on this branch? yes β€” confirmed by stashing only `startup-telemetry.ts` and re-running: both new cases failed (`expected undefined to be false`), then passed after the source change.

Validation

  • `pnpm guard` β€” pass (71/71).
  • `pnpm --filter @open-design/packaged --filter @open-design/contracts typecheck` β€” pass.
  • `pnpm --filter @open-design/packaged test` β€” pass (154/154, 14 files).

πŸ€– Generated with Claude Code

The pre-daemon startup crash class (`packaged_runtime_failed`, #4696) reports
only structured buckets today: failure_kind, exit_code, error_name, and a
log-tail-parsed error_code/missing_module. That is enough to see THAT startup
failed, but not WHY on a given machine:

- The mac `daemon-start` failures are better-sqlite3 ERR_MODULE_NOT_FOUND, yet
  the shipped 0.13.0 DMG's `better_sqlite3.node` is verified present, arm64,
  signed, notarized and resolvable β€” so this is a machine-side subset, not a
  build defect, and we have no per-machine signal to explain the subset.
- The Windows `unknown` bucket (the single largest slice) carries no exit code
  and no daemon log to parse, so today it is a dead end.

Enrich the event with on-machine crash evidence: the scrubbed + truncated
top-level error message/stack (the only signal the `unknown` bucket has), and a
best-effort probe of the daemon's better-sqlite3 binding on THIS machine
(present + size), which separates "file missing" from "file present but
unloadable" (arch mismatch / quarantine / AV). All paths and free-form text are
run through the existing `scrubUserPaths` and length-capped before send.

- startup-telemetry.ts: add `error_message`, `error_stack`,
  `native_module_present/size/path`; new `nativeModulePath` arg + injectable
  `statNativeModule` dep.
- index.ts: pass the packaged binding path
  (`getAppPath()/node_modules/better-sqlite3/build/Release/better_sqlite3.node`,
  layout verified against the shipped DMG) through the fatal-exit report.
- contracts: extend `PackagedRuntimeFailedProps` with the optional fields.
- tests: red-spec first β€” assert scrubbed message/stack and the native probe
  (present/missing) via the injected `statNativeModule`.
@lefarcen
lefarcen requested a review from a team as a code owner July 6, 2026 16:05
@lefarcen
lefarcen requested a review from PerishCode July 6, 2026 16:08
@lefarcen lefarcen added size/M PR changes 100-300 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/feature New feature labels Jul 6, 2026
@lefarcen lefarcen mentioned this pull request Jul 6, 2026
2 tasks
@lefarcen lefarcen added the skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. label Jul 6, 2026

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one blocking privacy issue in the new crash-scene telemetry. The added free-form error fields are sent under the existing home-dir scrubber, but that scrubber does not fully redact Windows profile paths when the profile directory contains whitespace, so the new event can leak part of the user's home path despite the PR's stated privacy bound.

πŸ” Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.

Comment thread apps/packaged/src/startup-telemetry.ts
Review (PerishCode/Looper): the new free-form error_message/error_stack fields
run through scrubUserPaths, but its Windows branch matched `[^\\\s]+` β€” it
stopped at the first whitespace. A Windows profile dir can contain a space
("C:\Users\John Doe\..."), so it produced "C:\Users\<redacted> Doe\..." and
leaked the rest of the segment, breaking this PR's stated privacy bound.

Consume the whole segment up to the next backslash (`[^\\]+`). POSIX home
segments cannot contain spaces and file:// URLs percent-encode them, so only
this backslash form needs the whitespace-tolerant boundary. Add fixture
coverage for the spaced profile β€” both standalone and embedded in a crash
message β€” asserting the surname never survives.
@lefarcen
lefarcen requested a review from PerishCode July 6, 2026 16:16

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found one remaining blocking privacy issue in the new crash-scene telemetry. The current update fixes backslash-form Windows paths with spaces, but the new free-form message/stack fields can still leak the same profile segment when the Windows path is slash-separated.

πŸ” Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.

Comment thread apps/packaged/src/startup-telemetry.ts Outdated
Review round 2 (PerishCode/Looper): the prior fix covered the backslash form,
but slash-normalized Windows paths ("C:/Users/John Doe/...", which JS/Electron/
Node diagnostics commonly emit) still leaked the surname β€” the POSIX "/Users/"
rule matched them and stopped at the first space, producing
"C:/Users/<redacted> Doe/...".

Replace the backslash-only Windows rule with a separator-agnostic one that
matches `<drive>:[\\/]Users[\\/]` and consumes the whole segment up to the next
slash OR backslash (spaces allowed), running before the POSIX rule. The `\r\n`
in the class keeps it from running across lines in a multi-line stack. POSIX
segments can't contain spaces, so that rule is left with its whitespace
boundary. Fixtures added for the slash form (standalone + embedded) and a
multi-line no-over-redaction case.
@lefarcen
lefarcen requested a review from PerishCode July 6, 2026 16:25

@PerishCode PerishCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lefarcen I reviewed the changed packaged startup telemetry ranges, including the added crash message/stack fields, native better-sqlite3 probe, contract extension, and the follow-up Windows path scrubber fixes. The latest head addresses the earlier profile-path privacy leaks, and the native module probe path lines up with the current non-ASAR packaged app layout used by the packager. Nice work tightening the diagnostic signal while keeping the emitted fields scrubbed and bounded.

πŸ” Powered by Looper Β· runner=reviewer Β· agent=codex Β· An autonomous AI dev team for your GitHub repos.

@lefarcen
lefarcen added this pull request to the merge queue Jul 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 6, 2026
@lefarcen
lefarcen added this pull request to the merge queue Jul 6, 2026
Merged via the queue into main with commit ba6435f Jul 6, 2026
21 checks passed
xxiaoxiong pushed a commit to xxiaoxiong/open-design that referenced this pull request Jul 13, 2026
nexu-io#5224)

* feat(packaged): attach crash-scene evidence to packaged_runtime_failed

The pre-daemon startup crash class (`packaged_runtime_failed`, nexu-io#4696) reports
only structured buckets today: failure_kind, exit_code, error_name, and a
log-tail-parsed error_code/missing_module. That is enough to see THAT startup
failed, but not WHY on a given machine:

- The mac `daemon-start` failures are better-sqlite3 ERR_MODULE_NOT_FOUND, yet
  the shipped 0.13.0 DMG's `better_sqlite3.node` is verified present, arm64,
  signed, notarized and resolvable β€” so this is a machine-side subset, not a
  build defect, and we have no per-machine signal to explain the subset.
- The Windows `unknown` bucket (the single largest slice) carries no exit code
  and no daemon log to parse, so today it is a dead end.

Enrich the event with on-machine crash evidence: the scrubbed + truncated
top-level error message/stack (the only signal the `unknown` bucket has), and a
best-effort probe of the daemon's better-sqlite3 binding on THIS machine
(present + size), which separates "file missing" from "file present but
unloadable" (arch mismatch / quarantine / AV). All paths and free-form text are
run through the existing `scrubUserPaths` and length-capped before send.

- startup-telemetry.ts: add `error_message`, `error_stack`,
  `native_module_present/size/path`; new `nativeModulePath` arg + injectable
  `statNativeModule` dep.
- index.ts: pass the packaged binding path
  (`getAppPath()/node_modules/better-sqlite3/build/Release/better_sqlite3.node`,
  layout verified against the shipped DMG) through the fatal-exit report.
- contracts: extend `PackagedRuntimeFailedProps` with the optional fields.
- tests: red-spec first β€” assert scrubbed message/stack and the native probe
  (present/missing) via the injected `statNativeModule`.

* fix(packaged): scrub full Windows profile segment even with spaces

Review (PerishCode/Looper): the new free-form error_message/error_stack fields
run through scrubUserPaths, but its Windows branch matched `[^\\\s]+` β€” it
stopped at the first whitespace. A Windows profile dir can contain a space
("C:\Users\John Doe\..."), so it produced "C:\Users\<redacted> Doe\..." and
leaked the rest of the segment, breaking this PR's stated privacy bound.

Consume the whole segment up to the next backslash (`[^\\]+`). POSIX home
segments cannot contain spaces and file:// URLs percent-encode them, so only
this backslash form needs the whitespace-tolerant boundary. Add fixture
coverage for the spaced profile β€” both standalone and embedded in a crash
message β€” asserting the surname never survives.

* fix(packaged): make Windows-home scrub separator-agnostic

Review round 2 (PerishCode/Looper): the prior fix covered the backslash form,
but slash-normalized Windows paths ("C:/Users/John Doe/...", which JS/Electron/
Node diagnostics commonly emit) still leaked the surname β€” the POSIX "/Users/"
rule matched them and stopped at the first space, producing
"C:/Users/<redacted> Doe/...".

Replace the backslash-only Windows rule with a separator-agnostic one that
matches `<drive>:[\\/]Users[\\/]` and consumes the whole segment up to the next
slash OR backslash (spaces allowed), running before the POSIX rule. The `\r\n`
in the class keeps it from running across lines in a multi-line stack. POSIX
segments can't contain spaces, so that rule is left with its whitespace
boundary. Fixtures added for the slash form (standalone + embedded) and a
multi-line no-over-redaction case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/M PR changes 100-300 lines skip-validation Maintainer override: bot will not auto-add needs-validation on this PR. type/feature New feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants