[PR #7059] [MERGED] fix(apple): keep the recorded image name and frame symbol offset of IPS reports #7613

Closed
opened 2026-11-03 13:39:07 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7059
Author: @Adolanium
Created: 10/25/2026
Status: ✅ Merged
Merged: 10/25/2026
Merged by: @morluto

Base: main ← Head: fix/ips-image-name-symbol-location


📝 Commits (2)

  • 30e90c4 fix(apple): keep the recorded image name and frame symbol offset of IPS reports
  • aba1bc7 fix(apple): count frame symbol offsets in the integer text check

📊 Changes

6 files changed (+94 additions, -2 deletions)

View changed files

📝 docs/apple-ips.md (+3 -1)
📝 src/application/binaryDiagnostics/AppleIpsService.test.ts (+43 -0)
📝 src/domain/native/appleIps.ts (+8 -1)
📝 src/native/apple/AppleIpsJsonShape.ts (+2 -0)
📝 src/native/apple/AppleIpsParser.test.ts (+33 -0)
📝 src/native/apple/AppleIpsParser.ts (+5 -0)

📄 Description

Summary

Fixes #7060.

inspect_recorded_crash drops two documented .ips fields: each binary image's name and each frame's symbolLocation. docs/apple-ips.md says documented body fields are reproduced and undocumented keys stay in unlisted_keys, but neither happens for these two: the parser shapes did not list them, so they were syntax-checked and discarded. A symbolicated frame showed __pthread_kill without its + 8, and every image lost its binary name. Both are now kept.

Problem and expected behavior

Apple's JSON crash report reference lists name ("The binary name") under binary images and symbolLocation ("The number of bytes from the executing instruction's entry point") under frames. ReportCrash writes both in ordinary reports; the existing fixture already contains them. Nothing in the result recorded either value, and no unlisted_keys entry showed they existed.

Expected: images[].name and frames[].symbol_location (a recorded integer, like the other integer fields), with type mismatches recorded the same way as for other fields.

Change and scope

  • src/native/apple/AppleIpsJsonShape.ts: usedImages rows list name; frame rows list symbolLocation.
  • src/native/apple/AppleIpsParser.ts: reads them with the existing read.text / read.integer readers.
  • src/domain/native/appleIps.ts: imageSchema.name: string | null and frameSchema.symbol_location: RecordedInteger | null.
  • docs/apple-ips.md names both fields.

Not included: no other IPS fields; recorded_symbol is unchanged and symbol_location sits next to it.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contribution scope: not applicable. This is a fix inside the existing Apple crash report parser, restoring fields its documentation says it reproduces.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the IPS JSON shapes in AppleIpsJsonShape.ts.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: inspect_recorded_crash output gains images[].name and threads[].frames[].symbol_location, both nullable. Existing fields are unchanged.
  • Provider, bridge, target-format, or platform compatibility: Apple .ips crash reports (bug_type 309).
  • Evidence, artifact, provenance, or reconstruction contract: two recorded producer facts are no longer discarded.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): the generated tool catalog reflects the two output fields; no package change.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated: src/native/apple/AppleIpsParser.test.ts, "keeps the recorded image name and symbol offset": on the existing ReportCrash-shaped fixture, image names are ["rea-crash-fixture", null], the faulting frame keeps symbol_location 12 next to rea_fixture_fault, an inline frame keeps 0, coverage stays complete, and a string symbolLocation gives null with a type mismatch at body.threads[0].frames[0].symbolLocation.
  • Base reproduction or other evidence: with the three source files from upstream/main 4ed33efb0 and the new test, the test fails. Before the new assertions, all existing tests passed with the fields dropped, so nothing covered them.
  • User-visible CLI/MCP output (if applicable): images[].name and frames[].symbol_location in inspect_recorded_crash results.
  • Remaining proof gaps: the fixture is synthetic, modelled on ReportCrash output; no real macOS crash report was parsed.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

On Linux (WSL Ubuntu, Node 24.22.1) at 30e90c435 (on upstream/main 4ed33efb0), after npm ci and npm run build:cached:

  • npx tsc -p . --noEmit passed.
  • On the same change at the previous base f62a2baad: npx oxfmt --check on the changed files clean, node scripts/ci/static-check-report.mjs lint 0 errors, npm run docs:check passed. The rebase onto the current base was clean.
  • npx vitest run src/native/apple src/domain/native tests/boundary/native tests/acceptance/appleIpsCli.test.ts: 555 passed, in two runs.
  • npx vitest run tests/boundary/mcp after npm run mcp-catalog:generate (one earlier run on the previous base): 1991 passed, 10 skipped, 1 failed, "advertises portable NUL escapes and patterns that compile in all JS modes". That test fails the same way on the base without this change.
  • The negative check above failed the new test.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; two nullable output fields are added.
  • Real Hopper/Ghidra, browser, or OS coverage: Linux only; no macOS.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the image name is already part of the recorded image path; no new data source.

Author readiness

Human owner: @Adolanium

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7059 **Author:** [@Adolanium](https://github.com/Adolanium) **Created:** 10/25/2026 **Status:** ✅ Merged **Merged:** 10/25/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/ips-image-name-symbol-location` --- ### 📝 Commits (2) - [`30e90c4`](https://github.com/morluto/rea/commit/30e90c435f4703288f5cb9983eefd0801332167d) fix(apple): keep the recorded image name and frame symbol offset of IPS reports - [`aba1bc7`](https://github.com/morluto/rea/commit/aba1bc735ea6ac3dc91293dc69ff62d3e789e712) fix(apple): count frame symbol offsets in the integer text check ### 📊 Changes **6 files changed** (+94 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `docs/apple-ips.md` (+3 -1) 📝 `src/application/binaryDiagnostics/AppleIpsService.test.ts` (+43 -0) 📝 `src/domain/native/appleIps.ts` (+8 -1) 📝 `src/native/apple/AppleIpsJsonShape.ts` (+2 -0) 📝 `src/native/apple/AppleIpsParser.test.ts` (+33 -0) 📝 `src/native/apple/AppleIpsParser.ts` (+5 -0) </details> ### 📄 Description ## Summary Fixes #7060. `inspect_recorded_crash` drops two documented `.ips` fields: each binary image's `name` and each frame's `symbolLocation`. `docs/apple-ips.md` says documented body fields are reproduced and undocumented keys stay in `unlisted_keys`, but neither happens for these two: the parser shapes did not list them, so they were syntax-checked and discarded. A symbolicated frame showed `__pthread_kill` without its `+ 8`, and every image lost its binary name. Both are now kept. ## Problem and expected behavior Apple's JSON crash report reference lists `name` ("The binary name") under binary images and `symbolLocation` ("The number of bytes from the executing instruction's entry point") under frames. ReportCrash writes both in ordinary reports; the existing fixture already contains them. Nothing in the result recorded either value, and no `unlisted_keys` entry showed they existed. Expected: `images[].name` and `frames[].symbol_location` (a recorded integer, like the other integer fields), with type mismatches recorded the same way as for other fields. ## Change and scope - `src/native/apple/AppleIpsJsonShape.ts`: `usedImages` rows list `name`; frame rows list `symbolLocation`. - `src/native/apple/AppleIpsParser.ts`: reads them with the existing `read.text` / `read.integer` readers. - `src/domain/native/appleIps.ts`: `imageSchema.name: string | null` and `frameSchema.symbol_location: RecordedInteger | null`. - `docs/apple-ips.md` names both fields. Not included: no other IPS fields; `recorded_symbol` is unchanged and `symbol_location` sits next to it. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. Contribution scope: not applicable. This is a fix inside the existing Apple crash report parser, restoring fields its documentation says it reproduces. ## Contract and boundary impact - Semantic owner and earliest changed stage: the IPS JSON shapes in `AppleIpsJsonShape.ts`. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: `inspect_recorded_crash` output gains `images[].name` and `threads[].frames[].symbol_location`, both nullable. Existing fields are unchanged. - Provider, bridge, target-format, or platform compatibility: Apple `.ips` crash reports (`bug_type` 309). - Evidence, artifact, provenance, or reconstruction contract: two recorded producer facts are no longer discarded. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): the generated tool catalog reflects the two output fields; no package change. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/native/apple/AppleIpsParser.test.ts`, "keeps the recorded image name and symbol offset": on the existing ReportCrash-shaped fixture, image names are `["rea-crash-fixture", null]`, the faulting frame keeps `symbol_location` 12 next to `rea_fixture_fault`, an inline frame keeps 0, coverage stays `complete`, and a string `symbolLocation` gives `null` with a type mismatch at `body.threads[0].frames[0].symbolLocation`. - Base reproduction or other evidence: with the three source files from upstream/main 4ed33efb0 and the new test, the test fails. Before the new assertions, all existing tests passed with the fields dropped, so nothing covered them. - User-visible CLI/MCP output (if applicable): `images[].name` and `frames[].symbol_location` in `inspect_recorded_crash` results. - Remaining proof gaps: the fixture is synthetic, modelled on ReportCrash output; no real macOS crash report was parsed. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed On Linux (WSL Ubuntu, Node 24.22.1) at 30e90c435 (on upstream/main 4ed33efb0), after `npm ci` and `npm run build:cached`: - `npx tsc -p . --noEmit` passed. - On the same change at the previous base f62a2baad: `npx oxfmt --check` on the changed files clean, `node scripts/ci/static-check-report.mjs lint` 0 errors, `npm run docs:check` passed. The rebase onto the current base was clean. - `npx vitest run src/native/apple src/domain/native tests/boundary/native tests/acceptance/appleIpsCli.test.ts`: 555 passed, in two runs. - `npx vitest run tests/boundary/mcp` after `npm run mcp-catalog:generate` (one earlier run on the previous base): 1991 passed, 10 skipped, 1 failed, "advertises portable NUL escapes and patterns that compile in all JS modes". That test fails the same way on the base without this change. - The negative check above failed the new test. ## Compatibility, safety, and release - Breaking changes or migration steps: none; two nullable output fields are added. - Real Hopper/Ghidra, browser, or OS coverage: Linux only; no macOS. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the image name is already part of the recorded image path; no new data source. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @Adolanium - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-03 13:39:07 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#7613
No description provided.