morluto/rea

Fork 0

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

[PR #2900] [CLOSED] fix(javascript): recognize comments in GraphQL operation headers #5483

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2900
Author: @nikhiljangid120
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/graphql-header-comments

📝 Commits (2)

08be204 fix(javascript): recognize comments in GraphQL operation headers
1a4f187 test(javascript): verify GraphQL comments through CLI and MCP

📊 Changes

5 files changed (+163 additions, -23 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+2 -1)
📝 src/domain/javascript/javascriptRpcEndpoints.test.ts (+39 -0)
📝 src/domain/javascript/javascriptRpcEndpoints.ts (+40 -22)
📝 src/domain/webBundleAnalyzer.test.ts (+2 -0)
➕ tests/acceptance/applications/graphqlHeaderComments.test.ts (+80 -0)

📄 Description

Summary

Fixes #2523. Recover named GraphQL operations when ignored line comments occur inside the operation header, not only before its keyword.

Problem and expected behavior

const document = "query # comment\nGetUser { user }"; currently produces no GraphQL endpoint. It should yield { kind: "network", value: "GetUser", mechanism: "graphql:query" }, without claiming that a request executed.

Change and scope

Replace the whitespace/comma-only header regex with bounded token recognition. One ignored-token scanner owns whitespace, comma, BOM and comment handling at each header boundary. Recover the operation type/name and retain the existing variable-definition, directive or named-first-field recognition scope. Update the owning documentation.

Anonymous operations, prose, incomplete headers, comments hiding the remaining header and fragment-spread-first selections remain unrecognized. No full GraphQL parser, interpolation folding or target execution is introduced.

Related #1810 changes template interpolation ownership; this patch addresses header token separators and does not implement that PR's behavior. Its helper extraction may require a small adjacent-hunk reconciliation if it lands first.

Contract and boundary impact

Semantic owner: javascriptRpcEndpoints.ts, inert JavaScript static analysis before graph projection; consumers share the same recovered facts.
Scope decision: not applicable; ordinary static-recovery fix within the existing contribution scope.
CLI/MCP/tool-catalog: unchanged schemas and operations; previously omitted static endpoint candidates can now appear.
Provider/platform: no provider-specific changes.
Evidence/provenance: original input and existing source-range attribution remain unchanged; no runtime-execution claim.
Authority/cleanup: no added target execution, injection, mutation, network access or cleanup responsibility. Existing analysis-worker behavior is unchanged.
Generated metadata/package/installation: no schema or version changes; no generated artifacts committed.

Expected effects of the affected existing analysis workflow:

Read existing files or captured data.

Evidence and regression coverage

Nine positive header-comment cases cover all three operation types, LF/CRLF/CR endings, repeated comments, variable definitions, directives and selection fields; each exercises both strings and cooked templates.
Eight negative cases preserve unsupported/incomplete-input behavior.
The captured-web-bundle regression checks the same recovery through the existing captured-script producer.
On unchanged production source at 72b9ab4, the two affected test files fail 10 tests, with 49 passing. With the fix, all 59 pass.
Initial branch baseline 7e3a198 differed only in unrelated logical-assignment analysis files; the broader affected suite passed 1,324 tests in 57 files. After synchronizing with upstream ebaaa79, the same 1,324 tests pass again.
New compiled CLI and real stdio MCP acceptance regressions recover all three operation types, exclude anonymous/incomplete/prose controls and verify AST authority, artifact SHA-256 and source-range provenance. Both tests fail with the pre-fix recognizer and pass with the fix. The inert fixture includes a top-level throw, so success also guards against executing it.
Performance: no speed/token/memory improvement is claimed; completed-task performance was not measured.

Validation performed

Linux x64; pinned Node 24.18.0 and npm 11.16.0.

npm run test:local -- src/domain/javascript/javascriptRpcEndpoints.test.ts src/domain/webBundleAnalyzer.test.ts — 59 passed.
vitest run --project domain src/domain/javascript src/domain/webBundleAnalyzer.test.ts — final baseline: 1,324 passed in 57 files.
oxlint on changed TypeScript files — zero warnings/errors.
oxfmt --check on all four changed files — passed.
git diff --check — passed.
npm run verify:test-discovery — all 1,107 test files discovered exactly once on the initial baseline.

Compatibility, safety and remaining proof gaps

Originally opened as a draft because the initial upstream checkout had build/type and formatting failures. The PR has since been marked ready, and its branch is now synchronized with upstream ebaaa79. Those earlier blockers are no longer current: npm run build:cached now passes.

Follow-up validation on Linux x64, pinned Node 24.18.0 / npm 11.16.0:

npm run test:focused -- tests/acceptance/applications/graphqlHeaderComments.test.ts — two compiled public CLI/stdio MCP tests pass; both fail with the pre-fix recognizer.
vitest run --project domain src/domain/javascript src/domain/webBundleAnalyzer.test.ts — 1,324 tests pass in 57 files.
GOMAXPROCS=2 GOMEMLIMIT=768MiB npm run typecheck — passed. The first unbounded attempt was killed with exit 137; bounded verification completed.
npm run docs:check — passed, including five README consistency tests.
npm run verify:test-discovery — all 1,113 test files discovered exactly once.
npm run lint — passed, including resolved module boundaries in 1,758 source files.
Targeted format checks for the new acceptance test and git diff --check — passed.

npm run format:check flags two upstream files outside this patch: docs/readme/README_tr.md and scripts/lib/clean-linux-hopper-runtime.mjs. They are unchanged in this contribution.

The complete npm run check still did not finish: Knip's oxc-parser failed with RangeError: Array buffer allocation failed in this resource-constrained runner. No unrelated checker/dependency code was changed to bypass it. Full deterministic CI and coverage remain pending; real-browser/native-provider and Windows/macOS execution were not run. The captured-bundle test is synthetic; the public static-analysis CLI and stdio MCP runs are real, on owned inert files. No breaking schema or migration change is intended.

AI assistance

This contribution was prepared, implemented and tested end-to-end by an AI coding assistant acting for the account owner. The validation above was executed locally. It does not assert human line-by-line review; the account owner has not yet performed that review.

Before merge

Concrete issue and focused conventional-commit outcome.
Ordinary static-analysis scope; no authority expansion.
Regression and negative coverage; owning documentation updated.
Compiled CLI/MCP regression acceptance completed.
Complete repository-wide CI/coverage gates completed.
Account-owner review and understanding of the final change completed.

_{🔄 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/2900 **Author:** [@nikhiljangid120](https://github.com/nikhiljangid120) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/graphql-header-comments` --- ### 📝 Commits (2) - [`08be204`](https://github.com/morluto/rea/commit/08be2042303fe341d5326996552d5dda21e5e2db) fix(javascript): recognize comments in GraphQL operation headers - [`1a4f187`](https://github.com/morluto/rea/commit/1a4f1872783ca9f7f1036719e565a43106030714) test(javascript): verify GraphQL comments through CLI and MCP ### 📊 Changes **5 files changed** (+163 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+2 -1) 📝 `src/domain/javascript/javascriptRpcEndpoints.test.ts` (+39 -0) 📝 `src/domain/javascript/javascriptRpcEndpoints.ts` (+40 -22) 📝 `src/domain/webBundleAnalyzer.test.ts` (+2 -0) ➕ `tests/acceptance/applications/graphqlHeaderComments.test.ts` (+80 -0) </details> ### 📄 Description ## Summary Fixes #2523. Recover named GraphQL operations when ignored line comments occur inside the operation header, not only before its keyword. ## Problem and expected behavior `const document = "query # comment\nGetUser { user }";` currently produces no GraphQL endpoint. It should yield `{ kind: "network", value: "GetUser", mechanism: "graphql:query" }`, without claiming that a request executed. ## Change and scope Replace the whitespace/comma-only header regex with bounded token recognition. One ignored-token scanner owns whitespace, comma, BOM and comment handling at each header boundary. Recover the operation type/name and retain the existing variable-definition, directive or named-first-field recognition scope. Update the owning documentation. Anonymous operations, prose, incomplete headers, comments hiding the remaining header and fragment-spread-first selections remain unrecognized. No full GraphQL parser, interpolation folding or target execution is introduced. Related #1810 changes template interpolation ownership; this patch addresses header token separators and does not implement that PR's behavior. Its helper extraction may require a small adjacent-hunk reconciliation if it lands first. ## Contract and boundary impact - Semantic owner: `javascriptRpcEndpoints.ts`, inert JavaScript static analysis before graph projection; consumers share the same recovered facts. - Scope decision: not applicable; ordinary static-recovery fix within the existing contribution scope. - CLI/MCP/tool-catalog: unchanged schemas and operations; previously omitted static endpoint candidates can now appear. - Provider/platform: no provider-specific changes. - Evidence/provenance: original input and existing source-range attribution remain unchanged; no runtime-execution claim. - Authority/cleanup: no added target execution, injection, mutation, network access or cleanup responsibility. Existing analysis-worker behavior is unchanged. - Generated metadata/package/installation: no schema or version changes; no generated artifacts committed. Expected effects of the affected existing analysis workflow: - [x] Read existing files or captured data. ## Evidence and regression coverage - Nine positive header-comment cases cover all three operation types, LF/CRLF/CR endings, repeated comments, variable definitions, directives and selection fields; each exercises both strings and cooked templates. - Eight negative cases preserve unsupported/incomplete-input behavior. - The captured-web-bundle regression checks the same recovery through the existing captured-script producer. - On unchanged production source at `72b9ab4`, the two affected test files fail **10 tests**, with 49 passing. With the fix, all **59** pass. - Initial branch baseline `7e3a198` differed only in unrelated logical-assignment analysis files; the broader affected suite passed **1,324 tests in 57 files**. After synchronizing with upstream `ebaaa79`, the same 1,324 tests pass again. - New compiled CLI and real stdio MCP acceptance regressions recover all three operation types, exclude anonymous/incomplete/prose controls and verify AST authority, artifact SHA-256 and source-range provenance. Both tests fail with the pre-fix recognizer and pass with the fix. The inert fixture includes a top-level throw, so success also guards against executing it. - Performance: no speed/token/memory improvement is claimed; completed-task performance was not measured. ## Validation performed Linux x64; pinned Node 24.18.0 and npm 11.16.0. - `npm run test:local -- src/domain/javascript/javascriptRpcEndpoints.test.ts src/domain/webBundleAnalyzer.test.ts` — 59 passed. - `vitest run --project domain src/domain/javascript src/domain/webBundleAnalyzer.test.ts` — final baseline: 1,324 passed in 57 files. - `oxlint` on changed TypeScript files — zero warnings/errors. - `oxfmt --check` on all four changed files — passed. - `git diff --check` — passed. - `npm run verify:test-discovery` — all 1,107 test files discovered exactly once on the initial baseline. ## Compatibility, safety and remaining proof gaps Originally opened as a draft because the initial upstream checkout had build/type and formatting failures. The PR has since been marked ready, and its branch is now synchronized with upstream `ebaaa79`. Those earlier blockers are no longer current: `npm run build:cached` now passes. Follow-up validation on Linux x64, pinned Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- tests/acceptance/applications/graphqlHeaderComments.test.ts` — two compiled public CLI/stdio MCP tests pass; both fail with the pre-fix recognizer. - `vitest run --project domain src/domain/javascript src/domain/webBundleAnalyzer.test.ts` — 1,324 tests pass in 57 files. - `GOMAXPROCS=2 GOMEMLIMIT=768MiB npm run typecheck` — passed. The first unbounded attempt was killed with exit 137; bounded verification completed. - `npm run docs:check` — passed, including five README consistency tests. - `npm run verify:test-discovery` — all 1,113 test files discovered exactly once. - `npm run lint` — passed, including resolved module boundaries in 1,758 source files. - Targeted format checks for the new acceptance test and `git diff --check` — passed. `npm run format:check` flags two upstream files outside this patch: `docs/readme/README_tr.md` and `scripts/lib/clean-linux-hopper-runtime.mjs`. They are unchanged in this contribution. The complete `npm run check` still did not finish: Knip's oxc-parser failed with `RangeError: Array buffer allocation failed` in this resource-constrained runner. No unrelated checker/dependency code was changed to bypass it. Full deterministic CI and coverage remain pending; real-browser/native-provider and Windows/macOS execution were not run. The captured-bundle test is synthetic; the public static-analysis CLI and stdio MCP runs are real, on owned inert files. No breaking schema or migration change is intended. ## AI assistance This contribution was prepared, implemented and tested end-to-end by an AI coding assistant acting for the account owner. The validation above was executed locally. It does not assert human line-by-line review; the account owner has not yet performed that review. ## Before merge - [x] Concrete issue and focused conventional-commit outcome. - [x] Ordinary static-analysis scope; no authority expansion. - [x] Regression and negative coverage; owning documentation updated. - [x] Compiled CLI/MCP regression acceptance completed. - [ ] Complete repository-wide CI/coverage gates completed. - [ ] Account-owner review and understanding of the final change completed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:25 +00:00

[PR #6530] [MERGED] feat(cli): report version and runtime with an explicit output format #7377

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

No description provided.

Rows
Columns