lhumina_code/hero_os
22
0
Fork 0
Code Issues 28 Pull requests 1 Projects 1 Releases Packages Wiki Activity Actions

Cleanup: dead code, competing impls, and refactor debris audit #36

New issue
Closed
opened 2026-04-14 17:33:22 +00:00 by timur · 4 comments
timur commented 2026-04-14 17:33:22 +00:00
Owner
Copy link

Audit of the hero_os codebase for cruft accumulated from parallel AI-assisted development. Grouped by priority. Use this issue as a punch list — will work on it incrementally.

Dead code (low-risk removal)

Leftovers from the hero_auth removal (commits 2c98634 → 6354b25, Apr 14):

  • crates/hero_os_app/src/storage.rs:172-197 — save_jwt/load_jwt/clear_jwt for hero_auth_token (kept "in case hero_proxy brings it back")
  • crates/hero_os_app/src/services/env_service.rs:105-108 — http_get() still reads hero_auth_token as Bearer, but config::auth_url() is gone, so the path is unreachable
  • crates/hero_os_web/src/components/island_frame.rs:30 — hardcoded route "auth" => "/hero_auth/ui/"
  • crates/hero_os_app/src/generated_profile.rs — COMPILED_ISLANDS still lists "auth"; drop from config.toml [profiles.default]
  • crates/hero_os_app/src/components/login_screen.rs (654 LOC) — on_auth callback never fires; decide: delete or gate behind feature flag
  • crates/hero_os_app/src/components/setup_wizard.rs (312 LOC) — same as above
  • crates/hero_os_app/src/storage.rs:115 — clear_desktop_state() marked dead, unused

Other dead-code hygiene:

  • Remove crate-wide #![allow(dead_code)] at crates/hero_os_app/src/main.rs:6 — it masks everything. Fix real warnings instead.
  • crates/hero_os_client/ — empty stub directory, never committed (only target/ + schema). Delete or populate.
  • Duplicate // TODO: Navigate to section view within focused app stubs at main.rs:1666 and main.rs:1698 — unreachable, from incomplete Phase 5 deeplink work.

Competing / unfinished implementations (deliberate before touching)

  • Two WindowState types: OSchema-generated canonical in crates/hero_os/src/desktop/types_generated.rs + app-local extension in crates/hero_os_app/src/islands.rs:25-172 adding route/route_stack/route_forward/nav_stack. Intentional split (ephemeral UI state) but unobvious — either unify or document the split.
  • Two routing layers: crates/hero_os_app/src/routing.rs (URL→context/apps, 323 LOC) and the new crates/hero_os_app/src/window_route.rs (island→header, 166 LOC, from 49bde9a). Phase 5 is supposed to unify them. Confirm plan is tracked.
  • hero_os_ui crate mid-deprecation — 710e98d dropped 255 LOC of RPC proxying now owned by hero_os_server. Inventory what's left and finish the deprecation.

Architectural debt / workarounds

  • 65+ island feature flags in Cargo.toml. fff3609 ("drop unbuildable islands") proves some combos don't build. Add a feature-matrix CI check or prune unused flags.
  • Socket strategy churn — 6+ follow-up commits Jan–Apr fixing path bugs after 557a06d. Confirm stabilized; document the final convention.
  • Guest-only boot with no proxy — auth is pulled, but hero_proxy auth injection (home#118) isn't landed. Current state is auth-neutral in a way nothing downstream handles. Track the proxy integration.
  • Tight sibling coupling (exposed by 1df8806, Apr 14, +3544 Cargo.lock) — 20 sibling-repo path deps became git deps to make fresh checkouts build. Revisit after sibling repos stabilize.

Generated artifacts — regeneration story unclear

  • types_generated.rs, types_wasm_generated.rs, osis_*_generated.rs, osis_client_generated.rs — no visible build.rs hooks. Regeneration appears manual. Document the command(s) and add a make regen target or build.rs. Risk of drift.
  • generated_profile.rs is built from config.toml via build.rs — fine as-is, just audit the COMPILED_ISLANDS list.

Suspicious large commits (reference)

  • 1df8806 (Apr 14, +3544) — path deps → git deps
  • f6024f1 (Apr 12, +1082/−3451) — "sessions 17-18", 18 new island crates + socket migration + OSIS auth fix; grab-bag
  • f393c8f (Mar 17) — cleaned up 14 non-existent _ui_wasm path deps + duplicate functions (confirms speculative AI-generated additions)

Suggested order of attack

  1. Auth orphan sweep (section 1) — trivial, safe.
  2. Decide LoginScreen/SetupWizard fate — don't leave "kept for future."
  3. Remove crate-wide allow(dead_code), fix resulting warnings.
  4. Deliberate on items in section 2 before any refactor.
  5. Structural items (feature matrix, regen story) as separate follow-up issues.
Audit of the hero_os codebase for cruft accumulated from parallel AI-assisted development. Grouped by priority. Use this issue as a punch list — will work on it incrementally. ## Dead code (low-risk removal) Leftovers from the hero_auth removal (commits `2c98634` → `6354b25`, Apr 14): - [ ] `crates/hero_os_app/src/storage.rs:172-197` — `save_jwt/load_jwt/clear_jwt` for `hero_auth_token` (kept "in case hero_proxy brings it back") - [ ] `crates/hero_os_app/src/services/env_service.rs:105-108` — `http_get()` still reads `hero_auth_token` as Bearer, but `config::auth_url()` is gone, so the path is unreachable - [ ] `crates/hero_os_web/src/components/island_frame.rs:30` — hardcoded route `"auth" => "/hero_auth/ui/"` - [ ] `crates/hero_os_app/src/generated_profile.rs` — `COMPILED_ISLANDS` still lists `"auth"`; drop from `config.toml` `[profiles.default]` - [ ] `crates/hero_os_app/src/components/login_screen.rs` (654 LOC) — `on_auth` callback never fires; decide: delete or gate behind feature flag - [ ] `crates/hero_os_app/src/components/setup_wizard.rs` (312 LOC) — same as above - [ ] `crates/hero_os_app/src/storage.rs:115` — `clear_desktop_state()` marked dead, unused Other dead-code hygiene: - [ ] Remove crate-wide `#![allow(dead_code)]` at `crates/hero_os_app/src/main.rs:6` — it masks everything. Fix real warnings instead. - [ ] `crates/hero_os_client/` — empty stub directory, never committed (only `target/` + schema). Delete or populate. - [ ] Duplicate `// TODO: Navigate to section view within focused app` stubs at `main.rs:1666` and `main.rs:1698` — unreachable, from incomplete Phase 5 deeplink work. ## Competing / unfinished implementations (deliberate before touching) - [ ] **Two `WindowState` types**: OSchema-generated canonical in `crates/hero_os/src/desktop/types_generated.rs` + app-local extension in `crates/hero_os_app/src/islands.rs:25-172` adding `route/route_stack/route_forward/nav_stack`. Intentional split (ephemeral UI state) but unobvious — either unify or document the split. - [ ] **Two routing layers**: `crates/hero_os_app/src/routing.rs` (URL→context/apps, 323 LOC) and the new `crates/hero_os_app/src/window_route.rs` (island→header, 166 LOC, from `49bde9a`). Phase 5 is supposed to unify them. Confirm plan is tracked. - [ ] **`hero_os_ui` crate** mid-deprecation — `710e98d` dropped 255 LOC of RPC proxying now owned by `hero_os_server`. Inventory what's left and finish the deprecation. ## Architectural debt / workarounds - [ ] **65+ island feature flags** in `Cargo.toml`. `fff3609` ("drop unbuildable islands") proves some combos don't build. Add a feature-matrix CI check or prune unused flags. - [ ] **Socket strategy churn** — 6+ follow-up commits Jan–Apr fixing path bugs after `557a06d`. Confirm stabilized; document the final convention. - [ ] **Guest-only boot with no proxy** — auth is pulled, but `hero_proxy` auth injection (home#118) isn't landed. Current state is auth-neutral in a way nothing downstream handles. Track the proxy integration. - [ ] **Tight sibling coupling** (exposed by `1df8806`, Apr 14, +3544 Cargo.lock) — 20 sibling-repo path deps became git deps to make fresh checkouts build. Revisit after sibling repos stabilize. ## Generated artifacts — regeneration story unclear - [ ] `types_generated.rs`, `types_wasm_generated.rs`, `osis_*_generated.rs`, `osis_client_generated.rs` — no visible `build.rs` hooks. Regeneration appears manual. Document the command(s) and add a `make regen` target or build.rs. Risk of drift. - [ ] `generated_profile.rs` is built from `config.toml` via `build.rs` — fine as-is, just audit the `COMPILED_ISLANDS` list. ## Suspicious large commits (reference) - `1df8806` (Apr 14, +3544) — path deps → git deps - `f6024f1` (Apr 12, +1082/−3451) — "sessions 17-18", 18 new island crates + socket migration + OSIS auth fix; grab-bag - `f393c8f` (Mar 17) — cleaned up 14 non-existent `_ui_wasm` path deps + duplicate functions (confirms speculative AI-generated additions) ## Suggested order of attack 1. Auth orphan sweep (section 1) — trivial, safe. 2. Decide LoginScreen/SetupWizard fate — don't leave "kept for future." 3. Remove crate-wide `allow(dead_code)`, fix resulting warnings. 4. Deliberate on items in section 2 before any refactor. 5. Structural items (feature matrix, regen story) as separate follow-up issues.
timur commented 2026-04-15 10:27:07 +00:00
Author
Owner
Copy link

Update after #35 merged

Issue #35 (unified window chrome, phases 1–5 + cleanup: 49bde9a, 58eba9b, ba3643c, dff7ef9, fdd7286, 161224e, 95a3e03, 94cec21) and concurrent merges (ea295e4, 382d2bf, fe880f2) resolved or changed several audit items. Re-scoping.

Resolved by #35 / concurrent merges

  • ✅ Two routing layers — crates/hero_os_app/src/window_route.rs is deleted (ba3643c relocated the types to hero_archipelagos_core). Phase 5 (fdd7286) wired WindowRoute.url_segment into the existing path-based URL writer as a per-app route query param. The two systems are now unified. Drop from the audit.
  • ✅ Two WindowState types — ba3643c removed the route/route_stack/route_forward fields from the app-local WindowState; the WindowRouteContext signal is now the single source of truth. The residual WindowState in islands.rs is a thin per-window holder, no longer overlapping with generated types. Drop from the audit.
  • ✅ Hardcoded contacts | filesystem headerless list at main.rs:1973 — removed by 58eba9b along with the entire dual-header branch in window.rs.
  • ✅ Unused native island deps — ea295e4 dropped hero_biz_app, hero_browser_app, hero_indexer_app crate deps and their island-*-native features. Partial progress on the feature-flag sprawl item.

Re-verified still present

  • ⚠ #![allow(dead_code)] at crates/hero_os_app/src/main.rs:6 — still there.
  • ⚠ "auth" still listed in generated_profile.rs:67 COMPILED_ISLANDS (source: config.toml [profiles.default]).
  • ⚠ "auth" => Some("/hero_auth/ui/".to_string()) still in crates/hero_os_web/src/components/island_frame.rs:30.
  • ⚠ env_service.rs:105 Bearer-token path still there and unreachable (config::auth_url() is gone).
  • ⚠ save_jwt / load_jwt in storage.rs:175,188 — confirmed dead (no callers). clear_jwt at storage.rs:202 is NOT dead — still called from the logout handler at main.rs:1434. Revised item: remove only save_jwt/load_jwt, keep clear_jwt.
  • ⚠ clear_desktop_state at storage.rs:116 — still dead.
  • ⚠ LoginScreen (654 LOC) and SetupWizard (312 LOC) — still present; on_auth callback still never fires. Logout handler at main.rs:1427-1444 now falls back to the in-tree login screen in guest mode — so the login screen UI path is reachable again, but its auth callback remains stubbed. Needs a product decision.
  • ⚠ crates/hero_os_client/ — still an empty stub (only target/).

New cleanup candidates introduced by recent merges

  • 382d2bf disabled island-intelligence and island-knowledge (hero_osis dropped the embedder domain); fe880f2 further disabled a broken island-knowledge build. Check that no orphaned references remain in Cargo.toml, config.toml profiles, island_content.rs conditional renders, and the web island_frame routes.
  • generated_profile.rs is build-artifact noise on checkout (first line is a developer-local absolute path, e.g. /Users/mahmoud/... vs /Users/timurgordon/...). Either strip the path from the header or .gitignore the file entirely — right now every developer's commits have a spurious diff on this file.

Starting work now

Beginning with the auth-orphan sweep (checkboxes above under Re-verified still present, excluding the LoginScreen/SetupWizard decision which needs product input). Will also delete hero_os_client/ and drop "auth" from the default profile. PR to follow.

## Update after #35 merged Issue #35 (unified window chrome, phases 1–5 + cleanup: `49bde9a`, `58eba9b`, `ba3643c`, `dff7ef9`, `fdd7286`, `161224e`, `95a3e03`, `94cec21`) and concurrent merges (`ea295e4`, `382d2bf`, `fe880f2`) resolved or changed several audit items. Re-scoping. ### Resolved by #35 / concurrent merges - ✅ **Two routing layers** — `crates/hero_os_app/src/window_route.rs` is **deleted** (`ba3643c` relocated the types to `hero_archipelagos_core`). Phase 5 (`fdd7286`) wired `WindowRoute.url_segment` into the existing path-based URL writer as a per-app `route` query param. The two systems are now unified. Drop from the audit. - ✅ **Two `WindowState` types** — `ba3643c` removed the `route/route_stack/route_forward` fields from the app-local `WindowState`; the `WindowRouteContext` signal is now the single source of truth. The residual `WindowState` in `islands.rs` is a thin per-window holder, no longer overlapping with generated types. Drop from the audit. - ✅ **Hardcoded `contacts | filesystem` headerless list** at `main.rs:1973` — removed by `58eba9b` along with the entire dual-header branch in `window.rs`. - ✅ **Unused native island deps** — `ea295e4` dropped `hero_biz_app`, `hero_browser_app`, `hero_indexer_app` crate deps and their `island-*-native` features. Partial progress on the feature-flag sprawl item. ### Re-verified still present - ⚠ `#![allow(dead_code)]` at `crates/hero_os_app/src/main.rs:6` — still there. - ⚠ `"auth"` still listed in `generated_profile.rs:67` `COMPILED_ISLANDS` (source: `config.toml` `[profiles.default]`). - ⚠ `"auth" => Some("/hero_auth/ui/".to_string())` still in `crates/hero_os_web/src/components/island_frame.rs:30`. - ⚠ `env_service.rs:105` Bearer-token path still there and unreachable (`config::auth_url()` is gone). - ⚠ `save_jwt` / `load_jwt` in `storage.rs:175,188` — **confirmed dead** (no callers). `clear_jwt` at `storage.rs:202` is **NOT dead** — still called from the logout handler at `main.rs:1434`. Revised item: remove only `save_jwt`/`load_jwt`, keep `clear_jwt`. - ⚠ `clear_desktop_state` at `storage.rs:116` — still dead. - ⚠ `LoginScreen` (654 LOC) and `SetupWizard` (312 LOC) — still present; `on_auth` callback still never fires. Logout handler at `main.rs:1427-1444` now falls back to the in-tree login screen in guest mode — so the login screen UI path is reachable again, but its auth callback remains stubbed. Needs a product decision. - ⚠ `crates/hero_os_client/` — still an empty stub (only `target/`). ### New cleanup candidates introduced by recent merges - `382d2bf` disabled `island-intelligence` and `island-knowledge` (hero_osis dropped the embedder domain); `fe880f2` further disabled a broken `island-knowledge` build. Check that no orphaned references remain in Cargo.toml, `config.toml` profiles, island_content.rs conditional renders, and the web island_frame routes. - `generated_profile.rs` is build-artifact noise on checkout (first line is a developer-local absolute path, e.g. `/Users/mahmoud/...` vs `/Users/timurgordon/...`). Either strip the path from the header or `.gitignore` the file entirely — right now every developer's commits have a spurious diff on this file. ### Starting work now Beginning with the auth-orphan sweep (checkboxes above under *Re-verified still present*, excluding the LoginScreen/SetupWizard decision which needs product input). Will also delete `hero_os_client/` and drop `"auth"` from the default profile. PR to follow.
timur commented 2026-04-15 10:48:00 +00:00
Author
Owner
Copy link

Slice 1 landed via #37 (squash-merged into development). Checked off:

  • ✅ "auth" removed from config.toml profiles
  • ✅ "auth" => "/hero_auth/ui/" route removed in island_frame.rs
  • ✅ Unreachable Bearer-token path removed in env_service.rs
  • ✅ Dead save_jwt / load_jwt / clear_desktop_state removed in storage.rs (kept clear_jwt — still called by logout handler)
  • ✅ Empty crates/hero_os_client/ stub deleted

Still open (see original body for full punch list):

  • LoginScreen (654 LOC) / SetupWizard (312 LOC) — product decision needed
  • Crate-wide #![allow(dead_code)] at main.rs:6
  • generated_profile.rs developer-path noise (build.rs header fix)
  • 65+ island feature flags audit / matrix
  • Socket strategy stabilization confirmation
  • Generated *_generated.rs regeneration story (OSchema/OpenRPC)
  • hero_proxy auth injection landing (home#118)
  • Orphan check for disabled island-intelligence / island-knowledge refs

Pre-existing issue spotted (unrelated, should be its own ticket): cargo test --workspace --no-run fails with unresolved import super::server in crates/hero_os_server/src/desktop/tests.rs:12. Present on development prior to #37.

Slice 1 landed via #37 (squash-merged into `development`). Checked off: - ✅ `"auth"` removed from `config.toml` profiles - ✅ `"auth" => "/hero_auth/ui/"` route removed in `island_frame.rs` - ✅ Unreachable Bearer-token path removed in `env_service.rs` - ✅ Dead `save_jwt` / `load_jwt` / `clear_desktop_state` removed in `storage.rs` (kept `clear_jwt` — still called by logout handler) - ✅ Empty `crates/hero_os_client/` stub deleted Still open (see original body for full punch list): - `LoginScreen` (654 LOC) / `SetupWizard` (312 LOC) — product decision needed - Crate-wide `#![allow(dead_code)]` at `main.rs:6` - `generated_profile.rs` developer-path noise (build.rs header fix) - 65+ island feature flags audit / matrix - Socket strategy stabilization confirmation - Generated `*_generated.rs` regeneration story (OSchema/OpenRPC) - hero_proxy auth injection landing (home#118) - Orphan check for disabled `island-intelligence` / `island-knowledge` refs **Pre-existing issue spotted (unrelated, should be its own ticket):** `cargo test --workspace --no-run` fails with `unresolved import super::server` in `crates/hero_os_server/src/desktop/tests.rs:12`. Present on `development` prior to #37.
timur commented 2026-04-15 11:53:33 +00:00
Author
Owner
Copy link

Fixed the cargo test --workspace blocker via #38 (merged): crates/hero_os_server/src/desktop/tests.rs:12 imported super::server::* (no such module); corrected to use super::*;. All 3 desktop CRUD tests now pass.

Remaining cargo test --workspace failures are in crates/hero_os_examples/tests/integration.rs and require hero_proc running with binaries in ~/hero/bin/ (documented at file top). Not a regression — environment-gated, expected.

Fixed the `cargo test --workspace` blocker via #38 (merged): `crates/hero_os_server/src/desktop/tests.rs:12` imported `super::server::*` (no such module); corrected to `use super::*;`. All 3 desktop CRUD tests now pass. Remaining `cargo test --workspace` failures are in `crates/hero_os_examples/tests/integration.rs` and require `hero_proc` running with binaries in `~/hero/bin/` (documented at file top). Not a regression — environment-gated, expected.
timur commented 2026-04-15 15:33:20 +00:00
Author
Owner
Copy link

Closing — enough ground covered for one pass

Shipped today via #37, #38, #54, #55, #64, #65:

  • Auth orphans swept (config.toml, island_frame, env_service, storage, hero_os_client stub)
  • desktop/tests.rs compile fix (super::server → super::*)
  • LoginScreen + SetupWizard gated behind legacy-auth-ui feature (off by default)
  • Orphan island-intelligence / island-knowledge / island-indexer-native refs removed
  • generated_profile.rs header path stabilized (no more per-developer diff)
  • Crate-wide #![allow(dead_code)] removed + -1712 LOC of real dead code swept (commands.rs, dom_automation.rs, Groq client, MCP schema types, scattered unused helpers)

Net: hero_os_app warning count 22 → 13, zero dead_code warnings.

Deferred to follow-up issues as needed

  • Feature-flag matrix / CI coverage for 60+ island flags
  • Socket strategy stabilization confirmation
  • Generated-code (*_generated.rs) regeneration story — no build.rs hooks for OSchema/OpenRPC artifacts
  • Remaining unused_variables / unexpected_cfgs warnings (smaller scope)
  • hero_os_ui crate deprecation finish
  • hero_proxy auth integration (home#118)

Closing — open a new focused issue for any of the above as they come up.

## Closing — enough ground covered for one pass Shipped today via #37, #38, #54, #55, #64, #65: - Auth orphans swept (config.toml, island_frame, env_service, storage, hero_os_client stub) - `desktop/tests.rs` compile fix (`super::server` → `super::*`) - `LoginScreen` + `SetupWizard` gated behind `legacy-auth-ui` feature (off by default) - Orphan `island-intelligence` / `island-knowledge` / `island-indexer-native` refs removed - `generated_profile.rs` header path stabilized (no more per-developer diff) - Crate-wide `#![allow(dead_code)]` removed + **-1712 LOC** of real dead code swept (commands.rs, dom_automation.rs, Groq client, MCP schema types, scattered unused helpers) **Net:** `hero_os_app` warning count 22 → 13, zero `dead_code` warnings. ## Deferred to follow-up issues as needed - Feature-flag matrix / CI coverage for 60+ island flags - Socket strategy stabilization confirmation - Generated-code (`*_generated.rs`) regeneration story — no build.rs hooks for OSchema/OpenRPC artifacts - Remaining `unused_variables` / `unexpected_cfgs` warnings (smaller scope) - `hero_os_ui` crate deprecation finish - hero_proxy auth integration (home#118) Closing — open a new focused issue for any of the above as they come up.
timur closed this issue 2026-04-15 15:33:21 +00:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
development
35-unified-window-chrome
development_auth_fixes
development_fixes_v2
development_fixes
development_30
development_16
development_mik2
main
v0.1.3
v0.1.2
v0.1.1
v0.1.0
v0.0.1-rc1
Labels
Clear labels   
prio_critical

   
prio_low

   
type_bug

   
type_contact

   
type_issue

   
type_lead

   
type_question

   
type_story

   
type_task
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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#36
No description provided.