Skip to content

feat: add debug symbols workflow#756

Open
thymikee wants to merge 1 commit into
mainfrom
codex/debug-symbols
Open

feat: add debug symbols workflow#756
thymikee wants to merge 1 commit into
mainfrom
codex/debug-symbols

Conversation

@thymikee

Copy link
Copy Markdown
Member

Summary

Adds a narrow agent-device debug symbols workflow for crash symbolication in the #694 native diagnostics rollout.

  • Supports Apple .ips, .crash, and log-style crash artifacts by matching UUIDs against local .dSYM bundles via dwarfdump --uuid, then running atos.
  • Keeps debug scoped to symbols/crash artifact space; logs, network, perf, record/trace, and React Native internals stay routed to existing commands.
  • Exposes CLI, typed client, and MCP command metadata with compact path/summary output.
  • Documents Android Java/R8 and native symbolication as unsupported/deferred with recovery hints.

Refs #699
Refs #694

Touched files: 21.

Validation

Worker validation passed:

  • Focused vitest: 5 files, 127 tests.
  • 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:

  • 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.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1

QR code for preview link

🚀 View preview at
https://callstack.github.io/agent-device/pr-preview/pr-756/

Built to branch gh-pages at 2026-06-11 11:21 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.2 MB 1.2 MB +13.1 kB
JS gzip 392.0 kB 396.7 kB +4.7 kB
npm tarball 510.2 kB 511.1 kB +870 B
npm unpacked 1.7 MB 1.7 MB -4.7 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.6 ms 28.4 ms +0.8 ms
CLI --help 42.9 ms 42.5 ms -0.4 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/9542.js +13.3 kB +4.6 kB
dist/src/2415.js -3.9 kB -1.2 kB
dist/src/args.js +1.1 kB +399 B
dist/src/8173.js +988 B +341 B
dist/src/1352.js +1.1 kB +315 B

@thymikee thymikee force-pushed the codex/debug-symbols branch from a25068d to 073d0cb Compare June 11, 2026 07:10

Copy link
Copy Markdown
Member Author

Code review

Verdict: significant issues — the headline .ips path will fail on real device-produced crash files; the rest of the design is sound.

Findings

  1. 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.

  2. Minorsrc/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.

  3. Minorsrc/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.

  4. Minorsrc/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.

  5. Minorsrc/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.

  6. Minorsrc/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.

  7. Minorsrc/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.

  8. Minorsrc/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).

  9. 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.

  10. Informationalfeat: add Apple xctrace perf profiling #755, feat: add Android native perf profiling #757, and feat: add perf memory diagnostics #759 touch the same shared files (cli-grammar/observability.ts, client-command-metadata.ts, cli-flags.ts, cli-command-overrides.ts, cli-help.ts, client-types.ts, skillgym smoke suite, docs); whichever lands second conflicts, though the additions here are append-style and should rebase mechanically.

Verified clean

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.


Generated by Claude Code

@thymikee

Copy link
Copy Markdown
Member Author

Holding merge readiness on this until real Apple verification is done. Fixture-backed tests and command-construction coverage are useful, but for this workflow we need an actual simulator/device crash artifact plus matching dSYM exercised end-to-end with agent-device debug symbols, or a concrete blocker after attempting it.

Please add validation evidence with the exact commands, simulator/device identity, crash artifact + dSYM provenance, output path, and proof the symbolicated artifact was produced. Until then I no longer consider #756 merge-ready.

@thymikee thymikee force-pushed the codex/debug-symbols branch from 59095db to e03d5c4 Compare June 11, 2026 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant