Skip to content

fix(adapters)!: return each adapter's own MissingFeature, not rind's - #420

Merged
kixelated merged 3 commits into
mainfrom
fix/adapter-missing-feature
Nov 6, 2026
Merged

kixelated merged 3 commits into
mainfrom
fix/adapter-missing-feature

Conversation

@kixelated

@kixelated kixelated commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

AGENTS.md puts each third-party adapter in its own rind-<consumer> crate so a major bump on one side never forces one on the other. But four adapters name a rind type in their public API, and rind-ring lets generic code reach one, so rind 0.2.0 (#367) would force their majors too:

  • rind_rustls::default_provider() and empty_provider(), rind_dimpl::default_provider(), and rind_object_store::default_provider() return rind::cpu::MissingFeature.
  • rind-ring re-exports rind::cpu (unreleased, from feat(rind-ring): re-export rind's CPU check as cpu #377).
  • rind-ring's signature::RsaEncoding lets generic code reach a &'static rind::signature::RsaSigningAlgorithm through its hidden supertrait.

Approach

Per the maintainer's decision (2026-11-06), each of those adapters defines its own pub struct MissingFeature (Debug, Display, Error) and returns it. It wraps rind's error in a private field and delegates Display, so the message stays rind's (for example "rind requires AES-NI CPU support on x86_64") without copying it into four crates, and rind needs no new accessor.

  • rind-rustls, rind-dimpl, rind-object-store: MissingFeature at the crate root.
  • rind-ring: pub mod cpu { pub fn check() -> Result<(), MissingFeature>; pub struct MissingFeature }, wrapping rind's.
  • rind_rustls::Ticketer::new() wraps the new rind_rustls::MissingFeature in rustls's OtherError, so a downcast finds the adapter's type too.
  • rind-ring's signature::RsaEncoding builds on a sealed, unnameable supertrait whose method generic code bounded by E: RsaEncoding can still call. It returned &'static rind::signature::RsaSigningAlgorithm; it now returns rind-ring's own (unnameable, Debug-only) Padding, whose rind field is visible only inside rsa.
  • Doc examples in rind-rustls (lib.rs, sign/any_of.rs) and the MIGRATING.md adapter section now name the adapter's type.

Impact

  • Breaking rind_rustls::default_provider(), rind_rustls::empty_provider(): return rind_rustls::MissingFeature. Migrate: replace rind::cpu::MissingFeature with rind_rustls::MissingFeature in signatures; ? into Box<dyn Error> and .to_string() are unchanged.
  • Breaking rind_dimpl::default_provider(): returns rind_dimpl::MissingFeature. Migrate the same way.
  • Breaking rind_object_store::default_provider(): returns rind_object_store::MissingFeature. Migrate the same way.
  • rind_rustls::Ticketer::new(): the rustls::OtherError it returns on an unsupported CPU now holds rind_rustls::MissingFeature; same message.
  • rind_ring::cpu: now check() and MissingFeature only, rind-ring's own types; drops has_aes_hardware(). Unreleased, so nothing shipped breaks.
  • rind_ring::signature::RsaEncoding: its hidden supertrait's method is now padding(&self) -> &Padding instead of rind(&self) -> &'static rind::signature::RsaSigningAlgorithm. Only reachable through a generic bound and never nameable, so no documented API changes.
  • New: rind_rustls::MissingFeature, rind_dimpl::MissingFeature, rind_object_store::MissingFeature, rind_ring::cpu::MissingFeature.
  • No wire impact.

The ! title makes release-plz bump rind-dimpl and rind-object-store to 0.2.0 instead of 0.1.14 (it plans each package from the conventional commits touching it; on 0.x a breaking commit bumps the minor).

Caused by

Decisions

  • Return type of each adapter's constructors: ✅ the adapter's own MissingFeature wrapping rind's (rejected: Box<dyn Error>, the consumer crate's error type), by the maintainer on 2026-11-06.
  • rind-ring's RsaEncoding supertrait method returning a rind type: ✅ hide it, in this PR (rejected: accept it), by the maintainer on 2026-11-06. It returns rind-ring's Padding rather than a new wrapper type, since Padding already exists and the RSA_* statics already hand it out.

Alternatives

  • Box<dyn Error> or the consumer crate's error type: rejected by the maintainer.
  • A feature() accessor on rind::cpu::MissingFeature, with each adapter holding the &'static str and formatting the message itself: copies the message into four crates; the private wrapper keeps one.

Validation

  • just check: passed on f66b62b, merged with main at ddd7fa0 (lint, build, tests, and doctests for every changed crate and its dependents).

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, high effort

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1b300-70e1-7fa1-879f-3bfece53e73c, round 1
  • Head: f66b62b
  • Base: origin/main at f8de447
  • Score: 10/10, The specified diff preserves CPU validation, diagnostics, and RSA signing behavior with no actionable findings; diff checks passed, but builds and tests were not run in the read-only environment.
  • Findings: none

rind-rustls, rind-dimpl, and rind-object-store's provider constructors now
return their crate's own MissingFeature, and rind-ring's cpu module wraps
rind's check with its own MissingFeature, so a rind major bump no longer
forces the adapters' majors.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated marked this pull request as ready for review November 6, 2026 22:01
@pullfrog

pullfrog Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Your OpenCode Go account is out of credit. Top it up, then re-trigger Pullfrog.

OpenCode Go's response
provider error: Insufficient account funds

Top up OpenCode Go →

Pullfrog  | Rerun failed job ➔ | View workflow run | via Pullfrog | Using glm-5.3

@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b9e72308-54d2-4cb7-a4e7-4778c2fbe95a
Commits

Reviewing files that changed from the base of the PR and between ddd7fa0 and f66b62b.

Files selected for processing: 9

  • MIGRATING.md
  • rind-dimpl/src/lib.rs
  • rind-object-store/src/lib.rs
  • rind-ring/src/cpu.rs
  • rind-ring/src/lib.rs
  • rind-ring/src/rsa.rs
  • rind-rustls/src/lib.rs
  • rind-rustls/src/sign/any_of.rs
  • rind-rustls/src/ticketer.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed 7413ad7. No actionable findings. Adapter-owned error wrappers are a sound direction for removing the exposed rind error type: the existing CPU checks remain before provider/ticketer creation, Display preserves the underlying diagnostic, and Ticketer::new consistently wraps the adapter's error for downcasts. The breaking return-type changes are documented.

Verification: GitHub-only static review of all eight changed files and the underlying CPU-check/error implementation. I did not compile the crates, run doctests, exercise unsupported-CPU failures, or independently verify the reported checks.

kixelated and others added 2 commits November 6, 2026 14:26
…g, not a rind type

Generic code bounded by `E: RsaEncoding` could call the sealed
supertrait's `rind()` and get a `rind::signature::RsaSigningAlgorithm`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated
kixelated added this pull request to the merge queue Nov 6, 2026

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review by review (OpenAI)

Reviewed f66b62b, including the update since 7413ad7. No actionable findings. Returning rind-ring's own Padding from the sealed supertrait closes the additional public type leak while preserving the same static RSA signing algorithm and signature-length checks. The rind field remains inaccessible to downstream callers; the adapter-owned CPU errors remain consistent with the earlier review.

Verification: GitHub-only static review of the current nine-file diff and surrounding RSA trait, re-export, and signing code. I did not compile, run tests or doctests, or independently verify the reported checks.

Merged via the queue into main with commit cc38f2b Nov 6, 2026
4 checks passed
This was referenced Nov 6, 2026
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.

1 participant