fix: never retry non-idempotent requests after ambiguous failures

[jonmarkgo]

Problem

APIRequestor#request retries transient failures without considering the HTTP method. retryable_error? branches only on error class and status:

rescue Forem::APIConnectionError
  if retries_left > 0
    retry            # ← no check of the verb
rescue Forem::RateLimitError, Forem::APIError => e
  if retries_left > 0 && retryable_error?(e)

A network error or a 5xx leaves it unknown whether the server processed the request. Replaying a POST in that state can create a second resource — so client.articles.create(...) can silently publish a duplicate article under a user's name when a request times out after Forem accepted it.

max_network_retries defaults to 1, so this is on by default for every consumer.

Change

Retries are now gated on idempotency:

• APIConnectionError and 5xx are replayed only for idempotent methods — GET, HEAD, PUT, DELETE, OPTIONS, TRACE (RFC 9110). A POST raises to the caller instead.
• 429 is still retried for every method, including POST. A rate-limited request was rejected without being processed, so replaying it cannot duplicate anything. This distinction is the reason the fix isn't simply "don't retry POST" — it keeps the genuinely useful rate-limit behaviour intact.
• Callers whose POST endpoint deduplicates server-side (idempotency key, natural key) can opt back in per request with opts[:idempotent] = true.

No configuration change is required, and idempotent requests behave exactly as before.

Tests

Six new tests in test/forem/api_requestor_test.rb, using a counting_requestor helper that reports how many attempts actually reached the transport:

• POST is not retried after a 5xx
• POST is not retried after a connection error
• PUT is still retried after a 5xx
• POST is retried after a 429
• POST is retried when explicitly marked idempotent
• the :idempotent option is consumed, not leaked as a request header

Full suite: 330 runs, 975 assertions, 0 failures.

I also checked the tests fail without the fix — reverting just the two guard conditions fails exactly the two POST-safety tests (330 runs, 2 failures) and nothing else, so they're pinning the behaviour rather than passing vacuously.

Why we hit this

We're building a delegated write proxy in MLH Core that forwards article creates to Forem on a user's behalf. We had to hand-roll a Faraday transport specifically to get POST-never-retry semantics, duplicating a good deal of what APIRequestor already does well. With this fix we can drop that and use the gem — and, more importantly, the safety property is enforced in the client for every consumer rather than re-implemented by each one.

Happy to adjust the naming or the opt-in mechanism if you'd prefer a different shape.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}