[OwaisByte]

Summary

Keep page navigation in the background menu and make item menus focus on the clicked item. Resume last watched restores the observed session through the existing playback history.

• Keep the background Back/Settings strip and a disabled Resume row when no session exists. Remove the item toolbar, image tools, and duplicate movie/series playback and sharing rows.
• Keep contextual collection share-link and player magnet copying as normal rows. Place Continue Watching removal next to source selection, preserve immediate list/collection membership changes, and update the Add to list icon and footer color.
• Restore the saved file, position, subtitle/off selection, audio choice and delays. Preserve account/profile ownership, existing retention, physical-release matching, and current player exit/progress behavior.
• Persist source identity and fingerprints instead of unclassified network transports. Paths, unusual query names and encoded redirects cannot make a streaming URL safe to save. Explicit projections also cover preference fields, URL-derived IDs, actor/track records, and the season-lock copy of a source preference.
• Scrub legacy entries without losing private-profile observations or letting one malformed record stop cleanup. Keep the existing empty guest scope valid.
• Restore external subtitles through an exact current-source fingerprint, provider identity or a durable local cache. A cached file takes precedence over its former remote URL; ambiguous matches remain unavailable.

Why

Repeated toolbars and image tools made item menus longer without helping their target. The playback shortcut also needs session choices rather than a stale source URL.

The source model does not establish public provenance for generic HTTP streams. History therefore keeps local file paths and stable source identities, and sends closed network sessions through existing source preparation or selection. It does not infer safety from a URL's query parameter names or move transports into another store.

This also affects URLs that happen to be public: they may require source selection after restarting Harbor. A manually entered URL, or a URL-encoded provider-page ID with no retained resolver, asks the user to reopen the video from its source or enter the link again. Returning to the exact still-live session stays direct. Unavailable remote subtitles report the existing restore error instead of selecting another file by label alone.

Known static TMDB artwork uses its documented image route. Arbitrary network artwork uses the existing fallback. Independent image and cover-management features are unchanged.

The local implementation and build use official base 29af43e714b106f2d740b36191e1ca60ead0ff95 (1.0.1). Later beta 67452cf7fd0db581c019f17f3dbb3883765b3b56 was checked for merge compatibility; it is not the source of the local binary. Queue integration and default native features remain intact.

Verification

• Formatting/lint: passed on the changed files, with no lint warnings or errors.
• pnpm run typecheck --incremental: passed after the final changes.
• Eight focused playback/history/recovery suites: 112 passed, 0 failed, none skipped. The new provenance suite covers path credentials, alternative query names, nested/encoded/relative URLs, free-form fields, malformed records, key collisions, guest/profile ownership, manual URL re-entry, and exact subtitle/cache matching.
• Related 104-file Node selection: 740 tests, 728 passed, 12 failed, none skipped. The failure cases and errors match the previous verified branch; no new failure appeared in this selection. This is not the full repository suite.
• The same 12 local failures were previously checked on clean official 29af43e7: nine errors match; three pre-existing cache-folder cases fail earlier on a missing pause-trace fixture dependency, then expose an absent h.createBridge helper when that dependency is supplied. Those three are not reported as identical errors. The files are continue-card-dismissal.test.ts (8), mpv-cache-folder.test.ts (3), and playback-critical-path.test.ts (1).
• The opt-in Windows mpv subprocess test passed with disposable media. A separate controlled HTTP check on the previous review fix confirmed successful direct loading/seek and a pre-load end-file error for HTTP 403. This is distinct from a complete Tauri restart or authenticated provider test.
• git diff --check: passed. No dependencies, lockfiles, CI rules or native feature defaults changed in the security follow-up.
• Windows Tauri debug build (--no-bundle --features context-review): passed with default native features and Capstan enabled. The source was frozen throughout the build, and a separate audit verified all 397 packaged files. Its source was then verified against commit 67e4377d. Build ID: ea26f4d4-provenance-local. The review configuration is not production packaging; no installer or GUI restart test is claimed.

The previous published-head Code Quality run completed with failures: frontend 6,948 passed, 125 failed, 22 skipped; native 1,386 passed, 71 failed, 81 ignored. Formatting/lint and TypeScript passed. A full clean Linux baseline was not run for every failing case, so these are not all classified as baseline failures. That historical run does not test this follow-up; GitHub shows the new commit's check status below.

No tests or checks were disabled to obtain these results.

Platform Impact

Windows native compilation and isolated mpv checks are covered. Full Tauri restart/restore with real authenticated providers, Linux and macOS remain unverified in this pass. Linux binary creation was unavailable on this Windows host without a WSL distribution. No real account data or media files were used as mutation fixtures.

UI Changes

Item menus lose the top strip and image submenu. The background menu keeps its two-button strip and disabled Resume row when empty. Back/Settings tooltips are removed while the saved title tooltip remains. A URL-derived private title uses a translated Direct stream label, and nonrenewable sources have a translated reopen/re-entry error.

Component tests cover the menus and keyboard/target boundaries. No fresh native before/after captures or interaction recording were collected for this follow-up.

Checklist

• This pull request is focused and contains no unrelated refactors.
• I ran vp check for the changed files.
• I ran the TypeScript check (pnpm run typecheck).
• I ran the relevant Cargo checks after Rust changes in the original contribution; this follow-up changes no Rust.
• I tested affected platforms when the change is platform-specific. See the limits above.
• I preserved playback, navigation, and configured hotkey behavior where applicable.
• I added or updated tests for behavior changes.
• I checked the contribution for secrets and personal data; local logs, builds and test state are excluded.

[Thunderhawkk]

Thanks for the work on this PR. The context-menu simplification looks good overall, and the playback restoration changes have solid test coverage.

I reviewed the latest changes and ran the relevant tests. All 112 focused tests passed, along with the TypeScript typecheck. However, I found two issues that should be addressed before merging.

1. [P1] Playback history persists media-server authentication tokens

In src/lib/player/actual-playback.ts:34-45 and 70-78, the playback snapshot copies nearly the entire PlayerSrc object into ActualPlayback.src, including src.headers.

The media-server adapters populate these headers with authentication credentials, including Jellyfin's Authorization, Emby's X-Emby-Token, and Plex's X-Plex-Token.

Since recordActualPlayback() serializes the snapshot into harbor.playback-history.v1.* in localStorage, these credentials are being persisted outside the existing secret store. Transcoding URLs may also contain authentication tokens in query parameters.

Please change the snapshot to persist only the fields required for restoring playback, excluding authentication headers, temporary session identifiers, and token-bearing URLs.

For home-server playback, the existing homeServer identifiers should be sufficient to regenerate a fresh authenticated source when resuming.

2. [P2] Resume last watched can reopen an expired direct streaming URL

In src/lib/player/continue-playback.ts:162-166, the continuation logic correctly refreshes torrent and home-server sources. However, direct HTTP streams without an infoHash or proxySessionId are reopened using their previously saved URL.

If an addon provides a temporary signed URL, that link may have expired by the time the user selects Resume last watched. Harbor would then attempt to play the expired URL without recovering through the source picker.

Please add a recovery path for direct streams that fail to reopen, preserving the saved position, audio/subtitle selections, and delays. Valid direct URLs should still retain their fast reopening behavior.

Validation

• 112 focused tests passed, 0 failed.
• TypeScript typecheck passed.
• Git diff whitespace check passed.

Overall, the menu simplification and playback restoration improvements look promising. The main blocker is the credential-persistence issue, which should be addressed before merging. The expired-source recovery issue should also be fixed so Resume last watched remains reliable for temporary streaming links.

Please address these two findings, and I can re-review the updated changes afterward.

[Thunderhawkk]

Thanks for the updates to PR #61. The latest changes address both issues from my previous review. Playback history now avoids directly persisting media-server authentication headers, and failed direct-stream resumes can return to the source picker while preserving the saved playback choices.

I re-reviewed the latest commit (ea26f4d4) and ran the relevant tests. 165 tests passed, with no failures and one optional native test skipped. The context-menu simplification, playback restoration, subtitle/audio selection, and recovery behavior look good overall.

However, I found one remaining issue that should be addressed before merging.

1. [P1] Playback history can still persist credentials embedded in streaming URLs

In src/lib/player/playback-history-source.ts, safeString() rejects URLs containing certain recognized authentication query parameters, but it does not reliably detect credentials embedded in URL paths, alternative query parameter names, or nested URLs.

I reproduced this using synthetic credentials:

• https://media.example/v1/key/PATHSECRET123/play.m3u8
• https://media.example/watch?accessCode=QUERYSECRET123
• https://proxy.example/watch?url=https%3A%2F%2Fcdn.example%2Fvideo%3Ftoken%3DINNERSECRET

All three URLs are retained by persistablePlaybackUrl().

Since persistablePlaybackSource() uses this function when saving playback history, authenticated direct-stream URLs can still end up in localStorage.

Please improve the sanitization so it does not rely exclusively on a query-parameter denylist. Ideally, use source provenance to determine whether a URL is safe to persist. When a source may contain credentials, retain its fingerprint and source identity instead of its complete transport URL.

Also, please review the free-form fields in persistableEntry() to ensure credentials cannot be retained through other playback-history fields.

Add regression tests covering credentials embedded in URL paths, alternative parameter names, and nested encoded URLs. Valid public direct URLs should retain their existing fast reopening behavior.

Regarding merging

The PR merges cleanly into the remote beta-branch. However, integrating it with my current local changes produces conflicts in four files:

• src/components/context-menu.tsx
• src/components/continue-card.tsx
• src/views/player/hooks/use-track-autoload.ts
• tests/helpers/mpv-bridge-harness.ts

I still have local commits that need to be pushed. Once those are available remotely, we can reconcile the overlapping changes while preserving both implementations.

Overall, the PR is in much better shape now. Once the remaining credential-persistence issue is addressed, I'll re-review it for merging.