fix(playground): emit PHP escapes instead of raw control characters - #2407
Merged
Conversation
query-recorder.ts builds PHP source inside a JavaScript template literal. The whitespace list " \t\n\r\0\x0B" was written with single backslashes, so JavaScript resolved each sequence at evaluation time and the generated PHP carried actual tab, newline, carriage return, NUL and vertical tab bytes rather than PHP's own escape sequences. PHP parses either form to the same string, so behaviour is unchanged, but the raw NUL makes the generated source unpassable as a process argument: Node rejects argv entries containing null bytes. Any harness invoking 'php -r' with this code fails before PHP runs. That blocked three tests, which is why they sat unmaintained since June and were excluded when discovery landed in #2406: tests/rest-request-query-params.test.ts tests/wordpress-crud-contracts.test.ts tests/performance-observation-contracts.test.ts Escape the backslashes so PHP receives the escape text. Generated output now contains zero raw NUL or vertical tab characters, the three tests pass, and they return to discovery. Exclusions drop from 15 to 12. Verified unchanged: bench-command-step-behavior, agent-task-contracts, playground-fuzz-suite-public, nested-fuzz-suite-recipe-command, and php-fuzz-suite-runner-smoke all still pass. Full aggregate green at 341 commands in 7m59s.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First slice of #2402 item 5. Fixes a real defect and returns three excluded tests to the gate.
The bug
packages/runtime-playground/src/query-recorder.ts:80builds PHP source inside a JavaScript template literal:Those are single backslashes, so JavaScript resolves every sequence when the template is evaluated. The generated PHP carries actual tab, newline, carriage return, NUL and vertical tab bytes instead of PHP's own escape sequences.
Measured before the fix — the generated source contained a raw NUL at index 4893:
Why it matters
PHP parses either form to the same string, so behaviour is unchanged. The problem is transport: Node refuses argv entries containing null bytes, so any harness invoking
php -rwith this code dies before PHP starts.scripts/test-kit.tsrunPhpJsondoes exactly that, which is why three tests could never pass. They were added in June 2026, never wired to a gate, and sat rotting until discovery in #2406 surfaced them — where they had to be excluded.The fix
Escape the backslashes so PHP receives the escape text rather than the resolved bytes. One line.
Verification
Generated output is clean:
The three blocked tests now pass, and return to discovery:
tests/rest-request-query-params.test.tstests/wordpress-crud-contracts.test.tstests/performance-observation-contracts.test.tsNothing that consumes the generator regressed. Since the emitted PHP text changes, I checked every dependent:
Full aggregate green:
Discovery exclusions drop from 15 to 12.
Remaining in item 5
Seven excluded tests still need triage — they fail for reasons unrelated to this bug and each needs its own judgement on fix-versus-delete. Separately, the
test:scripts inpackage.jsonare now largely redundant with discovery and can be pruned. Both are follow-ups; bundling them here would bury a one-line production fix under unrelated churn.AI assistance disclosure: authored by Claude (Sonnet 4.5) running in OpenCode, directed by @chubes4. The model traced the three excluded tests to a shared root cause, confirmed the raw NUL byte in generated output, verified the fix removes it, and re-ran every consumer of the generator plus the full aggregate before opening. All figures are measured. Reviewed by a human before opening.