STRATCONN-6806 - [Braze] - default sync mode to 'add' when missing - #3913
STRATCONN-6806 - [Braze] - default sync mode to 'add' when missing#3913monutwilio wants to merge 10 commits into
Conversation
…ta presets
The Order Placed/Checkout Started/Order Refunded/Order Cancelled/Product
Viewed (beta) presets never set __segment_internal_sync_mode in their
mapping, so syncMode resolved to undefined at runtime and every event
sent through these presets failed with "Invalid syncMode: undefined".
Adding the action's declared default ('add') directly to each preset
mapping ensures it's persisted when the subscription is created.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR fixes Braze ecommerce beta automatic presets failing due to a missing __segment_internal_sync_mode by explicitly injecting the default sync mode into the affected preset mappings.
Changes:
- Injects
__segment_internal_sync_mode: 'add'into 5 Braze ecommerce beta preset mappings. - Updates generated destination
metadata.jsonso newly created subscriptions persist the correct default mapping. - Minor formatting cleanup in
subscribeand mapping object literals.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/braze/metadata.json | Adds __segment_internal_sync_mode: "add" to the persisted preset mappings so the sync mode is present at subscription creation time. |
| packages/destination-actions/src/destinations/braze/index.ts | Injects __segment_internal_sync_mode: 'add' into the 5 ecommerce beta preset mappings to prevent syncMode from being undefined. |
The ecommerce/ecommerceSingleProduct actions' send() threw a hard error
whenever __segment_internal_sync_mode was missing from the mapping
(Invalid syncMode: undefined), instead of falling back to the action's
own declared default ('add'). This is the runtime-level fix that
immediately repairs every currently-affected subscription regardless of
how it was created (preset or manual mapping), independent of any
upstream fix to how presets/mappings get their sync mode populated.
A syncMode that's explicitly provided but genuinely unsupported by this
action (e.g. 'delete'/'mirror'/'upsert' from the broader framework-level
SyncMode type) still fails loudly, matching prior behavior -- only the
missing/empty case now defaults to 'add'.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…merce beta presets" This reverts commit 9d9db65.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (4)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:40
- The comment says the fallback to
'add'happens only whensyncModewasn't provided, but the implementation also falls back whensyncModeis an empty string ('') because it’s falsy. Either (a) update the comment to explicitly include blank/empty values as ‘missing’, or (b) change the condition to distinguishundefinedvs''so behavior matches the comment.
// 'add' is the action's declared default sync mode -- fall back to it only when syncMode wasn't
// provided at all. A syncMode that *was* explicitly provided but isn't one this action supports
// (e.g. 'delete'/'mirror'/'upsert' from the broader framework-level SyncMode type) is still a
// real error, not something to silently coerce into 'add'.
if (syncMode && !['update', 'add'].includes(syncMode)) {
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:55
- The PR description states this is a static preset-mapping data change (injecting
__segment_internal_sync_mode: 'add'into presets), but this diff also changes runtime behavior to default missing/blanksyncModeto'add'. Please update the PR description (or scope) to reflect this additional behavioral change so reviewers and release notes are accurate.
const resolvedSyncMode: SupportedSyncMode = syncMode === 'update' ? 'update' : 'add'
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:42
- This changes validation semantics: missing/blank
syncModeno longer errors, but unsupported values should still error. Add/adjust unit tests to assert that an invalid non-empty syncMode (e.g.'delete'/'upsert') still throws in single mode, and returns an index-wise 400 MultiStatusResponse in batch mode.
if (syncMode && !['update', 'add'].includes(syncMode)) {
const message = `Invalid syncMode: ${syncMode}. Supported sync modes are 'add' and 'update'.`
if (isBatch) {
packages/destination-actions/src/destinations/braze/ecommerce/tests/index.test.ts:513
- This test only proves the action doesn’t throw; it doesn’t assert the behavior is specifically defaulting to
'add'. Consider strengthening it by matching the posted request body (or by using a payload that does not setuser_alias, so_update_existing_onlybecomes a signal of'update'vs'add') to ensure the default syncMode path is actually exercised and correct.
it('should default syncMode to add if missing', async () => {
nock(settings.endpoint).post('/users/track').reply(200)
const response = await testDestination.testAction('ecommerce', {
event: payload,
settings,
useDefaultMappings: true,
mapping: {
...mapping,
__segment_internal_sync_mode: ''
}
})
expect(response.length).toBe(1)
})
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:40
- The comment says the fallback to
'add'happens “only when syncMode wasn't provided at all”, but the implementation also falls back whensyncModeis an empty string (and any other falsy value). Please align the comment with actual behavior (e.g., “missing or empty”), or change the condition to distinguish “not provided” from “provided but empty” if that distinction matters.
// 'add' is the action's declared default sync mode -- fall back to it only when syncMode wasn't
// provided at all. A syncMode that *was* explicitly provided but isn't one this action supports
// (e.g. 'delete'/'mirror'/'upsert' from the broader framework-level SyncMode type) is still a
// real error, not something to silently coerce into 'add'.
if (syncMode && !['update', 'add'].includes(syncMode)) {
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:55
- The updated tests cover the missing/empty
syncModedefault-to-'add'behavior, but I don’t see coverage in these diffs that asserts an explicitly unsupportedsyncMode(e.g.'upsert') still fails loudly (single + batch). Adding a unit test for an unsupported non-empty__segment_internal_sync_modewould lock in the intended contract described in the PR.
const resolvedSyncMode: SupportedSyncMode = syncMode === 'update' ? 'update' : 'add'
| payloadsWithIndexes.forEach((payload, index) => { | ||
|
|
||
| const error = errors.find(e => e.index === index) | ||
| const error = errors.find((e) => e.index === index) | ||
|
|
||
| if(error){ | ||
| if (error) { | ||
| msResponse.setErrorResponseAtIndex(index, { |
There was a problem hiding this comment.
Existing issue, needs to be addressed in a different PR. cc @joe-ayoub-segment
There was a problem hiding this comment.
Confirmed this is pre-existing (traces back to STRATCONN-6034, unrelated to this fix — just surfaced here because Prettier reformatted the surrounding lines). Filed as a separate ticket: STRATCONN-6937.
… into STRATCONN-6806/syncmode # Conflicts: # packages/destination-actions/src/destinations/braze/ecommerce/__tests__/index.test.ts # packages/destination-actions/src/destinations/braze/ecommerceSingleProduct/__tests__/index.test.ts
…RATCONN-6806/syncmode
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/pendo-audiences/index.ts:17
- This PR is scoped (per title/description) to a Braze
syncModeruntime fix, but it also includes unrelated formatting-only changes in Pendo Audiences (and other files). To keep the change set focused and easier to review/roll back, consider reverting these unrelated reformatting hunks or moving them into a separate PR.
description: 'Your Pendo Integration Key. Found in Pendo under Settings > Integrations > Integration Keys.',
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:40
- The comment says the fallback happens only when
syncMode“wasn't provided at all”, but the implementation also treats the empty string as missing because''is falsy. If the intended behavior is “missing or empty defaults to 'add'” (as described in the PR description/tests), update the comment to match; otherwise adjust the condition to only treatundefinedas missing.
// 'add' is the action's declared default sync mode -- fall back to it only when syncMode wasn't
// provided at all. A syncMode that *was* explicitly provided but isn't one this action supports
// (e.g. 'delete'/'mirror'/'upsert' from the broader framework-level SyncMode type) is still a
// real error, not something to silently coerce into 'add'.
if (syncMode && !['update', 'add'].includes(syncMode)) {
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:595
currencies()appears deterministic (no inputs) but recomputes a largeSetand allocates a new array on every call. If this function is called repeatedly at runtime, consider hoisting the computedunique.map(...)result to a module-level constant (or memoizing) so it’s computed once.
const unique = Array.from(new Set(codes))
return unique.map((code) => ({
label: code,
value: code
}))
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:53
resolvedSyncMode’s fallback toSUPPORTED_SYNC_MODES.ADDis currently dead code because the function throws for!isValidSyncModejust above. After adjusting validation to allow missing/emptysyncMode, consider simplifying the control flow so there’s a clear split between (1) defaulting when missing and (2) throwing when unsupported.
const resolvedSyncMode: SupportedSyncMode = isValidSyncMode ? (syncMode as SupportedSyncMode) : SUPPORTED_SYNC_MODES.ADD
packages/destination-actions/src/destinations/braze/ecommerce/index.ts:9
- This import formatting (
{ SUPPORTED_SYNC_MODES}) is inconsistent with the rest of the file and likely will be flagged by lint/prettier. Update it to match the project’s standard spacing.
import { SUPPORTED_SYNC_MODES} from './constants'
packages/destination-actions/src/destinations/pendo-audiences/syncAudience/fields.ts:26
- This PR is described as a Braze
syncModeruntime fix, but it also includes several formatting-only changes in unrelated destinations (Pendo Audiences, MS Bing Ads). Consider splitting these formatting changes into a separate PR to keep the Braze fix reviewable and reduce risk/noise in the deploy.
description:
'When enabled, events are batched and sent to Pendo using the batch patch endpoint (up to 1000 visitors per request).',
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:33
- Given the updated syncMode behavior, there should be explicit unit coverage asserting that an unsupported but present
syncModevalue (e.g.'delete') still fails (non-batch throwsPayloadValidationError; batch returns per-item 400s). Current added/updated tests cover the missing/empty case but don’t appear (in this diff) to cover the “present but invalid” case, which is easy to regress—especially with the new coercion logic.
export async function send(
request: RequestClient,
payloads: (Payload | SingleProductPayload)[],
settings: Settings,
isBatch: boolean,
syncMode?: SyncMode
) {
packages/destination-actions/src/destinations/pendo-audiences/index.ts:81
- The PR description is scoped to the Braze ecommerce syncMode runtime fix, but this PR also includes unrelated formatting-only changes in Pendo Audiences and Microsoft Bing Ads Audience utilities/tests. This adds review/merge noise and makes it harder to audit the functional change. Suggestion (optional): split purely formatting changes into a separate PR, or explicitly call out these non-functional edits in the PR description so the change set matches expectations.
const {
externalId,
settings: { region }
} = getAudienceInput
packages/destination-actions/src/destinations/braze/ecommerce/index.ts:9
- There’s a spacing issue in the import (
{ SUPPORTED_SYNC_MODES}) that may fail linting/format checks in repos enforcing consistent formatting. Suggestion: run the formatter (or adjust spacing) to match the surrounding import style.
import { SUPPORTED_SYNC_MODES} from './constants'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
packages/destination-actions/src/destinations/braze/ecommerce/tests/index.test.ts:500
- The tests now cover the missing-syncMode defaulting path, but there is no longer coverage ensuring that an explicitly provided unsupported
syncModevalue still fails (which the PR description states should remain unchanged). Please add a test that sets__segment_internal_sync_mode(or passessyncMode) to an unsupported value (e.g.'delete') and asserts the same error behavior as before (throw for non-batch; per-index error for batch).
it('should default syncMode to add if missing', async () => {
nock(settings.endpoint).post('/users/track').reply(200)
packages/destination-actions/src/destinations/braze/ecommerce/index.ts:9
- There’s a minor import formatting issue (missing space before
}) that may fail linting/format checks. Update to match the surrounding import style.
import { SUPPORTED_SYNC_MODES} from './constants'
packages/destination-actions/src/destinations/pendo-audiences/index.ts:81
- This PR is described as a Braze runtime fix, but it also contains multiple formatting-only changes in unrelated destinations (Pendo Audiences, MS Bing Ads Audiences). To keep the change focused and reduce review/merge risk, consider moving the unrelated formatting-only edits into a separate PR (or reverting them here).
const {
externalId,
settings: { region }
} = getAudienceInput
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/destination-actions/src/destinations/braze/ecommerce/index.ts:9
- There’s a missing space before the closing brace in the import (
{ SUPPORTED_SYNC_MODES}). This is likely to fail lint/format checks in repos enforcing consistent import formatting; please adjust to match the surrounding style.
import { SUPPORTED_SYNC_MODES} from './constants'
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:37
- The PR description says that once
send()runs, any presentsyncModevalue is trusted as'add'/'update'without redundant runtime validation; however this code still validates againstSUPPORTED_SYNC_MODESand silently defaults invalid values to'add'. Either (a) simplify to only default when missing/empty (aligning implementation with the description), or (b) keep this guard but update the PR description to reflect that invalid values are also coerced to'add'.
const isValidSyncMode = syncMode && (Object.values(SUPPORTED_SYNC_MODES) as SyncMode[]).includes(syncMode)
const resolvedSyncMode: SupportedSyncMode = isValidSyncMode ? (syncMode as SupportedSyncMode) : SUPPORTED_SYNC_MODES.ADD
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:37
- This logic defaults to
ADDnot only whensyncModeis missing/falsy, but also when it is present and invalid (e.g. a corrupted mapping value). The PR description claims explicit invalid values behave exactly as before, but this would silently proceed as'add'instead of erroring. If you want to preserve prior behavior, consider defaulting only when!syncMode(orsyncModeis empty) and throwing/reporting an error when a non-empty value is not inSUPPORTED_SYNC_MODES.
const isValidSyncMode = syncMode && (Object.values(SUPPORTED_SYNC_MODES) as SyncMode[]).includes(syncMode)
const resolvedSyncMode: SupportedSyncMode = isValidSyncMode ? (syncMode as SupportedSyncMode) : SUPPORTED_SYNC_MODES.ADD
packages/destination-actions/src/destinations/braze/ecommerce/functions.ts:36
syncMode && includes(syncMode)can yield a non-boolean value at runtime (e.g.,''results in''), and forces a cast onObject.values(...). Consider making this a strict boolean expression (e.g., compute the allowed values once and useincludes(syncMode ?? '')) to improve type-safety/readability and avoid repeatingObject.values(...)on every call.
const isValidSyncMode = syncMode && (Object.values(SUPPORTED_SYNC_MODES) as SyncMode[]).includes(syncMode)
packages/destination-actions/src/destinations/braze/ecommerce/index.ts:9
- There is a spacing issue in the import (
{ SUPPORTED_SYNC_MODES}), which can fail lint/prettier checks in stricter configs. Adjust to{ SUPPORTED_SYNC_MODES }for consistent formatting.
import { SUPPORTED_SYNC_MODES} from './constants'
Fixes STRATCONN-6806: the Braze
Order Placed (Beta)action (and the other beta ecommerce presets — Checkout Started, Order Refunded, Order Cancelled, Product Viewed) failed withInvalid syncMode: undefined. Supported sync modes are 'add' and 'update'.Root cause
__segment_internal_sync_modenever made it into the mapping for these subscriptions — traced through the full pipeline (preset generation in this repo, the internalaction-cli pushtool,control-plane, and the app's mapping-editor frontend): the value is only ever written by a specific UI interaction that, for these presets, never fires. A companion fix inaction-cli(https://github.com/segmentio/action-cli/pull/271) stops newly-pushed presets from being born without the key, but that's prospective only — it can't repair subscriptions that already exist (including the two customers named in the ticket, who are failing today).This PR
ecommerce/functions.ts'ssend()— shared by both theecommerceandecommerceSingleProductactions — threw a hard error wheneversyncModewas missing, instead of falling back to the action's own declared default ('add'). This is the runtime-level fix: it repairs every affected subscription immediately on deploy, regardless of how or when it was created, with no dependency oncontrol-planestate, a re-push, or any customer action.syncModeis validated when a mapping/subscription is created — the action'ssyncMode.choicesrestricts it to'add'/'update'— so by the timesend()runs, the value is either one of those two or missing/empty entirely. The resolution logic reflects that: any falsy value defaults to'add'; anything present is trusted as'add'/'update'without a redundant runtime check.An earlier version of this PR attempted the fix by hardcoding
__segment_internal_sync_mode: 'add'into the 5 affected presets' mappings directly inbraze/index.ts. That approach is reverted in this PR (see the revert commit) — it failsyarn validate, since preset mapping keys are validated against the action's declaredfields, andsyncModeisn't a field.Also folds in 3 follow-up commits from @joeayoub-segment refactoring the valid sync-mode values into a shared
SUPPORTED_SYNC_MODESconstant (ecommerce/constants.ts) used by bothfunctions.tsandindex.ts, and dropping the now-redundant explicit-invalid-value guard described above.Testing
'add'success behavior instead, and added equivalent coverage forecommerceSingleProduct(which shares the samesend()and previously had none for this path).Tested in staging, it defaults to syncMode
addwhen not selected.Screen.Recording.2026-08-05.at.5.43.35.PM.mov
Note:
TZ=UTC yarn cloud jest --testPathPatterns="braze"could not be run in the dev sandbox due to a pre-existing, unrelated broken Jest install (jest-runtimethrowsTypeError: Cannot read properties of undefined (reading 'bind'), reproduced identically on an untouched destination, andyarn installcan't reach the registry from this sandbox to attempt a fix).tsc --noEmitpasses cleanly. Please confirm the Jest suite is green in CI before merging.Security Review
type: 'password'— no field definitions changed in this PR.New Destination Checklist
N/A — not a new destination.
🤖 Generated with Claude Code