fix(shell): render placeholder window for unknown island ids instead of silent URL rewrite #122

Merged

rawdaGastan merged 1 commit from development_show_unknown_island_placeholder into development

2026-04-30 14:44:42 +00:00

rawdaGastan commented

2026-04-30 14:40:42 +00:00

Member

Summary

When a URL referenced an island id not in the runtime registry (e.g. a feature-gated island not compiled in this build), both the first-load URL effect and the popstate listener silently dropped it from the windows list. The window→URL sync effect then rewrote the address bar to match the (now-missing) window — producing the silent rewrite the issue reports. Users (and agents) following a stale or wrong URL got the impression nothing happened.

This change pushes a WindowState for the unknown id verbatim, so the URL stays put. The existing IslandContent fallback arm now renders an explicit Island "<id>" not installed placeholder (replacing the previous DynamicIslandLoader call, which produced a misleading "Failed to load island: …" after a delay).

Closes #109

Changes

crates/hero_os_app/src/main.rs — both the first-load loop and the popstate loop now push a placeholder WindowState (id verbatim, title = id, default 600x400) when reg.get(app_id) is None. The tracing::warn! is preserved alongside the new visual.
crates/hero_os_app/src/island_content.rs — the web-platform other arm now renders a static island-placeholder div with Island "<id>" not installed plus a clarifying detail line. The not(web-platform) arm copy is updated to match for consistency.
crates/hero_os_app/src/dynamic_loader.rs — module-level #![allow(dead_code)] since DynamicIslandLoader is no longer wired into any caller. Module preserved per the spec for future runtime island distribution.
crates/hero_os_app/src/routing.rs — two new pure-parser tests pin the contract that parse_url is registry-agnostic and keeps unknown ids in route.apps.

Test Results

cargo check -p hero_os_app — clean.
make check (full workspace) — clean, 25.18s.
cargo test -p hero_os_app — 42 passed, 0 failed (includes the two new parser tests).
End-to-end browser smoke via Hero Browser MCP:
- Navigated to /space/default/nonexistent_island_xyz → URL preserved verbatim, window opened with title nonexistent_island_xyz, body renders Island "nonexistent_island_xyz" not installed followed by This island is not part of the current Hero OS build.
- Browser back → returns to previous island (Conference, real island).
- Browser forward (popstate) → placeholder window renders again, URL preserved.
- Existing islands (messaging, Conference) load normally.

Note on the issue's original repro id

The issue body uses room as the unknown-id example. Between the issue being filed and this fix, room (Conference) is now compiled in by default on development head, so it's no longer a useful repro id. The fix was verified with nonexistent_island_xyz instead — the underlying behaviour is identical for any id not in the runtime registry.

Caveats

The previously-installed window state can persist in localStorage for the unknown-id placeholder window. If the user closes that window, normal close handling cleans it up. If they refresh while it's open, it's restored from localStorage — desirable, since the URL still references it.
dynamic_loader.rs is now dead at the crate level. Left in place per the spec; can be removed in a follow-up if/when the team decides to drop the runtime-load story entirely.

## Summary When a URL referenced an island id not in the runtime registry (e.g. a feature-gated island not compiled in this build), both the first-load URL effect and the popstate listener silently dropped it from the windows list. The window→URL sync effect then rewrote the address bar to match the (now-missing) window — producing the silent rewrite the issue reports. Users (and agents) following a stale or wrong URL got the impression nothing happened. This change pushes a `WindowState` for the unknown id verbatim, so the URL stays put. The existing `IslandContent` fallback arm now renders an explicit `Island "<id>" not installed` placeholder (replacing the previous `DynamicIslandLoader` call, which produced a misleading "Failed to load island: …" after a delay). ## Related Issue Closes https://forge.ourworld.tf/lhumina_code/hero_os/issues/109 ## Changes - `crates/hero_os_app/src/main.rs` — both the first-load loop and the popstate loop now push a placeholder `WindowState` (id verbatim, title = id, default 600x400) when `reg.get(app_id)` is `None`. The `tracing::warn!` is preserved alongside the new visual. - `crates/hero_os_app/src/island_content.rs` — the `web-platform` `other` arm now renders a static `island-placeholder` div with `Island "<id>" not installed` plus a clarifying detail line. The `not(web-platform)` arm copy is updated to match for consistency. - `crates/hero_os_app/src/dynamic_loader.rs` — module-level `#![allow(dead_code)]` since `DynamicIslandLoader` is no longer wired into any caller. Module preserved per the spec for future runtime island distribution. - `crates/hero_os_app/src/routing.rs` — two new pure-parser tests pin the contract that `parse_url` is registry-agnostic and keeps unknown ids in `route.apps`. ## Test Results - `cargo check -p hero_os_app` — clean. - `make check` (full workspace) — clean, 25.18s. - `cargo test -p hero_os_app` — 42 passed, 0 failed (includes the two new parser tests). - End-to-end browser smoke via Hero Browser MCP: - Navigated to `/space/default/nonexistent_island_xyz` → URL preserved verbatim, window opened with title `nonexistent_island_xyz`, body renders `Island "nonexistent_island_xyz" not installed` followed by `This island is not part of the current Hero OS build.` - Browser back → returns to previous island (Conference, real island). - Browser forward (popstate) → placeholder window renders again, URL preserved. - Existing islands (messaging, Conference) load normally. ## Note on the issue's original repro id The issue body uses `room` as the unknown-id example. Between the issue being filed and this fix, `room` (Conference) is now compiled in by default on `development` head, so it's no longer a useful repro id. The fix was verified with `nonexistent_island_xyz` instead — the underlying behaviour is identical for any id not in the runtime registry. ## Caveats - The previously-installed window state can persist in `localStorage` for the unknown-id placeholder window. If the user closes that window, normal close handling cleans it up. If they refresh while it's open, it's restored from localStorage — desirable, since the URL still references it. - `dynamic_loader.rs` is now dead at the crate level. Left in place per the spec; can be removed in a follow-up if/when the team decides to drop the runtime-load story entirely.

rawdaGastan added 1 commit

2026-04-30 14:40:42 +00:00

fix(shell): render placeholder window for unknown island ids instead of silent URL rewrite

Build and Test / build (pull_request) Successful in 2m55s

Details

e9c8d4e951

The first-load and popstate effects in main.rs both used to silently
drop unknown island ids: `if let Some(meta) = reg.get(app_id) { push }
else { warn }`. Combined with the window->URL sync effect (which
rewrites the URL to match the actual windows list), navigating to an
unknown id silently rewrote the URL back to the previously open
island. The user got the impression nothing happened.

Push a `WindowState` for the unknown id verbatim so:
- the URL stays at what the user typed,
- the IslandContent `_` arm can render a "not installed" placeholder.

Replace the `web-platform` `other` arm in island_content.rs (which
previously delegated to DynamicIslandLoader and produced a misleading
"Failed to load" after a delay) with an explicit `Island "<id>" not
installed` placeholder div. Mark dynamic_loader.rs `dead_code` since
nothing wires it in any more — kept around per the spec for future
runtime island distribution.

Add two pure-parser tests pinning the contract that `parse_url` is
registry-agnostic and keeps unknown ids in `route.apps`.

#109