Skip to content

Fix quickstart consumer smoke negative control to test missing Targeting Key - #608

Merged
isuttell merged 1 commit into
mainfrom
cursor/fix-quickstart-smoke-targeting-key-4925
Sep 16, 2026
Merged

isuttell merged 1 commit into
mainfrom
cursor/fix-quickstart-smoke-targeting-key-4925

Conversation

@isuttell

@isuttell isuttell commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

The packed SDK consumer smoke treated any TypeScript failure as proof that omitting targetingKey is rejected. The stale-snippet transform left a dangling comma (evaluate(..., {, })), so the negative control could pass for a syntax error instead of the required Targeting Key contract.

Changes

  • stripTargetingKeyFromSnippet now removes targetingKey: userId without leaving a dangling comma, and leaves optional neighbors such as idempotencyKey intact.
  • The consumer-smoke negative control requires TypeScript to report that targetingKey is missing. Object-literal syntax errors and a successful typecheck (optional Targeting Key) fail the guard.
  • Helper unit tests cover evaluate and evaluateDetails call shapes, syntax validity, and the optional-targetingKey mutation.

Risk: LOW

  • Areas touched: SDK consumer-smoke helpers and their unit tests only
  • Security: no credentials, Targeting Keys, or customer data added to artifacts or logs
  • Performance: none
  • Breaking: none; public evaluate contract and optional evaluation idempotency keys are unchanged

Test plan

  • node --test packages/sdk/scripts/extract-quickstart-snippet.test.mjs
  • pnpm --filter @splitch/sdk test (306 vitest + 19 node:test passed)
  • pnpm --filter @splitch/sdk typecheck
  • pnpm --filter @splitch/sdk test:consumer-smoke (positive packed smoke + stale targetingKey guard)
  • pnpm verify:push

Issue: SPL-626

Linear Issue: SPL-626

Open in Web Open in Cursor 

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Keep the stale quickstart snippet syntactically valid when stripping
targetingKey, and require TypeScript to report that required property
instead of treating any typecheck failure as a pass.

Issue: SPL-626

Co-authored-by: Isaac Suttell <isaac@zaks.io>
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. To trigger a review, include @coderabbitai review in the PR description. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 70921e8e-bc42-4612-89f9-930c92e68e71

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@isuttell
isuttell marked this pull request as ready for review September 16, 2026 17:18
@isuttell
isuttell merged commit bbfade0 into main Sep 16, 2026
5 of 6 checks passed
@isuttell
isuttell deleted the cursor/fix-quickstart-smoke-targeting-key-4925 branch September 16, 2026 17:48
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