Skip to content

fix(signals,solid): reserve _parent from mangling (two prod/observe cross-package bugs); retire the wall-clock tripwires; PII surface in RFC 08 - #3496

Merged
ryansolid merged 1 commit into
nextfrom
cross-package-fields
Sep 16, 2026
Merged

ryansolid merged 1 commit into
nextfrom
cross-package-fields

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Three of the four remaining in-repo items from the Sentry/observe plan: the observe-tier mangling audit (which found two real bugs), the flaky timing tripwires, and Solid's half of the PII question. The engine diet stays a separate decision.

_parent is the second cross-package owner field

Signals' prod and observe artifacts mangle every _-prefixed property except a reserved list (scripts/mangle-props.mjs); the dev artifact is unmangled, and every suite runs against dev. Two reads of _parent cross the package boundary and only ever worked in dev:

  • solid-js client hydration (hydration.ts, markTopLevelSnapshotScope) walks owner._parent to the root to mark the hydration snapshot scope. In the built prod and observe artifacts owner._parent is undefined, so the walk marked the current owner — computations created outside that owner's subtree during hydration read live values instead of the server snapshot (CONFIG_IN_SNAPSHOT_SCOPE never set on them). Prod-only, invisible to the suite.
  • The core's owner walks over server ownersownerPath and OBSERVE.exclude/isExcluded. ownerPath had a server-side shim (located() in solid/src/server/diagnostics.ts, removed here); OBSERVE.exclude on a server owner was a silent no-op outside dev.

_parent joins _name on the reserved list. Cost is mangling only — nothing added to any code path: core floor +8, createStore +20, isPending +32, simple app +20, hydrating +43, hydrating+stores +19, CSR +41, observe CSR −38, attribution −2, frames 0 (bytes, brotli). Two caps bumped (hydrating 19.80→19.85, CSR 15.15→15.20) with notes.

Pinned from both ends:

  • packages/solid/test/cross-package-fields.spec.ts — the reserved fields survive in the mangled signals artifacts (and _firstChild does not, proving the mangler ran); every built client artifact of solid-js, @solidjs/web, @solidjs/universal is scanned for ._field accesses, and any signals _ field (parsed from types.ts) not on the reserved list fails with the file and context. Un-reserving _parent makes it flag hydration.ts's walk.
  • packages/web/test/server/server-owner-walks.spec.tsxownerPath (component labels included) and OBSERVE.exclude over server owners, against the built observe and development artifacts in a child Node under --conditions. Rebuilt without the reservation, both fail on the observe artifact.

Wall-clock tripwires retired

Four timing assertions were costing CI reruns under turbo's parallel load:

  • heap-mark-incremental: re-measures a round over the cap (up to three) — the quadratic regime is over the cap every round, a GC pause on the one large sample is not.
  • attribution-navigation: asserted settledMs >= 10 after a setTimeout(10), which can fire a hair under 10 ms on the performance.now() clock; now asserts against the wait actually taken.
  • lazy-shell-gating / dynamic-shell-gating: proved ORDER (shell before/after a source settled) with wall-clock deltas against timers (shellAt < DATA). Rewritten on shell-gating-harness.ts: every async source is a gate the test settles by hand, so "the shell waited for X" is "no shell before X settled, shell after" — no clock, and ~1 s instead of ~2 s of real timers.

PII surface (RFC 08)

"Values in records — the PII surface": the complete list of record and finding fields carrying user data — ChangeRecord/HeldWrite previews (strings cut at 40 chars), ChangeOrigin/InteractionRef.target text (up to 30 chars of textContent), navigation to/from/params, the responsiveness findings' data.interaction/data.navigation, and data.error on the server error findings (the error as thrown, deliberately unsanitized — the wire got the generic message so the observer could see the real one; connection strings land here). And what is safe by construction: re-runs, and the runtimes' records, which keep args/results/thrown values on live. This is Solid's half of plan open question 6; the adapter's half is scrubbing per this list.

Verification

signals 2736 · solid 623 + 825 types · web 991 + 185 types · diagnostics 34 · universal 56. Size caps within limits. Changeset: @solidjs/signals, solid-js.

…cond cross-package owner field; retire the wall-clock tripwires

Signals' prod/observe artifacts rename every `_` field but `_name`; the dev
artifact the suites run against is unmangled. Two `_parent` reads crossed
the package boundary and only worked in dev: solid-js's client hydration
walked `owner._parent` to mark the snapshot root (prod marked the current
owner instead), and the core's `ownerPath`/`isExcluded` walks over server
owners (`ownerPath` shimmed by `located()`, now removed; `OBSERVE.exclude`
a silent no-op for a server owner outside dev). `_parent` is reserved
alongside `_name`; ~40 B brotli on the prod app scenarios, two caps bumped.

Pinned from both ends: solid's cross-package-fields spec checks the
reserved fields survive mangling and scans every built client artifact for
a signals `_` field not on the list; web's server-owner-walks spec runs
`ownerPath` and `OBSERVE.exclude` over server owners against the built
observe and development artifacts.

Timing tripwires that flaked under parallel CI load: heap-mark re-measures
a round over the cap; attribution-navigation asserts against the wait
actually taken rather than a timer's nominal 10ms; both shell-gating specs
prove ordering by settling gates by hand (shell-gating-harness.ts) instead
of wall-clock deltas against timers.

RFC 08 gains "Values in records — the PII surface": every record and
finding field that carries user data, `data.error` on the server error
findings included, for exporters that leave the process.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cc0396b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
solid-js Patch
test-integration Patch
@solidjs/web Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
@solidjs/universal Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35122897082

Warning

No base build found for commit 59c4192 on next.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 71.46%

Details

  • Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 1018
Covered Lines: 772
Line Coverage: 75.83%
Relevant Branches: 790
Covered Branches: 520
Branch Coverage: 65.82%
Branches in Coverage %: Yes
Coverage Strength: 14.94 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Sep 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 175 untouched benchmarks


Comparing cross-package-fields (cc0396b) with next (59c4192)

Open in CodSpeed

@ryansolid
ryansolid merged commit d741d1e into next Sep 16, 2026
7 checks passed
ryansolid added a commit that referenced this pull request Sep 16, 2026
Regenerate RULES-INDEX after the reporter-liveness docs update and reconcile the combined Brotli limits after rebasing over #3496.

Co-authored-by: GPT-5.6 Sol via Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants