morluto/rea

Fork 0

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

[PR #839] [MERGED] fix(javascript): analyze sources containing empty string literals #2277

New issue

Closed

opened 2026-10-16 21:16:38 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:16:38 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/empty-literal-graph-labels

📝 Commits (4)

f7954a8 fix(javascript): keep exact empty env keys and method names
ad1f6b5 fix(javascript): keep exact empty require paths and request fields
11fc3b9 fix(javascript): analyze sources containing empty string literals
58ef8ac fix(javascript): analyze source maps with empty source names

📊 Changes

9 files changed (+285 additions, -18 deletions)

View changed files

➕ src/application/JavaScriptApplicationEmptyLiterals.test.ts (+105 -0)
📝 src/application/JavaScriptArtifactGraphAccumulator.ts (+49 -2)
📝 src/application/JavaScriptSemanticGraphBuilder.test.ts (+32 -0)
📝 src/application/JavaScriptSemanticGraphConstruction.ts (+2 -1)
📝 src/application/JavaScriptSemanticGraphValueProjection.ts (+1 -1)
📝 src/domain/javascriptAstValues.ts (+8 -0)
📝 src/domain/javascriptSemanticAnalysis.empty_keys.test.ts (+72 -0)
📝 src/domain/javascriptSemanticDataEffects.ts (+8 -4)
📝 src/domain/javascriptSemanticProjection.ts (+8 -10)

📄 Description

Builds on #836. This branch contains #836's two commits, followed by one commit for this PR. It targets main so CI runs. Merge #836 first, or review only the last commit (fix(javascript): analyze sources containing empty string literals).

Summary

Addresses the unreadable_output failure in #746: the root-cause comment traced Obsidian’s /identity/key (too_small) rejection to empty literals. All nine of its minimal repros (including ipcRenderer.send("") and new BrowserWindow().loadURL("")) are the failure class fixed here. #746’s separate request for progress output during long silent runs is not addressed, so this PR uses Refs, not Fixes.

Refs #746

On main, one legal empty string literal anywhere in an analyzed application makes the entire analyze_javascript_application / rea analyze-javascript-application run fail with a generic error:

{ "code": "unreadable_output", "details": { "operation": "analyze_javascript_application",
  "reason": "Result schema rejected 2 issues at /identity/key (too_small)" } }

The literal becomes an application-graph observation label, an artifact-local-key identity key, or a semantic node label — all z.string().min(1). A minified bundle only needs one fetch("") or on("", …) to lose all analysis.

Reproduced on origin/main (75f50d86) through the public service — each line below is a whole app.js:

Source	Rejected field(s)
`fetch('')`, `new WebSocket('')`	`identity/key`, `observations/0/label`
`new Worker('')`, `navigator.serviceWorker.register('')`	`identity/key`, `observations/0/label`
`localStorage.getItem('')`, `localStorage.setItem('', 1)`, `indexedDB.open('')`	`identity/key`, `observations/0/label`
`win.loadURL('')`, `app.get('', h)`	`identity/key`, `observations/0/label`
`require('')`, `import y from ''`, `import('')`, `win.loadFile('')`	`observations/0/label`
`ipcMain.handle('', f)`, `ipcRenderer.invoke('')`, `contextBridge.exposeInMainWorld('', {})`	`observations/0/label`
`process.on('', f)`, `emitter.on('', f)`, `child.on('', f)`	semantic `label`
`app.js.map` with `"sources": [""]` (plain or indexed map)	`identity/original_source`

Fix

Normalize display at the two graph construction seams rather than at ~30 individual producers:

JavaScriptArtifactGraphAccumulator.addNode — every application-graph producer already goes through it. Empty observation labels, artifact-local-key keys, and source-map-original original_source names are displayed as "". Source-map sources entries of "" are legal relative references; some bundlers emit them for virtual or generated modules.
constructSemanticGraphNode — every semantic node goes through it. Empty labels are displayed as "".

Both use one domain helper, javascriptDisplayText (in javascriptAstValues.ts), which applies the "" display convention from #764/#836. That makes #836's per-site semanticNameLabel calls redundant, so this PR removes them.

addNode now takes a structural DisplayableNodeInput (only identity.strategy/key and observations[].label) instead of unknown. It remains generic, so producers' readonly property types still work, and createJavaScriptApplicationNode still parses the full input.

Exact values are preserved

The display form appears only in labels and identity keys. Every affected node keeps the exact empty value in its properties, which I checked on the built service output. Examples: endpoint properties.value: "", storage name: "", worker declared_specifier: "", module reference specifier: "", IPC channel: "", renderer declared_path: "", source module source: "", semantic event event_name: "", and request endpoint: "".

Trade-off considered

Alternatively, identity.key could be relaxed from min(1) to admit "". That would change the advertised MCP output schema and the catalog, skill, and conformance-manifest digests, which conflicts with concurrent PRs. Keeping the schemas means an artifact-local-key for the empty value ("") could share an identity with a literal value of two quote characters in the same artifact and namespace. If that contrived case happened, the node would merge, but each observation would still keep its exact properties value, so no evidence is lost.

Contract / behavior changes

No schema or catalog changes. Inputs that used to fail now succeed. Output for inputs without empty literals is unchanged: the display mapping is the identity function for nonempty strings.

Verification

New src/application/JavaScriptApplicationEmptyLiterals.test.ts: 19 empty-literal sources through the public analyzeJavaScriptApplication service, an exact-value test that compares endpoint and event display against fetch('/api'), and two source-map cases (plain and indexed). All 22 fail without the source changes and pass with them.
npm run check:changed: 39 files / 737 tests passed.
npm run check:fast: passed.
I probed 26 one-line sources against built dist/ on origin/main and on this branch: 20 failures on main, 0 here.

No Hopper/Ghidra/IDA verification needed (static JavaScript analysis only).

🤖 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/839 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/empty-literal-graph-labels` --- ### 📝 Commits (4) - [`f7954a8`](https://github.com/morluto/rea/commit/f7954a8fd692f262ef3ce1507b957681150ce9c4) fix(javascript): keep exact empty env keys and method names - [`ad1f6b5`](https://github.com/morluto/rea/commit/ad1f6b508fbdd44d8bf1884ed96ae69082808e7d) fix(javascript): keep exact empty require paths and request fields - [`11fc3b9`](https://github.com/morluto/rea/commit/11fc3b981509e8107b10b2e75363f5bb489879db) fix(javascript): analyze sources containing empty string literals - [`58ef8ac`](https://github.com/morluto/rea/commit/58ef8ac3aa803d7b64dd12a5b2a2e8ec24261060) fix(javascript): analyze source maps with empty source names ### 📊 Changes **9 files changed** (+285 additions, -18 deletions) <details> <summary>View changed files</summary> ➕ `src/application/JavaScriptApplicationEmptyLiterals.test.ts` (+105 -0) 📝 `src/application/JavaScriptArtifactGraphAccumulator.ts` (+49 -2) 📝 `src/application/JavaScriptSemanticGraphBuilder.test.ts` (+32 -0) 📝 `src/application/JavaScriptSemanticGraphConstruction.ts` (+2 -1) 📝 `src/application/JavaScriptSemanticGraphValueProjection.ts` (+1 -1) 📝 `src/domain/javascriptAstValues.ts` (+8 -0) 📝 `src/domain/javascriptSemanticAnalysis.empty_keys.test.ts` (+72 -0) 📝 `src/domain/javascriptSemanticDataEffects.ts` (+8 -4) 📝 `src/domain/javascriptSemanticProjection.ts` (+8 -10) </details> ### 📄 Description > **Builds on #836.** This branch contains #836's two commits, followed by one commit for this PR. It targets `main` so CI runs. Merge #836 first, or review only the last commit (`fix(javascript): analyze sources containing empty string literals`). ## Summary Addresses the `unreadable_output` failure in #746: [the root-cause comment](https://github.com/morluto/rea/issues/746) traced Obsidian’s `/identity/key (too_small)` rejection to empty literals. All nine of its minimal repros (including `ipcRenderer.send("")` and `new BrowserWindow().loadURL("")`) are the failure class fixed here. #746’s separate request for progress output during long silent runs is not addressed, so this PR uses `Refs`, not `Fixes`. Refs #746 On `main`, one legal empty string literal anywhere in an analyzed application makes **the entire** `analyze_javascript_application` / `rea analyze-javascript-application` run fail with a generic error: ```json { "code": "unreadable_output", "details": { "operation": "analyze_javascript_application", "reason": "Result schema rejected 2 issues at /identity/key (too_small)" } } ``` The literal becomes an application-graph observation `label`, an `artifact-local-key` identity `key`, or a semantic node `label` — all `z.string().min(1)`. A minified bundle only needs one `fetch("")` or `on("", …)` to lose all analysis. Reproduced on `origin/main` (75f50d86) through the public service — each line below is a whole `app.js`: | Source | Rejected field(s) | |---|---| | `fetch('')`, `new WebSocket('')` | `identity/key`, `observations/0/label` | | `new Worker('')`, `navigator.serviceWorker.register('')` | `identity/key`, `observations/0/label` | | `localStorage.getItem('')`, `localStorage.setItem('', 1)`, `indexedDB.open('')` | `identity/key`, `observations/0/label` | | `win.loadURL('')`, `app.get('', h)` | `identity/key`, `observations/0/label` | | `require('')`, `import y from ''`, `import('')`, `win.loadFile('')` | `observations/0/label` | | `ipcMain.handle('', f)`, `ipcRenderer.invoke('')`, `contextBridge.exposeInMainWorld('', {})` | `observations/0/label` | | `process.on('', f)`, `emitter.on('', f)`, `child.on('', f)` | semantic `label` | | `app.js.map` with `"sources": [""]` (plain or indexed map) | `identity/original_source` | ## Fix Normalize display at the two graph construction seams rather than at ~30 individual producers: - `JavaScriptArtifactGraphAccumulator.addNode` — every application-graph producer already goes through it. Empty observation labels, `artifact-local-key` keys, and `source-map-original` `original_source` names are displayed as `""`. Source-map `sources` entries of `""` are legal relative references; some bundlers emit them for virtual or generated modules. - `constructSemanticGraphNode` — every semantic node goes through it. Empty labels are displayed as `""`. Both use one domain helper, `javascriptDisplayText` (in `javascriptAstValues.ts`), which applies the `""` display convention from #764/#836. That makes #836's per-site `semanticNameLabel` calls redundant, so this PR removes them. `addNode` now takes a structural `DisplayableNodeInput` (only `identity.strategy`/`key` and `observations[].label`) instead of `unknown`. It remains generic, so producers' readonly property types still work, and `createJavaScriptApplicationNode` still parses the full input. ### Exact values are preserved The display form appears only in labels and identity keys. Every affected node keeps the exact empty value in its properties, which I checked on the built service output. Examples: endpoint `properties.value: ""`, storage `name: ""`, worker `declared_specifier: ""`, module reference `specifier: ""`, IPC `channel: ""`, renderer `declared_path: ""`, source module `source: ""`, semantic event `event_name: ""`, and request `endpoint: ""`. ### Trade-off considered Alternatively, `identity.key` could be relaxed from `min(1)` to admit `""`. That would change the advertised MCP output schema and the catalog, skill, and conformance-manifest digests, which conflicts with concurrent PRs. Keeping the schemas means an `artifact-local-key` for the empty value (`""`) could share an identity with a literal value of two quote characters in the same artifact and namespace. If that contrived case happened, the node would merge, but each observation would still keep its exact `properties` value, so no evidence is lost. ## Contract / behavior changes No schema or catalog changes. Inputs that used to fail now succeed. Output for inputs without empty literals is unchanged: the display mapping is the identity function for nonempty strings. ## Verification - New `src/application/JavaScriptApplicationEmptyLiterals.test.ts`: 19 empty-literal sources through the public `analyzeJavaScriptApplication` service, an exact-value test that compares endpoint and event display against `fetch('/api')`, and two source-map cases (plain and indexed). **All 22 fail without the source changes and pass with them.** - `npm run check:changed`: 39 files / 737 tests passed. - `npm run check:fast`: passed. - I probed 26 one-line sources against built `dist/` on `origin/main` and on this branch: 20 failures on `main`, 0 here. No Hopper/Ghidra/IDA verification needed (static JavaScript analysis only). 🤖 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-16 21:16:38 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:04:14 +00:00

[PR #2277] [MERGED] feat(browser): inspect retained aioquic qlog JSON captures #5133

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

No description provided.

Rows
Columns