Promote the experimental architecture to main - #44
Merged
Conversation
Assisted-by: Codex-gpt-5.6-sol
Add new minimal core and runtime architecture Add actor endpoint with generics Add negotiation features Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Codex:gpt-5.6-sol
Port RSA actor key generation and persisted PEM validation into ref-feder-core. Redact private key material from debug output and keep actor dispatch as a minimal capability called directly by the server runtime. Assisted-by: Codex:gpt-5.6-sol
Parse and validate local acct resources, resolve actors through the actor dispatcher, and return JRD discovery responses from the experimental server router. Assisted-by: Codex:gpt-5.6-sol
Introduce a stateless receive_follow transition in ref-feder-core. Validate that the Follow actor matches the resolved remote actor and that its object targets the selected local actor, then return the follower facts and Accept delivery through a transient FollowOutcome without retaining protocol state. Extend the portable key module with SHA-256 digest generation and draft-Cavage RSA-SHA256 signing and verification primitives. Add a personal inbox endpoint to ref-feder-runtime-server. Validate ActivityPub content types, body size, request host and date, body digests, signature headers, remote key ownership, and actor identity before invoking the core Follow transition. Persist the follower before sending the generated Accept and return 202 Accepted for successfully handled or irrelevant activities. Represent local actor access and runtime services through FederServer<A, S>, while placing Arc only at the Axum router boundary for concurrent request sharing. Keep signed authentication as the default and temporarily retain an explicit insecure development policy until the reference example can issue signed Follow requests. Update the reference server example with bounded adapters for remote resolution, follower storage, and Accept sending. Document and exercise the actor, WebFinger, and personal inbox endpoints without accumulating an in-memory activity history. Shared inbox and Undo handling remain out of scope for this change. Assisted-by: Codex:gpt-5.6-sol
Define ServerStorage as the application-owned persistence boundary for follower relationships and per-actor signing keys. Allow runtimes to persist Follow outcomes and retrieve the appropriate ActorKeyPair without retaining protocol state inside the core. Assisted-by: Codex:gpt-5.6-sol
Port protected remote actor and key resolution to the reference server runtime. Reject private and special-use destinations by default, disable redirects and proxies, and enforce request timeouts and response size limits. Give FederServer concrete resolver and activity sender components. Verify signed inbox requests, persist Follow relationships through ServerStorage, load per-actor signing keys, and deliver signed Accept activities. Migrate the reference actor-server example to ServerStorage and the fallible server constructor. Use the bundled test key pair and a local recipient inbox to demonstrate signed Accept delivery without generating keys at startup. Assisted-by: Codex:gpt-5.6-sol
Reject the deprecated fec0::/10 site-local range when outbound networking uses the PublicOnly policy, closing an SSRF path through literal URLs and DNS resolution. Assisted-by: Codex:gpt-5.6-sol
Extend ServerStorage with an idempotent follower-removal operation for the upcoming Undo Follow transition. Update the reference actor-server storage adapter to remove its retained follower only when both sides of the relationship match. Assisted-by: Codex:gpt-5.6-sol
Validate that an Undo actor owns its embedded Follow and that the Follow targets the local actor. Return a transient follower-removal outcome without retaining protocol state or performing storage operations inside core. Assisted-by: Codex:gpt-5.6-sol
Dispatch Undo activities through the pure core transition and remove validated follower relationships through ServerStorage. Update the reference actor server example with idempotent follower removal and a documented Follow-to-Undo flow. Assisted-by: Codex:gpt-5.6-sol
Add canonical actor ID lookup to ActorDispatcher so shared inbox activities can be routed without assuming an application URL structure. Route Follow and Undo Follow activities from the shared inbox through the existing authentication, core transition, storage, and delivery flow. Update the reference actor server to advertise and demonstrate the shared inbox endpoint. Assisted-by: Codex:gpt-5.6-sol
Add a follower-listing capability to ServerStorage and expose each local actor's followers as an ActivityStreams OrderedCollection. Advertise the collection from the example actor and demonstrate how Follow and Undo requests update its contents. Assisted-by: Codex:gpt-5.6-sol
Add a pure core operation that constructs an outbound Follow activity and its transient pending relationship. Extend ServerStorage with pending Follow persistence and add FederServer::follow_actor to resolve the remote actor, persist intent, load the local signing key, and deliver the signed activity. Allow applications to retain shared FederServer state alongside the router, and update the reference example with bounded pending storage and an end-to-end outbound Follow demonstration. Assisted-by: Codex:gpt-5.6-sol
Add a pure core transition that validates inbound Accept activities against the authenticated remote actor and the stored pending Follow relationship. Extend ServerStorage with pending Follow lookup and compare-and-set confirmation capabilities. Route linked and embedded Accept Follow activities through personal and shared inboxes, and update the reference example with a bounded pending-to-accepted state transition. Assisted-by: Codex:gpt-5.6-sol
Add a pure core operation that constructs a local Note and its corresponding Create activity from runtime-provided IDs, addressing, and content. Introduce a dedicated NoteStore capability and add FederServer::create_note to load the local actor and persist only the durable Note without retaining protocol history. Update the reference example with bounded Note storage and a local creation flow while leaving Create delivery for the next migration step. Assisted-by: Codex:gpt-5.6-sol
Derive transient delivery intents from Note addressing in core without retaining recipient or activity history. Add follower delivery and shared storage error capabilities, then expand followers, resolve direct actors, deduplicate inboxes, and deliver signed Create activities after Note persistence. Update the reference example to demonstrate signed Create delivery to a stored follower. Assisted-by: Codex:gpt-5.6-sol
Extend NoteStore with typed Note loading and add a pure core check for public ActivityStreams addressing. Expose canonical local post paths through the reference server runtime with ActivityPub content negotiation while hiding missing and non-public Notes. Update the reference example to load and serve its bounded persisted Note. Assisted-by: Codex:gpt-5.6-sol
Require FederServer users to provide the authoritative WebFinger handle host and validate acct resources against it. Stop trusting the client-controlled Host header when deciding whether an account belongs to the local deployment, and update the reference actor server with its configured handle host. Assisted-by: Codex:gpt-5.6-sol
Preserve the inbox endpoint selected during request routing and use its authority when verifying signed requests. Personal inbox requests continue to use the actor inbox, while shared inbox requests use the actor's advertised sharedInbox endpoint. Treat requests for actors without a shared inbox as accepted no-ops. Assisted-by: Codex:gpt-5.6-sol
Restore the http-signatures feature boundary around actor keys, digest generation, and draft-Cavage signing and verification. Keep RSA, Base64, and zeroization out of the portable core dependency tree unless explicitly requested. Enable the feature from the server runtime and reference actor example. Assisted-by: Codex:gpt-5.6-sol
Replace complete stored Actor values in follower fan-out with the minimal addressing facts required for delivery: actor ID, inbox, and optional shared inbox. Update Note delivery and the reference example to use the compact target, keeping the storage contract compatible with the existing SQLite follower schema and avoiding unnecessary actor retention. Assisted-by: Codex:gpt-5.6-sol
Provide file-backed and in-memory SQLite storage implementing the new server, Note, and follower-delivery storage capabilities. Preserve the existing followers, actor keys, and objects schema while adding persistent outbound Follow state. Synchronize connection access for shared Axum state and use an immediate transaction for exact pending-Follow confirmation. Add focused coverage for follower delivery facts, Notes, pending Follow transitions, actor keys, and Send and Sync compatibility. Assisted-by: Codex:gpt-5.6-sol
Add a SQLite-backed load-or-generate operation for actor signing identities. Reuse existing actor keys without invoking the random number generator. When a key is absent, generate it outside the database lock and insert it without replacing an identity established by a concurrent provisioner. Add focused coverage that verifies initial provisioning and stable reuse. Assisted-by: Codex:gpt-5.6-sol
Replace the original RuntimeConfig-based example with the new FederServer, actor dispatcher, and built-in SQLite storage adapter. Provision the actor signing key through SQLite, publish its public key on the actor document, and preserve the signing identity across server restarts. Allow the database path to be configured through FEDER_DATABASE. Update the example documentation with actor and WebFinger requests. Assisted-by: Codex:gpt-5.6-sol
Move protocol transition tests into ref-feder-core/tests, covering Follow, Accept, Undo, Note construction, actor keys, digests, and HTTP signatures. Move the SQLite adapter tests out of the implementation module and into the ref-feder-runtime-server integration test suite. Add runtime coverage for actor and object endpoints, WebFinger, follower collections, actor resolution, outbound delivery, signed personal and shared inboxes, outbound Follow, incoming Undo, and Note persistence and delivery. Keep test keys and shared test infrastructure local to each crate so the reference crates can be tested independently. Assisted-by: Codex:gpt-5.6-sol
Replace the previous stateful feder-core implementation with the portable protocol transition functions and capability traits developed in the reference crate. Publish the standard operating system runtime as feder-server and update its dependencies, imports, tests, and documentation to use the final feder-core package. Remove the superseded feder-runtime-server, temporary ref-* crates, and the redundant custom-storage example. Retain the SQLite single-user server as a non-publishable end-to-end example. Restrict release version stamping to publishable workspace packages and update the project documentation to describe the stateless core and server runtime boundaries. Assisted-by: Codex:gpt-5.6-sol
Refactor architecture
Return 202 Accepted for Accept and Undo activities wrapping unsupported activity types before attempting Follow-specific deserialization. Assisted-by: Codex:gpt-5.6-sol
📝 WalkthroughWalkthroughThe project replaces the stateful ChangesStateless federation architecture
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (13)
crates/feder-server/src/inbox.rs (4)
103-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog the silent 202 branches in
shared_inbox.Three separate conditions return
202 Acceptedand drop the activity: no routable target, no local actor for the target, and no advertisedsharedInboxendpoint. The third condition indicates a server misconfiguration, not a remote error. Without a log line, an operator cannot distinguish a dropped activity from a processed one.Add a debug or warning log for each branch before returning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/inbox.rs` around lines 103 - 119, Add a debug or warning log immediately before each 202 Accepted return in shared_inbox: when shared_inbox_target yields no routable target, when get_actor_by_id finds no local actor, and when the actor has no advertised shared_inbox endpoint. Include enough context to distinguish the three dropped-activity cases, treating the missing endpoint as a server configuration issue.
51-59: 🔒 Security & Privacy | 🔵 TrivialTrack removal of
AllowUnsignedInsecureDev.The variant disables all inbox authentication and is part of the public API. The default is
RequireSigned, so the exposure needs an explicit opt-in. The FIXME records the plan to remove the variant after the reference example signs its Follow requests.Do you want me to open an issue to track the removal and the example update?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/inbox.rs` around lines 51 - 59, Track the planned removal of the public InboxAuthPolicy::AllowUnsignedInsecureDev variant and the corresponding reference-example update by opening an issue. Record that the example must send signed Follow requests before removing the variant, while preserving RequireSigned as the default until then.
469-474: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse a core helper instead of duplicating
follow_reference_id.This function repeats the private
follow_reference_idincrates/feder-core/src/follow.rslines 132-137. The same duplication exists for theReference<Actor>variant. Exporting one small accessor fromfeder-core, for example an inherentid()onReference<T>whereThas an id, removes both copies and keeps the two crates aligned ifReferencegains a variant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/inbox.rs` around lines 469 - 474, Remove the local follow_reference_id helper in inbox.rs and reuse a shared accessor from feder-core for both Reference<Follow> and Reference<Actor>. Expose an inherent id() method on Reference<T> in the core implementation, constrained to referenced types with an id, then update callers to use it so both crates share one variant-aware implementation.
315-391: 🔒 Security & Privacy | 🔵 TrivialConsider a replay guard for signed inbox requests.
The date window allows a captured request to be replayed for up to 65 minutes. The digest binds the body, so a replay repeats the identical activity. Follow, Undo, and Accept handling is idempotent today, so the current impact is low. If the inbox later handles non-idempotent activities, a short-lived cache of seen signature values, or of activity IDs, prevents duplicate processing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/inbox.rs` around lines 315 - 391, Extend verify_signed_request with a short-lived replay guard keyed by each accepted signature value (or activity ID), rejecting entries already seen within the allowed date window while preserving normal validation and idempotent handling. Ensure the guard is concurrency-safe, bounded/expiring, and only records requests after signature verification succeeds.crates/feder-server/tests/cases/inbox.rs (1)
336-363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for the Accept confirmation path.
The suite covers Follow, Undo, and unsupported nested activities. It does not cover a valid Accept that confirms a pending outbound Follow. That path in
crates/feder-server/src/inbox.rslines 187-231 loads the pending relationship, validates it throughreceive_accept_follow, and callsconfirm_pending_follow. It is the newest logic in the handler and has no server-level test.Add one test that stores a pending Follow, posts a signed Accept to the personal inbox, and asserts the relationship became confirmed. Add a second test that posts the same Accept to the shared inbox, to cover the
shared_inbox_targetlookup at lines 445-456.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/tests/cases/inbox.rs` around lines 336 - 363, Add server-level tests for the valid Accept confirmation flow: create and persist a pending outbound Follow, post its signed Accept to the recipient’s personal inbox, and assert the relationship is confirmed; add a second test posting the same Accept to the shared inbox to exercise shared_inbox_target lookup. Reuse existing Follow, signing, inbox, and relationship assertion helpers, anchoring the tests near ignores_accept_and_undo_of_unsupported_activities.crates/feder-core/tests/follow.rs (1)
113-142: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the remaining
AcceptFollowErrorvariants.The tests assert
WrongActorandWrongFollowonly.WrongLocalActor,WrongFollowActor, andWrongFollowObjectstay untested. Those branches guard cross-actor confirmation of a pending Follow, so a regression there would be silent.🧪 Suggested additional cases
#[test] fn rejects_accept_for_another_local_actor() { let local = actor("https://local.example/users/alice"); let remote = actor("https://remote.example/users/bob"); let pending = PendingFollow { local_actor: iri("https://local.example/users/carol"), remote_actor: remote.clone(), follow_activity: iri("https://local.example/activities/follow/1"), }; let accept = Accept::new( iri("https://remote.example/activities/accept/1"), Reference::id(remote.id.clone()), Reference::id(pending.follow_activity.clone()), ); assert_eq!( receive_accept_follow(&local, &remote, &pending, accept), Err(AcceptFollowError::WrongLocalActor) ); } #[test] fn rejects_accept_whose_embedded_follow_has_wrong_endpoints() { let local = actor("https://local.example/users/alice"); let remote = actor("https://remote.example/users/bob"); let other = actor("https://local.example/users/mallory"); let pending = PendingFollow { local_actor: local.id.clone(), remote_actor: remote.clone(), follow_activity: iri("https://local.example/activities/follow/1"), }; let follow = Follow::new( pending.follow_activity.clone(), Reference::id(other.id.clone()), Reference::id(remote.id.clone()), ); let accept = Accept::new( iri("https://remote.example/activities/accept/1"), Reference::id(remote.id.clone()), Reference::object(follow), ); assert_eq!( receive_accept_follow(&local, &remote, &pending, accept), Err(AcceptFollowError::WrongFollowActor) ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-core/tests/follow.rs` around lines 113 - 142, Extend the receive_accept_follow test coverage with cases for WrongLocalActor, WrongFollowActor, and WrongFollowObject. Add assertions using mismatched pending local actors and embedded Follow objects with incorrect actor and object endpoints, ensuring each case returns its corresponding AcceptFollowError variant.crates/feder-core/tests/undo.rs (1)
29-70: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a case for a mismatched embedded Follow actor.
The three cases cover
LinkedFollow, the outer-actorWrongActorbranch, andWrongObject. They do not cover the secondWrongActorbranch incrates/feder-core/src/undo.rsat lines 42-44, whereundo.actormatchesremote_actor.idbut the embeddedfollow.actordoes not.That branch carries the authorization guarantee. It stops a verified signer from undoing a Follow that another actor created.
crates/feder-server/src/inbox.rsmaps the result to 401.💚 Proposed additional case
+ let wrong_follow_actor = Undo::new( + iri("https://remote.example/activities/undo/4"), + Reference::id(remote.id.clone()), + Reference::object(Follow::new( + iri("https://remote.example/activities/follow/4"), + Reference::id(other.id.clone()), + Reference::id(local.id.clone()), + )), + ); + assert_eq!( receive_undo_follow(&local, &remote, linked), Err(UndoFollowError::LinkedFollow) ); + assert_eq!( + receive_undo_follow(&local, &remote, wrong_follow_actor), + Err(UndoFollowError::WrongActor) + );Change
Reference::id(other.id)at line 41 toReference::id(other.id.clone())sootherremains usable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-core/tests/undo.rs` around lines 29 - 70, Add a test case in rejects_linked_follow_and_wrong_actor_or_object for an Undo whose outer actor matches remote.id but whose embedded Follow actor is other.id, and assert receive_undo_follow returns UndoFollowError::WrongActor. Clone other.id where needed so the existing wrong_actor fixture remains usable.crates/feder-server/src/lib.rs (1)
73-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated constructor body.
newandwith_outbound_address_policybuild the same value. Only the policy differs. Letnewdelegate towith_outbound_address_policy. This keeps future field additions in one place.Also consider the naming.
with_inbox_auth_policyon Line 107 is a consuming builder method, butwith_outbound_address_policyis a constructor with a different shape. A name such aswith_configor a builder method that mirrorswith_inbox_auth_policywould make the public API consistent.♻️ Proposed change
pub fn new(actors: A, storage: S, handle_host: impl Into<String>) -> Result<Self, Error> { - let policy = OutboundAddressPolicy::PublicOnly; - let resolver = ActorResolver::new(policy)?; - let sender = ActivitySender::new(policy)?; - Ok(Self { - actors, - storage, - handle_host: handle_host.into(), - resolver, - sender, - inbox_auth_policy: InboxAuthPolicy::RequireSigned, - }) + Self::with_outbound_address_policy( + actors, + storage, + handle_host, + OutboundAddressPolicy::PublicOnly, + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/lib.rs` around lines 73 - 104, Remove the duplicated construction logic by making new delegate to with_outbound_address_policy with OutboundAddressPolicy::PublicOnly, preserving the existing default behavior and centralizing field initialization. Rename with_outbound_address_policy to a configuration-oriented or consuming-builder name consistent with with_inbox_auth_policy, and update its callers accordingly.crates/feder-server/Cargo.toml (1)
12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid an exact version pin for
axumin a library crate.
= 0.8resolves to exactly0.8.0. Cargo excludes every later patch release, so the crate cannot receive0.8.xbug and security fixes. An exact pin in a published library also creates conflicts for downstream users who need a newer patch. Use a caret requirement instead.♻️ Proposed change
-axum = "=0.8" +axum = "0.8"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/Cargo.toml` at line 12, Update the axum dependency declaration in the crate manifest from an exact version requirement to a caret-compatible 0.8 requirement, allowing later 0.8.x patch releases while preserving the current minor-version constraint.crates/feder-server/src/follow.rs (1)
73-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneric error variants drop the underlying cause in both workflow error types.
FollowActorErrorandCreateNoteErrorcarry the dispatcher and storage errors by value without#[source], while the non-generic variants in the same enums use#[source]. Callers that walk the error chain lose the real reason for a dispatcher or storage failure.
crates/feder-server/src/follow.rs#L73-L83: add#[source]toActorDispatcher(A)andStorage(S), and addA: std::error::Error + 'staticandS: std::error::Error + 'staticbounds toFollowActorError.crates/feder-server/src/note.rs#L141-L148: apply the same#[source]attributes and generic bounds toActorDispatcher(A)andStorage(S)inCreateNoteError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/follow.rs` around lines 73 - 83, Update FollowActorError in crates/feder-server/src/follow.rs lines 73-83 by marking ActorDispatcher(A) and Storage(S) as #[source] and adding A: std::error::Error + 'static and S: std::error::Error + 'static bounds. Apply the same #[source] attributes and generic bounds to ActorDispatcher(A) and Storage(S) in CreateNoteError at crates/feder-server/src/note.rs lines 141-148, preserving the existing non-generic error behavior.crates/feder-server/src/send.rs (1)
57-66: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueThe
Reference::Idbranch skips the key consistency check.The
Reference::Objectbranch verifies the owner and the PEM againstkey_pair. TheReference::Idbranch accepts the key id without any check. An id-only reference carries no PEM, so a local comparison is not possible. The result is thatActorKeyMismatchonly protects the embedded-key case.If the actor document is always expected to embed the key, reject
Reference::IdwithMissingActorKeyinstead. If both forms are supported, document the reduced guarantee at the method level.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/send.rs` around lines 57 - 66, Update the key selection logic around local_actor.public_key so Reference::Id cannot bypass key consistency validation. Prefer rejecting id-only references with SendError::MissingActorKey if embedded keys are required; otherwise document the reduced validation guarantee at the enclosing method and preserve support for both reference forms.crates/feder-server/src/note.rs (1)
69-86: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftDelivery failures after the first one are discarded without a trace.
The loop keeps only
first_send_error. Every later failure is dropped. There is no log statement. If delivery to 50 of 100 follower inboxes fails, the caller sees one error and cannot identify the affected inboxes.The loop is also sequential. Each inbox waits for the previous request, and
build_clientsets a request timeout. For a large follower collection the total time grows linearly with the follower count on the caller's task.Two suggestions for the operator-facing path:
- Log each delivery failure with the inbox, or return the full list of failures instead of one error.
- Consider bounded concurrency for the fan-out, for example a
FuturesUnorderedwith a concurrency limit, or hand the deliveries to a background queue so the caller does not block.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/src/note.rs` around lines 69 - 86, Update the inbox delivery loop around send_activity to record every delivery failure with its inbox through the existing logging facility, while preserving the first_send_error result behavior. Replace the sequential fan-out with bounded concurrency, such as a concurrency-limited FuturesUnordered flow, so large follower collections do not block linearly on the caller task. Preserve the existing ActorResolver error precedence when no delivery error occurs.crates/feder-server/tests/cases/operation.rs (1)
82-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert pending Follow persistence.
This test never loads
outbound_follows. It passes iffollow_actorsends the Follow without storing the pending relationship.Assert that
server.storage().load_pending_follow(&follow_id)returns the expected relationship. If delivery ordering is required, block the remote response and inspect storage after the handler receives the request but before it releases the response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-server/tests/cases/operation.rs` around lines 82 - 100, Extend outbound_follow_is_persisted_before_signed_delivery to load the pending relationship with server.storage().load_pending_follow(&follow_id) after follow_actor completes, and assert it matches the expected local and remote actors. If persistence must be verified before delivery completes, hold the remote response, inspect storage after receiving the request, then release the response.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/feder-core/src/note.rs`:
- Around line 81-104: Add a shared public-address predicate that recognizes both
PUBLIC_COLLECTION and the compact "as:Public" alias, then use it in
is_public_note and note_recipients. Ensure alias recipients are skipped as
public addressing rather than reported as NoteRecipient::Actor targets.
In `@crates/feder-server/src/follow.rs`:
- Around line 56-60: Update follow_actor’s inbox selection to always use
remote_actor.inbox for the targeted Follow, removing the shared_inbox fallback
and any related endpoint preference. Preserve the existing delivery flow while
ensuring no follower endpoint recipients are used.
In `@crates/feder-server/src/inbox.rs`:
- Around line 226-229: Update the confirmation flow in the inbox handler around
confirm_pending_follow to bind its returned bool and detect false. Log an
appropriate diagnostic when the exact pending relationship was not confirmed,
while preserving the existing internal-server-error handling for returned
errors.
- Around line 601-611: The signature parsing and verification flow around
parse_signature_header and verify_signed_request must accept an omitted
algorithm and algorithm="hs2019" for RSA signatures. Treat either case as the
existing RSA-SHA256 verification path while preserving current handling for
explicit rsa-sha256 and rejecting unsupported algorithms.
In `@crates/feder-server/src/lib.rs`:
- Around line 87-93: Add rustdoc to the public constructor
with_outbound_address_policy describing that
OutboundAddressPolicy::AllowPrivateAddress disables SSRF protection and should
be limited to development or tests; replace or supplement the non-rustdoc marker
so published crate consumers see the warning. If the project’s feature structure
supports it, gate this constructor behind an insecure-dev cargo feature to
prevent access in production builds.
In `@crates/feder-server/src/storage/sqlite.rs`:
- Around line 260-264: Update the SQLite upsert for follow activities so a
conflict on follow_activity_id never changes an existing accepted relationship
back to pending. Replace the unconditional state update with DO NOTHING, or
condition updates so they apply only when the stored state is already pending,
while preserving the current insert behavior.
In `@examples/single-user-server/src/main.rs`:
- Around line 98-104: Update the example server construction using
FederServer::with_outbound_address_policy to use
OutboundAddressPolicy::PublicOnly and the default InboxAuthPolicy::RequireSigned
instead of permitting private addresses or unsigned insecure development
requests. Keep private-address access and unsigned local-federation behavior
available only through explicit opt-in configuration.
---
Nitpick comments:
In `@crates/feder-core/tests/follow.rs`:
- Around line 113-142: Extend the receive_accept_follow test coverage with cases
for WrongLocalActor, WrongFollowActor, and WrongFollowObject. Add assertions
using mismatched pending local actors and embedded Follow objects with incorrect
actor and object endpoints, ensuring each case returns its corresponding
AcceptFollowError variant.
In `@crates/feder-core/tests/undo.rs`:
- Around line 29-70: Add a test case in
rejects_linked_follow_and_wrong_actor_or_object for an Undo whose outer actor
matches remote.id but whose embedded Follow actor is other.id, and assert
receive_undo_follow returns UndoFollowError::WrongActor. Clone other.id where
needed so the existing wrong_actor fixture remains usable.
In `@crates/feder-server/Cargo.toml`:
- Line 12: Update the axum dependency declaration in the crate manifest from an
exact version requirement to a caret-compatible 0.8 requirement, allowing later
0.8.x patch releases while preserving the current minor-version constraint.
In `@crates/feder-server/src/follow.rs`:
- Around line 73-83: Update FollowActorError in
crates/feder-server/src/follow.rs lines 73-83 by marking ActorDispatcher(A) and
Storage(S) as #[source] and adding A: std::error::Error + 'static and S:
std::error::Error + 'static bounds. Apply the same #[source] attributes and
generic bounds to ActorDispatcher(A) and Storage(S) in CreateNoteError at
crates/feder-server/src/note.rs lines 141-148, preserving the existing
non-generic error behavior.
In `@crates/feder-server/src/inbox.rs`:
- Around line 103-119: Add a debug or warning log immediately before each 202
Accepted return in shared_inbox: when shared_inbox_target yields no routable
target, when get_actor_by_id finds no local actor, and when the actor has no
advertised shared_inbox endpoint. Include enough context to distinguish the
three dropped-activity cases, treating the missing endpoint as a server
configuration issue.
- Around line 51-59: Track the planned removal of the public
InboxAuthPolicy::AllowUnsignedInsecureDev variant and the corresponding
reference-example update by opening an issue. Record that the example must send
signed Follow requests before removing the variant, while preserving
RequireSigned as the default until then.
- Around line 469-474: Remove the local follow_reference_id helper in inbox.rs
and reuse a shared accessor from feder-core for both Reference<Follow> and
Reference<Actor>. Expose an inherent id() method on Reference<T> in the core
implementation, constrained to referenced types with an id, then update callers
to use it so both crates share one variant-aware implementation.
- Around line 315-391: Extend verify_signed_request with a short-lived replay
guard keyed by each accepted signature value (or activity ID), rejecting entries
already seen within the allowed date window while preserving normal validation
and idempotent handling. Ensure the guard is concurrency-safe, bounded/expiring,
and only records requests after signature verification succeeds.
In `@crates/feder-server/src/lib.rs`:
- Around line 73-104: Remove the duplicated construction logic by making new
delegate to with_outbound_address_policy with OutboundAddressPolicy::PublicOnly,
preserving the existing default behavior and centralizing field initialization.
Rename with_outbound_address_policy to a configuration-oriented or
consuming-builder name consistent with with_inbox_auth_policy, and update its
callers accordingly.
In `@crates/feder-server/src/note.rs`:
- Around line 69-86: Update the inbox delivery loop around send_activity to
record every delivery failure with its inbox through the existing logging
facility, while preserving the first_send_error result behavior. Replace the
sequential fan-out with bounded concurrency, such as a concurrency-limited
FuturesUnordered flow, so large follower collections do not block linearly on
the caller task. Preserve the existing ActorResolver error precedence when no
delivery error occurs.
In `@crates/feder-server/src/send.rs`:
- Around line 57-66: Update the key selection logic around
local_actor.public_key so Reference::Id cannot bypass key consistency
validation. Prefer rejecting id-only references with SendError::MissingActorKey
if embedded keys are required; otherwise document the reduced validation
guarantee at the enclosing method and preserve support for both reference forms.
In `@crates/feder-server/tests/cases/inbox.rs`:
- Around line 336-363: Add server-level tests for the valid Accept confirmation
flow: create and persist a pending outbound Follow, post its signed Accept to
the recipient’s personal inbox, and assert the relationship is confirmed; add a
second test posting the same Accept to the shared inbox to exercise
shared_inbox_target lookup. Reuse existing Follow, signing, inbox, and
relationship assertion helpers, anchoring the tests near
ignores_accept_and_undo_of_unsupported_activities.
In `@crates/feder-server/tests/cases/operation.rs`:
- Around line 82-100: Extend outbound_follow_is_persisted_before_signed_delivery
to load the pending relationship with
server.storage().load_pending_follow(&follow_id) after follow_actor completes,
and assert it matches the expected local and remote actors. If persistence must
be verified before delivery completes, hold the remote response, inspect storage
after receiving the request, then release the response.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ed53852-d143-46bd-b143-33f6adea91b9
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockcrates/feder-server/tests/fixtures/rsa-private-key.pemis excluded by!**/*.pemcrates/feder-server/tests/fixtures/rsa-public-key.pemis excluded by!**/*.pem
📒 Files selected for processing (63)
CONTRIBUTING.mdCargo.tomlREADME.mdcrates/feder-core/Cargo.tomlcrates/feder-core/src/follow.rscrates/feder-core/src/key.rscrates/feder-core/src/lib.rscrates/feder-core/src/note.rscrates/feder-core/src/storage.rscrates/feder-core/src/undo.rscrates/feder-core/tests/common/mod.rscrates/feder-core/tests/follow.rscrates/feder-core/tests/key.rscrates/feder-core/tests/note.rscrates/feder-core/tests/undo.rscrates/feder-runtime-server/README.mdcrates/feder-runtime-server/src/app.rscrates/feder-runtime-server/src/config.rscrates/feder-runtime-server/src/error.rscrates/feder-runtime-server/src/inbox.rscrates/feder-runtime-server/src/lib.rscrates/feder-runtime-server/src/operation.rscrates/feder-runtime-server/src/storage/mod.rscrates/feder-runtime-server/src/storage/sqlite.rscrates/feder-runtime-server/tests/cases/actor.rscrates/feder-runtime-server/tests/cases/app.rscrates/feder-runtime-server/tests/cases/followers.rscrates/feder-runtime-server/tests/cases/inbox.rscrates/feder-runtime-server/tests/cases/object.rscrates/feder-runtime-server/tests/cases/operation.rscrates/feder-runtime-server/tests/cases/send.rscrates/feder-runtime-server/tests/cases/webfinger.rscrates/feder-runtime-server/tests/common/mod.rscrates/feder-server/Cargo.tomlcrates/feder-server/src/actor.rscrates/feder-server/src/config.rscrates/feder-server/src/follow.rscrates/feder-server/src/followers.rscrates/feder-server/src/inbox.rscrates/feder-server/src/lib.rscrates/feder-server/src/negotiation.rscrates/feder-server/src/note.rscrates/feder-server/src/object.rscrates/feder-server/src/send.rscrates/feder-server/src/storage/mod.rscrates/feder-server/src/storage/sqlite.rscrates/feder-server/src/url.rscrates/feder-server/src/webfinger.rscrates/feder-server/tests/cases/actor.rscrates/feder-server/tests/cases/followers.rscrates/feder-server/tests/cases/inbox.rscrates/feder-server/tests/cases/object.rscrates/feder-server/tests/cases/operation.rscrates/feder-server/tests/cases/send.rscrates/feder-server/tests/cases/webfinger.rscrates/feder-server/tests/common/mod.rscrates/feder-server/tests/runtime.rscrates/feder-server/tests/storage.rscspell.jsonexamples/single-user-server/Cargo.tomlexamples/single-user-server/README.mdexamples/single-user-server/src/main.rsmise.toml
💤 Files with no reviewable changes (19)
- crates/feder-runtime-server/tests/cases/webfinger.rs
- crates/feder-runtime-server/src/lib.rs
- crates/feder-runtime-server/tests/cases/actor.rs
- crates/feder-runtime-server/src/inbox.rs
- crates/feder-server/src/negotiation.rs
- crates/feder-runtime-server/tests/cases/operation.rs
- crates/feder-runtime-server/tests/cases/inbox.rs
- crates/feder-runtime-server/src/app.rs
- crates/feder-runtime-server/README.md
- crates/feder-runtime-server/tests/cases/send.rs
- crates/feder-runtime-server/src/operation.rs
- crates/feder-runtime-server/src/config.rs
- crates/feder-runtime-server/src/error.rs
- crates/feder-runtime-server/src/storage/mod.rs
- crates/feder-runtime-server/tests/cases/object.rs
- crates/feder-runtime-server/tests/cases/followers.rs
- crates/feder-runtime-server/src/storage/sqlite.rs
- crates/feder-runtime-server/tests/cases/app.rs
- crates/feder-runtime-server/tests/common/mod.rs
Comment on lines
+226
to
+229
| server | ||
| .storage() | ||
| .confirm_pending_follow(&pending) | ||
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use the bool returned by confirm_pending_follow.
confirm_pending_follow returns false when the exact relationship is no longer pending. That happens if a duplicate Accept, or a concurrent Undo, changed the row between load_pending_follow at line 208 and this call. The code discards the value, so a lost confirmation produces the same 202 Accepted as a successful one and leaves no trace.
Bind the result and log the false case.
♻️ Proposed change
- server
- .storage()
- .confirm_pending_follow(&pending)
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+ let confirmed = server
+ .storage()
+ .confirm_pending_follow(&pending)
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+ if !confirmed {
+ tracing::debug!(
+ follow_activity = %pending.follow_activity,
+ "Accept did not confirm a pending Follow; the relationship changed concurrently"
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| server | |
| .storage() | |
| .confirm_pending_follow(&pending) | |
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | |
| let confirmed = server | |
| .storage() | |
| .confirm_pending_follow(&pending) | |
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; | |
| if !confirmed { | |
| tracing::debug!( | |
| follow_activity = %pending.follow_activity, | |
| "Accept did not confirm a pending Follow; the relationship changed concurrently" | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/feder-server/src/inbox.rs` around lines 226 - 229, Update the
confirmation flow in the inbox handler around confirm_pending_follow to bind its
returned bool and detect false. Log an appropriate diagnostic when the exact
pending relationship was not confirmed, while preserving the existing
internal-server-error handling for returned errors.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Assisted-by: Codex:gpt-5.6-sol
Normalize compact Public addressing, improve inbox signature compatibility, deliver direct Follows to personal inboxes, and keep accepted outbound relationships from reopening. Add regression coverage for Follow and Accept flows and document outbound policy security. Assisted-by: Codex:gpt-5.6-sol
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/feder-vocab/src/lib.rs (1)
686-706: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for array addressing.
The implementation accepts scalar and array values, but this test covers only the scalar branch. Add an array case with a compact public term and another IRI. Assert that every entry becomes the expected canonical
Iri.🧪 Suggested test
+ #[test] + fn addressing_normalizes_public_compact_terms_in_arrays() { + let note: Note = serde_json::from_value(json!({ + "type": "Note", + "id": "https://example.com/notes/1", + "to": ["Public", "https://example.com/users/bob"], + })) + .expect("deserialize array addressing"); + + assert_eq!( + note.to, + References::many([ + iri("https://www.w3.org/ns/activitystreams#Public"), + iri("https://example.com/users/bob"), + ]) + ); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/feder-vocab/src/lib.rs` around lines 686 - 706, Add an array-addressing regression case to addressing_normalizes_public_compact_terms, deserializing a Note whose to field contains a compact Public term and another IRI. Assert that both entries are represented as the expected canonical Iri values, while preserving the existing scalar assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/feder-vocab/src/lib.rs`:
- Around line 686-706: Add an array-addressing regression case to
addressing_normalizes_public_compact_terms, deserializing a Note whose to field
contains a compact Public term and another IRI. Assert that both entries are
represented as the expected canonical Iri values, while preserving the existing
scalar assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ca0c6de3-0bc6-4df9-a555-0cd3d9b5d79c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
crates/feder-server/Cargo.tomlcrates/feder-server/src/follow.rscrates/feder-server/src/inbox.rscrates/feder-server/src/lib.rscrates/feder-server/src/storage/sqlite.rscrates/feder-server/tests/cases/inbox.rscrates/feder-server/tests/cases/operation.rscrates/feder-server/tests/common/mod.rscrates/feder-server/tests/storage.rscrates/feder-vocab/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/feder-server/tests/cases/operation.rs
- crates/feder-server/Cargo.toml
- crates/feder-server/src/lib.rs
- crates/feder-server/tests/common/mod.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Promote the architecture developed on
feat/experiment/architectureto themain development line.
feder-coreimplementation with portable,stateless protocol transition functions and capability traits.
feder-runtime-serverwith the publicfeder-serverruntime.SQLite persistence, followers, Follow/Accept/Undo handling, and Note delivery.
202 Acceptedfor unsupported nested Accept and Undo activities.Compatibility
This intentionally replaces the previous public core and runtime APIs.
Consumers of
feder-runtime-servermust migrate tofeder-server.The existing SQLite follower, signing-key, and object tables remain compatible.
The new runtime adds storage for outbound Follow relationships.
The legacy
/healthzroute is intentionally not carried forward.Migration notes
This change replaces the experimental legacy API rather than preserving
backward compatibility.
feder-runtime-serverdependencies and imports withfeder-server.FederCoreworkflow with stateless protocol functionsand runtime-provided capability traits.
FederServer, anActorDispatcher,and storage implementations.
A new table stores outbound Follow relationships.
/healthzendpoint is not included.Validation
mise run checkmise run testmainThis PR is assisted by Codex: gpt-sol-5.6.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes