fix: handle constructor keys without crashing flag evaluation

[steipete]

What Problem This Solves

Fixes flag evaluation crashing after a segment named constructor is deleted, and missing attributes or SDK flags with inherited JavaScript names producing unintended values.

User Impact

Missing segments and attributes fall through to the next evaluation step, and missing SDK flags retain their code defaults. Explicitly supplied keys such as constructor, toString, and __proto__ still work. No configuration migration, dependency update, or SDK export change is needed.

Why This Change Was Made

Dictionary lookups previously treated inherited JavaScript properties as supplied data. The evaluator and SDK now require explicit entries, and remote-value maps preserve special keys safely. The change keeps the existing evaluation order and missing-value contracts.

Evidence

All ten new regression cases failed before the fix: six core cases, three React cases, and a real Worker/D1 lifecycle that returned HTTP 500 after deleting the constructor segment. They now pass.

The full local gate passed with Node 24.21.0 and pnpm 11.24.0: typecheck, tests (43 core, 36 React, 48 CLI, 129 Worker, 6 script tests), dashboard/demo/CLI builds, docs build, and packed SDK verification. Lint passed with six existing CSS warnings; Git-ignore integration excluded an unrelated ignored local proof artifact. The Worker suite printed two existing non-failing internal-error diagnostics.

The packed core SDK and public server-evaluator entry point were also loaded in a standalone consumer and exercised against a real synthetic local HTTP server:

Packed core: missing segments and attributes fall through; explicit values still match for all three names.
Packed SSR evaluator + real HTTP: defaults and explicit false values preserved for constructor, toString, and __proto__.

Independent Codex autoreview is clean through P2. These checks use synthetic data; production services were not contacted.

Exact-head verification for baa285b42f8ca961c007b5a7a8501d87aef7c1af: CI passed and CodeQL passed, with no reruns. All required checks are green. @vincentkoc has been requested for the required code-owner review; this PR remains open pending approval.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs changes before merge.

What this changes

This PR excludes inherited JavaScript properties from evaluation lookups while preserving explicitly supplied special keys.

Example: Delete the constructor segment, then evaluate theme for beta-user.

• Before: The missing segment resolves to an inherited function and evaluation returns HTTP 500.
• After: Evaluation returns HTTP 200 and falls through to the default variation.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A useful, focused repair with supplied package-entry-point runtime proof and meaningful regression coverage; the merge conflict is a workflow blocker.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): Contributor terminal output exercises the packed core SDK and public server evaluator against real local HTTP, confirming missing defaults and explicit values for all three special names; Worker/D1 lifecycle regression coverage supports deletion fallback.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Deleting a constructor segment can break evaluation, and absent special-name attributes or flags can produce unintended targeting or SDK values.
Reason: The focused repair restores documented missing-value behavior in existing shared owners without adding a capability or option.

Merge readiness

⛔ Needs changes before merge - 1 item remains

This PR fixes a concrete evaluation defect still present on current main; no actionable code findings remain, but GitHub reports a merge conflict.

Priority: P2
Reviewed head: baa285b42f8ca961c007b5a7a8501d87aef7c1af

Before merge

• Complete next step - Synchronize with current main, resolve the reported conflict while preserving both sets of Unreleased entries and current-main regression coverage, then validate the updated head.

Findings

None.

Agent review details

How this fits together

The shared core evaluator consumes flag configurations, segments, and caller attributes; the React SDK merges evaluation responses with application-defined defaults.

flowchart TD
  A[Stored flags and segments] --> C[Core evaluator]
  B[Caller context] --> C
  C --> D[Variation or default]
  D --> E[Public evaluation response]
  E --> G[SDK manifest merge]
  F[Application defaults] --> G
  G --> H[Application flag values]

Loading

Technical review

Best possible solution:

Land the focused shared lookup repair with its regression coverage and current-main changelog entries preserved.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically exposes the missing-constructor segment crash and inherited-value lookups, corroborated by reported before-fix failures; this review executed no tests.

Is this the best way to solve the issue?

Own-property checks in the shared owners and safe dictionary construction directly restore existing contracts without changing evaluation order, public exports, or configuration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against c6843b3efda4.

Provenance checked

• Core segment lookup and Worker lifecycle keeps the original intent (cbe5760: Reusable audiences require unknown or deleted segments to remain unmatched; lifecycle coverage includes deletion fallback.)
• Shared attribute lookup keeps the original intent (c06f4eb: Missing attributes must remain unmatched for every operator, including negation.)
• SDK manifest merging and remote parsing keeps the original intent (51df4d3: SSR evaluation merges only declared, type-compatible remote values over application defaults.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Verified introduced patch: The pinned diff adds own-property checks in shared core and SDK owners, safe remote dictionary construction, focused regression coverage, documentation, and an Unreleased entry. (packages/core/src/evaluator.ts:59, baa285b42f8c)
• Current-main defect: Main still directly indexes segments, attributes, and candidate flag values. A missing constructor segment reaches segment.contextKeys.includes through the inherited Object constructor. (packages/core/src/evaluator.ts:59, c6843b3efda4)
• Established targeting contracts: feat: LaunchDarkly-grade flag operations on Carapace #46 establishes deleted-segment fallback; feat: targeting rule operators (negation, string, numeric, semver, date) #54 establishes unmatched missing attributes, including negation. (packages/core/src/operators.ts:3, c06f4eb7da75)
• Established SDK contract: feat(react): add SSR flag bootstrap #43 establishes declared, type-compatible remote values over manifest defaults; the patch preserves that contract and both SDK modes. (packages/react/src/evaluation.ts:51, 51df4d3a2d9e)
• Supplied runtime proof: The full body, captured as sourceRevision 70498a9a04d3812a138c54322a510336f99b5d239c971b969cdbe4f1a330c0ad, records packed-core and public server-evaluator output using real local HTTP: missing defaults and explicit false values are preserved for constructor, toString, and proto. These are contributor observations; this review ran no target code or second reviewer. (packages/react/src/evaluation.ts:118, baa285b42f8c)
• Reported merge conflict: REST reports mergeable=false and mergeable_state=dirty for the reviewed head. Main and this PR insert Fixed entries at the same Unreleased location; the previous review's conflict-resolution action remains applicable. (CHANGELOG.md:7, baa285b42f8c)

Likely related people:

• Vincent Koc: Raw commit cbe5760 adds packages/core/src/evaluator.ts:12 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: cbe5760464f1; files: packages/core/src/evaluator.ts)
• Jesse Merhi: Raw commit 51df4d3 adds packages/react/src/evaluation.ts:51 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 51df4d3a2d9e; files: packages/react/src/evaluation.ts)

Review metrics

Metric	Value	Why it matters
Production and test delta	production +16/-10; tests +138/-75	The production repair is small; most test churn parameterizes the existing Worker lifecycle.

Labels

Label changes:

No label changes.

Label justifications:

• P2: Special property names can crash evaluation or select unintended values, with a bounded targeting and SDK blast radius.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (4 earlier review cycles)

• reviewed 2026-09-16T03:03:10.654Z sha baa285b :: needs maintainer review before merge. :: none
• reviewed 2026-10-02T07:37:18.890Z sha baa285b :: needs maintainer review before merge. :: none
• reviewed 2026-10-08T09:07:19.282Z sha baa285b :: blocked before merge. :: none
• reviewed 2026-10-11T11:26:16.771Z sha baa285b :: needs changes before merge. :: none

Reviewed October 12, 2026, 11:52 AM ET / 15:52 UTC (Revision 5).