Skip to main content

max / makenotwork

audit Run 16 Phase 2: Data integrity / Types - enums: two shipped enums (ModerationActionType, CheckoutType) derived serde without rename_all, so serde emitted PascalCase while the DB/ Display form is snake_case — a latent mismatch the moment either lands in a JSON body. Add #[serde(rename_all = "snake_case")] to both and a generic test that drives every serde-deriving impl_str_enum! enum through VARIANTS and asserts serde == Display both ways, so future drift fails at test time instead of at the first poisoned read. - synckit app-sub checkout: the billing interval was written into a Stripe metadata key named `tier` and read back as the interval — internally consistent but mislabeled. Rename the key to `interval` on both sides, drop the redundant SynckitAppSubCheckoutParams.tier field, and keep a legacy `tier` fallback on read so sessions in flight across the deploy still settle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-02 13:27 UTC
Commit: 95a103ef7996e5718e531013c014bb2dbddf7dcb
Parent: fb85bab
5 files changed, +126 insertions, -11 deletions
M server/src/db/enums.rs +79
@@ -1022,6 +1022,7 @@ impl_str_enum!(ImportJobStatus {
1022 1022 // -- Moderation action types --------------------------------------------------
1023 1023
1024 1024 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
1025 + #[serde(rename_all = "snake_case")]
1025 1026 pub enum ModerationActionType {
1026 1027 Warning,
1027 1028 Suspension,
@@ -1040,6 +1041,7 @@ impl_str_enum!(ModerationActionType {
1040 1041
1041 1042 /// Discriminator for checkout session types stored in Stripe metadata.
1042 1043 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
1044 + #[serde(rename_all = "snake_case")]
1043 1045 pub enum CheckoutType {
1044 1046 Guest,
1045 1047 Subscription,
@@ -1605,4 +1607,81 @@ mod tests {
1605 1607 assert_eq!("content_removal".parse::<ModerationActionType>().unwrap(), ModerationActionType::ContentRemoval);
1606 1608 assert!("bogus".parse::<ModerationActionType>().is_err());
1607 1609 }
1610 +
1611 + /// Every `impl_str_enum!` enum that also derives serde must agree on its wire
1612 + /// string across BOTH representations: the macro's Display/FromStr/sqlx form
1613 + /// (`VARIANTS`) and serde's JSON form. These are defined in two places (the
1614 + /// macro literal and the `#[serde(rename_all = ...)]` attribute), so they can
1615 + /// silently diverge — a variant travels via Display today and via serde
1616 + /// tomorrow, and the string disagrees with what is already in Postgres. This
1617 + /// test drives every serde-deriving enum through its `VARIANTS` (which round
1618 + /// through `FromStr`) and asserts serde emits the identical string, in both
1619 + /// directions. Add every serde-deriving enum here.
1620 + macro_rules! assert_serde_matches_wire {
1621 + ($($enum:ty),+ $(,)?) => {{
1622 + $(
1623 + for wire in <$enum>::VARIANTS {
1624 + let value: $enum = wire.parse().unwrap_or_else(|e| {
1625 + panic!("{}: VARIANTS string {wire:?} does not parse: {e}", stringify!($enum))
1626 + });
1627 + let json = serde_json::to_string(&value).expect("serialize");
1628 + assert_eq!(
1629 + json,
1630 + format!("\"{wire}\""),
1631 + "{}: serde emits {json} but the DB/Display wire string is {wire:?} \
1632 + (add or fix `#[serde(rename_all = ...)]`)",
1633 + stringify!($enum)
1634 + );
1635 + let back: $enum = serde_json::from_str(&json).unwrap_or_else(|e| {
1636 + panic!("{}: serde cannot round-trip {json}: {e}", stringify!($enum))
1637 + });
1638 + assert_eq!(
1639 + back.to_string(),
1640 + *wire,
1641 + "{}: serde deserialize of {json} disagrees with Display",
1642 + stringify!($enum)
1643 + );
1644 + }
1645 + )+
1646 + }};
1647 + }
1648 +
1649 + #[test]
1650 + fn serde_repr_matches_db_wire_repr_for_every_enum() {
1651 + assert_serde_matches_wire!(
1652 + DiscountType,
1653 + CodePurpose,
1654 + WaitlistStatus,
1655 + SelectionMethod,
1656 + TransactionStatus,
1657 + FollowTargetType,
1658 + SubscriptionStatus,
1659 + SyncBillingStatus,
1660 + SyncEnforcementMode,
1661 + Visibility,
1662 + ProjectRole,
1663 + SyncOperation,
1664 + SyncPlatform,
1665 + FileScanStatus,
1666 + InsertionPosition,
1667 + AppealDecision,
1668 + DiscoverSort,
1669 + ItemType,
1670 + IssueStatus,
1671 + ReportTargetType,
1672 + ReportType,
1673 + ReportStatus,
1674 + CreatorTier,
1675 + AiTier,
1676 + ProjectFeature,
1677 + ProjectType,
1678 + BuildStatus,
1679 + PricingKind,
1680 + MailingListType,
1681 + ImportSource,
1682 + ImportJobStatus,
1683 + ModerationActionType,
1684 + CheckoutType,
1685 + );
1686 + }
1608 1687 }
M server/src/payments/checkout.rs +1 -2
@@ -103,7 +103,6 @@ pub struct SynckitAppSubCheckoutParams<'a> {
103 103 pub interval: &'a str,
104 104 pub user_id: UserId,
105 105 pub app_id: SyncAppId,
106 - pub tier: &'a str,
107 106 pub storage_limit_bytes: Option<i64>,
108 107 pub success_url: &'a str,
109 108 pub cancel_url: &'a str,
@@ -459,7 +458,7 @@ impl StripeClient {
459 458 metadata.insert("checkout_type".to_string(), CheckoutType::SynckitAppSub.to_string());
460 459 metadata.insert("user_id".to_string(), p.user_id.to_string());
461 460 metadata.insert("app_id".to_string(), p.app_id.to_string());
462 - metadata.insert("tier".to_string(), p.tier.to_string());
461 + metadata.insert("interval".to_string(), p.interval.to_string());
463 462 if let Some(bytes) = p.storage_limit_bytes {
464 463 metadata.insert("storage_limit_bytes".to_string(), bytes.to_string());
465 464 }
M server/src/payments/checkout_metadata.rs +45 -4
@@ -180,12 +180,14 @@ pub fn is_synckit_app_sub_checkout(meta: Option<&CheckoutMetaMap>) -> bool {
180 180 get_checkout_type(meta) == Some(CheckoutType::SynckitAppSub)
181 181 }
182 182
183 - /// Parsed metadata for an end-user subscribing to an app's cloud sync.
183 + /// Parsed metadata for an end-user subscribing to an app's cloud sync. The
184 + /// billing cadence is carried as `interval` ("monthly"/"annual"); a SyncKit app
185 + /// sub has no creator-tier concept.
184 186 #[derive(Debug)]
185 187 pub struct SynckitAppSubCheckoutMetadata {
186 188 pub user_id: UserId,
187 189 pub app_id: SyncAppId,
188 - pub tier: String,
190 + pub interval: String,
189 191 pub storage_limit_bytes: Option<i64>,
190 192 }
191 193
@@ -193,11 +195,18 @@ impl SynckitAppSubCheckoutMetadata {
193 195 pub fn from_metadata(meta: Option<&CheckoutMetaMap>) -> Result<Self> {
194 196 let user_id: UserId = parse_uuid_to(require(meta, "user_id")?, "user_id")?;
195 197 let app_id: SyncAppId = parse_uuid_to(require(meta, "app_id")?, "app_id")?;
196 - let tier = require(meta, "tier")?.clone();
198 + // Read the billing cadence from `interval`, falling back to the legacy
199 + // `tier` key that older checkout sessions (created before this key was
200 + // corrected) still carry — so a session in flight across the deploy
201 + // still settles.
202 + let interval = match meta.and_then(|m| m.get("interval")) {
203 + Some(v) => v.clone(),
204 + None => require(meta, "tier")?.clone(),
205 + };
197 206 let storage_limit_bytes = meta
198 207 .and_then(|m| m.get("storage_limit_bytes"))
199 208 .and_then(|v| v.parse::<i64>().ok());
200 - Ok(SynckitAppSubCheckoutMetadata { user_id, app_id, tier, storage_limit_bytes })
209 + Ok(SynckitAppSubCheckoutMetadata { user_id, app_id, interval, storage_limit_bytes })
201 210 }
202 211 }
203 212
@@ -209,6 +218,38 @@ mod tests {
209 218 entries.iter().map(|(k, v)| (k.to_string(), v.to_string())).collect()
210 219 }
211 220
221 + // --- SynckitAppSubCheckoutMetadata: interval key + legacy `tier` fallback ---
222 +
223 + #[test]
224 + fn synckit_app_sub_reads_interval_key() {
225 + let user = UserId::new();
226 + let app = SyncAppId::new();
227 + let m = meta_of(&[
228 + ("user_id", &user.to_string()),
229 + ("app_id", &app.to_string()),
230 + ("interval", "annual"),
231 + ("storage_limit_bytes", "1073741824"),
232 + ]);
233 + let parsed = SynckitAppSubCheckoutMetadata::from_metadata(Some(&m)).unwrap();
234 + assert_eq!(parsed.interval, "annual");
235 + assert_eq!(parsed.storage_limit_bytes, Some(1073741824));
236 + }
237 +
238 + #[test]
239 + fn synckit_app_sub_falls_back_to_legacy_tier_key() {
240 + // Sessions created before the key was corrected carry the cadence under
241 + // `tier`; those must still settle after the deploy.
242 + let user = UserId::new();
243 + let app = SyncAppId::new();
244 + let m = meta_of(&[
245 + ("user_id", &user.to_string()),
246 + ("app_id", &app.to_string()),
247 + ("tier", "monthly"),
248 + ]);
249 + let parsed = SynckitAppSubCheckoutMetadata::from_metadata(Some(&m)).unwrap();
250 + assert_eq!(parsed.interval, "monthly");
251 + }
252 +
212 253 // --- CheckoutMetadata ---
213 254
214 255 #[test]
M server/src/routes/stripe/webhook/checkout.rs +1 -1
@@ -777,7 +777,7 @@ pub(super) async fn handle_synckit_app_sub_checkout_completed(
777 777 app_id: meta.app_id,
778 778 stripe_subscription_id: &stripe_subscription_id,
779 779 stripe_customer_id: &stripe_customer_id,
780 - interval: &meta.tier, // metadata "tier" carries the interval string ("monthly"/"annual")
780 + interval: &meta.interval,
781 781 storage_limit_bytes: meta.storage_limit_bytes.unwrap_or(0),
782 782 },
783 783 )
M server/src/routes/synckit/sync.rs -4
@@ -380,10 +380,6 @@ pub(super) async fn create_subscription_checkout(
380 380 interval: interval.as_str(),
381 381 user_id: sync_user.user_id,
382 382 app_id: sync_user.app_id,
383 - // NOTE: `tier` carries the interval string here, matching the prior
384 - // positional call. Looks suspect (tier vs interval) but preserved
385 - // verbatim by this refactor — review separately, don't silently change.
386 - tier: interval.as_str(),
387 383 storage_limit_bytes: Some(req.cap_bytes),
388 384 success_url: &success_url,
389 385 cancel_url: &cancel_url,