[PR #4094] [MERGED] fix(apple): select only entries named Assets.car as asset catalogs #6052

New issue

Closed

opened 2026-10-31 01:11:29 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:29 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4094
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3914-asset-catalog-name-match

📝 Commits (1)

47ab680 fix(apple): select only entries named Assets.car as asset catalogs

📊 Changes

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

View changed files

📝 src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts (+3 -0)
📝 src/artifacts/apple/AppleAssetCatalogAnalysis.ts (+3 -1)

📄 Description

Summary

Fixes #3914. inspect-asset-catalog treated every bundle entry whose relative path ended in Assets.car as a compiled catalog. An unrelated NotAssets.car directory was listed as a skipped catalog. If the app had no real Assets.car, inspection then failed instead of returning the supported empty inventory. Only entries named exactly Assets.car are catalogs now.

Problem and expected behavior

collectAssetCatalogs (src/artifacts/apple/AppleAssetCatalogAnalysis.ts) filtered with entry.path.endsWith("Assets.car"). A suffix match also accepts:

the directory Contents/Resources/NotAssets.car, which is recorded as uninspected and, with no catalog present, makes inspection throw ArtifactReaderFailure("path", …);
a regular file such as OldAssets.car, which would be sent to assetutil as a catalog.

Reproduced with the issue's fixture: an inert /usr/bin/true copy as the executable, an Info.plist, and an empty NotAssets.car directory.

fixture	main	this PR
with `NotAssets.car` directory	exit 1, `artifact_operation_failed`: `Asset catalog entries are not regular files and were not inspected: Contents/Resources/NotAssets.car (directory)`, 564 bytes	exit 0, `catalogs: []`, `total_records: 0`, no-catalog limitation, 2,169 bytes
control (directory removed)	exit 0, empty inventory, 2,162 bytes	unchanged

The 7-byte difference between the two passing outputs is the longer fixture path in the subject.

Change and scope

Select an entry only when its final /-separated path component is exactly Assets.car. DirectoryArtifactReader always reports /-joined relative paths.
Unchanged:
- symlinked or other non-file entries named Assets.car are still disclosed and still block an absence claim;
- nested real catalogs (for example in frameworks) are still inspected.

Contract and boundary impact

Semantic owner and earliest changed stage: asset catalog entry selection in collectAssetCatalogs.
Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged.
Provider, bridge, target-format, or platform compatibility: macOS assetutil path only.
Evidence, artifact, provenance, or reconstruction contract: an app with no catalog now reports the documented empty inventory instead of a false skipped catalog.
Target selection, authority, upstream behavior, and cleanup details: assetutil no longer runs on files that are not named Assets.car.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts. The "app without compiled catalogs" case now also contains a NotAssets.car directory and an OldAssets.car file. It still asserts the empty inventory and that assetutil never runs. The case fails on main (1 failed / 9 passed) and passes with the fix.
Real application: /Applications/KeepingYouAwake.app, which has two real Assets.car catalogs (61 records). Median of 3 CLI runs:

build	elapsed	output	result SHA-256 (prefix)
main	553 ms	57,252 bytes, 2 catalogs, 61 records	`8b6dd01b6e8eea3d`
this PR	537–542 ms	57,252 bytes, 2 catalogs, 61 records	`8b6dd01b6e8eea3d`

Real catalogs are selected identically, and timing is unchanged within noise.

Remaining proof gaps: none known for this selector.

Validation performed

npx vitest run src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts — 10 passed; against main, 1 failed as expected.
npx vitest run src/artifacts/apple src/domain/apple/appleAssetCatalog.test.ts tests/boundary/filesystem/nativeArtifactGoldens.test.ts tests/acceptance/applications/assetCatalogResourceKeys.test.ts — 9 files, 89 passed.
npm run check:fast — passed.
Real CLI runs on the issue's fixture and on KeepingYouAwake, as tabulated.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 with the system assetutil.
Package or release metadata impact: none.
Security, privacy, process, or containment review: assetutil is no longer launched on unrelated *Assets.car files.

Author readiness

Human owner:

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.

🤖 Generated with Claude Code

_{🔄 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/4094 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3914-asset-catalog-name-match` --- ### 📝 Commits (1) - [`47ab680`](https://github.com/morluto/rea/commit/47ab6803ad731d7c17f3116ae6a112aab8c11258) fix(apple): select only entries named Assets.car as asset catalogs ### 📊 Changes **2 files changed** (+6 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts` (+3 -0) 📝 `src/artifacts/apple/AppleAssetCatalogAnalysis.ts` (+3 -1) </details> ### 📄 Description ## Summary Fixes #3914. `inspect-asset-catalog` treated every bundle entry whose relative path *ended* in `Assets.car` as a compiled catalog. An unrelated `NotAssets.car` directory was listed as a skipped catalog. If the app had no real `Assets.car`, inspection then failed instead of returning the supported empty inventory. Only entries named exactly `Assets.car` are catalogs now. ## Problem and expected behavior `collectAssetCatalogs` (`src/artifacts/apple/AppleAssetCatalogAnalysis.ts`) filtered with `entry.path.endsWith("Assets.car")`. A suffix match also accepts: - the directory `Contents/Resources/NotAssets.car`, which is recorded as uninspected and, with no catalog present, makes inspection throw `ArtifactReaderFailure("path", …)`; - a regular file such as `OldAssets.car`, which would be sent to `assetutil` as a catalog. Reproduced with the issue's fixture: an inert `/usr/bin/true` copy as the executable, an `Info.plist`, and an empty `NotAssets.car` directory. | fixture | main | this PR | |---|---|---| | with `NotAssets.car` directory | exit 1, `artifact_operation_failed`: `Asset catalog entries are not regular files and were not inspected: Contents/Resources/NotAssets.car (directory)`, 564 bytes | exit 0, `catalogs: []`, `total_records: 0`, no-catalog limitation, 2,169 bytes | | control (directory removed) | exit 0, empty inventory, 2,162 bytes | unchanged | The 7-byte difference between the two passing outputs is the longer fixture path in the subject. ## Change and scope - Select an entry only when its final `/`-separated path component is exactly `Assets.car`. `DirectoryArtifactReader` always reports `/`-joined relative paths. - Unchanged: - symlinked or other non-file entries named `Assets.car` are still disclosed and still block an absence claim; - nested real catalogs (for example in frameworks) are still inspected. ## Contract and boundary impact - Semantic owner and earliest changed stage: asset catalog entry selection in `collectAssetCatalogs`. - Contribution scope fit and linked maintainer decision: a bug fix within the existing static, read-only boundary; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. - Provider, bridge, target-format, or platform compatibility: macOS `assetutil` path only. - Evidence, artifact, provenance, or reconstruction contract: an app with no catalog now reports the documented empty inventory instead of a false skipped catalog. - Target selection, authority, upstream behavior, and cleanup details: `assetutil` no longer runs on files that are not named `Assets.car`. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts`. The "app without compiled catalogs" case now also contains a `NotAssets.car` directory and an `OldAssets.car` file. It still asserts the empty inventory and that `assetutil` never runs. The case fails on main (1 failed / 9 passed) and passes with the fix. - Real application: `/Applications/KeepingYouAwake.app`, which has two real `Assets.car` catalogs (61 records). Median of 3 CLI runs: | build | elapsed | output | result SHA-256 (prefix) | |---|---|---|---| | main | 553 ms | 57,252 bytes, 2 catalogs, 61 records | `8b6dd01b6e8eea3d` | | this PR | 537–542 ms | 57,252 bytes, 2 catalogs, 61 records | `8b6dd01b6e8eea3d` | Real catalogs are selected identically, and timing is unchanged within noise. - Remaining proof gaps: none known for this selector. ## Validation performed - `npx vitest run src/artifacts/apple/AppleAssetCatalogAnalysis.test.ts` — 10 passed; against main, 1 failed as expected. - `npx vitest run src/artifacts/apple src/domain/apple/appleAssetCatalog.test.ts tests/boundary/filesystem/nativeArtifactGoldens.test.ts tests/acceptance/applications/assetCatalogResourceKeys.test.ts` — 9 files, 89 passed. - `npm run check:fast` — passed. - Real CLI runs on the issue's fixture and on KeepingYouAwake, as tabulated. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: macOS arm64 with the system `assetutil`. - Package or release metadata impact: none. - Security, privacy, process, or containment review: `assetutil` is no longer launched on unrelated `*Assets.car` files. ## Author readiness Human owner:  - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:11:29 +00:00