You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
pnpm check:unit: 249 unit files, 2333 tests, 8 smoke tests.
pnpm build.
git diff --check.
Fixture-backed CLI evidence:
Apple fixture wrote a symbolicated artifact and printed only the artifact path plus a short summary/proof line.
Android Java crash fixture returned the explicit unsupported/deferred hint for mapping.txt/ndk-stack/addr2line external workflows.
SkillGym case debug-symbols-apple-crash was added but not runnable in the sandbox because external runner approval was denied.
Residual risk: no real simulator crash/dSYM artifact was generated in this sandbox; coverage is fixture-backed with command construction and artifact proof.
Verdict: significant issues — the headline .ips path will fail on real device-produced crash files; the rest of the design is sound.
Findings
Major (arguably blocker) — src/debug-symbols.ts:136: real .ips files written by iOS/macOS since iOS 15 are two JSON documents — a single-line JSON header followed by the JSON payload. JSON.parse(text) on the whole file throws, the parser falls through to the text-crash reader, finds no Binary Images: section, and dies with UNSUPPORTED_OPERATION — i.e. the primary advertised input format fails for every real device artifact. The fixture in src/__tests__/debug-symbols.test.ts:91 is a single headerless JSON object, which is exactly why tests don't catch it. Fix: split on the first newline, parse the payload, preserve the header when rewriting output.
Minor — src/debug-symbols.ts:243: the Binary Images: regex requires an arch token, but legacy macOS.crash files use +com.example.App (1.0 - 1) <UUID> /path with a version instead — those parse to zero images and error as unsupported; arm64_32 (watchOS) is also missing from the alternation.
Minor — src/debug-symbols.ts:161,182,211,254: readNumber accepts any finite number, then BigInt(base)/BigInt(imageOffset) is called on it — a malformed artifact with a fractional value throws an uncaught RangeError instead of a normalized AppError.
Minor — src/debug-symbols.ts:307: findDsymBundles calls fs.stat(root) unguarded; a nonexistent --search-path surfaces as a raw ENOENT rather than an INVALID_ARGS AppError with a hint.
Minor — src/debug-symbols.ts:268-274: text-crash frames are matched to images by name only; when two binary images share a name (app + extension — common in real crashes), the first image's base is used for all frames with that name, producing wrong -l load addresses and silently wrong symbols.
Minor — src/debug-symbols.ts:429-434: unsymbolicated detection relies on atos echoing the address byte-for-byte; variants that zero-pad or print 0x… (in App) get treated as successful symbols. Also, atos output lines are zipped with input addresses after filtering blank lines — any stray blank line shifts the mapping and mis-attributes symbols across frames.
Minor — src/debug-symbols.ts:378-386: on no-match, artifactUuids dumps every image UUID into the error details; a real .ips has 300+ usedImages, so the payload is effectively unbounded relative to the "compact summary" goal.
Minor — src/debug-symbols.ts:482: readTextFile reads the whole artifact with no size cap; a multi-hundred-MB log is loaded fully into memory (twice, via split/join).
Minor (tests) — all five tests mock xcrun/dwarfdump/atos with idealized outputs; no two-document .ips fixture (masking finding 1), no multi-arch/multi-dSYM case, no garbage-input case. At least one fixture captured from a real device .ips should be checked in.
No command injection (spawn with shell: false argv arrays); the atos -l load address is computed correctly (image base as -l, base + imageOffset as absolute addresses — the right convention); UUID normalization symmetric; dSYM search bounded; output is paths + summary only.
Overall
Good architecture — clean parse/match/symbolicate separation, bounded search, well-normalized errors. But the flagship .ips format can't parse real device files at all; that must be fixed and validated against a real crash before merge. The rest is robustness hardening, worth doing but not individually blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a narrow
agent-device debug symbolsworkflow for crash symbolication in the #694 native diagnostics rollout..ips,.crash, and log-style crash artifacts by matching UUIDs against local.dSYMbundles viadwarfdump --uuid, then runningatos.debugscoped to symbols/crash artifact space; logs, network, perf, record/trace, and React Native internals stay routed to existing commands.Refs #699
Refs #694
Touched files: 21.
Validation
Worker validation passed:
pnpm format.pnpm check:quick.pnpm check:unit: 249 unit files, 2333 tests, 8 smoke tests.pnpm build.git diff --check.Fixture-backed CLI evidence:
mapping.txt/ndk-stack/addr2lineexternal workflows.SkillGym case
debug-symbols-apple-crashwas added but not runnable in the sandbox because external runner approval was denied.Residual risk: no real simulator crash/dSYM artifact was generated in this sandbox; coverage is fixture-backed with command construction and artifact proof.