feat: remote media mode + k8s chart hardening (deploy branch promotion) - #762
Conversation
MediaStorageService delegates to a backend selected by OMNI_MEDIA_MODE. Adds media-backends/: local (unchanged) + s3 (Bun.S3Client, no npm dep). Remote mode records the stable S3 key. G1 of omni-remote-media-mode.
Add a bundled MinIO StatefulSet (default-on for k8s dev, off in base/prod) and wire omni-api's OMNI_MEDIA_* env for the remote media backend from G1. - templates/minio.yaml: MinIO Secret + Service (api 9000/console 9001) + StatefulSet with volumeClaimTemplates, mirroring nats.yaml. Gated on minio.enabled. Idempotent bucket bootstrap via a post-install/post-upgrade Helm hook Job (minio/mc, mc mb --ignore-existing). - deployment.yaml: initContainer wait-for-media-bucket as a hard per-pod gate so omni-api cannot serve before the bucket exists; S3 creds injected by secretKeyRef only (never plaintext). - configmap.yaml: non-secret OMNI_MEDIA_* env, endpoint derived from the MinIO Service when bundled or an explicit external endpoint otherwise. - _helpers.tpl: omni.minio.fullname, omni.media.remote, omni.media.s3Endpoint, omni.media.secretName helpers. - values.yaml: media.* + minio.* blocks; values-dev.yaml turns on remote+MinIO with path-style and dev creds; values-prod.yaml uses external S3 via media.s3.existingSecret (no plaintext creds) with MinIO off.
…upload Relocate the pluggable media backend module from @omni/api to @omni/channel-sdk so channel plugins can reach it without importing the whole @omni/api graph (db, server) or creating an import cycle. Only the open primitive was shared before; the backend now lives beside the download-guard that both channels and api already depend on. Add storeStream to the MediaStorageBackend contract for size-guarded streaming ingest: - local: pipes through a size-guarded createWriteStream (the exact WhatsApp writeMediaStreamToFile behavior). - remote: streaming/multipart Bun.S3Client upload, aborting past maxSizeBytes. MediaStorageService now imports the backend from @omni/channel-sdk; Group-1 backend tests move with the source (the MediaStorageService round-trip test stays in api and imports the class from channel-sdk).
In remote mode the primary channels now persist ingested media via the
Group-1 backend instead of writing straight to local disk, so media
actually lands in S3 and the message row records the S3 key.
- Telegram already buffers the file, so it routes through the backend's
buffer store(); local mode writes the same {basePath}/{key} path.
- WhatsApp streams Baileys media through the new storeStream() so large
video is never buffered in the heap; the size-guard is preserved. Local
mode keeps the exact createWriteStream path unchanged.
Each channel lazily constructs one backend from OMNI_MEDIA_MODE and the
download functions accept an optional backend for injection in tests.
Wire a MediaStorageService into the shared Services object and branch the two dispatch seams on storage mode: - extractMediaFiles: remote presigns the stored S3 key into ProviderFile.url (no path); local keeps resolving mediaUrl to a local path. Audio is excluded remotely so it stays URL-less on the transcription path. - resolveDispatchMediaPath: processed-media text carries the presigned URL in remote mode and the absolute local path in local mode. Presigns are minted at dispatch time with the config TTL and never stored on the row. ProviderFile.url is already forwarded by nats-genie-provider, so no contract change was needed. Tests: remote/local extractMediaFiles + in-text URL/path and a TTL-expiry round-trip run against a real MinIO container (skip-with-reason if Docker is absent); nats-genie-provider carries files[].url in remote mode.
In remote mode messages.mediaLocalPath holds an S3 key, so the media processor can no longer join(basePath, key) and read local disk to feed mediaService.process(). Add MediaStorageBackend.read(key) (local disk read / S3 GET) plus a MediaStorageService passthrough, and branch the processor on storage mode: remote fetches the object into an os.tmpdir temp file removed in a finally (even on error); local keeps the exact on-disk read path. Fixes fully-broken remote-mode audio (transcription) and lost omni-side image/video/doc vision text. Adds a MinIO round-trip test plus a local-mode disk-read assertion; updates the two channel ingest mock backends for read().
Add docs/_internal/remote-media-mode.md covering the OMNI_MEDIA_MODE flag, the OMNI_MEDIA_S3_* config, the k8s default (remote + bundled MinIO), how to point at external managed S3 via media.s3.existingSecret + minio.enabled=false, and the presigned-URL-at-dispatch / TTL and audio-is-transcribed behavior. Add all eight OMNI_MEDIA_* vars to .env.example with local-mode defaults (mode=local; S3 vars commented since local disk needs none).
…rFile.url Against a real minio/minio container: store media through MediaStorageService in remote mode, drive extractMediaFiles to presign the S3 key into a ProviderFile.url, then fetch() that URL and assert the GET returns HTTP 200 with the exact stored bytes. Skips with a clear reason when Docker is absent.
Four remote-media integration tests each started their own minio/minio container in beforeAll. Under root `bun test` (one process) the concurrent containers starved Docker and blew the 30s readiness deadline, failing the pre-push gate. Add a shared harness that starts ONE container per process (cached promise, stopped once on exit) and give each suite a unique bucket. Also route every harness/test HTTP call through a fetch reference captured at module load: bun runs test files concurrently and tts.test.ts swaps the process-global globalThis.fetch, which would otherwise poison these round trips now that they run fast enough to overlap that window.
PR #761 review findings: - GET /media route reads via the backend (S3 in remote mode) — the disk-only readMedia 404'd every remote-stored mediaUrl (Codex P2, validated HIGH). - LocalMediaBackend rejects keys escaping the storage root (defense-in-depth for the exported channel-sdk contract; Gemini finding, downgraded to MEDIUM). - Temp-file cleanup if writeFile fails mid-materialize (Gemini, LOW).
…tion Verified in the wild on a node restart (2026-07-03): kubelet fsGroup re-perms pgdata on every mount (postgres refuses setgid/g+w) and an unclean shutdown leaves a stale postmaster.pid that fatals under PID reuse — a fix-pgdata initContainer repairs both each pod start. Also: autopg's postmaster hardcodes the default superuser password for its admin pool, so the Job's rotation crash-looped recreated pods; the Job now pins the default (self-heals already-rotated clusters).
feat: remote media mode — S3/MinIO presigned URLs
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pluggable remote media storage mode (S3/MinIO) alongside the existing local disk storage. It abstracts media operations into a MediaStorageBackend interface with LocalMediaBackend and S3MediaBackend implementations, leveraging Bun's native S3 client. In remote mode, media is uploaded to S3 under stable keys, and the agent dispatcher generates short-lived presigned GET URLs at dispatch time. The media processor is updated to materialize S3 objects into temporary files for transcription/vision, ensuring cleanup on completion or failure. Ingest paths for Telegram and WhatsApp channels are also updated to stream media directly to the active backend. The review feedback highlights a type mismatch in the /media route handler, suggesting that the shared services.mediaStorage instance be retrieved directly from the container to avoid potential runtime crashes and clean up dead code.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const db = c.get('services'); | ||
| const storage = getMediaStorage(db); |
There was a problem hiding this comment.
The route handler currently retrieves the services container (named db here) and passes it to getMediaStorage(db). However, getMediaStorage expects a raw Database instance, not the Services container, which causes a type mismatch (suppressed by // @ts-expect-error at the top of the file) and would lead to runtime crashes if any database-touching methods were called on that duplicate MediaStorageService instance.
Since MediaStorageService is now registered as a first-class service on the Services container (services.mediaStorage), you should retrieve the shared, correctly-instantiated instance directly from the container. This also makes the getMediaStorage helper and the global mediaStorage variable at the top of the file dead code, which can be removed.
| const db = c.get('services'); | |
| const storage = getMediaStorage(db); | |
| const services = c.get('services'); | |
| const storage = services.mediaStorage; |
![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: fd4a5f7af2
ℹ️ 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".
| }); | ||
|
|
||
| return { localPath: relativePath }; | ||
| return { localPath: stored.reference }; |
There was a problem hiding this comment.
When OMNI_MEDIA_MODE=remote, stored.reference is an S3 key and no file exists under MEDIA_STORAGE_PATH. The existing POST /api/v2/messages/media/download flow still checks existsSync(join(mediaStorage.getBasePath(), mediaLocalPath)) and, on a miss, calls storeFromUrl(message.mediaUrl); for Telegram messages that mediaUrl is just the Telegram file_id, not a URL, so already-ingested remote Telegram attachments fall through and error instead of returning the stored media. Please make that route check/read through the media backend before treating the cache as missing.
Useful? React with 👍 / 👎.
CI (Blacksmith 4vcpu) failed all four MinIO suites: readiness exceeded 45s under full-suite load, and the harness cached the rejected promise so the other three suites failed instantly. Now: pre-pull the image, 120s readiness, failed starts retry with a fresh container instead of being cached, and readiness timeouts include container state + logs. beforeAll budgets raised to 180s to sit above the new deadline.
1 participant
Promote feat/deploy-k8s → dev (remote media mode + chart hardening)
Everything merged into
feat/deploy-k8safter #759's snapshot landed in dev (16 commits):Remote media mode (#761, wish
omni-remote-media-mode)OMNI_MEDIA_MODE=remote: WhatsApp/Telegram media stored in S3/MinIO (Bun-nativeS3Client, zero new deps); agents receive presigned URLs via the existingProviderFile.url, minted at dispatch; audio stays URL-less and is transcribed from S3. Local mode byte-for-byte unchanged (default).@omni/channel-sdk(no api→channel import cycle); streaming upload preserves the WhatsApp size-guard.GET /api/v2/mediaserves through the backend (S3 in remote mode) — PR-review fix for deadmediaUrls.LocalMediaBackend; MinIO bundled in the chart (dev) / external S3 viaexistingSecret(prod).Chart restart-resilience (verified in the wild, node restart 2026-07-03)
fix-pgdatainitContainer: repairs kubelet fsGroup re-perms + stalepostmaster.pidon every pod start.existingSecret; helm-dep.tgzartifacts gitignored.Merge with "Create a merge commit" (squash skips the version.yml release trigger).