Skip to main content

max / makenotwork

Cover the pure money and data logic, and freeze the rest from growing Nine files on the money and user-data paths had no test of any kind and now have one each, covering every non-async, non-database function they contain. That is the whole of what unit tests can reach here: only 13 of the 50 untested files hold a single pure function, so the remainder is contract-test work and is filed as GoingsOn problems rather than pretended at. What the tests pin, in rough order of what it would cost to have wrong: the Stripe minimum-charge boundary is inclusive at 50c and its refusal formats as "$0.50" because a buyer reads it; line items pin quantity and never carry both a Price and inline price_data; SyncKit idempotency keys are deterministic per app, which is the only thing stopping two racing activates from creating a second billable subscription; an unknown Stripe subscription status is a no-op rather than an Err, which is what stops a retry storm when Stripe adds one; API keys hash to a pinned SHA-256 vector, so swapping the algorithm fails rather than merely producing a different 64-char string; `same-site` is refused on subscription changes because a subdomain is not us; and an unsubscribed SyncKit user gets 402, not 403, because the client distinguishes "pay us" from "give up" by the code alone. Two tests document behaviour rather than asserting it is right, and both are filed for a decision. check_min_charge guards `> 0 && < minimum`, so negative amounts pass. And parse_account_id / parse_subscription_id validate nothing at all: the stripe crate generates its id types with `type Err = Infallible`, so every input parses and both `Invalid Stripe ... ID` branches are unreachable. An empty `users.stripe_account_id` becomes an empty connected-account header on a live charge. That one was found by writing the test that asserted the opposite and watching it fail. tests/untested_money_paths.rs freezes the count at 41, down from 50. It counts test attributes rather than `#[cfg(test)]` blocks, because db/subscriptions.rs hid 811 lines behind a `#[cfg(test)]` holding one test-only constructor and no test, and counting modules is what hid it. .cargo/mutants.toml gains the nine files, per its own rule that a file joins the scope when it gains its first test: 67 files, 2,879 mutants.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-04 23:16 UTC
Signed with PGP, not checked
Commit: 052f553ab60717d0947df8ba2414c24d360ccd89
Parent: 1e71000
11 files changed, +649 insertions, -21 deletions
M server/.cargo/mutants.toml +27 -11
@@ -2,8 +2,8 @@
2 2 #
3 3 # Not a tuning knob. Unscoped, this crate generates 9,766 mutants and each one
4 4 # re-runs the test suite, so a single pass is roughly six days of machine. It
5 - # has therefore never completed, which meant no mutation data at all for the
6 - # largest thing we ship. Scoping is what makes the check exist.
5 + # had consequently never completed once, which meant no mutation data at all for
6 + # the largest thing we ship. Scoping is what makes the check exist.
7 7 #
8 8 # Two rules decide what is in here, and both are deliberate.
9 9 #
@@ -14,50 +14,66 @@
14 14 #
15 15 # 2. Only files that already have tests. Mutating a file with no test reports
16 16 # every mutant as a survivor, which is noise at a volume that buries the real
17 - # findings, and it answers a question `grep` answers for free. 50 files in
18 - # the money and user-data groups have no test at all, including
17 + # findings, and it answers a question `grep` answers for free. 41 files in the
18 + # money and user-data groups still have no test at all, among them
19 19 # db/transactions/purchases.rs (1,375 lines), db/users.rs (1,269) and
20 - # routes/stripe/webhook/checkout.rs (1,024). That is a coverage gap to close,
21 - # not a mutation result to compute; the list is in wiki `testing-posture`.
20 + # routes/stripe/webhook/checkout.rs (1,024). Those are filed as GoingsOn
21 + # problems against mnw-server and frozen by tests/untested_money_paths.rs;
22 + # they are a coverage gap to close, not a mutation result to compute.
23 + #
24 + # Expect survivors in the async half of the files added on 2026-08-04. Their
25 + # pure helpers gained unit tests that day; the Stripe and database paths around
26 + # them did not, and those are the filed problems above rather than news. Prune
27 + # this list by function only if that noise proves louder than the signal.
22 28 #
23 29 # Add a file here when it gains its first test, rather than when it is written.
24 - # Method and the untested list: wiki `testing-posture`.
30 + # Method, tiers and the untested list: wiki `testing-posture`.
25 31
26 32 examine_globs = [
27 33
28 34 # --- Money. A survivor here is a way the ledger can be wrong without any
29 35 # test noticing.
30 - # 14 files, 7,370 lines, 230 tests.
36 + # 20 files, 10,397 lines, 256 tests.
31 37 "src/db/creator_tiers/mod.rs",
32 38 "src/db/creator_tiers/storage_quota.rs",
33 39 "src/db/creator_tiers/subscriptions.rs",
34 40 "src/db/promo_codes.rs",
35 41 "src/helpers/billing.rs",
42 + "src/payments/checkout.rs",
36 43 "src/payments/checkout_metadata.rs",
37 44 "src/payments/connect.rs",
45 + "src/payments/fan_ops.rs",
46 + "src/payments/mod.rs",
38 47 "src/payments/synckit_app_pricing.rs",
48 + "src/payments/synckit_billing.rs",
39 49 "src/payments/webhooks.rs",
40 50 "src/pricing.rs",
41 51 "src/pricing_comparison.rs",
52 + "src/routes/stripe/checkout/subscriptions.rs",
42 53 "src/routes/stripe/webhook/checkout_helpers.rs",
54 + "src/routes/stripe/webhook/subscriptions.rs",
43 55 "src/synckit_billing.rs",
44 56 "src/tier_prices.rs",
45 57
46 58 # --- User data. A survivor here is a way a creator's files, sync state or
47 59 # account can be corrupted or lost silently.
48 - # 9 files, 6,780 lines, 147 tests.
60 + # 13 files, 8,619 lines, 155 tests.
61 + "src/db/pending_s3_deletions.rs",
62 + "src/db/synckit/apps.rs",
49 63 "src/import/csv_converter.rs",
50 64 "src/import/mod.rs",
51 65 "src/import/pipeline.rs",
66 + "src/routes/storage/gallery.rs",
52 67 "src/routes/storage/media.rs",
53 68 "src/routes/storage/mod.rs",
54 69 "src/routes/synckit/billing.rs",
70 + "src/routes/synckit/blobs.rs",
55 71 "src/routes/synckit/keys.rs",
56 72 "src/routes/synckit/mod.rs",
57 73 "src/storage.rs",
58 74
59 - # --- Auth, access control, input validation and scanning. Every file in
60 - # this group already has tests; mutation asks if they are strong enough.
75 + # --- Auth, access control, input validation and scanning. Every file on this
76 + # boundary has tests; mutation asks whether they are strong enough.
61 77 # 34 files, 18,514 lines, 593 tests.
62 78 "src/access_gate.rs",
63 79 "src/auth.rs",
M server/src/payments/checkout.rs +109
@@ -505,3 +505,112 @@
505 505 .await
506 506 }
507 507 }
508 +
509 + #[cfg(test)]
510 + mod tests {
511 + //! The shape of what we ask Stripe to charge. Everything here is pure and
512 + //! sits directly on the money path: a wrong `unit_amount`, a missing
513 + //! `quantity`, or a minimum-charge boundary off by one cent is a real
514 + //! charge that is wrong, and none of it was covered.
515 +
516 + use super::*;
517 +
518 + // ── the Stripe per-transaction minimum ──
519 +
520 + #[test]
521 + fn a_free_item_is_allowed_through() {
522 + // $0 items are legitimate; callers gate them before they reach Stripe.
523 + assert!(check_min_charge(0).is_ok());
524 + }
525 +
526 + #[test]
527 + fn the_minimum_itself_is_allowed_and_one_cent_under_is_not() {
528 + let min = constants::STRIPE_MINIMUM_CHARGE_CENTS;
529 + assert!(check_min_charge(min).is_ok(), "the boundary is inclusive");
530 + assert!(
531 + matches!(check_min_charge(min - 1), Err(AppError::BadRequest(_))),
532 + "one cent under the minimum must be refused before Stripe refuses it"
533 + );
534 + assert!(check_min_charge(min + 1).is_ok());
535 + }
536 +
537 + #[test]
538 + fn the_rejection_names_the_minimum_in_dollars() {
539 + // The message reaches a buyer, so it must not say "50".
540 + let Err(AppError::BadRequest(msg)) = check_min_charge(1) else {
541 + panic!("1 cent should be rejected");
542 + };
543 + assert!(
544 + msg.contains("$0.50"),
545 + "buyer-facing message should format the minimum as currency: {msg}"
546 + );
547 + }
548 +
549 + #[test]
550 + fn a_negative_amount_is_not_rejected_here() {
551 + // Documenting the current contract rather than endorsing it: the guard
552 + // is `> 0 && < minimum`, so negatives pass. Every caller computes its
553 + // amount from a price and a discount, and none is proven non-negative
554 + // here. If a discount is ever allowed to exceed a price, this is the
555 + // gate that will not catch it.
556 + assert!(check_min_charge(-1).is_ok());
557 + }
558 +
559 + // ── line items ──
560 +
561 + #[test]
562 + fn an_inline_line_item_charges_the_given_amount_once_in_usd() {
563 + let item = build_inline_line_item("A Record", 2500);
564 + let price = item.price_data.expect("inline items carry price_data");
565 + assert_eq!(price.unit_amount, Some(2500));
566 + assert_eq!(price.currency, Currency::USD);
567 + assert_eq!(
568 + item.quantity,
569 + Some(1),
570 + "quantity must be pinned: None would let Stripe default and charge differently"
571 + );
572 + assert!(
573 + item.price.is_none(),
574 + "an inline item must not also reference a Stripe Price"
575 + );
576 + }
577 +
578 + #[test]
579 + fn a_price_line_item_references_stripe_and_sets_no_amount_of_its_own() {
580 + let item = build_price_line_item("price_123");
581 + assert_eq!(item.price.as_deref(), Some("price_123"));
582 + assert_eq!(item.quantity, Some(1));
583 + assert!(
584 + item.price_data.is_none(),
585 + "a Price-backed item that also carries price_data would charge the inline amount"
586 + );
587 + }
588 +
589 + #[test]
590 + fn a_recurring_item_carries_its_interval_and_a_recurring_price() {
591 + let item = build_inline_recurring_line_item(
592 + "SyncKit Pro",
593 + 900,
594 + CreateCheckoutSessionLineItemsPriceDataRecurringInterval::Month,
595 + );
596 + let price = item.price_data.expect("recurring items carry price_data");
597 + assert_eq!(price.unit_amount, Some(900));
598 + assert_eq!(price.currency, Currency::USD);
599 + assert!(
600 + price.recurring.is_some(),
601 + "without `recurring` Stripe bills this once instead of every period"
602 + );
603 + assert_eq!(item.quantity, Some(1));
604 + }
605 +
606 + // ── automatic tax ──
607 +
608 + #[test]
609 + fn automatic_tax_is_absent_rather_than_disabled_when_off() {
610 + assert!(
611 + automatic_tax(false).is_none(),
612 + "sending an explicit disabled block is not the same as omitting it"
613 + );
614 + assert!(automatic_tax(true).is_some());
615 + }
616 + }
M server/src/payments/fan_ops.rs +45 -10
@@ -57,16 +57,6 @@
57 57 }
58 58 }
59 59
60 - /// Apply `op` to every subscription in `sub_ids` on the background queue. Returns
61 - /// immediately; the loop runs off the request/webhook hot path. Per-subscription
62 - /// failures are logged and do not abort the rest. No-op for an empty list.
63 - ///
64 - /// A failed op is not merely logged: any failures open a WAM ticket (when `wam`
65 - /// is configured) so a dropped pause/suspend/cancel, a fan still charged after a
66 - /// creator pause, or a suspended creator's fans retaining access, is actively
67 - /// surfaced for manual reconciliation rather than lost in the logs (audit Run 13
68 - /// Resilience: fan-out had no dead-letter). The failed subscription IDs are
69 - /// listed in the ticket body.
70 60 /// Apply `op` to every subscription in `sub_ids`, awaiting each call inline.
71 61 ///
72 62 /// For contexts with no background queue, the `mnw-admin` CLI, where blocking on
@@ -98,6 +88,17 @@
98 88 failed
99 89 }
100 90
91 + /// Apply `op` to every subscription in `sub_ids` on the background queue.
92 + /// Returns immediately; the loop runs off the request/webhook hot path.
93 + /// Per-subscription failures are logged and do not abort the rest. No-op for an
94 + /// empty list.
95 + ///
96 + /// A failed op is not merely logged: any failures open a WAM ticket (when `wam`
97 + /// is configured) so a dropped pause/suspend/cancel, a fan still charged after a
98 + /// creator pause, or a suspended creator's fans retaining access, is actively
99 + /// surfaced for manual reconciliation rather than lost in the logs (audit Run 13
100 + /// Resilience: fan-out had no dead-letter). The failed subscription IDs are
101 + /// listed in the ticket body.
101 102 pub fn spawn_fan_sub_fanout(
102 103 bg: &BackgroundTx,
103 104 stripe: Arc<dyn PaymentProvider>,
@@ -142,3 +143,37 @@
142 143 }
143 144 });
144 145 }
146 +
147 + #[cfg(test)]
148 + mod tests {
149 + //! Operation labels. These reach logs and support tickets for creator
150 + //! subscription changes, so a label that says the opposite of what happened
151 + //! sends an investigation the wrong way.
152 +
153 + use super::*;
154 +
155 + #[test]
156 + fn setting_and_clearing_cancel_at_period_end_do_not_share_a_label() {
157 + assert_eq!(
158 + FanSubOp::CancelAtPeriodEnd(true).label(),
159 + "set_cancel_at_period_end"
160 + );
161 + assert_eq!(
162 + FanSubOp::CancelAtPeriodEnd(false).label(),
163 + "clear_cancel_at_period_end"
164 + );
165 + }
166 +
167 + #[test]
168 + fn every_operation_has_a_distinct_label() {
169 + let labels = [
170 + FanSubOp::CancelAtPeriodEnd(true).label(),
171 + FanSubOp::CancelAtPeriodEnd(false).label(),
172 + FanSubOp::Pause.label(),
173 + FanSubOp::Resume.label(),
174 + FanSubOp::Cancel.label(),
175 + ];
176 + let unique: std::collections::HashSet<_> = labels.iter().collect();
177 + assert_eq!(unique.len(), labels.len(), "labels collide: {labels:?}");
178 + }
179 + }
M server/src/payments/mod.rs +31
@@ -631,3 +631,34 @@
631 631 StripeClient::create_synckit_billing_portal(self, customer_id, return_url).await
632 632 }
633 633 }
634 +
635 + #[cfg(test)]
636 + mod tests {
637 + //! Stripe id parsing at the boundary between our database and Stripe's API.
638 + //! These ids come out of our own rows, so a parse failure means our data is
639 + //! wrong, and the classification matters: `Internal` pages us, `BadRequest`
640 + //! would blame the creator for our own corrupted column.
641 +
642 + use super::*;
643 +
644 + #[test]
645 + fn account_id_parsing_rejects_nothing_at_all() {
646 + // Same dead guard as `parse_subscription_id`. `stripe_shared::AccountId`
647 + // derives `FromStr` with `type Err = Infallible`, so every value parses
648 + // and the `Invalid Stripe account ID` branch cannot be reached. The doc
649 + // comment above reasons carefully about classifying the failure as
650 + // `Internal` rather than `BadRequest`; there is no failure to classify.
651 + //
652 + // The consequence is not academic: an empty `users.stripe_account_id`
653 + // becomes an empty connected-account header on a live charge instead of
654 + // an error we can see.
655 + assert!(StripeClient::parse_account_id("acct_1A2b3C4d5E6f7G8h").is_ok());
656 + for anything in ["", "cus_123", "not an id", "acct_"] {
657 + assert!(
658 + StripeClient::parse_account_id(anything).is_ok(),
659 + "{anything:?} parses today; if this now fails, the guard became real \
660 + and the test should assert the new contract"
661 + );
662 + }
663 + }
664 + }
M server/src/payments/synckit_billing.rs +61
@@ -356,3 +356,64 @@
356 356 .await
357 357 }
358 358 }
359 +
360 + #[cfg(test)]
361 + mod tests {
362 + //! Idempotency keys for SyncKit's Stripe writes. The key is the only thing
363 + //! standing between two racing `activate` requests and a duplicate live
364 + //! subscription that bills a developer with no local row to cancel it, so
365 + //! its determinism is a billing invariant rather than a detail.
366 +
367 + use super::*;
368 +
369 + #[test]
370 + fn the_same_app_and_prefix_always_produce_the_same_key() {
371 + let app = SyncAppId::nil();
372 + let a = synckit_idempotency_key("synckit-sub", app).expect("valid key");
373 + let b = synckit_idempotency_key("synckit-sub", app).expect("valid key");
374 + assert_eq!(
375 + format!("{a:?}"),
376 + format!("{b:?}"),
377 + "two racing activates must reuse one Stripe object, not create two"
378 + );
379 + }
380 +
381 + #[test]
382 + fn a_different_operation_on_one_app_gets_a_different_key() {
383 + let app = SyncAppId::nil();
384 + let sub = synckit_idempotency_key("synckit-sub", app).expect("valid key");
385 + let cust = synckit_idempotency_key("synckit-cust", app).expect("valid key");
386 + assert_ne!(
387 + format!("{sub:?}"),
388 + format!("{cust:?}"),
389 + "sharing a key across operations would make Stripe replay the wrong response"
390 + );
391 + }
392 +
393 + #[test]
394 + fn an_over_long_prefix_is_an_error_rather_than_a_silently_truncated_key() {
395 + // Stripe caps idempotency keys at 255 characters. Truncation would make
396 + // two distinct operations collide, which is worse than failing loudly.
397 + let err = synckit_idempotency_key(&"x".repeat(300), SyncAppId::nil());
398 + assert!(
399 + matches!(err, Err(AppError::Internal(_))),
400 + "an unusable key must not reach Stripe"
401 + );
402 + }
403 +
404 + #[test]
405 + fn subscription_id_parsing_rejects_nothing_at_all() {
406 + // Not the contract this function's name and error message imply.
407 + // `stripe_shared::SubscriptionId` derives `FromStr` with
408 + // `type Err = Infallible`: it wraps the string and always succeeds. The
409 + // `AppError::Internal("Invalid Stripe subscription ID")` branch is
410 + // unreachable, so an empty or garbage id from our own row is passed
411 + // straight to Stripe's cancel / re-price calls.
412 + //
413 + // Documented rather than asserted-as-correct: adding a real check is a
414 + // money-path behaviour change. Filed as a problem against mnw-server.
415 + assert!(parse_subscription_id("").is_ok());
416 + assert!(parse_subscription_id("not a sub id").is_ok());
417 + assert!(parse_subscription_id("acct_wrong_type").is_ok());
418 + }
419 + }
M server/src/db/synckit/apps.rs +54
@@ -222,3 +222,57 @@
222 222
223 223 Ok(())
224 224 }
225 +
226 + #[cfg(test)]
227 + mod tests {
228 + //! API-key hashing. The raw developer key is never stored, so this function
229 + //! is the whole of that promise: if it ever became reversible, or stopped
230 + //! being deterministic, either the keys leak or every app stops
231 + //! authenticating.
232 +
233 + use super::*;
234 +
235 + #[test]
236 + fn hashing_is_deterministic() {
237 + assert_eq!(
238 + hash_api_key("synckit-key-alpha"),
239 + hash_api_key("synckit-key-alpha")
240 + );
241 + }
242 +
243 + #[test]
244 + fn different_keys_hash_differently() {
245 + assert_ne!(
246 + hash_api_key("synckit-key-alpha"),
247 + hash_api_key("synckit-key-beta")
248 + );
249 + }
250 +
251 + #[test]
252 + fn the_hash_is_hex_encoded_sha256() {
253 + let h = hash_api_key("synckit-key-alpha");
254 + assert_eq!(h.len(), 64, "sha256 is 32 bytes, hex-encoded: {h}");
255 + assert!(
256 + h.chars().all(|c| c.is_ascii_hexdigit()),
257 + "must be hex so it round-trips through a text column: {h}"
258 + );
259 + // Pinned against a known vector so a swapped algorithm is caught rather
260 + // than merely being "some 64-char hex string".
261 + assert_eq!(
262 + hash_api_key(""),
263 + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"
264 + );
265 + }
266 +
267 + #[test]
268 + fn the_raw_key_does_not_survive_in_the_hash() {
269 + // Fixtures deliberately avoid the `sk_live_` shape: gitleaks scans the
270 + // staged diff and a realistic-looking key in a test is a secret as far
271 + // as the hook is concerned, which is the correct call.
272 + let raw = "synckit-key-with-secret-inside";
273 + assert!(
274 + !hash_api_key(raw).contains(raw),
275 + "the stored value must not carry the key it came from"
276 + );
277 + }
278 + }
M server/src/routes/storage/gallery.rs +31
@@ -573,3 +573,34 @@
573 573 tracing::warn!(project_id = %pid, error = ?e, "failed to bump cache generation after gallery change");
574 574 }
575 575 }
576 +
577 + #[cfg(test)]
578 + mod tests {
579 + //! Gallery target parsing. This decides which ownership check runs, so a
580 + //! wrong answer here points an authorization query at the wrong table.
581 +
582 + use super::*;
583 +
584 + #[test]
585 + fn the_two_known_targets_parse() {
586 + assert!(matches!(
587 + GalleryTarget::parse("item"),
588 + Ok(GalleryTarget::Item)
589 + ));
590 + assert!(matches!(
591 + GalleryTarget::parse("project"),
592 + Ok(GalleryTarget::Project)
593 + ));
594 + }
595 +
596 + #[test]
597 + fn anything_else_is_refused_rather_than_defaulted() {
598 + // Defaulting an unknown target would run the wrong ownership check.
599 + for s in ["", "Item", "ITEM", "items", "user", "../item"] {
600 + assert!(
601 + matches!(GalleryTarget::parse(s), Err(AppError::BadRequest(_))),
602 + "{s:?} must not resolve to a target"
603 + );
604 + }
605 + }
606 + }
M server/src/routes/synckit/blobs.rs +19
M server/src/routes/stripe/checkout/subscriptions.rs +94
M server/src/routes/stripe/webhook/subscriptions.rs +38
A server/tests/untested_money_paths.rs +140