feat: embed system integration for community encoders#20
Open
feat: embed system integration for community encoders#20
Conversation
Adds a new, clean code path for the 3Speak embed system alongside the legacy gateway. Community encoders can now poll the embed gateway for jobs via JWS-authenticated API, feeding into the existing Direct API pipeline. Configurable via EMBED_SYSTEM_ENABLED, EMBED_SYSTEM_MODE, and EMBED_GATEWAY_URL env vars. Also fixes WebhookService progress URL to match the encoder contract and a Gateway Aid logging bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an Embed System integration: new env vars and config schema for Changes
Sequence DiagramsequenceDiagram
participant Encoder as ThreeSpeakEncoder
participant Poller as EmbedPollerService
participant Identity as IdentityService
participant Gateway as Embed Gateway
participant Queue as JobQueue
Encoder->>Poller: start()
Poller->>Identity: create JWS (node registration)
Identity-->>Poller: JWS
Poller->>Gateway: POST /api/v0/gateway/updateNode (JWS)
Gateway-->>Poller: registration OK
loop Every 60s (with jitter)
Poller->>Gateway: POST /api/v0/gateway/heartbeat (JWS)
Gateway-->>Poller: heartbeat OK
Poller->>Gateway: POST /api/v0/gateway/myJob (JWS)
alt Job Available
Gateway-->>Poller: job data
Poller->>Queue: addDirectJob(jobPayload)
Queue-->>Poller: enqueued
else No Job
Gateway-->>Poller: no job
end
end
Encoder->>Poller: stop()
Poller-->>Encoder: stopped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/config/ConfigLoader.ts`:
- Around line 143-147: The embed_system.mode value in ConfigLoader.ts is
currently being cast with "as 'managed' | 'community'" which is redundant
because ConfigSchema.parse() enforces the enum; remove the type assertion from
the embed_system object (leave mode as (process.env.EMBED_SYSTEM_MODE ||
'managed')) and rely on ConfigSchema.parse() to validate/assign the correct
union type, ensuring any TypeScript typings use the parsed schema (e.g.,
z.infer<typeof ConfigSchema>) rather than this inline cast.
In `@src/services/EmbedPollerService.ts`:
- Around line 65-67: The initial calls to sendHeartbeat() and pollForJob()
swallow errors with .catch(() => {}); replace those empty catches with proper
logging so startup failures are visible: change this.sendHeartbeat().catch(() =>
{}); and this.pollForJob().catch(() => {}); to .catch(err =>
this.logger?.error('sendHeartbeat initial run failed', err) ||
console.error('sendHeartbeat initial run failed', err)) and .catch(err =>
this.logger?.error('pollForJob initial run failed', err) ||
console.error('pollForJob initial run failed', err)); include error.message or
err.stack in the log to provide actionable details and keep the same
non-throwing behavior so the service can continue starting.
- Around line 159-167: Replace the unsafe cast of data.job to DirectJobRequest
in EmbedPollerService: validate that data.job is an object and contains required
fields (owner, permlink, input_cid, webhook_url, api_key) before assigning to
jobPayload and enqueueing; if validation fails, log an error and skip. To
prevent duplicate enqueues, compute a stable fingerprint (e.g.,
owner+permlink+input_cid) for the incoming jobPayload and check the jobQueue for
an existing pending/queued job with that fingerprint (or use a
jobQueue.hasPending/findByFingerprint helper); only call
this.jobQueue.addDirectJob(jobPayload) when validation passes and no duplicate
exists. Ensure addDirectJob continues to generate its unique id and leave that
behavior unchanged.
- Around line 21-24: embedPollerService currently types pollCronJob as any;
update project types to use `@types/node-cron` v11 (to match node-cron v4.x) and
change the pollCronJob declaration in EmbedPollerService.ts to the exact return
type of cron.schedule() (i.e., the ScheduledTask-like return) instead of any;
locate the pollCronJob field in the EmbedPollerService class and replace the any
type with the cron.schedule() return type, and update package.json
devDependencies to `@types/node-cron` v11 so imports/types align.
In `@src/services/ThreeSpeakEncoder.ts`:
- Around line 174-194: Replace the string concatenation in the managed-mode log
with a template literal for consistency: in the block that checks embedMode ===
'managed' (references: embedMode, this.config.direct_api?.port, logger.info,
this.directApi), change the logger.info call that currently builds the message
with '+' to use a template literal so it matches the other log lines' style.
In `@src/services/WebhookService.ts`:
- Around line 71-72: The construction of progressUrl using
`${webhookUrl}/progress` can produce a double-slash when webhookUrl ends with a
slash; update the logic in WebhookService (where progressUrl is assigned) to
normalize webhookUrl first (e.g., strip any trailing slashes) or join paths
safely so that progressUrl always becomes a single-slash concatenation (for
example, remove trailing '/' from webhookUrl before appending '/progress' or use
a URL-join helper), ensuring progressUrl is well-formed regardless of the
incoming webhookUrl value.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b188f6c0-48a5-4443-9509-6d17718198d3
📒 Files selected for processing (5)
.env.examplesrc/config/ConfigLoader.tssrc/services/EmbedPollerService.tssrc/services/ThreeSpeakEncoder.tssrc/services/WebhookService.ts
- Remove redundant type assertion on embed_system.mode (Zod validates) - Type pollCronJob properly instead of any - Log errors on initial heartbeat/poll instead of swallowing silently - Validate required fields on job payload before casting - Use template literal for managed mode log consistency - Strip trailing slash from webhook URL before appending /progress Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/services/EmbedPollerService.ts`:
- Around line 164-178: The requiredFields validation in EmbedPollerService
misses the non-optional boolean "short", so update the requiredFields array to
include 'short' and after casting to DirectJobRequest (jobPayload) explicitly
validate that typeof jobPayload.short === 'boolean' (mirror DirectApiService's
typeof check) before calling this.jobQueue.addDirectJob; log and return on
missing/invalid 'short' to prevent downstream code (VideoProcessor,
ThreeSpeakEncoder) from receiving undefined.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 5ce64b8d-db9f-4cdc-9326-c25eea4aebfe
📒 Files selected for processing (4)
src/config/ConfigLoader.tssrc/services/EmbedPollerService.tssrc/services/ThreeSpeakEncoder.tssrc/services/WebhookService.ts
…elds The !value check would incorrectly flag short:false as missing. Use == null check instead, and add explicit boolean type validation for the short field to mirror DirectApiService behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add embed system node types, configuration options, and example configs for community mode, managed mode, and transition mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All three install scripts (bash, PowerShell, batch) now offer option 4 for the new embed system community mode. Default for invalid choices changed from legacy gateway to embed community mode, preparing for the eventual deprecation of the legacy system. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 301-331: Add blank lines before and after each heading and fenced
code block in the README sections for "Embed System — Community Mode (New)",
"Embed System — Managed Mode (New)", and "Transition Mode — Legacy + Embed
Community" so that each heading is separated from adjacent text and each ```bash
code block is preceded and followed by a blank line; update the three instances
where headings immediately follow or precede code fences to ensure there is an
empty line between the heading and the opening/closing ``` so the markdown
conforms to markdownlint rules.
- Around line 53-67: The heading "### Embed System Encoders (New)" is not
surrounded by blank lines; update the README by inserting a blank line
immediately before and after that heading (and ensure there is a blank line
between the heading and the following paragraph/list items) so the section
follows markdownlint's rule for blank lines around headings and improves
readability.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 514eae0f-edcb-4183-9bca-b4197c4fbb02
📒 Files selected for processing (2)
README.mdsrc/services/EmbedPollerService.ts
Comment on lines
+53
to
+67
| ### Embed System Encoders (New) | ||
| The new 3Speak embed system supports two modes: | ||
|
|
||
| **Community Mode** — Encoder polls the embed system for jobs: | ||
| - JWS-authenticated polling every 60 seconds | ||
| - Automatic registration and heartbeat | ||
| - Same encoding pipeline and webhook reporting | ||
|
|
||
| **Managed Mode** — Embed system pushes jobs to the encoder: | ||
| - Uses the Direct API endpoint (`POST /encode`) | ||
| - Embed system dispatches jobs directly to your encoder | ||
| - Requires the encoder to be reachable from the internet | ||
|
|
||
| Both modes can run alongside the legacy gateway during the transition period. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fix markdown formatting — add blank lines around headings.
Per markdownlint, headings should be surrounded by blank lines for better readability and consistent formatting.
📝 Proposed fix
## 🎖️ Node Types
+
### Embed System Encoders (New)
+
The new 3Speak embed system supports two modes:And at line 68:
Both modes can run alongside the legacy gateway during the transition period.
+
### Legacy Community Nodes (Default)
+
**Standard operation** for everyone:🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 53-53: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 53 - 67, The heading "### Embed System Encoders
(New)" is not surrounded by blank lines; update the README by inserting a blank
line immediately before and after that heading (and ensure there is a blank line
between the heading and the following paragraph/list items) so the section
follows markdownlint's rule for blank lines around headings and improves
readability.
Collaborator
Author
There was a problem hiding this comment.
Fixed in ad97c83 — added blank lines around headings.
There was a problem hiding this comment.
@menobass, thanks for the confirmation! The blank-line fix around headings in README.md is noted. ✅
🧠 Learnings used
Learnt from: menobass
Repo: Mantequilla-Soft/3speakencoder PR: 20
File: src/services/EmbedPollerService.ts:163-178
Timestamp: 2026-03-26T19:32:21.529Z
Learning: In the 3speakencoder embed system, the backend gateway uses an atomic `findOneAndUpdate` operation to claim jobs (setting their status to `encoding`) before returning them to a polling encoder. This guarantees that the same job cannot be returned to multiple encoders or on consecutive polls, so no client-side duplicate guard is needed in `EmbedPollerService`.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
install.bat (1)
125-130:⚠️ Potential issue | 🟡 MinorUpdate the validation error text to match non-direct modes.
Line 128 still says “required for gateway mode,” but this branch now applies to gateway, dual, and embed modes.
💡 Proposed wording fix
- echo ❌ Hive username is required for gateway mode! + echo ❌ Hive username is required for this mode!🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.bat` around lines 125 - 130, The validation message for empty HIVE_USERNAME currently says "required for gateway mode" but the same branch applies to gateway, dual, and embed modes; update the echo text in the HIVE_USERNAME check (the block that reads set /p HIVE_USERNAME and tests "!HIVE_USERNAME!"=="" ) to use a neutral message such as "Hive username is required for gateway, dual, or embed mode!" (or "required for non-direct modes") so it accurately reflects all supported modes.install.ps1 (1)
125-131:⚠️ Potential issue | 🟡 MinorUpdate the blank-username error for non-gateway modes.
Line 129 still says
gateway mode, but this branch now also handles embed and dual. An embed user who presses Enter gets a misleading error.✏️ Suggested copy fix
- Write-Host "❌ Hive username is required for gateway mode!" -ForegroundColor Red + Write-Host "❌ Hive username is required for the selected encoding mode!" -ForegroundColor Red🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.ps1` around lines 125 - 131, Update the error message shown when $hiveUsername is blank: locate the Read-Host loop that sets $hiveUsername and the Write-Host call that prints "❌ Hive username is required for gateway mode!" and change the text to a neutral message (e.g., "❌ Hive username is required!" or "❌ Hive username is required for gateway/embed/dual modes!") so it no longer incorrectly references only gateway mode; keep the same Write-Host call and variable ($hiveUsername) but replace the hard-coded "gateway mode" copy with the new, accurate wording.install.sh (1)
249-253:⚠️ Potential issue | 🟡 MinorUpdate the blank-username error for embed and dual modes.
The prompt now covers every non-
directmode, but Line 252 still says “gateway mode.” Embed users who leave it blank will get the wrong guidance.✏️ Suggested fix
- echo "❌ Hive username is required for gateway mode" + echo "❌ Hive username is required for this mode"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.sh` around lines 249 - 253, The error message when HIVE_USERNAME is empty incorrectly references "gateway mode"; update the echo that prints "❌ Hive username is required for gateway mode" to reference the correct modes covered by the prompt (e.g., "gateway/embed/dual modes" or "non-direct modes") so users in embed or dual modes get accurate guidance; locate the read prompt and the HIVE_USERNAME check handling (the read -p "Hive username: " HIVE_USERNAME block and the subsequent if [[ -z "$HIVE_USERNAME" ]]; then echo ... exit 1) and replace the message text accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.ps1`:
- Around line 245-269: The installer currently always regenerates
$encoderPrivateKey before writing .env which breaks persistent encoder identity;
before generating a new key, check for an existing .env (use the same installDir
/.env path logic), extract ENCODER_PRIVATE_KEY (e.g. via Select-String) and use
that value if present and non-empty, otherwise generate and assign
$encoderPrivateKey; then write the .env block unchanged so ENCODER_PRIVATE_KEY
remains stable across reinstalls.
In `@install.sh`:
- Around line 416-420: install.sh currently overwrites ENCODER_PRIVATE_KEY on
every run; modify the script so it only generates a new encoder key when none
exists and otherwise preserves the existing persistent identity. Specifically,
add logic around ENCODER_PRIVATE_KEY (and any key generation function/step) to
first check for an existing stored key (e.g., environment variable, file like
.encoder_key or state file used by the installer) and export/use that value if
present, and only call the key-generation routine to create and persist a new
ENCODER_PRIVATE_KEY when no existing key is found; ensure the chosen storage
location is readable/writable by the installer and that subsequent runs source
that stored key instead of regenerating it.
---
Outside diff comments:
In `@install.bat`:
- Around line 125-130: The validation message for empty HIVE_USERNAME currently
says "required for gateway mode" but the same branch applies to gateway, dual,
and embed modes; update the echo text in the HIVE_USERNAME check (the block that
reads set /p HIVE_USERNAME and tests "!HIVE_USERNAME!"=="" ) to use a neutral
message such as "Hive username is required for gateway, dual, or embed mode!"
(or "required for non-direct modes") so it accurately reflects all supported
modes.
In `@install.ps1`:
- Around line 125-131: Update the error message shown when $hiveUsername is
blank: locate the Read-Host loop that sets $hiveUsername and the Write-Host call
that prints "❌ Hive username is required for gateway mode!" and change the text
to a neutral message (e.g., "❌ Hive username is required!" or "❌ Hive username
is required for gateway/embed/dual modes!") so it no longer incorrectly
references only gateway mode; keep the same Write-Host call and variable
($hiveUsername) but replace the hard-coded "gateway mode" copy with the new,
accurate wording.
In `@install.sh`:
- Around line 249-253: The error message when HIVE_USERNAME is empty incorrectly
references "gateway mode"; update the echo that prints "❌ Hive username is
required for gateway mode" to reference the correct modes covered by the prompt
(e.g., "gateway/embed/dual modes" or "non-direct modes") so users in embed or
dual modes get accurate guidance; locate the read prompt and the HIVE_USERNAME
check handling (the read -p "Hive username: " HIVE_USERNAME block and the
subsequent if [[ -z "$HIVE_USERNAME" ]]; then echo ... exit 1) and replace the
message text accordingly.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 100819fb-cf3d-4284-a204-d9ff69e8fd54
📒 Files selected for processing (3)
install.batinstall.ps1install.sh
Install scripts now check for an existing ENCODER_PRIVATE_KEY in .env before generating a new one. Prevents the encoder from registering as a new node every time the installer is rerun. Applied to all three scripts (bash, PowerShell, batch). Also fixes markdown heading spacing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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
EmbedPollerServicehandles registration, heartbeat, and job polling — feeds jobs into the existing Direct API pipeline (no duplication of encoding/webhook logic)EMBED_SYSTEM_ENABLED,EMBED_SYSTEM_MODE(managed/community), andEMBED_GATEWAY_URLenv varsWebhookServiceprogress URL derivation to match the encoder contract ({webhook_url}/progress)Files changed
src/services/EmbedPollerService.tssrc/config/ConfigLoader.tsembed_systemconfig sectionsrc/services/ThreeSpeakEncoder.tssrc/services/WebhookService.ts.env.exampleTest plan
npm run buildcompiles cleanlyREMOTE_GATEWAY_ENABLED=true, embed disabled) — no behavior changeEMBED_SYSTEM_ENABLED=true,EMBED_SYSTEM_MODE=community) — registers, polls every 60s, logs heartbeatEMBED_SYSTEM_ENABLED=true,EMBED_SYSTEM_MODE=managed) — validates Direct API is running{webhook_url}/progresscorrectly🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration
Bug Fixes
Chores