bug: some islands use ExternalServiceIframe instead of available archipelago embeds #74

Open
opened 2026-04-19 11:14:59 +00:00 by salmaelsoly · 3 comments
Member

Problem

Several islands have archipelago embed components available in hero_archipelagos but island_content.rs uses hardcoded ExternalServiceIframe instead.

Affected islands

Island Current (hero_os) Available embed
books ExternalServiceIframe { src: "/hero_books/ui/" } hero_archipelagos_books
compute ExternalServiceIframe { src: "/hero_compute/ui/" } hero_archipelagos_compute
voice ExternalServiceIframe hero_archipelagos_voice::island::VoiceApp

Why this matters

The archipelago embed components handle theme sync, auth token passing, and hero_router URL resolution correctly. ExternalServiceIframe is a generic fallback that lacks per-island logic and has known issues with window_id not being appended to the iframe src.

Files

  • crates/hero_os_app/src/island_content.rs:367 — books
  • crates/hero_os_app/src/island_content.rs:428 — compute
  • crates/hero_os_app/src/island_content.rs:375-384 — voice
## Problem Several islands have archipelago embed components available in `hero_archipelagos` but `island_content.rs` uses hardcoded `ExternalServiceIframe` instead. ## Affected islands | Island | Current (hero_os) | Available embed | |--------|-------------------|------------------| | books | `ExternalServiceIframe { src: "/hero_books/ui/" }` | `hero_archipelagos_books` | | compute | `ExternalServiceIframe { src: "/hero_compute/ui/" }` | `hero_archipelagos_compute` | | voice | `ExternalServiceIframe` | `hero_archipelagos_voice::island::VoiceApp` | ## Why this matters The archipelago embed components handle theme sync, auth token passing, and hero_router URL resolution correctly. `ExternalServiceIframe` is a generic fallback that lacks per-island logic and has known issues with `window_id` not being appended to the iframe src. ## Files - `crates/hero_os_app/src/island_content.rs:367` — books - `crates/hero_os_app/src/island_content.rs:428` — compute - `crates/hero_os_app/src/island_content.rs:375-384` — voice
Author
Member

Implementation Spec for Issue #74

Objective

Replace the two remaining ExternalServiceIframe fallbacks (books, compute) in island_content.rs with the real archipelago embed components (BooksApp, ComputeApp). The voice arm cited in the issue is already wired correctly to VoiceApp — no change needed there. The archipelago embed components construct their iframe src via IslandContext::with_window_id(...) which appends hero_window_id=<id>, fixing the missing-window_id bug that ExternalServiceIframe exhibits and using the per-island theme/auth sync logic rather than the generic fallback.

Requirements

  • After the change, when island-books / island-compute features are enabled, hero_os must render BooksApp / ComputeApp (from the embed crates) instead of ExternalServiceIframe.
  • For compute, cargo features gate the switch: island-compute selects the embed path, island-compute-native (currently a dead code path) selects the native arm, otherwise the iframe fallback remains.
  • For books, the arm was already split on island-books-native; the non-native arm must be further gated on feature island-books so the iframe fallback is reachable when the feature is off.
  • Feature island-books must be added to the default web feature list so the switch takes effect in the default web build (it is currently absent from web, likely because the iframe was used regardless).
  • No new crate dependencies — hero_archipelagos_books, hero_archipelagos_compute, and hero_archipelagos_voice are already declared as optional git deps in hero_os_app/Cargo.toml on branch = "development". The workspace Cargo.toml does not re-declare archipelago crates.

Files to Modify

  • crates/hero_os_app/src/island_content.rs — swap the two iframe arms (books + compute), add two imports
  • crates/hero_os_app/Cargo.toml — add island-books to the web feature list (no dep changes)

Implementation Plan

Step 1: Add island-books to the web feature in hero_os_app/Cargo.toml

File: crates/hero_os_app/Cargo.toml
Rationale: island-compute and island-voice are already listed; island-books is absent, which means the #[cfg(feature = "island-books")] gate added in Step 3 would exclude the embed path in the default build. Adding it here makes the new arm reachable.

Add "island-books", alongside the other island-* features already listed in the web array (around lines 114–130). The canonical place is between "island-foundry-admin", and "island-embedder",, matching the order in which other embed islands are listed.

Dependencies: none.

Step 2: Add BooksApp and ComputeApp imports to island_content.rs

File: crates/hero_os_app/src/island_content.rs
Rationale: Follow the per-island pub use pattern used for every other archipelago embed in this file (e.g. ProcApp, RedisApp). Placed near the other embed/* re-exports, replacing the stale "Books UI is served as standalone iframe" comment.

Old snippet (around lines 36–39):

// Router removed — duplicate of Proxy
// Books UI is served as standalone iframe (hero_books_ui), not as a WASM island
#[cfg(all(feature = "island-embedder", not(feature = "island-embedder-native")))]
pub use hero_archipelagos_embedder::island::EmbedderApp;

New snippet:

// Router removed — duplicate of Proxy
#[cfg(all(feature = "island-books", not(feature = "island-books-native")))]
pub use hero_archipelagos_books::BooksApp;
#[cfg(all(feature = "island-compute", not(feature = "island-compute-native")))]
pub use hero_archipelagos_compute::ComputeApp;
#[cfg(all(feature = "island-embedder", not(feature = "island-embedder-native")))]
pub use hero_archipelagos_embedder::island::EmbedderApp;

hero_archipelagos_books::BooksApp and hero_archipelagos_compute::ComputeApp are both crate-root re-exports (verified in each crate's lib.rs). This matches how CallIslandApp / MessagingArchipelagoApp are imported.

Dependencies: none (compiles independently; Step 1 only affects which arm the default build selects at runtime).

Step 3: Replace the books iframe arm with a feature-gated BooksApp

File: crates/hero_os_app/src/island_content.rs
Rationale: Mirror the proc / redis / voice three-way arm pattern — embed crate when island-books, native app when island-books-native, iframe fallback when neither.

Old snippet (around lines 365–371):

        // Router removed — duplicate of Proxy
        // Books: iframe or native Dioxus island
        #[cfg(not(feature = "island-books-native"))]
        "books" => rsx! { ExternalServiceIframe { src: "/hero_books/ui/", id: "books-iframe" } },
        #[cfg(feature = "island-books-native")]
        "books" => rsx! {
            hero_books_app::HeroBooksApp { context: Some(context) }
        },

New snippet:

        // Router removed — duplicate of Proxy
        // Books: archipelagos embed or native Dioxus island
        #[cfg(all(feature = "island-books", not(feature = "island-books-native")))]
        "books" => rsx! {
            BooksApp {
                context: context,
            }
        },
        #[cfg(feature = "island-books-native")]
        "books" => rsx! {
            hero_books_app::HeroBooksApp { context: Some(context) }
        },

Dependencies: Step 2 (for the BooksApp import). Step 1 (so the default web build enables island-books and the embed arm is selected).

The iframe-fallback arm is dropped on purpose: every practical build path goes through web (→ enables island-books) or web-native (→ enables island-books-native), so a third "neither feature" arm would be dead code.

Step 4: Replace the compute iframe arm with a feature-gated ComputeApp

File: crates/hero_os_app/src/island_content.rs
Rationale: The current compute arm is unconditional — split it so the embed is selected when island-compute is enabled and island-compute-native isn't (that is the current default in web). Keep the iframe fallback for the "neither" case.

Old snippet (around line 427–428):

        // Compute: iframe (hero_cloud_app path dep temporarily disabled — hero_compute on a side branch)
        "compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } },

New snippet:

        // Compute: archipelagos embed (hero_cloud_app path dep temporarily disabled — hero_compute on a side branch)
        #[cfg(all(feature = "island-compute", not(feature = "island-compute-native")))]
        "compute" => rsx! {
            ComputeApp {
                context: context,
            }
        },
        #[cfg(all(not(feature = "island-compute"), not(feature = "island-compute-native")))]
        "compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } },

The island-compute-native feature currently has no native app dep hooked up (hero_cloud_app is commented out in Cargo.toml). The cfg(not(...)) guards are defensive for symmetry with other island arms.

Dependencies: Step 2 (for the ComputeApp import).

Acceptance Criteria

  • cargo check -p hero_os_app --no-default-features --features web succeeds — the BooksApp and ComputeApp imports and match arms compile.
  • cargo check -p hero_os_app --no-default-features --features web-native still succeeds (native arms remain valid).
  • In a running hero_os web build, opening the books island renders the .books-island wrapper from BooksApp rather than .embed-container-plain from ExternalServiceIframe. Same for compute — wrapper becomes .compute-embed.embed-container.
  • The iframe src for compute contains hero_window_id=<id> when a window_id is provided (ComputeApp uses with_window_id).
  • The voice island continues to work unchanged — it was already using VoiceApp, the issue body was out of date for that row.
  • No default-feature code path reaches ExternalServiceIframe for books or compute.

Notes

Confirmed line numbers after drift check (current HEAD of hero_os, crates/hero_os_app/src/island_content.rs):

  • books: line 367 — "books" => rsx! { ExternalServiceIframe { src: "/hero_books/ui/", id: "books-iframe" } }, (gated by #[cfg(not(feature = "island-books-native"))] at line 366). Matches the issue.
  • compute: line 428 — "compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } }, (ungated). Matches the issue.
  • voice: lines 375–384 — already using VoiceApp via the archipelago embed. The issue description is stale here; no change needed for voice. The VoiceApp re-export at line 49 is already in place.

Dependency status (Cargo.toml):

  • hero_archipelagos_books — declared optional at line 296, branch development. No change needed.
  • hero_archipelagos_compute — declared optional at line 300, branch development. No change needed.
  • hero_archipelagos_voice — declared optional at line 303, branch development. Already in use.
  • Workspace root Cargo.toml does not re-declare archipelago crates, so no workspace change is needed.
  • Lines 128–130 of the hero_os workspace Cargo.toml contain a [patch] for hero_archipelagos_voice + hero_archipelagos_core pointing to ../hero_archipelagos/..., marked "DO NOT COMMIT — injected at build time". This is an unintended leak and is not in scope for this issue, but the implementer should be aware it exists: when testing locally, the build will pick up the sibling checkout instead of the git-pinned version. Do NOT rely on this patch being present in CI.

hero_archipelagos embed APIs (verified):

  • hero_archipelagos_books::BooksApp with BooksAppProps { context: IslandContext } — ready.
  • hero_archipelagos_compute::ComputeApp with ComputeAppProps { context: IslandContext } — ready.
  • hero_archipelagos_voice::island::VoiceApp with VoiceAppProps { context: IslandContext } — ready and already in use.

WASM readiness of voice crate: the voice crate pulls in wasm-bindgen, js-sys, web-sys unconditionally. Listed as a hero_archipelagos workspace member. Already in use in hero_os — wasm32 builds are confirmed to work.

## Implementation Spec for Issue #74 ### Objective Replace the two remaining `ExternalServiceIframe` fallbacks (`books`, `compute`) in `island_content.rs` with the real archipelago embed components (`BooksApp`, `ComputeApp`). The `voice` arm cited in the issue is already wired correctly to `VoiceApp` — no change needed there. The archipelago embed components construct their iframe src via `IslandContext::with_window_id(...)` which appends `hero_window_id=<id>`, fixing the missing-window_id bug that `ExternalServiceIframe` exhibits and using the per-island theme/auth sync logic rather than the generic fallback. ### Requirements - After the change, when `island-books` / `island-compute` features are enabled, hero_os must render `BooksApp` / `ComputeApp` (from the embed crates) instead of `ExternalServiceIframe`. - For `compute`, cargo features gate the switch: `island-compute` selects the embed path, `island-compute-native` (currently a dead code path) selects the native arm, otherwise the iframe fallback remains. - For `books`, the arm was already split on `island-books-native`; the non-native arm must be further gated on feature `island-books` so the iframe fallback is reachable when the feature is off. - Feature `island-books` must be added to the default `web` feature list so the switch takes effect in the default web build (it is currently absent from `web`, likely because the iframe was used regardless). - No new crate dependencies — `hero_archipelagos_books`, `hero_archipelagos_compute`, and `hero_archipelagos_voice` are already declared as optional git deps in `hero_os_app/Cargo.toml` on `branch = "development"`. The workspace `Cargo.toml` does not re-declare archipelago crates. ### Files to Modify - `crates/hero_os_app/src/island_content.rs` — swap the two iframe arms (books + compute), add two imports - `crates/hero_os_app/Cargo.toml` — add `island-books` to the `web` feature list (no dep changes) ### Implementation Plan #### Step 1: Add `island-books` to the `web` feature in `hero_os_app/Cargo.toml` File: `crates/hero_os_app/Cargo.toml` Rationale: `island-compute` and `island-voice` are already listed; `island-books` is absent, which means the `#[cfg(feature = "island-books")]` gate added in Step 3 would exclude the embed path in the default build. Adding it here makes the new arm reachable. Add `"island-books",` alongside the other `island-*` features already listed in the `web` array (around lines 114–130). The canonical place is between `"island-foundry-admin",` and `"island-embedder",`, matching the order in which other embed islands are listed. Dependencies: none. #### Step 2: Add `BooksApp` and `ComputeApp` imports to `island_content.rs` File: `crates/hero_os_app/src/island_content.rs` Rationale: Follow the per-island `pub use` pattern used for every other archipelago embed in this file (e.g. `ProcApp`, `RedisApp`). Placed near the other `embed/*` re-exports, replacing the stale "Books UI is served as standalone iframe" comment. Old snippet (around lines 36–39): ```rust // Router removed — duplicate of Proxy // Books UI is served as standalone iframe (hero_books_ui), not as a WASM island #[cfg(all(feature = "island-embedder", not(feature = "island-embedder-native")))] pub use hero_archipelagos_embedder::island::EmbedderApp; ``` New snippet: ```rust // Router removed — duplicate of Proxy #[cfg(all(feature = "island-books", not(feature = "island-books-native")))] pub use hero_archipelagos_books::BooksApp; #[cfg(all(feature = "island-compute", not(feature = "island-compute-native")))] pub use hero_archipelagos_compute::ComputeApp; #[cfg(all(feature = "island-embedder", not(feature = "island-embedder-native")))] pub use hero_archipelagos_embedder::island::EmbedderApp; ``` `hero_archipelagos_books::BooksApp` and `hero_archipelagos_compute::ComputeApp` are both crate-root re-exports (verified in each crate's `lib.rs`). This matches how `CallIslandApp` / `MessagingArchipelagoApp` are imported. Dependencies: none (compiles independently; Step 1 only affects which arm the default build selects at runtime). #### Step 3: Replace the books iframe arm with a feature-gated `BooksApp` File: `crates/hero_os_app/src/island_content.rs` Rationale: Mirror the `proc` / `redis` / `voice` three-way arm pattern — embed crate when `island-books`, native app when `island-books-native`, iframe fallback when neither. Old snippet (around lines 365–371): ```rust // Router removed — duplicate of Proxy // Books: iframe or native Dioxus island #[cfg(not(feature = "island-books-native"))] "books" => rsx! { ExternalServiceIframe { src: "/hero_books/ui/", id: "books-iframe" } }, #[cfg(feature = "island-books-native")] "books" => rsx! { hero_books_app::HeroBooksApp { context: Some(context) } }, ``` New snippet: ```rust // Router removed — duplicate of Proxy // Books: archipelagos embed or native Dioxus island #[cfg(all(feature = "island-books", not(feature = "island-books-native")))] "books" => rsx! { BooksApp { context: context, } }, #[cfg(feature = "island-books-native")] "books" => rsx! { hero_books_app::HeroBooksApp { context: Some(context) } }, ``` Dependencies: Step 2 (for the `BooksApp` import). Step 1 (so the default `web` build enables `island-books` and the embed arm is selected). The iframe-fallback arm is dropped on purpose: every practical build path goes through `web` (→ enables `island-books`) or `web-native` (→ enables `island-books-native`), so a third "neither feature" arm would be dead code. #### Step 4: Replace the compute iframe arm with a feature-gated `ComputeApp` File: `crates/hero_os_app/src/island_content.rs` Rationale: The current compute arm is unconditional — split it so the embed is selected when `island-compute` is enabled and `island-compute-native` isn't (that is the current default in `web`). Keep the iframe fallback for the "neither" case. Old snippet (around line 427–428): ```rust // Compute: iframe (hero_cloud_app path dep temporarily disabled — hero_compute on a side branch) "compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } }, ``` New snippet: ```rust // Compute: archipelagos embed (hero_cloud_app path dep temporarily disabled — hero_compute on a side branch) #[cfg(all(feature = "island-compute", not(feature = "island-compute-native")))] "compute" => rsx! { ComputeApp { context: context, } }, #[cfg(all(not(feature = "island-compute"), not(feature = "island-compute-native")))] "compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } }, ``` The `island-compute-native` feature currently has no native app dep hooked up (`hero_cloud_app` is commented out in Cargo.toml). The `cfg(not(...))` guards are defensive for symmetry with other island arms. Dependencies: Step 2 (for the `ComputeApp` import). ### Acceptance Criteria - [ ] `cargo check -p hero_os_app --no-default-features --features web` succeeds — the `BooksApp` and `ComputeApp` imports and match arms compile. - [ ] `cargo check -p hero_os_app --no-default-features --features web-native` still succeeds (native arms remain valid). - [ ] In a running hero_os web build, opening the `books` island renders the `.books-island` wrapper from `BooksApp` rather than `.embed-container-plain` from `ExternalServiceIframe`. Same for `compute` — wrapper becomes `.compute-embed.embed-container`. - [ ] The iframe src for `compute` contains `hero_window_id=<id>` when a window_id is provided (`ComputeApp` uses `with_window_id`). - [ ] The `voice` island continues to work unchanged — it was already using `VoiceApp`, the issue body was out of date for that row. - [ ] No default-feature code path reaches `ExternalServiceIframe` for `books` or `compute`. ### Notes **Confirmed line numbers after drift check (current HEAD of hero_os, `crates/hero_os_app/src/island_content.rs`):** - books: line 367 — `"books" => rsx! { ExternalServiceIframe { src: "/hero_books/ui/", id: "books-iframe" } },` (gated by `#[cfg(not(feature = "island-books-native"))]` at line 366). Matches the issue. - compute: line 428 — `"compute" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } },` (ungated). Matches the issue. - voice: lines 375–384 — **already using `VoiceApp`** via the archipelago embed. The issue description is stale here; no change needed for voice. The `VoiceApp` re-export at line 49 is already in place. **Dependency status (Cargo.toml):** - `hero_archipelagos_books` — declared optional at line 296, branch `development`. No change needed. - `hero_archipelagos_compute` — declared optional at line 300, branch `development`. No change needed. - `hero_archipelagos_voice` — declared optional at line 303, branch `development`. Already in use. - Workspace root `Cargo.toml` does not re-declare archipelago crates, so no workspace change is needed. - Lines 128–130 of the hero_os workspace `Cargo.toml` contain a `[patch]` for `hero_archipelagos_voice` + `hero_archipelagos_core` pointing to `../hero_archipelagos/...`, marked "DO NOT COMMIT — injected at build time". This is an unintended leak and is **not in scope for this issue**, but the implementer should be aware it exists: when testing locally, the build will pick up the sibling checkout instead of the git-pinned version. Do NOT rely on this patch being present in CI. **hero_archipelagos embed APIs (verified):** - `hero_archipelagos_books::BooksApp` with `BooksAppProps { context: IslandContext }` — ready. - `hero_archipelagos_compute::ComputeApp` with `ComputeAppProps { context: IslandContext }` — ready. - `hero_archipelagos_voice::island::VoiceApp` with `VoiceAppProps { context: IslandContext }` — ready and already in use. **WASM readiness of voice crate:** the voice crate pulls in `wasm-bindgen`, `js-sys`, `web-sys` unconditionally. Listed as a hero_archipelagos workspace member. Already in use in hero_os — wasm32 builds are confirmed to work.
Author
Member

Test Results

Ran cargo check -p hero_os_app on branch development_fix_island_embeds with both relevant feature sets:

  • cargo check -p hero_os_app --no-default-features --features web — succeeded (13 warnings, all pre-existing, none related to this change).
  • cargo check -p hero_os_app --no-default-features --features web-native — succeeded (15 warnings, all pre-existing, none related to this change).

No new warnings or errors introduced by the books/compute swap. Voice was already in place on the branch and continues to compile.

Runtime verification (iframe src contains hero_window_id=<id>, .books-island / .compute-embed wrapper classes) to be validated during PR review against a running hero_os web build.

## Test Results Ran `cargo check -p hero_os_app` on branch `development_fix_island_embeds` with both relevant feature sets: - `cargo check -p hero_os_app --no-default-features --features web` — succeeded (13 warnings, all pre-existing, none related to this change). - `cargo check -p hero_os_app --no-default-features --features web-native` — succeeded (15 warnings, all pre-existing, none related to this change). No new warnings or errors introduced by the books/compute swap. Voice was already in place on the branch and continues to compile. Runtime verification (iframe `src` contains `hero_window_id=<id>`, `.books-island` / `.compute-embed` wrapper classes) to be validated during PR review against a running hero_os web build.
Author
Member

Pull request opened: #75

This PR implements the changes discussed in this issue.

Pull request opened: https://forge.ourworld.tf/lhumina_code/hero_os/pulls/75 This PR implements the changes discussed in this issue.
mahmoud added this to the ACTIVE project 2026-04-20 08:20:02 +00:00
mahmoud added this to the now milestone 2026-04-20 08:20:05 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_os#74
No description provided.