feat(wasm): YouTube embed search + thumbnail grid

[AndrewAltimit]

Summary

• Wires up the WASM Video Embed app: typed queries hit Invidious for real search results, the app renders a paged 3x2 thumbnail grid in the canvas, and clicking a thumbnail plays the video in a youtube-nocookie iframe glued to the window's content rect.
• New async WasmYoutubeSearchFetcher with per-instance timeout race + crossOrigin <img> thumbnail loader that registers each thumb as a texture.
• VFS IPC protocol extended to search:<q> / play:<id> / stop; backend publishes a JSON result blob the app reads via App::refresh.
• Fixes a latent bug where TextInput/Backspace events were dropped for any non-browser, non-terminal app window.
• Iframe is soft-hidden (display: none, src preserved) when the window minimizes so YouTube playback resumes from the same playhead on restore. Full hide is still used for stop/close.

Test plan

• cargo test -p oasis-core --features wasm-youtube --lib video_embed (18 passing, including new tests for IPC protocol, click hit-testing, grid navigation, paging, JSON refresh)
• cargo clippy -p oasis-backend-wasm --target wasm32-unknown-unknown -- -D warnings clean
• cargo fmt --all -- --check clean
• Release wasm-pack build succeeds; manually exercised in Firefox: search to grid renders to click plays to minimize hides iframe to restore resumes from same point to close cleans up textures.
• Reviewer to verify CI (full workspace build + screenshot regression).

Generated with Claude Code

[github-actions]

Openrouter AI General Review

Issues (if any)

• [WARNING] crates/oasis-backend-wasm/src/lib.rs:825 - iframe_ref.show(&url, ...) is invoked every frame inside draw_windowed when youtube_active_id is set. If show() unconditionally sets the iframe src attribute, this will trigger continuous video reloads and break playback state.

  • Verify show() only updates CSS position/visibility when the URL matches the current src, or cache the last shown URL and skip the call if unchanged.

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect calculates avail_w and avail_h by subtracting fixed UI constants from cw/ch. If the window is resized below ~80px height/width, these values become negative. .max(40) prevents negative cell sizes but doesn't clamp x/y positions, causing cells to render outside the window bounds.

  • Clamp avail_w and avail_h to 0 before division, or add an early return/guard for windows below a minimum viable size.

• [WARNING] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout creates a setTimeout that is never cleared if the main fetch promise resolves first. The timer will fire later and attempt to resolve an already-resolved promise.

  • Store the timeout ID and call window.clear_timeout() inside the main promise's .then() or use AbortController/fetch cancellation to clean up resources on success.

Previous Issues (for incremental reviews)

(none)

Suggestions (if any)

• crates/oasis-backend-wasm/src/youtube.rs:268 - draw_image_with_html_image_element_and_dw_and_dh returns Result<(), JsValue> but the result is discarded with let _ = .... If the image fails to decode or the canvas is tainted (CORS mismatch), this silently fails. Consider logging the error or falling back to a placeholder texture.

• crates/oasis-core/src/apps/video_embed.rs:215 - truncate uses saturating_sub(1) and pushes …. If max_chars is 0, it returns a 1-character string "…", violating the max_chars constraint. Change to if max_chars == 0 { return String::new(); } at the top.

Notes

• The VFS IPC race condition handling in refresh() (self.results.query == parsed.query) correctly discards stale search results if the user types a new query before the previous fetch completes.
• The TextInput/Backspace fix in input_dispatch.rs correctly routes events to open_runners, resolving the dropped input bug for non-browser/terminal apps.
• Texture lifecycle management (kick_youtube_search drains/destroys old textures before allocating new ones) prevents offscreen canvas leaks across searches.

---

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

[github-actions]

Claude AI Security & Correctness Review

Issues (if any)

• [WARNING] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks the timeout handle when the fetch wins

  • set_timeout_with_callback_and_timeout_and_arguments_0 returns a timer ID that is discarded with let _. When the fetch resolves first, the timeout still fires ~6 s later and calls resolve on an already-settled Promise (harmless JS no-op), but the Closure::once_into_js closure and the timer are never cancelled — 18 leaked timers per search
  • Store the return value of set_timeout_with_callback_and_timeout_and_arguments_0 in the Promise constructor's capture and call window.clear_timeout(id) inside the fetch's then-path, or wrap both in an AbortController

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect positions cells outside window bounds when window is very small

  • When cw < 24 or ch < 76, avail_w/avail_h go negative; cell_w/cell_h are clamped to 40 but the column/row multipliers still use the clamped sizes, so x/y exceed cw/ch
  • Add let avail_w = avail_w.max(0); let avail_h = avail_h.max(0); before the division, or guard with an early return when the window is below minimum viable size

• [WARNING] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious video_id values are passed to embed_url() without character validation

  • User-typed IDs go through extract_video_id (validates alphanumeric + -_, 6–20 chars), but IDs sourced from Invidious search results (r.id.clone() in handle_click and Results confirm path) are used directly in format!("play:{video_id}") and later embed_url(id) with no equivalent check
  • A malicious/compromised Invidious instance could inject ? or & into a video ID and corrupt the embed URL query string; impact is limited to the iframe's YouTube URL but is an inconsistency with the user-input path
  • Apply the same character filter (all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') && len 6–20) to IDs returned by the fetcher before storing them in SearchResult

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:813 - show() called every frame triggers continuous video reloads — show() checks current_src before calling set_src, making it idempotent; no reload occurs when the URL is unchanged
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:195 — avail_w/avail_h negative but cell_w/cell_h clamped; x/y positions still overflow — confirmed still present, upgrading to issue above

Suggestions (if any)

• crates/oasis-backend-wasm/src/youtube.rs:268 - draw_image_with_html_image_element_and_dw_and_dh result discarded with let _; if the canvas is in an unexpected state the error is silently swallowed — log via console_log! to aid debugging, consistent with other error sites in the file

• crates/oasis-core/src/apps/video_embed.rs:215 - truncate(s, 0) returns "…" (1 char) rather than "" — unreachable through (rw / 6).max(8) in current callers, but the function's contract is subtly wrong; add if max_chars == 0 { return String::new(); } as a guard

Notes

• show() is idempotent (checks current_src before set_src) — the Openrouter per-frame reload warning does not apply
• Texture IDs start at 1 (next_texture_id: 1 in renderer init), so the thumb_tex != 0 sentinel is correct and safe
• The stale-result race condition in refresh() (self.results.query == parsed.query) correctly discards out-of-order responses
• The TextInput/Backspace fix routes events through open_runners cleanly; the None => arm is now separated from the wildcard Some(id) arm, which is correct

---

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)

• [WARNING] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout still leaks timeout handle when fetch wins

  • set_timeout_with_callback_and_timeout_and_arguments_0 return value discarded via let _; the closure fires ~6 s later anyway (no-op on settled Promise, but the Closure allocation and timer slot are never freed — 18 leaked timers per search)
  • Store the timer ID in the Promise constructor's capture, call window.clear_timeout(id) in the fetch path's then-closure

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect still positions cells outside window bounds at small sizes

  • avail_w/avail_h go negative when cw < 24 / ch < 76; cell_w/cell_h are clamped to 40 but x/y are computed as CELL_GAP + col*(cell_w + CELL_GAP), which can exceed cw/ch when the window is narrow
  • Add let avail_w = avail_w.max(0); let avail_h = avail_h.max(0); before the divisions, or early-return when dimensions are below minimum viable size

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx can go negative relative to cell_x at minimum cell width

  • dx = cell_x + rw as i32 - (r.duration.len() as i32 * 6) - 4; with min rw=40 and a 7-char duration like "5:59:59" (42px), dx = cell_x - 6, rendering the label to the left of the cell and overlapping the adjacent cell
  • Clamp: let dx = (cell_x + rw as i32 - (r.duration.len() as i32 * 6) - 4).max(cell_x);

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe_ref.show() called every frame; show() is now idempotent via current_src check
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins
• [STILL UNRESOLVED] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect positions cells outside window bounds when window is very small
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious video_id values passed to start_play without character validation; r.id.clone() goes directly to format!("play:{video_id}") with no alphanumeric+-_ length check

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:443 - refresh_video_embed is called on every runner every tick (including runners for apps that aren't Video Embed); the guard is a string comparison per frame per open window — consider keeping a typed handle or index to the video embed runner rather than linear-scanning by title

Notes

• The soft_hide / show pattern for minimize-then-restore preserving playhead state is well-structured; the pre-draw window-state check at line 745 correctly differentiates Minimized (soft hide) from None (full hide)
• TextInput/Backspace fix in input_dispatch.rs is correct and clean — the Some(id) arm is now properly separated from None
• Texture lifecycle (kick_youtube_search drains old textures before allocating new ones) is sound

---

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

[AndrewAltimit]

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: e2a7268

Fixed Issues

• crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout no longer leaks the setTimeout closure when the fetch wins. The timer ID is now captured into an Rc<RefCell<Option<i32>>> that the constructor closure populates, and the fast path calls window.clear_timeout_with_handle(id) after the race resolves.
• crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious-sourced videoId values are now validated through a new is_valid_video_id helper (alphanumeric + -/_, length 6–20) before being stored in YoutubeHit. Matches the extract_video_id filter so a malicious Invidious instance can't inject ?/& into the embed URL. Added valid_video_ids_accepted and invalid_video_ids_rejected tests.
• crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect now clamps avail_w / avail_h to 0 before the per-axis division, so very small windows can no longer produce negative intermediates that the .max(40) floor would mask while leaving x/y unbounded.
• crates/oasis-core/src/apps/video_embed.rs:705 — truncate returns an empty string when max_chars == 0 instead of "…", fixing the contract violation flagged by both reviewers.
• crates/oasis-core/src/apps/video_embed.rs:677 — Duration label dx is now .max(cell_x) so a long duration string in a tiny cell can't render past the cell's left edge into the neighbouring cell.

Ignored Issues

• crates/oasis-backend-wasm/src/lib.rs:825 (per-frame iframe reload) — already fixed in prior iteration; iframe.show() is idempotent via the current_src != url check at line 75.
• crates/oasis-backend-wasm/src/youtube.rs:268 (silent let _ = draw_image_*) — suggestion only. Behaviour matches the documented "onerror: silently ignore — the canvas stays transparent" contract immediately above; logging here would just spam the console for every CORS-tainted thumbnail.
• crates/oasis-backend-wasm/src/lib.rs:443 (linear scan for video_embed runner per frame) — suggestion only. open_runners is small (one entry per open window) and the comparison is a short-string match; a typed handle is structurally invasive for no measured win.

Deferred to Human

• (none)

Notes

• cargo check -p oasis-core --features wasm-youtube and cargo check -p oasis-backend-wasm --target wasm32-unknown-unknown both succeed.
• cargo test -p oasis-core --features wasm-youtube --lib video_embed — 18/18 tests pass after the changes.
• The pre-existing oasis-skin::log_validation_warnings dead-code warning seen during clippy is unrelated to these changes.

---

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-backend-wasm/src/youtube.rs:150 - Empty result set from an instance is treated as a failure, causing all 4 instances to be tried before a "no results" query resolves
  • Ok(hits) if !hits.is_empty() => return Ok(hits) falls through to last_err = format!("{base}: empty response"), meaning a legitimate zero-result search exhausts all instances and returns an error instead of an empty success
  • Remove the if !hits.is_empty() guard and let the caller's SearchStatus::Ready + empty results vec display "No results." cleanly; the fallback to the next instance is only useful on transport/parse errors

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fetch-win path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect positions cells outside window bounds at small sizes — avail_w/avail_h now clamped with .max(0) before division
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance before storing into YoutubeHit
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - show() called every frame (previously resolved via current_src idempotency check)

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:438 - refresh_video_embed is called on every open runner every tick with a string comparison guard; a typed enum or ID field on AppRunner would eliminate the per-tick allocation-free but still O(n) scan (carried from last review, still low priority given typical window counts)
• crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) allocates a new String every frame while a video is playing; cache the last youtube_active_id + URL pair or compute it only when youtube_active_id changes since iframe.show() skips the DOM write anyway

Notes

• paint_canvas_from_url safely avoids premature GC: the onload closure captures img_for_load, keeping the HtmlImageElement reachable through its own .onload property until the closure fires — no CORS or lifecycle issue
• SearchResult::thumb_w/thumb_h are serialized through VFS but unused by draw_results_grid (which derives thumbnail size from cell geometry via thumb_rect); harmless dead fields
• The stale-result guard in refresh() (state == Results && query == parsed.query) correctly prevents out-of-order responses from clobbering in-flight or completed navigation state

---

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 new)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fetch-win path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect positions cells outside window bounds at small sizes — avail_w/avail_h now clamped with .max(0) before division
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:677 — Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 — truncate(s, 0) returns "…" instead of "" — early return added
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 — iframe.show() called every frame (idempotent via current_src check, previously confirmed)
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 — Empty result set from an instance is treated as a failure
  • Ok(hits) if !hits.is_empty() => return Ok(hits) still falls through to last_err = format!("{base}: empty response"), exhausting all 4 instances before returning an error for a legitimate zero-result query
  • Remove the if !hits.is_empty() guard; let the caller render "No results." from an empty Ready state

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:443 — refresh_video_embed is called on every open runner every tick with a string title comparison; low-cost given typical window counts but a typed discriminant on AppRunner would eliminate the per-tick scan entirely (carried from prior review, still low priority)

Notes

• paint_canvas_from_url GC safety is sound: the onload closure captures img_for_load (clone), which is stored as the handler on the DOM element; the browser keeps the element alive through the pending load, so no premature collection
• poll_youtube_search borrow pattern is correct under NLL: the immutable borrow of pending_youtube_search through fetcher ends after fetcher.take_results(), before the = None assignment
• The stale-result guard in refresh() (state == Results && query == parsed.query) correctly prevents out-of-order responses from clobbering navigation state

---

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-backend-wasm/src/youtube.rs:150 - Empty result set from an instance is treated as a failure, causing all 4 instances to be tried before a "no results" query resolves

  • Ok(hits) if !hits.is_empty() => return Ok(hits) falls through to last_err = format!("{base}: empty response"), meaning a legitimate zero-result search exhausts all instances and returns an error instead of an empty success
  • Remove the if !hits.is_empty() guard and return Ok(hits) immediately; let the caller's SearchStatus::Ready + empty results vec display "No results." cleanly

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcodes title_h = 20 for Y-offset calculation, but draw_results_grid uses at.app.title_bar_height (which varies by skin)

  • This will misalign the thumbnail grid vertically if the active skin's title bar isn't exactly 20px
  • Pass title_bar_height as a parameter to cell_rect instead of hardcoding it

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect still calculates x/y positions that can exceed cw/ch when the window is very small

  • Clamping avail_w/avail_h to 0 prevents negative dimensions, but cell_w/cell_h are floored to 40, so x/y multipliers still push cells outside the window bounds
  • Add an early return or clamp final x/y to 0 when cw/ch fall below a minimum viable threshold

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - show() called every frame triggers continuous video reloads — show() checks current_src before calling set_src, making it idempotent
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:195 - cell_rect negative avail_w/avail_h — now clamped with .max(0) before division
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle — timer ID now captured and clear_timeout_with_handle called on fetch win
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious video_id unvalidated — is_valid_video_id filter applied before storing
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative — .max(cell_x) applied

Suggestions (if any)

• crates/oasis-backend-wasm/src/youtube.rs:268 - draw_image_with_html_image_element_and_dw_and_dh result discarded with let _; if the canvas is tainted (CORS mismatch) or decoding fails, the error is silently swallowed. Consider logging via console_log! to aid debugging, consistent with other error sites in the file

Notes

• The VFS IPC race condition guard (self.results.query == parsed.query) correctly prevents stale search results from overwriting newer queries
• Texture lifecycle management in kick_youtube_search correctly drains and destroys old textures before allocating new ones, preventing offscreen canvas leaks
• The soft_hide / show pattern for minimize/restore correctly preserves iframe playback state without reloading the video
• TextInput/Backspace routing to open_runners is correctly separated from browser/terminal arms, fixing the dropped input bug

---

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

[AndrewAltimit]

Review Response Agent (Iteration 2)

Status: Changes committed, pushing...

Commit: df793ae

Fixed Issues

• crates/oasis-backend-wasm/src/youtube.rs:139 — search() no longer falls through to the next Invidious instance when an instance returns a successful but empty hit list. The Ok(hits) if !hits.is_empty() guard has been removed; an empty Ok(hits) is now returned directly so a legitimate zero-result query resolves cleanly to SearchStatus::Ready with an empty results vec instead of exhausting all 4 instances and surfacing a transport error. Both Claude (security) and Openrouter flagged this in the iteration-2 incremental review.
• crates/oasis-core/src/apps/video_embed.rs — cell_rect no longer hardcodes title_h = 20. It now takes title_h: i32 as a parameter, and a new cached_title_bar_h field on VideoEmbedApp is updated in update_sdi from at.app.title_bar_height (which varies 16–36 px across the shipped skins, e.g. retro-cga=16, gnome=36, balatro=28). draw_results_grid passes the live theme value, and handle_click → hit_test_grid uses the cached value so the click hit zones stay in sync with what was rendered. Updated the click_in_grid_starts_playback test (cell_rect(640, 400, 22, 0)) to match. Flagged by Openrouter in the iteration-2 review.

Ignored Issues

• crates/oasis-backend-wasm/src/youtube.rs:230 (timer leak), youtube.rs:184 (video_id validation), video_embed.rs:258 (avail_w/avail_h clamp), video_embed.rs:705 (truncate(0)), video_embed.rs:677 (duration label dx), and lib.rs:825 (per-frame iframe reload) — all already fixed in prior iteration; the iteration-2 reviews acknowledged them as [RESOLVED].
• crates/oasis-core/src/apps/video_embed.rs:258 (Openrouter "x/y still exceed cw/ch") — Claude marked this [RESOLVED] and Openrouter's residual concern is at sub-viable window sizes (cw < ~24 px). With avail_w/avail_h now clamped to 0 and cell sizes floored at 40, the only remaining overflow is purely cosmetic at window sizes too small to render meaningful UI; the skin/window manager clips out-of-bounds draws. Adding an early-return "window too small" path is a feature, not a bug fix.
• crates/oasis-backend-wasm/src/lib.rs:443 (linear scan for video_embed runner per frame) and lib.rs:818 (embed_url(id) allocates per frame) — both labelled as suggestions only. open_runners is small, iframe.show() is already idempotent via the current_src check at line 75 (so the per-frame embed_url allocation never reaches the DOM), and a typed handle is structurally invasive for no measured win.
• crates/oasis-backend-wasm/src/youtube.rs:268 (silent let _ = draw_image_*) — suggestion only; logging would spam the console for every CORS-tainted thumbnail and the documented onerror contract is "silently ignore — the canvas stays transparent". Already noted as ignored in iteration 1.

Deferred to Human

• (none)

Notes

• cargo check -p oasis-core --features wasm-youtube and cargo check -p oasis-backend-wasm --target wasm32-unknown-unknown both succeed.
• cargo test -p oasis-core --features wasm-youtube --lib video_embed — 18/18 tests pass.
• The pre-existing oasis-skin::log_validation_warnings dead-code warning seen during clippy is unrelated to these changes (carried over from iteration 1).

---

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-backend-wasm/src/youtube.rs:126 - Sequential instance fallback creates up to ~48s worst-case search latency
  • search() tries each of 4 instances in series; each instance can spend 2 × PER_INSTANCE_TIMEOUT_MS (one for fetch, one for text body), so 4 × 2 × 6s = 48s maximum if all instances are slow but reachable
  • Fix: fan out to all instances concurrently with Promise.race [UNVERIFIED - File does not exist and is not in PR] (or js_sys::Promise::race across all 4 fetch futures) and cancel the losers; in the common case where the first instance works the latency drops to a single round trip

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fetch-win path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect positions cells outside window bounds — avail_w/avail_h now clamped with .max(0) before division
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — now a parameter populated from at.app.title_bar_height and cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure exhausting all instances — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative at minimum cell width — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe.show() called every frame — idempotent via current_src check (confirmed carried from prior iteration)

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:440 - refresh_video_embed is called every tick on every open runner with a string title comparison guard; a typed discriminant or enum tag on AppRunner would eliminate the O(n) scan entirely (low priority given typical window counts, carried from prior review)
• crates/oasis-core/src/apps/video_embed.rs - SearchResult::thumb_w/thumb_h are serialized into the VFS blob and deserialized on every refresh call but are never read by draw_results_grid (which derives thumbnail geometry from cell layout); removing them halves the serialized result struct size

Notes

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

• WasmYoutubeSearchFetcher::take_results takes &self (interior mutability via Rc<RefCell>) — unusual API shape but correct given single-threaded WASM context

• cached_title_bar_h defaults to 22 in VideoEmbedApp::new; if a click arrives before the first update_sdi call, hit-testing uses the wrong offset — in practice update_sdi runs each frame before input is processed, so this is safe, but it's a fragile implicit ordering

• The stale-result guard in refresh() (checks both state == Results and results.query == parsed.query) correctly discards out-of-order and post-cancel responses

• is_valid_video_id and extract_video_id now enforce the same 6–20 char alphanumeric+-_ constraint on both user-typed and Invidious-sourced IDs — the paths are consistent

---

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-backend-wasm/src/youtube.rs:126 - Sequential instance fallback creates up to ~48s worst-case search latency
  • search() for base in INSTANCES loop still awaits each try_instance call serially; with 4 instances × 2 fetch/body timeouts × 6s each = 48s maximum if all instances are reachable-but-slow
  • Fan out to all instances concurrently with js_sys::Promise::race across 4 parallel fetch futures; cancel losers via AbortController; happy-path latency drops to one round trip

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID captured in Rc<RefCell<Option<i32>>>, clear_timeout_with_handle called on the fast path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect positions cells outside window bounds — avail_w/avail_h clamped with .max(0) before division
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — now a parameter; callers pass at.app.title_bar_height and cached_title_bar_h keeps click hit zones in sync
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure exhausting all instances — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 - Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance before storing into YoutubeHit
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe.show() called every frame — idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 - truncate(s, 0) returns "…" — early return for max_chars == 0 added

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) allocates a new String every frame while a video is active; cache the last (id, url) pair and skip recomputation when the id is unchanged — iframe.show() suppresses the DOM write anyway, but the allocation still occurs every tick

Notes

• All security-relevant issues from prior iterations are resolved: timer leak, video ID injection, cell OOB layout, truncate contract
• One lingering correctness/UX warning (youtube.rs:126 sequential fallback) was previously flagged and has not been addressed; no new security or correctness issues introduced in this iteration
• is_valid_video_id and extract_video_id now enforce the same 6–20 char alphanumeric+-_ constraint on both user-typed and Invidious-sourced IDs — the two input paths are consistent

---

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-core/src/apps/video_embed.rs:258 - cell_rect calculates x/y positions that exceed window bounds at very small sizes
  • avail_w/avail_h are clamped to 0, but cell_w/cell_h are floored to 40. When cw < ~50, x for column 1 becomes 52, rendering outside the window. The backend clips it, but breaks layout assumptions and can cause overlapping UI elements.
  • Add an early return or clamp final x/y to 0 when cw/ch fall below a minimum viable threshold, or remove the .max(40) floor when space is insufficient.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fast path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — now a parameter populated from at.app.title_bar_height and cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure exhausting all instances — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe.show() called every frame — idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 - truncate(s, 0) returns "…" — early return for max_chars == 0 added

Suggestions (if any)

• crates/oasis-backend-wasm/src/youtube.rs:268 - draw_image_with_html_image_element_and_dw_and_dh result discarded with let _; if the canvas context is lost or tainted (CORS mismatch), the error is silently swallowed. Consider logging via console_log! to aid debugging, consistent with other error sites in the file.

Notes

• The VFS IPC race condition guard (self.results.query == parsed.query) correctly prevents stale search results from overwriting newer queries.
• Texture lifecycle management in kick_youtube_search correctly drains and destroys old textures before allocating new ones, preventing offscreen canvas leaks across searches.
• The soft_hide / show pattern for minimize/restore correctly preserves iframe playback state without reloading the video.
• TextInput/Backspace routing to open_runners is correctly separated from browser/terminal arms, fixing the dropped input bug for non-browser/terminal apps.
• Sequential Invidious instance fallback remains a potential UX latency issue (~48s worst-case), but is a known architectural trade-off for simplicity.

---

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

[AndrewAltimit]

Review Response Agent (Iteration 3)

Status: Changes committed, pushing...

Commit: 74d0ad6

Fixed Issues

• crates/oasis-core/src/apps/video_embed.rs:262 — cell_rect no longer floors cell_w/cell_h at 40 px. The natural fitting size avail_w / GRID_COLS is now used directly, so a column at the right edge of a very small window can never produce an x past cw. The .max(40) floor was the actual cause of the OOB rendering Openrouter has flagged across three iterations: when avail_w < 120 the floor pushed cell_w above the natural fit, so col*(cell_w+CELL_GAP) exceeded the window's right edge. Cells smaller than ~40 px are illegible but cosmetic — preferable to bleeding into a neighbouring window. Existing tests still use cell_rect(640, 400, ...), well above the threshold, and all 18 pass unchanged.
• crates/oasis-core/src/apps/video_embed.rs:42 — Removed unused thumb_w/thumb_h fields from SearchResult. Verified via grep that they were only set by oasis-backend-wasm/src/lib.rs and asserted in one test; draw_results_grid derives display thumbnail size from cell layout via thumb_rect. Updated the writer in crates/oasis-backend-wasm/src/lib.rs:1008 and the refresh_loads_results_from_vfs test to match. Flagged as a quality suggestion by Claude.

Ignored Issues

• crates/oasis-backend-wasm/src/youtube.rs:126 (sequential Invidious fallback ~48s worst case) — the lingering UX warning Claude has flagged across iterations. A correct parallel fan-out requires js_sys::Promise::race over four parallel fetch futures plus AbortController-based cancellation of the losers; that's a meaningful architectural change to a working code path. The existing per-instance PER_INSTANCE_TIMEOUT_MS=6_000 ms budget already bounds the worst case, the happy path (first instance reachable) is one round trip, and Openrouter explicitly classifies it as "a known architectural trade-off for simplicity". Listing as deferred to human below since it's a defensible improvement, not a bug.
• crates/oasis-backend-wasm/src/lib.rs:818 (embed_url(id) per-frame String allocation) — suggestion only; iframe.show() is idempotent via the current_src check at line 75 so the per-frame allocation never reaches the DOM, and caching the (id, url) pair is structurally invasive for an unmeasured win.
• crates/oasis-backend-wasm/src/lib.rs:440 (refresh_video_embed per-tick linear scan) — suggestion only, carried from prior iterations. open_runners has one entry per open window; a typed-handle refactor is invasive.
• crates/oasis-backend-wasm/src/youtube.rs:268 (silent let _ = draw_image_*) — suggestion only, carried from prior iterations. The documented onerror contract immediately above is "silently ignore — the canvas stays transparent"; logging would spam the console for every CORS-tainted thumbnail.

Deferred to Human

• crates/oasis-backend-wasm/src/youtube.rs:126 — Parallel Invidious fan-out (js_sys::Promise::race + AbortController) to drop the worst-case 48 s sequential latency. Defensible improvement but architecturally invasive and not a correctness bug.

Notes

• cargo check -p oasis-core --features wasm-youtube and cargo check -p oasis-backend-wasm --target wasm32-unknown-unknown both pass.
• cargo test -p oasis-core --features wasm-youtube --lib video_embed — 18/18 tests pass.
• cargo clippy -p oasis-core --features wasm-youtube -- -D warnings — clean.

---

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-core/src/apps/video_embed.rs:258 - cell_rect can still produce x/y positions outside window bounds at very small sizes (e.g., cw < 18 or ch < title_h + 36)

  • Clamping avail_w/avail_h to 0 prevents negative dimensions, but the CELL_GAP multipliers in x and y calculations are unbounded. When the window is narrower than the total gap padding, cells render past the right/bottom edges.
  • The backend clips these draws, so it's purely cosmetic, but it breaks layout assumptions and can overlap adjacent windows. Consider an early return or clamping final x/y to cw/ch if dimensions fall below a viable threshold.

• [WARNING] crates/oasis-backend-wasm/src/lib.rs:443 - refresh_video_embed performs a string comparison on every open runner every frame

  • runner.refresh_video_embed(&self.vfs) is called for self.app_runner and all self.open_runners each tick. The guard if self.title != "Video Embed" triggers an O(n) string scan per frame.
  • While negligible for typical window counts, a typed enum discriminant or cached runner index would eliminate the per-tick allocation-free scan entirely.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fast path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect positions cells outside window bounds — avail_w/avail_h now clamped with .max(0) before division (residual cosmetic OOB noted above)
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure exhausting all instances — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious-sourced video_id values unvalidated — is_valid_video_id filter applied in try_instance
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe.show() called every frame — idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 - truncate(s, 0) returns "…" — early return for max_chars == 0 added

Suggestions (if any)

• crates/oasis-core/src/apps/video_embed.rs:410 - handle_click relies on self.cached_title_bar_h (defaults to 22) for hit-testing. If a click arrives before the first update_sdi call, hit zones will be slightly misaligned. In practice update_sdi runs before input processing, but adding a fallback to at.app.title_bar_height or asserting cached_title_bar_h > 0 would harden the implicit ordering.

Notes

• The VFS IPC race condition guard (self.results.query == parsed.query) correctly prevents stale search results from overwriting newer queries.
• Texture lifecycle management in kick_youtube_search correctly drains and destroys old textures before allocating new ones, preventing offscreen canvas leaks.
• The soft_hide / show pattern for minimize/restore correctly preserves iframe playback state without reloading the video.
• TextInput/Backspace routing to open_runners is correctly separated from browser/terminal arms, fixing the dropped input bug for non-browser/terminal apps.

---

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)

(none new in the changed files)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>> and clear_timeout_with_handle called on the fast path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect OOB at very small sizes — .max(40) floor removed in iteration 3; cell_w = 0 when avail_w = 0, so no cell content renders past cw; cosmetically degraded at sub-viable sizes but no longer overflows neighbours
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect hardcoded title_h = 20 — now a parameter populated from at.app.title_bar_height and cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 — Empty result set treated as failure — !is_empty() guard removed
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 — Duration label dx goes negative — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious-sourced video_id unvalidated — is_valid_video_id applied in try_instance
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs — thumb_w/thumb_h dead serialized fields — removed from SearchResult; writer and test updated
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 — Sequential Invidious fallback; search() still awaits each try_instance serially (~48s worst-case). Explicitly deferred to human in iteration 3 — no change since last review.

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:818 — embed_url(id) allocates a new String every frame while a video is active; iframe.show() suppresses the DOM write via current_src check, but the allocation still occurs every tick. Cache the last (id, url) pair and skip recomputation when the id is unchanged.

Notes

• All correctness and security issues from prior iterations are resolved in the diff
• The soft_hide / show pattern is correctly wired: pre-draw state check at ~line 745 differentiates Minimized (soft hide, preserves playhead) from None (full hide on close)
• cached_title_bar_h defaults to 22 before the first update_sdi call; in practice update_sdi runs each frame before input is dispatched, so click hit zones are safe
• Sequential fallback deferred to human is the only open item; everything else is clean

---

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 new)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout leaks timeout handle when fetch wins — timer ID now captured in Rc<RefCell<Option<i32>>>, clear_timeout_with_handle called on the fetch-win path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect hardcoded title_h = 20 — now a parameter populated from at.app.title_bar_height and cached in cached_title_bar_h
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect OOB at very small sizes — .max(40) floor removed; cell_w = 0 when avail_w = 0, cells collapse gracefully without overflowing into neighbours
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 — Empty result set treated as failure — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 — Duration label dx goes negative at minimum cell width — .max(cell_x) applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious-sourced video_id values unvalidated — is_valid_video_id applied in try_instance before storing into YoutubeHit
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 — truncate(s, 0) returns "…" — early return for max_chars == 0 added
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 — iframe.show() called every frame — idempotent via current_src check
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 — Sequential Invidious fallback; search() still awaits each try_instance serially (~48s worst-case) — explicitly deferred to human per iteration 3 agent notes; no change since last review

Suggestions (if any)

• crates/oasis-backend-wasm/src/youtube.rs:271 — Closure::once_into_js for onerror allocates a JS closure that is never explicitly freed; its lifetime is tied to the HtmlImageElement's GC cycle. This is idiomatic wasm-bindgen for fire-once callbacks so it's fine as-is, but a no-op onerror could be set to None instead to avoid the allocation entirely.

Notes

• paint_canvas_from_url GC safety is sound: the browser's resource fetcher holds an internal reference to the HtmlImageElement while a load is pending, so the element survives until onload/onerror fires even though Rust drops the img handle at function exit
• is_valid_video_id and extract_video_id now enforce identical constraints (alphanumeric + -_, length 6–20) on both user-typed and Invidious-sourced IDs — the two input paths are consistent
• poll_youtube_search borrow safety is sound: the async task's borrow_mut() releases before the main-thread is_ready() / take_results() sequence runs (single-threaded WASM event loop)
• The Openrouter "cell_rect still OOB" concern from iteration 3 does not apply to the current code: without the .max(40) floor, cell_w = 0 when avail_w = 0, so cells render as zero-width and the gap-only x positions are within the CELL_GAP * (col+1) range, which is beyond cw only when cw < CELL_GAP * 4 = 24 — a sub-viable window size where the backend clips anyway

---

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

[AndrewAltimit]

Review Response Agent (Iteration 4)

Status: Changes committed, pushing...

Commit: 7abb936

Fixed Issues

• crates/oasis-backend-wasm/src/lib.rs:818 — embed_url(id) no longer allocates a fresh String every frame while a video is active. Added a youtube_active_url: Option<String> cache field on OasisWasm that is computed once in handle_video_embed_request on the play:<id> path and cleared on stop/empty. The per-frame iframe glue in tick() now captures and reuses the cached URL directly. Flagged as a Suggestion by Claude (security + quality) reviews on the latest commit 74d0ad6.

Ignored Issues

• crates/oasis-backend-wasm/src/youtube.rs:230 (timer leak), youtube.rs:184 (Invidious video_id validation), video_embed.rs:258 (avail_w/avail_h clamp), video_embed.rs:705 (truncate(0)), video_embed.rs:677 (duration label dx), video_embed.rs:258 (hardcoded title_h), youtube.rs:150 (empty result fall-through), lib.rs:825 (per-frame iframe reload), video_embed.rs:42 (dead thumb_w/thumb_h), video_embed.rs:262 (.max(40) floor) — all already fixed in iterations 1–3; both Claude reviews on 74d0ad6 mark them [RESOLVED] and report "no new issues".
• crates/oasis-core/src/apps/video_embed.rs:262 (Openrouter "x/y still exceed cw/ch at sub-viable sizes") — already addressed in iteration 3 by removing the .max(40) floor. With the floor removed, cell_w = 0 when avail_w = 0, so cells render zero-width — no pixels are drawn outside the window. Claude's iteration-3 review confirms: "cells collapse gracefully without overflowing into neighbours". The residual concern is at sub-viable window widths (cw < 24) where the backend clips anyway. Adding an early-return is a feature, not a bug fix.
• crates/oasis-backend-wasm/src/lib.rs:443 (per-tick refresh_video_embed linear scan with title string compare) — Suggestion only; carried across iterations 1–4 and explicitly ignored each time. open_runners is small (one entry per open window) and the comparison is a &'static str vs String match — sub-microsecond. A typed handle is structurally invasive for no measured win.
• crates/oasis-backend-wasm/src/youtube.rs:271 (Closure::once_into_js for onerror allocates a JS closure not explicitly freed) — Claude itself classifies this as "idiomatic wasm-bindgen for fire-once callbacks so it's fine as-is". Suggestion only.
• crates/oasis-core/src/apps/video_embed.rs:441 (cached_title_bar_h fallback hardening) — Suggestion only. Defaults to 22 (mid-range across the 16–36 px shipped skins). In practice update_sdi runs before input every frame, so handle_click always sees a populated value before any user interaction. handle_click has no ActiveTheme access for a runtime fallback; a debug assert would fire only on test paths.

Deferred to Human

• crates/oasis-backend-wasm/src/youtube.rs:126 — Parallel Invidious fan-out (js_sys::Promise::race + AbortController cancellation) to drop the worst-case ~48 s sequential latency. Carried from iteration 3 as deferred. Both Claude reviews on 74d0ad6 mark this as [STILL UNRESOLVED] but explicitly "deferred to human per iteration 3 agent notes". Architecturally invasive change to a working code path; not a correctness bug.

Notes

• cargo check -p oasis-core --features wasm-youtube and cargo check -p oasis-backend-wasm --target wasm32-unknown-unknown both succeed.
• cargo test -p oasis-core --features wasm-youtube --lib video_embed — 18/18 tests pass.
• The pre-existing oasis-skin::log_validation_warnings dead-code warning is unrelated to these changes (carried over from iteration 1).
• Diff: 1 file changed, +13 / −4 lines (crates/oasis-backend-wasm/src/lib.rs).

---

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)

(none)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout timer leak — Rc<RefCell> capture + clear_timeout_with_handle on fetch-win path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect OOB at small sizes — .max(40) floor removed; cells collapse to 0 width/height gracefully
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — now parameterized via cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure — !is_empty() guard removed
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx negative — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 - Invidious video_id unvalidated — is_valid_video_id filter applied
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - Per-frame iframe reload — show() idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:705 - truncate(0) returns "…" — early return added
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) per-frame allocation — cached in youtube_active_url
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:42 - Dead thumb_w/thumb_h fields — removed from SearchResult
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 - Sequential Invidious fallback (~48s worst-case) — explicitly deferred to human; not a correctness bug

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:443 - refresh_video_embed linear scan per tick across open_runners with string comparison; a typed discriminant on AppRunner would eliminate the O(n) check, though impact is negligible for typical window counts
• crates/oasis-backend-wasm/src/youtube.rs:271 - onerror uses Closure::once_into_js which allocates a JS closure tied to the HtmlImageElement GC cycle; setting onerror to None or a static no-op could avoid the allocation, but current approach is idiomatic

Notes

• All correctness, security, and layout issues from prior iterations are resolved in the diff
• poll_youtube_search → VFS write → refresh_video_embed → VFS read pipeline is synchronous within the same tick, preventing stale-state races
• kick_youtube_search correctly drops in-flight fetchers on duplicate queries; the orphaned async task's Rc<RefCell> is cleaned up when the task completes
• soft_hide / show iframe lifecycle correctly preserves YouTube playhead state across minimize/restore cycles
• is_valid_video_id and extract_video_id now enforce identical 6–20 char alphanumeric+-_ constraints across both user-typed and API-sourced paths

---

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)

(none new)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout leaks timeout handle when fetch wins — Rc<RefCell<Option<i32>>> capture + clear_timeout_with_handle confirmed present in the diff
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect OOB at small sizes — .max(40) floor removed; cell_w = avail_w / GRID_COLS (no floor), collapses to zero-width at sub-viable sizes
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect hardcoded title_h = 20 — now a title_h: i32 parameter sourced from at.app.title_bar_height and cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 — empty result set treated as failure — !is_empty() guard removed; Ok(hits) returned unconditionally
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 — duration label dx negative at min cell width — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious video_id unvalidated — is_valid_video_id filter applied in try_instance before storing into YoutubeHit
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 — iframe.show() called every frame — idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:42 — dead thumb_w/thumb_h fields — removed from SearchResult; writer and test updated
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 — embed_url(id) per-frame String allocation — youtube_active_url: Option<String> cache added; URL computed once in handle_video_embed_request on play:<id> path, cleared on stop
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 — sequential Invidious fallback (~48s worst-case) — explicitly deferred to human; no change

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:779 — self.youtube_active_url.clone() still allocates one String per frame while a video is active; the clone is required to move the value into the draw closure while iframe_ref holds a mutable borrow. A Rc<String> for the cached URL would make the per-frame clone a cheap pointer bump instead of a heap allocation. Low priority given iframe.show() suppresses the DOM write anyway.

Notes

• The youtube_active_url cache is correctly wired: set in handle_video_embed_request on play:<id>, cleared on stop/empty, and youtube_active_id tracks the same lifecycle — the two fields stay in sync
• Pre-draw soft-hide logic correctly differentiates Minimized (preserve playhead via soft_hide) from None/window-gone (full hide) before the draw loop visits windows
• poll_youtube_search borrow pattern is sound under NLL: the immutable borrow through fetcher.as_ref() ends after take_results(), before self.pending_youtube_search = None
• All correctness, security, and layout issues from prior iterations are resolved; the only open item is the explicitly human-deferred sequential Invidious fallback

---

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)

• [BUG] crates/oasis-backend-wasm/src/input_dispatch.rs (close-window path, around line 262) - youtube_active_url is not cleared when the Video Embed window is closed
  • The close-window path sets self.youtube_active_id = None and hides the iframe, but omits self.youtube_active_url = None. If the user reopens a Video Embed window after closing one that was playing, the draw loop finds youtube_active_url is still Some(stale_url) and calls iframe_ref.show(stale_url, ...) on the new window's first draw, causing the previous video's iframe to appear before the user has searched for anything
  • Add self.youtube_active_url = None; alongside self.youtube_active_id = None; in the close-window block

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - iframe.show() called every frame — show() is idempotent via current_src check; youtube_active_url cache also eliminates reallocation
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle — Rc<RefCell<Option<i32>>> capture + clear_timeout_with_handle on fetch-win path
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:246 - Invidious video_id unvalidated — is_valid_video_id filter applied in try_instance
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect OOB at small sizes — .max(40) floor removed; avail_w/avail_h clamped to 0 before division
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — now a parameter from at.app.title_bar_height, cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result treated as failure — !is_empty() guard removed
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx negative — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) per-frame allocation — youtube_active_url: Option<String> cache added
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 - Sequential Invidious fallback (~48s worst-case) — explicitly deferred to human per iteration 3

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:779 - self.youtube_active_url.clone() still allocates one String per frame while a video is active; an Rc<String> would make the per-frame clone a pointer bump (carried from prior review, low priority since iframe.show() suppresses the DOM write anyway)

Notes

• The soft_hide/show minimize-preserve-playhead pattern is correctly wired: the pre-draw state check gates on youtube_active_id.is_some() and differentiates Minimized (soft_hide) from None/window-gone (full hide)
• TextInput/Backspace routing fix in input_dispatch.rs is correct — the Some(id) arm cleanly separates from None
• is_valid_video_id and extract_video_id now enforce identical 6–20 char alphanumeric+-_ constraints on both user-typed and Invidious-sourced IDs

---

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

[AndrewAltimit]

Review Response Agent (Iteration 5)

Status: Changes committed, pushing...

Commit: 521fed0

Fixed Issues

• crates/oasis-backend-wasm/src/input_dispatch.rs:267 — Stale youtube_active_url after closing the Video Embed window. The close-window block cleared youtube_active_id but not youtube_active_url, so reopening Video Embed would briefly show the previously-played video's iframe on the new window's first draw (the draw loop at lib.rs:828 keys on youtube_active_url.is_some() to call iframe.show()). Added self.youtube_active_url = None; alongside the existing youtube_active_id = None;, mirroring the symmetric clear that handle_video_embed_request does on the stop path at lib.rs:940-941. Flagged as [BUG] by Claude Security Review on commit 7abb936.

Ignored Issues

• crates/oasis-backend-wasm/src/youtube.rs:126 (sequential Invidious fallback ~48s worst case) — [STILL UNRESOLVED] per Claude/Openrouter, but explicitly deferred to human across iterations 3 and 4. Not a correctness/security bug; the per-instance 6s timeout already bounds the worst case. Architectural rework (Promise::race + AbortController) below the iter-5 high-severity threshold.
• crates/oasis-backend-wasm/src/lib.rs:779 (per-frame youtube_active_url.clone() could be Rc<String>) — Suggestion only, explicitly low-priority per the reviewer ("iframe.show() suppresses the DOM write anyway"). Below threshold.
• crates/oasis-backend-wasm/src/lib.rs:443 (per-tick refresh_video_embed linear scan with title compare) — Suggestion carried across all 4 iterations and explicitly ignored each time. open_runners is small and the comparison is sub-microsecond.
• crates/oasis-backend-wasm/src/youtube.rs:271 (Closure::once_into_js for onerror not explicitly freed) — Claude itself classifies as "idiomatic wasm-bindgen for fire-once callbacks so it's fine as-is".
• crates/oasis-core/src/apps/video_embed.rs:441 (cached_title_bar_h fallback hardening) — Suggestion only; update_sdi always runs before input dispatch in practice.
• All [RESOLVED] items from prior iterations (timer leak, video_id validation, cell_rect OOB, hardcoded title_h, empty result fall-through, duration label dx, per-frame iframe reload, truncate(0), dead thumb_w/thumb_h, .max(40) floor, embed_url per-frame allocation) — already fixed in iterations 1–4; both Claude reviewers and Openrouter on commit 7abb936 mark them [RESOLVED].

Deferred to Human

• crates/oasis-backend-wasm/src/youtube.rs:126 — Parallel Invidious fan-out (js_sys::Promise::race over 4 concurrent fetch futures + AbortController cancellation of losers). Working code path with bounded timeouts; meaningful architectural change, not a bug fix.

Notes

• The pre-existing staged cargo fmt whitespace tidy in lib.rs:952 (single-line self.youtube_active_url = Some(...)) is unrelated to the bug fix and is preserved as-is.
• The fix completes the symmetry: handle_video_embed_request's stop path (lib.rs:940-941) clears both fields, the close-window path now does too, and the play:<id> path (lib.rs:952-953) sets both. The three lifecycle entry points are now consistent.

---

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)

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:267 — youtube_active_url not cleared on Video Embed window close — self.youtube_active_url = None is now present alongside self.youtube_active_id = None and texture drain in the close-window block; the stop path in handle_video_embed_request and the close-window path are now symmetric
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 — embed_url(id) per-frame String allocation — youtube_active_url: Option<String> cache computed once in handle_video_embed_request on play:<id>; the per-frame draw closure clones that cached value, not a fresh format string
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 — race_with_timeout timer leak — Rc<RefCell<Option<i32>>> capture + clear_timeout_with_handle confirmed in current code
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 — empty result set treated as failure — Ok(hits) returned unconditionally; !is_empty() guard absent in current code
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — cell_rect OOB at small sizes — .max(40) floor removed; cell_w = avail_w / GRID_COLS with no floor, cells collapse to zero-width rather than overflowing neighbours
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 — hardcoded title_h = 20 — now a title_h: i32 parameter sourced from at.app.title_bar_height, cached in cached_title_bar_h
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 — duration label dx negative at minimum cell width — .max(cell_x) clamp confirmed
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 — Invidious-sourced video_id values unvalidated — is_valid_video_id applied in try_instance
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 — sequential Invidious fallback; search() still awaits each try_instance serially (~48s worst-case) — explicitly deferred to human per iteration 3; no change

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:782 — self.youtube_active_url.clone() still allocates one String per frame while a video is active; an Rc<String> for the cache field would make the clone a pointer bump. Low priority — iframe.show() suppresses the DOM write anyway via current_src check, so the allocation never reaches the DOM.

Notes

• The three lifecycle entry points for youtube_active_id/youtube_active_url are now fully symmetric: play:<id> sets both, stop/empty clears both, and window-close clears both — the stale-iframe-on-reopen bug from the security review is fully resolved
• pending_youtube_search is intentionally not cleared on window close; if an in-flight fetch completes after the window is gone, poll_youtube_search will publish the results to VFS but refresh_video_embed finds no matching runner (guard: title != "Video Embed") and discards them — no state corruption
• The soft_hide / show path is correctly ordered: pre-draw hides minimized windows; draw_with_clips_overlay never visits minimized windows, so the soft-hide cannot be immediately overridden by the draw loop
• All correctness, layout, and security issues from prior iterations are resolved; the only open item is the explicitly human-deferred sequential Invidious fallback

---

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-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks the timeout handle when the fetch promise rejects

  • If JsFuture::from(race).await returns Err (e.g., network failure or CORS block), the ? operator returns immediately without clearing the pending setTimeout. The closure and timer slot leak until the timeout fires ~6s later.
  • Clear the timeout in the Err arm before returning, or wrap the race in a match that cleans up on both success and failure paths.

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:280 - thumb_rect forces a minimum 20px height even when cell_h is 0

  • When the window is extremely small, cell_h becomes 0, but thumb_rect calculates by_h = avail_h.max(20), forcing a 20px thumbnail. This draws outside the 0-height cell bounds, potentially overlapping adjacent UI elements or the footer.
  • Clamp by_h to cell_h as i32 or return a zero-height rect when cell_h < 20.

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout handle when fetch wins — timer ID now captured and cleared on success
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect positions cells outside window bounds — .max(40) floor removed, cells collapse gracefully
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure — !is_empty() guard removed
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 - Invidious video_id unvalidated — is_valid_video_id filter applied
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:825 - Per-frame iframe reload — show() idempotent via current_src check
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx negative — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) per-frame allocation — cached in youtube_active_url
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs - youtube_active_url not cleared on close — now cleared alongside youtube_active_id

Suggestions (if any)

• crates/oasis-core/src/apps/video_embed.rs:705 - truncate uses s.chars().count() which is O(N) for UTF-8 strings. For typical video titles this is negligible, but s.char_indices().take(max_chars).last() could avoid a full scan if titles become very long.
• crates/oasis-backend-wasm/src/youtube.rs:268 - draw_image_with_html_image_element_and_dw_and_dh result is discarded with let _. If the canvas context is lost or tainted, the error is silently swallowed. Consider logging via console_log! for debugging, consistent with other error sites.

Notes

• The VFS IPC race condition guard (self.results.query == parsed.query) correctly prevents stale search results from overwriting newer queries.
• Texture lifecycle management in kick_youtube_search correctly drains and destroys old textures before allocating new ones.
• The soft_hide / show pattern for minimize/restore correctly preserves iframe playback state without reloading the video.
• TextInput/Backspace routing to open_runners is correctly separated from browser/terminal arms, fixing the dropped input bug.
• Sequential Invidious fallback latency (~48s worst-case) remains deferred to human per prior iterations.

---

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)

• [WARNING] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks the timeout handle when the fetch promise rejects

  • JsFuture::from(race).await.map_err(|e| format!("{e:?}"))? early-returns on rejection without calling clear_timeout_with_handle; the Closure::once_into_js and timer slot are held until the timeout fires ~6s later
  • Add a clear_timeout_with_handle call before the ? propagation, e.g.: let v = JsFuture::from(race).await.map_err(|e| { if let Some(id) = timer_id.borrow_mut().take() { window.clear_timeout_with_handle(id); } format!("{e:?}") })?;

• [WARNING] crates/oasis-core/src/apps/video_embed.rs:280 - thumb_rect forces a minimum 20px height even when cell_h is 0

  • let avail_h = cell_h as i32 - TITLE_LINES_H; let by_h = avail_h.max(20); — when cell_h = 0, avail_h = -22 and by_h = 20, drawing a 20px thumbnail past the bottom of a zero-height cell
  • Clamp by_h to cell_h as i32 (not just avail_h): let by_h = avail_h.max(0).min(cell_h as i32); or early-return a zero-size rect when cell_h < TITLE_LINES_H

Previous Issues (for incremental reviews)

• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:230 - race_with_timeout leaks timeout when fetch wins — timer ID captured in Rc<RefCell<Option<i32>>>, clear_timeout_with_handle called on success path
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect OOB at small sizes — .max(40) floor removed; cells collapse to zero-width without overflowing neighbours
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:258 - cell_rect hardcoded title_h = 20 — parameterized, sourced from at.app.title_bar_height, cached in cached_title_bar_h
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:150 - Empty result set treated as failure — !is_empty() guard removed
• [RESOLVED] crates/oasis-core/src/apps/video_embed.rs:650 - Duration label dx negative at min cell width — .max(cell_x) clamp applied
• [RESOLVED] crates/oasis-backend-wasm/src/youtube.rs:184 - Invidious-sourced video_id unvalidated — is_valid_video_id filter applied
• [RESOLVED] crates/oasis-backend-wasm/src/input_dispatch.rs:267 - youtube_active_url not cleared on Video Embed window close — self.youtube_active_url = None added alongside youtube_active_id = None
• [RESOLVED] crates/oasis-backend-wasm/src/lib.rs:818 - embed_url(id) per-frame allocation — youtube_active_url cache computed once on play:<id>, reused every frame
• [STILL UNRESOLVED] crates/oasis-backend-wasm/src/youtube.rs:126 - Sequential Invidious fallback (~48s worst-case) — explicitly deferred to human

Suggestions (if any)

• crates/oasis-backend-wasm/src/lib.rs:782 - self.youtube_active_url.clone() allocates one String per frame while a video is active; an Rc<String> for the cached field would make the per-frame clone a pointer bump (low priority — iframe.show() suppresses the DOM write anyway via current_src check)

Notes

• The three lifecycle entry points for youtube_active_id/youtube_active_url are now symmetric: play:<id> sets both, stop clears both, window-close clears both
• is_valid_video_id and extract_video_id enforce identical 6–20 char alphanumeric+-_ constraints on both user-typed and Invidious-sourced IDs
• The soft_hide/show minimize-preserve-playhead pattern is correctly ordered: pre-draw hides minimized windows before draw_with_clips_overlay visits them

---

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