Add cloud integration layer with Kaltura as first provider#415
Closed
zoharbabin wants to merge 4 commits intosiddharthvaddem:mainfrom
Closed
Add cloud integration layer with Kaltura as first provider#415zoharbabin wants to merge 4 commits intosiddharthvaddem:mainfrom
zoharbabin wants to merge 4 commits intosiddharthvaddem:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds an opt-in Kaltura integration: Electron main/service + IPC handlers, preload/types, kaltura-client dependency, renderer React UI (login, settings, browse, upload), session persistence/encryption, chunked uploads/downloads with progress, i18n, docs, and minor UI wiring across editor and launch screens. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Connect OpenScreen to the Kaltura video platform for cloud-based video storage. Users can sign in to their Kaltura account, upload recordings with metadata (title, description, tags, categories), and browse/load videos from their Kaltura library back into the editor. Features: - Kaltura sign-in with multi-account support and account switching - Upload to Kaltura with real-time progress tracking - Browse and search Kaltura media library - Load any Kaltura video into the editor for further editing - Free account signup flow from within the app - Encrypted session persistence via Electron safeStorage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Restructure UI as generic "Cloud providers" dropdown (extensible for YouTube, Facebook, etc.) instead of hardcoded Kaltura buttons - Fix Kaltura entitlement: add disableentitlement privilege to KS for category assignment via categoryEntry service - Extract shared KalturaLoginForm component from Settings/Browse dialogs - Add Sign Out button to browse window header - Add "Open in Kaltura" link on upload success - Auto-trigger export before upload when no exported file exists - Fix z-index layering for settings dialog and category dropdown - Fix Electron no-drag regions for Radix UI portal menus - Add KALTURA.md architecture documentation - i18n: add cloud provider strings for en/es/zh-CN Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
zoharbabin
force-pushed
the
feat/kaltura-cloud-integration
branch
from
262209b to
f0030ce
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 262209b03f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/video-editor/ExportDialog.tsx (1)
50-59:⚠️ Potential issue | 🟠 MajorDon't auto-close the success state when there's a follow-up action.
Now that the success view contains Upload to Kaltura, this 2-second timer makes the CTA kinda dead-on-arrival. Keep the dialog open for MP4 +
onUploadToKaltura, or only auto-close after explicit dismiss.nit: cleaner fix
useEffect(() => { if (!isExporting && progress && progress.percentage >= 100 && !error) { setShowSuccess(true); - const timer = setTimeout(() => { - setShowSuccess(false); - onClose(); - }, 2000); - return () => clearTimeout(timer); + if (!onUploadToKaltura || exportFormat !== "mp4") { + const timer = setTimeout(() => { + setShowSuccess(false); + onClose(); + }, 2000); + return () => clearTimeout(timer); + } } -}, [isExporting, progress, error, onClose]); +}, [isExporting, progress, error, onClose, onUploadToKaltura, exportFormat]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/ExportDialog.tsx` around lines 50 - 59, The current useEffect in ExportDialog.tsx auto-closes the success view after 2s unconditionally, which hides the "Upload to Kaltura" CTA; change the effect so it only starts the 2s timer when the success state has no follow-up action: i.e., keep the dialog open if onUploadToKaltura is provided and the export format is MP4 (check props like onUploadToKaltura and exportFormat or similar), otherwise keep the existing setShowSuccess(true) + timer -> setShowSuccess(false) + onClose behavior; ensure you reference isExporting, progress.percentage, error, setShowSuccess, onClose and onUploadToKaltura in the condition and return the clearTimeout cleanup as before.
🧹 Nitpick comments (1)
src/vite-env.d.ts (1)
126-226: export a sharedElectronAPIinterface instead of hand-copying the bridge contract.
src/vite-env.d.tsis already drifting from the actual contract inelectron/electron-env.d.tsandelectron/preload.ts. methods likereadBinaryFile,getPlatform,revealInFolder,getShortcuts,saveShortcuts,hudOverlayHide,hudOverlayCloseexist in both those files but not here. each new kaltura method you add to this copy makes future type breakage lowkey inevitable. cleaner move: export oneElectronAPIinterface from the preload contract and import it everywhere. kills the duplication problem altogether.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vite-env.d.ts` around lines 126 - 226, The current vite-env.d.ts copy of the renderer bridge is drifting and duplicating the preload contract; remove the hand-copied Kaltura/Electron method declarations and instead re-export the canonical ElectronAPI interface from the preload/electron contract (the one that defines ElectronAPI) and update the ambient declaration to reference that single interface; ensure the shared interface includes methods like readBinaryFile, getPlatform, revealInFolder, getShortcuts, saveShortcuts, hudOverlayHide, hudOverlayClose and all kaltura* methods (kalturaLogin, kalturaUpload, onKalturaUploadProgress, kalturaGetSessionState, etc.) so consumers use the single exported ElectronAPI type rather than a duplicated copy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/kaltura/kaltura-ipc.ts`:
- Around line 54-66: Validate the renderer-supplied options.filePath inside the
ipcMain.handle("kaltura-upload") handler before calling uploadToKaltura: check
that options.filePath is present and call the existing approval logic (e.g.,
isPathAllowed(path) or compare against approvedPaths/export paths) and
reject/throw (or return an error result) if the path is not allowed; only then
call uploadToKaltura(options, onProgress) and return the result with uploadId.
Ensure you reference the UploadOptions.filePath property and reuse the same
isPathAllowed/approvedPaths guard used elsewhere to prevent client-supplied
arbitrary file access.
In `@electron/kaltura/kaltura-service.ts`:
- Around line 757-770: downloadFromKaltura is using currentKs directly and
bypasses the ks refresh logic, so when ksExpiry passes downloads fail; update
downloadFromKaltura to call getAuthenticatedClient (or reuse the same refresh
path used by uploadToKaltura/listCategories) to obtain a refreshed session and
ks (check ksExpiry/currentSession/currentKs via getAuthenticatedClient), then
use the returned session.serviceUrl, session.partnerId and refreshed ks when
building the playManifest downloadUrl instead of using currentKs directly.
- Around line 851-865: The current implementation using new
kaltura.objects.CategoryFilter(), new kaltura.objects.FilterPager(),
pager.pageSize = CATEGORY_PAGE_SIZE and a single
execKaltura(kaltura.services.category.listAction(filter, pager), client) call
only fetches page 1; update the logic to page through results (call listAction
repeatedly while result.objects length equals pager.pageSize or until totalCount
exhausted) by setting pager.pageIndex for each iteration, accumulating objects
into the categories array, and only return success when all pages have been
fetched; reuse execKaltura, CATEGORY_PAGE_SIZE, and map each returned object to
{id, name, fullName} as done now.
- Around line 338-346: The login function currently accepts any serviceUrl and
immediately calls loginByLoginId; add upfront validation/normalization in login
to parse and enforce a strict serviceUrl policy (require https scheme, normalize
trailing slashes, reject IPs if desired) and check the hostname against a
maintained allowlist of supported Kaltura hosts before any credentials are sent;
if validation fails, throw a clear error and do not call loginByLoginId (also
ensure subsequent callers that reuse the base URL rely on this validated value),
referencing the login function and loginByLoginId to locate where to add the
check and error handling.
- Around line 801-823: The manual reader loop using body.getReader() and direct
fileStream.write() ignores backpressure and must be replaced by streaming with
pipeline: create a Node Readable via Readable.fromWeb(body), pipe it through a
Transform (e.g., a bytes-counting transformer that increments a local downloaded
counter, calls onProgress when contentLength > 0, and forwards chunks), and then
into createWriteStream(destPath); use the promisified pipeline function to
connect Readable.fromWeb(body) -> Transform -> fileStream so backpressure,
finish, and errors are handled automatically instead of the manual
reader/finish/error listeners (replace the reader/read loop and the
fileStream.end + finish/error Promise with the pipeline usage and keep
onProgress/contentLength logic inside the Transform).
In `@electron/windows.ts`:
- Around line 40-42: There are two places in electron/windows.ts where
webSecurity: false is set; identify both BrowserWindow option objects (the one
documented for Kaltura CDN widget and the one used for the editor window, e.g.,
createMainWindow / createEditorWindow or similar constructors) and either remove
the webSecurity: false from the editor window options or add a clear
justification comment there; if the editor truly needs relaxed webSecurity, add
a comment explaining the exact reason and restrict allowed origins via a
Content-Security-Policy header or whitelist specific remote origins (e.g.,
Kaltura CDN) instead of disabling webSecurity globally.
In `@KALTURA.md`:
- Around line 99-103: Update the documentation to match the implementation:
replace the step that says Kaltura downloads are streamed to a temp directory
with a statement that kaltura-service.ts writes downloaded files into the app
recordings directory; reference the kalturaDownload flow and the events
kaltura-download-progress and kaltura-video-loaded so readers know the service
and events involved and avoid chasing the wrong path when debugging
import/approval issues.
In `@README.md`:
- Around line 25-28: The fenced code block under the "### End-to-End Workflow"
header is missing a language specifier which triggers markdownlint; update the
triple-backtick fence that surrounds `Record → Edit → Save to Cloud → Load
Anywhere` to include a language token (e.g., change ``` to ```text) so the block
is valid and lint-friendly while preserving the displayed workflow.
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 203-210: The onKalturaVideoLoaded callback always calls
window.electronAPI.switchToEditor regardless of whether setCurrentVideoPath
succeeded; change the handler in LaunchWindow.tsx so you await and check the
result of window.electronAPI.setCurrentVideoPath (or catch exceptions) and only
call window.electronAPI.switchToEditor when setCurrentVideoPath indicates
success; keep the existing comment about not calling
setCurrentRecordingSession(null), and ensure errors are handled (return early or
log) to prevent navigating to the editor with no video.
In `@src/components/video-editor/KalturaBrowseDialog.tsx`:
- Around line 160-177: The code is resolving media by display name (entryName)
which can pick the wrong asset; instead use the stable entry id provided by the
widget/runtime: change the lookup to use the widget's entry id (e.g., entryId)
when calling handleEntrySelected rather than searching by entryName, or if you
must fetch list results, find the object by its id (objects.find(o => o.id ===
entryId)) and do not fall back to objects[0]; update the fetch.then block to
read sessionInfo/service response but select by the known entry id and then call
handleEntrySelected(entryId, entryName).
- Around line 93-97: The dynamic runtime import that builds loaderUrl and does
await import(/* `@vite-ignore` */ loaderUrl) in KalturaBrowseDialog.tsx should not
fetch remote JS in the renderer; replace this pattern by bundling the Kaltura
loader locally and importing it statically (or moving its loading into an
isolated context). Specifically, remove the runtime construction of loaderUrl
and the await import of loader; instead add the vendored loader into the
codebase and import its exported loader symbol directly, or if you must keep it
external, load it inside a sandboxed BrowserWindow/webview with no electronAPI
bridge exposed so the renderer never executes third-party code with privileged
access.
In `@src/components/video-editor/KalturaLoginForm.tsx`:
- Around line 114-117: The handler handleSignup currently sets clickedSignup
immediately before calling window.electronAPI.kalturaOpenSignup(), so UI shows
success even if the IPC call fails; change handleSignup to await the result of
window.electronAPI.kalturaOpenSignup(), only call setClickedSignup(true) after
the promise resolves successfully, and call the component's onError callback (or
propagate the error) if kalturaOpenSignup rejects or returns a failure so
failures are reported instead of flipping clickedSignup prematurely.
In `@src/components/video-editor/KalturaUploadDialog.tsx`:
- Around line 52-79: The effect that runs when the KalturaUploadDialog opens
currently leaves stale category data from a previous account; clear the
categories state immediately when isOpen becomes true and mark a failure state
if the fetch fails so stale ids aren't selectable. Specifically, in the
useEffect that calls window.electronAPI.kalturaListCategories(), call
setCategories([]) (or clearCategories) at the start of the open-path, and in the
.then/.catch handlers update categories only on success and set an explicit
failure flag (e.g., setIsLoadingCategories(false) already exists—add
setCategories([]) on failure or setUploadError/state like setCategoryLoadError)
so UI disables or indicates category loading failure instead of reusing old
categories; reference the useEffect, setCategories, kalturaListCategories, and
setIsLoadingCategories identifiers when making the change.
- Around line 82-125: The progress listener currently accepts all
kaltura-upload-progress events; capture the uploadId returned by
window.electronAPI.kalturaUpload inside handleUpload (result.uploadId) and store
it (use a state or a ref like currentUploadId). Update the useEffect that
registers window.electronAPI.onKalturaUploadProgress to ignore any progress
whose progress.uploadId !== currentUploadId, and when you detect phase
"complete" or "error" clear currentUploadId and update
setIsUploading/setUploadError/setUploadProgress as before. Ensure cleanup still
unregisters the listener and that currentUploadId is reset on catch paths where
upload failed to start.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 1996-2000: The onLogout handler is doing more than clearing
Kaltura state by calling window.electronAPI.startNewRecording(); remove that
call so onLogout only clears UI/state (call setShowKalturaSettings(false) and
setKalturaConnected(false)) and does not trigger any recording; locate the
onLogout callback in VideoEditor.tsx and delete or comment out the
window.electronAPI.startNewRecording() invocation.
- Around line 1979-1986: The pending-upload path unconditionally opens
KalturaUploadDialog even when exportedFilePath is null, causing a broken upload;
modify the pendingKalturaUploadRef handling in the
window.electronAPI.kalturaGetSessionState() callback (the block using
pendingKalturaUploadRef.current, pendingKalturaBrowseRef.current,
setShowKalturaUpload) to check exportedFilePath (the exported file path
state/variable) and if it's null/empty either call handleUploadToKaltura() to
trigger export+upload or branch to start the export first before calling
setShowKalturaUpload(true); ensure you preserve the existing
pendingKalturaBrowseRef clearing semantics and only open the dialog when an
exported file path exists or after handleUploadToKaltura completes.
---
Outside diff comments:
In `@src/components/video-editor/ExportDialog.tsx`:
- Around line 50-59: The current useEffect in ExportDialog.tsx auto-closes the
success view after 2s unconditionally, which hides the "Upload to Kaltura" CTA;
change the effect so it only starts the 2s timer when the success state has no
follow-up action: i.e., keep the dialog open if onUploadToKaltura is provided
and the export format is MP4 (check props like onUploadToKaltura and
exportFormat or similar), otherwise keep the existing setShowSuccess(true) +
timer -> setShowSuccess(false) + onClose behavior; ensure you reference
isExporting, progress.percentage, error, setShowSuccess, onClose and
onUploadToKaltura in the condition and return the clearTimeout cleanup as
before.
---
Nitpick comments:
In `@src/vite-env.d.ts`:
- Around line 126-226: The current vite-env.d.ts copy of the renderer bridge is
drifting and duplicating the preload contract; remove the hand-copied
Kaltura/Electron method declarations and instead re-export the canonical
ElectronAPI interface from the preload/electron contract (the one that defines
ElectronAPI) and update the ambient declaration to reference that single
interface; ensure the shared interface includes methods like readBinaryFile,
getPlatform, revealInFolder, getShortcuts, saveShortcuts, hudOverlayHide,
hudOverlayClose and all kaltura* methods (kalturaLogin, kalturaUpload,
onKalturaUploadProgress, kalturaGetSessionState, etc.) so consumers use the
single exported ElectronAPI type rather than a duplicated copy.
🪄 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: CHILL
Plan: Pro
Run ID: ec355ede-8be6-419f-8744-85b7a6b92080
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
.gitignoreKALTURA.mdREADME.mdelectron/electron-env.d.tselectron/ipc/handlers.tselectron/kaltura/kaltura-ipc.tselectron/kaltura/kaltura-service.tselectron/main.tselectron/preload.tselectron/windows.tspackage.jsonsrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/ExportDialog.tsxsrc/components/video-editor/KalturaBrowseDialog.tsxsrc/components/video-editor/KalturaLoginForm.tsxsrc/components/video-editor/KalturaSettingsDialog.tsxsrc/components/video-editor/KalturaUploadDialog.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/i18n/config.tssrc/i18n/locales/en/kaltura.jsonsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/es/kaltura.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/kaltura.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/index.csssrc/vite-env.d.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (10)
src/components/launch/LaunchWindow.tsx (1)
203-210:⚠️ Potential issue | 🟠 MajorOnly switch to the editor after
setCurrentVideoPath()succeeds.Right now this still navigates even if the import/store step fails, which can dump users into an empty editor. Guard the IPC result and bail early on error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/launch/LaunchWindow.tsx` around lines 203 - 210, The handler registered in useEffect via window.electronAPI.onKalturaVideoLoaded calls setCurrentVideoPath and then unconditionally switches to the editor; change it to await/set and check the IPC result from setCurrentVideoPath (or catch its thrown error) and only call window.electronAPI.switchToEditor() when setCurrentVideoPath succeeded; if setCurrentVideoPath returns an error result or throws, bail early and do not call switchToEditor (do not clear setCurrentRecordingSession), and ensure any errors are handled/logged so the editor is not entered with no video.src/components/video-editor/VideoEditor.tsx (2)
2161-2168:⚠️ Potential issue | 🟠 MajorDon't open
KalturaUploadDialoguntil an export actually exists.If the user clicked Upload while disconnected and
exportedFilePathis still empty, this path opens the dialog withfilePath=""and the flow is dead. Re-runhandleUploadToKaltura()here, or branch to export first instead of callingsetShowKalturaUpload(true)unconditionally.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 2161 - 2168, The pending-Kaltura branch opens KalturaUploadDialog unconditionally even if no export exists; update the success branch in the window.electronAPI.kalturaGetSessionState().then handler to check exportedFilePath (or call handleUploadToKaltura()) before calling setShowKalturaUpload(true): if exportedFilePath is empty, clear pendingKalturaUploadRef/current and invoke handleUploadToKaltura() (or trigger the export flow) so an export is produced first, otherwise setShowKalturaUpload(true); keep the existing pendingKalturaBrowseRef handling unchanged.
2178-2182:⚠️ Potential issue | 🟠 MajorSigning out shouldn't kick off a new recording.
Cloud logout shouldn't yank the user into a fresh capture flow. This callback should just clear Kaltura UI/session state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/VideoEditor.tsx` around lines 2178 - 2182, The onLogout callback currently clears Kaltura state and also starts a new recording via window.electronAPI.startNewRecording(); remove the call to startNewRecording so the logout only clears UI/session state (keep setShowKalturaSettings(false) and setKalturaConnected(false) in the onLogout handler inside VideoEditor.tsx) to avoid kicking the user into a new capture flow on cloud logout.electron/kaltura/kaltura-ipc.ts (1)
54-66:⚠️ Potential issue | 🟠 MajorValidate
options.filePathin main before uploading.This still trusts a renderer-supplied path and sends whatever readable file it points at upstream. In Electron that's lowkey risky: a compromised renderer can exfiltrate arbitrary local files unless this reuses the same approved-path guard used elsewhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/kaltura/kaltura-ipc.ts` around lines 54 - 66, Validate options.filePath in the ipcMain handler before calling uploadToKaltura: check that options.filePath is present, points to a file (not a directory), and is allowed by your existing approved-path guard (reuse the same guard/function used elsewhere in the codebase), and reject/throw if the check fails so uploadToKaltura is never called with an unvalidated renderer-supplied path; keep the existing onProgress/event.sender.send logic but perform this validation in the "kaltura-upload" ipcMain.handle callback prior to invoking uploadToKaltura.src/components/video-editor/KalturaUploadDialog.tsx (2)
70-79:⚠️ Potential issue | 🟠 MajorClear stale categories before reloading them.
Reopening this dialog after an account switch can keep the previous account’s categories around until the new fetch wins, and a failed fetch leaves those stale ids selectable. Reset
categorieson open and clear them on load failure too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/KalturaUploadDialog.tsx` around lines 70 - 79, When loading Kaltura categories in the KalturaUploadDialog, clear any stale categories before the fetch and also clear them on failure: call setCategories([]) right before invoking window.electronAPI.kalturaListCategories(), and ensure the promise path that handles failure (the .then branch when result.success is false and the .catch path if using one) also calls setCategories([]) so stale ids aren’t left selectable; keep setIsLoadingCategories(true) / .finally(() => setIsLoadingCategories(false)) as is.
82-125:⚠️ Potential issue | 🟠 MajorFilter progress events by the upload you just started.
kaltura-upload-progressis a shared channel, but this dialog still accepts every event and never pins itself toresult.uploadId. Another upload can hijack this modal’s state pretty easily.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/KalturaUploadDialog.tsx` around lines 82 - 125, The progress listener accepts all shared events and needs to be scoped to the upload this dialog started: add a piece of state (e.g., currentUploadId) set in handleUpload from the result.uploadId returned by window.electronAPI.kalturaUpload, then change the useEffect listener (the one using window.electronAPI.onKalturaUploadProgress) to ignore progress events whose uploadId !== currentUploadId and to clear currentUploadId when phase is "complete" or "error"; also ensure you clear currentUploadId when the dialog closes and keep the existing cleanup return from useEffect. This uses the existing symbols useEffect, onKalturaUploadProgress, handleUpload, kalturaUpload, setUploadProgress, setIsUploading, setUploadError and currentUploadId (new).src/components/video-editor/KalturaBrowseDialog.tsx (2)
93-96:⚠️ Potential issue | 🔴 CriticalDon't execute Kaltura's remote loader inside this renderer.
Pulling an ESM bundle from Kaltura at runtime gives third-party code this window’s DOM plus the exposed
window.electronAPIbridge. In Electron that’s kinda cursed from a supply-chain standpoint. Bundle the loader locally, or isolate it in a window/webview with no privileged bridge.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/KalturaBrowseDialog.tsx` around lines 93 - 96, The code dynamically imports Kaltura's remote ESM loader via loaderUrl (serverUrl/loaderUrl) inside KalturaBrowseDialog, which exposes this window and window.electronAPI to third‑party code; replace this runtime remote import with a safe approach: vendor/package the Kaltura loader into our repo and import it locally (use a local module instead of constructing loaderUrl and await import(/* `@vite-ignore` */ loaderUrl)), or move remote loading into an isolated context (create a dedicated BrowserWindow or webview with NO privileged electronAPI bridge) and communicate only sanitized results back to KalturaBrowseDialog; update references to loader, loaderUrl and serverUrl accordingly.
160-177:⚠️ Potential issue | 🟠 MajorResolve media by a stable entry id, not display name.
This lookup still throws away the widget’s identity and re-finds media by
entryName, then falls back toobjects[0]. Duplicate titles or fuzzy matches can download the wrong asset.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/KalturaBrowseDialog.tsx` around lines 160 - 177, The code is resolving media by display name and falling back to objects[0], which risks picking the wrong asset; change KalturaBrowseDialog to prefer a stable entry id (use the widget-provided id/prop or a passed-in entryId) and, in the fetch result handling inside the function that calls handleEntrySelected, look up the entry by its id (e.g., find(e => e.id === providedEntryId)) instead of by name, and do not fall back to objects[0]; if the id is missing or not found, surface an explicit error (throw or call the UI error handler) rather than using the first search result. Ensure handleEntrySelected is invoked with the resolved id and name only after a successful id match.README.md (1)
25-28:⚠️ Potential issue | 🟡 MinorAdd a language to this fenced block.
markdownlint is still gonna complain here.
textis enough and keeps the rendering identical.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 25 - 28, The fenced code block under the "End-to-End Workflow" heading is missing a language identifier which triggers markdownlint; update the triple-backtick fence around "Record → Edit → Save to Cloud → Load Anywhere" to include a language (use "text") so the block becomes ```text and keeps rendering identical.src/components/video-editor/KalturaLoginForm.tsx (1)
114-117:⚠️ Potential issue | 🟡 MinorOnly show signup success after the IPC call actually succeeds.
This is still flipping the success state before
kalturaOpenSignup()resolves, so a blocked/failed browser launch tells the user everything worked. Await it, and send failures throughonErrorinstead.small fix
-const handleSignup = useCallback(() => { - setClickedSignup(true); - window.electronAPI.kalturaOpenSignup(); -}, []); +const handleSignup = useCallback(async () => { + try { + await window.electronAPI.kalturaOpenSignup(); + setClickedSignup(true); + } catch (err) { + onError(String(err)); + } +}, [onError]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/KalturaLoginForm.tsx` around lines 114 - 117, handleSignup currently sets setClickedSignup(true) before the IPC call resolves; change it to await window.electronAPI.kalturaOpenSignup() inside handleSignup (which is an async function) and only call setClickedSignup(true) after the promise resolves successfully; catch errors from kalturaOpenSignup and call the component's onError handler with the error instead of marking signup as successful; also update the useCallback dependency list to include onError (and any other used variables) to avoid stale closures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/video-editor/KalturaSettingsDialog.tsx`:
- Around line 32-49: The useEffect block calling
window.electronAPI.kalturaLoadSession() needs a guarded .catch and should avoid
late updates when the dialog is closed: update the useEffect (the effect that
checks isOpen) to attach a .catch handler to kalturaLoadSession() that calls
setError(...) and setPhase("error" or "logged_out") on failure, and ensure
before calling setConnected/setPhase/setError you re-check isOpen (or use an
aborted/active flag inside the effect) so setConnected, setPhase, and setError
are not invoked after the dialog has closed; reference the useEffect,
kalturaLoadSession, setConnected, setPhase, setError and isOpen symbols when
making the change.
- Around line 114-132: handleLogout and handleSwitchAccount call
window.electronAPI.kalturaLogout / kalturaListPartners without error handling,
so an IPC rejection can leave stale UI state; wrap both handlers in try/catch
blocks around the await calls (kalturaLogout and kalturaListPartners), move
state updates (setConnected, setPhase, setPartners, setError, onLogout) into the
success path, and in the catch branch call kalturaLogout if appropriate, clear
connected state (setConnected(null)), set an error via setError(...) and
setPhase("logged_out") so the UI is consistent; keep existing success/error
branches for kalturaListPartners but ensure any thrown errors are caught and
handled similarly.
---
Duplicate comments:
In `@electron/kaltura/kaltura-ipc.ts`:
- Around line 54-66: Validate options.filePath in the ipcMain handler before
calling uploadToKaltura: check that options.filePath is present, points to a
file (not a directory), and is allowed by your existing approved-path guard
(reuse the same guard/function used elsewhere in the codebase), and reject/throw
if the check fails so uploadToKaltura is never called with an unvalidated
renderer-supplied path; keep the existing onProgress/event.sender.send logic but
perform this validation in the "kaltura-upload" ipcMain.handle callback prior to
invoking uploadToKaltura.
In `@README.md`:
- Around line 25-28: The fenced code block under the "End-to-End Workflow"
heading is missing a language identifier which triggers markdownlint; update the
triple-backtick fence around "Record → Edit → Save to Cloud → Load
Anywhere" to include a language (use "text") so the block becomes ```text and
keeps rendering identical.
In `@src/components/launch/LaunchWindow.tsx`:
- Around line 203-210: The handler registered in useEffect via
window.electronAPI.onKalturaVideoLoaded calls setCurrentVideoPath and then
unconditionally switches to the editor; change it to await/set and check the IPC
result from setCurrentVideoPath (or catch its thrown error) and only call
window.electronAPI.switchToEditor() when setCurrentVideoPath succeeded; if
setCurrentVideoPath returns an error result or throws, bail early and do not
call switchToEditor (do not clear setCurrentRecordingSession), and ensure any
errors are handled/logged so the editor is not entered with no video.
In `@src/components/video-editor/KalturaBrowseDialog.tsx`:
- Around line 93-96: The code dynamically imports Kaltura's remote ESM loader
via loaderUrl (serverUrl/loaderUrl) inside KalturaBrowseDialog, which exposes
this window and window.electronAPI to third‑party code; replace this runtime
remote import with a safe approach: vendor/package the Kaltura loader into our
repo and import it locally (use a local module instead of constructing loaderUrl
and await import(/* `@vite-ignore` */ loaderUrl)), or move remote loading into an
isolated context (create a dedicated BrowserWindow or webview with NO privileged
electronAPI bridge) and communicate only sanitized results back to
KalturaBrowseDialog; update references to loader, loaderUrl and serverUrl
accordingly.
- Around line 160-177: The code is resolving media by display name and falling
back to objects[0], which risks picking the wrong asset; change
KalturaBrowseDialog to prefer a stable entry id (use the widget-provided id/prop
or a passed-in entryId) and, in the fetch result handling inside the function
that calls handleEntrySelected, look up the entry by its id (e.g., find(e =>
e.id === providedEntryId)) instead of by name, and do not fall back to
objects[0]; if the id is missing or not found, surface an explicit error (throw
or call the UI error handler) rather than using the first search result. Ensure
handleEntrySelected is invoked with the resolved id and name only after a
successful id match.
In `@src/components/video-editor/KalturaLoginForm.tsx`:
- Around line 114-117: handleSignup currently sets setClickedSignup(true) before
the IPC call resolves; change it to await window.electronAPI.kalturaOpenSignup()
inside handleSignup (which is an async function) and only call
setClickedSignup(true) after the promise resolves successfully; catch errors
from kalturaOpenSignup and call the component's onError handler with the error
instead of marking signup as successful; also update the useCallback dependency
list to include onError (and any other used variables) to avoid stale closures.
In `@src/components/video-editor/KalturaUploadDialog.tsx`:
- Around line 70-79: When loading Kaltura categories in the KalturaUploadDialog,
clear any stale categories before the fetch and also clear them on failure: call
setCategories([]) right before invoking
window.electronAPI.kalturaListCategories(), and ensure the promise path that
handles failure (the .then branch when result.success is false and the .catch
path if using one) also calls setCategories([]) so stale ids aren’t left
selectable; keep setIsLoadingCategories(true) / .finally(() =>
setIsLoadingCategories(false)) as is.
- Around line 82-125: The progress listener accepts all shared events and needs
to be scoped to the upload this dialog started: add a piece of state (e.g.,
currentUploadId) set in handleUpload from the result.uploadId returned by
window.electronAPI.kalturaUpload, then change the useEffect listener (the one
using window.electronAPI.onKalturaUploadProgress) to ignore progress events
whose uploadId !== currentUploadId and to clear currentUploadId when phase is
"complete" or "error"; also ensure you clear currentUploadId when the dialog
closes and keep the existing cleanup return from useEffect. This uses the
existing symbols useEffect, onKalturaUploadProgress, handleUpload,
kalturaUpload, setUploadProgress, setIsUploading, setUploadError and
currentUploadId (new).
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2161-2168: The pending-Kaltura branch opens KalturaUploadDialog
unconditionally even if no export exists; update the success branch in the
window.electronAPI.kalturaGetSessionState().then handler to check
exportedFilePath (or call handleUploadToKaltura()) before calling
setShowKalturaUpload(true): if exportedFilePath is empty, clear
pendingKalturaUploadRef/current and invoke handleUploadToKaltura() (or trigger
the export flow) so an export is produced first, otherwise
setShowKalturaUpload(true); keep the existing pendingKalturaBrowseRef handling
unchanged.
- Around line 2178-2182: The onLogout callback currently clears Kaltura state
and also starts a new recording via window.electronAPI.startNewRecording();
remove the call to startNewRecording so the logout only clears UI/session state
(keep setShowKalturaSettings(false) and setKalturaConnected(false) in the
onLogout handler inside VideoEditor.tsx) to avoid kicking the user into a new
capture flow on cloud logout.
🪄 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: CHILL
Plan: Pro
Run ID: ea3601e3-3e91-4923-aac5-30f468ffb060
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
.gitignoreKALTURA.mdREADME.mdelectron/electron-env.d.tselectron/ipc/handlers.tselectron/kaltura/kaltura-ipc.tselectron/kaltura/kaltura-service.tselectron/main.tselectron/preload.tselectron/windows.tspackage.jsonsrc/App.tsxsrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/ExportDialog.tsxsrc/components/video-editor/KalturaBrowseDialog.tsxsrc/components/video-editor/KalturaLoginForm.tsxsrc/components/video-editor/KalturaSettingsDialog.tsxsrc/components/video-editor/KalturaUploadDialog.tsxsrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/i18n/config.tssrc/i18n/locales/en/kaltura.jsonsrc/i18n/locales/en/launch.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/editor.jsonsrc/i18n/locales/es/kaltura.jsonsrc/i18n/locales/es/launch.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/zh-CN/editor.jsonsrc/i18n/locales/zh-CN/kaltura.jsonsrc/i18n/locales/zh-CN/launch.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/index.csssrc/vite-env.d.ts
✅ Files skipped from review due to trivial changes (14)
- .gitignore
- package.json
- src/i18n/locales/es/launch.json
- src/i18n/locales/zh-CN/launch.json
- src/i18n/locales/en/launch.json
- src/i18n/locales/zh-CN/editor.json
- src/i18n/locales/es/editor.json
- src/i18n/locales/en/settings.json
- src/i18n/locales/zh-CN/settings.json
- src/i18n/locales/es/kaltura.json
- src/i18n/locales/zh-CN/kaltura.json
- src/index.css
- src/i18n/locales/en/kaltura.json
- KALTURA.md
🚧 Files skipped from review as they are similar to previous changes (10)
- src/App.tsx
- src/i18n/config.ts
- electron/windows.ts
- electron/main.ts
- src/i18n/locales/es/settings.json
- electron/ipc/handlers.ts
- src/components/video-editor/SettingsPanel.tsx
- src/components/video-editor/ExportDialog.tsx
- src/vite-env.d.ts
- electron/kaltura/kaltura-service.ts
- Validate serviceUrl (require HTTPS, normalize) in login flow - URL-encode KS token in download URL to prevent injection - Use getAuthenticatedClient in download for session refresh - Add pagination to listCategories for large category trees - Validate upload file paths in IPC handler (absolute, no traversal) - Add .catch handlers to session restore promises - Wrap logout/switchAccount in try/catch for resilience - Filter upload progress events by uploadId to ignore stale events - Clear stale categories when upload dialog reopens - Remove unsafe objects[0] fallback in browse entry lookup - Make handleSignup async to await IPC before updating state - Keep export dialog open when Upload to Kaltura CTA is available - Guard LaunchWindow video load with error handling - Remove startNewRecording side-effect from settings logout - Add security justification comments for webSecurity: false - Add architecture comment for remote Kaltura widget loader - Fix docs: code fence language tag, recordings directory reference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
KALTURA.md (2)
17-25: nit: add language specifier to fenced code blocksmarkdownlint is flagging these blocks.
textorplaintextworks if you just want it rendered as-is.🔧 Proposed fix
-``` +```text Renderer (React) Main Process (Node)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@KALTURA.md` around lines 17 - 25, The fenced code block in KALTURA.md is missing a language specifier which triggers markdownlint; add a language token (e.g., "text" or "plaintext") after the opening triple backticks for the ASCII diagram so the block is treated as plain text (update the block shown with the Renderer/Main Process diagram that currently starts with ``` to ```text).
29-44: nit: same thing here — missing language specifier🔧 Proposed fix
-``` +```text electron/ kaltura/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@KALTURA.md` around lines 29 - 44, The fenced code block showing the directory tree in KALTURA.md is missing a language specifier; update the triple-backtick fence that wraps the lines starting with "electron/" through "kaltura.json" to include a language tag (e.g., ```text or ```bash) so the block is properly identified and rendered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@KALTURA.md`:
- Around line 17-25: The fenced code block in KALTURA.md is missing a language
specifier which triggers markdownlint; add a language token (e.g., "text" or
"plaintext") after the opening triple backticks for the ASCII diagram so the
block is treated as plain text (update the block shown with the Renderer/Main
Process diagram that currently starts with ``` to ```text).
- Around line 29-44: The fenced code block showing the directory tree in
KALTURA.md is missing a language specifier; update the triple-backtick fence
that wraps the lines starting with "electron/" through "kaltura.json" to include
a language tag (e.g., ```text or ```bash) so the block is properly identified
and rendered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae462baa-70cb-4929-8b45-a4d4f775c659
📒 Files selected for processing (12)
KALTURA.mdREADME.mdelectron/kaltura/kaltura-ipc.tselectron/kaltura/kaltura-service.tselectron/windows.tssrc/components/launch/LaunchWindow.tsxsrc/components/video-editor/ExportDialog.tsxsrc/components/video-editor/KalturaBrowseDialog.tsxsrc/components/video-editor/KalturaLoginForm.tsxsrc/components/video-editor/KalturaSettingsDialog.tsxsrc/components/video-editor/KalturaUploadDialog.tsxsrc/components/video-editor/VideoEditor.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/video-editor/KalturaLoginForm.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- electron/kaltura/kaltura-ipc.ts
- src/components/video-editor/ExportDialog.tsx
- src/components/video-editor/KalturaUploadDialog.tsx
- electron/windows.ts
- electron/kaltura/kaltura-service.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
Addressing remaining nitpick commentsKALTURA.md fenced code blocks (review nitpick) — Fixed in 9aca831. Added vite-env.d.ts type duplication (review nitpick) — Acknowledged. The duplication between All review comments from both CodeRabbit and Codex have now been addressed or acknowledged. |
Owner
|
@zoharbabin will be closing this as there are no plans to add any external provider or cloud solutions. If someone wants something like that I'd recommend they stick to the commercial apps |
Author
|
Thanks for taking the time to review this, @siddharthvaddem — totally respect the decision. Just wanted to note for anyone who stumbles on this later: the integration was intentionally designed to be provider-agnostic. The UI surfaces a generic "Cloud" menu, and individual providers (Kaltura in this case) plug in behind it. The idea was to make it straightforward to add other providers down the road. It enables sharing/publishing to external systems rather than creating a dependency on them — OpenScreen works exactly the same with or without a provider configured. That said, completely understand wanting to keep the project focused on its core scope. For anyone who does need cloud share capability, the fork at kaltura/openscreen-kaltura has this functionality and will be maintained. Thanks again for building OpenScreen — it's a great project! 🙏 |
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
3 participants
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.
Summary
This PR adds a cloud integration layer to OpenScreen, with Kaltura as the first provider. Users can browse, download, edit, and re-upload recordings to their Kaltura account — all without leaving the app.
The architecture is designed to be provider-agnostic: the UI presents a generic "Cloud" menu, and Kaltura is simply the first item. Adding YouTube, Facebook, Vimeo, or any other provider later requires only a new service module and a menu item — no structural changes.
What users get
Architecture highlights
electron/kaltura/contains all Kaltura-specific logic (API service + IPC handlers), completely isolated from the rest of the appKalturaLoginFormcomponent extracted and reused by both the settings dialog and the browse windowcategoryEntryservice withdisableentitlementprivilege, chunked uploads for large files, proper KS lifecycle managementkaltura-client)File overview
electron/kaltura/*,electron/main.ts,electron/preload.ts,electron/windows.tssrc/components/video-editor/Kaltura*.tsxLaunchWindow.tsx,VideoEditor.tsx,ExportDialog.tsx,SettingsPanel.tsxsrc/i18n/locales/*/kaltura.json,settings.json,launch.jsonKALTURA.mdFuture enhancements (zero structural changes needed)
electron/youtube/module + menu itemScreenshots
The integration uses the existing dark theme and design system (shadcn/ui). The cloud menu blends into the toolbar as a subtle Cloud icon — it doesn't dominate the UI or push any specific provider.
Happy to address any feedback. This was built to be contributor-friendly —
KALTURA.mddocuments the full architecture for anyone who wants to add the next cloud provider.Summary by CodeRabbit
New Features
Localization
Documentation
Chore