Commit Graph
5 Commits
Author SHA1 Message Date
awkawbandClaude Opus 5 935690c8f0 Make the stable-toolchain signal visible
The advisory job was marked `continue-on-error`, which GitHub renders as
a green check with an annotation — a signal nobody reads. Removed, so a
failure shows as a real red X.

What now keeps it from blocking merges is branch protection: it has to
stay out of the required status checks. That is a repo setting rather
than anything in this file, so it is called out in ci.yml and CLAUDE.md.
Expect the job red occasionally through no fault of the change under
test; that is the cost of it being visible.

Also runs weekly (Mondays 06:00 UTC). A Rust release lands on its own
schedule, so on push/PR alone breakage could sit unnoticed for weeks.
The pinned `test` and `msrv` jobs skip scheduled runs — a pinned
toolchain against a locked dependency graph cannot change on a timer.

Adds a top-level `permissions: contents: read`, since the workflow needs
nothing more.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-23 19:14:44 -04:00
awkawbandClaude Opus 5 aa802bc40c Add an advisory CI job on latest stable
Pinning CI meant a future Rust release could break the build without
anyone noticing until the pin moved. This job tracks latest stable and
runs the test suite and clippy, so that breakage is visible early.

Marked `continue-on-error`, so it reports rather than gates: a new
clippy lint landing in stable should not fail a pull request that did
not cause it. The pinned `test` job remains the gate.

Note that GitHub renders a `continue-on-error` job as passing with an
annotation rather than as failed. If it should be more visible, make it
a required check and drop the flag.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-23 19:01:42 -04:00
awkawbandClaude Opus 5 ad9553516e Pin the Rust toolchain and declare an MSRV of 1.88
The dev shell tracked `stable.latest` and CI used `@stable`, so nothing
pinned a version and nothing declared a floor. A consumer on an older
toolchain hit a confusing error deep in a dependency rather than cargo
saying the version was too old, and a new stable release could turn a
lint into an unrelated red build.

- `rust-version = "1.88"` in Cargo.toml. The floor comes from the
  dependency graph, not this crate's code: reqwest pulls in url -> idna
  -> idna_adapter -> icu_*, and the icu crates require 1.88. Verified by
  running the full suite against 1.88, not inferred.
- flake.nix pins the dev toolchain to 1.99.1 and adds an `msrv` devShell,
  so the floor can be rechecked with
  `nix develop .#msrv --command cargo test`.
- CI pins the main job to the same 1.99.1 and adds a job building on
  1.88. Lint and format stay on the pinned toolchain only, since their
  output varies by release.

Note the tradeoff: with CI pinned, a future stable that breaks the build
will not be noticed until the pin moves. Worth adding a non-blocking
`stable` job if that matters.

CLAUDE.md records the three places that must agree when either version
moves, and how to recheck the floor after a dependency bump.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-23 18:46:32 -04:00
awkawbandClaude Opus 5 8ff947455e Harden error reporting, retry bounds, and credential handling
Second hardening pass, over the code added in 23c660d.

Error reporting:
- Decode failures now carry an excerpt of the body that arrived. The
  serde message alone ("expected value at line 1 column 1") gives no hint
  that the response was an HTML error page from a proxy or a login
  redirect from a wrong base URL. The excerpt is collapsed to one line
  and truncated so a large body cannot become a large error.
- API errors with an empty or message-less body report the status reason
  ("Not Found") rather than rendering blank.
- `Error` is `#[non_exhaustive]`, so future variants are not breaking.

Retry bounds:
- `RetryPolicy::max_elapsed` (120s default) bounds the time one call may
  spend across attempts. The request timeout is per attempt, so the
  previous defaults could spend four minutes on a call that kept timing
  out — a surprise for anyone reading `with_timeout(60)` as a bound on
  the call.
- `backoff` uses saturating arithmetic; a large `max_delay` could
  overflow the jitter span and panic in a debug build.
- Response bodies are drained before a retry so the connection returns to
  the pool instead of the next attempt opening a fresh one.

Credentials:
- Test pinning that neither the token nor the password appears in `Debug`
  output. `reqwest`'s `Client` prints its default headers, so the
  sensitive flag set when building them is the only thing keeping the
  token out of logs, and nothing enforced that.

Also drops `Option::is_none_or`, which raised the minimum toolchain to
1.82 for no benefit over a match.

91 tests, up from 80.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-23 18:33:43 -04:00
awkawbandClaude Opus 5 23c660d2f9 Add retry with backoff, and narrow the untyped responses
Retry:
- `RetryPolicy` (3 retries, full-jitter exponential backoff) applied in a
  new `execute` dispatch chokepoint, configurable via
  `with_retry_policy`. `Retry-After` is honoured, and one longer than
  `max_delay` is surfaced rather than slept through.
- What may be replayed depends on the method, not just the status. A 429
  means the server refused the request without processing it, so any
  method is safe. A 5xx or timeout gives no such guarantee, so only
  idempotent methods are retried: this is a billing API, and replaying a
  POST the server had already applied would duplicate an invoice,
  payment, or email.

Response types:
- `statics`, `search`, and `health_check` return `Statics`,
  `SearchResults`, and `HealthCheck` instead of being generic.
  `HealthCheck` covers both deployments — hosted returns a bare message
  rather than a report — and `SearchResults` covers both the
  Elasticsearch and non-Elasticsearch bucket sets.
- Each keeps the `extra` convention, and `statics_as` / `search_as` /
  `health_check_as` remain for pinning shapes left untyped. Nested config
  blobs (gateways, templates, extensions) stay `Value`, same reasoning as
  `Company::settings`.
- `send_email` previously decoded the response as a bare body, but the
  controller returns `itemResponse(...)` — a `data`-wrapped entity. It now
  unwraps, so `T` is the entity. Still generic, since the endpoint serves
  invoices, quotes, credits, and purchase orders.
- `chart` stays generic on purpose: the eleven charts share no response
  shape.
- Adds `id_as_string`, since `/statics` returns `payment_types.id` as a
  string and the other tables return numbers.

Adds CHANGELOG.md (Keep a Changelog), and a CLAUDE.md note to keep it
current. 17 new tests, 43 in client_requests.rs total.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-23 18:18:12 -04:00