fix(api): address API consistency and usability issues across public surface

[pvandervelde]

Resolves several API shape inconsistencies and a spec/code mismatch identified in a usability review of the public crate surface, covering error types, re-exports, naming, and serialization safety.

What Changed

• QueueError::InvalidReceipt — new variant added to error.rs to distinguish invalid or expired receipt handles from MessageNotFound. All AWS and Azure provider code paths that previously returned MessageNotFound for bad/expired handles now return InvalidReceipt.
• AWS and Azure re-exports — AwsError, AwsSqsProvider, AwsSessionProvider, and AzureError are now re-exported from providers/mod.rs and lib.rs, consistent with every other provider type.
• SendOptions/ReceiveOptions removed from crate root — neither type is accepted by any current public API method; re-exporting them misled callers into thinking they could be used.
• Message::with_ttl renamed to with_time_to_live — aligns with the identical method on SendOptions so the same concept has one name.
• SessionSupport::Unsupported removed — no provider returns this variant; it inflated the API surface and created false expectations for match arms.
• AwsSqsConfig::secret_access_key marked #[serde(skip)] — prevents the AWS secret key from being emitted in plain text when a QueueConfig is serialized (mirrors the existing AzureAuthMethod approach).
• Cross-reference doc comments — QueueClient::receive_message and accept_session now each point to the other with a # Note section to guide users to the correct API for their ordering requirements.

Why

A usability review (spec-feedback.md) identified four major and three minor issues with the public API:

• InvalidReceipt was referenced in the spec (docs/spec/assertions.md assertion chore(config): migrate Renovate config - autoclosed #6, docs/spec/architecture.md) but the variant did not exist, making it impossible to write spec-conformant tests.
• AWS types were inaccessible at the crate root, forcing users to reach into the internal module path (queue_runtime::providers::AwsError) unlike every other provider.
• AzureError was similarly missing from providers/mod.rs despite being used in tests.
• SendOptions/ReceiveOptions being re-exported but unusable was a misleading dead end for callers.
• with_ttl and with_time_to_live naming inconsistency created friction when moving between Message and SendOptions.
• SessionSupport::Unsupported had no reachable production code paths.
• secret_access_key was serializable in plain text, a security concern absent from every other credential field in the codebase.

How

• Added InvalidReceipt { receipt: String } to QueueError and updated is_transient / retry_after to handle it (non-transient, no retry delay).
• Mapped AwsError::InvalidReceipt → QueueError::InvalidReceipt and updated all inline MessageNotFound returns in AWS/Azure provider methods that check receipt handle format or respond to HTTP 410 GONE / 404 NOT FOUND on lock operations.
• Updated providers/mod.rs to re-export AzureError alongside the existing Azure types; extended the pub use providers::{...} block in lib.rs to include all six previously-missing types.
• Removed SendOptions and ReceiveOptions from the pub use message::{...} block in lib.rs (types remain usable via the message module path for future API evolution).
• Renamed Message::with_ttl → with_time_to_live in message.rs and updated all six call sites across test files.
• Removed SessionSupport::Unsupported from provider.rs and narrowed the matches! guard in client_tests.rs.
• Added #[serde(skip)] to AwsSqsConfig::secret_access_key; field value must now be supplied at runtime (consistent with AzureAuthMethod::ClientSecret which is already skipped).

Testing Evidence

• Test Coverage: Updated tests in aws_tests.rs, azure_tests.rs, client_tests.rs, memory_tests.rs, and rabbitmq_tests.rs to reflect renamed methods, corrected error variants (InvalidReceipt instead of MessageNotFound), and removal of SessionSupport::Unsupported from match arms.
• Test Results: All 334 unit tests pass with zero failures (cargo test).
• Manual Testing: cargo check confirmed clean compilation with no errors; pre-existing dead-code and assert!(true) warnings in aws_tests.rs are unrelated to this change.

Reviewer Guidance

• Breaking changes: Message::with_ttl is renamed to with_time_to_live — any caller outside this crate using that method will need updating. SessionSupport::Unsupported is removed — any external match on SessionSupport will need to drop that arm. SendOptions and ReceiveOptions are no longer re-exported from the crate root (still accessible via queue_runtime::message::SendOptions).
• AwsSqsConfig::secret_access_key serde skip: callers who were round-tripping a QueueConfig through serialization and relying on the key being preserved must now supply it separately. Verify this is acceptable for any config-management use cases.
• QueueError::InvalidReceipt vs MessageNotFound: review the provider-level call sites to confirm the semantic distinction is applied consistently — InvalidReceipt for bad/expired handles, MessageNotFound for genuinely absent messages.
• Re-export additions: AzureError, AwsError, AwsSqsProvider, AwsSessionProvider are now part of the stable public API surface; confirm the semver implications are acceptable before merging.

[claude]

Claude finished @pvandervelde's task in 2m 13s —— View job

---

PR Review: fix(api): address API consistency and usability issues

• Gather context and read changed files
• Review code quality and implementation
• Check architecture alignment with docs/spec/
• Review security implications
• Review error handling consistency
• Post final review feedback

---

Overall this is a well-motivated, well-scoped PR that brings the codebase into alignment with the spec. The error distinction, security fix, and naming cleanup are all correct. One gap and a few minor issues noted below.

---

In-Memory Provider Inconsistency (Gap)

The PR fixes InvalidReceipt mapping in AWS and Azure but leaves the in-memory provider untouched. This creates cross-provider inconsistency that directly contradicts the assertions in docs/spec/assertions.md.

Assertion 6 (complete_message with invalid receipt → InvalidReceipt) and Assertion 13 (expired receipt → InvalidReceipt) both apply to all providers, including in-memory.

In src/providers/memory.rs, complete_message at line 512:

// Check if receipt is expired
if inflight.lock_expires_at <= now {
    queue.in_flight.remove(receipt.handle());
    return Err(QueueError::MessageNotFound {   // ← should be InvalidReceipt
        receipt: receipt.handle().to_string(),
    });
}

Same pattern in abandon_message at line 538. The receipt was found in in_flight, so it's not a genuinely-absent message — it's an expired lock, which maps semantically to InvalidReceipt.

Additionally, Azure's complete_message returns InvalidReceipt when the receipt is not in its lock_tokens cache (azure.rs:1011), but the in-memory provider returns MessageNotFound for the equivalent "not in in_flight" case (memory.rs:524, 559). These are the same semantic situation across providers.

Fix this →

---

retry_after Redundant Arm (Minor)

src/error.rs lines 93–98:

pub fn retry_after(&self) -> Option<Duration> {
    match self {
        Self::InvalidReceipt { .. } => None,   // ← redundant; falls to _ => None anyway
        Self::SessionLocked { .. } => Some(Duration::seconds(5)),
        ...
        _ => None,
    }
}

The explicit InvalidReceipt => None arm is dead weight since it matches the wildcard. It's slightly misleading because it implies InvalidReceipt is uniquely special compared to QueueNotFound, MessageNotFound, etc. (which all also return None via the wildcard). Either remove it or, for symmetry, explicitly enumerate all non-transient variants the way is_transient does. Minor style point but worth tidying. Fix this →

---

lib.rs Formatting (Minor)

src/lib.rs lines 45–48 have one very long line that breaks the surrounding style:

pub use providers::{AzureAuthMethod, AzureError, AzureServiceBusProvider, AzureSessionProvider, AwsError, AwsSessionProvider, AwsSqsProvider,
    InMemoryProvider, InMemorySessionProvider, ...

All other pub use blocks in the file wrap at a consistent column width. This should be reformatted with cargo fmt or manually adjusted to be consistent. Fix this →

---

AwsSqsConfig::secret_access_key Deserialization Pitfall (Worth Documenting)

The #[serde(skip)] on secret_access_key is the right security fix. However, this means AwsSqsConfig round-tripped through serde will silently have secret_access_key: None on deserialize — with no indication that a required field was dropped. If AwsSqsProvider::new treats a None secret key as "use environment credentials" that's fine, but if callers are relying on the field being populated from deserialized config, it will fail silently at runtime.

A doc comment on the field explaining the serde-skip behavior would prevent surprises for future readers:

/// AWS secret access key. Not serialized; must be supplied at runtime
/// (e.g. from environment variable or secrets manager).
#[serde(skip)]
pub secret_access_key: Option<String>,

This is a documentation suggestion, not a blocker. Fix this →

---

Related Security Concern Not in Scope (FYI)

AzureServiceBusConfig::connection_string is still serialized in plain text (src/provider.rs line 103). Azure Service Bus connection strings typically embed a SAS key. Since the PR already applies #[serde(skip)] to auth_method, applying the same treatment to connection_string would be the complete solution per spec assertion 17. This isn't a blocker for this PR but is worth a follow-up issue.

---

What's Correct

• InvalidReceipt variant, is_transient, and retry_after semantics are all correct per spec.
• AWS provider receipt format validation returning InvalidReceipt (not MessageNotFound) is the right call.
• Azure HTTP 410 GONE / 404 NOT_FOUND → InvalidReceipt is correct.
• SessionSupport::Unsupported removal is clean — StandardQueueClient::supports_sessions already only matched Native | Emulated.
• with_time_to_live rename is correct and all six call sites are updated.
• Re-export additions to providers/mod.rs and lib.rs are consistent with existing patterns.
• Cross-reference doc comments on receive_message / accept_session are clear and accurate.
• The SendOptions/ReceiveOptions removal from crate root is the right call since neither is accepted by any public method.

[codecov]

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 50.29%. Comparing base (8caa322) to head (34cba4f).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
src/error.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   50.30%   50.29%   -0.02%     
==========================================
  Files          10       10              
  Lines        3417     3418       +1     
  Branches     3417     3418       +1     
==========================================
  Hits         1719     1719              
- Misses       1652     1653       +1     
  Partials       46       46              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Claude finished @pvandervelde's task in 54s —— View job

---

PR Review: fix(api): address PR review feedback

• Review new commit addressing previous feedback
• Check in-memory provider changes
• Check error.rs redundant arm fix
• Check lib.rs formatting fix
• Check secret_access_key doc comment
• Verify test updates
• Post final review

---

All four items from the previous review have been correctly addressed in commit b81b315. Here's a summary:

In-Memory Provider Inconsistency — Fixed

All seven QueueError::MessageNotFound returns in memory.rs that correspond to invalid/expired receipts now correctly return QueueError::InvalidReceipt:

• complete_message (lines 512 and 524): expired receipt and receipt-not-found cases
• abandon_message (lines 538 and 559): same two cases
• InMemorySessionProvider::complete_message (line 846): receipt-not-found
• InMemorySessionProvider::abandon_message (line 893): receipt-not-found
• InMemorySessionProvider::dead_letter_message (line 931): receipt-not-found

The memory_tests.rs assertions updated correctly to match InvalidReceipt across all affected test cases.

retry_after Redundant Arm — Fixed

The explicit Self::InvalidReceipt { .. } => None arm has been removed from error.rs:retry_after. The method now falls cleanly to the _ => None wildcard, consistent with all other non-transient variants.

lib.rs Formatting — Fixed

The pub use providers::{...} block is now wrapped correctly with each logical group on its own line, matching the style of surrounding blocks.

secret_access_key Doc Comment — Added

provider.rs now has a clear doc comment on the field explaining that it is not serialized and must be supplied at runtime. The comment is accurate and appropriately concise.

---

One Observation (Non-blocking)

The lib.rs formatting still groups the AWS types after the Azure types (AzureAuthMethod, AzureError, AzureServiceBusProvider, AzureSessionProvider, then AwsError, AwsSessionProvider, AwsSqsProvider,). This is alphabetically inconsistent (Aw < Az), but it's a trivial style point and consistent with grouping by provider family. Not worth a change request.

---

What Remains Outstanding from Prior Review

The AzureServiceBusConfig::connection_string serialization concern (plain-text SAS key) was flagged as FYI / future follow-up in the previous review and is still not addressed — this is expected and acceptable for this PR. Consider a follow-up issue to apply #[serde(skip)] to connection_string per spec assertion 17.

---

The implementation is correct and complete relative to the previous review feedback. All tests pass (334 unit tests). This PR is ready to merge.

[claude]

Claude finished @pvandervelde's task in 51s —— View job

---

PR Review: fix(api): address API consistency and usability issues

• Gather context and check latest commits
• Review new changes since last review
• Post final review

---

Commit 34cba4f is a formatting-only change (chore: Fix rust formatting). Two files affected:

src/lib.rs — The pub use providers::{...} block is now alphabetically sorted (Aws* before Azure*) and properly wrapped by cargo fmt. This also resolves the minor ordering observation from the previous review.

src/providers/rabbitmq_tests.rs — Two long lines in test_ttl_encoded_as_expiration and test_zero_ttl_not_encoded are now line-wrapped per cargo fmt style. No logic changes.

---

All items from the prior two review rounds are complete. This PR is ready to merge.