feat: per-space bootstrap auth and relay configuration#479
Open
feat: per-space bootstrap auth and relay configuration#479
Conversation
|
The following will be added to the changelog [0.5.0-dev.2] - 2026-04-21Miscellaneous Tasks
[0.5.0-dev.1] - 2026-04-20Features
Bug Fixes
Miscellaneous Tasks
CI
Automated Changes
First-time Contributors
[0.5.0-dev.0] - 2026-04-09Features
Bug Fixes
Miscellaneous Tasks
Testing
Documentation
Other Changes
[0.4.0-dev.7] - 2026-04-02Features
Bug Fixes
Miscellaneous Tasks
Testing
Refactor
Automated Changes
[0.4.0-dev.6] - 2026-03-23Features
Bug Fixes
Miscellaneous Tasks
Refactor[0.4.0-dev.5] - 2026-03-20Features
Bug Fixes
Miscellaneous Tasks
[0.4.0-dev.4] - 2026-03-10Features
Automated Changes
[0.4.0-dev.3] - 2026-03-02Features
Bug Fixes
Miscellaneous Tasks
Build System
CI
Refactor
Automated Changes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds per-space relay configuration and optional Base64 auth material, validates/decodes it, wires dynamic relay insertion through a new transport API into the iroh transport and endpoint layers, updates tests, and adds a workspace Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/core/src/factories/core_bootstrap/test.rs (1)
83-88: LGTM, but consider adding test coverage for the new base64 path.The new field is correctly initialized to
None. However, the existing tests only exercise the builder-levelauth_materialfallback path. Consider adding a test that providesauth_material_base64to verify the per-space override and base64 decoding logic.Would you like me to help draft a test case that exercises the
auth_material_base64override path?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/core/src/factories/core_bootstrap/test.rs` around lines 83 - 88, Add a unit test that supplies CoreBootstrapConfig.auth_material_base64 (instead of None) to exercise the per-space override and base64 decoding logic; specifically, create a CoreBootstrapConfig with server_url set and auth_material_base64 populated with a known base64-encoded credential, call the builder/path that currently falls back to builder-level auth_material, and assert that the resulting credential used by the relevant component (the code that reads auth_material_base64 and decodes it) matches the decoded value; reference CoreBootstrapConfig.auth_material_base64 and any builder method that previously used auth_material to locate where to insert the new test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/factories/core_bootstrap.rs`:
- Around line 170-179: The code currently calls expect("invalid base64 in
auth_material_base64") while decoding config.auth_material_base64 which can
panic; replace this with error-safe handling by either validating the base64
during validate_config_for_context (check config.auth_material_base64 parses
with base64::engine::general_purpose::STANDARD.decode and return a K2Error on
failure) or, if you prefer to surface the error in creation, change the decode
in CoreBootstrap::new / BootstrapFactory::create to use decode(...).map_err(|e|
K2Error::new(...)) and propagate the error through the K2Result so
auth_material_bytes is built only on successful decode instead of calling
expect. Ensure references to auth_material_base64, auth_material_bytes,
validate_config_for_context, CoreBootstrap::new, and BootstrapFactory::create
are updated accordingly.
---
Nitpick comments:
In `@crates/core/src/factories/core_bootstrap/test.rs`:
- Around line 83-88: Add a unit test that supplies
CoreBootstrapConfig.auth_material_base64 (instead of None) to exercise the
per-space override and base64 decoding logic; specifically, create a
CoreBootstrapConfig with server_url set and auth_material_base64 populated with
a known base64-encoded credential, call the builder/path that currently falls
back to builder-level auth_material, and assert that the resulting credential
used by the relevant component (the code that reads auth_material_base64 and
decodes it) matches the decoded value; reference
CoreBootstrapConfig.auth_material_base64 and any builder method that previously
used auth_material to locate where to insert the new test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6fda860-f005-4432-99ab-2d0a289b3da2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
crates/core/Cargo.tomlcrates/core/src/factories/core_bootstrap.rscrates/core/src/factories/core_bootstrap/test.rscrates/kitsune2/tests/integration.rs
2 tasks
Deploying kitsune2 with
|
| Latest commit: |
fef7d21
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ef08918a.kitsune2.pages.dev |
| Branch Preview URL: | https://feat-per-space-boostrap-auth.kitsune2.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/core/src/factories/core_space.rs (1)
106-108: Silent failure when config retrieval fails.Using
if let Ok(...)silently ignores any error fromget_module_config(). While this may be intentional to allow spaces without bootstrap config, consider logging at debug level when the config is missing or malformed to aid troubleshooting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/core/src/factories/core_space.rs` around lines 106 - 108, The current use of if let Ok(bootstrap_config) = builder.config.get_module_config::<CoreBootstrapModConfig>() swallows errors silently; change it to match on the Result so that Err(e) is logged at debug level (e.g., using the existing logger) before continuing, e.g., replace the if let with a match or if let Err(e) branch and call debug!("failed to get CoreBootstrapModConfig: {:?}", e) (referencing get_module_config::<CoreBootstrapModConfig>() and the surrounding bootstrap_config handling code) so missing or malformed config is recorded without changing existing control flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/core/src/factories/core_space.rs`:
- Around line 116-120: Replace the panic-causing .expect("validated during
config validation") in the base64 decode closure with proper error
propagation/handling: change the closure inside map(...) to decode with
.map_err(...) or use the ? operator so decoding returns a Result<Vec<u8>, _>,
propagate that error out of the enclosing function (e.g., the space creation
factory function in core_space.rs that processes config_override) by updating
its return type to Result and mapping the decode error into the crate's error
type; ensure callers of the factory handle the Result so a bad config_override
no longer panics but returns a controlled error.
---
Nitpick comments:
In `@crates/core/src/factories/core_space.rs`:
- Around line 106-108: The current use of if let Ok(bootstrap_config) =
builder.config.get_module_config::<CoreBootstrapModConfig>() swallows errors
silently; change it to match on the Result so that Err(e) is logged at debug
level (e.g., using the existing logger) before continuing, e.g., replace the if
let with a match or if let Err(e) branch and call debug!("failed to get
CoreBootstrapModConfig: {:?}", e) (referencing
get_module_config::<CoreBootstrapModConfig>() and the surrounding
bootstrap_config handling code) so missing or malformed config is recorded
without changing existing control flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac7b9735-583e-44b5-8d48-914f19f89b1c
📒 Files selected for processing (8)
crates/api/src/transport.rscrates/core/src/factories/core_bootstrap.rscrates/core/src/factories/core_bootstrap/test.rscrates/core/src/factories/core_space.rscrates/kitsune2/tests/integration.rscrates/transport_iroh/src/endpoint.rscrates/transport_iroh/src/lib.rscrates/transport_iroh/src/tests/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/core/src/factories/core_bootstrap/test.rs
- crates/kitsune2/tests/integration.rs
- crates/core/src/factories/core_bootstrap.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/transport_iroh/src/url.rs (1)
80-94: Consider extracting duplicated relay URL reconstruction.The logic for reconstructing the relay URL from the peer address (lines 82-86) is duplicated in the
Nonecase (lines 89-93). A small helper or restructuring could reduce this duplication.♻️ Possible refactor
+ let reconstruct_relay_url = || -> K2Result<RelayUrl> { + let relay_scheme = if url.uses_tls() { "https" } else { "http" }; + let relay_url = format!("{relay_scheme}://{peer_addr}"); + let relay_url = ::url::Url::from_str(&relay_url) + .map_err(|err| K2Error::other_src("invalid relay url", err))?; + Ok(RelayUrl::from(relay_url)) + }; + let relay_url = if let Some(configured) = configured_relay_url { // ... host comparison logic ... if configured_host == peer_host { RelayUrl::from_str(configured).map_err(|err| { K2Error::other_src("invalid configured relay url", err) })? } else { - // Peer is on a different relay — reconstruct from the peer URL - let relay_scheme = if url.uses_tls() { "https" } else { "http" }; - let relay_url = format!("{relay_scheme}://{peer_addr}"); - let relay_url = ::url::Url::from_str(&relay_url) - .map_err(|err| K2Error::other_src("invalid relay url", err))?; - RelayUrl::from(relay_url) + reconstruct_relay_url()? } } else { - let relay_scheme = if url.uses_tls() { "https" } else { "http" }; - let relay_url = format!("{relay_scheme}://{peer_addr}"); - let relay_url = ::url::Url::from_str(&relay_url) - .map_err(|err| K2Error::other_src("invalid relay url", err))?; - RelayUrl::from(relay_url) + reconstruct_relay_url()? };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/transport_iroh/src/url.rs` around lines 80 - 94, The relay URL reconstruction from peer_addr is duplicated; extract a small helper (e.g., fn relay_from_peer(peer_addr: &str, url: &Url) -> Result<RelayUrl, K2Error>) that encapsulates the relay_scheme determination (url.uses_tls()), formatting the "{scheme}://{peer_addr}", parsing via ::url::Url::from_str and mapping errors with K2Error::other_src, then replace both duplicated blocks with a call to that helper and return RelayUrl::from(parsed_url).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/transport_iroh/src/url.rs`:
- Around line 70-76: The host comparison fails for IPv6 because peer_host is
extracted via peer_addr.split(':'), which breaks addresses like "[::1]:8080";
instead, parse peer_addr into a Url to extract the host the same way
configured_host is extracted: construct a temporary URL (e.g., prefix peer_addr
with a scheme like "http://" before calling ::url::Url::from_str), then use the
Url.host_str() result to set peer_host and compare against configured_host
(update the peer_host extraction and the conditional that compares
configured_host == peer_host).
---
Nitpick comments:
In `@crates/transport_iroh/src/url.rs`:
- Around line 80-94: The relay URL reconstruction from peer_addr is duplicated;
extract a small helper (e.g., fn relay_from_peer(peer_addr: &str, url: &Url) ->
Result<RelayUrl, K2Error>) that encapsulates the relay_scheme determination
(url.uses_tls()), formatting the "{scheme}://{peer_addr}", parsing via
::url::Url::from_str and mapping errors with K2Error::other_src, then replace
both duplicated blocks with a call to that helper and return
RelayUrl::from(parsed_url).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9368d151-3148-430e-9a83-c0c1e5f70e55
📒 Files selected for processing (1)
crates/transport_iroh/src/url.rs
…-boostrap-auth-override
lucksus
changed the title
auth_material_base64 to CoreBoostrapConfig
ThetaSinner
requested changes
Mar 31, 2026
Member
ThetaSinner
left a comment
ThetaSinner
left a comment
There was a problem hiding this comment.
Pausing my review because I can see a number of issues that suggest to me I should do so.
- I don't see integration tests that multiple relays and multiple spaces in various configurations actually work and stay separate. I'm also unclear looking at this whether it's intended and tested that multiple relays may exist within a single space and how that is handled. Either by rejecting it or allowing it, but either way, I intended that to be part of the design for this feature.
- The boundary hygiene seems problematic. The method I was concerned about at first, for adding a relay, I understand why that's needed. Why the bootstrap server needs to be aware of relays now, I don't understand. We made a major effort to make modules independent of each other's implementations and to primarily rely on their API. In a few cases we do rely on behavior but it's documented and not part of the implementation where possible.
- The bootstrap configuration logic has been duplicated. There was already a way to do that, and it's integrated into Holochain and we'd discussed that but there's now also configuration on the
CoreBootstrapConfig. - This PR introduces per-space pre-flight which may well be needed, but that's something that has a design under the name "hello" module. Not attached to the name but that module had a purpose and constraints - so quietly introducing an alternative is a problem we'd have to unpick later.
The missing testing, I'm just unhappy about. That's always expected for PRs where something can be tested. We have all the test infrastructure we'd need to make that happen in integration tests within the repo.
The rest of my feedback requires project context - but I think between conversations and the issue list for the repo most of that context does exist. I don't feel like it's a good use of my time to review code that doesn't fit the project well, even if it solves a problem in the short term. Especially in Kitsune2, where it's been created to replace Kitsune where the code became so interleaved that it was unmaintainable. It's important that the API+module structure, testing and design is defended.
| .set_module_config(&super::CoreBootstrapModConfig { | ||
| core_bootstrap: super::CoreBootstrapConfig { | ||
| server_url: Some(server.into()), | ||
| auth_material_base64: None, |
Member
There was a problem hiding this comment.
Should not be a module configuration, already available on the builder in the factory.
| core_bootstrap: super::CoreBootstrapConfig { | ||
| server_url: Some(server.into()), | ||
| auth_material_base64: None, | ||
| relay_url: None, |
Member
There was a problem hiding this comment.
Bootstrap shouldn't need to know about the relay. I get that logic needs to exist somewhere for the space to choose to use a given relay but the bootstrap service isn't where I'd expect.
Comment on lines
+187
to
+197
| // Prefer per-space auth_material from config over builder-level. | ||
| // Safety: auth_material_base64 is validated during config validation. | ||
| let auth_material_bytes: Option<Vec<u8>> = config | ||
| .auth_material_base64 | ||
| .as_ref() | ||
| .map(|b64| { | ||
| base64::engine::general_purpose::STANDARD | ||
| .decode(b64) | ||
| .expect("validated during config validation") | ||
| }) | ||
| .or_else(|| builder.auth_material_bootstrap.clone()); |
Member
There was a problem hiding this comment.
This was already working, the builder config can be overridden before being provided here. That should have had existing test coverage.
…per-space override config
…transport concern
…lay feature works as expected
…t since config is applied internally now
ThetaSinner
reviewed
Apr 1, 2026
| &self, | ||
| _space_id: SpaceId, | ||
| _config: &config::Config, | ||
| _auth_material_relay: Option<Vec<u8>>, |
Member
There was a problem hiding this comment.
This should come from the Config if possible. The setup of a space can clone and edit the configuration before spawning (already implemented in Holochain) so we shouldn't need to pass anything extra. The auth material and relay url can come from the config.
Comment on lines
+459
to
+460
| /// Returns an optional URL that should be used as this space's | ||
| /// pinned address for agent info (e.g., the endpoint's address on |
Member
There was a problem hiding this comment.
This is problematic. If the node is offline, that will block the function from completing. We can't assume we can reach the relay at this point, we just need to notify the transport that we want to use it. There's already a mechanism for returning the listening address to the space to get it into the agent info. That's intentionally designed to handle relay unavailable or unreachable situations. Yes it means you don't have the url right away, but you can't guarantee that and trying will lead to other undesirable behavior.
Collaborator
Author
There was a problem hiding this comment.
I see. That complicates things. I look into using that mechanism for late returning of the address.
Collaborator
Author
There was a problem hiding this comment.
Hm, that existing mechanism doesn't seem to be ready for different addresses per space. So using it and getting rid of pinned_url won't work.
Claude says:
Per-space URL broadcast overwrite problem
When multiple spaces each have their own relay, the address watcher's new_listening_address broadcasts a single URL to all spaces. This overwrites per-space URLs — e.g., space B ends up with relay A's URL. Additionally, get_url_with_first_relay only produces a URL for the first relay, so per-space relays added later never surface through the watcher at all.
Something needs to prevent the global broadcast from overwriting per-space URLs. The question is which layer owns that:
| Approach | Where | Pro | Con |
|---|---|---|---|
per_space_managed set on TxImpHnd |
API layer | Space stays simple; transport controls everything | Adds new state/concepts to the shared API layer |
has_per_space_url flag on InnerData |
Space layer | Minimal code, easy to understand | Transport-specific concern leaks into the space |
| Smart address watcher in iroh | Transport impl | All logic stays in iroh, no API changes | Watcher still needs to know which spaces to skip — filtering problem just moves |
I have trade-off 1 per_space_managed set on TxImpHnd in my local files and will review and push if sounds good.
| /// to the bootstrap module: space creation succeeds and agents can be | ||
| /// exchanged when auth material is provided only via per-space config. | ||
| #[tokio::test(flavor = "multi_thread")] | ||
| async fn bootstrap_with_per_space_auth_override() { |
Member
There was a problem hiding this comment.
This test doesn't add much value. It's configuring an auth material override but the bootstrap service doesn't require authentication. I'd suggest that this case doesn't make a good unit test. A functional/integration test that runs two bootstrap services, each with their own auth service is the required setup for such a test. Then you can demonstrate starting a space without an override and check that no bootstrap happens. Then you can restart the space with an auth material override and check that it does manage to publish its agent info to the bootstrap server.
Collaborator
Author
There was a problem hiding this comment.
yeah that integration test is below. this is just testing the override flow. do you want me to remove this one?
| pub use config::*; | ||
|
|
||
| /// Per-space override configuration types. | ||
| pub mod per_space_overrides_config { |
Member
There was a problem hiding this comment.
I think this isn't needed. The existing pattern of clone and update the config when creating a space should cover this. The SpaceFactory has a parameter config_override: Option<Config>, which allows updated configuration to be passed for just that space. Where possible, I'd like to just use that pattern - and I think for this PR it should be possible to follow.
| /// Maps relay host -> our per-space URL on that relay. | ||
| /// Used to send the correct local URL in preflights for connections | ||
| /// through non-global relays. | ||
| per_space_urls: Arc<RwLock<HashMap<String, Url>>>, |
Member
There was a problem hiding this comment.
So this is the transport remembering what URL was configured for a given space? Should be SpaceId as the key again? Then I think this makes sense, it needs to know this.
Comment on lines
+737
to
+739
| /// Returns the correct local URL for a connection to the given remote. | ||
| /// If the remote is on an inserted (per-space) relay, returns our URL | ||
| /// on that relay. Otherwise returns the global local URL. |
Member
There was a problem hiding this comment.
local and remote are quite confusing in here. Local means our URL? Or something else?
This may be me not being clear on existing naming - @jost-s
Comment on lines
+869
to
+873
| let relay_url_str = if relay_url.ends_with('/') { | ||
| relay_url | ||
| } else { | ||
| format!("{relay_url}/") | ||
| }; |
Member
There was a problem hiding this comment.
Or just reject it in the configuration validation so that the configuration must be correctly provided by the user?
| ) | ||
| })?; | ||
| let mut server_url = server_url; | ||
| server_url.set_path("/"); |
Member
There was a problem hiding this comment.
This forces any custom path out right? So the problem raised above it at least made worse here?
| // This strips any path from the relay URL (like /relay/) | ||
| // Construct kitsune2 URL: scheme://host:port/endpoint_id | ||
| // The relay path (e.g. /relay/) is not included here because the | ||
| // kitsune2 Url format only allows a single path segment. |
Member
There was a problem hiding this comment.
That seems like something to address, possible as a change before this one?
Member
There was a problem hiding this comment.
The logic is that the peer URL format is known there, because the path is always just the endpoint id but there's no reason it can't just be "the last path segment is the endpoint id" right?
Collaborator
Author
There was a problem hiding this comment.
This is what I was raising and why this PR was not tidied up in its first form. We could maybe do a smarter hack than this map of know relays, but I from my still limited vantage point, it seems preferable to make K2::Url allow for /what/ever/../<endpoint id>/, introducing a breaking protocol change.
1. Simplified test builder (integration.rs:648-651): Removed the redundant transport: IrohTransportFactory::create() override — default_builder() already uses iroh since it became the default transport. 2. Wrapped current_url() in iter_check! (integration.rs:799+): The relay address is assigned asynchronously, so the test now polls with iter_check!(10_000, 500, ...) instead of calling .expect() synchronously. 3. Assert hosts differ (integration.rs:836): Changed from assert_ne!(url_a_str, url_b_str) (which could pass if only public keys differ) to assert_ne!(host_a, host_b) which verifies the relay servers themselves are distinct.
Remove auth_material_relay parameter (transport reads auth from its own per-space config section). Change return type from K2Result<Option<Url>> to K2Result<()> — the transport will deliver per-space URLs asynchronously instead of returning them. Made-with: Cursor
Add new_listening_address_for_space for targeted per-space URL delivery (vs the existing broadcast new_listening_address). Includes: - pending_space_urls buffer: if the space handler isn't registered yet when the URL arrives, it's stored and delivered on register - per_space_managed set: spaces with per-space URLs are excluded from the global new_listening_address broadcast to prevent overwrite - DefaultTransport::register_space_handler checks pending URLs Made-with: Cursor
Counterpart to configure_for_space. Called when a space is torn down so the transport can release per-space resources (e.g., remove a relay that was added exclusively for that space). Default is a no-op. Made-with: Cursor
- Move auth_material_relay_base64 into IrohTransportPerSpaceConfig so the transport reads its own auth material from config - Add base64 as a runtime dependency for decoding per-space auth - configure_for_space spawns relay insertion as a background task and returns immediately; URL delivered asynchronously via TxImpHnd::new_listening_address_for_space - Replace insert_relay(&self) with static do_insert_relay() so the spawned task doesn't need &self - Add space_relay_hosts map to track SpaceId -> relay host - Implement unconfigure_for_space: cleans up relay state when no remaining spaces use that relay, unmarks per_space_managed Made-with: Cursor
- Remove pinned_url from InnerData — the transport now delivers per-space URLs asynchronously via new_listening_address_for_space and the per_space_managed set prevents global broadcast overwrite - Simplify new_url (no pinned_url guard), current_url, local_agent_join - Remove auth_material_relay_base64 from CoreSpacePerSpaceOverridesConfig (now lives in IrohTransportPerSpaceConfig where it belongs) - Simplify apply_per_space_auth_overrides (bootstrap auth only) - Update CoreSpaceFactory::create for non-blocking configure_for_space Made-with: Cursor
- configure_for_space_adds_relay: non-blocking call, no return value assertion, use ..Default::default() for new config fields - configure_for_space_noop_without_config: simplified assertion - two_spaces_different_relays: use ..Default::default() for IrohTransportPerSpaceConfig Made-with: Cursor
…-boostrap-auth-override
ThetaSinner
requested changes
Apr 15, 2026
lucksus
force-pushed
the
feat/per-space-boostrap-auth-override
branch
from
ef1cbbc to
baf6f10
Compare
lucksus
force-pushed
the
feat/per-space-boostrap-auth-override
branch
from
e1ae8b1 to
a3596cf
Compare
|
✔️ b3f64cf...fef7d21 - Conventional commits check succeeded. |
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
2 participants
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.
tl;dr
This allows per space bootstrap and relay overrides, and with auth material.
To be used in holochain/holochain#5684
Remark
The handling of the /relay path made this a bit more complicated. Same reason I had to make endpoint_from_url() compare against the configured relay, which was a fix that worked with one relay. I would love to clean this up more, but it would need a change to K2's URL type, allowing more than one path segment, which would be a breaking change. That is the reason we need the known_relays map.
Claude summary below here
Summary
Adds per-space network isolation: each space can have its own bootstrap auth material and iroh relay server, allowing authed and open spaces to coexist on a single node.
Claude: Architecture
Per-space configuration follows the established module pattern — each module owns its own config section and handles its own per-space setup:
CoreSpacePerSpaceOverridesModConfig): The space factory reads base64-encoded per-space auth material from a dedicated config section and injects it into a cloned builder. Bootstrap and relay modules consume it from the builder as usual.IrohTransportPerSpaceModConfig): TheTransporttrait exposes a generalconfigure_for_space()method. The iroh transport implements this by reading its own per-space config section for arelay_urland calling its internalinsert_relay(). Other transports can implement this differently or leave the default no-op.insert_relay()is an internal implementation detail of the iroh transport, not part of the publicTransporttrait.Claude: Key changes
Transport::configure_for_space()— New trait method onTxImp/Transport, called byCoreSpaceFactoryduring space creation. Default is a no-op. Iroh transport readsIrohTransportPerSpaceModConfigand dynamically adds a relay.CoreSpacePerSpaceOverridesModConfig— New config section for per-space auth material (bootstrap + relay), decoded from base64 and applied to a cloned builder.Per-space URL pinning — Spaces with a per-space relay pin their URL from that relay and ignore global transport URL updates.
Correct peer addressing —
endpoint_from_url()uses aknown_relaysmap to reconstruct relay URLs with their original path (e.g./relay/) since the kitsune2Urlwire format only allows single-segment paths.Per-space preflights — Outbound and return preflights advertise the correct per-space relay URL instead of always using the global one.
Bootstrap server fixes — Handle HTTP 202 pending-approval, mirror request origin for auth-enabled servers.
Test plan
known_relayslookup, precedence over configured relay, roundtrip with/without pathrelay_auth.rs)Summary by CodeRabbit
New Features
Tests
Chores