#2012 · stacks/stx

Reproduction

NODE_ENV=development bun test packages/stx/test/process/script-setup.test.ts

Three failures in Script Processing Fixes > Server script stripping with </script> in source:

does not terminate early on </script> inside a double-quoted string
does not terminate early on </script> inside a single-quoted string
does not terminate early on </script> inside a block comment

The same file passes with no failures under a bare bun test, which sets NODE_ENV=test.

What happens

Given

<script server>const html = "<script>x</script>"; const visible = 1;</script><div>after</div>

the output is

<div data-stx-server-script-error role="alert" …>
  stx: a &lt;script server&gt; does not parse, so this component rendered nothing.
  /test.stx — Unexpected EOF
</div>"; const visible = 1;</script><div>after</div>

The boundary markup is spliced in over a span that ends at the nested </script>, so everything from there to the real close tag — "; const visible = 1;</script> — is left behind as literal page text. The server script's source leaks into the rendered document.

Why it is invisible in CI and in tests

process.ts:

const showBoundaries = !isProduction() && !isTest()

and env.ts:

export function isTest(): boolean {
  return process.env.NODE_ENV === 'test'
}

bun test sets NODE_ENV=test, so showBoundaries is false and the boundary is never rendered during the suite. The splice is therefore untested, and the only mode where the boundary does render — development — is the one nobody exercises.

That also explains the intermittent red I saw on a full bun test packages/stx/test/: several test files legitimately mutate process.env.NODE_ENV (server-script-error-boundary.test.ts sets development per test and restores it correctly; directives/conditionals.test.ts sets production). A save/restore in one file that captures "original" while another file's beforeEach has it set to development will restore development permanently, and from then on every suite takes the boundary branch. Running the two files together passes; the full package run does not.

Suggested direction

Two separable things:

The splice span. The boundary's index/length should cover the whole <script server>…</script> element as findScriptBodyEnd resolves it — the same quote/comment/template-aware scan the stripper already uses — not the first </script> the splice site happens to find. The stripper gets this right; the boundary path does not reuse it.
isTest() is not a reliable guard. It trusts an env var that any test may set, so a mode flag derived from it flips mid-suite. Detecting the runner directly would make showBoundaries deterministic, and would also mean the boundary path gets covered by the suite instead of being skipped by it.

Found while counting what remained of stacksjs/stx#1993; not caused by that work. Introduced in f71f544f17.

Fixed in 87ef44658a.

Both of the separable things in the suggested direction, and it turned out they were one fix.

The splice span. The stripper two hundred lines further down already did this correctly — scanScriptTags plus findScriptBodyEnd, a walk that tracks string, template, comment and regex state and returns the index of the real close tag. The extraction loop was the only place still using a non-greedy regex. It uses the same pair now.

That fixes both halves at once, which I had not expected: with the correct body the script parses, so there is no failure to report and no boundary to splice. The truncated body was not a second symptom, it was the cause of the first.

NODE_ENV=development bun test packages/stx/test/process/script-setup.test.ts
  60 pass, 0 fail      (was 3 fail)

The coverage gap. You were right that this is the more important half. showBoundaries is !isProduction() && !isTest(), so the only mode that renders a boundary was the one the suite never ran in — the splice was unreachable from the suite by construction, which is why a path that leaked source into a rendered page sat there with CI green.

Four cases now live in server-script-error-boundary.test.ts, which already sets up the development environment per test and restores it. They cover a valid script with </script> in a double-quoted string, one with it in a single-quoted string and a block comment, a genuinely failing script asserting no fragment of its source survives, and one asserting no stray close tag is left behind — that last separately, because a leaked "; const visible = 1;</script> is worse than a leaked identifier: it closes a tag the document never opened.

I left isTest() alone. Making it detect the runner would widen the blast radius to every other mode flag derived from it, and the narrower change — covering the path in the environment that actually renders it — gets the same protection. Worth revisiting if another guard turns out to have the same hole.

One of my assertions was wrong on the first pass, and the failure was instructive: I asserted the identifier thisDoesNotExist was absent from the output. It is present — in the boundary's own message, thisDoesNotExist is not defined, which is the diagnostic working. The call expression thisDoesNotExist() is what only the leaked source would contain.

The full suite is green: 14,143 pass, 0 fail. This was the last red on main.