morluto/rea

Fork 0

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

[PR #3846] [CLOSED] fix(javascript): bound getter reference expansion #5943

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3846
Author: @hannnnnnnny
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: codex/fix-getter-expansion-3839

📝 Commits (2)

44aa86d fix(javascript): append unbounded object references without argument spreading
05bc96f fix(javascript): bound getter reference expansion and report unknown coverage

📊 Changes

12 files changed (+285 additions, -27 deletions)

View changed files

📝 src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts (+14 -0)
➕ src/domain/javascript/javascriptSemanticAnalysis.getter_expansion.test.ts (+94 -0)
📝 src/domain/javascript/javascriptSemanticAnalysis.ts (+25 -2)
📝 src/domain/javascript/javascriptSemanticCoverage.ts (+5 -0)
📝 src/domain/javascript/javascriptSemanticObjectReferences.test.ts (+27 -1)
📝 src/domain/javascript/javascriptSemanticObjectReferences.ts (+49 -21)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+18 -2)
📝 src/domain/javascript/javascriptSemanticResourceLimits.ts (+12 -0)
📝 src/domain/javascript/javascriptSemanticValueTypes.ts (+2 -1)
📝 src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts (+25 -0)
📝 src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts (+1 -0)
➕ tests/fixtures/javascriptGetterExpansion.ts (+13 -0)

📄 Description

Summary

Unknown computed receiver keys can cause class getters to enumerate permutations of other getters, overflowing argument lists and exhausting memory. Bound getter expansion per property-reference query and report explicit unknown coverage when the bound is reached. Refs #3839.

property-reference query
  shared getter visit/result budget
    exhausted -> partial IR, lexical facts retained, values/provenance unknown

Problem and expected behavior

A synthetic class with ten getters returning this[key].fieldN reproduced RangeError: Maximum call stack size exceeded at the reported getter-result append after 8.972 seconds. Per-chain cycle guards do not bound permutations across different getters. Expansion should terminate within a declared resource bound, and incomplete effect collection must not establish trusted values or absence claims.

Change and scope

Replace unbounded argument-spreading appends with loops. Share a budget of 256 getter visits and retained results across recursive and sibling getter expansion in one property-reference query. On exhaustion, retain lexical scopes, binding identities, callable identities, and references; mark binding values and provenance unknown and omit module links and derived relations because member effects are incomplete. Preserve the resource classification through worker transfer and semantic graph coverage.

This is an ordinary static-analysis bug fix within existing contribution boundaries. A separate maintainer scope decision is not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: domain property-reference expansion during member mutation collection.
Contribution scope fit and linked maintainer decision: existing inert JavaScript analysis; no new provider or execution capability.
CLI and MCP/tool-catalog contract: no command, parameter, or tool-catalog changes.
Provider, bridge, target-format, or platform compatibility: add the getter-expansion classification to the internal worker schema. Runtime verified on Windows/Node v24.14.1; macOS/Node 26.5 remains unverified.
Evidence, artifact, provenance, or reconstruction contract: budget exhaustion is explicitly partial/unknown; incomplete effects do not produce value or module-provenance claims.
Target selection, authority, upstream behavior, and cleanup details: existing target selection and worker lifecycle; no new target execution or mutation authority.
Generated metadata, package, or installation impact: no authored package metadata, dependency, installation, or generated catalog changes.

Expected effects:

Read existing files or captured data (existing inert source analysis).
Launch or attach to a process (existing analyzer worker and verification processes only).
Inject instrumentation or run scripts/code in the analyzed target.
Change target process memory, registers, files, or configuration.
Contact network services as part of source analysis.
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: dynamic instance/static/inherited getter permutations, a deep acyclic getter chain, independent query budgets, lexical-fact retention and unknown values/provenance, worker serialization, graph coverage, and a 600,000-field reference list.
Before: ten-getter regression threw the reported RangeError; the field-capacity test independently failed at the unbounded instance-field append.
After: ten-getter regression returned partial coverage in 25 ms on its first post-fix Vitest run. These timings are synthetic observations, not a benchmark of the shipped bundle.
Compiled analyzer, normal Node process: 30 dynamic getters returned limited coverage in 29 ms; a subsequent 600-getter chain returned limited coverage in 73 ms.
User-visible CLI/MCP output: full CLI verification was attempted, but worker startup was blocked by missing packaged Windows native controls (native/windows/build/manifest.json and rea-windows-x64.node). No CLI/MCP success is claimed.
Remaining proof gaps: the original Claude macOS bundle was not attached to the issue and was unavailable for verification. Node v24.18.0 pinned by the repository and the reported Node 26.5/macOS environment were not tested.
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 check — passed all six static tasks and architecture guards across 1,408 source files.
After rebasing onto 788964d (including #3836 and #3831), npm run test:run -- src/domain/javascript/javascriptSemantic src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts — 982 tests across 38 files passed. Both upstream exact-key lookup coverage and the new capacity regression were retained when resolving the test-file conflict.
npm run typecheck — passed.
npm run lint (run by npm run check after rebasing) — zero errors, 1,444 warnings; module boundaries passed for 1,911 source files.
npm run format:check — passed.
npm run verify:test-discovery — all 1,313 test files discovered exactly once.
npm run compile — passed with two ineffective dynamic-import warnings.
git diff --check — passed before committing.
Independent automated read-only review — no actionable findings; reviewer separately ran 160 focused tests. This does not establish human review.

Compatibility, safety, and release

Breaking changes or migration steps: no caller migration. Queries exceeding the getter budget now produce explicit partial/unknown coverage; all value-derived facts in that file are conservatively omitted.
Real Hopper/Ghidra, browser, or OS coverage: no engine integration changes; source analysis checked on Windows. Original macOS bundle and complete worker launch remain unverified as explained above.
Package or release metadata impact: none.
Security, privacy, process, or containment review: analyzed JavaScript remains inert. The budget bounds combinatorial getter expansion before further getter/result allocation. The change can be reverted without data migration; the behavioral tradeoff is reduced semantic precision on queries exceeding the bound.

Author readiness

Human owner: @hannnnnnnny; human review and readiness are pending.

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.

_{🔄 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/3846 **Author:** [@hannnnnnnny](https://github.com/hannnnnnnny) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `codex/fix-getter-expansion-3839` --- ### 📝 Commits (2) - [`44aa86d`](https://github.com/morluto/rea/commit/44aa86d6fe376608bbb412d2f40ddaa73621d616) fix(javascript): append unbounded object references without argument spreading - [`05bc96f`](https://github.com/morluto/rea/commit/05bc96f24e3de7fb86e5f4b0a13e509d83b275a6) fix(javascript): bound getter reference expansion and report unknown coverage ### 📊 Changes **12 files changed** (+285 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts` (+14 -0) ➕ `src/domain/javascript/javascriptSemanticAnalysis.getter_expansion.test.ts` (+94 -0) 📝 `src/domain/javascript/javascriptSemanticAnalysis.ts` (+25 -2) 📝 `src/domain/javascript/javascriptSemanticCoverage.ts` (+5 -0) 📝 `src/domain/javascript/javascriptSemanticObjectReferences.test.ts` (+27 -1) 📝 `src/domain/javascript/javascriptSemanticObjectReferences.ts` (+49 -21) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+18 -2) 📝 `src/domain/javascript/javascriptSemanticResourceLimits.ts` (+12 -0) 📝 `src/domain/javascript/javascriptSemanticValueTypes.ts` (+2 -1) 📝 `src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts` (+25 -0) 📝 `src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts` (+1 -0) ➕ `tests/fixtures/javascriptGetterExpansion.ts` (+13 -0) </details> ### 📄 Description ## Summary Unknown computed receiver keys can cause class getters to enumerate permutations of other getters, overflowing argument lists and exhausting memory. Bound getter expansion per property-reference query and report explicit unknown coverage when the bound is reached. Refs #3839. ```text property-reference query shared getter visit/result budget exhausted -> partial IR, lexical facts retained, values/provenance unknown ``` ## Problem and expected behavior A synthetic class with ten getters returning `this[key].fieldN` reproduced `RangeError: Maximum call stack size exceeded` at the reported getter-result append after 8.972 seconds. Per-chain cycle guards do not bound permutations across different getters. Expansion should terminate within a declared resource bound, and incomplete effect collection must not establish trusted values or absence claims. ## Change and scope Replace unbounded argument-spreading appends with loops. Share a budget of 256 getter visits and retained results across recursive and sibling getter expansion in one property-reference query. On exhaustion, retain lexical scopes, binding identities, callable identities, and references; mark binding values and provenance unknown and omit module links and derived relations because member effects are incomplete. Preserve the resource classification through worker transfer and semantic graph coverage. This is an ordinary static-analysis bug fix within existing contribution boundaries. A separate maintainer scope decision is not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain property-reference expansion during member mutation collection. - Contribution scope fit and linked maintainer decision: existing inert JavaScript analysis; no new provider or execution capability. - CLI and MCP/tool-catalog contract: no command, parameter, or tool-catalog changes. - Provider, bridge, target-format, or platform compatibility: add the `getter-expansion` classification to the internal worker schema. Runtime verified on Windows/Node v24.14.1; macOS/Node 26.5 remains unverified. - Evidence, artifact, provenance, or reconstruction contract: budget exhaustion is explicitly partial/unknown; incomplete effects do not produce value or module-provenance claims. - Target selection, authority, upstream behavior, and cleanup details: existing target selection and worker lifecycle; no new target execution or mutation authority. - Generated metadata, package, or installation impact: no authored package metadata, dependency, installation, or generated catalog changes. Expected effects: - [x] Read existing files or captured data (existing inert source analysis). - [x] Launch or attach to a process (existing analyzer worker and verification processes only). - [ ] Inject instrumentation or run scripts/code in the analyzed target. - [ ] Change target process memory, registers, files, or configuration. - [ ] Contact network services as part of source analysis. - [ ] 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: dynamic instance/static/inherited getter permutations, a deep acyclic getter chain, independent query budgets, lexical-fact retention and unknown values/provenance, worker serialization, graph coverage, and a 600,000-field reference list. - Before: ten-getter regression threw the reported `RangeError`; the field-capacity test independently failed at the unbounded instance-field append. - After: ten-getter regression returned partial coverage in 25 ms on its first post-fix Vitest run. These timings are synthetic observations, not a benchmark of the shipped bundle. - Compiled analyzer, normal Node process: 30 dynamic getters returned limited coverage in 29 ms; a subsequent 600-getter chain returned limited coverage in 73 ms. - User-visible CLI/MCP output: full CLI verification was attempted, but worker startup was blocked by missing packaged Windows native controls (`native/windows/build/manifest.json` and `rea-windows-x64.node`). No CLI/MCP success is claimed. - Remaining proof gaps: the original Claude macOS bundle was not attached to the issue and was unavailable for verification. Node v24.18.0 pinned by the repository and the reported Node 26.5/macOS environment were not tested. - [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 check` — passed all six static tasks and architecture guards across 1,408 source files. - After rebasing onto `788964d` (including #3836 and #3831), `npm run test:run -- src/domain/javascript/javascriptSemantic src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts` — 982 tests across 38 files passed. Both upstream exact-key lookup coverage and the new capacity regression were retained when resolving the test-file conflict. - `npm run typecheck` — passed. - `npm run lint` (run by `npm run check` after rebasing) — zero errors, 1,444 warnings; module boundaries passed for 1,911 source files. - `npm run format:check` — passed. - `npm run verify:test-discovery` — all 1,313 test files discovered exactly once. - `npm run compile` — passed with two ineffective dynamic-import warnings. - `git diff --check` — passed before committing. - Independent automated read-only review — no actionable findings; reviewer separately ran 160 focused tests. This does not establish human review. ## Compatibility, safety, and release - Breaking changes or migration steps: no caller migration. Queries exceeding the getter budget now produce explicit partial/unknown coverage; all value-derived facts in that file are conservatively omitted. - Real Hopper/Ghidra, browser, or OS coverage: no engine integration changes; source analysis checked on Windows. Original macOS bundle and complete worker launch remain unverified as explained above. - Package or release metadata impact: none. - Security, privacy, process, or containment review: analyzed JavaScript remains inert. The budget bounds combinatorial getter expansion before further getter/result allocation. The change can be reverted without data migration; the behavioral tradeoff is reduced semantic precision on queries exceeding the bound. ## Author readiness Human owner: @hannnnnnnny; human review and readiness are pending. - [ ] 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. --- <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:37 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:36 +00:00

[PR #6023] [MERGED] fix(electron): record actions skipped after a failed action #7128

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

No description provided.

Rows
Columns