Unify and harden media Save As pipeline - #82
Conversation
![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: 594e88342b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mimeType: MIME_BY_FORMAT[format], | ||
| suggestedName: `venice-speech.${format}`, |
There was a problem hiding this comment.
Confirmed defect — static proof: if a user generates speech and then changes the Format selector before clicking Download, audioUrl still references the prior blob but this uses the newly selected format as its MIME. saveMediaAs consequently relabels the bytes, and the main-process signature check rejects the otherwise valid audio; retain the format/MIME associated with the completed generation or prefer the fetched blob's non-empty MIME, with a regression test covering a post-generation format change.
AGENTS.md reference: AGENTS.md:L277-L280
Useful? React with 👍 / 👎.
| const response = await fetch(input.source); | ||
| if (response.ok === false) return { status: "failed", error: `Media source returned ${response.status}.` }; |
There was a problem hiding this comment.
Confirmed defect — static proof: in web mode, video retrieval can store a provider download_url directly as resultUrl (src/stores/background-task-store.ts:329-330), but the unified Save As path now fetches every source before creating the download anchor. A cross-origin signed or legacy media URL without an Access-Control-Allow-Origin header therefore fails at fetch, whereas the previous Video and Music download controls navigated directly to the URL; retain a direct-anchor web fallback or retrieve these bytes through a contract-compatible broker, and cover a rejected cross-origin fetch in the web transport tests.
AGENTS.md reference: AGENTS.md:L233-L237
Useful? React with 👍 / 👎.
| case "video/mp4": | ||
| case "audio/mp4": | ||
| return hasIsoBaseMediaSignature(bytes); |
There was a problem hiding this comment.
Confirmed defect — static proof: video/mp4 and audio/mp4 both accept the same generic ftyp buffer, while persistGeneratedMedia deduplicates solely by SHA-256 and returns the newly requested MIME when that hash already exists without updating the stored metadata. If identical ISO-BMFF bytes arrive under the two labels, the second caller can receive audio/M4A metadata while the protocol resolver and later Save As still use the original video/MP4 metadata, or vice versa; distinguish the track type or reject/reconcile MIME conflicts, with tests covering both insertion orders.
AGENTS.md reference: AGENTS.md:L290-L292
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR consolidates all single-item media “Save As” flows (image/audio/video across multiple UI surfaces) behind a single renderer entry point (desktopMedia.saveMediaAs) and a single Electron main-process export owner (electron/services/generatedMediaExport.ts), strengthening validation and ensuring atomic, byte-preserving writes through the preload/IPC boundary.
Changes:
- Unified UI/media surfaces to call
desktopMedia.saveMediaAs, removing legacy fixed-folder writers and renderer-driven Electron downloads. - Centralized media MIME/extension/signature validation into a shared main-process policy (
electron/services/mediaFormat.ts) used by both persistence and export. - Tightened CI/release dependency audit gating and recorded architecture/audit evidence in docs.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/desktop.ts | Removes obsolete fixed-folder export APIs from the desktop bridge type surface. |
| src/services/desktopBridge.ts | Introduces canonical desktopMedia.saveMediaAs with Electron IPC + web fallback behavior. |
| src/services/desktopBridge.media-save.test.ts | Adds focused tests covering save routing, normalization, and cancellation/error handling. |
| src/components/video/video-view.tsx | Routes video Save As through desktopMedia.saveMediaAs. |
| src/components/music/music-view.tsx | Replaces anchor downloads with canonical Save As flow for music output. |
| src/components/image/image-view.tsx | Routes Image Studio downloads through the unified Save As pipeline. |
| src/components/image/image-view.test.tsx | Updates mocks to reflect the new Save As API. |
| src/components/image/image-tools.tsx | Routes Image Tools results through canonical Save As handling. |
| src/components/gallery/media-inspector.tsx | Routes inspector Save As through the canonical Save As path using mediaItemSource. |
| src/components/gallery/gallery-view.tsx | Routes Media Studio Save As to the unified service and standardizes outcomes. |
| src/components/gallery/gallery-view.test.tsx | Adds regression coverage asserting generated-media Save As path usage. |
| src/components/audio/audio-view.tsx | Routes TTS audio downloads through canonical Save As with explicit MIME mapping. |
| src/components/audio/audio-view.test.tsx | Adds a regression test ensuring AudioView uses saveMediaAs. |
| scripts/verify-image-policy.cjs | Updates policy verifier to target the new canonical Save As boundary and removed IPCs. |
| package.json | Adjusts ci audit gating to separate prod moderate+ from full-graph critical advisories. |
| electron/services/mediaService.ts | Removes obsolete fixed-folder export writer logic and related helpers. |
| electron/services/mediaService.test.ts | Removes tests that exclusively covered the deleted fixed-folder writer helpers/export. |
| electron/services/mediaFormat.ts | Adds shared MIME/extension + magic-byte validation policy for supported media formats. |
| electron/services/generatedMediaStore.ts | Reuses shared media-format policy and fixes Windows fsync handle mode. |
| electron/services/generatedMediaStore.test.ts | Expands persistence coverage for additional supported MIME/container types. |
| electron/services/generatedMediaExport.ts | Centralizes Save As validation/filename normalization + atomic overwrite semantics. |
| electron/services/generatedMediaExport.test.ts | Expands coverage for formats, signature checks, filename sanitization, and atomic overwrite. |
| electron/preload.ts | Removes obsolete fixed-folder writer IPC methods from the preload bridge. |
| electron/ipc/handlers/fileHandlers.ts | Removes obsolete IPC channels and associated validation logic. |
| electron/ipc/handlers.test.ts | Updates handler registration tests to assert removed IPC channels are not present. |
| electron/agent/approvals/approval-coordinator.ts | Fixes Windows fsync compatibility by using a writable handle mode for sync. |
| docs/summary_of_work.md | Records VF-VERIFY-005 Save As consolidation evidence and validation matrix updates. |
| docs/ROADMAP.md | Updates VF-VERIFY-005 status and clarifies remaining externally blocked release evidence. |
| docs/reports/MEDIA_SAVE_PIPELINE_AUDIT_2026-07-28.md | Adds an audit/report documenting the write-path inventory and verification evidence. |
| docs/DOCS_INDEX.md | Registers the new audit/report and removes non-portable index entries. |
| .github/workflows/release.yml | Aligns release workflow audit gating with the new production-vs-full-graph policy. |
| .github/workflows/ci.yml | Aligns CI workflow audit gating with the new production-vs-full-graph policy. |
| const url = URL.createObjectURL(blob); | ||
| const anchor = document.createElement("a"); | ||
| anchor.href = url; | ||
| anchor.download = input.suggestedName || "venice-forge-media"; | ||
| anchor.click(); | ||
| setTimeout(() => URL.revokeObjectURL(url), 0); | ||
| return { status: "saved", filename: anchor.download, bytes: blob.size }; |
| mustNotContain("electron/ipc/handlers/fileHandlers.ts", [ | ||
| "\".gif\", \".mp4\", \".webm\"", | ||
| "Buffer.from(rawData, \"base64\")", | ||
| ], "unsafe routed image policy"); | ||
| "app:saveRoutedImage", | ||
| "app:media:export\"", | ||
| ], "obsolete fixed-folder media writers"); |
| case "audio/ogg": | ||
| case "audio/opus": | ||
| return bytes.length >= 4 && bytes.subarray(0, 4).toString("ascii") === "OggS"; |
Summary
desktopMedia.saveMediaAsgeneratedMediaExport.tsArchitectural and behavioral decisions
Generated-media IDs use the opaque-ID save route. Legacy data, blob, HTTP, and allowlisted custom-protocol sources are normalized to bytes before crossing the typed preload bridge. The main process validates signatures, opens the native dialog, and atomically writes the original bytes. Bulk export remains a distinct multi-file operation in the same export service. Web mode uses the same renderer entry point with its browser-download fallback.
Supported output policy covers PNG, JPEG, WebP, GIF, AVIF, MP4 video, MP3, WAV, FLAC, Ogg, Opus, AAC, and audio MP4/M4A. Filename normalization preserves Unicode while handling platform-invalid, reserved, and oversized names.
Affected subsystems
docs/summary_of_work.md,docs/ROADMAP.md,docs/DOCS_INDEX.md, and the media pipeline auditValidation
Executed under Node 22.13.1/npm 10.9.2:
npm run lint:eslint— PASS, zero warningsnpm run typecheck— PASS, renderer and Electron projectsnpm test— PASS, 4,834 passed / 1 skipped across 444 files in 362.21snpm run verify:contracts— PASS, including 103 release-packaging checksnpm run verify:safety-guard— PASSnpm run verify:markdown-links— PASS, 246 filesnpm run verify:i18n— PASS with 297 known non-English missing-marker warningsnpm run verify:i18n-hardcoded-regressions— PASS, zero regressionsnpm run verify:storage-privacy— PASSnpm run dist:mac:arm64— PASS; app, DMG, ZIP, blockmaps, and checksums producedRUN_ELECTRON_SMOKE=true npm run smoke:electron— PASS, 1/1Manual and packaged verification
Prior headed packaged verification on this branch lineage confirmed native Save As for an existing Media Studio video, valid MP4 bytes, cancellation without false success, detail-dialog retention, and sidebar width persistence/collapse/reset. The remediated arm64 package passed launch/shutdown smoke.
Known limitations
The package is ad-hoc/unsigned because this Mac has no Developer ID identity or notarization Keychain profile. Signed/notarized macOS, hardened-runtime/App Sandbox behavior, Intel macOS, Windows signing, paid-provider generation, a second device, accessibility, and real private backup/migration fixtures remain externally blocked under VF-VERIFY-005.
spctlrejection is recorded as blocker evidence, not a passed distribution check.The 297 localization warnings are existing
__MISSING__markers in non-English catalogs; the localization gate and hardcoded-string regression gate pass.Full evidence:
docs/reports/MEDIA_SAVE_PIPELINE_AUDIT_2026-07-28.md.