morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4131] [MERGED] fix(artifacts): never treat a directory named *.map as a source map #6076

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/3909-sourcemap-directory-edge

📝 Commits (1)

0ae520a fix(artifacts): never treat a directory named *.map as a source map

📊 Changes

2 files changed (+47 additions, -9 deletions)

View changed files

📝 src/artifacts/inventory/ArtifactGraphConstruction.test.ts (+37 -0)
📝 src/artifacts/inventory/ArtifactGraphConstruction.ts (+10 -9)

📄 Description

Summary

Fixes #3909. Artifact inventory gave an archive directory named like a source map, such as main.js.map/, a maps-source edge whose parent was the sibling main.js. The same occurrence was correctly classified as a directory container. Edges now use the occurrence's own classification, so the directory is contained by its archive. Real source-map files keep maps-source.

Problem and expected behavior

createArtifactEdges (src/artifacts/inventory/ArtifactGraphConstruction.ts) decided "source map" from logical_path.endsWith(".map"), and relationFor repeated the same suffix rule. Both ignored the producer's parsed fact. createOccurrence classifies a directory entry as artifact_kind: "container" (or framework) with artifact_format: "directory". Only regular files are path-classified as source-map.

Reproduced with the issue's ZIP (main.js plus the empty directory main.js.map/) through inspect-artifact --json, together with a real file source-map control:

ZIP	main	this PR
`main.js` + directory `main.js.map/`	directory `container/directory`, `maps-source`, parent `main.js`, 7,963 bytes, 503 ms	directory `container/directory`, `contains`, parent archive root, 7,960 bytes, 522 ms
`main.js` + file `main.js.map` (control)	`source-map`, `maps-source`, parent `main.js`, 7,934 bytes	unchanged (426–433 ms)

Change and scope

isSourceMap is now occurrence.artifact_kind === "source-map", the classification the inventory producer already owns, including after content digestion. Matching sibling lookup, ambiguity and identity limitations are unchanged for real source maps.
The relation is chosen once: slice-of; maps-source for a source map with a unique identified target, otherwise contains; embeds or contains for everything else.
relationFor no longer handles .map. That duplicate rule is removed.

Contract and boundary impact

Semantic owner and earliest changed stage: inventory edge construction now consumes the occurrence classification owned by createOccurrence and the content digest, instead of re-deriving it from the path.
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: provider-independent artifact inventory.
Evidence, artifact, provenance, or reconstruction contract: no false derivation edge for directories. Directory containment is preserved.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests added or updated: src/artifacts/inventory/ArtifactGraphConstruction.test.ts adds a case with an identified app/main.js file and an identified app/main.js.map directory. It asserts the directory's container/directory classification, a contains edge to its parent, and no limitations. It fails on main and passes with the fix. The existing source-map relationship cases (missing, unread, unique and duplicate targets) still pass.
Real data: the shipped @modelcontextprotocol/server dist/ (34 real .js.map files), zipped with /usr/bin/zip and inventoried through the CLI:

build	occurrences	relations	edges digest	elapsed	output
main	85	`contains` 50, `maps-source` 34	`f97a72ad743bdb00`	500 ms	100,685 bytes
this PR	85	`contains` 50, `maps-source` 34	`f97a72ad743bdb00`	500–576 ms	100,685 bytes

Real source maps and the full edge set are byte-identical, and timing is unchanged.

Remaining proof gaps: none known.

Validation performed

npx vitest run src/artifacts/inventory/ArtifactGraphConstruction.test.ts — 25 passed; against main, 1 failed as expected.
npx vitest run src/artifacts tests/boundary/filesystem/artifactComparison.test.ts tests/boundary/filesystem/artifactDirectoryIdentity.test.ts tests/boundary/mcp/artifactMcp.test.ts tests/boundary/providers/native/artifactProvider.archiveInventory.test.ts src/domain/artifactInventoryEvidence.test.ts — 21 files, 204 passed.
npm run check:fast — passed.
CLI runs on the issue's ZIP, the file control and the real package archive, as tabulated.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: static inventory only.

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/4131 **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/3909-sourcemap-directory-edge` --- ### 📝 Commits (1) - [`0ae520a`](https://github.com/morluto/rea/commit/0ae520afa9ab4ec0e8ce67676adb9ba307c5427e) fix(artifacts): never treat a directory named *.map as a source map ### 📊 Changes **2 files changed** (+47 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `src/artifacts/inventory/ArtifactGraphConstruction.test.ts` (+37 -0) 📝 `src/artifacts/inventory/ArtifactGraphConstruction.ts` (+10 -9) </details> ### 📄 Description ## Summary Fixes #3909. Artifact inventory gave an archive *directory* named like a source map, such as `main.js.map/`, a `maps-source` edge whose parent was the sibling `main.js`. The same occurrence was correctly classified as a directory container. Edges now use the occurrence's own classification, so the directory is contained by its archive. Real source-map files keep `maps-source`. ## Problem and expected behavior `createArtifactEdges` (`src/artifacts/inventory/ArtifactGraphConstruction.ts`) decided "source map" from `logical_path.endsWith(".map")`, and `relationFor` repeated the same suffix rule. Both ignored the producer's parsed fact. `createOccurrence` classifies a directory entry as `artifact_kind: "container"` (or `framework`) with `artifact_format: "directory"`. Only regular files are path-classified as `source-map`. Reproduced with the issue's ZIP (`main.js` plus the empty directory `main.js.map/`) through `inspect-artifact --json`, together with a real file source-map control: | ZIP | main | this PR | |---|---|---| | `main.js` + directory `main.js.map/` | directory `container/directory`, **`maps-source`, parent `main.js`**, 7,963 bytes, 503 ms | directory `container/directory`, **`contains`, parent archive root**, 7,960 bytes, 522 ms | | `main.js` + file `main.js.map` (control) | `source-map`, `maps-source`, parent `main.js`, 7,934 bytes | unchanged (426–433 ms) | ## Change and scope - `isSourceMap` is now `occurrence.artifact_kind === "source-map"`, the classification the inventory producer already owns, including after content digestion. Matching sibling lookup, ambiguity and identity limitations are unchanged for real source maps. - The relation is chosen once: `slice-of`; `maps-source` for a source map with a unique identified target, otherwise `contains`; `embeds` or `contains` for everything else. - `relationFor` no longer handles `.map`. That duplicate rule is removed. ## Contract and boundary impact - Semantic owner and earliest changed stage: inventory edge construction now consumes the occurrence classification owned by `createOccurrence` and the content digest, instead of re-deriving it from the path. - 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: provider-independent artifact inventory. - Evidence, artifact, provenance, or reconstruction contract: no false derivation edge for directories. Directory containment is preserved. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `src/artifacts/inventory/ArtifactGraphConstruction.test.ts` adds a case with an identified `app/main.js` file and an identified `app/main.js.map` directory. It asserts the directory's container/directory classification, a `contains` edge to its parent, and no limitations. It fails on main and passes with the fix. The existing source-map relationship cases (missing, unread, unique and duplicate targets) still pass. - Real data: the shipped `@modelcontextprotocol/server` `dist/` (34 real `.js.map` files), zipped with `/usr/bin/zip` and inventoried through the CLI: | build | occurrences | relations | edges digest | elapsed | output | |---|---|---|---|---|---| | main | 85 | `contains` 50, `maps-source` 34 | `f97a72ad743bdb00` | 500 ms | 100,685 bytes | | this PR | 85 | `contains` 50, `maps-source` 34 | `f97a72ad743bdb00` | 500–576 ms | 100,685 bytes | Real source maps and the full edge set are byte-identical, and timing is unchanged. - Remaining proof gaps: none known. ## Validation performed - `npx vitest run src/artifacts/inventory/ArtifactGraphConstruction.test.ts` — 25 passed; against main, 1 failed as expected. - `npx vitest run src/artifacts tests/boundary/filesystem/artifactComparison.test.ts tests/boundary/filesystem/artifactDirectoryIdentity.test.ts tests/boundary/mcp/artifactMcp.test.ts tests/boundary/providers/native/artifactProvider.archiveInventory.test.ts src/domain/artifactInventoryEvidence.test.ts` — 21 files, 204 passed. - `npm run check:fast` — passed. - CLI runs on the issue's ZIP, the file control and the real package archive, as tabulated. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: static inventory only. ## 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:40 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:46:32 +00:00

[PR #6287] [MERGED] fix(managed): refuse non-PE inputs as unsupported targets #7258

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#6076

No description provided.

Rows
Columns