bug: some islands use ExternalServiceIframe instead of available archipelago embeds #74
Labels
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
lhumina_code/hero_os#74
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
Several islands have archipelago embed components available in
hero_archipelagosbutisland_content.rsuses hardcodedExternalServiceIframeinstead.Affected islands
ExternalServiceIframe { src: "/hero_books/ui/" }hero_archipelagos_booksExternalServiceIframe { src: "/hero_compute/ui/" }hero_archipelagos_computeExternalServiceIframehero_archipelagos_voice::island::VoiceAppWhy this matters
The archipelago embed components handle theme sync, auth token passing, and hero_router URL resolution correctly.
ExternalServiceIframeis a generic fallback that lacks per-island logic and has known issues withwindow_idnot being appended to the iframe src.Files
crates/hero_os_app/src/island_content.rs:367— bookscrates/hero_os_app/src/island_content.rs:428— computecrates/hero_os_app/src/island_content.rs:375-384— voiceImplementation Spec for Issue #74
Objective
Replace the two remaining
ExternalServiceIframefallbacks (books,compute) inisland_content.rswith the real archipelago embed components (BooksApp,ComputeApp). Thevoicearm cited in the issue is already wired correctly toVoiceApp— no change needed there. The archipelago embed components construct their iframe src viaIslandContext::with_window_id(...)which appendshero_window_id=<id>, fixing the missing-window_id bug thatExternalServiceIframeexhibits and using the per-island theme/auth sync logic rather than the generic fallback.Requirements
island-books/island-computefeatures are enabled, hero_os must renderBooksApp/ComputeApp(from the embed crates) instead ofExternalServiceIframe.compute, cargo features gate the switch:island-computeselects the embed path,island-compute-native(currently a dead code path) selects the native arm, otherwise the iframe fallback remains.books, the arm was already split onisland-books-native; the non-native arm must be further gated on featureisland-booksso the iframe fallback is reachable when the feature is off.island-booksmust be added to the defaultwebfeature list so the switch takes effect in the default web build (it is currently absent fromweb, likely because the iframe was used regardless).hero_archipelagos_books,hero_archipelagos_compute, andhero_archipelagos_voiceare already declared as optional git deps inhero_os_app/Cargo.tomlonbranch = "development". The workspaceCargo.tomldoes 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 importscrates/hero_os_app/Cargo.toml— addisland-booksto thewebfeature list (no dep changes)Implementation Plan
Step 1: Add
island-booksto thewebfeature inhero_os_app/Cargo.tomlFile:
crates/hero_os_app/Cargo.tomlRationale:
island-computeandisland-voiceare already listed;island-booksis 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 otherisland-*features already listed in thewebarray (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
BooksAppandComputeAppimports toisland_content.rsFile:
crates/hero_os_app/src/island_content.rsRationale: Follow the per-island
pub usepattern used for every other archipelago embed in this file (e.g.ProcApp,RedisApp). Placed near the otherembed/*re-exports, replacing the stale "Books UI is served as standalone iframe" comment.Old snippet (around lines 36–39):
New snippet:
hero_archipelagos_books::BooksAppandhero_archipelagos_compute::ComputeAppare both crate-root re-exports (verified in each crate'slib.rs). This matches howCallIslandApp/MessagingArchipelagoAppare 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
BooksAppFile:
crates/hero_os_app/src/island_content.rsRationale: Mirror the
proc/redis/voicethree-way arm pattern — embed crate whenisland-books, native app whenisland-books-native, iframe fallback when neither.Old snippet (around lines 365–371):
New snippet:
Dependencies: Step 2 (for the
BooksAppimport). Step 1 (so the defaultwebbuild enablesisland-booksand the embed arm is selected).The iframe-fallback arm is dropped on purpose: every practical build path goes through
web(→ enablesisland-books) orweb-native(→ enablesisland-books-native), so a third "neither feature" arm would be dead code.Step 4: Replace the compute iframe arm with a feature-gated
ComputeAppFile:
crates/hero_os_app/src/island_content.rsRationale: The current compute arm is unconditional — split it so the embed is selected when
island-computeis enabled andisland-compute-nativeisn't (that is the current default inweb). Keep the iframe fallback for the "neither" case.Old snippet (around line 427–428):
New snippet:
The
island-compute-nativefeature currently has no native app dep hooked up (hero_cloud_appis commented out in Cargo.toml). Thecfg(not(...))guards are defensive for symmetry with other island arms.Dependencies: Step 2 (for the
ComputeAppimport).Acceptance Criteria
cargo check -p hero_os_app --no-default-features --features websucceeds — theBooksAppandComputeAppimports and match arms compile.cargo check -p hero_os_app --no-default-features --features web-nativestill succeeds (native arms remain valid).booksisland renders the.books-islandwrapper fromBooksApprather than.embed-container-plainfromExternalServiceIframe. Same forcompute— wrapper becomes.compute-embed.embed-container.computecontainshero_window_id=<id>when a window_id is provided (ComputeAppuseswith_window_id).voiceisland continues to work unchanged — it was already usingVoiceApp, the issue body was out of date for that row.ExternalServiceIframeforbooksorcompute.Notes
Confirmed line numbers after drift check (current HEAD of hero_os,
crates/hero_os_app/src/island_content.rs):"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" => rsx! { ExternalServiceIframe { src: "/hero_compute/ui/", id: "compute-iframe" } },(ungated). Matches the issue.VoiceAppvia the archipelago embed. The issue description is stale here; no change needed for voice. TheVoiceAppre-export at line 49 is already in place.Dependency status (Cargo.toml):
hero_archipelagos_books— declared optional at line 296, branchdevelopment. No change needed.hero_archipelagos_compute— declared optional at line 300, branchdevelopment. No change needed.hero_archipelagos_voice— declared optional at line 303, branchdevelopment. Already in use.Cargo.tomldoes not re-declare archipelago crates, so no workspace change is needed.Cargo.tomlcontain a[patch]forhero_archipelagos_voice+hero_archipelagos_corepointing 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::BooksAppwithBooksAppProps { context: IslandContext }— ready.hero_archipelagos_compute::ComputeAppwithComputeAppProps { context: IslandContext }— ready.hero_archipelagos_voice::island::VoiceAppwithVoiceAppProps { context: IslandContext }— ready and already in use.WASM readiness of voice crate: the voice crate pulls in
wasm-bindgen,js-sys,web-sysunconditionally. Listed as a hero_archipelagos workspace member. Already in use in hero_os — wasm32 builds are confirmed to work.Test Results
Ran
cargo check -p hero_os_appon branchdevelopment_fix_island_embedswith 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
srccontainshero_window_id=<id>,.books-island/.compute-embedwrapper classes) to be validated during PR review against a running hero_os web build.Pull request opened: #75
This PR implements the changes discussed in this issue.