From 9ba33c73d02cf0628e4f442a0535e94f5470075d Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 07:11:39 -0400 Subject: [PATCH 1/9] feat(common): add IdempotencyKey domain type (#1086) --- common/src/idempotency_key.rs | 93 +++++++++++++++++++ common/src/lib.rs | 1 + .../2026-08-26-issue-1086-idempotency-key.md | 69 ++++++++++++++ .../2026-08-26-issue-1086-idempotency-key.md | 88 ++++++++++++++++++ 4 files changed, 251 insertions(+) create mode 100644 common/src/idempotency_key.rs create mode 100644 docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md create mode 100644 docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md diff --git a/common/src/idempotency_key.rs b/common/src/idempotency_key.rs new file mode 100644 index 00000000..316da6d7 --- /dev/null +++ b/common/src/idempotency_key.rs @@ -0,0 +1,93 @@ +use std::str::FromStr; + +use macros::StrNewtype; +use thiserror::Error; + +/// A retry key that identifies one post-creation attempt for a user. +/// +/// [`FromStr`] is the single validation and canonicalization chokepoint: outer +/// whitespace is removed, and an empty result is rejected. Any other text is +/// preserved, because key format is chosen by the client rather than this domain +/// type. The ADR-0063 string-newtype trailer (`Display`, `AsRef`/`Borrow`/ +/// `Deref`, owned `String` conversions, `PartialEq`/`<&str>`, ordering, +/// and validating serde and sqlx bridges) is generated by `#[derive(StrNewtype)]`. +#[derive(Clone, Debug, PartialEq, Eq, StrNewtype)] +pub struct IdempotencyKey(String); + +/// Error returned when a string cannot be parsed as an [`IdempotencyKey`]. +#[derive(Debug, Error)] +#[error("idempotency key must be non-empty")] +pub struct InvalidIdempotencyKey; + +impl FromStr for IdempotencyKey { + type Err = InvalidIdempotencyKey; + + fn from_str(s: &str) -> Result { + let trimmed = s.trim(); + if trimmed.is_empty() { + return Err(InvalidIdempotencyKey); + } + Ok(Self(trimmed.to_owned())) + } +} + +#[cfg(test)] +mod tests { + use std::borrow::Borrow; + + use super::*; + + #[test] + fn parses_trimmed_arbitrary_text_without_format_restrictions() { + let key: IdempotencyKey = " \t retry:🦀/v1 \n".parse().unwrap(); + + assert_eq!(key, "retry:🦀/v1"); + let canonical: &str = key.as_ref(); + assert_eq!(canonical, "retry:🦀/v1"); + } + + #[test] + fn rejects_empty_and_whitespace_only_keys() { + for input in ["", " ", "\t\n", "\u{2003}"] { + assert!(input.parse::().is_err(), "{input:?}"); + } + assert_eq!( + InvalidIdempotencyKey.to_string(), + "idempotency key must be non-empty" + ); + } + + #[test] + fn serde_round_trips_canonical_key_and_rejects_blank_input() { + let key: IdempotencyKey = " retry key ".parse().unwrap(); + + assert_eq!(serde_json::to_string(&key).unwrap(), "\"retry key\""); + assert_eq!( + serde_json::from_str::("\" retry key \"").unwrap(), + key + ); + assert!(serde_json::from_str::("\"\"").is_err()); + assert!(serde_json::from_str::("\" \\t \"").is_err()); + } + + #[test] + fn standard_trailer_supports_owned_and_borrowed_access() { + let key = IdempotencyKey::try_from(" retry-key ".to_owned()).unwrap(); + + assert_eq!(key.as_ref(), "retry-key"); + let via_borrow: &str = key.borrow(); + assert_eq!(via_borrow, "retry-key"); + let borrowed: &str = &key; + assert_eq!(borrowed, "retry-key"); + assert_eq!(String::from(key), "retry-key"); + } + + #[test] + fn standard_trailer_compares_and_orders_by_canonical_string() { + let earlier: IdempotencyKey = " alpha ".parse().unwrap(); + let later: IdempotencyKey = "beta".parse().unwrap(); + + assert_eq!(earlier, "alpha"); + assert!(earlier < later); + } +} diff --git a/common/src/lib.rs b/common/src/lib.rs index 71c943b8..c7d4b340 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -17,6 +17,7 @@ pub mod display_name; pub mod email; pub mod etag; pub mod feed; +pub mod idempotency_key; pub mod ids; pub mod invite; pub mod list_state; diff --git a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md new file mode 100644 index 00000000..8266a25d --- /dev/null +++ b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md @@ -0,0 +1,69 @@ +# Idempotency key domain type implementation outline + +> Execute with dev-cycle-iterate. This outline exists because a new shared +> domain type changes storage/service and AtomPub contracts in dependency order. + +## Scope + +In: + +- Shared `IdempotencyKey` type and standard trailer. +- Typed post-service/storage contracts and existing-schema SQLx use. +- Compatible AtomPub header parsing and replay behavior. +- Focused type, dual-backend storage, and AtomPub boundary coverage. +- ADR-0063 and current architecture projection updates. + +Out: + +- Schema changes, retention, payload fingerprints, new response statuses, client + format changes, and edits to frozen #697 artifacts. + +## Task outline + +- [x] Task 1: Establish the shared idempotency-key contract + - Contract: `common::idempotency_key::IdempotencyKey` is a non-secret + SQLx-enabled string newtype; `FromStr` trims, rejects empty, and otherwise + preserves the trimmed string. Its named error and standard trailer follow + existing ADR-0063 types. + - Verification: focused `common` tests prove canonicalization, + arbitrary-string acceptance, empty rejection, serde/owned-borrowed behavior, + and the repository's standard trailer requirements. + +- [ ] Task 2: Carry typed keys from AtomPub through persistence + - Depends on: Task 1. + - Contract: the AtomPub handler preserves `HeaderValue::to_str` compatibility, + maps missing/whitespace-only/unreadable values to `None`, and parses + readable non-empty text once into owned `IdempotencyKey`. Borrowed + service/orchestration and lookup seams use `Option<&IdempotencyKey>` / + `&IdempotencyKey`; lifetime-free content/input structs own + `Option`; SQL lookup and insert bind the type directly. + Every caller migrates in this task, with no primitive compatibility overload + or migration. + - Verification: dual-backend storage/service tests prove typed SQLx behavior + and atomic rollback on collision. Dual-backend router tests prove valid + first/reused keys, different content on reuse, per-user scope, + whitespace-only, non-ASCII UTF-8 bytes, invalid UTF-8 bytes, and no-key + behavior through real HTTP requests. + +- [ ] Task 3: Record the completed domain contract + - Depends on: Tasks 1–2. + - Contract: ADR-0063 and `docs/ARCHITECTURE.md` describe the type, + compatibility boundary, per-user persistence, and replay semantics; frozen + #697 artifacts remain unchanged. + - Verification: documentation/gate checks resolve all live paths and + references without modifying archived history. + +## Risk checks + +- Every idempotency-specific exported symbol migration includes all callsites + before its task is committed; no raw-string shim or deprecated path remains. +- AtomPub unreadable-header cases are constructed as raw `HeaderValue` bytes so + tests exercise `to_str` rejection rather than only the domain parser. +- SQLx decode validates stored rows; both existing database dialects remain + schema-identical. +- Collision tests distinguish the original committed post from every row + belonging to the rolled-back attempt. +- Slug collision retries remain separate from idempotency conflicts and retain + their current retry behavior. +- Integrated verification runs the repository's changed-contract checks and + normal commit gate after all tasks land. diff --git a/docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md b/docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md new file mode 100644 index 00000000..a397d76c --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md @@ -0,0 +1,88 @@ +# Issue #1086 — Model post idempotency keys as a domain value + +## Outcome + +AtomPub create requests parse a retry token once into `IdempotencyKey`, and that +domain type remains intact through post creation, duplicate lookup, and database +insertion. Existing client compatibility and replay behavior remain unchanged. + +The type removes primitive transposition and stripping risk without changing the +HTTP grammar or database schema. + +## Load-bearing decisions + +- `IdempotencyKey` is a non-secret string domain type in `common`, following + ADR-0063's standard string-newtype trailer. +- `FromStr` is the validation and canonicalization chokepoint: trim outer + whitespace, reject an empty result, and store the trimmed value. +- `IdempotencyKey` itself accepts any otherwise-arbitrary non-empty string. + There is no character-set, byte-length, scalar-length, UUID, base64url, or RFC + 8941 structured-field restriction. +- AtomPub preserves the existing `HeaderValue::to_str` compatibility boundary: + - missing header means no idempotency key; + - whitespace-only readable header text means no idempotency key; + - bytes rejected by `HeaderValue::to_str` — including non-ASCII UTF-8 and + invalid UTF-8 — mean no idempotency key; + - non-empty readable header text becomes an owned `IdempotencyKey`. +- Boundary compatibility is deliberate: empty and unreadable headers are treated + as absent rather than rejected with HTTP 400. +- Borrowed orchestration and lookup APIs use `Option<&IdempotencyKey>`; + lifetime-free creation inputs and persisted values use owned + `Option`. +- The ordinary non-secret SQLx bridge remains enabled. SQL binds and decodes + operate on `IdempotencyKey`; decode revalidates the domain invariant. +- No migration is needed. Both backends already store the key as `TEXT NOT NULL` + with `UNIQUE(user_id, key)` and no length/check constraint. +- Uniqueness remains scoped by `UserId`, not global. +- Existing replay semantics remain authoritative: + - a fresh keyed create returns 201; + - reusing the key for the same user returns the original post as 200; + - payload equality is not checked, so reuse with different content still + returns the original post; + - another user may independently use the same key; + - the key row is inserted atomically with the post, and a uniqueness collision + rolls the attempted post back; + - keys remain retained indefinitely. +- The issue's request to remove #697's follow-up note is superseded by the + repository's frozen-archive rule: the historical plan remains unchanged. + Closing #1086 resolves the follow-up, while ADR-0063 records `IdempotencyKey` + as an adopted domain value. + +## Acceptance + +- `common` exposes `IdempotencyKey` with the standard owned/borrowed/serde/SQLx + string-newtype interface and a named parse error. +- Type-level tests prove trimming, non-empty enforcement, acceptance of + otherwise arbitrary UTF-8 text, serde behavior, SQLx compatibility where + convention requires it, and the standard trailer contract. +- AtomPub parses a present header into an owned `IdempotencyKey` before calling + post creation. +- No raw `String` or `str` represents an idempotency key in post-service inputs, + post-storage inputs, duplicate lookup, or database binds. +- Valid keyed create behavior remains 201 on first use and 200 with the original + post on reuse. +- Whitespace-only, non-ASCII UTF-8, and invalid-UTF-8 header cases exercise the + real AtomPub request boundary and behave exactly like an absent key. +- Reusing a key with different post content returns the original post without + creating the attempted post. +- The same key can create one post for each of two different users. +- Existing transactional behavior remains intact: a collision leaves no + attempted post, audience, media-reference, or idempotency row behind. +- Both SQLite and PostgreSQL execute the storage/AtomPub contract tests + according to repository backend conventions. +- ADR-0063 and the current architecture projection describe the typed + idempotency-key contract; frozen #697 artifacts remain historical. + +## Boundaries + +- No database migration, new index, retention policy, garbage collection, + expiry, or background cleanup. +- No payload fingerprint, request-body comparison, conflict response, or 409 + behavior. +- No new client-generated key format and no change to the Emacs/client retry + algorithm. +- No RFC 8941 structured-field parsing and no dependence on the expired IETF + idempotency-key draft. +- No broad HTTP-header validation cleanup outside `Idempotency-Key`. +- No behavior change to unkeyed post creation, slug retries, tags, audiences, + media references, or publication state. From 5d7b682e8ab8961d12b818519ba7d7f180875a69 Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 07:27:11 -0400 Subject: [PATCH 2/9] refactor(posts): thread typed idempotency keys (#1086) --- .../2026-08-26-issue-1086-idempotency-key.md | 2 +- server/src/atompub/posts.rs | 13 +-- server/tests/atompub/atompub_posts.rs | 70 ++++++++++++++- storage/src/post_service.rs | 86 ++++++++++++++----- storage/src/posts.rs | 12 +-- 5 files changed, 145 insertions(+), 38 deletions(-) diff --git a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md index 8266a25d..3e340e86 100644 --- a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md +++ b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md @@ -29,7 +29,7 @@ Out: arbitrary-string acceptance, empty rejection, serde/owned-borrowed behavior, and the repository's standard trailer requirements. -- [ ] Task 2: Carry typed keys from AtomPub through persistence +- [x] Task 2: Carry typed keys from AtomPub through persistence - Depends on: Task 1. - Contract: the AtomPub handler preserves `HeaderValue::to_str` compatibility, maps missing/whitespace-only/unreadable values to `None`, and parses diff --git a/server/src/atompub/posts.rs b/server/src/atompub/posts.rs index 43ef80b5..67875d20 100644 --- a/server/src/atompub/posts.rs +++ b/server/src/atompub/posts.rs @@ -12,6 +12,7 @@ use serde::Deserialize; use common::atompub::{CollectionFeedTitle, Entry, FeedMeta, entry_to_xml, render_feed}; use common::etag::{ETag, post_content_etag}; +use common::idempotency_key::IdempotencyKey; use common::ids::PostId; use common::org::{OrgOperation, OrgStructuredMetadata, Presence, PublicationState, normalize_org}; use common::pagination::PageSize; @@ -479,11 +480,11 @@ pub async fn collection_post( Presence::Absent => vec![site_config.get_default_audience().await?.into()], }; // A client-supplied idempotency key dedups a retried create (duplicate-on-retry). - let idem = headers + // Preserve the historical `HeaderValue::to_str` compatibility boundary: + // unreadable or blank values do not opt a request into deduplication. + let idempotency_key = headers .get("idempotency-key") - .and_then(|v| v.to_str().ok()) - .map(str::trim) - .filter(|s| !s.is_empty()); + .and_then(|value| value.to_str().ok()?.parse::().ok()); let created = storage::perform_post_creation( posts, @@ -497,7 +498,7 @@ pub async fn collection_post( max_attempts: 100, summary, audiences, - idempotency_key: idem, + idempotency_key: idempotency_key.as_ref(), expectations, }, ) @@ -511,7 +512,7 @@ pub async fn collection_post( // A reused idempotency key returns the original post as `200` — skipping category // re-application (the original already carries its tags). if let Err(storage::PerformCreationError::IdempotencyConflict) = &created { - let key = idem.ok_or(HandlerError::Invariant)?; + let key = idempotency_key.as_ref().ok_or(HandlerError::Invariant)?; let post_id = posts .post_id_for_idempotency_key(auth_user.user_id, key) .await? diff --git a/server/tests/atompub/atompub_posts.rs b/server/tests/atompub/atompub_posts.rs index baedbcf6..296d4e6d 100644 --- a/server/tests/atompub/atompub_posts.rs +++ b/server/tests/atompub/atompub_posts.rs @@ -1,6 +1,6 @@ use axum::{ body::Body, - http::{Method, Request, StatusCode, header}, + http::{HeaderValue, Method, Request, StatusCode, header}, }; use common::ids::PostId; use common::root_relative_url::RootRelativeUrl; @@ -2238,12 +2238,29 @@ async fn update_with_explicit_draft_no_preserves_published_instant(#[case] backe ); } -/// POST a create as alice, optionally with an `Idempotency-Key`. +/// POST a create as `session`, optionally with an `Idempotency-Key`. async fn create_post_keyed( app: axum::Router, session: &SeededSession, xml: &str, idempotency_key: Option<&str>, +) -> axum::response::Response { + create_post_with_idempotency_header( + app, + session, + xml, + idempotency_key.map(|key| HeaderValue::try_from(key).unwrap()), + ) + .await +} + +/// POST a create with a raw idempotency header value so boundary tests exercise +/// `HeaderValue::to_str`, rather than pre-validating through request strings. +async fn create_post_with_idempotency_header( + app: axum::Router, + session: &SeededSession, + xml: &str, + idempotency_key: Option, ) -> axum::response::Response { let mut builder = atompub(session, Method::POST, "posts") .header(header::CONTENT_TYPE, "application/atom+xml"); @@ -2279,7 +2296,8 @@ async fn create_with_same_idempotency_key_dedups(#[case] backend: Backend) { let etag1 = etag_of(&first); let body1 = body_string(first).await; - let second = create_post_keyed(app, &session, &xml, Some("idem-1")).await; + let retry_xml = entry_xml("Changed", "text", "different retry content"); + let second = create_post_keyed(app, &session, &retry_xml, Some("idem-1")).await; assert_eq!(second.status(), StatusCode::OK); assert_eq!( location_of(&second), @@ -2332,6 +2350,52 @@ async fn create_without_idempotency_key_is_201(#[case] backend: Backend) { assert_eq!(response.status(), StatusCode::CREATED); } +#[apply(backends)] +#[tokio::test] +async fn unreadable_or_blank_idempotency_keys_do_not_dedup(#[case] backend: Backend) { + let TestEnv { state, base } = setup_with_base_url(backend).await; + let session = create_user_and_session(&state).await; + let app = make_app(&state, &base); + let xml = entry_xml("Hello", "text", "the body"); + + for header_value in [ + HeaderValue::from_static(" \t "), + HeaderValue::from_bytes("rétry".as_bytes()).unwrap(), + HeaderValue::from_bytes(&[0xff]).unwrap(), + ] { + let first = create_post_with_idempotency_header( + app.clone(), + &session, + &xml, + Some(header_value.clone()), + ) + .await; + let second = + create_post_with_idempotency_header(app.clone(), &session, &xml, Some(header_value)) + .await; + assert_eq!(first.status(), StatusCode::CREATED); + assert_eq!(second.status(), StatusCode::CREATED); + assert_ne!(location_of(&first), location_of(&second)); + } +} + +#[apply(backends)] +#[tokio::test] +async fn idempotency_key_is_scoped_to_the_authenticated_user(#[case] backend: Backend) { + let TestEnv { state, base } = setup_with_base_url(backend).await; + let alice = create_user_and_session(&state).await; + let bob = create_user_and_session(&state).await; + let app = make_app(&state, &base); + let xml = entry_xml("Hello", "text", "the body"); + + let alice_response = create_post_keyed(app.clone(), &alice, &xml, Some("shared-key")).await; + let bob_response = create_post_keyed(app, &bob, &xml, Some("shared-key")).await; + + assert_eq!(alice_response.status(), StatusCode::CREATED); + assert_eq!(bob_response.status(), StatusCode::CREATED); + assert_ne!(location_of(&alice_response), location_of(&bob_response)); +} + #[apply(backends)] #[tokio::test] async fn create_writes_the_entrys_media_rows(#[case] backend: Backend) { diff --git a/storage/src/post_service.rs b/storage/src/post_service.rs index 38f55482..347a446e 100644 --- a/storage/src/post_service.rs +++ b/storage/src/post_service.rs @@ -10,6 +10,7 @@ use crate::{ CreatePostError, CreatePostInput, PostBookkeepingExpectation, PostFormat, PostRecord, PostStorage, PublishUpdate, UpdatePostError, UpdatePostInput, }; +use common::idempotency_key::IdempotencyKey; use common::ids::{PostId, UserId}; use common::post_body::PostBody; use common::post_summary::PostSummary; @@ -44,7 +45,7 @@ pub struct RenderedPostContent { /// Audience targeting for the new post. pub audiences: Vec, /// Owned idempotency key to register with the post, or `None`. - pub idempotency_key: Option, + pub idempotency_key: Option, /// Non-authoritative Org bookkeeping expected to match the final stored row. pub expectations: PostBookkeepingExpectation, } @@ -381,7 +382,7 @@ pub struct PostCreation<'a> { pub audiences: Vec, /// Client-supplied idempotency key (already trimmed / non-empty), or `None` /// to create without deduplication. - pub idempotency_key: Option<&'a str>, + pub idempotency_key: Option<&'a IdempotencyKey>, /// Non-authoritative Org bookkeeping expected to match the collision winner. pub expectations: PostBookkeepingExpectation, } @@ -445,7 +446,7 @@ pub async fn perform_post_creation( published_at, summary: summary.clone(), audiences: audiences.clone(), - idempotency_key: idempotency_key.map(str::to_owned), + idempotency_key: idempotency_key.cloned(), expectations: expectations.clone(), }, ) @@ -492,7 +493,10 @@ pub async fn perform_post_creation( #[cfg(test)] mod tests { use super::*; - use crate::test_support::{Backend, SeedUser, backends}; + use crate::test_support::{ + Backend, SeedUser, backends, fetch_post_media, media_ref_for, media_url_for, seed_media, + }; + use common::idempotency_key::IdempotencyKey; use common::test_support::{parse_post_body, parse_post_title, parse_row_limit, parse_slug}; use rstest::*; use rstest_reuse::*; @@ -1522,10 +1526,13 @@ mod tests { } // -- idempotency-key tests -- - /// Builds a minimal public Markdown [`PostCreation`] carrying `key`, so the /// dedup tests vary only the user, body, and key. - fn creation_with_key(user_id: UserId, body: PostBody, key: Option<&str>) -> PostCreation<'_> { + fn creation_with_key( + user_id: UserId, + body: PostBody, + key: Option<&IdempotencyKey>, + ) -> PostCreation<'_> { PostCreation { user_id, body, @@ -1541,37 +1548,67 @@ mod tests { } } + fn parse_idempotency_key(key: &str) -> IdempotencyKey { + key.parse().unwrap() + } + #[apply(backends)] #[tokio::test] async fn perform_post_creation_dedups_on_idempotency_key(#[case] backend: Backend) { let env = backend.setup().await; let user_id = SeedUser::new().seed(&env.state).await.user_id; let storage = &*env.state.posts; + let key = parse_idempotency_key("k"); + seed_media(&env.state, user_id, "original.jpg").await; + seed_media(&env.state, user_id, "attempted.jpg").await; let first = perform_post_creation( storage, - creation_with_key(user_id, parse_post_body("First body"), Some("k")), + creation_with_key( + user_id, + parse_post_body(&format!("", media_url_for("original.jpg"))), + Some(&key), + ), ) .await .unwrap(); - // A second create with the same (user, key) is a duplicate: the DB unique - // constraint fires in the create transaction, rolling the whole thing back. - let err = perform_post_creation( - storage, - creation_with_key(user_id, parse_post_body("Second body"), Some("k")), - ) - .await - .unwrap_err(); + // The duplicate reaches the post, audience, and media writes before its + // idempotency insert collides; the transaction must roll every attempted row back. + let mut replay = creation_with_key( + user_id, + parse_post_body(&format!("", media_url_for("attempted.jpg"))), + Some(&key), + ); + replay.audiences = vec![AudienceTarget::Subscribers]; + let err = perform_post_creation(storage, replay).await.unwrap_err(); assert!(matches!(err, PerformCreationError::IdempotencyConflict)); - // No second post row committed — the user still has exactly one post. let posts = storage .list_collection_by_user(user_id, None, parse_row_limit("50")) .await .unwrap(); assert_eq!(posts.len(), 1); assert_eq!(posts[0].post_id, first.post_id); + assert_eq!( + fetch_post_media(&env.base, first.post_id).await, + vec![media_ref_for("original.jpg")] + ); + assert_eq!( + storage.get_post_audiences(first.post_id).await.unwrap(), + vec![AudienceTarget::Public] + ); + for table in ["posts", "post_audiences", "post_media", "idempotency_keys"] { + assert_eq!( + env.base + .pool() + .scalar_i64(&format!("SELECT COUNT(*) FROM {table}")) + .await + .unwrap(), + 1, + "the conflicting create left a row in {table}" + ); + } } // -- sanitization (#445) -- @@ -1624,22 +1661,24 @@ mod tests { let env = backend.setup().await; let user_id = SeedUser::new().seed(&env.state).await.user_id; let storage = &*env.state.posts; + let key = parse_idempotency_key("k"); + let missing_key = parse_idempotency_key("unknown"); let record = perform_post_creation( storage, - creation_with_key(user_id, parse_post_body("Body"), Some("k")), + creation_with_key(user_id, parse_post_body("Body"), Some(&key)), ) .await .unwrap(); let mapped = storage - .post_id_for_idempotency_key(user_id, "k") + .post_id_for_idempotency_key(user_id, &key) .await .unwrap(); assert_eq!(mapped, Some(record.post_id)); let missing = storage - .post_id_for_idempotency_key(user_id, "unknown") + .post_id_for_idempotency_key(user_id, &missing_key) .await .unwrap(); assert_eq!(missing, None); @@ -1652,17 +1691,18 @@ mod tests { let user_a = SeedUser::new().seed(&env.state).await.user_id; let user_b = SeedUser::new().seed(&env.state).await.user_id; let storage = &*env.state.posts; + let key = parse_idempotency_key("k"); // The same key string from two users creates two independent posts. let post_a = perform_post_creation( storage, - creation_with_key(user_a, parse_post_body("A body"), Some("k")), + creation_with_key(user_a, parse_post_body("A body"), Some(&key)), ) .await .unwrap(); let post_b = perform_post_creation( storage, - creation_with_key(user_b, parse_post_body("B body"), Some("k")), + creation_with_key(user_b, parse_post_body("B body"), Some(&key)), ) .await .unwrap(); @@ -1670,14 +1710,14 @@ mod tests { assert_eq!( storage - .post_id_for_idempotency_key(user_a, "k") + .post_id_for_idempotency_key(user_a, &key) .await .unwrap(), Some(post_a.post_id) ); assert_eq!( storage - .post_id_for_idempotency_key(user_b, "k") + .post_id_for_idempotency_key(user_b, &key) .await .unwrap(), Some(post_b.post_id) diff --git a/storage/src/posts.rs b/storage/src/posts.rs index d503317e..bfe05cce 100644 --- a/storage/src/posts.rs +++ b/storage/src/posts.rs @@ -10,6 +10,7 @@ use thiserror::Error; use crate::InstanceId; use common::etag::{ETag, post_content_etag}; use common::feed::FeedPath; +use common::idempotency_key::IdempotencyKey; use common::ids::{AudienceId, ChannelId, PostId, RevisionId, TagId, UserId}; use common::media::{MediaRef, MediaReference, MediaReferenceForm, MediaReferenceKind}; use common::pagination::RowLimit; @@ -346,7 +347,7 @@ pub struct CreatePostInput { /// If `Some`, register this idempotency key against the new post in the /// same transaction. A `(user_id, key)` collision maps to /// [`CreatePostError::IdempotencyConflict`] and rolls the whole create back. - pub idempotency_key: Option, + pub idempotency_key: Option, } /// What an update does to a Post's publication state. @@ -918,7 +919,7 @@ pub trait PostStorage: Send + Sync { async fn post_id_for_idempotency_key( &self, user_id: UserId, - key: &str, + key: &IdempotencyKey, ) -> Result, sqlx::Error>; /// Fetches a post by its ID, applying the viewer-resolution filter: the post @@ -1487,6 +1488,7 @@ where // bind, forwarded from `write_post_in_tx` (create paths). String: sqlx::Type, for<'q> String: sqlx::Encode<'q, DB>, + for<'q> &'q IdempotencyKey: sqlx::Encode<'q, DB> + sqlx::Type, for<'q> Option<&'q PostTitle>: sqlx::Encode<'q, DB> + sqlx::Type, // `summary` binds as `Option<&PostSummary>` via the ADR-0071 sqlx bridge on // the create paths, mirroring the `Option<&PostTitle>` bound above. @@ -1556,13 +1558,12 @@ where async fn post_id_for_idempotency_key( &self, user_id: UserId, - key: &str, + key: &IdempotencyKey, ) -> Result, sqlx::Error> { let post_id = sqlx::query_scalar::<_, PostId>( "SELECT post_id FROM idempotency_keys WHERE user_id = $1 AND key = $2", ) .bind(user_id) - // sqlx-newtype-bind:allow deferred-newtype #1086 — idempotency keys are tracked for a domain value. .bind(key) .fetch_optional(&self.pool) .await?; @@ -2925,6 +2926,7 @@ where for<'q> &'q str: sqlx::Encode<'q, DB> + sqlx::Type, for<'q> Option<&'q str>: sqlx::Encode<'q, DB> + sqlx::Type, for<'q> Option: sqlx::Encode<'q, DB> + sqlx::Type, + for<'q> &'q IdempotencyKey: sqlx::Encode<'q, DB> + sqlx::Type, for<'q> UtcInstant: sqlx::Encode<'q, DB> + sqlx::Type, for<'q> Option: sqlx::Encode<'q, DB> + sqlx::Type, // `Slug`/`PostBody` bind as themselves and `PostTitle` as `Option<&PostTitle>` @@ -2986,7 +2988,7 @@ where // an `IdempotencyConflict` (a duplicate create), distinct from the post // INSERT's `SlugConflict` above. Attribution is by which statement's // `map_err` fires, not by inspecting the constraint name. - if let Some(key) = input.idempotency_key.as_deref() { + if let Some(key) = input.idempotency_key.as_ref() { sqlx::query("INSERT INTO idempotency_keys (user_id, key, post_id) VALUES ($1, $2, $3)") .bind(input.user_id) .bind(key) From 760749f09e90bf1e088e390f75ba35422b7eb500 Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 07:32:22 -0400 Subject: [PATCH 3/9] docs(types): record IdempotencyKey contract (#1086) --- docs/ARCHITECTURE.md | 27 +++++++++++-------- .../0063-domain-value-newtype-convention.md | 16 +++++++++++ .../2026-08-26-issue-1086-idempotency-key.md | 2 +- 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7cc76b55..1f14e42e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -266,19 +266,24 @@ not canonicalized across backends ### Idempotent post creation -Post creation accepts an optional client-supplied idempotency key, so a retried +Post creation accepts an optional client-supplied +[`IdempotencyKey`](adr/0063-domain-value-newtype-convention.md), so a retried AtomPub POST does not create a duplicate post — the mechanism decided in issue [#79](https://github.com/jaunder-org/jaunder/issues/79) as a follow-on to -ADR-0047, not in an ADR of its own. The `idempotency_keys` table (migration -`0023_create_idempotency_keys`, `UNIQUE(user_id, key)`) is written in the same -transaction as the post (`storage/src/posts.rs:2274`); a duplicate key surfaces -as `CreatePostError::IdempotencyConflict` and is deliberately _not_ retried as a -slug collision (`storage/src/post_service.rs:466`), and -`PostStorage::post_id_for_idempotency_key` maps a replayed key back to the post -it originally created. AtomPub is its only caller -(`server/src/atompub/posts.rs:366`); the web composer passes -`idempotency_key: None` (`web/src/posts/api.rs:188`), so the mechanism is a -machine-client contract, not a browser one. +ADR-0047. At the AtomPub boundary, a missing header, a value rejected by +`HeaderValue::to_str` (including non-ASCII UTF-8 bytes and invalid UTF-8), or +text that is blank after trimming means no key rather than a `400`. A readable, +non-blank value is parsed once into an owned `IdempotencyKey`; typed borrowed +keys carry it through post creation and duplicate lookup, and the owned type is +bound for persistence. + +The existing `idempotency_keys` table needs no schema migration: it stores the +key as `TEXT NOT NULL` and enforces `UNIQUE(user_id, key)`. A fresh keyed create +writes its post and key row atomically; a uniqueness collision rolls the +attempted creation back. The fresh keyed create returns `201`; when its original +post remains available, same-user key reuse returns that original post as `200`, +even when the new payload differs. Another user may use the same key +independently, and key rows are retained indefinitely. ### Testing (summary) diff --git a/docs/adr/0063-domain-value-newtype-convention.md b/docs/adr/0063-domain-value-newtype-convention.md index 91d37f5d..a207efc7 100644 --- a/docs/adr/0063-domain-value-newtype-convention.md +++ b/docs/adr/0063-domain-value-newtype-convention.md @@ -436,6 +436,22 @@ values with no serialization hop and so take the newtype like any other surface we define (`CapturedPing`, `server/tests/helpers/websub_capturing.rs`). The distinction is whether the double is observing bytes or receiving values. +**2026-08-27 adoption — `IdempotencyKey`.** Issue #1086 adopts `IdempotencyKey` +as a non-secret string-backed domain value. It prevents a retry key from being +transposed with another string while it crosses AtomPub, post creation, +duplicate lookup, and persistence. Its `FromStr` trims outer whitespace, rejects +an empty result, and preserves every other string; the standard trailer supplies +the validating serde and SQLx bridges. + +AtomPub retains one compatibility seam before that outermost parse: +`HeaderValue::to_str` may reject a header value (including non-ASCII UTF-8 bytes +and invalid UTF-8), and missing, rejected, or blank-after-trimming values mean +no key rather than a `400`. Only readable, non-blank header text becomes an +owned `IdempotencyKey`; every Jaunder-defined surface thereafter remains typed, +with borrowed keys for orchestration and lookup and owned keys for creation +input and persistence. This type-only adoption needs no schema migration. The +frozen #697 historical artifacts remain unchanged. + ## Consequences - **One decision surface.** "Does this value deserve a type, and what shape does diff --git a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md index 3e340e86..1a8d2634 100644 --- a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md +++ b/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md @@ -45,7 +45,7 @@ Out: whitespace-only, non-ASCII UTF-8 bytes, invalid UTF-8 bytes, and no-key behavior through real HTTP requests. -- [ ] Task 3: Record the completed domain contract +- [x] Task 3: Record the completed domain contract - Depends on: Tasks 1–2. - Contract: ADR-0063 and `docs/ARCHITECTURE.md` describe the type, compatibility boundary, per-user persistence, and replay semantics; frozen From 22c6055b7670c16261f378987259e8835ae39bd8 Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 07:44:58 -0400 Subject: [PATCH 4/9] fix(storage): validate restored idempotency keys (#1086) --- storage/src/backup/restore_validation.rs | 47 +++++++++++++++++++++--- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/storage/src/backup/restore_validation.rs b/storage/src/backup/restore_validation.rs index 99563816..dfd2f4e1 100644 --- a/storage/src/backup/restore_validation.rs +++ b/storage/src/backup/restore_validation.rs @@ -8,6 +8,7 @@ use common::config_key::{SiteConfigKey, UserConfigKey}; use common::display_name::DisplayName; use common::email::Email; use common::feed::{FeedEventStatus, FeedPath}; +use common::idempotency_key::IdempotencyKey; use common::media::{ ByteSize, ContentHash, ContentType, Filename, MediaReferenceForm, MediaReferenceKind, MediaSource, @@ -91,6 +92,9 @@ pub(crate) fn validate_restore_row( validate_typed_restore_row::(row, report); } "feed_events" => validate_typed_restore_row::(row, report), + "idempotency_keys" => { + validate_typed_restore_row::(row, report); + } "invites" => validate_typed_restore_row::(row, report), "media" => validate_typed_restore_row::(row, report), "password_resets" => validate_typed_restore_row::(row, report), @@ -248,6 +252,10 @@ typed_restore_row!(FeedEventsRestoreRow, "feed_events" { status: FeedEventStatus => ("status", "feed event status"), }); +typed_restore_row!(IdempotencyKeysRestoreRow, "idempotency_keys" { + key: IdempotencyKey => ("key", "idempotency key"), +}); + typed_restore_row!(InvitesRestoreRow, "invites" { code: InviteCode => ("code", "invite code"), }); @@ -437,6 +445,7 @@ pub(crate) const RESTORE_COLUMN_COVERAGE: &[RestoreColumnCoverage] = &[ RestoreBadValue::Text("not a feed"), ), covered("feed_events", "status", RestoreBadValue::Text("sideways")), + covered("idempotency_keys", "key", RestoreBadValue::Text("")), covered("invites", "code", RestoreBadValue::Text("!")), covered("media", "sha256", RestoreBadValue::Text("bad")), covered("media", "filename", RestoreBadValue::Text("my photo.jpg")), @@ -512,11 +521,6 @@ pub(crate) const RESTORE_COLUMN_COVERAGE: &[RestoreColumnCoverage] = &[ "author_user_id", "foreign-key id: schema validation preserves the value", ), - primitive( - "idempotency_keys", - "key", - "primitive restore until idempotency keys are typed (#1086)", - ), primitive( "post_revisions", "rendered_html", @@ -730,6 +734,39 @@ mod tests { } } + #[test] + fn idempotency_key_restore_row_accepts_valid_key() { + let mut row = serde_json::Map::new(); + row.insert("key".to_owned(), serde_json::json!("retry-key")); + let mut report = RestoreValidationReport::default(); + + validate_restore_row("idempotency_keys", &row, &mut report); + + assert!(report.is_empty(), "valid key produced {report:?}"); + } + + #[test] + fn idempotency_key_restore_row_reports_blank_keys() { + for key in ["", " \t\n"] { + let mut row = serde_json::Map::new(); + row.insert("key".to_owned(), serde_json::json!(key)); + let mut report = RestoreValidationReport::default(); + + validate_restore_row("idempotency_keys", &row, &mut report); + + assert_eq!( + report.issues(), + &[RestoreValidationIssue { + table: "idempotency_keys".to_owned(), + column: "key".to_owned(), + value_class: "idempotency key".to_owned(), + reason: "idempotency key must be non-empty".to_owned(), + }], + "key {key:?}" + ); + } + } + #[test] fn media_filename_diagnostic_names_the_noncanonical_value_class() { let mut row = serde_json::Map::new(); From 037e9dc5319381602570aa53a03f2401540724e9 Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 07:58:25 -0400 Subject: [PATCH 5/9] fix(common): complete IdempotencyKey trailer (#1086) --- common/src/idempotency_key.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/src/idempotency_key.rs b/common/src/idempotency_key.rs index 316da6d7..060995c7 100644 --- a/common/src/idempotency_key.rs +++ b/common/src/idempotency_key.rs @@ -11,7 +11,7 @@ use thiserror::Error; /// type. The ADR-0063 string-newtype trailer (`Display`, `AsRef`/`Borrow`/ /// `Deref`, owned `String` conversions, `PartialEq`/`<&str>`, ordering, /// and validating serde and sqlx bridges) is generated by `#[derive(StrNewtype)]`. -#[derive(Clone, Debug, PartialEq, Eq, StrNewtype)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, StrNewtype)] pub struct IdempotencyKey(String); /// Error returned when a string cannot be parsed as an [`IdempotencyKey`]. From 49c2205e059910d3a084c2ccf3b390897b59c28f Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 08:00:59 -0400 Subject: [PATCH 6/9] docs(#1086): archive idempotency key cycle --- .../2026-08-26-issue-1086-idempotency-key-plan.md} | 0 .../2026-08-26-issue-1086-idempotency-key-spec.md} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename docs/{superpowers/plans/2026-08-26-issue-1086-idempotency-key.md => archive/2026-08-26-issue-1086-idempotency-key-plan.md} (100%) rename docs/{superpowers/specs/2026-08-26-issue-1086-idempotency-key.md => archive/2026-08-26-issue-1086-idempotency-key-spec.md} (100%) diff --git a/docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md b/docs/archive/2026-08-26-issue-1086-idempotency-key-plan.md similarity index 100% rename from docs/superpowers/plans/2026-08-26-issue-1086-idempotency-key.md rename to docs/archive/2026-08-26-issue-1086-idempotency-key-plan.md diff --git a/docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md b/docs/archive/2026-08-26-issue-1086-idempotency-key-spec.md similarity index 100% rename from docs/superpowers/specs/2026-08-26-issue-1086-idempotency-key.md rename to docs/archive/2026-08-26-issue-1086-idempotency-key-spec.md From e03efd963801edc1b2efe9bca06e6c78fc36b05d Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 08:17:09 -0400 Subject: [PATCH 7/9] fix(storage): preserve idempotency invariants after rebase (#1086) --- storage/src/backup/restore_validation.rs | 62 ++++++++++++++++++++++-- storage/src/post_service.rs | 9 +++- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/storage/src/backup/restore_validation.rs b/storage/src/backup/restore_validation.rs index dfd2f4e1..652f62ba 100644 --- a/storage/src/backup/restore_validation.rs +++ b/storage/src/backup/restore_validation.rs @@ -252,9 +252,45 @@ typed_restore_row!(FeedEventsRestoreRow, "feed_events" { status: FeedEventStatus => ("status", "feed event status"), }); -typed_restore_row!(IdempotencyKeysRestoreRow, "idempotency_keys" { - key: IdempotencyKey => ("key", "idempotency key"), -}); +struct IdempotencyKeysRestoreRow { + key: Option, +} + +impl RestoreTableRow for IdempotencyKeysRestoreRow { + fn from_restore(row: &RestoreRowMap) -> Self { + Self { + key: restore_text(row, "key"), + } + } + + fn validate(&self, report: &mut RestoreValidationReport) { + let Some(raw_key) = &self.key else { + return; + }; + let key = match raw_key.as_str().parse::() { + Ok(key) => key, + Err(error) => { + push_issue( + report, + "idempotency_keys", + "key", + "idempotency key", + error.to_string(), + ); + return; + } + }; + if key.as_ref() != raw_key.as_str() { + push_issue( + report, + "idempotency_keys", + "key", + "idempotency key", + "idempotency key must be canonical (without surrounding whitespace)", + ); + } + } +} typed_restore_row!(InvitesRestoreRow, "invites" { code: InviteCode => ("code", "invite code"), @@ -767,6 +803,26 @@ mod tests { } } + #[test] + fn idempotency_key_restore_row_reports_padded_keys() { + let mut row = serde_json::Map::new(); + row.insert("key".to_owned(), serde_json::json!(" retry-key\t")); + let mut report = RestoreValidationReport::default(); + + validate_restore_row("idempotency_keys", &row, &mut report); + + assert_eq!( + report.issues(), + &[RestoreValidationIssue { + table: "idempotency_keys".to_owned(), + column: "key".to_owned(), + value_class: "idempotency key".to_owned(), + reason: "idempotency key must be canonical (without surrounding whitespace)" + .to_owned(), + }] + ); + } + #[test] fn media_filename_diagnostic_names_the_noncanonical_value_class() { let mut row = serde_json::Map::new(); diff --git a/storage/src/post_service.rs b/storage/src/post_service.rs index 347a446e..77002319 100644 --- a/storage/src/post_service.rs +++ b/storage/src/post_service.rs @@ -497,6 +497,7 @@ mod tests { Backend, SeedUser, backends, fetch_post_media, media_ref_for, media_url_for, seed_media, }; use common::idempotency_key::IdempotencyKey; + use common::media::{MediaReferenceForm, MediaReferenceKind}; use common::test_support::{parse_post_body, parse_post_title, parse_row_limit, parse_slug}; use rstest::*; use rstest_reuse::*; @@ -1592,7 +1593,13 @@ mod tests { assert_eq!(posts[0].post_id, first.post_id); assert_eq!( fetch_post_media(&env.base, first.post_id).await, - vec![media_ref_for("original.jpg")] + vec![( + media_ref_for("original.jpg"), + MediaReferenceKind::Local, + media_url_for("original.jpg") + .parse::() + .expect("valid media reference form"), + )] ); assert_eq!( storage.get_post_audiences(first.post_id).await.unwrap(), From 88ad08d3d8fd86da0d7c5a5dd3e3b408ecf23382 Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 08:48:28 -0400 Subject: [PATCH 8/9] test(storage): cover missing restored idempotency key (#1086) --- storage/src/backup/restore_validation.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/storage/src/backup/restore_validation.rs b/storage/src/backup/restore_validation.rs index 652f62ba..4b8b1d80 100644 --- a/storage/src/backup/restore_validation.rs +++ b/storage/src/backup/restore_validation.rs @@ -770,6 +770,16 @@ mod tests { } } + #[test] + fn idempotency_key_restore_row_allows_missing_key_for_structural_validation() { + let row = serde_json::Map::new(); + let mut report = RestoreValidationReport::default(); + + validate_restore_row("idempotency_keys", &row, &mut report); + + assert!(report.is_empty(), "missing key produced {report:?}"); + } + #[test] fn idempotency_key_restore_row_accepts_valid_key() { let mut row = serde_json::Map::new(); From 2d7b4717e90217125ed4126653172173128bdd6f Mon Sep 17 00:00:00 2001 From: Michael Alan Dorman Date: Thu, 27 Aug 2026 09:57:37 -0400 Subject: [PATCH 9/9] refactor(atompub): name idempotency header adapter (#1086) --- server/src/atompub/posts.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/src/atompub/posts.rs b/server/src/atompub/posts.rs index 67875d20..01489006 100644 --- a/server/src/atompub/posts.rs +++ b/server/src/atompub/posts.rs @@ -121,6 +121,15 @@ fn if_match_satisfied(headers: &HeaderMap, etag: &ETag) -> bool { None => true, } } + +/// Parses the optional retry key while preserving `AtomPub`'s compatibility policy: +/// unreadable or blank headers do not opt the request into deduplication. +fn idempotency_key_from_headers(headers: &HeaderMap) -> Option { + headers + .get("idempotency-key") + .and_then(|value| value.to_str().ok()?.parse().ok()) +} + fn scalar_presence(value: Option<&T>) -> Presence { value.cloned().map_or(Presence::Absent, Presence::Present) } @@ -479,12 +488,7 @@ pub async fn collection_post( Presence::Present(audiences) => audiences, Presence::Absent => vec![site_config.get_default_audience().await?.into()], }; - // A client-supplied idempotency key dedups a retried create (duplicate-on-retry). - // Preserve the historical `HeaderValue::to_str` compatibility boundary: - // unreadable or blank values do not opt a request into deduplication. - let idempotency_key = headers - .get("idempotency-key") - .and_then(|value| value.to_str().ok()?.parse::().ok()); + let idempotency_key = idempotency_key_from_headers(&headers); let created = storage::perform_post_creation( posts,