packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts (1)

228-232: Incomplete string escaping in assertion — CodeQL is correct.

The replace(/"/g, '\\"') only escapes double quotes but does not first escape backslash characters. If payload ever contained a \, the expected string would be wrong (e.g., \n in the payload would remain \n instead of becoming \\n before quote escaping).

For the current hardcoded payload this doesn't cause a test failure, but the pattern is fragile. Consider using a proper JSON string escaping or escaping backslashes first:

♻️ Robust escaping

-      expect(result).toContain(payload.replace(/"/g, '\\"'));
+      expect(result).toContain(payload.replace(/\\/g, '\\\\').replace(/"/g, '\\"'));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts` around lines 228
- 232, The assertion builds an expected string by only escaping double quotes
(payload.replace(/"/g, '\\"')) which misses escaping backslashes; update the
test in RSCRequestTracker.test.ts to produce a robust escaped string for the
payload used in the expect(result).toContain calls—either escape backslashes
first (replace(/\\/g, '\\\\') then escape quotes) or use
JSON.stringify(payload). Ensure you update the reference to the payload variable
used in the expect assertions so the expected value matches how the payload
appears in the generated output (REACT_ON_RAILS_RSC_PAYLOADS and the .push(... )
checks).

packages/react-on-rails-pro/tests/streamBackpressure.e2e.test.tsx (1)

9-14: Stale comment: "pipe-based tee" no longer accurate.

Line 11 refers to RSCRequestTracker.getRSCPayloadStream (pipe-based tee), but the implementation now uses manual event-based forwarding. Consider updating to reflect the current approach (e.g., "event-based tee" or just "tee").

📝 Suggested comment update

 *   streamServerRenderedReactComponent
 *     → createReactOutput (render function)
-*     → RSCRequestTracker.getRSCPayloadStream (pipe-based tee)
+*     → RSCRequestTracker.getRSCPayloadStream (event-based tee)
 *     → renderToPipeableStream (React SSR)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/streamBackpressure.e2e.test.tsx` around
lines 9 - 14, Update the stale inline comment that describes
RSCRequestTracker.getRSCPayloadStream: replace "pipe-based tee" with wording
that reflects the current implementation (e.g., "event-based tee" or simply
"tee") so the comment matches the actual manual event-based forwarding logic
used by getRSCPayloadStream; locate the comment near the test header referencing
streamServerRenderedReactComponent → createReactOutput →
RSCRequestTracker.getRSCPayloadStream and change that phrase accordingly.

packages/react-on-rails-pro/tests/streamBackpressure.e2e.test.tsx (2)

38-52: collectChunks assumes each data event contains complete JSON lines.

If a multi-byte character or JSON boundary ever falls across two Buffer events, JSON.parse(line) will throw. This is fine for these tests since streamServerRenderedReactComponent emits whole JSON objects per write, but consider adding a brief comment noting that assumption so future maintainers don't reuse this helper in contexts where partial chunks are possible.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/streamBackpressure.e2e.test.tsx` around
lines 38 - 52, collectChunks assumes each 'data' event yields complete
newline-delimited JSON lines and will throw if a JSON object is split across
Buffer events; add a brief inline comment inside the collectChunks helper (near
the stream.on('data'...) handler) stating the assumption that
streamServerRenderedReactComponent writes complete JSON objects per write and
that this helper is NOT safe for streams that may emit partial/chunked JSON
across data events (mention multi-byte character boundaries and partial JSON as
causes), so future maintainers know not to reuse it for fragmented streams.

---

155-184: setInterval is not cleaned up on test failure.

If collectChunks rejects before all chunks are pushed (e.g., a stream error), the interval keeps ticking and calling source.push() until jest kills the process. Wrapping the push loop with a try/finally or tracking the interval for cleanup in afterEach would make this more robust.

Proposed fix

+    let pushInterval: ReturnType<typeof setInterval>;
     const pushInterval = setInterval(() => {
       if (pushed >= chunkCount) {
         clearInterval(pushInterval);
         source.push(null);
         return;
       }
       source.push(chunk);
       pushed++;
     }, 1);
 
-    const chunks = await collectChunks(renderResult);
+    let chunks;
+    try {
+      chunks = await collectChunks(renderResult);
+    } finally {
+      clearInterval(pushInterval);
+    }
     const allHtml = chunks.map((c) => c.html).join('');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/streamBackpressure.e2e.test.tsx` around
lines 155 - 184, The setInterval started in the test "does not deadlock when
large RSC payload chunks arrive asynchronously" (pushInterval) is not cleared if
collectChunks rejects, leading to a stray interval calling source.push; update
the test to ensure pushInterval is always cleared by wrapping the chunk push
loop and the await collectChunks(renderResult) in a try/finally that calls
clearInterval(pushInterval) (or register pushInterval for cleanup in an
afterEach teardown) so clearInterval runs on both success and failure and the
source.push loop cannot continue after the test ends.

packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts (2)

25-33: pushChunks return value may exceed totalBytes — naming could mislead.

Math.ceil(totalBytes / chunkSize) means the actual bytes pushed can exceed the requested totalBytes (e.g., pushChunks(source, 100) pushes 1024 bytes). The return value is correct (actual bytes), but the parameter name totalBytes reads as "exact amount" while it's really a minimum. The tests all use the return value correctly, so this is just a naming nit.

Consider renaming for clarity

-const pushChunks = (stream: PassThrough, totalBytes: number, chunkSize = 1024) => {
+const pushChunks = (stream: PassThrough, minBytes: number, chunkSize = 1024) => {
   const chunk = Buffer.alloc(chunkSize, 0x61); // fill with 'a'
-  const count = Math.ceil(totalBytes / chunkSize);
+  const count = Math.ceil(minBytes / chunkSize);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts` around lines 25
- 33, The helper pushChunks currently takes totalBytes but uses
Math.ceil(totalBytes / chunkSize) so it may push more than requested; rename the
parameter and related identifiers to reflect it is a minimum (e.g., minBytes or
requestedBytes) and update the inline comment to state it pushes at least that
many bytes (returns actual bytes pushed), and adjust any test callsites and the
function signature in RSCRequestTracker.test.ts to use the new name so the API
and naming are not misleading.

---

165-192: Interval-based async tests lack cleanup on failure — same pattern as the e2e file.

All three setInterval-based tests share the same risk: if the awaited stream collection rejects, the interval keeps firing source.push() into a potentially destroyed stream. A try/finally guard or storing the interval handle in a variable cleared in afterEach would improve robustness.

Example: guard with try/finally

     const pushInterval = setInterval(() => {
       if (pushed >= chunkCount) {
         clearInterval(pushInterval);
         source.push(null);
         return;
       }
       source.push(chunk);
       pushed++;
     }, 1);

-    const data1 = await collectStreamData(stream1);
-    const data2 = await collectStreamData(stream2);
+    let data1: Buffer, data2: Buffer;
+    try {
+      data1 = await collectStreamData(stream1);
+      data2 = await collectStreamData(stream2);
+    } finally {
+      clearInterval(pushInterval);
+    }

Also applies to: 265-295, 344-372

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/react-on-rails-pro/tests/RSCRequestTracker.test.ts` around lines 165
- 192, The interval-based drip-feed in the test using setupSourceStream()/source
and pushInterval must be cleared on test failure to avoid pushing into a
destroyed stream; wrap the await
collectStreamData(stream1)/collectStreamData(stream2) calls in a try/finally (or
capture pushInterval in a variable reachable by afterEach) and call
clearInterval(pushInterval) and source.push(null) in the finally block so the
interval is always cleaned up; apply the same pattern to the other tests that
create pushInterval (the tests using setupSourceStream, source, pushInterval and
collectStreamData).