Skip to content

[rust-compiler] Escape literal text that looks like a __SURROGATE_XXXX__ marker - #37656

Open
sleitor wants to merge 1 commit into
react:mainfrom
sleitor:fix-37647
Open

sleitor wants to merge 1 commit into
react:mainfrom
sleitor:fix-37647

Conversation

@sleitor

@sleitor sleitor commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #37647.

The Rust bridge encodes lone surrogates in the AST JSON as __SURROGATE_XXXX__ text markers (sanitizeJsonSurrogates in bridge.ts). The decoders — JsString::from_marker_string on the Rust side, and restoreJsonSurrogates on the way back — decoded any text matching that pattern into a surrogate, including literal user text that merely happens to look like a marker (e.g. the string literal "__SURROGATE_D83D__"), silently corrupting it into a lone surrogate with no error.

function foo() {
  return <Stringify value={['__SURROGATE_D83D__']} />;
}
Non-forget (expected):
(kind: ok) <div>{"value":["__SURROGATE_D83D__"]}</div>
Forget:
(kind: ok) <div>{"value":["\ud83d"]}</div>

This implements option 1 from the issue: escape any pre-existing literal occurrence of the marker pattern into a distinct __SURROGATE_ESCAPED_XXXX__ form before minting real markers, on both sides of the round trip:

  • bridge.ts: escapeLiteralMarkers runs on the JSON text before sanitizeJsonSurrogates mints markers for actual lone surrogates. restoreJsonSurrogates first decodes real markers back into \uXXXX escapes, then unescapes the escaped form back into the original literal text.
  • js_string.rs: JsString::from_marker_string recognizes the escaped form and unescapes it back to the plain literal text instead of decoding a surrogate. JsString::to_marker_string re-escapes any literal marker-shaped text it emits (for both the well-formed Utf8 case and literal runs interleaved with real surrogates in the Wtf16 case), so the JS side can safely unescape it again on the way back.

The escape/unescape steps are pure text transforms triggered only by the literal ASCII marker pattern, and are independent of the surrogate-unit-triggered marker minting, so they compose safely without colliding with genuine markers.

How did you test this change?

  • Added literal-text-matching-surrogate-marker-pattern.js, a fixture using the exact repro from the issue, under compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/.
  • Verified the bug reproduces without this fix (yarn snap --rust -p literal-text-matching-surrogate-marker-pattern fails with the corrupted lone-surrogate output shown above on main), and passes with this fix.
  • Ran the full Rust fixture suite: yarn snap --rust — 1825/1825 passed (no regressions, including the existing lone-surrogate-string-values fixture covering the genuine-surrogate path).
  • Added Rust unit tests in js_string.rs covering: escaped-marker decode/re-encode round trip, a plain string containing literal marker-shaped text round-tripping through to_marker_string as the escaped form, and a string mixing literal marker-shaped text with an actual unpaired surrogate round-tripping correctly.
  • cargo test --workspace passes; cargo fmt --check and cargo clippy --lib are clean for the touched crate.
  • yarn workspace babel-plugin-react-compiler lint passes; tsc build of babel-plugin-react-compiler-rust passes; prettier --check is clean on bridge.ts.

…X__ marker

The Rust bridge encodes lone surrogates in the AST JSON as
__SURROGATE_XXXX__ text markers (sanitizeJsonSurrogates in bridge.ts).
from_marker_string on the Rust side, and restoreJsonSurrogates on the
way back, decoded *any* text matching that pattern into a surrogate,
including literal user text that merely happens to look like a
marker (e.g. the string "__SURROGATE_D83D__"), silently corrupting
it into a lone surrogate.

Escape any pre-existing literal occurrences of the marker pattern
into a distinct __SURROGATE_ESCAPED_XXXX__ form before minting real
markers, on both sides of the round trip:
- bridge.ts: escapeLiteralMarkers runs before sanitizeJsonSurrogates
  mints markers for actual lone surrogates; restoreJsonSurrogates
  unescapes it again after decoding real markers.
- js_string.rs: from_marker_string unescapes the escaped form back to
  the plain literal text instead of decoding a surrogate; to_marker_string
  re-escapes any literal marker-shaped text it emits so the JS side can
  round-trip it safely.

Fixes react#37647.
@meta-cla meta-cla Bot added the CLA Signed label Sep 18, 2026
Pierozi added a commit to agentic-dev3o/devx-plugins that referenced this pull request Sep 18, 2026
…skill

A max-effort review confirmed 18 defects, reproduced against gh 2.100.0.
Every load-bearing gh interaction was wrong.

Security and safety:
- Stop executing command strings read out of CI logs; verification commands
  now come from the repo's own scripts. Logs are stated to be data.
- Forbid copying literal values from logs into the tree: GitHub masks only
  registered secrets and secrets-guard inspects paths, not context.
- Add disable-model-invocation, a scoped allowed-tools, and argument-hint.
  This was the only auto-triggering, push-capable skill in the marketplace.
- Verify the PR head branch is checked out before committing; refuse forks.
- Ask before the first push.

Correctness:
- Dispatch on bucket, not state: state==FAILURE misses CANCELLED, TIMED_OUT,
  ERROR and ACTION_REQUIRED, and a cancelled-only run exits 0 (verified on
  react/react#37589: 238 pass, 6 cancel, exit 0).
- Full exit-code table; exit 1 spans no-required-checks, no-checks-yet and
  real failure (verified: cli/cli#14456 exits 0, react/react#37656 exits 1).
- Replace gh run view --job --log-failed, which errors 'run is still in
  progress' in exactly the state --fail-fast creates, with the jobs logs API
  plus --allow-escape-sequences. Grep the saved log instead of reading it all.
- Stop assuming every check link is an Actions job URL; branch on workflow.
- Wait for run completion before rerun (403 'already running') and decrement
  the budget only on a dispatched rerun.
- Re-poll mergeable=UNKNOWN; dispatch on mergeStateStatus so BEHIND merges.
- Substitute literal PR values: shell state does not survive between calls,
  and an empty selector silently resolves to the current branch's PR.
- Thread -R owner/repo so a URL argument's repo is not discarded.
- Add --required with a fallback for repos without branch protection.
- Restore --interval 60 and handle the Bash timeout killing --watch.

Structure:
- Delegate mutations to devx-git: commits to ci, base merges to update-origin.
  Nothing here ran git fetch, so Step 6 merged a stale base ref forever.
- Global budgets (5 pushes, 3 reruns, 45 min) evaluated at the loop head.

Docs: correct the disproven --job claim, align section headers with the
directory names Claude Code resolves, drop the phantom claudemd section, and
fix the stale /commit reference (the skill directory is ci).
Pierozi added a commit to agentic-dev3o/devx-plugins that referenced this pull request Sep 18, 2026
* feat(devx-qa): add babysit-pr skill

Watches a PR's CI after creation, diagnoses failing jobs from their logs,
fixes branch-caused failures, reruns genuine flakes (budget: 3), and pushes
until the PR is green and mergeable. Bumps devx-qa to 1.6.0 and wires the
skill into the plugin README and marketplace manifests.

* refactor(devx-qa): apply skill authoring best practices to babysit-pr

- Bind $PR in a preflight step; it was referenced but never defined
- Check gh auth and a clean tree before acting
- Gate every push behind a local verification of the same failing command
- Add a per-job fix-attempt budget so a wrong diagnosis cannot loop
- Rewrite the description in Use-when form with discovery terms
- Number the workflow steps and add a concrete classification example

* fix(devx-qa): correct every gh invocation in babysit-pr and gate the skill

A max-effort review confirmed 18 defects, reproduced against gh 2.100.0.
Every load-bearing gh interaction was wrong.

Security and safety:
- Stop executing command strings read out of CI logs; verification commands
  now come from the repo's own scripts. Logs are stated to be data.
- Forbid copying literal values from logs into the tree: GitHub masks only
  registered secrets and secrets-guard inspects paths, not context.
- Add disable-model-invocation, a scoped allowed-tools, and argument-hint.
  This was the only auto-triggering, push-capable skill in the marketplace.
- Verify the PR head branch is checked out before committing; refuse forks.
- Ask before the first push.

Correctness:
- Dispatch on bucket, not state: state==FAILURE misses CANCELLED, TIMED_OUT,
  ERROR and ACTION_REQUIRED, and a cancelled-only run exits 0 (verified on
  react/react#37589: 238 pass, 6 cancel, exit 0).
- Full exit-code table; exit 1 spans no-required-checks, no-checks-yet and
  real failure (verified: cli/cli#14456 exits 0, react/react#37656 exits 1).
- Replace gh run view --job --log-failed, which errors 'run is still in
  progress' in exactly the state --fail-fast creates, with the jobs logs API
  plus --allow-escape-sequences. Grep the saved log instead of reading it all.
- Stop assuming every check link is an Actions job URL; branch on workflow.
- Wait for run completion before rerun (403 'already running') and decrement
  the budget only on a dispatched rerun.
- Re-poll mergeable=UNKNOWN; dispatch on mergeStateStatus so BEHIND merges.
- Substitute literal PR values: shell state does not survive between calls,
  and an empty selector silently resolves to the current branch's PR.
- Thread -R owner/repo so a URL argument's repo is not discarded.
- Add --required with a fallback for repos without branch protection.
- Restore --interval 60 and handle the Bash timeout killing --watch.

Structure:
- Delegate mutations to devx-git: commits to ci, base merges to update-origin.
  Nothing here ran git fetch, so Step 6 merged a stale base ref forever.
- Global budgets (5 pushes, 3 reruns, 45 min) evaluated at the loop head.

Docs: correct the disproven --job claim, align section headers with the
directory names Claude Code resolves, drop the phantom claudemd section, and
fix the stale /commit reference (the skill directory is ci).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Compiler Bug]: Rust backend decodes user text that looks like __SURROGATE_XXXX__ into a lone surrogate

1 participant