Conversation
Sync next with main
Assisted-by: Codex:gpt-5.5
Assisted-by: Codex:gpt-5.5
Add server runtime skeleton
Update license description to allow only 3.0
Assisted-by: Codex:gpt-5.5
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
Implement ActivityPub discovery endpoints
📝 WalkthroughWalkthroughThis PR adds a new Changesfeder-core state engine
feder-runtime-server crate
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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: 2
🧹 Nitpick comments (3)
crates/feder-runtime-server/src/app.rs (1)
64-73: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueNo tracing/observability middleware on the router.
tracing/tracing-subscriberare dependencies, butbuild_routerdoesn't attach a request-tracing layer (e.g.tower_http::trace::TraceLayer), so per-request spans/logs won't be emitted for the HTTP endpoints.🤖 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 64 - 73, The router built in build_router currently has no request-level tracing, so add an HTTP tracing middleware layer to the Router returned from build_router using the existing tracing-related dependencies. Attach a tower_http::trace::TraceLayer (or equivalent request span/log layer) to the Router before with_state(state), so all routes like healthz, webfinger, actor, and note emit per-request spans/logs.crates/feder-runtime-server/Cargo.toml (1)
7-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent dependency management: some deps centralized, others not.
serde,serde_json, andtowerare pulled via.workspace = true, butaxum,thiserror,tokio,tracing, andtracing-subscriberare declared with hardcoded versions directly in this crate even though the rootCargo.toml's[workspace.dependencies]table already centralizes shared deps. As more crates are added to this workspace, version drift becomes likely.♻️ Suggested consolidation
# In root Cargo.toml [workspace.dependencies] +axum = "0.8" +thiserror = "2" +tokio = { version = "1", features = ["macros", "net", "rt-multi-thread"] } +tracing = "0.1" +tracing-subscriber = { version = "0.3", features = ["env-filter"] } # In crates/feder-runtime-server/Cargo.toml -axum = "0.8" -thiserror = "2" -tokio = { version = "1", features = ["macros", "net", "rt-multi-thread"] } -tracing = "0.1" -tracing-subscriber = { version = "0.3", features = ["env-filter"] } +axum.workspace = true +thiserror.workspace = true +tokio.workspace = true +tracing.workspace = true +tracing-subscriber.workspace = true🤖 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 7 - 19, The dependency list in the feder-runtime-server Cargo.toml is mixing workspace-managed and hardcoded versions, which can lead to version drift. Update the package manifest to use the root workspace dependency entries for axum, thiserror, tokio, tracing, and tracing-subscriber, similar to serde and serde_json. Keep the feder-core and feder-vocab path dependencies as-is, and adjust the [dependencies] and [dev-dependencies] entries to reference the workspace-managed symbols consistently.crates/feder-runtime-server/src/main.rs (1)
29-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider graceful shutdown handling.
axum::serveruns without a shutdown signal, so in-flight requests are dropped abruptly on SIGINT/SIGTERM during restarts/redeploys.♻️ Suggested addition
let listener = tokio::net::TcpListener::bind(config.bind) .await .map_err(Error::Bind)?; - axum::serve(listener, app).await.map_err(Error::Serve)?; + axum::serve(listener, app) + .with_graceful_shutdown(async { + let _ = tokio::signal::ctrl_c().await; + }) + .await + .map_err(Error::Serve)?;🤖 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/main.rs` around lines 29 - 32, Add graceful shutdown handling to the server startup flow in main by wiring a shutdown signal into the axum::serve call instead of awaiting it directly. Update the listener/app serving block to use a shutdown future (for example from a SIGINT/SIGTERM handler) so in-flight requests can finish cleanly during restarts, and keep the error mapping through Error::Bind and Error::Serve intact.
🤖 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 @.github/workflows/main.yaml:
- Around line 24-37: The new cross-platform job’s checkout step should disable
Git credential persistence to avoid unnecessary repo write access. Update the
actions/checkout usage in the cross-platform workflow step to include
persist-credentials set to false, since this job only runs cargo test and does
not need to push or write back. Refer to the cross-platform job and its checkout
step in the workflow when making the change.
In `@crates/feder-runtime-server/src/main.rs`:
- Around line 18-25: main is still using RuntimeConfig::default_local(), which
hardcodes local-only settings instead of reading deployment configuration.
Update the main entry point to construct the RuntimeConfig from
environment-backed settings or a loader method, and then pass that env-derived
config into build_router so the server respects deployment values rather than
127.0.0.1:3000 and alice.
---
Nitpick comments:
In `@crates/feder-runtime-server/Cargo.toml`:
- Around line 7-19: The dependency list in the feder-runtime-server Cargo.toml
is mixing workspace-managed and hardcoded versions, which can lead to version
drift. Update the package manifest to use the root workspace dependency entries
for axum, thiserror, tokio, tracing, and tracing-subscriber, similar to serde
and serde_json. Keep the feder-core and feder-vocab path dependencies as-is, and
adjust the [dependencies] and [dev-dependencies] entries to reference the
workspace-managed symbols consistently.
In `@crates/feder-runtime-server/src/app.rs`:
- Around line 64-73: The router built in build_router currently has no
request-level tracing, so add an HTTP tracing middleware layer to the Router
returned from build_router using the existing tracing-related dependencies.
Attach a tower_http::trace::TraceLayer (or equivalent request span/log layer) to
the Router before with_state(state), so all routes like healthz, webfinger,
actor, and note emit per-request spans/logs.
In `@crates/feder-runtime-server/src/main.rs`:
- Around line 29-32: Add graceful shutdown handling to the server startup flow
in main by wiring a shutdown signal into the axum::serve call instead of
awaiting it directly. Update the listener/app serving block to use a shutdown
future (for example from a SIGINT/SIGTERM handler) so in-flight requests can
finish cleanly during restarts, and keep the error mapping through Error::Bind
and Error::Serve intact.
🪄 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: a443c948-c7e3-41a1-9c95-8b2b08a8ea9f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
.github/workflows/main.yamlCargo.tomlcrates/feder-core/src/lib.rscrates/feder-runtime-server/Cargo.tomlcrates/feder-runtime-server/README.mdcrates/feder-runtime-server/src/actor.rscrates/feder-runtime-server/src/app.rscrates/feder-runtime-server/src/config.rscrates/feder-runtime-server/src/error.rscrates/feder-runtime-server/src/lib.rscrates/feder-runtime-server/src/main.rscrates/feder-runtime-server/src/note.rscrates/feder-runtime-server/src/webfinger.rscrates/feder-vocab/src/lib.rscrates/feder-vocab/tests/phase1_shapes.rsmise.toml
Comment on lines
+24
to
+37
| cross-platform: | ||
| name: cargo test (${{ matrix.os }}) | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: | ||
| - ubuntu-latest | ||
| - macos-latest | ||
| - windows-latest | ||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| - run: cargo test --workspace |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Add persist-credentials: false to the new checkout step.
Static analysis flags the new cross-platform job's checkout for credential persistence (artipacked). Since this job only runs tests and doesn't push/write to the repo, disable credential persistence.
🔒️ Proposed fix
steps:
- uses: actions/checkout@v6
+ with:
+ persist-credentials: false
- uses: dtolnay/rust-toolchain@stable
- run: cargo test --workspace📝 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
| cross-platform: | |
| name: cargo test (${{ matrix.os }}) | |
| runs-on: ${{ matrix.os }} | |
| strategy: | |
| fail-fast: false | |
| matrix: | |
| os: | |
| - ubuntu-latest | |
| - macos-latest | |
| - windows-latest | |
| steps: | |
| - uses: actions/checkout@v6 | |
| - uses: dtolnay/rust-toolchain@stable | |
| - run: cargo test --workspace | |
| cross-platform: | |
| name: cargo test (${{ matrix.os }}) | |
| runs-on: ${{ matrix.os }} | |
| strategy: | |
| fail-fast: false | |
| matrix: | |
| os: | |
| - ubuntu-latest | |
| - macos-latest | |
| - windows-latest | |
| steps: | |
| - uses: actions/checkout@v6 | |
| with: | |
| persist-credentials: false | |
| - uses: dtolnay/rust-toolchain@stable | |
| - run: cargo test --workspace |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 35-35: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 24-38: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 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 @.github/workflows/main.yaml around lines 24 - 37, The new cross-platform
job’s checkout step should disable Git credential persistence to avoid
unnecessary repo write access. Update the actions/checkout usage in the
cross-platform workflow step to include persist-credentials set to false, since
this job only runs cargo test and does not need to push or write back. Refer to
the cross-platform job and its checkout step in the workflow when making the
change.
Source: Linters/SAST tools
Comment on lines
+18
to
+25
| #[tokio::main] | ||
| async fn main() -> Result<(), Error> { | ||
| tracing_subscriber::fmt() | ||
| .with_env_filter(tracing_subscriber::EnvFilter::from_default_env()) | ||
| .init(); | ||
|
|
||
| let config = RuntimeConfig::default_local(); | ||
| let app = build_router(&config); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'impl RuntimeConfig' -A 40 crates/feder-runtime-server/src/config.rs
rg -n 'from_env|std::env|env::var' crates/feder-runtime-server/src/config.rs crates/feder-runtime-server/src/main.rsRepository: fedify-dev/feder
Length of output: 1218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== main.rs ==\n'
cat -n crates/feder-runtime-server/src/main.rs | sed -n '1,220p'
printf '\n== config.rs ==\n'
cat -n crates/feder-runtime-server/src/config.rs | sed -n '1,220p'
printf '\n== runtime config usages ==\n'
rg -n "RuntimeConfig|default_local|from_env|std::env|env::var" crates/feder-runtime-server -SRepository: fedify-dev/feder
Length of output: 6046
Load runtime config from the environment RuntimeConfig::default_local() hardcodes 127.0.0.1:3000 and alice, so this entry point still ignores deployment settings. Wire main to an env-backed constructor/loader instead of the local defaults.
🤖 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/main.rs` around lines 18 - 25, main is still
using RuntimeConfig::default_local(), which hardcodes local-only settings
instead of reading deployment configuration. Update the main entry point to
construct the RuntimeConfig from environment-backed settings or a loader method,
and then pass that env-derived config into build_router so the server respects
deployment values rather than 127.0.0.1:3000 and alice.
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.
Merged next to main. next will be removed afterwards since it is unnecessary.
Summary by CodeRabbit
404.400.