Add regression for failed sloppak websocket load

[carochacs]

What

Follow-up to #16. That fix guarded the later stem-access branch against a None loaded_slop, but the highway websocket handler still dereferenced loaded_slop.song immediately after the sloppak load — before the stem branch is ever reached. A failed load (cache corruption, partial extraction, etc.) still crashed the connection instead of reporting the failure.

• Adds an explicit guard right after sloppak_mod.load_song(...) returns, before song = loaded_slop.song is dereferenced.
• On a None result: cancels the keepalive task, sends {"error": "Failed to load sloppak"}, closes the socket, and returns — matching the existing failure-reporting pattern used elsewhere in this handler.
• Adds websocket regression coverage asserting the exact message sequence (loading → error), a single close() call, and that the handler never reaches stem access.

feedpak surface

• This PR does not change how the app reads/writes feedpaks (manifest keys, pack files, folder layout)

Checklist

• CHANGELOG.md [Unreleased] updated (user-visible changes)
• Tests added/updated for new behaviour — strengthened per CodeRabbit review to assert the exact message sequence and close-call count, not just that the error appeared somewhere in the messages list
• Commits are DCO signed off (git commit -s) — not on all commits; this PR targets this fork's own main, not got-feedback/feedBack upstream

Notes

convert_wem is called directly (not via run_in_executor) inside this same async handler at two pre-existing call sites (lines 468, 490) — flagged by CodeRabbit's automated checks. That's out of scope here: neither call site is touched by this PR's diff, and wrapping them is a separate, more invasive change than a load-failure regression test warrants.

Validation

• python -m pytest tests/test_highway_ws_failed_sloppak_load.py -v — not run in this sandbox because pytest is not installed
• git diff --check — passes
• GitHub reports the PR as mergeable

[Copilot]

Pull request overview

Adds a regression test and a server-side guard to ensure the highway WebSocket fails cleanly when a sloppak/feedpak load returns None, preventing a loaded_slop.song dereference crash and verifying the client sees a clear failure.

Changes:

• Add an explicit early-return guard in highway_ws when sloppak_mod.load_song(...) returns None.
• Add a new pytest regression covering the failed sloppak load path (accept → error message → close).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lib/routers/ws_highway.py	Adds a guard to send {"error": "Failed to load sloppak"} and close the socket when the sloppak loader returns None.
tests/test_highway_ws_failed_sloppak_load.py	New regression test that simulates load_song returning None and asserts error emission + socket close.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

📝 Walkthrough

Walkthrough

The highway WebSocket handler now handles failed sloppak loads by stopping keepalive notifications, sending an error, closing the connection, and returning. A regression test verifies the response and confirms that stems remain unchanged.

Changes

Highway WebSocket sloppak failure

Layer / File(s)	Summary
Handle failed sloppak loads lib/routers/ws_highway.py, tests/test_highway_ws_failed_sloppak_load.py	The handler exits when sloppak loading returns no song. The test verifies the error message, WebSocket closure, acceptance, and unchanged stems.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: byrongamatos

🚥 Pre-merge checks | ✅ 20 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Blocking Route Handlers Use Def Not Async	⚠️ Warning	Modified async handler highway_ws calls convert_wem directly at lines 468 and 490; convert_wem uses subprocess.run and is not wrapped in run_in_executor or asyncio.to_thread.	Wrap each audio conversion call, and related blocking file discovery if needed, in run_in_executor/asyncio.to_thread, or make the handler synchronous and delegate it to an executor.
Changelog Unreleased Section Updated	⚠️ Warning	The PR changes application behavior in lib/routers/ws_highway.py, but its two-file diff does not modify CHANGELOG.md or add an [Unreleased] bullet.	Update CHANGELOG.md with a new Keep a Changelog bullet under ## [Unreleased] describing the failed sloppak load handling.

✅ Passed checks (20 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Plugin Folder Name Matches Manifest Id	✅ Passed	The PR touches feedBack core with plugins//plugin.json; all 12 manifests have an id exactly matching their containing folder name.
No Print()/Console.Log In Routes.Py	✅ Passed	Added lines in lib/routers/ws_highway.py and the new test contain no print(...) or traceback.print_exc(...); the change adds no setup() or logging-style output.
Sibling Imports Use Load_sibling	✅ Passed	The PR changes only lib/routers/ws_highway.py and a top-level test; no module inside a plugin directory adds an import.
Routes Namespaced Under /Api/Plugins/Id	✅ Passed	The commit changes only lib/routers/ws_highway.py. No plugin route or manifest changed; existing HTTP routes use the required /api/plugins// prefixes, and no WebSocket routes exist.
Plugin.Json Version Bumped On Change	✅ Passed	The PR changes core lib/routers/ws_highway.py and a test; it does not change a plugin source file, so the plugin.json version rule is not applicable.
No Per-Frame Dom Queries In Draw/Raf	✅ Passed	The PR changes only lib/routers/ws_highway.py; no modified .js files or new JavaScript hot-path query patterns exist.
Shortcuts Unregistered With Matching Scope	✅ Passed	The pull-request patch only changes lib/routers/ws_highway.py; it adds no window.registerShortcut call, so no shortcut cleanup check applies.
Idempotent Guard On Top-Level Listeners	✅ Passed	The pull request patch changes only lib/routers/ws_highway.py; it adds no screen.js file or top-level listener/wrapper call, so this check is not applicable.
Server_files Entries Are Safe Relpaths	✅ Passed	The PR changes only lib/routers/ws_highway.py and tests/test_highway_ws_failed_sloppak_load.py; no plugin.json diff exists to violate the path rules.
Setrenderer Factory Has Init/Draw/Destroy	✅ Passed	The pull request changes only lib/routers/ws_highway.py; it adds no window.feedBackViz_ factory or renderer lifecycle code, so this check is not applicable.
Overlay Gates On Isdefaultrenderer Instructions	✅ Passed	The PR changes only lib/routers/ws_highway.py and a Python regression test; it adds no overlay drawing code or highway.project/fretX positioning path.
V3 Ui Mounts Via Playercontrolslot	✅ Passed	The PR changes only Python websocket handling and a Python regression test; no new #player-controls query or DOM injection code is present.
New Feedpak Manifest Keys Declared In Spec	✅ Passed	The PR changes only websocket load handling and its regression test; it adds no manifest key reads or writes in the checked implementation files.
Feedpak Manifest Required Keys Present	✅ Passed	The PR diff changes only lib/routers/ws_highway.py and a websocket regression test; it does not touch feedpak manifest-assembly code.
New Python Modules Have Pytest Coverage	✅ Passed	The PR adds only tests/test_highway_ws_failed_sloppak_load.py; lib/routers/ws_highway.py is modified, not added, so no new lib/ Python module triggers this check.
No Hardcoded Secrets Or Tokens In Diff	✅ Passed	The 69 added lines contain zero AWS key, PEM private-key, or disallowed credential-assignment matches.
Title check	✅ Passed	The title clearly identifies the failed sloppak WebSocket load regression covered by the pull request.
Description check	✅ Passed	The description explains the fix, test coverage, feedpak impact, checklist status, scope, and validation results.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/sloppak-load-none-regression

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_highway_ws_failed_sloppak_load.py`:
- Around line 10-27: Strengthen the test using _CapturingWS by recording close
calls and asserting the handler terminates immediately after the sloppak load
error. Verify the exact two-frame message sequence and the expected close
behavior, so a handler that sends the error and continues processing fails the
test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7640adb5-2b17-4f03-a584-024f666bc7c1

📥 Commits

Reviewing files that changed from the base of the PR and between 5b443e9 and e2614a8.

📒 Files selected for processing (2)

• lib/routers/ws_highway.py
• tests/test_highway_ws_failed_sloppak_load.py