feat: support Remote Development via Plugin Model V2 content modules#43
Merged
Conversation
Convert the single-module plugin into a Plugin Model V2 modular build with split mode enabled. Add shared/frontend/backend Gradle modules, the rpc + kotlinx.serialization plugins, and V2 module descriptors; rewrite the root plugin.xml to a <content> block. The entire dev.twango.jetplay source tree moves into shared temporarily (bytedeco parked on shared) and is partitioned in later steps. player/ and messages/ resources move to frontend; buildPlayerUi and the vite outDir now target the frontend module.
Move ffmpeg/byte-access (MediaTranscoder, MediaInfoExtractor, WaveformExtractor, FfmpegAvailability, LocalFileMediaSource) and the bytedeco natives into the backend content module so they are backend-only. Define the @rpc MediaAccessor contract and @serializable DTOs (MediaInfo/MediaTag, TranscodeEvent) in shared, implement MediaAccessorImpl with chunked Flow streaming plus a readRange fallback, and register it via the platform.rpc.backend.remoteApiProvider EP. Relocate the editor/JCEF/MediaServer classes into the frontend module and rewire MediaLoader onto the RPC API with a monolith fast path (serve the real file directly when bytes are readable in-process). Hoist needsTranscoding/rawAudioExtensions into shared MediaClassification and bump the Kotlin toolchain to 21 (required by the RPC inline API).
…requests Guard MediaFileEditorProvider on JBCefApp.isSupported(): render a plain-Swing MediaErrorEditor instead of an empty browser pane that would spin indefinitely. Add per-request debug logging plus a missing-token warning in MediaServer.handle so split-mode/remote-dev serving failures are diagnosable.
The shared/frontend/backend restructure left detekt scanning the root project's now-empty source set, so the CI lint gate silently passed everything. Point the root detekt task at each module's src/main/kotlin and fix the ktlint signature violations it surfaced.
…rowser In remote dev the JCEF Chromium may run frontend-side and never reach the loopback MediaServer, leaving the player spinning forever. Track which served tokens the browser actually fetches and arm a per-load watchdog: if nothing fetches the token before the deadline, surface an explicit error state pointing at the Remote Development limitation instead of an indefinite spinner. The fetch tracking doubles as the telemetry needed to diagnose split-mode serving failures.
… path Each test now lives in the module owning the class under test: the editor/browser/ star/MediaServer suites move to frontend, the ffmpeg extractor suites to backend, and the format classifier + RPC event tests to shared. Fixes stale transcode.MediaInfo imports left by the package move and routes the fileType-registration check at the frontend descriptor. Adds coverage for the migration's new surface: MediaAccessorImpl byte streaming (1MB chunk boundary, range reads, file length) over a real VirtualFileId round-trip, the monolith local-file fast-path in EditorMediaSource, MediaClassification, and the TranscodeEvent value semantics.
…te files Guard streamToTemp against the backend's empty-flow result (returned when a VirtualFile cannot be resolved to a local nio path): delete the temp and show an explicit error rather than handing the player a permanently broken 0-byte URL. Route transcoding ahead of the remote-download path so transcodable remote files transcode backend-side instead of discarding a wasted download, and show a loading shell before the split-mode streaming RPC so the tab is never blank. Delete frontend transcode/stream temps on dispose and failure, and the backend transcode output after it has streamed. Fill readRange across partial reads.
Suppress UnstableApiUsage on the @rpc interface and the backend provider (both touch ApiStatus.Internal fleet.rpc APIs). Drop the redundant intellij.platform.rpc.backend module dep from backend.xml and the redundant intellij.platform.rpc bundledModule from frontend (both satisfied transitively, matching the modular template and the markdown plugin). Remove the unreachable LocalFileMediaSource left over from the pre-split monolith.
Modular content modules, Fleet RPC, and split mode all require 2025.3+ (253.x); the old 223 falsely advertised compatibility with releases that cannot load the plugin (JDK 21 bytecode + Fleet RPC). The release-please generic marker only tracks pluginVersion, so this line is safe to change.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe PR restructures JetPlay into a Plugin Model V2 multi-module layout ( ChangesJetPlay Plugin Model V2: Multi-module restructure with backend RPC, shared contracts, and frontend rewrite
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
note over MediaFileEditorProvider,MediaErrorEditor: Editor Creation
MediaFileEditorProvider->>MediaFileEditorProvider: canRenderJcefHere()?
alt JCEF unavailable
MediaFileEditorProvider->>MediaErrorEditor: new MediaErrorEditor(file, message)
else JCEF available
MediaFileEditorProvider->>MediaFileEditor: new MediaFileEditor(project, file, EditorMediaSource(file))
end
end
rect rgba(144, 238, 144, 0.5)
note over MediaLoader,PlayerBridge: Playback Orchestration
MediaFileEditor->>MediaLoader: load()
alt needsTranscoding
MediaLoader->>MediaAccessor: transcodeFile(fileId, projectId)
MediaAccessor-->>MediaLoader: Flow<TranscodeEvent>
MediaLoader->>MediaServer: serve(FileByteSource(tempFile))
else remote file
MediaLoader->>MediaAccessor: fileLength(fileId, projectId)
MediaLoader->>MediaServer: serve(RemoteRangeByteSource(readRange RPC))
else local file
MediaLoader->>MediaServer: serve(FileByteSource(localFile))
end
MediaServer-->>MediaLoader: loopbackUrl
MediaLoader->>PlayerBridge: mediaReady(loopbackUrl)
MediaLoader->>MediaAccessor: extractWaveform / extractMediaInfo
MediaAccessor-->>MediaLoader: List<Double> / MediaInfo?
MediaLoader->>PlayerBridge: sendWaveform / sendMediaInfo
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
backend/src/test/kotlin/dev/twango/jetplay/rpc/MediaAccessorImplTest.kt (1)
54-72: ⚡ Quick winAdd regression tests for invalid range arguments.
Please add cases for negative
offsetand negativelengthso RPC error-handling behavior is pinned by tests.🤖 Prompt for 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. In `@backend/src/test/kotlin/dev/twango/jetplay/rpc/MediaAccessorImplTest.kt` around lines 54 - 72, Add two tests to MediaAccessorImplTest.kt that verify invalid range arguments produce an RPC error: create testReadRangeNegativeOffset and testReadRangeNegativeLength that call runBlocking { impl.readRange(fileId(data), projectId(), offset = -1, length = 10) } and runBlocking { impl.readRange(fileId(data), projectId(), offset = 0, length = -1) } respectively, and assert the calls throw (use kotlin.test.assertFailsWith or assertThrows to expect an exception from the RPC layer). Place them alongside the existing testReadRange* functions so the negative-offset and negative-length behavior is pinned in CI. Ensure you reuse fileId(data) and projectId() helpers and keep the byte-array fixtures minimal.
🤖 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 `@backend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.kt`:
- Around line 43-46: The functions readRange, extractWaveform, and
extractMediaInfo perform blocking disk and ffmpeg/process work (e.g.,
RandomAccessFile and external process calls via resolveFile path) on coroutine
threads; wrap those blocking sections in withContext(Dispatchers.IO) so the
blocking IO runs on the IO dispatcher rather than the default RPC dispatcher.
Locate the blocking regions inside readRange (RandomAccessFile.seek/read),
extractWaveform (ffmpeg/process invocation and any file reads), and
extractMediaInfo and move just the synchronous work into
withContext(Dispatchers.IO) blocks, preserving suspend signatures and returning
results from the withContext calls. Ensure any resources opened inside those
blocks (files, processes) are closed within the withContext scope to avoid
leaking.
- Around line 24-26: The file resolution currently ignores the RPC-supplied
projectId and resolves by VirtualFileId alone; change resolveFile to accept a
projectId parameter and enforce that the resolved VirtualFile's project matches
the provided projectId (e.g., add projectId: ProjectId to resolveFile(projectId:
ProjectId, fileId: VirtualFileId): File? and inside verify vf.projectId ==
projectId or consult the project registry/manager before converting to NIO
path), return null if mismatched, and update every caller that used
resolveFile(VirtualFileId) (the public RPC methods in MediaAccessorImpl and any
helpers at the listed call sites) to pass the incoming projectId so project
scoping is enforced. Ensure all resolve* usages (including the other similar
helper overloads at the noted locations) are updated consistently.
- Around line 43-47: In readRange, validate inputs before seeking/allocating:
ensure offset >= 0 and length > 0, and check resolved file's length (via
resolveFile result) so offset is not beyond file size; for invalid values or
out-of-range requests return an empty ByteArray instead of calling
RandomAccessFile.seek or allocating ByteArray(length). Update logic around
resolveFile, RandomAccessFile/raf.seek and the ByteArray allocation to perform
these checks and to cap length to remaining bytes if needed.
In `@frontend/src/main/kotlin/dev/twango/jetplay/media/MediaServer.kt`:
- Around line 51-53: wasFetched, release and the request-path handling derive
tokens inconsistently which can miss tokens when URLs have query strings or
extra suffixes; add a single helper function (e.g., normalizeTokenFromUrl(url:
String): String) and use it everywhere instead of ad-hoc substringAfterLast('/')
or other extraction logic; the helper should extract the last path segment,
strip query/fragment suffixes and optional trailing slashes, and perform
URL-decoding so wasFetched(url), release(url) and the request-path code all
compute the same token.
In `@frontend/src/main/resources/dev.twango.jetplay.frontend.xml`:
- Around line 8-11: The fileType declaration in dev.twango.jetplay.frontend.xml
currently hardcodes the extensions attribute (the Media fileType element
referencing implementationClass="dev.twango.jetplay.editor.MediaFileType" and
fieldName="INSTANCE"); remove the extensions="..." attribute from this frontend
descriptor so extension ownership is not duplicated, and ensure that
MediaFileType/INSTANCE continues to read supported extensions from the single
source-of-truth in src/main/resources/META-INF/plugin.xml (or rely on the
platform registration that reads plugin.xml) rather than declaring them here.
In
`@frontend/src/test/kotlin/dev/twango/jetplay/media/RawAudioRegistrationTest.kt`:
- Around line 10-18: The test RawAudioRegistrationTest currently loads the wrong
descriptor into xml and validates registered against it; change the resource
lookup so xml loads the repository's canonical plugin descriptor (the plugin.xml
used as the single source of truth), then keep the existing parsing logic
(registered variable and split/filter flow) and compare
MediaClassification.rawAudioExtensions against that registered set; update the
resource path used in javaClass.getResource(...) to point at the canonical
plugin descriptor and keep the assertTrue check unchanged.
In `@src/main/resources/META-INF/plugin.xml`:
- Around line 6-11: Add a single <fileType name="Media" ... extensions="...">
entry to src/main/resources/META-INF/plugin.xml and populate its extensions
attribute with the combined, deduplicated values from VIDEO_EXTENSIONS,
JCEF_NATIVE_EXTENSIONS, and rawAudioExtensions (copy the literal extension
strings currently declared in dev.twango.jetplay.frontend.xml into plugin.xml);
then remove the duplicate <fileType ...> registration from
dev.twango.jetplay.frontend.xml so plugin.xml is the single source of truth and
ensure the extension list is properly comma/space separated and escaped for XML.
---
Nitpick comments:
In `@backend/src/test/kotlin/dev/twango/jetplay/rpc/MediaAccessorImplTest.kt`:
- Around line 54-72: Add two tests to MediaAccessorImplTest.kt that verify
invalid range arguments produce an RPC error: create testReadRangeNegativeOffset
and testReadRangeNegativeLength that call runBlocking {
impl.readRange(fileId(data), projectId(), offset = -1, length = 10) } and
runBlocking { impl.readRange(fileId(data), projectId(), offset = 0, length = -1)
} respectively, and assert the calls throw (use kotlin.test.assertFailsWith or
assertThrows to expect an exception from the RPC layer). Place them alongside
the existing testReadRange* functions so the negative-offset and negative-length
behavior is pinned in CI. Ensure you reuse fileId(data) and projectId() helpers
and keep the byte-array fixtures minimal.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: f22d7446-0293-4a30-820a-b8ab2ffdd41c
📒 Files selected for processing (71)
.gitignorebackend/build.gradle.ktsbackend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.ktbackend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorProvider.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/FfmpegAvailability.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractor.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/TranscodeRunner.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.ktbackend/src/main/resources/dev.twango.jetplay.backend.xmlbackend/src/test/kotlin/dev/twango/jetplay/rpc/MediaAccessorImplTest.ktbackend/src/test/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractorTest.ktbackend/src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.ktbackend/src/test/kotlin/dev/twango/jetplay/transcode/WaveformExtractorTest.ktbuild.gradle.ktsfrontend/build.gradle.ktsfrontend/src/main/kotlin/dev/twango/jetplay/JetPlayBundle.ktfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.ktfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerConfig.ktfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaErrorEditor.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditor.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileType.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/MediaServer.ktfrontend/src/main/kotlin/dev/twango/jetplay/star/StarReminder.ktfrontend/src/main/kotlin/dev/twango/jetplay/star/StarReminderPolicy.ktfrontend/src/main/resources/dev.twango.jetplay.frontend.xmlfrontend/src/main/resources/messages/JetPlayBundle.propertiesfrontend/src/main/resources/messages/JetPlayBundle_de.propertiesfrontend/src/main/resources/messages/JetPlayBundle_es.propertiesfrontend/src/main/resources/messages/JetPlayBundle_fr.propertiesfrontend/src/main/resources/messages/JetPlayBundle_it.propertiesfrontend/src/main/resources/messages/JetPlayBundle_ja.propertiesfrontend/src/main/resources/messages/JetPlayBundle_ko.propertiesfrontend/src/main/resources/messages/JetPlayBundle_pl.propertiesfrontend/src/main/resources/messages/JetPlayBundle_pt_BR.propertiesfrontend/src/main/resources/messages/JetPlayBundle_ru.propertiesfrontend/src/main/resources/messages/JetPlayBundle_zh_CN.propertiesfrontend/src/main/resources/messages/JetPlayBundle_zh_TW.propertiesfrontend/src/test/kotlin/dev/twango/jetplay/browser/PlayerBridgeEscapeTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoaderTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/editor/MediaFileEditorProviderTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/EditorMediaSourceTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/MediaServerTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/RawAudioRegistrationTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/star/StarReminderPolicyTest.ktgradle.propertiesgradle/libs.versions.tomlsettings.gradle.ktsshared/build.gradle.ktsshared/src/main/kotlin/dev/twango/jetplay/JetPlayConstants.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaInfo.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaSource.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessor.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/TranscodeEvent.ktshared/src/main/resources/dev.twango.jetplay.shared.xmlshared/src/test/kotlin/dev/twango/jetplay/media/MediaClassificationTest.ktshared/src/test/kotlin/dev/twango/jetplay/rpc/TranscodeEventTest.ktsrc/main/kotlin/dev/twango/jetplay/editor/MediaLoader.ktsrc/main/kotlin/dev/twango/jetplay/media/LocalFileMediaSource.ktsrc/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktsrc/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.ktsrc/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.ktsrc/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.ktsrc/main/resources/META-INF/plugin.xmlsrc/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.ktui/vite.config.ts
💤 Files with no reviewable changes (8)
- shared/src/main/kotlin/dev/twango/jetplay/media/MediaSource.kt
- src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt
- src/main/kotlin/dev/twango/jetplay/media/RemoteFileMediaSource.kt
- src/main/kotlin/dev/twango/jetplay/transcode/TranscodeSession.kt
- src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt
- src/main/kotlin/dev/twango/jetplay/media/LocalFileMediaSource.kt
- src/main/kotlin/dev/twango/jetplay/transfer/DownloadSession.kt
- src/test/kotlin/dev/twango/jetplay/transcode/MediaTranscoderTest.kt
Contributor
Qodana Community for JVM16 new problems were found
💡 Qodana analysis was run in the pull request mode: only the changed files were checked View the detailed Qodana reportTo be able to view the detailed Qodana report, you can either:
To get - name: 'Qodana Scan'
uses: JetBrains/qodana-action@v2026.1.0
with:
upload-result: trueContact Qodana teamContact us at qodana-support@jetbrains.com
|
…ocking work on IO resolveFile now rejects an unresolvable projectId (findProjectOrNull), so the RPC bytes/transcode surface only serves files within a live project context. readRange/fileLength/extractWaveform/extractMediaInfo run their blocking disk and ffmpeg work under withContext(Dispatchers.IO) instead of the RPC dispatcher, and readRange guards against negative offset/length before seek/allocation.
wasFetched/release now derive the token via the same request-path parsing as handle(), so query/fragment suffixes can't desync fetch-state from the served key.
Under the Plugin Model V2 split the fileType/fileEditorProvider live in the frontend content module (frontend-side APIs), so the frontend descriptor — not the root plugin.xml — owns the supported-extension list.
…dings verifyPlugin failed on two categories the V2 restructure introduced: INTERNAL_API_USAGES (the RPC framework — RemoteApiProviderService, the remoteApiProvider EP, ProjectId/ VirtualFileId — has no stable equivalent for split mode) and COMPATIBILITY_PROBLEMS from javacv's optional capture/vision frame-grabbers referencing native bindings the build excludes. Narrow failureLevel to real incompatibilities + invalid structure, and mark the excluded javacv packages as external prefixes. Verified Compatible against IU-2026.1.
…ppression list The 16 javacv 'package not found' compatibility problems and the 3 'config file not found' defects were one root cause: IPGP names the content-module jars <rootProject>.<module> (jetplay.backend), but the platform and plugin verifier resolve content modules by lib/modules/<moduleId>.jar. With the names mismatched the verifier could not load the module descriptors and fell back to scanning every bundled jar — surfacing javacv's excluded-binding references. Override each module's composedJar archiveBaseName to its module id so the descriptors resolve and javacv is never scanned: drops externalPrefixes entirely and clears the config defects. failureLevel returns to the default minus INTERNAL_API_USAGES (split-mode RPC has no stable API), with OVERRIDE_ONLY restored. Verified Compatible against IU-2026.1.3 with no config defects.
Condense multi-line rationale blocks to their core, drop redundant section labels and restated-WHAT comments across the modular media code. Comments only; no logic changes.
Open and play media in split mode (Remote Dev / Gateway), where the editor renders on the JetBrains Client but file bytes live on the backend host. - frontend module loads on host AND client so the host detects media files and selects the editor; JCEF is guarded off on the host (AppMode.isRemoteDevHost) - new client-only frontend-split module registers rdclient.fileEditorModelHandler to build the JCEF editor on the client (internal RD APIs, compileOnly) - MediaServer serves a MediaByteSource; MediaLoader streams backend media on demand via MediaAccessor.readRange instead of pre-downloading (instant seek) - move JetPlayBundle + messages to shared; make splitMode a Gradle property
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@frontend-split/build.gradle.kts`:
- Around line 19-26: The compileOnly block with hardcoded jar paths lacks
defensive validation, which means build failures will occur with cryptic errors
if any jar file is missing or paths change. Add validation logic before the
compileOnly block that checks whether each jar file referenced in the files()
call actually exists at the specified paths. If any jar is missing, throw a
clear, descriptive error message that identifies which specific jar file is not
found and helps users understand the problem, then let the build fail explicitly
with that message rather than allowing Gradle to fail cryptically later.
- Around line 5-7: The ideHome variable uses eager evaluation at Gradle
configuration time, but the IDE files are not available until the execution
phase when the prepareIde task runs, causing build failures in CI and fresh
clones. Additionally, the lexicographic name sorting is fragile for semantic
versioning. Replace the eager val assignment with a lazy Gradle provider that
defers IDE resolution until task execution time, then update all usages of
ideHome to call .get() to retrieve the resolved value during execution. This
ensures the IDE files exist when needed and allows for proper semantic version
ordering.
- Line 11: The bundledModule declaration is using an incorrect module identifier
that does not match the module descriptor dependency. In the bundledModule
function call, change the module name from "intellij.platform.frontend" to
"intellij.platform.frontend.split" to align with the dependency declared in the
module descriptor file (dev.twango.jetplay.frontend.split.xml) and match the
naming pattern used in other modules within the project. This ensures the gradle
dependency resolver can properly locate the correct module.
In `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt`:
- Around line 117-129: The submit block in MediaLoader has a race condition
where MediaServer.serve() can be called after dispose() has already cleaned up
URLs, causing leaked tokens. Add a loader-level disposed flag that is checked
before calling MediaServer.serve(), and then immediately after serve() returns,
check the disposed flag again and release the URL if disposal has occurred in
the interim. This prevents the race where the async task registers a URL after
dispose() has already released the previous ones, by ensuring any URL created
after disposal is immediately cleaned up.
In `@frontend/src/main/kotlin/dev/twango/jetplay/media/MediaServer.kt`:
- Around line 161-174: The streamRange function does not validate that
source.read(offset, want) returns at most want bytes. If the source returns more
bytes than requested, the response will exceed the declared Content-Length
header (especially problematic for HTTP 206 Partial Content responses), breaking
clients. After calling source.read(offset, want), verify that the returned bytes
array does not exceed the requested size (want), and if it does, truncate it to
only include the first want bytes before writing to the output stream via
out.write(bytes).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7da3888f-07bc-468b-85ee-22e8961e52cc
📒 Files selected for processing (39)
AGENTS.mdbackend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractor.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.ktbuild.gradle.ktsfrontend-split/build.gradle.ktsfrontend-split/src/main/kotlin/dev/twango/jetplay/editor/split/MediaFrontendEditorModelHandler.ktfrontend-split/src/main/resources/dev.twango.jetplay.frontend.split.xmlfrontend/build.gradle.ktsfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaErrorEditor.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/MediaByteSource.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/MediaServer.ktfrontend/src/main/resources/dev.twango.jetplay.frontend.xmlfrontend/src/test/kotlin/dev/twango/jetplay/media/MediaByteSourceTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/MediaServerSourceTest.ktsettings.gradle.ktsshared/src/main/kotlin/dev/twango/jetplay/JetPlayBundle.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaInfo.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessor.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/TranscodeEvent.ktshared/src/main/resources/messages/JetPlayBundle.propertiesshared/src/main/resources/messages/JetPlayBundle_de.propertiesshared/src/main/resources/messages/JetPlayBundle_es.propertiesshared/src/main/resources/messages/JetPlayBundle_fr.propertiesshared/src/main/resources/messages/JetPlayBundle_it.propertiesshared/src/main/resources/messages/JetPlayBundle_ja.propertiesshared/src/main/resources/messages/JetPlayBundle_ko.propertiesshared/src/main/resources/messages/JetPlayBundle_pl.propertiesshared/src/main/resources/messages/JetPlayBundle_pt_BR.propertiesshared/src/main/resources/messages/JetPlayBundle_ru.propertiesshared/src/main/resources/messages/JetPlayBundle_zh_CN.propertiesshared/src/main/resources/messages/JetPlayBundle_zh_TW.propertiessrc/main/resources/META-INF/plugin.xml
💤 Files with no reviewable changes (3)
- frontend/src/main/resources/dev.twango.jetplay.frontend.xml
- shared/src/main/resources/messages/JetPlayBundle.properties
- frontend/build.gradle.kts
✅ Files skipped from review due to trivial changes (3)
- frontend-split/src/main/resources/dev.twango.jetplay.frontend.split.xml
- backend/src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.kt
- AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (13)
- src/main/resources/META-INF/plugin.xml
- frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaErrorEditor.kt
- frontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.kt
- shared/src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt
- shared/src/main/kotlin/dev/twango/jetplay/media/MediaInfo.kt
- shared/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessor.kt
- shared/src/main/kotlin/dev/twango/jetplay/rpc/TranscodeEvent.kt
- backend/src/main/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractor.kt
- frontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.kt
- backend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.kt
- settings.gradle.kts
- backend/src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt
- build.gradle.kts
…thout a cached IDE The compileOnly jar paths were resolved eagerly at configuration time, but on a fresh CI checkout .intellijPlatform/ides is empty until the platform plugin downloads the IDE — so the lookup failed during task-graph calculation. Defer it to a Callable evaluated at compile time, capturing the dir as a config-cache-safe File.
Guard against a MediaByteSource that returns more bytes than requested, which would push the HTTP body past its Content-Length/Content-Range and corrupt a 206.
An async load task could call MediaServer.serve() after dispose() had already released the served URLs, leaking the token/source. Gate registration on a loader-level disposed flag and release immediately if disposal wins the race.
Validate the IDE-internal RD jars exist when resolving the compileOnly classpath, so an IDE layout change reports the missing paths instead of cryptic compile errors.
…utines Collapse three overlapping concurrency mechanisms (pooled-thread Futures + runBlocking, the ScheduledFuture watchdog, and manual cancellation) into a single per-editor childScope of a project @service CoroutineScope. The MediaAccessor RPCs are already suspend, so launches call them directly rather than blocking a pooled thread per round-trip -- relevant under Remote Dev where each is a network hop. dispose() cancels the scope; the loopback-token release race guard is kept and transcode temp cleanup moved into finally so cancellation can't leak it. Uses only platform-bundled kotlinx-coroutines + childScope -- no new dependencies.
Remote Dev loads the JCEF page slowly while the media pipeline is fast, exposing two races that stranded editors on "loading"/"transcoding": - executeJavaScript is dropped until the page finishes loading, so progress/ready pushes were lost. PlayerBridge now queues JS and flushes it on onLoadEnd. - the remote and transcode paths issued a second loadHTML that raced the shell's load; whichever finished last won the screen. They now load the shell once and push the URL in-page via mediaReady. Also gate the load watchdog on editor visibility, so background tabs (whose JCEF browser never loads until shown) stop false-firing "unable to play".
The remote path streams bytes on demand via the readRange proxy rather than pre-downloading the whole file, so the "downloading" state, its progress bar, and reason text became unreachable. Drop them across Kotlin, the i18n bundles, and the Svelte player (DownloadingState component, updateDownloadProgress).
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt (1)
94-110:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep media metadata enabled for remote-backed files.
The new backend
MediaAccessorpath already supports remote byte access; returning onsource.isRemoteprevents Remote Development users from getting waveform and codec-inspector metadata.Proposed fix
private fun maybeSendWaveform() { - if (source.isVideo || source.isRemote) return + if (source.isVideo) return // Raw telephony codecs lack the demuxer hints to decode cleanly, risking a garbage waveform. if (source.extension.lowercase() in MediaClassification.rawAudioExtensions) return scope.launch { val bars = MediaAccessor.getInstance().extractWaveform(fileId, projectId) if (bars.isNotEmpty() && !bridge.disposed) bridge.sendWaveform(bars) @@ private fun maybeSendMediaInfo() { - if (source.isRemote) return if (source.extension.lowercase() in MediaClassification.rawAudioExtensions) return scope.launch { val info = MediaAccessor.getInstance().extractMediaInfo(fileId, projectId)🤖 Prompt for 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. In `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt` around lines 94 - 110, Remove the remote source checks from both media metadata extraction functions since the new MediaAccessor backend already supports remote byte access. In maybeSendWaveform(), remove the source.isRemote condition from the early return statement (keeping only the source.isVideo check). In maybeSendMediaInfo(), remove the entire early return line that checks if source.isRemote is true. This will allow remote development users to receive waveform and media info metadata for remote-backed files.ui/src/App.svelte (1)
60-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the remaining
downloadingstate callers/tests.The supplied
ui/tests/states.spec.tssnippets still seedstate: 'downloading'; after this branch removal, those cases fall through to the audio/video player path instead of a loading UI.Replace those scenarios with
loadingor remove them alongside the deleted download UI.🤖 Prompt for 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. In `@ui/src/App.svelte` around lines 60 - 68, Update the test file ui/tests/states.spec.ts to reflect the removal of the 'downloading' state from the main App.svelte component. Any test cases that currently seed state: 'downloading' should either be replaced with state: 'loading' (to test the TranscodingState component path) or removed entirely if they are no longer relevant. This ensures the tests align with the updated state handling logic in the if-else chain where 'downloading' is no longer a valid state option.
🤖 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 `@frontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.kt`:
- Around line 88-94: The loadHtml function directly calls browser.loadHTML(html)
without marshaling the call onto the EDT, making it unsafe when invoked from
background threads like the coroutine in MediaLoader.runTranscode(). Wrap the
browser.loadHTML(html) call in SwingUtilities.invokeLater to ensure thread-safe
access to JCEF/Swing, following the same pattern already used in the runJs
method.
In `@frontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerConfig.kt`:
- Line 14: The UiStrings data class in PlayerConfig.kt has fields defaulting to
empty strings, which causes PlayerHtmlLoader to inject blank values into
window.jetplay.ui that override Svelte component defaults. Change the
transcodingLabel, transcodingTip, and errorTitle fields in UiStrings to be
nullable (String?) with null as the default instead of empty strings. Then
update PlayerHtmlLoader to filter out null values when injecting the UiStrings
fields into the UI, so that the Svelte component defaults can be applied when a
caller uses PlayerConfig() without custom ui values.
In `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt`:
- Around line 174-200: The temporary `.webm` file created in the runTranscode()
method persists until JVM exit when transcoding succeeds because the finally
block only deletes it when served is false, relying on deleteOnExit() for
cleanup. This causes disk space to accumulate for large videos during the IDE
session. Implement a disposal mechanism (such as a dispose listener on the
bridge) that tracks served temp files and deletes them when the editor/bridge is
disposed, ensuring proper cleanup of transcoded files rather than deferring to
JVM exit. Apply the same disposal pattern to the other affected location at
lines 248-252 that has the identical temp file cleanup issue.
In `@frontend/src/main/kotlin/dev/twango/jetplay/media/MediaByteSource.kt`:
- Line 58: In MediaByteSource.kt at line 58, the MIME type mapping pairs "m4a"
and "aac" together to return "audio/mp4", but AAC files require the MIME type
"audio/aac" instead since AAC is a raw stream format distinct from MP4
containers. Separate the mapping so that "aac" maps to "audio/aac" while "m4a"
continues to map to "audio/mp4", ensuring the correct Content-Type header is
sent by MediaServer for AAC file playback.
---
Outside diff comments:
In `@frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.kt`:
- Around line 94-110: Remove the remote source checks from both media metadata
extraction functions since the new MediaAccessor backend already supports remote
byte access. In maybeSendWaveform(), remove the source.isRemote condition from
the early return statement (keeping only the source.isVideo check). In
maybeSendMediaInfo(), remove the entire early return line that checks if
source.isRemote is true. This will allow remote development users to receive
waveform and media info metadata for remote-backed files.
In `@ui/src/App.svelte`:
- Around line 60-68: Update the test file ui/tests/states.spec.ts to reflect the
removal of the 'downloading' state from the main App.svelte component. Any test
cases that currently seed state: 'downloading' should either be replaced with
state: 'loading' (to test the TranscodingState component path) or removed
entirely if they are no longer relevant. This ensures the tests align with the
updated state handling logic in the if-else chain where 'downloading' is no
longer a valid state option.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba096edc-6771-467d-a508-3d0dd04e4ad0
📒 Files selected for processing (46)
AGENTS.mdbackend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractor.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.ktbackend/src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.ktbuild.gradle.ktsfrontend-split/build.gradle.ktsfrontend-split/src/main/kotlin/dev/twango/jetplay/editor/split/MediaFrontendEditorModelHandler.ktfrontend-split/src/main/resources/dev.twango.jetplay.frontend.split.xmlfrontend/build.gradle.ktsfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerBridge.ktfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerConfig.ktfrontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaCoroutineService.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaErrorEditor.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.ktfrontend/src/main/kotlin/dev/twango/jetplay/editor/MediaLoader.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/MediaByteSource.ktfrontend/src/main/kotlin/dev/twango/jetplay/media/MediaServer.ktfrontend/src/main/resources/dev.twango.jetplay.frontend.xmlfrontend/src/test/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoaderTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/MediaByteSourceTest.ktfrontend/src/test/kotlin/dev/twango/jetplay/media/MediaServerSourceTest.ktsettings.gradle.ktsshared/src/main/kotlin/dev/twango/jetplay/JetPlayBundle.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaClassification.ktshared/src/main/kotlin/dev/twango/jetplay/media/MediaInfo.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessor.ktshared/src/main/kotlin/dev/twango/jetplay/rpc/TranscodeEvent.ktshared/src/main/resources/messages/JetPlayBundle.propertiesshared/src/main/resources/messages/JetPlayBundle_de.propertiesshared/src/main/resources/messages/JetPlayBundle_es.propertiesshared/src/main/resources/messages/JetPlayBundle_fr.propertiesshared/src/main/resources/messages/JetPlayBundle_it.propertiesshared/src/main/resources/messages/JetPlayBundle_ja.propertiesshared/src/main/resources/messages/JetPlayBundle_ko.propertiesshared/src/main/resources/messages/JetPlayBundle_pl.propertiesshared/src/main/resources/messages/JetPlayBundle_pt_BR.propertiesshared/src/main/resources/messages/JetPlayBundle_ru.propertiesshared/src/main/resources/messages/JetPlayBundle_zh_CN.propertiesshared/src/main/resources/messages/JetPlayBundle_zh_TW.propertiessrc/main/resources/META-INF/plugin.xmlui/src/App.svelteui/src/global.d.tsui/src/lib/DownloadingState.svelte
💤 Files with no reviewable changes (17)
- ui/src/lib/DownloadingState.svelte
- shared/src/main/resources/messages/JetPlayBundle_ja.properties
- frontend/src/main/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoader.kt
- frontend/src/main/resources/dev.twango.jetplay.frontend.xml
- shared/src/main/resources/messages/JetPlayBundle_zh_CN.properties
- shared/src/main/resources/messages/JetPlayBundle_zh_TW.properties
- shared/src/main/resources/messages/JetPlayBundle_fr.properties
- frontend/src/test/kotlin/dev/twango/jetplay/browser/PlayerHtmlLoaderTest.kt
- shared/src/main/resources/messages/JetPlayBundle.properties
- shared/src/main/resources/messages/JetPlayBundle_it.properties
- shared/src/main/resources/messages/JetPlayBundle_pl.properties
- shared/src/main/resources/messages/JetPlayBundle_pt_BR.properties
- shared/src/main/resources/messages/JetPlayBundle_ru.properties
- shared/src/main/resources/messages/JetPlayBundle_es.properties
- shared/src/main/resources/messages/JetPlayBundle_ko.properties
- shared/src/main/resources/messages/JetPlayBundle_de.properties
- frontend/build.gradle.kts
✅ Files skipped from review due to trivial changes (2)
- frontend-split/src/main/resources/dev.twango.jetplay.frontend.split.xml
- backend/src/main/kotlin/dev/twango/jetplay/transcode/WaveformExtractor.kt
🚧 Files skipped from review as they are similar to previous changes (12)
- src/main/resources/META-INF/plugin.xml
- settings.gradle.kts
- frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaErrorEditor.kt
- frontend/src/main/kotlin/dev/twango/jetplay/media/EditorMediaSource.kt
- shared/src/main/kotlin/dev/twango/jetplay/media/MediaClassification.kt
- shared/src/main/kotlin/dev/twango/jetplay/rpc/TranscodeEvent.kt
- frontend/src/main/kotlin/dev/twango/jetplay/editor/MediaFileEditorProvider.kt
- shared/src/main/kotlin/dev/twango/jetplay/media/MediaInfo.kt
- backend/src/main/kotlin/dev/twango/jetplay/transcode/MediaTranscoder.kt
- shared/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessor.kt
- backend/src/main/kotlin/dev/twango/jetplay/transcode/MediaInfoExtractor.kt
- backend/src/main/kotlin/dev/twango/jetplay/rpc/MediaAccessorImpl.kt
- serve .aac as audio/aac instead of audio/mp4 - marshal PlayerBridge.loadHtml onto the EDT for off-thread error paths - delete served transcode temps on dispose instead of leaking until JVM exit - emit ui.* config only when non-empty so Svelte defaults stand
- glob frontend-split/*.jar instead of naming rd-client.jar, which EAP builds rename (fixes :frontend-split verify on IU-262) - remove Playwright tests for the deleted downloading state
The module loads only in the JetBrains Client process, so 'client' reads clearer than 'frontend-split' (which collided with the platform's own frontend.split vocabulary). Renames the gradle subproject, content-module id (dev.twango.jetplay.client), descriptor file, and editor.client package. JetBrains-owned names (the intellij.platform.frontend.split dependency and the cwm-plugin/lib/frontend-split jar dir) are unchanged.
…ython recommended() pulls in the latest EAP (e.g. IU-262.7581.18), but EAPs relocate the @ApiStatus.Internal split-mode RD classes the client module compiles against, so the plugin can't build there. Keep only released, year-versioned IDEs; 2026.2 rejoins automatically once it ships. Rewrites the discovery script in Python (clearer than the bash+jq regex) and dedupes the matrix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
In Remote Development the JCEF browser runs on the frontend client while jetplay's
MediaServerbinds the backend loopback, so the<video>src is unreachable and playback spins forever. Official plugins (Markdown, image viewer) fix this by running the viewer frontend-side and pulling bytes from the backend over RPC — this PR does the same via Plugin Model V2.What
shared(RPC contract + media types),frontend(editor, JCEF player, loopbackMediaServer— now client-side, so Range/206 seeking is intact),backend(ffmpeg/bytedeco +remoteApiProvider; natives stay off the client).FlowRPC into a frontend temp file. Monolith/local IDE keeps a direct-serve fast path, so local behavior is unchanged.MediaServertracing.pluginSinceBuild223 → 253 (V2 floor).Notes
./gradlew clean buildgreen (87 tests, detekt, kover); plugin verifier Compatible across the matrix.Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes