[PR #4759] [MERGED] fix(javascript): bind response endpoints only from writes that reach the read #6392

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/js-response-endpoint-order

📝 Commits (1)

0772178 fix(javascript): bind response endpoints only from writes that reach the read

📊 Changes

2 files changed (+54 additions, -3 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts (+35 -0)
📝 src/domain/javascript/javascriptSemanticDataEffects.ts (+19 -3)

📄 Description

Summary

Fixes #4606

analyze-javascript-application gave a res.json() response node the exact endpoint of a fetch whose only assignment to res was conditional or came after the read. In those fixtures res is an incoming parameter, so the consumer may read a response the caller passed in, which has nothing to do with that fetch. With this fix those cases report endpoint: null. An unconditional assignment before the read keeps the exact endpoint, and the two-request ambiguity control stays null.

Problem and expected behavior

Smallest trigger:

async function load(res, maybe) { if (maybe) res = await fetch("https://example.invalid/api"); return res.json(); }
async function load(res) { const value = await res.json(); res = await fetch("https://example.invalid/api"); return value; }

Violated invariant: an unresolved identity must remain unknown (AGENTS.md, Architecture and Cleanup). If an initializer is a binding's only write, that is a source fact. It does not prove the write runs before this read (semanticStableInitializer documents the same distinction).

Root cause (source-derived; I confirmed the diff but did not bisect at runtime): 0323ce0a2 (#3822) added soleWrite = binding?.initializers.length === 1 in collectResponseConsumers. Under that rule, a request that is the binding's single write supplies the consumer's endpoint whether or not the write reaches the read. A parameter's incoming value is not counted as an initializer, so a parameter that is overwritten only conditionally, or only after the read, passes this check. Before that commit the endpoint was always null, which is why 6.5.0 returned null for all four fixtures. ab13a1c67 changes only query seed matching and does not produce this endpoint.

Change and scope

The response-consumer producer (src/domain/javascript/javascriptSemanticDataEffects.ts) now binds the endpoint only when the sole write covers the read. It uses the coverage rule the call resolver already applies to incoming values (javascriptSemanticCallResolution.ts): either the write is the binding's stable unconditional declaration initializer (semanticStableInitializer), or the shared read-position owner semanticBindingAtRead reports hasDefiniteInitializer. Otherwise the endpoint stays null. The read-position logic is not duplicated here, and the projection layer does not change.

Not included: the response consumer's resolution and the consumed-by edge are unchanged. The issue says it does not require those to change. A complete resolution with a null endpoint already occurred on main for the "replaced by a non-request value" case.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: domain JavaScript data-effect collection (collectResponseConsumers), the producer of response-consumer endpoints.
Contribution scope fit and linked maintainer decision: not applicable (bug fix inside an existing analysis).
CLI and MCP/tool-catalog contract: no schema change. Some semantic_graph response nodes now report properties.endpoint: null where they used to report an unsupported exact endpoint.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: response endpoint provenance is now null/unknown when the request write does not reach the read. Everything else is unchanged.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

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/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts (next to the #3822 tests, at the same level) has a table test with the issue's four fixtures as written (ordered, parameter-conditional, parameter-later, multiple-requests). It also adds a parameter-replaced-before control, so an unconditional assignment to a parameter before the read still keeps the exact endpoint. That case exercises the semanticBindingAtRead branch rather than the stable-initializer branch.
Base reproduction (executed): I restored origin/main's javascriptSemanticDataEffects.ts and kept the new tests. Result: Tests 2 failed | 6 passed (8). The two failures are parameter-conditional and parameter-later, both returning https://example.invalid/api instead of null. With the fix: 8 passed (8).

User-visible CLI output (executed after the fix, with the issue's harness against this worktree's built scripts/rea.mjs):

{"case": "ordered", "endpoint": "https://example.invalid/api", "consumed_by": ["resolved"], "graph_coverage": "unknown"}
{"case": "parameter-conditional", "endpoint": null, "consumed_by": ["resolved"], "graph_coverage": "unknown"}
{"case": "parameter-later", "endpoint": null, "consumed_by": ["resolved"], "graph_coverage": "unknown"}
{"case": "multiple-requests", "endpoint": null, "consumed_by": ["candidate"], "graph_coverage": "unknown"}

I did not re-run the base-revision CLI output myself. The base rows above under "Problem" come from the issue report.

Remaining proof gaps: MCP transport was not exercised for these fixtures. Only json() was tested; text() and arrayBuffer() share the same code path but have no tests for this case. A read inside a loop or a nested closure, where the receiver is a reassigned parameter or a let rather than a stable declaration, now reports null (conservative). No test pins that.

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:local -- src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts with main's source restored: 2 failed, 6 passed (8). The expected failures are the two parameter fixtures.
npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts src/domain/javascript/javascriptSemanticGraphQuery.test.ts src/domain/javascript/javascriptSemanticAnalysis.native_bindings.test.ts src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts src/domain/javascript/javascriptSemanticCallResolution.test.ts src/domain/javascript/javascriptApplicationGraph.test.ts src/application/javascript/JavaScriptSemanticGraphResources.test.ts: 7 files, 87 passed. Run before and after rebasing onto current main.
npm run test:focused -- tests/acceptance/applications/applicationWorkflowCli.test.ts (includes the ab13a1c67 endpoint-trace CLI/MCP test): 35 passed. Run before the rebase; the rebase touched no JavaScript files.
npx tsc --noEmit -p .: exit 0 (after rebase).
npx oxfmt and npx oxlint on the two changed files: formatted, 0 errors. One pre-existing complexity warning on collectResponseConsumers went from 22 to 21.
Issue reproduction harness through the CLI (python3 -I repro.py scripts/rea.mjs ...): output shown above.

Compatibility, safety, and release

Breaking changes or migration steps: none. Endpoints that the source does not support now come back as null.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. Static JavaScript analysis only, run on Darwin arm64.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new process, network, or file effects. Fixtures are inert and never executed.

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/4759 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/17/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/js-response-endpoint-order` --- ### 📝 Commits (1) - [`0772178`](https://github.com/morluto/rea/commit/0772178777acc0acadc4a8c2a5b7461405cc463f) fix(javascript): bind response endpoints only from writes that reach the read ### 📊 Changes **2 files changed** (+54 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts` (+35 -0) 📝 `src/domain/javascript/javascriptSemanticDataEffects.ts` (+19 -3) </details> ### 📄 Description ## Summary Fixes #4606 `analyze-javascript-application` gave a `res.json()` response node the exact endpoint of a `fetch` whose only assignment to `res` was conditional or came after the read. In those fixtures `res` is an incoming parameter, so the consumer may read a response the caller passed in, which has nothing to do with that fetch. With this fix those cases report `endpoint: null`. An unconditional assignment before the read keeps the exact endpoint, and the two-request ambiguity control stays null. ## Problem and expected behavior Smallest trigger: ```js async function load(res, maybe) { if (maybe) res = await fetch("https://example.invalid/api"); return res.json(); } async function load(res) { const value = await res.json(); res = await fetch("https://example.invalid/api"); return value; } ``` Violated invariant: an unresolved identity must remain unknown (AGENTS.md, Architecture and Cleanup). If an initializer is a binding's only write, that is a source fact. It does not prove the write runs before this read (`semanticStableInitializer` documents the same distinction). Root cause (source-derived; I confirmed the diff but did not bisect at runtime): 0323ce0a2 (#3822) added `soleWrite = binding?.initializers.length === 1` in `collectResponseConsumers`. Under that rule, a request that is the binding's single write supplies the consumer's endpoint whether or not the write reaches the read. A parameter's incoming value is not counted as an initializer, so a parameter that is overwritten only conditionally, or only after the read, passes this check. Before that commit the endpoint was always `null`, which is why 6.5.0 returned null for all four fixtures. ab13a1c67 changes only query seed matching and does not produce this endpoint. ## Change and scope The response-consumer producer (`src/domain/javascript/javascriptSemanticDataEffects.ts`) now binds the endpoint only when the sole write covers the read. It uses the coverage rule the call resolver already applies to incoming values (`javascriptSemanticCallResolution.ts`): either the write is the binding's stable unconditional declaration initializer (`semanticStableInitializer`), or the shared read-position owner `semanticBindingAtRead` reports `hasDefiniteInitializer`. Otherwise the endpoint stays `null`. The read-position logic is not duplicated here, and the projection layer does not change. Not included: the response consumer's `resolution` and the `consumed-by` edge are unchanged. The issue says it does not require those to change. A `complete` resolution with a `null` endpoint already occurred on main for the "replaced by a non-request value" case. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain JavaScript data-effect collection (`collectResponseConsumers`), the producer of `response-consumer` endpoints. - Contribution scope fit and linked maintainer decision: not applicable (bug fix inside an existing analysis). - CLI and MCP/tool-catalog contract: no schema change. Some `semantic_graph` response nodes now report `properties.endpoint: null` where they used to report an unsupported exact endpoint. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: response endpoint provenance is now null/unknown when the request write does not reach the read. Everything else is unchanged. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. 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/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts` (next to the #3822 tests, at the same level) has a table test with the issue's four fixtures as written (ordered, parameter-conditional, parameter-later, multiple-requests). It also adds a `parameter-replaced-before` control, so an unconditional assignment to a parameter before the read still keeps the exact endpoint. That case exercises the `semanticBindingAtRead` branch rather than the stable-initializer branch. - Base reproduction (executed): I restored `origin/main`'s `javascriptSemanticDataEffects.ts` and kept the new tests. Result: `Tests 2 failed | 6 passed (8)`. The two failures are `parameter-conditional` and `parameter-later`, both returning `https://example.invalid/api` instead of `null`. With the fix: `8 passed (8)`. - User-visible CLI output (executed after the fix, with the issue's harness against this worktree's built `scripts/rea.mjs`): ```jsonl {"case": "ordered", "endpoint": "https://example.invalid/api", "consumed_by": ["resolved"], "graph_coverage": "unknown"} {"case": "parameter-conditional", "endpoint": null, "consumed_by": ["resolved"], "graph_coverage": "unknown"} {"case": "parameter-later", "endpoint": null, "consumed_by": ["resolved"], "graph_coverage": "unknown"} {"case": "multiple-requests", "endpoint": null, "consumed_by": ["candidate"], "graph_coverage": "unknown"} ``` I did not re-run the base-revision CLI output myself. The base rows above under "Problem" come from the issue report. - Remaining proof gaps: MCP transport was not exercised for these fixtures. Only `json()` was tested; `text()` and `arrayBuffer()` share the same code path but have no tests for this case. A read inside a loop or a nested closure, where the receiver is a reassigned parameter or a `let` rather than a stable declaration, now reports `null` (conservative). No test pins that. 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:local -- src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts` with main's source restored: 2 failed, 6 passed (8). The expected failures are the two parameter fixtures. - `npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.response_endpoints.test.ts src/domain/javascript/javascriptSemanticGraphQuery.test.ts src/domain/javascript/javascriptSemanticAnalysis.native_bindings.test.ts src/domain/javascript/javascriptSemanticAnalysis.runtime_boundaries.test.ts src/domain/javascript/javascriptSemanticCallResolution.test.ts src/domain/javascript/javascriptApplicationGraph.test.ts src/application/javascript/JavaScriptSemanticGraphResources.test.ts`: 7 files, 87 passed. Run before and after rebasing onto current `main`. - `npm run test:focused -- tests/acceptance/applications/applicationWorkflowCli.test.ts` (includes the ab13a1c67 endpoint-trace CLI/MCP test): 35 passed. Run before the rebase; the rebase touched no JavaScript files. - `npx tsc --noEmit -p .`: exit 0 (after rebase). - `npx oxfmt` and `npx oxlint` on the two changed files: formatted, 0 errors. One pre-existing `complexity` warning on `collectResponseConsumers` went from 22 to 21. - Issue reproduction harness through the CLI (`python3 -I repro.py scripts/rea.mjs ...`): output shown above. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Endpoints that the source does not support now come back as `null`. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. Static JavaScript analysis only, run on Darwin arm64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, network, or file effects. Fixtures are inert and never executed. ## 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:14:08 +00:00