Implement ActivityPub discovery endpoints#33
Conversation
Assisted-by: Codex:gpt-5.5
Expose GET /users/{username} from the server runtime and return the
configured local actor as application/activity+json. Keep the local actor
in
AppState and pass a clone into FederCore so HTTP serving does not need to
lock
the core state.
Cover the configured local actor route, unknown usernames, response content type, and serialized ActivityPub actor fields. Assisted-by: Codex:gpt-5.5
Cover the seeded public Note route, unknown Note IDs, response content type, and serialized ActivityPub Note fields. Assisted-by: Codex:gpt-5.5
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR implements ActivityPub discovery endpoints in feder-runtime-server: WebFinger resolution, actor document retrieval, and public note retrieval. AppState and RuntimeConfig are extended with identity/storage fields, new routes are wired, dependencies added, and a mise test task introduced. ChangesActivityPub Endpoints and Runtime Wiring
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router as AxumRouter
participant WebFingerHandler
participant ActorHandler
participant NoteHandler
participant State as AppState
Client->>Router: GET /.well-known/webfinger?resource=...
Router->>WebFingerHandler: dispatch(state, query)
WebFingerHandler->>State: read username, local_actor
WebFingerHandler-->>Client: 200 jrd+json / 400 / 404
Client->>Router: GET /users/username
Router->>ActorHandler: dispatch(state, username)
ActorHandler->>State: compare username, read local_actor
ActorHandler-->>Client: 200 activity+json / 404
Client->>Router: GET /notes/id
Router->>NoteHandler: dispatch(state, id)
NoteHandler->>State: search notes by canonical id
NoteHandler-->>Client: 200 activity+json / 404
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cc594f510
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
crates/feder-runtime-server/Cargo.toml (1)
15-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider workspace-managed version for
towerlike the other dependencies.
serdeandserde_jsonuse.workspace = true, buttower = "0.5"is pinned locally, inconsistent with the workspace dependency management style used elsewhere in this file.🤖 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-runtime-server/Cargo.toml` around lines 15 - 19, The dependency declaration for tower is inconsistent with the workspace-managed style used for serde and serde_json. Update the dev-dependencies entry in Cargo.toml for tower to use the workspace version management pattern instead of a locally pinned version, matching the existing dependency declarations in this manifest.crates/feder-runtime-server/src/config.rs (1)
20-30: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winHost/port now duplicated across three config representations.
handle_hostduplicates the host:port that's already embedded inbind(asSocketAddr) and inactor_id/inbox/outbox/note_id(asIris). Nothing enforces these stay consistent — e.g. changingactor_id's host without updatinghandle_hostwould silently break WebFinger (Line 54 in webfinger.rs constructsacct:{username}@{handle_host}) and the note lookup (note.rs Line 33), even though the actor/note IDs themselves would still resolve correctly.Consider deriving
handle_hostfromactor_id's authority instead of storing it independently, or adding a debug assertion/validation indefault_local()(and future config loaders) that the host segments agree.🤖 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-runtime-server/src/config.rs` around lines 20 - 30, RuntimeConfig currently stores handle_host separately even though the host:port is already present in bind and the IRI authorities for actor_id/inbox/outbox/note_id, which can drift out of sync. Update RuntimeConfig/default_local() to derive handle_host from actor_id’s authority (or otherwise validate that these host segments match when loading config) so WebFinger’s acct:{username}@{handle_host} and note lookup always use the same host as the actor/note IDs.crates/feder-runtime-server/src/app.rs (2)
25-33: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNo cross-validation between
actor_id,username, andhandle_host.
from_configindependently derives the actor'sidfromconfig.actor_id, but the/users/{username}route (inactor.rs) matches onconfig.username, and WebFinger's subject/alias (inwebfinger.rs) is built fromconfig.username+config.handle_host. Nothing here enforces thatactor_id's path segment actually corresponds tousername, or that its host matcheshandle_host. With the shipped defaults these are consistent, but a future config change to any one of these fields independently (e.g. renaming the account) would silently desynchronize WebFinger's advertised actor URL from what/users/{username}can actually serve, without any error at startup.Consider deriving
actor_id/inbox/outboxfromusername+handle_host(or asserting consistency) infrom_config.Also applies to: 36-61
🤖 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-runtime-server/src/app.rs` around lines 25 - 33, `AppState::from_config` currently builds `actor_id`, `inbox`, and `outbox` independently from config values that are also used by `/users/{username}` and WebFinger, so add a consistency check or derive all actor URLs from `username` + `handle_host` in one place. Update `from_config` to assert that `config.actor_id`’s path segment matches `config.username` and its host matches `config.handle_host`, or replace the separate construction with a single canonical builder used by `AppState`, `actor.rs`, and `webfinger.rs` so the advertised actor URL cannot drift from the served route.
26-33: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
notes/local_actorcloned per-request unlikecore.
AppStatewrapscoreinArc<Mutex<..>>for cheap cloning, butlocal_actor: Actorandnotes: Vec<Note>are plain fields cloned on everyState<AppState>extraction (i.e. on every request, including unrelated routes like/healthz). Negligible today with a single seeded note, but inconsistent with the Arc pattern already used forcore, and will matter oncenotesgrows per TODO(#25).Also applies to: 54-60
🤖 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-runtime-server/src/app.rs` around lines 26 - 33, `AppState` currently clones `local_actor: Actor` and `notes: Vec<Note>` on every `State<AppState>` extraction, unlike `core` which is cheap-cloned via `Arc<Mutex<FederCore>>`. Update `AppState` and its use in the request handlers to store `local_actor` and `notes` behind shared ownership (matching the `core` pattern) so unrelated routes do not duplicate them per request. Focus on the `AppState` struct and any constructor/handler code that accesses `local_actor` or `notes`, especially around the routes mentioned in the review.
🤖 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-runtime-server/Cargo.toml`:
- Line 19: The `tower` dependency is missing the `util` feature, so
`ServiceExt::oneshot` used by the tests will not be available. Update the
`tower` entry in `Cargo.toml` for `crates/feder-runtime-server` to enable
`features = ["util"]`, keeping the dependency version at 0.5 so the
`tower::ServiceExt` import resolves correctly.
In `@crates/feder-runtime-server/src/note.rs`:
- Around line 25-42: The note lookup in note() is re-deriving the canonical URL
with a hardcoded http scheme, which can fail when the configured note id uses a
different scheme. Update the lookup logic to compare against the note’s existing
id value directly instead of rebuilding it from app_state.handle_host, and keep
the change localized to the note() function so it matches the config.rs contract
for note_id.
---
Nitpick comments:
In `@crates/feder-runtime-server/Cargo.toml`:
- Around line 15-19: The dependency declaration for tower is inconsistent with
the workspace-managed style used for serde and serde_json. Update the
dev-dependencies entry in Cargo.toml for tower to use the workspace version
management pattern instead of a locally pinned version, matching the existing
dependency declarations in this manifest.
In `@crates/feder-runtime-server/src/app.rs`:
- Around line 25-33: `AppState::from_config` currently builds `actor_id`,
`inbox`, and `outbox` independently from config values that are also used by
`/users/{username}` and WebFinger, so add a consistency check or derive all
actor URLs from `username` + `handle_host` in one place. Update `from_config` to
assert that `config.actor_id`’s path segment matches `config.username` and its
host matches `config.handle_host`, or replace the separate construction with a
single canonical builder used by `AppState`, `actor.rs`, and `webfinger.rs` so
the advertised actor URL cannot drift from the served route.
- Around line 26-33: `AppState` currently clones `local_actor: Actor` and
`notes: Vec<Note>` on every `State<AppState>` extraction, unlike `core` which is
cheap-cloned via `Arc<Mutex<FederCore>>`. Update `AppState` and its use in the
request handlers to store `local_actor` and `notes` behind shared ownership
(matching the `core` pattern) so unrelated routes do not duplicate them per
request. Focus on the `AppState` struct and any constructor/handler code that
accesses `local_actor` or `notes`, especially around the routes mentioned in the
review.
In `@crates/feder-runtime-server/src/config.rs`:
- Around line 20-30: RuntimeConfig currently stores handle_host separately even
though the host:port is already present in bind and the IRI authorities for
actor_id/inbox/outbox/note_id, which can drift out of sync. Update
RuntimeConfig/default_local() to derive handle_host from actor_id’s authority
(or otherwise validate that these host segments match when loading config) so
WebFinger’s acct:{username}@{handle_host} and note lookup always use the same
host as the actor/note IDs.
🪄 Autofix (Beta)
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
Run ID: 4de8e3d0-1a4f-4188-9648-7605841dcf10
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/feder-runtime-server/Cargo.tomlcrates/feder-runtime-server/src/actor.rscrates/feder-runtime-server/src/app.rscrates/feder-runtime-server/src/config.rscrates/feder-runtime-server/src/lib.rscrates/feder-runtime-server/src/note.rscrates/feder-runtime-server/src/webfinger.rsmise.toml
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
1 participant
Summary
/users/{username}asapplication/activity+json./notes/{id}asapplication/activity+json.Design Notes
This PR adds the public discovery/read surface for the server runtime, but does not introduce durable runtime storage yet. The public Note endpoint uses a minimal in-memory seeded note to prove canonical URL routing and
feder-vocabserialization before storage exists.Durable storage, state restoration, and action execution are intentionally left to #25.
Validation
mise run checkmise run test/notes/1Closes #23
Summary by CodeRabbit
New Features
Bug Fixes
Tests