Conversation
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (23 files)
Previous Review Summary (commit 20063e2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 20063e2)Status: No Issues Found | Recommendation: Merge Files Reviewed (19 files)
Reviewed by gpt-5.6-luna · Input: 0 · Output: 0 · Cached: 0 |
capy-ai
Bot
changed the title
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } | ||
| configLoaded = true; | ||
| const saveBtn = byId<HTMLButtonElement>("skipme-save-btn"); | ||
| if (saveBtn) saveBtn.disabled = false; |
There was a problem hiding this comment.
WARNING: A stale initialization can enable Save on a newly mounted page
init() is asynchronous, but mountPage() resets the page state and starts another init() without cancelling or versioning the previous one. If the old request resolves after remount, this new code sets configLoaded and enables the current page's Save button using the old configuration; saving then combines that stale load with the current page state and can overwrite the newly mounted page's settings, including the integration flag. Gate initialization updates by a mount/request generation (or cancel the prior initialization) before enabling controls.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
Is it really necessary to change the .net version, rewrite the entire plugin, and add an extensive test suite for this? The original premise was a separate provider that can be used with or without intro skipper, but this seems to be heading toward making it an extension of intro skipper. |
2 participants
Companion to intro-skipper/intro-skipper#975. Add an off-by-default Use Intro Skipper for segment analysis setting on SkipMe's Sync tab. After explicit opt-in, saving, and restarting Jellyfin, a compatible Intro Skipper receives SkipMe's local segment reader as an authoritative analyzer input instead of a second Jellyfin segment provider. Startup reads saved consent through Jellyfin's existing path/XML-serializer instances without building a second DI container; missing, legacy, disabled, or unreadable configuration stays standalone. Absent, incompatible, or failed host registration retains standalone behavior; the plugin has no Intro Skipper assembly dependency.
Successful syncs and integrated configuration changes queue Intro Skipper detection. At startup, the integrated handover waits for Jellyfin's scheduled tasks to initialize, queues an initial analysis of already-synced data, and retires only legacy SkipMe-owned Jellyfin rows. Cleanup retries failures without touching the local SkipMe database or other providers. Jellyfin already hides rows belonging to unregistered providers; Intro Skipper publishes its replacement segments asynchronously.
Keep Sync/Share features and series, season, movie, and specials restrictions. Integrated source reads also honor existing per-library SkipMe provider exclusions. The README describes enabling Intro Skipper at the library level and restarting to switch integration modes.
Validation: Release build with zero warnings/errors, 43 regression tests, TypeScript/Vite build, formatting, and
git diff --checkpass. A separate smoke harness passed fourteen assertions using the actual companion IntroSkipper.dll, including lazy singleton registration, exact timestamps read from SkipMe's SQLite store, library exclusions, missing/legacy configuration staying standalone, and opt-out taking effect on restart. Browser checks of the actual compiled dashboard verified default-off, enable/save/reload, disable/save, Share-tab isolation, preserved unrelated settings, and failed-load/save safety. Jellyfin startup/provider-discovery behavior was checked against its source; a full running Jellyfin deployment and playback were not exercised.The new test project is included in the solution. CI now installs .NET 10 to match the existing target framework, and two XML parameter-documentation orderings are corrected for the existing StyleCop checks.
Dashboard preview — the compiled settings UI with synthetic fixture data, not a live Jellyfin server.