Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions e2e/react-start/import-protection/tests/violations.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,54 @@ import { chromium } from '@playwright/test'
import { getTestServerPort } from '@tanstack/router-e2e-utils'
import packageJson from '../package.json' with { type: 'json' }

import { extractViolationsFromLog } from './violations.utils'
import { extractViolationsFromLog, stripAnsi } from './violations.utils'
import type { FullConfig } from '@playwright/test'
import type { Violation } from './violations.utils'

const VIOLATION_MARKER = '[import-protection] Import denied in'

/**
* Counts the number of violation markers currently present in the log chunks.
*/
function countViolations(logChunks: Array<string>): number {
let count = 0
for (const chunk of logChunks) {
const cleaned = stripAnsi(chunk)
let idx = 0
while ((idx = cleaned.indexOf(VIOLATION_MARKER, idx)) !== -1) {
count++
idx += VIOLATION_MARKER.length
}
}
return count
Comment on lines +17 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n e2e/react-start/import-protection/tests/violations.setup.ts | head -60

Repository: TanStack/router

Length of output: 2473


🏁 Script executed:

wc -l e2e/react-start/import-protection/tests/violations.setup.ts

Repository: TanStack/router

Length of output: 122


🏁 Script executed:

# Verify the chunk-boundary marker issue with the provided test
python3 <<'PY'
marker = '[import-protection] Import denied in'
# Simulate marker split across chunks
chunks = ['prefix ' + marker[:18], marker[18:] + ' suffix']

def current_counter(parts):
    """Current implementation - chunk by chunk"""
    c = 0
    for chunk in parts:
        i = 0
        while True:
            i = chunk.find(marker, i)
            if i == -1:
                break
            c += 1
            i += len(marker)
    return c

def joined_counter(parts):
    """Fixed implementation - join first"""
    s = ''.join(parts)
    c = 0
    i = 0
    while True:
        i = s.find(marker, i)
        if i == -1:
            break
        c += 1
        i += len(marker)
    return c

print(f"Current implementation (chunk-by-chunk): {current_counter(chunks)}")
print(f"Fixed implementation (joined): {joined_counter(chunks)}")
print(f"Chunks: {chunks}")
print(f"Joined: {''.join(chunks)}")
PY

Repository: TanStack/router

Length of output: 260


🏁 Script executed:

# Find where logChunks is created and populated
rg -n "logChunks" e2e/react-start/import-protection/tests/violations.setup.ts -A 2 -B 2

Repository: TanStack/router

Length of output: 1560


🏁 Script executed:

# Check the full stabilization function and its call site
sed -n '30,110p' e2e/react-start/import-protection/tests/violations.setup.ts

Repository: TanStack/router

Length of output: 2304


🏁 Script executed:

# Verify type safety: check stripAnsi return type and VIOLATION_MARKER type
rg -n "stripAnsi|VIOLATION_MARKER" e2e/react-start/import-protection/tests/ -A 1 | head -40

Repository: TanStack/router

Length of output: 3020


Fix marker counting to handle split markers across stream chunks.

Stream chunks have arbitrary boundaries. The current chunk-by-chunk approach misses markers split across chunks, which causes countViolations to return an incorrect count. This can result in premature stabilization detection, causing the test to proceed before all violations are logged.

The demonstration confirms the issue: when a marker is split across two chunks, the current implementation counts 0 while the corrected implementation counts 1.

🔧 Proposed fix
 function countViolations(logChunks: Array<string>): number {
+  const cleanedLog = stripAnsi(logChunks.join(''))
   let count = 0
-  for (const chunk of logChunks) {
-    const cleaned = stripAnsi(chunk)
-    let idx = 0
-    while ((idx = cleaned.indexOf(VIOLATION_MARKER, idx)) !== -1) {
-      count++
-      idx += VIOLATION_MARKER.length
-    }
+  let idx = 0
+  while ((idx = cleanedLog.indexOf(VIOLATION_MARKER, idx)) !== -1) {
+    count++
+    idx += VIOLATION_MARKER.length
   }
   return count
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function countViolations(logChunks: Array<string>): number {
let count = 0
for (const chunk of logChunks) {
const cleaned = stripAnsi(chunk)
let idx = 0
while ((idx = cleaned.indexOf(VIOLATION_MARKER, idx)) !== -1) {
count++
idx += VIOLATION_MARKER.length
}
}
return count
function countViolations(logChunks: Array<string>): number {
const cleanedLog = stripAnsi(logChunks.join(''))
let count = 0
let idx = 0
while ((idx = cleanedLog.indexOf(VIOLATION_MARKER, idx)) !== -1) {
count++
idx += VIOLATION_MARKER.length
}
return count
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/react-start/import-protection/tests/violations.setup.ts` around lines 17
- 27, countViolations misses markers split across stream chunks; fix by keeping
a rolling buffer between chunks (e.g., a string "carry" appended with each
cleaned chunk) and searching that buffer for VIOLATION_MARKER instead of
inspecting each chunk in isolation; after consuming matches advance the search
position (or remove consumed portion) and then retain only the last
VIOLATION_MARKER.length - 1 characters in carry before processing the next chunk
so a marker split across boundaries will be detected; update the function
countViolations and references to VIOLATION_MARKER accordingly.

}

/**
* Polls the log output and waits until the violation count stops changing
* for `stableMs` milliseconds, or until `hardTimeoutMs` is reached.
* This avoids fixed-delay race conditions in slow CI environments where
* deferred violation processing may take longer than expected.
*/
async function waitForViolationStabilization(
logChunks: Array<string>,
{ stableMs = 2_000, hardTimeoutMs = 30_000, pollMs = 250 } = {},
): Promise<void> {
const start = Date.now()
let lastCount = countViolations(logChunks)
let lastChangeAt = Date.now()

while (Date.now() - start < hardTimeoutMs) {
await new Promise((r) => setTimeout(r, pollMs))
const currentCount = countViolations(logChunks)
if (currentCount !== lastCount) {
lastCount = currentCount
lastChangeAt = Date.now()
} else if (Date.now() - lastChangeAt >= stableMs) {
return
}
}
}
Comment on lines +36 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stabilization can return too early and hard-timeout currently fails open.

The loop can return after stableMs even if no violation has appeared yet, and if hardTimeoutMs is hit it exits silently. Both paths can still produce incomplete captures.

🔧 Proposed fix
 async function waitForViolationStabilization(
   logChunks: Array<string>,
   { stableMs = 2_000, hardTimeoutMs = 30_000, pollMs = 250 } = {},
 ): Promise<void> {
   const start = Date.now()
   let lastCount = countViolations(logChunks)
-  let lastChangeAt = Date.now()
+  let lastChangeAt = start
+  let hasObservedViolation = lastCount > 0
 
   while (Date.now() - start < hardTimeoutMs) {
     await new Promise((r) => setTimeout(r, pollMs))
     const currentCount = countViolations(logChunks)
     if (currentCount !== lastCount) {
       lastCount = currentCount
       lastChangeAt = Date.now()
-    } else if (Date.now() - lastChangeAt >= stableMs) {
+      if (currentCount > 0) hasObservedViolation = true
+    } else if (hasObservedViolation && Date.now() - lastChangeAt >= stableMs) {
       return
     }
   }
+
+  throw new Error(
+    `Timed out waiting for violation log stabilization after ${hardTimeoutMs}ms (lastCount=${lastCount})`,
+  )
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/react-start/import-protection/tests/violations.setup.ts` around lines 36
- 54, waitForViolationStabilization currently considers "stable" even if no
violations ever appeared and silently returns when hardTimeoutMs is reached;
update it so stabilization only succeeds when lastCount > 0 (i.e., at least one
violation recorded via countViolations) and, if the hardTimeoutMs elapses before
stabilization, throw an Error describing the timeout, elapsed time, and
lastCount. Modify the loop/return condition in waitForViolationStabilization to
check lastCount > 0 before returning and add a final throw with contextual info
including stableMs, hardTimeoutMs, and lastCount so callers can't treat a
timeout as success.


async function waitForHttpOk(url: string, timeoutMs: number): Promise<void> {
const start = Date.now()

Expand Down Expand Up @@ -154,7 +198,9 @@ async function runDevPass(
await browser.close()
}

await new Promise((r) => setTimeout(r, 750))
// Wait until the violation count in the server log stabilizes.
// This replaces a fixed delay and is more robust in slow CI.
await waitForViolationStabilization(logChunks)
} finally {
await killChild(child)
}
Expand Down
Loading