morluto/rea

Fork 0

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

[PR #3853] [MERGED] fix(managed): project metadata with empty names without a display label #5949

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/managed-empty-name-label

📝 Commits (1)

04981eb fix(managed): project metadata with empty names without a display label

📊 Changes

2 files changed (+61 additions, -7 deletions)

View changed files

📝 src/domain/managed/managedApplicationGraphNodes.ts (+16 -7)
📝 tests/composition/analysis-sessions/managedApplicationGraph.test.ts (+45 -0)

📄 Description

Summary

Fixes #3352. inspect-managed-members returns complete, authenticated Evidence for a PE/CLI Module row whose name is the empty string. Passing that exact Evidence to project-managed-application-graph failed with exit 1: invalid_request at observations.0.label. No graph was returned, so the readable type, method and field observations were lost.

Now the projection succeeds. The module node's observation has label: null, and properties.name keeps the source value "".

Problem and expected behavior

managedApplicationGraphNodes.ts copied metadata names directly into graph observation labels. The shared application-graph schema allows label to be a non-empty string or null. The managed inspector schema admits any string (z.string()) for these names. An empty metadata name (heap index 0) is therefore valid inspector Evidence, but it was an invalid graph label, so a display constraint rejected an identity fact.

Change and scope

The managed graph node builder, src/domain/managed/managedApplicationGraphNodes.ts, gains metadataLabel(name), which maps "" to null. It is applied to every label derived from a metadata name, all of which the inspector admits as empty:
- assembly and module names
- type full name
- method and field labels (bare names when there is no declaring type)
- P/Invoke import name
- native implementation name
Each node's properties keeps the source values unchanged.
Not included: the artifact node label (basename of the path, never empty) and JavaScript graph labels.

Contract and boundary impact

Semantic owner and earliest changed stage: managed application graph node construction (display labels).
Contribution scope fit and linked maintainer decision: correctness fix within existing read-only projection; not applicable.
CLI and MCP/tool-catalog contract: schemas unchanged. label was already nullable, and a previously rejected input now projects.
Provider, bridge, target-format, or platform compatibility: unchanged.
Evidence, artifact, provenance, or reconstruction contract: identity values stay in properties; only the display label becomes null.
Target selection, authority, upstream behavior, and cleanup details: unchanged.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added: tests/composition/analysis-sessions/managedApplicationGraph.test.ts builds the repository PE fixture and points the Module row's Name at the empty string-heap entry, as the issue does. It inspects the result with the production inspectManagedMembersBytes and projects that Evidence. It asserts the module observation { label: null, properties: { name: "" } } and that the type, method and field nodes are retained.
Base reproduction: against the unfixed module, the new test fails (1 failed | 17 passed).
User-visible CLI output for the issue's reproduction script, after:
```
control projection 0  nodes 5
inspection {"exit":0,"module_name":"","coverage":{"state":"complete","issues":[]}}
projection 0  nodes=5
```
Before: projection 1 {"code":"invalid_request",...,"message":"A nested value failed validation at observations.0.label: Too small: expected string to have >=1 characters"}.
Remaining proof gaps: there was no real managed assembly with empty metadata names to test against; the fixture is built with the repository's PE builder and read by the production inspector.

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

npm run test:focused -- tests/boundary/cli/jsonInputStatus.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts: 51 passed
npm run check:fast, npm run knip: passed
The issue's CLI reproduction (built CLI on macOS, Darwin 25.6 arm64): the control and empty-name projections both exit 0 with 5 nodes, at about 0.4 s each, the same as the control before the change.

Compatibility, safety, and release

Breaking changes or migration steps: none. Labels for non-empty names are unchanged.
Real Hopper/Ghidra, browser, or OS coverage: not applicable.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none.

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/3853 **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/managed-empty-name-label` --- ### 📝 Commits (1) - [`04981eb`](https://github.com/morluto/rea/commit/04981eb839146b6cb3bd607140ff2085ff46d261) fix(managed): project metadata with empty names without a display label ### 📊 Changes **2 files changed** (+61 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/managed/managedApplicationGraphNodes.ts` (+16 -7) 📝 `tests/composition/analysis-sessions/managedApplicationGraph.test.ts` (+45 -0) </details> ### 📄 Description ## Summary Fixes #3352. `inspect-managed-members` returns complete, authenticated Evidence for a PE/CLI Module row whose name is the empty string. Passing that exact Evidence to `project-managed-application-graph` failed with exit `1`: `invalid_request` at `observations.0.label`. No graph was returned, so the readable type, method and field observations were lost. Now the projection succeeds. The module node's observation has `label: null`, and `properties.name` keeps the source value `""`. ## Problem and expected behavior `managedApplicationGraphNodes.ts` copied metadata names directly into graph observation labels. The shared application-graph schema allows `label` to be a non-empty string or `null`. The managed inspector schema admits any string (`z.string()`) for these names. An empty metadata name (heap index 0) is therefore valid inspector Evidence, but it was an invalid graph label, so a display constraint rejected an identity fact. ## Change and scope - The managed graph node builder, `src/domain/managed/managedApplicationGraphNodes.ts`, gains `metadataLabel(name)`, which maps `""` to `null`. It is applied to every label derived from a metadata name, all of which the inspector admits as empty: - assembly and module names - type full name - method and field labels (bare names when there is no declaring type) - P/Invoke import name - native implementation name - Each node's `properties` keeps the source values unchanged. - Not included: the artifact node label (`basename` of the path, never empty) and JavaScript graph labels. ## Contract and boundary impact - Semantic owner and earliest changed stage: managed application graph node construction (display labels). - Contribution scope fit and linked maintainer decision: correctness fix within existing read-only projection; not applicable. - CLI and MCP/tool-catalog contract: schemas unchanged. `label` was already nullable, and a previously rejected input now projects. - Provider, bridge, target-format, or platform compatibility: unchanged. - Evidence, artifact, provenance, or reconstruction contract: identity values stay in `properties`; only the display label becomes null. - Target selection, authority, upstream behavior, and cleanup details: unchanged. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: `tests/composition/analysis-sessions/managedApplicationGraph.test.ts` builds the repository PE fixture and points the Module row's Name at the empty string-heap entry, as the issue does. It inspects the result with the production `inspectManagedMembersBytes` and projects that Evidence. It asserts the module observation `{ label: null, properties: { name: "" } }` and that the type, method and field nodes are retained. - Base reproduction: against the unfixed module, the new test fails (`1 failed | 17 passed`). - User-visible CLI output for the issue's reproduction script, after: ``` control projection 0 nodes 5 inspection {"exit":0,"module_name":"","coverage":{"state":"complete","issues":[]}} projection 0 nodes=5 ``` Before: `projection 1 {"code":"invalid_request",...,"message":"A nested value failed validation at observations.0.label: Too small: expected string to have >=1 characters"}`. - Remaining proof gaps: there was no real managed assembly with empty metadata names to test against; the fixture is built with the repository's PE builder and read by the production inspector. 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 - `npm run test:focused -- tests/boundary/cli/jsonInputStatus.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts`: 51 passed - `npm run check:fast`, `npm run knip`: passed - The issue's CLI reproduction (built CLI on macOS, Darwin 25.6 arm64): the control and empty-name projections both exit `0` with 5 nodes, at about 0.4 s each, the same as the control before the change. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Labels for non-empty names are unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## 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 - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] 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:10:40 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #6371] [MERGED] fix(native): report plist symlink loops and overlong paths as selection failures #7315

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

No description provided.

Rows
Columns