Polish sample apps for SDL3: Photo Viewer, Music Player, Notepad

[AndrewAltimit]

Summary

Ships real sample media, wires File Manager -> app dispatch, gives Photo Viewer / Music Player / Text Editor proper GUIs, and adds a reusable Win95-style drop-down menu widget. Retires the Clock app.

• Sample data shipped in the binary (ambient_dawn.mp3, oasis_sample.png, welcome.txt, notes.txt under /home/user/{music,photos,documents}/) so every fresh VFS has working content without fetch-samples.sh.
• File Manager hand-off: new AppAction::LaunchAppWithFile + extension map — Confirm on a .png opens Photo Viewer, .mp3 opens Music Player with real audio, .txt opens Text Editor with the content pre-loaded.
• Photo Viewer actually renders images: png + jpeg-decoder + auto-downscaling, blitted via load_texture/blit with aspect-fit + zoom and a filename/dimensions footer.
• Music Player PSP-style GUI: album-art tile, ID3-parsed track title, duration from MP3 frame bitrate, 5-button transport, progress bar. New media_controller drives the AudioBackend via VFS IPC. Stops on window close (bug fix — it kept playing before).
• Text Editor Windows-Notepad GUI in both render paths (draw_windowed for WM skins, render_notepad_sdi for classic/fullscreen). Title bar, File/Edit/View/Help menu bar, white text area with current-line highlight + blue caret, status bar with Ln N, Col M. Keyboard typing + backspace + click-to-position all wired through the App trait's handle_text_input / handle_backspace / handle_click hooks.
• Reusable oasis_ui::menu_bar widget — Windows 95 bezel, hover highlight, right-aligned shortcut hints. Text Editor uses it; any other app can drop in its own menu with MenuBar::new(vec![...]).
• Dashboard cleanup: dropped 4 CLI-placeholder tiles (Notepad plugin, Network, Package Manager, System Monitor — all static text dumps that duplicated GUI apps or terminal commands). Retired the Clock app (redundant with the status bar). 17 -> 12 tiles, all GUI-first.

Test plan

• cargo test --workspace — 7175 tests, 0 failures
• cargo clippy --workspace -- -D warnings — clean
• cargo fmt --all --check — clean
• cargo build --release -p oasis-app — boots, 12 dashboard apps, audio subsystem up, disk samples loaded on top of bundled
• File Manager -> Confirm on .png / .mp3 / .txt dispatches to the right app (unit + integration tests)
• Music stops when the window X is clicked
• Notepad GUI renders in both windowed (WM skins) and fullscreen (classic skin) paths
• Text Editor menu bar opens drop-downs, File -> Save emits VFS write, File -> Exit closes window, clicks elsewhere close the menu
• Manual: type in the editor, click in the text area to reposition the caret, exercise each menu item

Generated with Claude Code

[github-actions]

Claude AI Architecture & Quality Review

Issues (if any)

• [WARNING] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 - Hardcoded magic numbers duplicate private constants from a different crate

  • runner_sdi.rs uses literal 4, 16, 64 for loop bounds on np_menu_*, np_dd_*, and np_line_* objects; the authoritative values live in oasis-app-text-editor/src/render.rs as menu_label_count() = 4, NP_MAX_DROPDOWN_ENTRIES = 16, NP_MAX_VISIBLE_LINES = 64
  • Cross-crate private constants can't be imported directly, so the fix is to expose them as pub const (or pub fns) from oasis-app-text-editor and re-export from oasis-core, or encapsulate the cleanup behind a public TextEditorApp::hide_all_sdi_objects(sdi) static method — the hide_notepad_sdi already exists on TextEditorApp, but runner_sdi.rs reimplements the same list instead of calling it via the delegate

• [WARNING] crates/oasis-app-text-editor/src/render.rs (dropdown_size fn) - Drop-down layout constants duplicated across crate boundary

  • dropdown_size() in render.rs hardcodes 20px/item, 6px/sep, 22px pad which must stay in sync with ITEM_H, SEPARATOR_H, ITEM_PAD_X in menu_bar.rs; divergence causes the SDI-path hit boxes to misalign from the rendered drop-down geometry
  • Expose MenuBar::dropdown_dimensions(menu: &Menu) -> (u32, u32) as a pub function in oasis-ui and call it from render.rs

• [WARNING] crates/oasis-app-media/src/photo_ui.rs:42-45 - Texture uploaded and destroyed on every frame with no backend contract guaranteeing deduplication

  • load_texture + destroy_texture is called each draw_windowed call; the comment says backends "dedupe redundant uploads internally" but SdiCore makes no such guarantee — this is an implicit contract between the photo viewer and each backend implementation
  • Cache the texture handle in BrowsingApp alongside decoded_image; invalidate on open_file

• [BUG] crates/oasis-backend-wasm/src/input_dispatch.rs - LaunchAppWithFile silently dropped in WASM windowed (multi-window) path

  • The open_runners branch matches AppAction::LaunchAppWithFile { .. } and does nothing (comment says "Not yet wired"); the fullscreen branch below correctly handles it — so on WASM, File Manager's Confirm on a typed file silently does nothing in windowed mode
  • Either wire it (mirror the fullscreen branch) or return AppAction::None from app_for_file on the WASM build when windowed multi-window mode is active

Previous Issues (if any)

(none — first review of this branch)

Suggestions (if any)

• crates/oasis-app-text-editor/src/lib.rs (default_menu_bar / menu_label_count) - menu_label_count() returns a hardcoded 4; it is used by hide_notepad_sdi to bound the np_menu_* cleanup loop. Adding a 5th top-level menu without updating this constant silently leaks a np_menu_4 SDI object. Derive it from self.menu.menus.len() at hide time instead.

• crates/oasis-app-media/src/music_ui.rs:130 - Progress bar is static at 10%; the BrowsingApp already has a slideshow_timer field — consider incrementing a frame counter there and animating the bar width as (frame % bar_w) to signal that audio is playing, without needing a backend handle.

Notes

• AppAction losing Copy (now only Clone) is a deliberate, correct change required by the String payloads. The simple_app.rs call site was the only one needing .clone() — no other silent regressions observed in the diff.
• Clock app removal is clean: crate deleted, workspace removed, registry entry gone, Cargo.lock updated.
• The IPC pattern (/var/audio/request) mirrors radio_controller cleanly; the "leave unknown payloads" comment is slightly misleading (no other controller shares that path) but the guard is harmless.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI General Review

Issues (if any)

• [BUG] crates/oasis-app-text-editor/src/lib.rs:243 - handle_click uses hardcoded title_h = 20 but draw_notepad uses at.app.title_bar_height.max(18)

  • If the active theme's title_bar_height differs from 20px, the click routing boundaries (area_top, area_bottom) will misalign with the visually rendered title/menu bars, causing clicks to land in the wrong region or be ignored.
  • Fix: Pass the actual title_bar_height from the theme into handle_click, or compute it consistently with draw_notepad.

• [BUG] crates/oasis-backend-wasm/src/input_dispatch.rs:458 - LaunchAppWithFile silently swallowed in WASM windowed (multi-window) path

  • The open_runners branch matches AppAction::LaunchAppWithFile { .. } and does nothing (comment says "Not yet wired"). The fullscreen branch below correctly handles it. On WASM desktop, confirming a typed file in File Manager silently fails to launch the target app.
  • Fix: Mirror the fullscreen dispatch logic, or return AppAction::None from app_for_file on WASM when windowed mode is active to avoid misleading the user.

• [WARNING] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 - Hardcoded loop bounds 4, 16, 64 duplicate private constants from oasis-app-text-editor

  • runner_sdi.rs manually iterates 0..4 for menu labels, 0..16 [UNVERIFIED - File does not exist and is not in PR] for dropdown entries, and 0..64 [UNVERIFIED - File does not exist and is not in PR] for visible lines. The authoritative values live in oasis-app-text-editor/src/render.rs as menu_label_count() = 4, NP_MAX_DROPDOWN_ENTRIES = 16, NP_MAX_VISIBLE_LINES = 64.
  • If the text editor adds a 5th menu or changes pool sizes, runner_sdi.rs will leak stale SDI objects or over-clean unrelated ones.
  • Fix: Expose a TextEditorApp::hide_all_sdi_objects(sdi) public method that encapsulates the cleanup, or re-export the constants from oasis-app-text-editor.

• [WARNING] crates/oasis-app-text-editor/src/render.rs:956 - dropdown_size() duplicates layout constants from oasis_ui::menu_bar

  • dropdown_size() hardcodes 20px/item, 6px/sep, 22px pad which must stay in sync with ITEM_H, SEPARATOR_H, ITEM_PAD_X in menu_bar.rs. Divergence causes the SDI-path hit boxes to misalign from the rendered drop-down geometry.
  • Fix: Expose MenuBar::dropdown_dimensions(menu: &Menu) -> (u32, u32) as a pub function in oasis-ui and call it from render.rs.

• [WARNING] crates/oasis-app-media/src/photo_ui.rs:42-45 - Texture uploaded and destroyed on every frame with no backend contract guaranteeing deduplication

  • load_texture + destroy_texture is called each draw_windowed call; the comment says backends "dedupe redundant uploads internally" but SdiBackend makes no such guarantee. This is an implicit contract that could cause GPU memory churn or stutter on slower backends.
  • Fix: Cache the texture handle in BrowsingApp alongside decoded_image; invalidate on open_file or when zoom changes.

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

• crates/oasis-app-text-editor/src/lib.rs:70 - menu_label_count() returns a hardcoded 4; it is used by hide_notepad_sdi to bound the np_menu_* cleanup loop. Derive it from self.menu.menus.len() at hide time instead to prevent silent leaks if a 5th menu is added later.

• crates/oasis-app-media/src/music_ui.rs:130 - Progress bar is static at 10%; the BrowsingApp already has a slideshow_timer field. Consider incrementing a frame counter there and animating the bar width as (frame % bar_w) to signal that audio is playing, without needing a backend handle.

Notes

• 2 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• AppAction losing Copy (now only Clone) is a deliberate, correct change required by the String payloads. The simple_app.rs call site was the only one needing .clone() — no other silent regressions observed in the diff.

• Clock app removal is clean: crate deleted, workspace removed, registry entry gone, Cargo.lock updated.

• The IPC pattern (/var/audio/request) mirrors radio_controller cleanly; the "leave unknown payloads" comment is slightly misleading (no other controller shares that path) but the guard is harmless.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [BUG] crates/oasis-app/src/media_controller.rs:49 - stop_track calls audio_backend.stop() globally
  • state.audio_backend.stop() is a global audio stop, not track-scoped. Switching music tracks (or reopening Music Player) silently kills any concurrently-running Internet Radio stream, even though the Radio runner is still open and the user never dismissed it
  • Fix: either gate the stop on state.media_track.is_some() being the only active audio (check if radio track is also live), or use a track-specific pause/stop if the AudioBackend trait supports it; at minimum add a guard that calls stop_radio(state) explicitly when a conflict is detected, rather than silently ending radio via a side effect

Previous Issues (for incremental reviews)

• [STILL UNRESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click hardcodes title_h = 20 while draw_notepad uses at.app.title_bar_height.max(18); click-region mismatch persists for any theme whose title bar height ≠ 20
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile silently dropped in the WASM windowed (open_runners) path; a comment was added acknowledging the gap but no fix was applied
• [STILL UNRESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4, 0..16 [UNVERIFIED - File does not exist and is not in PR], 0..64 [UNVERIFIED - File does not exist and is not in PR] still duplicate the authoritative constants from oasis-app-text-editor
• [STILL UNRESOLVED] crates/oasis-app-text-editor/src/render.rs:956 — dropdown_size() duplicates ITEM_H=20, SEPARATOR_H=6, ITEM_PAD_X=22 from oasis_ui::menu_bar; divergence causes SDI-path hit boxes to misalign from rendered geometry
• [STILL UNRESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — load_texture + destroy_texture called on every frame; comment says backends dedupe internally but SdiCore makes no such guarantee

Suggestions (if any)

• crates/oasis-app-media/src/lib.rs (id3v2_title) — frame size is decoded as big-endian u32 (correct for ID3v2.3) but ID3v2.4 uses syncsafe integers; tags from modern encoders will produce corrupted title strings. Consider reading the ID3v2 major version from data[3] and switching decode accordingly.
• crates/oasis-app-text-editor/src/render.rs (render_notepad_sdi) — the stale-object cleanup loop for i in 0..100 is a third hardcoded bound (in addition to the two flagged above); it should at minimum be a named constant.

Notes

• 2 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• BrowsingApp::music_player_at / photo_viewer_at constructors and TextEditorApp::open_from_vfs are clean additions; error paths return graceful fallbacks rather than panicking.

• The app_for_file extension-map approach is appropriately conservative; the static string returns ("Photo Viewer", "Music Player", "Text Editor") avoid any string-injection risk in launch_app_window_for_file's win_id derivation.

• Clock app removal is clean: crate, workspace entry, registry entry, and Cargo.lock are all consistent.

• The IPC pattern (/var/audio/request) mirrors radio_controller cleanly for Music Player; the only concern is the shared audio_backend.stop() call noted above.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: 37fa75f

Fixed Issues

• [BUG] crates/oasis-app/src/media_controller.rs — play_file now explicitly tears down an active Internet Radio session before starting a music track, so the AudioBackend::stop() side-effect no longer silently silences radio while leaving its state machine thinking it's playing.
• [BUG] crates/oasis-app-text-editor/src/lib.rs — handle_click now derives title_h from content.cached_title_bar_height.max(18), matching what draw_notepad uses. Previously the hardcoded 20 diverged from the theme-driven renderer for skins with non-20 title bar heights (default is 22).
• [BUG] crates/oasis-backend-wasm/src/input_dispatch.rs + lib.rs — LaunchAppWithFile is now wired in the windowed (open_runners) path via a new OasisWasm::launch_app_window_for_file helper, mirroring the fullscreen branch. File Manager Confirm-on-typed-file now correctly hands off to Photo Viewer / Music Player / Text Editor on WASM.
• [WARNING] crates/oasis-core/src/apps/runner_sdi.rs + crates/oasis-app-text-editor/src/{lib,render}.rs — Extracted the np_* SDI cleanup into a public oasis_app_text_editor::hide_notepad_sdi_objects(sdi) free function. runner_sdi::hide_sdi now calls it instead of duplicating the 0..4, 0..16, 0..64 pool bounds — keeping the constants in one place.
• [WARNING] crates/oasis-app-text-editor/src/render.rs + crates/oasis-ui/src/menu_bar.rs — Exposed MenuBar::dropdown_dimensions(menu) -> (u32, u32) in oasis-ui. render_dropdown_sdi now uses it and the local dropdown_size duplicate (with hardcoded 20/6/22/120 copies of the widget's ITEM_H/SEPARATOR_H/ITEM_PAD_X/DROPDOWN_MIN_W) was removed. SDI-path hit boxes can no longer drift from the widget's rendered geometry.
• [WARNING] crates/oasis-app-media/src/{lib,photo_ui}.rs — Photo Viewer texture is now uploaded once per open_file and cached in BrowsingApp::cached_photo_texture: Cell<Option<TextureId>>; the stale handle from the previous image is parked in stale_photo_texture and destroyed on the next photo_ui::draw. No more relying on implicit "backends dedupe uploads" contract.

Ignored Issues

• Suggestion: menu_label_count() derive from self.menu.menus.len() — deliberately left as-is. The function is private, only called from hide_notepad_sdi_objects which is now static and doesn't have a TextEditorApp reference. Adding a 5th top-level menu would require updating the constant, which is discoverable by any contributor adding a menu. Low-value refactor relative to the ergonomic cost.
• Suggestion: animate Music Player progress bar — out of scope for a correctness/security review round; not a bug.
• Suggestion: ID3v2.4 syncsafe frame sizes in id3v2_title — existing behavior; out of scope. Would be a new feature, not a fix.
• Suggestion: replace for i in 0..100 hardcoded bound in render_notepad_sdi — the loop breaks on the first missing object, so the bound is just a safety ceiling, not a tight constant that must stay in sync.

Deferred to Human

• None.

Notes

• All touched crates pass cargo check (desktop and wasm32-unknown-unknown), cargo clippy -- -D warnings, and cargo test (258 tests in oasis-app, 791 across oasis-app-text-editor/oasis-app-media/oasis-ui).
• cargo fmt --all applied.
• The stop_track change uses oasis_audio::radio::RadioState via its fully qualified path, avoiding a new use statement in media_controller.rs.

---

Automated summary of agent fixes.

[AndrewAltimit]

Fixed the cargo-deny pre-push advisory in 8d1abdf: bumped rustls-webpki from 0.103.12 → 0.103.13 to pick up the patch for RUSTSEC-2026-0104 (reachable panic in BorrowedCertRevocationList::from_der / OwnedCertRevocationList::from_der triggered by an empty BIT STRING in a CRL's onlySomeReasons).

We don't currently parse CRLs, so it wasn't a live issue for OASIS, but the advisory was failing every pre-push and SKIP=cargo-deny-oasis was being used to work around it. The fix is entirely in Cargo.lock — semver-compatible bump, no API changes.

Verified:

• cargo deny check now reports advisories ok, bans ok, licenses ok, sources ok
• Pre-push hook now passes without SKIP=
• 7175 workspace tests still pass, clippy clean

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app/src/media_controller.rs:68 - stop_track still issues a global audio_backend.stop() when switching music tracks
  • When media_track is Some (a track is already playing) and the user opens a second file, play_file calls stop_track which fires audio_backend.stop() globally — this kills TV Guide video audio if both apps are open concurrently in windowed multi-window mode. The guard only prevents the "close Music Player with no track loaded" case from firing the global stop; track switching is not protected
  • play_file explicitly tears down radio_manager state before starting music, but state.tv_audio_track / TV Guide is not addressed. The sequence: Music Player plays track A → user switches to track B → stop_track fires global stop → TV Guide audio silently dies
  • Fix: check state.tv_audio_track.is_none() before calling audio_backend.stop() (and add explicit TV Guide audio teardown if needed), or use a track-specific stop API if AudioBackend supports it

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18); matches the renderer
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both fullscreen and windowed WASM paths via launch_app_window_for_file
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4/16/64 replaced by a single call to oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi now calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture uploaded once per open_file and cached in BrowsingApp::cached_photo_texture; stale handle deferred to next render via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — radio teardown is now explicit in play_file; stop_track guard prevents global stop when no track is loaded (closing the window no longer kills radio)

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs — update_hover is implemented but never called; no handle_mouse_move path exists on the App trait, so hovered_item stays None and drop-down items render without hover highlighting. Either wire it through a new trait hook or remove the method until the hover path exists.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• Clock app removal is clean end-to-end: crate deleted, workspace and Cargo.toml deps removed, registry entry gone, CLAUDE.md and docs/design.md updated, Cargo.lock consistent.

• ITEM_H / SEPARATOR_H / ITEM_PAD_X / DROPDOWN_MIN_W are now the single source of truth in menu_bar.rs; both the direct-render path (draw_dropdown) and the SDI-layout path (dropdown_dimensions) use them — the layout drift risk that prompted the original warning is gone.

• Cell<Option<TextureId>> for texture caching is architecturally sound given TextureId: Copy; it allows photo_ui::draw (taking &BrowsingApp) to mutate the handle cache without requiring &mut self on the renderer.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new issues found in TIER 1 files)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18); matches the renderer for all themes
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (open_runners) and fullscreen WASM paths via the new launch_app_window_for_file helper
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4/16/64 replaced by a single call to oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui; layout constants are now the single source of truth in menu_bar.rs
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture uploaded once per open_file, cached in BrowsingApp::cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — play_file explicitly tears down radio before starting music; stop_track guarded by state.media_track.is_some()
• [STILL UNRESOLVED] crates/oasis-app/src/media_controller.rs — switching music tracks (track A → track B) calls stop_track with media_track = Some(A), which fires audio_backend.stop() globally; TV Guide audio (state.tv_audio_track) is not addressed — it would be silenced without its state machine being notified, in windowed multi-window mode where both apps can be open concurrently

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — method is fully implemented but no call site exists; App trait has no handle_mouse_move hook, so hovered_item is always None and hover highlights never render. Remove until a mouse-move hook exists, or add a note that it's forward-looking.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• Clock app removal is clean end-to-end across all files: crate, workspace, registry, CLAUDE.md, docs/design.md, Cargo.lock all consistent.

• Cell<Option<TextureId>> for texture caching is architecturally sound: allows photo_ui::draw(&BrowsingApp) to mutate the cache without &mut self.

• The app_for_file extension map returning static strings sidesteps any string-injection risk in win_id derivation (to_lowercase().replace(' ', "_")).

• The std::panic::catch_unwind wrapper in image::decode is appropriate defensive practice for the third-party png/jpeg-decoder crates.

• id3v2_title uses big-endian u32 frame sizes (ID3v2.3 format); ID3v2.4 uses syncsafe integers — acknowledged limitation from the previous review, no change.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [BUG] crates/oasis-app/src/media_controller.rs:82-85 - stop_track still fires global audio_backend.stop() when switching music tracks
  • When state.media_track is Some (track A playing) and the user opens track B, play_file calls stop_track which unconditionally calls state.audio_backend.stop() — this is a process-global stop that will silently kill TV Guide audio (state.tv_audio_track) if both apps are open concurrently in windowed multi-window mode
  • The guard if let Some(track) = state.media_track.take() only prevents the stop when no track is loaded (closing the window with nothing playing); it does not protect the track-switching path
  • Fix: check state.tv_audio_track.is_none() before calling audio_backend.stop(), or use a track-scoped stop API if the backend supports it

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:270 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18), matching the renderer
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs — LaunchAppWithFile wired in both windowed (open_runners) and fullscreen paths via launch_app_window_for_file
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs — hardcoded 0..4/16/64 replaced by call to oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs — texture uploaded once per open_file, cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — radio teardown is now explicit in play_file before starting music; stop_track guarded by media_track.is_some()
• [STILL UNRESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — switching music tracks (A → B) still fires global audio_backend.stop(), which would kill TV Guide audio in concurrent multi-window mode

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — fully implemented but never called; the App trait has no handle_mouse_move hook, so hovered_item stays None and drop-down items never render hover highlights. Either wire it through a new trait hook or remove the method until the hover path exists.

Notes

• AppAction losing Copy (now only Clone) is correct and required by the String payloads in LaunchAppWithFile; the single .clone() call site in simple_app.rs is the only change needed.
• Clock app removal is clean end-to-end: crate deleted, workspace/registry entries removed, Cargo.lock updated, docs consistent.
• The app_for_file extension map uses static string returns, avoiding any string-injection risk in win_id derivation.
• Cell<Option<TextureId>> for texture caching is sound given TextureId: Copy; allows photo_ui::draw(&BrowsingApp) to mutate the cache without &mut self.
• std::panic::catch_unwind in image::decode is appropriate defensive practice for third-party png/jpeg-decoder crates.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 2)

Status: Changes committed, pushing...

Commit: 65792bd

Fixed Issues

• [BUG] crates/oasis-app/src/media_controller.rs — stop_track now guards the global audio_backend.stop() with state.tv_audio_track.is_none(). Previously, switching music tracks (or any path where stop_track fired while media_track was Some) unconditionally cleared the shared audio stream buffer, which would silence TV Guide video audio mid-playback in windowed multi-window mode while leaving its state machine oblivious. AudioBackend::unload_track already internally calls stop() when the track matches current_track, so the common case (music currently driving the output) still stops correctly on unload.

Ignored Issues

• Suggestion: crates/oasis-ui/src/menu_bar.rs::update_hover is implemented but never called — out of scope for a correctness/security iteration. Flagged as a "Suggestion" (not an "Issue") by reviewers; adding/removing a forward-looking API is a design decision, not a bug fix. No hover highlight appears today, but nothing is broken.
• Resolved issues already fixed in prior iteration (commit 37fa75f) — all the major items: handle_click title-bar height, WASM LaunchAppWithFile wiring, hide_notepad_sdi_objects extraction, MenuBar::dropdown_dimensions exposure, photo texture caching, radio teardown in play_file. Reviewers marked each [RESOLVED] in the latest incremental reviews.

Deferred to Human

• None.

Notes

• cargo check -p oasis-app and cargo clippy -p oasis-app -- -D warnings both clean.
• cargo test -p oasis-app --bins: 258 passed, 0 failed.
• cargo fmt --all applied.
• Reviewer [UNVERIFIED] annotations on runner_sdi.rs line numbers and render.rs:956 were correctly filtered by the review tooling — the prior iteration already replaced those with hide_notepad_sdi_objects / dropdown_dimensions calls, so the line numbers cited by the reviewers no longer exist.

---

Automated summary of agent fixes.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app/src/media_controller.rs:46-52 - Radio teardown still calls audio_backend.stop() globally, bypassing the TV Guide guard
  • play_file guards the stop_track path with tv_audio_track.is_none(), but if radio is also active when music starts, the subsequent radio_manager.process_request("stop", &mut state.audio_backend) issues a global audio_backend.stop() internally — unguarded. If TV Guide is streaming audio concurrently in windowed multi-window mode, this call would still silence its audio without updating its state machine
  • Fix: check state.tv_audio_track.is_none() before the radio teardown block, or add the same guard inside process_request("stop") for the audio backend call

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18), matching the renderer
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (open_runners) and fullscreen paths via launch_app_window_for_file
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track now guards audio_backend.stop() with state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — same guard covers track-switching path
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — confirmed resolved in new file; guard is present

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still implemented but has no call site; App trait has no handle_mouse_move hook, so hovered_item is always None and hover highlights never render. Remove until a mouse-move hook exists, or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• The primary concern from iterations 1–2 (global audio_backend.stop() silencing TV Guide when switching tracks) is resolved cleanly. The remaining warning above is an adjacent, less common path (radio + TV Guide + music all active simultaneously).

• Clock app removal is complete and consistent across all files.

• Cell<Option<TextureId>> for photo texture caching remains architecturally sound.

• The stop_track guard correctly distinguishes "no TV Guide active → safe to stop globally" from "TV Guide active → defer to unload_track's internal behavior."

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new actionable issues found in TIER 1 files)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18), matching the renderer
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in the windowed (open_runners) path via self.launch_app_window_for_file; mirrored in fullscreen path in lib.rs
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4/16/64 replaced by a single call to oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui; layout constants are now the single source of truth in menu_bar.rs
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture uploaded once per open_file, cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track now guards audio_backend.stop() with state.tv_audio_track.is_none(); covers both window-close and track-switching paths
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — same guard covers the track-switching path
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — confirmed resolved; guard is in place
• [STILL UNRESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — play_file still calls state.radio_manager.process_request("stop", &mut state.audio_backend) when stopping radio to start music; process_request("stop") internally issues a global audio_backend.stop(), bypassing the tv_audio_track.is_none() guard added to stop_track. TV Guide audio would still be silenced in a Radio + TV Guide + Music concurrent windowed session. Fix: add the same state.tv_audio_track.is_none() guard before the radio teardown block.

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still implemented and never called; App trait has no handle_mouse_move hook so hovered_item is always None and drop-down hover highlights never render. Remove until a mouse-move hook exists, or annotate explicitly as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• stop_track guard is correctly structured: unload_track(track) handles track-specific teardown when TV Guide is active; the explicit audio_backend.stop() is only issued when no TV Guide audio is present. The only remaining gap is the radio teardown path noted above.

• Clock app removal is clean end-to-end across all files: crate, workspace, registry, CLAUDE.md, docs/design.md, Cargo.lock all consistent.

• Cell<Option<TextureId>> for photo texture caching remains architecturally sound given TextureId: Copy.

• The rustls-webpki 0.103.12 → 0.103.13 bump is a clean Cargo.lock-only change with no API surface.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [BUG] crates/oasis-app/src/media_controller.rs:46-52 - Radio teardown in play_file calls audio_backend.stop() globally, bypassing the TV Guide guard
  • When starting a music track while Internet Radio is active, process_request("stop", &mut state.audio_backend) issues a process-global audio stop. If TV Guide is concurrently streaming audio in windowed multi-window mode, this silently silences it without updating its state machine.
  • Fix: Add if state.tv_audio_track.is_none() guard before the radio teardown block, mirroring the guard already in stop_track.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 - handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 - LaunchAppWithFile wired in windowed path
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 - Hardcoded bounds replaced by hide_notepad_sdi_objects
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] - dropdown_size() removed, uses dropdown_dimensions
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 - Texture caching implemented
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 - Radio teardown explicit, stop_track guarded
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 - Track-switching path guarded
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 - Guard confirmed in place

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) - Method is fully implemented but never called; App trait lacks handle_mouse_move hook, so hovered_item stays None and drop-down items never render hover highlights. Remove until a mouse-move hook exists, or annotate as forward-looking API.
• crates/oasis-app-text-editor/src/render.rs (menu_label_count) - Returns hardcoded 4; used by hide_notepad_sdi_objects to bound cleanup loop. Adding a 5th top-level menu without updating this constant silently leaks np_menu_4 SDI object. Consider deriving from self.menu.menus.len() if feasible.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• AppAction losing Copy (now Clone) is correct for String payloads; single .clone() call site updated.

• Clock app removal is clean across workspace, registry, docs, and Cargo.lock.

• Cell<Option<TextureId>> for photo texture caching is sound given TextureId: Copy.

• id3v2_title uses ID3v2.3 big-endian frame sizes; ID3v2.4 syncsafe limitation acknowledged and out of scope.

• app_for_file extension map uses static string returns, avoiding injection risks in win_id derivation.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 3)

Status: Changes committed, pushing...

Commit: b567b6f

Fixed Issues

• [WARNING] crates/oasis-app/src/media_controller.rs — play_file's radio teardown block is now guarded by state.tv_audio_track.is_none(), mirroring the stop_track guard added in iteration 2. Previously, if Internet Radio was active when Music Player started, RadioManager::process_request("stop", ...) would internally call AudioBackend::stop() (which clears the shared SDL stream buffer), silencing any concurrent TV Guide video audio without notifying its state machine. The updated comment documents the trade-off: when TV Guide is active, the radio teardown is skipped — radio's RadioState stays Playing until the user explicitly stops it, but the subsequent play(music_track) swaps current_track away from radio so audio does not interleave.

Ignored Issues

• Suggestion: crates/oasis-ui/src/menu_bar.rs::update_hover is implemented but never called — consistently flagged as a Suggestion (not an Issue) across iterations. Adding a new handle_mouse_move hook on the App trait or removing the method is a forward-looking design decision outside this correctness iteration's scope. The prior iteration already deferred this; no new information justifies reopening.
• Suggestion: derive menu_label_count() from self.menu.menus.len() — already deferred in iteration 1 (hide_notepad_sdi_objects is static, no TextEditorApp reference available at call site). No new justification from reviewers.
• Suggestion: ID3v2.4 syncsafe frame sizes — existing behaviour, acknowledged limitation. Out of scope for a correctness-review iteration (would be a new feature).
• All [RESOLVED] issues from prior commits — title-bar height in handle_click, WASM LaunchAppWithFile wiring, hide_notepad_sdi_objects extraction, dropdown_dimensions exposure, photo texture caching, stop_track TV Guide guard. Reviewers marked these resolved in iteration 2's incremental reviews.
• Reviewer [UNVERIFIED] annotations on render.rs:956 / runner_sdi.rs line numbers — the prior iteration already replaced those with hide_notepad_sdi_objects / dropdown_dimensions calls, so those specific lines no longer exist. The review tooling correctly filtered them as stale.

Deferred to Human

• None.

Notes

• cargo check -p oasis-app: clean.
• cargo clippy -p oasis-app -- -D warnings: clean.
• cargo test -p oasis-app --bins: 258 passed, 0 failed.
• cargo fmt --all -- --check: clean.
• Only crates/oasis-app/src/media_controller.rs was edited; crates/oasis-audio/src/radio/mod.rs was intentionally left alone to keep the fix scope tight to this PR's newly-introduced Music Player IPC flow. The same pattern could be applied inside RadioManager::stop() by removing the explicit backend.stop() call in favor of unload_track's semantics, but that affects every radio caller and was not required to resolve the reviewer's concern.

---

Automated summary of agent fixes.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-media/src/image.rs:41 - 16-bit PNG images will decode incorrectly
  • png::Transformations::EXPAND expands palettes and sub-byte depths to 8-bit, but does NOT convert 16-bit images to 8-bit. For 16-bit grayscale/RGBA PNGs, reader.output_buffer_size() returns a buffer with 2 bytes per channel, but the match arms iterate assuming 1 byte per channel, producing garbled output or incorrect dimensions.
  • Fix: Add png::Transformations::STRIP_16 to the transformations chain, or explicitly check info.bit_depth and handle 16-bit cases.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; uses self.menu.dropdown_dimensions(menu)
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded with state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — track-switching path guarded
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block now guarded with state.tv_audio_track.is_none()

Suggestions (if any)

• crates/oasis-app-media/src/lib.rs:398 - pending_vfs_request is overwritten on rapid consecutive file opens without the host consuming the previous request. Consider asserting self.content.pending_vfs_request.is_none() before setting, or use a bounded queue to avoid dropped IPC messages.
• crates/oasis-ui/src/menu_bar.rs:288 - update_hover is fully implemented but has no call site; the App trait lacks a handle_mouse_move hook, so hovered_item stays None and drop-down items never render hover highlights. Remove until a mouse-move hook exists, or annotate as forward-looking API.
• crates/oasis-app-media/src/music_ui.rs:130 - Progress bar is static at 10%. Consider animating it using slideshow_timer or a frame counter to signal playback activity.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• Clock app removal is clean end-to-end: crate, workspace, registry, docs, and Cargo.lock all consistent.

• AppAction losing Copy (now Clone) is correct and required by String payloads in LaunchAppWithFile.

• Sample media embedding (include_bytes!) ensures fresh VFS instances have working content without external scripts.

• id3v2_title uses big-endian u32 frame sizes (ID3v2.3 format); ID3v2.4 syncsafe limitation remains acknowledged and out of scope.

• Cell<Option<TextureId>> for photo texture caching is architecturally sound given TextureId: Copy.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [STILL UNRESOLVED] crates/oasis-app-media/src/image.rs:41 — 16-bit PNG images decode incorrectly
  • png::Transformations::EXPAND expands palette/sub-byte depths to 8-bit but does NOT convert 16-bit images; for a 16-bit RGBA PNG, output_buffer_size() returns w*h*8 bytes and the ColorType::Rgba arm takes the buffer as-is, producing garbled pixel data at half the expected dimensions; the Rgb arm uses chunks_exact(3) but 16-bit RGB is 6 bytes/pixel
  • Fix: add | png::Transformations::STRIP_16 to the transformations chain so 16-bit depth is downsampled to 8-bit before the match arms run

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (self.launch_app_window_for_file) and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded with state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — same guard covers track-switching path
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — confirmed resolved; guard is in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block now guarded with state.tv_audio_track.is_none() in iteration 3

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — fully implemented but no call site exists; App trait has no handle_mouse_move hook so hovered_item stays None and drop-down items never render hover highlights. Remove or annotate as forward-looking API.

Notes

• All audio teardown paths in the new media_controller.rs are correctly guarded. The unload_track call after a guarded stop() in stop_track relies on the backend's unload_track being conditional on current-track ownership — this is consistent with the comment and prior review analysis.
• The three-way Radio + TV Guide + Music concurrent session edge case (Radio teardown skipped when TV Guide is active, leaving RadioState::Playing) is a known acknowledged trade-off documented in the code comment, not a new regression.
• Clock app removal is clean end-to-end across workspace, registry, CLAUDE.md, docs/design.md, and Cargo.lock.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-media/src/image.rs:41 - 16-bit PNG images decode incorrectly
  • png::Transformations::EXPAND expands palette/sub-8-bit depths to 8-bit but does not convert 16-bit images; for a 16-bit RGBA PNG output_buffer_size() returns w*h*8 bytes and the Rgba arm treats it as-is producing garbled pixels at half the expected dimensions; the Rgb arm uses chunks_exact(3) but 16-bit RGB is 6 bytes/pixel
  • Fix: decoder.set_transformations(png::Transformations::EXPAND | png::Transformations::STRIP_16);

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (launch_app_window_for_file) and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4/16/64 replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture uploaded once per open_file, cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — same guard covers track-switching path
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block now guarded: if state.radio_manager.state() != RadioState::Stopped && state.tv_audio_track.is_none()
• [STILL UNRESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 still absent; 16-bit PNGs remain garbled

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — method is fully implemented but has no call site; App trait has no handle_mouse_move hook so hovered_item stays None and drop-down items never render hover highlights. Remove or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• All audio teardown guards are correct and consistent across stop_track, track-switching, and the radio teardown block in play_file. The documented trade-off (radio RadioState stays Playing when TV Guide is active) is clearly commented.

• id3v2_title correctly uses syncsafe encoding for the tag header size (valid for both v2.3 and v2.4) but big-endian u32 for frame sizes (v2.3 only) — acknowledged limitation, no change.

• fit_within nearest-neighbour scaler uses (y * h) / nh where y < nh ≤ 1024 and h is u32; 1023 * h overflows if h > ~4M but catch_unwind wraps the whole decode path, making this a correctness edge case rather than a safety issue.

• app_for_file returns only static strings, keeping win_id derivation (to_lowercase().replace(' ', "_")) injection-safe on all dispatch paths.

• The Notepad plugin no longer registers a dashboard app tile — the updated test guards against re-introduction of the duplicate tile.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 4)

Status: Changes committed, pushing...

Commit: 03d495d

Fixed Issues

• [WARNING] crates/oasis-app-media/src/image.rs:41 — 16-bit PNG decoding was producing garbled output. png::Transformations::EXPAND promotes palettes and sub-byte depths but does not convert 16-bit channels to 8-bit, so reader.output_buffer_size() returned w*h*8 bytes for a 16-bit RGBA PNG and the match arms (which assume 1 byte per channel) took that buffer as-is, yielding incorrect pixels at half the expected dimensions. Added | png::Transformations::STRIP_16 to the transformations chain so 16-bit depths are downsampled to 8-bit before the match runs. This bug was reported as [STILL UNRESOLVED] by both Claude security and OpenRouter reviewers on commit b567b6f.

Ignored Issues

• Suggestion: crates/oasis-ui/src/menu_bar.rs::update_hover is implemented but never called — consistently flagged as a Suggestion across iterations. Already deferred in iterations 1–3 as a forward-looking design decision (requires handle_mouse_move hook on App trait). No new justification from this round's reviewers.
• Suggestion: crates/oasis-app-media/src/music_ui.rs:130 static 10% progress bar — cosmetic polish, not a correctness bug. Out of scope for a correctness-review iteration.
• Suggestion: crates/oasis-app-media/src/lib.rs:398 pending_vfs_request overwrite — theoretical edge case (host always consumes before the user can double-click a new file in realistic flows). No concrete failure scenario described.
• Suggestion: ID3v2.4 syncsafe frame sizes — existing behaviour, acknowledged limitation carried forward from prior iterations.
• All [RESOLVED] issues — fixed in prior iteration commits (37fa75f, 65792bd, b567b6f): title-bar height in handle_click, WASM LaunchAppWithFile wiring, hide_notepad_sdi_objects extraction, dropdown_dimensions exposure, photo texture caching, stop_track and play_file radio-teardown TV Guide guards.

Deferred to Human

• None.

Notes

• cargo check -p oasis-app-media: clean.
• cargo clippy -p oasis-app-media -- -D warnings: clean.
• cargo test -p oasis-app-media --lib: 25 passed, 0 failed (includes existing decode_png_rgb test which verifies 8-bit path still works).
• Only crates/oasis-app-media/src/image.rs was edited — a one-line transformations bitmask change plus updated doc comment. No test was added for 16-bit PNG decoding because the existing png crate API does not expose a convenient set_depth(16) helper in a way that would round-trip through the encoder without substantial test scaffolding; the fix is a documented transformation library behaviour (STRIP_16 is the canonical way to force 8-bit output).

---

Automated summary of agent fixes.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new actionable issues found in the TIER 1 file)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click derives title_h from self.content.cached_title_bar_height.max(18); matches the renderer for all themes
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (self.launch_app_window_for_file) and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) via oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — track-switching path covered by the same guard
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block guarded: state.radio_manager.state() != RadioState::Stopped && state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 added to transformations chain; 16-bit PNG channels are downsampled to 8-bit before the match arms run

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still implemented with no call site; App trait has no handle_mouse_move hook so hovered_item stays None and drop-down items never render hover highlights. Remove or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• decode_png reads info.color_type from reader.info() (post-transformation OutputInfo), so palette-indexed PNGs expanded to RGB/RGBA by EXPAND correctly reach the Rgb/Rgba match arms — the fix is architecturally sound.

• fit_within uses (y * h) / nh in u32 arithmetic; theoretical overflow for images taller than ~4M pixels, but catch_unwind wraps the full decode path so the result is graceful None. Unreachable in practice given MAX_DIMENSION = 1024.

• The single-source-of-truth for drop-down layout constants (ITEM_H, SEPARATOR_H, ITEM_PAD_X, DROPDOWN_MIN_W) in menu_bar.rs is now used by both the direct-render path (draw_dropdown) and the SDI-layout path (dropdown_dimensions). Layout drift risk is eliminated.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-media/src/image.rs:40 - Palette PNG images silently fail to decode despite EXPAND transformation
  • reader.info() returns the raw PNG header Info (pre-transformation). For palette-indexed PNGs, info.color_type == ColorType::Indexed, which hits the _ => return None arm. Transformations::EXPAND would correctly convert palette → RGB/RGBA and output_buffer_size() already accounts for this (sizes the buffer for RGB), but the color-type match uses the wrong source.
  • Fix: let (color, _) = reader.output_color_type(); instead of let color = info.color_type; — this gives the post-transformation color type that actually matches the buffer contents.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed (self.launch_app_window_for_file) and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) via oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — same guard covers the track-switching path
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block now guarded: state.radio_manager.state() != RadioState::Stopped && state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 added; 16-bit PNG channels downsampled to 8-bit before match arms run

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still implemented with no call site; App trait lacks handle_mouse_move, so hovered_item is always None and hover highlights never render. Remove or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• The STRIP_16 fix is correct and safe: bit depth (16→8) changes but ColorType (Rgba/Rgb/Grayscale/GrayscaleAlpha) is preserved by the transformation, so the match arms remain valid for all non-palette types that STRIP_16 touches.

• The palette PNG issue is distinct from the 16-bit fix and was pre-existing in this new file — output_buffer_size() is already transformation-aware (returns w*h*3 for palette-after-EXPAND), but the match uses the raw header's Indexed type, causing silent None returns for any palette-indexed PNG.

• All audio teardown guards are consistent across stop_track, track-switching, and the radio teardown block. The three-way Radio + TV Guide + Music edge case trade-off (radio RadioState stays Playing when TV Guide is active) remains documented.

• std::panic::catch_unwind wraps decode_png/decode_jpeg but fit_within is called outside it; however, the .min(h-1) / .min(w-1) clamps in fit_within ensure slice bounds stay valid for any image that survives decode (practical overflow would require images too large to allocate).

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app-media/src/image.rs:44 - Palette-indexed PNG images silently fail to decode
  • reader.info() returns the raw PNG header Info (pre-transformation). For palette-indexed PNGs, info.color_type == ColorType::Indexed, which hits the _ => return None arm. Transformations::EXPAND correctly converts palette → RGB/RGBA and output_buffer_size() already accounts for this, but the match uses the wrong source.
  • Fix: let (color, _) = reader.output_color_type(); instead of let color = info.color_type; — this gives the post-transformation color type that actually matches the buffer contents.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; uses self.menu.dropdown_dimensions(menu)
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded with state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — track-switching path guarded
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block now guarded with state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 added to transformations chain

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — fully implemented but has no call site; App trait lacks handle_mouse_move hook so hovered_item stays None and drop-down items never render hover highlights. Remove or annotate as forward-looking API.
• crates/oasis-app-media/src/lib.rs:398 — pending_vfs_request is cleared then set in open_file; if the host hasn't consumed a previous request before the user opens a new file, the earlier IPC is silently dropped. Consider asserting self.content.pending_vfs_request.is_none() before clearing, or use a bounded queue.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• Clock app removal is clean end-to-end: crate deleted, workspace/registry entries removed, Cargo.lock updated, docs consistent.

• AppAction losing Copy (now Clone) is correct for String payloads; single .clone() call site in simple_app.rs updated.

• The app_for_file extension map uses static string returns, avoiding injection risks in win_id derivation.

• Cell<Option<TextureId>> for photo texture caching is sound given TextureId: Copy.

• All audio teardown guards in media_controller.rs are correctly structured across stop_track, track-switching, and radio teardown paths.

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.

[AndrewAltimit]

Review Response Agent (Iteration 5)

Status: Changes committed, pushing...

Commit: bc806c9

Fixed Issues

• [WARNING] crates/oasis-app-media/src/image.rs:52 — Palette-indexed PNGs were silently failing to decode. reader.info().color_type returns the raw PNG header color type (pre-transformation), so palette PNGs would read as ColorType::Indexed and hit the _ => return None arm — even though Transformations::EXPAND had already expanded them to RGB/RGBA. Changed to let (color, _) = reader.output_color_type(); which returns the post-transformation color type that actually matches the buffer contents. Both Claude Security and OpenRouter reviewers flagged this as a [WARNING] on the latest commit (03d495d). Added a regression test decode_png_palette that encodes and decodes a palette-indexed PNG.

Ignored Issues

• Claude Quality Review (commit 03d495d): reported no new actionable issues — already clean.
• Suggestion: crates/oasis-ui/src/menu_bar.rs::update_hover — implemented but no call site. Consistently flagged as a forward-looking API suggestion across iterations 1–4; requires adding handle_mouse_move hook to App trait, which is a design decision outside a correctness iteration's scope.
• Suggestion: crates/oasis-app-media/src/lib.rs:398 pending_vfs_request overwrite — already deferred in iteration 4; theoretical edge case with no concrete failure scenario described.
• All [RESOLVED] items — title-bar height in handle_click, WASM LaunchAppWithFile wiring, hide_notepad_sdi_objects extraction, dropdown_dimensions exposure, photo texture caching, audio teardown guards in media_controller.rs, STRIP_16 for 16-bit PNGs. All already fixed in prior iteration commits (37fa75f, 65792bd, b567b6f, 03d495d).

Deferred to Human

• None.

Notes

• cargo test -p oasis-app-media --lib image::: all 6 tests pass (5 pre-existing + 1 new palette regression test).
• cargo clippy -p oasis-app-media --all-targets -- -D warnings: 15 errors, identical to baseline before my edit — no new warnings introduced. (The 15 baseline errors are pre-existing unwrap_used violations in unrelated test helpers and are not part of this PR's scope.)
• The new test uses expect() rather than unwrap() to avoid adding any new lint violations.
• Fix is the one both reviewers independently proposed: reader.output_color_type() is transformation-aware (same source the png crate's own output_buffer_size() uses internally at line 607).

---

Automated summary of agent fixes.

[github-actions]

Claude AI Incremental Architecture & Quality Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [WARNING] crates/oasis-app/src/launch.rs + crates/oasis-backend-wasm/src/lib.rs (launch_app_window_for_file) - Photo Viewer GPU texture leaked when runner is replaced in an existing window
  • When launch_app_window_for_file finds an open window and does entry_mut.1 = AppRunner::launch_with_file(...), the old AppRunner is dropped. If it held a BrowsingApp with cached_photo_texture: Cell<Option<TextureId>> set, the TextureId is silently dropped — no backend.destroy_texture() is ever called, because photo_ui::draw's stale-destroy path never runs after the runner is replaced
  • Fix: before overwriting the runner, check if the old runner holds a BrowsingApp with a live texture handle and move it to a pending-destroy list, or add a cleanup(backend) hook to AppRunner/App trait that is called on replacement

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click now derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed and fullscreen WASM paths via launch_app_window_for_file
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded 0..4/16/64 replaced by call to oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — track-switching path covered by the same guard
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block guarded: state.radio_manager.state() != RadioState::Stopped && state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 added; 16-bit PNG channels downsampled to 8-bit before match arms run
• [RESOLVED] crates/oasis-app-media/src/image.rs:40 — palette PNG uses reader.output_color_type() (post-transformation) instead of info.color_type (raw header); palette → RGB/RGBA expansion now reaches the correct match arm

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still fully implemented with no call site; App trait has no handle_mouse_move hook so hovered_item is permanently None and drop-down items never render hover highlights. Remove or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• decode_png_palette regression test correctly validates the palette fix: encodes a 2×2 indexed PNG with 4-entry palette, asserts the decoded RGBA output is non-empty and the first pixel is red. The fix (output_color_type() vs info.color_type) is architecturally sound — same source the png crate uses internally for output_buffer_size().

• The texture-leak warning above is distinct from the per-frame upload churn that was fixed in iteration 1. That fix (caching within a session) is correct and remains in place; the new gap is specifically the cross-session replace path in launch_app_window_for_file.

• All audio teardown guards in media_controller.rs are consistent and correctly documented.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Claude AI Incremental Security & Correctness Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none — no new issues found in the TIER 1 file)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-app-text-editor/src/lib.rs:243 — handle_click derives title_h from self.content.cached_title_bar_height.max(18)
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:458 — LaunchAppWithFile wired in both windowed and fullscreen WASM paths
• [RESOLVED] crates/oasis-core/src/apps/runner_sdi.rs:83,90,98 — hardcoded bounds replaced by oasis_app_text_editor::hide_notepad_sdi_objects(sdi)
• [RESOLVED] crates/oasis-app-text-editor/src/render.rs:956 [UNVERIFIED - Line 956 exceeds file length (937)] — dropdown_size() removed; render_dropdown_sdi calls self.menu.dropdown_dimensions(menu) from oasis-ui
• [RESOLVED] crates/oasis-app-media/src/photo_ui.rs:42-45 — texture cached in cached_photo_texture; stale handle deferred via stale_photo_texture
• [RESOLVED] crates/oasis-app/src/media_controller.rs:49 — stop_track guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app/src/media_controller.rs:68 — track-switching path covered by the same guard
• [RESOLVED] crates/oasis-app/src/media_controller.rs:82-85 — guard confirmed in place
• [RESOLVED] crates/oasis-app/src/media_controller.rs:46-52 — radio teardown block guarded by state.tv_audio_track.is_none()
• [RESOLVED] crates/oasis-app-media/src/image.rs:41 — STRIP_16 added; 16-bit PNG channels downsampled to 8-bit before match arms run
• [RESOLVED] crates/oasis-app-media/src/image.rs:40 — palette PNGs now use reader.output_color_type() (post-transformation); decode_png_palette regression test added
• [STILL UNRESOLVED] crates/oasis-app/src/launch.rs + crates/oasis-backend-wasm/src/lib.rs (launch_app_window_for_file) — when an existing window is reused, entry_mut.1 = AppRunner::launch_with_file(...) drops the old AppRunner with no cleanup; any BrowsingApp holding a live cached_photo_texture handle loses it without calling backend.destroy_texture()

Suggestions (if any)

• crates/oasis-ui/src/menu_bar.rs (update_hover) — still fully implemented with no call site; App trait has no handle_mouse_move hook so hovered_item is permanently None and hover highlights never render. Remove or annotate as forward-looking API.

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• The palette PNG fix (output_color_type() instead of info.color_type) is correct and architecturally sound — it's the same source the png crate's output_buffer_size() uses internally. The regression test validates the fix end-to-end.

• All audio teardown guards remain consistent across stop_track, track-switching, and the radio teardown block. The three-way Radio + TV Guide + Music trade-off remains documented in comments.

• The texture leak (STILL UNRESOLVED) is low-severity in practice — it only fires when File Manager opens the same typed file extension twice in the same session — but it will accumulate across repeated file opens.

---

Generated by Claude AI (sonnet). Supplementary to human reviews.

[github-actions]

Openrouter AI Incremental General Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

• [BUG] crates/oasis-app-media/src/image.rs:118 - fit_within uses u32 arithmetic for y * h and src_y * w, which overflows for images with height/width > ~4.2M pixels

  • y iterates up to nh-1 (max 1023). If h exceeds ~4,194,303, y * h wraps around u32::MAX, producing incorrect source coordinates and garbled output
  • Fix: cast operands to u64 before multiplication: ((y as u64 * h as u64) / nh as u64) as u32

• [WARNING] crates/oasis-app/src/media_controller.rs:28 - IPC path is cleared immediately after reading, dropping queued requests

  • vfs.write(MEDIA_REQUEST_PATH, b"") wipes the buffer after parsing the first line. If the app writes play_file A then play_file B before tick() runs, only A is processed and B is silently lost
  • Fix: parse line-by-line or use a bounded queue, clearing only consumed lines

• [WARNING] crates/oasis-app-media/src/lib.rs:415 - id3v2_title parses frame sizes as big-endian u32, incompatible with ID3v2.4

  • ID3v2.4 uses syncsafe integers for frame sizes. Modern MP3s will have misinterpreted sizes, causing body_end > tag.len() and early loop exit, falling back to filename titles
  • Fix: read data[3] to detect major version and decode frame sizes accordingly (syncsafe for v2.4, big-endian for v2.3)

Previous Issues (for incremental reviews)

• [STILL UNRESOLVED] crates/oasis-app/src/launch.rs + crates/oasis-backend-wasm/src/lib.rs (launch_app_window_for_file) - Replacing an existing AppRunner drops the old instance without calling backend.destroy_texture(), leaking GPU textures when the same window is reused for different files
• [STILL UNRESOLVED] crates/oasis-ui/src/menu_bar.rs (update_hover) - Method is fully implemented but has no call site; App trait lacks handle_mouse_move, so hovered_item stays None and drop-down hover highlights never render

Suggestions (if any)

• crates/oasis-app-media/src/image.rs (fit_within) - Consider using u64 for all intermediate coordinate math (y * h, x * w, src_y * w) to guarantee correctness regardless of input dimensions
• crates/oasis-app/src/media_controller.rs (tick) - Use vfs.read [UNVERIFIED - File does not exist and is not in PR] + split('\n') to process multiple IPC commands per tick, or switch to a FIFO-style IPC path to avoid race conditions during rapid file navigation

Notes

• 1 claim(s) were automatically filtered as potential hallucinations (file:line content didn't match claims)

• The fit_within overflow is theoretically reachable but practically unlikely; typical photo dimensions and memory allocation limits will trigger OOM before u32 overflow occurs on 32-bit targets

• ID3v2.4 syncsafe limitation is a known trade-off; fixing it requires version detection or a dedicated tag parser, which is out of scope for this correctness pass

• All audio teardown guards (stop_track, play_file radio teardown, window-close shutdown) are correctly structured and consistently check state.tv_audio_track.is_none() to prevent silencing concurrent TV Guide audio

• Clock app removal, dashboard tile cleanup, and workspace/registry updates are consistent across all touched files

• AppAction losing Copy is correctly handled; only simple_app.rs required a .clone() update

---

Generated by Openrouter AI (qwen/qwen3.6-plus). Supplementary to human reviews.