Skip to content

Add a fault-injecting fake of Orange and a chaos suite over the message state machine - #121

Merged
stephane-segning merged 1 commit into
mainfrom
claude/fake-orange-chaos
Aug 7, 2026
Merged

Add a fault-injecting fake of Orange and a chaos suite over the message state machine#121
stephane-segning merged 1 commit into
mainfrom
claude/fake-orange-chaos

Conversation

@stephane-segning

Copy link
Copy Markdown
Contributor

Summary

  • crates/sms-fake-orange — a fault-injecting fake of Orange Cameroon's SMS HTTP API. A participant in the send/DLR lifecycle (inbound stubbing, an autonomous DLR scheduler over real HTTP, a request ledger), not a response stub — see the crate's own module doc.
  • crates/sms-worker/tests/chaos_live_postgres.rs — 10 scripted tests (one fault mode each) plus a 5-seed (1–5) seeded chaos sweep asserting invariants over the message state machine, wired into the existing live CI job automatically (it's a normal *_live_postgres.rs suite, cargo test --workspace -- --ignored already picks it up — no workflow YAML change needed).
  • schema/schema.cstackDeliveryReceipt.list/.detail gains || hasRole('system'), the same fix shape applied six times before to other models. Found live by this PR's own a_duplicate_delivered_dlr_is_idempotent test.
  • AGENTS.md — records the new suite, both real findings below, and an environment note about a CLI/library version skew encountered while verifying the schema change.

Intent

Source of truth: this PR's own assignment brief — build the automatable complement to docs/runbooks/36-handset-gate.md by fuzzing the message state machine against a fault-injecting fake of Orange, per #36's own framing that "lease reclamation is the kind of code that is easy to write and easy to never actually exercise" (the same reasoning #114's kill-9 gate already applied to lease reclaim; this PR applies it to the DLR/submit fault space instead).

This PR does not close #36 and does not claim to. A mock cannot tell you Orange's real DLR payload shape, whether receiptRequest is genuinely honoured, or whether a handset ever buzzes — docs/runbooks/36-handset-gate.md stays the actual gate for that, unchanged by this PR.

Scope

In scope:

  • Inbound stubbing (token + submit endpoints) with two fault policies: Scripted (exact ordered sequence, for deterministic tests) and Seeded (a seeded PRNG drawing a realistic-weighted mix — reproducible by construction, seed named in every assertion).
  • An autonomous DLR scheduler: independent tokio tasks POST real deliveryInfoNotification bodies to a genuine, real, in-process POST /dlr/{providerKey} route (hand-rolled here since app/sms-gateway is a binary crate with no lib.rs — same constraint oidc_flow_live.rs already worked around, same fix shape).
  • A request ledger, queryable from the provider's own side of the wire — this is how the seeded sweep proves "never resubmitted after Indeterminate" from Orange's own perspective, not by inferring it from this system's database.
  • Fault modes modelled, prioritised per the brief: submit-accepted-then-timeout (Indeterminate), 200 OK with no DLR ever, a DLR racing the submit response (the one the brief called out for "real attention" — see Verification), duplicate DLRs, out-of-order DLRs, DLRs for an unknown ref, DLRs after a terminal state, 429 (with recovery), 5xx, a malformed/missing-resourceURL 201, and a 401 at the token endpoint.
  • Fault modes explicitly skipped, with reasoning: connection-level nastiness (RST mid-response, refused connections, byte-dribble) — wiremock's Respond trait can only ever emit a well-formed HTTP response, delayed or not; modelling a severed connection needs connection-level control this crate doesn't take on. Noted as future work in the crate's own module doc, not half-done. The connect-refused half of that same distinction is already covered directly, against a real socket, by sms-provider-orange-cm's own a_connect_refusal_is_still_unavailable — this PR doesn't re-prove it.

Out of scope, deliberately: no change to crates/sms-provider-orange-cm or crates/sms-worker/src/dispatch.rs — the fake exercises existing, already-shipped classification logic, it doesn't change it. The one production change (schema.cstack's DeliveryReceipt policy) is the smallest fix that unblocks this PR's own test from reading back what it just wrote, following an established six-times-precedented pattern rather than inventing a new one.

Verification

just check, just lint (fmt + clippy -D warnings), just test, just parity, ./ci/assert-no-raw-sqlx.sh — all green, workspace-wide, including the new crate and test file.

cargo metadata --locked — green, Cargo.lock up to date.

Migration safety for the schema.cstack edit: an @@allow change never touches DDL (@@allow compiles into generated Rust policy code via include_server_schema!, not SQL) — confirmed two ways: (1) #100's own commit message documents its analogous Job policy fix as producing a byte-identical migration diff, and (2) cratestack migrate diff was actually rerun against both the pre- and post-edit schema here. It turned out this machine's globally installed cratestack CLI is 0.7.4, newer than the pinned library family (=0.6.7), and emits foreign-key ALTER TABLE statements the pinned emitter never has — confirmed this is pre-existing CLI/library skew, unrelated to this PR's edit, by rerunning the identical diff against the unmodified main schema and finding it already disagrees with the committed 0001_init the same way. The committed migrations were left untouched, correctly, on the strength of the policy-only-change reasoning above rather than regenerated with a mismatched CLI. Recorded in AGENTS.md as an environment note for whoever hits this next.

Three full just test-live runs (cargo test --workspace -- --ignored), container-count-verified before and after each:

Run Total live suites chaos_live_postgres Failures Flake
1 all green 16 passed / 0 failed, 7.33s 0 none
2 all green 16 passed / 0 failed, 7.47s 0 none
3 all green 16 passed / 0 failed, 7.42s 0 none

Every other live suite (claim_live_postgres, dispatch_live_postgres, jobs_live_postgres, send_message_live_postgres, dlr_ingestion_live_postgres, policy_golden_list_live_postgres, errors_live_postgres, kill9_reclaim_live, m1_acceptance_gate_live_postgres, oidc_flow_live, provision_app_client_live_postgres, rbac_layer2_live_postgres, live_postgres ×2) stayed green across all 3 runs too — no regression from the DeliveryReceipt policy change.

Container count: 12 before, 12 after every run (docker ps -a), the one vsms-test-harness-postgres reused (not recreated) each time, the pre-existing tenant-provisioner-* stack (11 containers) never touched. Scoped throughout by the harness's own dev.vsms.test-harness=true label, never by image or bare name.

Added CI runtime: chaos_live_postgres itself runs in ~7.4s per the table above — comfortably inside the ~60s budget for the live job (whose 15 non-chaos suites summed to roughly 85s of the ~93s total per run above, so the chaos suite's own share is small relative to the existing job, not the dominant cost).

The race scenario proven, not just modelled: a_dlr_racing_the_submit_response_is_dropped_then_the_message_expires schedules a Delivered DLR with zero delay against a submit response deliberately held open for 120ms. At the moment the DLR arrives, Message.providerMessageRef and providerMessageRefAlt are both still NULL (the latter is only stamped by write_submitted/write_transition, both of which run strictly after the submit HTTP response is processed) — so sms_api::dlr::ingest_one's correlation query matches nothing, the DLR is silently dropped, and the message reaches submitted normally moments later with no further DLR ever arriving. The test then forces expire_stale forward and confirms the message reaches a real terminal state (expired) rather than sitting stuck or being wrongly resolved to delivered by a receipt that shouldn't have matched. This is the scenario the assignment brief called out for "real attention," and it's now a permanent regression assertion, not a one-off finding.

Invariant list actually asserted by the seeded sweep (run_seed's own doc has the full reasoning): no message lost (terminal, or the documented undelivered gap below); nothing left in accepted/queued/routed after the tick loop settles; attempts <= maxAttempts; and — checked against the fake's own ledger, not this system's database — a message that went uncertain via an Indeterminate-shaped submit call is never submitted again afterward (#119's own guarantee).

Two real findings — not invented

  1. DeliveryReceipt.list/.detail was missing hasRole('system') — the seventh instance of the exact policy-gap shape this codebase has now found six times before (App, AppClient, SenderIdRegistration, OperatorPrefixRule, Provider, Job). Found by a_duplicate_delivered_dlr_is_idempotent trying to read back DeliveryReceipt rows under a system context after two DLRs landed, and getting zero rows instead of two. Fixed the same way as every prior instance, in schema.cstack only. Not seed-reproducible (a schema bug, not a fault-injection finding), but a direct, real defect the chaos suite's own assertions surfaced by actually running against a live database.
  2. The undelivered gap dlr.rs's own module doc already documents in prose is empirically real, not just a documented intention: nothing currently drives undelivered -> queued, and expire_stale only reaps submitted/uncertain, never undelivered — so a message that receives exactly one retryable-failure DLR (submitted -> undelivered) and no follow-up sits in undelivered forever under this system's current scope. The seeded sweep's own final-state invariant treats this as an accepted, already-documented outcome rather than a sweep failure — re-litigating a known gap would have been the wrong response, and the sweep's own reasoning for why is spelled out inline in run_seed's doc comment.

No SM001/illegal-transition surfaced as a real failure in any of the 3×5=15 seeded runs, and none of the 3×10=30 scripted runs. Finding none of that particular class is itself a real, reportable (not invented) result.

Risk Assessment

Low. The only production-code change is the single-model, two-line schema.cstack policy addition — same shape, same verification discipline (byte-identical-DDL check) as six prior instances of this exact fix in this codebase. Everything else is new, test-only code: a new dev-only crate (sms-fake-orange, never depended on by any production binary — same discipline as sms-test-support) and a new #[ignore]d live test file that only runs under --ignored, exactly like every other live suite in this workspace.

AI Usage Declaration

This PR was implemented by Claude Code (Claude Sonnet 5) under the user's direction, from a detailed assignment brief specifying the fake's design (three responsibilities: inbound stubbing, an autonomous DLR scheduler, a request ledger), the fault taxonomy to prioritise, the invariants to assert, and the verification bar.

  • I read every piece of ground truth named in the brief before designing anything (ProviderError/RoutingConsequence, the real OrangeCmProvider adapter and its own transport-error classification, dispatch::tick's claim-then-submit split, claim.rs's no-real-lease accepted -> queued hop, dlr.rs's correlation logic and its own documented undelivered gap, the message state-transition table) rather than assuming any of it from the brief's own prose, which the brief itself warned may contain errors (and did — flagging RoutingConsequence::RetryThisProvider as belonging to RoutingConsequence, not ProviderError, matches what I verified against the source).
  • I found, diagnosed, and fixed a real test-harness bug during verification (the provider tie-break — cheapest_active_provider has no tie-breaker and every test in this file seeds a provider at the same fixed cost, so a later test could route to an earlier test's row and its DLR would then correlate against the wrong provider_row_id) by reproducing it live (4 of 16 tests genuinely failed on the first real run against Postgres), root-causing it against dispatch_live_postgres.rs's own already-documented instance of the identical shape, and reverifying the fix with 3 full clean reruns — not by reasoning about it in the abstract.
  • I take responsibility for the accuracy of every pass count, container count, and timing figure above; they are copied from this session's actual command output, not estimated. The CLI/library version-skew note was investigated to the point of a clean A/B (same diff, modified vs. unmodified schema) rather than asserted from a single observation.

Reviewer Focus

  • Whether hand-rolling POST /dlr/{providerKey} a second time (a third time counting app/sms-gateway/src/dlr.rs itself) inside the test file is the right call, versus giving app/sms-gateway a lib.rs at some point so every live suite that needs its real routes stops re-deriving them. Out of scope for this PR (the existing oidc_flow_live.rs precedent already made this call), but worth a second opinion given this is now the second time it's been duplicated.
  • The Seeded fault-weight distribution in crates/sms-fake-orange/src/fault.rs (seeded_decision/accepted_with_random_dlr) — chosen to be realistic-skewed per §6.1's own framing, but the exact percentages are a judgement call, not derived from any real traffic data (none exists yet for this system).
  • Whether the DeliveryReceipt policy fix should also extend to an appId-scoped clause (the way Message's own policy has appId == auth().appId) rather than just hasRole('system') — out of scope here since nothing in this deployment currently needs an app-scoped read of DeliveryReceipt, but it's the same shape Message's policy already has and a future admin-console read path will likely want it.

… machine

crates/sms-fake-orange is a participant in the send/DLR lifecycle, not a
response stub: it answers the token/submit HTTP calls per a FaultPolicy
(Scripted for deterministic CI-gate tests, Seeded for a reproducible
weighted-random mix) and independently schedules DLR deliveries as real
HTTP POSTs against a caller-supplied /dlr/{providerKey} endpoint, plus a
request ledger queryable from the provider's own side of the wire.

crates/sms-worker/tests/chaos_live_postgres.rs hand-rolls the same small
POST /dlr/{providerKey} handler app/sms-gateway/src/dlr.rs owns in
production (app/sms-gateway is a binary crate with no lib.rs) against a
real axum::serve on an ephemeral port, so DLR delivery is a genuine HTTP
round trip. Ten scripted tests cover one fault mode each; five seeded
sweeps (fixed seeds 1-5) assert invariants over a weighted-random fault
mix rather than exact outcomes. Connection-level nastiness (RST, refused
connections, byte-dribble) is explicitly out of scope - noted as future
work, not half-done.

This is the automatable complement to docs/runbooks/36-handset-gate.md
and does not close #36: it cannot prove anything about Orange's real DLR
payload shape, receiptRequest honouring, or a real handset.

Two real findings surfaced by actually running the suite, not invented:
DeliveryReceipt's list/detail policy was missing hasRole('system') - the
seventh instance of a gap shape this codebase has now found six times
before - fixed in schema.cstack only (an @@Allow change never touches
DDL, confirmed both by #100's own precedent and by diffing this repo's
own migration output before/after). Second, the seeded sweep empirically
confirms dlr.rs's own documented `undelivered` gap is real: nothing
drives undelivered -> queued and expire_stale doesn't reap it either, so
a message can sit in undelivered forever under current scope - the
sweep's own invariant treats that as accepted, not a new bug.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 9c836bc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@stephane-segning
stephane-segning merged commit 855a39d into main Aug 7, 2026
6 checks passed
stephane-segning added a commit that referenced this pull request Aug 7, 2026
…NTS.md (#123)

Four PRs landed in one session (#118 harness, #120 gate stabilisation,
#119 ProviderError::Indeterminate, #121 fake Orange + chaos suite) and the
context file only picked up what the last two agents wrote inline. The
biggest omission was the most consequential change: `sms-test-support` and
the fact that CI now runs the 14 live-Postgres suites at all — previously
`cargo test --workspace` ran without `--ignored`, so they were skipped in
CI and had only ever run by hand.

Records, in each case why it exists rather than just that it does:

- The harness's five load-bearing constraints, each learned by something
  breaking: no container handle in a `static` (Drop never runs for statics,
  which leaked 56 containers), label-scoped cleanup only (an image-scoped
  sweep would destroy a developer's own database), one database per test
  binary (a shared one let a real dispatch subprocess steal other suites'
  messages), migration-content fingerprinting (an existence check served a
  stale schema silently), and the global container name that makes
  concurrent test runs corrupt each other.
- That `ca653a1`'s mutex was never applied to two suites, both of which
  flaked in CI within hours of #118 running them for the first time.
- `Indeterminate`'s connect-vs-read predicate, the `routed -> uncertain`
  edge it needed, and the fact that it trades a possibly lost message for
  never sending a duplicate — a product decision, not a free win.
- A CLI-skew warning placed at both sites that tell you to run
  `cratestack migrate diff`, since the installed 0.7.4 CLI emits DDL the
  pinned 0.6.7 library never does. The warning previously existed only in
  an unrelated paragraph 150 lines away from the commands themselves.
- The missing-`hasRole('system')` gap as an explicitly *unguarded* failure
  mode, now found seven times. The three entries above it each turned a
  silent failure loud; this one has no equivalent, which is why it recurs.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

Gate: real handset delivery, and kill -9 lease reclaim

1 participant