Skip to content

Capacity diagnostics, clearer errors, 5+ streams verified#3

Merged
jerryagenyi merged 8 commits intomainfrom
dev
Feb 2, 2026
Merged

Capacity diagnostics, clearer errors, 5+ streams verified#3
jerryagenyi merged 8 commits intomainfrom
dev

Conversation

@jerryagenyi
Copy link
Copy Markdown
Owner

@jerryagenyi jerryagenyi commented Feb 2, 2026

Summary

  • Capacity diagnostics: /api/system/config now returns sourceLimit, activeStreams, remaining, configPath. StreamingService checks capacity before start and ensures sourceLimit/remaining never NaN (ensureInitialized + safe coercion).
  • Clearer stream errors: Backend sends shortMessage (diagnosis title) and capacity when relevant; frontend shows troubleshooting link. TROUBLESHOOTING.md updated (exit -5, empty stderr).
  • 5+ streams verified: Root cause of "5th stream fails" was bad device choice (e.g. OBS Virtual Camera not present), not Icecast limit. Confirmed 6/6 live with VB-Cable A/B and mics.
  • Project/CI: AGENTS.md, simplified TODO, CI installs Icecast for tests, removed unused Claude workflows, analysis_output trimmed, updater/hooks/custom-instructions aligned.

All 26 tests pass.


Note

Medium Risk
Medium risk because it changes stream-start failure handling and adds a hard capacity gate based on parsed icecast.xml, which could block starts if parsing/config detection is wrong. CI/workflow and release-updater tweaks may also affect pipeline and packaging behavior.

Overview
Adds Icecast source-capacity awareness end-to-end: IcecastService now parses <limits><sources> and GET /api/system/config exposes sourceLimit, activeStreams, remaining, and configPath.

StreamingService.startStream() now performs a preflight capacity check (with NaN-safe defaults) and attaches error.capacity; stream start/restart endpoints propagate shortMessage + capacity metadata so the UI can display concise failures.

Frontend stream manager improves UX by delaying success toasts until after render, surfacing capacity details in errors, and linking to the troubleshooting guide; docs/CI/tooling are cleaned up (updated troubleshooting notes, CI installs Icecast, removed Claude workflows, updater excludes/cleans dev artifacts, and AI agent instructions consolidated).

Written by Cursor Bugbot for commit 37c624f. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • New Features

    • Stream capacity checking prevents starting new streams when Icecast source limit is reached.
    • Icecast source limit tracking and active stream reporting exposed via API.
  • Bug Fixes

    • Enhanced error messages now include troubleshooting links and capacity details.
    • Improved non-error notification auto-dismiss timing (4 seconds instead of 8).
  • Documentation

    • Expanded troubleshooting guide with Windows-specific diagnostics and port conflict resolution.
    • Added comprehensive UI/UX recommendations for dashboard modernization.

jerryagenyi and others added 8 commits February 1, 2026 00:42
- Add AGENTS.md with expert agent guidance for FFmpeg, Icecast, Node.js
- Add Tests row to agent selection table
- Document FFmpegService.js as legacy/deprecated
- Add listener playback proxy documentation
- Update CLAUDE.md to reference AGENTS.md and fix troubleshooting link
- Add TODO items #4 (streams/listener IP) and refactor section
- Add 13 priority items to TODO.md for stability and future improvements

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove claude-code-review.yml and claude.yml (unused, ci.yml is sufficient)
- Clean up analysis_output directory (remove old analysis files, add README)
- Update TODO.md and CI workflow

Co-Authored-By: Claude <noreply@anthropic.com>
- Add parseSourceLimit() to IcecastService to read <sources> from icecast.xml
- Add getSourceLimit() getter and integrate into init/config watcher/update flows
- Add capacity check log before each stream start (sourceLimit, activeCount, remaining, configPath)
- Add fail-fast check: throw if remaining <= 0 before starting stream
- Attach capacity info to mount_point errors (FFmpeg "too many sources")
- Enhanced GET /api/system/config with sourceLimit, activeStreams, remaining, configPath
- Add capacity to /start and /restart error responses for UI display
- Add "Debugging 5+ streams" section to TROUBLESHOOTING.md
- Add config-endpoint test for new capacity fields
- Refactor system.js to use top-level streamingService import
- Add error handling refactor to TODO.md

Co-Authored-By: Claude <noreply@anthropic.com>
- Put capacity values in log message (single-line grep friendly)
- Update TROUBLESHOOTING.md with debugging guidance
- Add error handling refactor to TODO.md

Co-Authored-By: Claude <noreply@anthropic.com>
- Add err.shortMessage with diagnosis title (kept long message for logs)
- Frontend: show short message + link to troubleshooting guide
- Enhanced TROUBLESHOOTING.md with exit code -5 (4294967291), empty stderr, 5th stream
- Add TROUBLESHOOTING_GUIDE_URL constant in FFmpegStreamsManager
- Update streams.js to send shortMessage to client if available

Work in progress by Cursor; frontend changes partially complete.

Co-Authored-By: Claude <noreply@anthropic.com>
- Root cause: Device selection issue (OBS Virtual Camera doesn't exist)
- NOT an Icecast source limit problem
- Document all diagnostics, completed work, and next steps
- Reference commands, file modifications, server state
- Ready for continuation after VB-Cable reboot
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 2, 2026

📝 Walkthrough

Walkthrough

This pull request consolidates AI guidance documentation, removes automated code review workflows, enhances Icecast stream capacity tracking with error reporting, and updates CI/CD configuration to install Icecast. Changes span configuration cleanup, documentation centralization, error handling enhancements, and API endpoint expansion to expose capacity metrics.

Changes

Cohort / File(s) Summary
Documentation & Configuration Consolidation
.augment/README.md, .augment/rules/kingmode.md, .claude/custom-instructions.md, .claude/hooks/pre-code-write.md, .claude/hooks/post-code-write.md, .cursorrules, CLAUDE.md, AGENTS.md
Consolidates system role directives and behavioral protocols into centralized CLAUDE.md and AGENTS.md documents; removes per-file role definitions and protocol triggers; updates hook guidance to reference external documentation instead of inline instructions.
Workflow & CI/CD Updates
.github/workflows/claude-code-review.yml, .github/workflows/claude.yml, .github/workflows/ci.yml, .github/README.md, .claude/settings.local.json
Removes Claude Code Review automation workflows; adds Icecast installation step to CI pipeline; documents workflow setup in .github/README.md; expands Bash permission allowlist for testing and diagnostics commands.
Project Cleanup & Metadata
.augment/, .gitignore, LANStreamer.bat, update_exclude.txt, analysis_output/*, TODO.md
Updates gitignore entries, expands cleanup scope in update script (.augment, analysis_output, .nyc_output), removes historical regression analysis files, adds update exclusion manifest, restructures TODO format from operational to task-oriented.
Icecast Stream Capacity Tracking
src/services/IcecastService.js, src/services/StreamingService.js, src/routes/system.js
Adds runtime parsing of source limit from Icecast config; implements capacity validation in stream start logic; blocks stream creation when capacity exhausted; augments error objects with capacity metadata (sourceLimit, activeCount, remaining, configPath).
Error Handling & UI Enhancements
src/routes/streams.js, public/components/FFmpegStreamsManager.js, public/index.html
Extends error responses to include shortMessage and capacity payloads; adds helper methods for error formatting and paint-deferred UI updates; incorporates troubleshooting guide links in error notifications; adjusts notification auto-dismiss timing.
Testing & API Documentation
tests/integration/config-endpoint.test.js, docs/TROUBLESHOOTING.md, docs/UI-UX-RECOMMENDATIONS.md
Adds integration test verifying capacity metrics in /api/system/config; expands troubleshooting guide with Windows-specific exit code mapping and capacity debugging workflows; adds comprehensive UI/UX modernization recommendations with CSS/HTML examples.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #2: Directly removes the Claude GitHub Action workflows (.github/workflows/claude.yml, .github/workflows/claude-code-review.yml) that were previously configured.

Poem

🐰 With capacity now measured and limits well defined,
The streams flow steady, no more are left behind,
Errors speak clearly through the troubleshooting way,
While docs guide the path—hip-hop hooray!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: capacity diagnostics with error improvements and verification of 5+ streams functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Comment thread src/routes/system.js
host,
sourceLimit,
activeStreams,
remaining: sourceLimit - activeStreams,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent remaining calculation allows negative values in API

Low Severity

The /api/system/config endpoint calculates remaining as sourceLimit - activeStreams without clamping to zero, while StreamingService.js uses Math.max(0, sourceLimit - activeCount) in all three places where it computes this value. This inconsistency means the API could return negative remaining values if more streams are running than the current limit allows (e.g., after lowering <sources> in icecast.xml while streams are active).

Additional Locations (1)

Fix in Cursor Fix in Web

const sourceLimit = (typeof rawLimit === 'number' && !Number.isNaN(rawLimit)) ? rawLimit : 2
const activeCount = Object.values(this.activeStreams).filter(s => s.status === 'running').length
const remaining = Math.max(0, sourceLimit - activeCount)
const configPath = IcecastService.paths?.config
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated capacity calculation logic in four places

Medium Severity

Capacity calculation (sourceLimit, activeCount, remaining, configPath) is duplicated in 4 places. StreamingService uses NaN safeguards while system.js doesn't, creating inconsistency. Extract into a single helper method like getCapacityInfo() on StreamingService or IcecastService to centralize this logic.

Fix in Cursor Fix in Web

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 22-27: The CI step that installs the icecast2 package (the
"Install Icecast (for IcecastService / api/system/config)" run block) can hang
on debconf prompts; modify the install command to run non-interactively by
setting DEBIAN_FRONTEND=noninteractive for the apt-get install step (i.e.,
ensure the command that installs icecast2 is prefixed or run with
DEBIAN_FRONTEND=noninteractive so prompts are skipped and defaults applied,
keeping the existing sudo ln -sf /usr/bin/icecast2 /usr/bin/icecast line
unchanged).

In `@AGENTS.md`:
- Around line 9-10: The table separator row in AGENTS.md violates markdownlint
MD060 due to inconsistent spacing; edit the separator line between the header
row ("| Situation | Agent |") and the data rows so the pipes and dashes use the
same column spacing style (e.g., align separators with single spaces around
column content), ensuring the separator row matches the header spacing and
resolves MD060.

In `@public/components/FFmpegStreamsManager.js`:
- Around line 38-52: formatStreamError currently builds the message from
data.error/data.message and capacity.activeStreams/sourceLimit, which misses new
fields; update formatStreamError to prefer data.shortMessage over
data.error/data.message and prefer data.capacity.activeCount over
data.capacity.activeStreams when present, falling back to the existing fields or
defaultMsg; ensure the capacity text uses activeCount/sourceLimit (with '?'
fallbacks) and still appends (config: configPath) when data.capacity.configPath
exists.

In `@src/routes/system.js`:
- Around line 157-170: The response currently always returns
icecastService.paths?.config under configPath in the /api/system/config route,
which leaks the Icecast config file path; change this so configPath is either
removed or only included under a safe condition (e.g., non-production/dev
environment or when a feature flag is enabled). Locate the JSON response
construction in the route handler (the res.json block referencing
icecastService.paths?.config and the symbol configPath) and wrap the inclusion
in an environment/flag check or omit it entirely for production builds so the
path is not exposed publicly.

In `@src/services/IcecastService.js`:
- Around line 863-865: getSourceLimit currently uses `this.sourceLimit || 2`,
which treats 0 as falsy and incorrectly falls back to 2; update the method
(getSourceLimit) to preserve explicit 0 by performing a nullish/finite check
instead (e.g., use the nullish coalescing or Number.isFinite check on
`this.sourceLimit`) so that 0 is returned when explicitly set, otherwise return
2.

In `@src/services/StreamingService.js`:
- Around line 121-141: The capacity race happens because we only count streams
with status 'running' in the activeCount calculation, so concurrent starts slip
past the remaining check; update the start flow to reserve a slot by setting
this.activeStreams[streamId].status = 'starting' (or similar) before any async
work/FFmpeg spawn and change the activeCount computation in StreamingService to
count both 'running' and 'starting' states (e.g., where activeCount is computed
from this.activeStreams). Ensure the slot is cleared on failure (remove entry or
set to 'stopped') and transitioned to 'running' on successful start so the
existing remaining check and thrown capacity Error (err.capacity) reflect
reserved slots.

In `@TODO.md`:
- Line 9: The line containing the checklist item "Verify 5+ streams — Confirmed
6/6 live (VB-Cable A/B, mics). Root cause of “5th fails” was bad device choice,
not Icecast limit)" has an extra closing parenthesis at the end; edit that
checklist entry to remove the stray ")" so the sentence ends cleanly (e.g.,
after "limit").
🧹 Nitpick comments (2)
.claude/settings.local.json (1)

45-45: Note: xargs:* grants broad execution capability.

While consistent with existing broad permissions in this file (e.g., powershell:*, cmd /c:*), be aware that xargs can pipe input to arbitrary commands. This is acceptable for local development settings but worth noting for awareness.

docs/UI-UX-RECOMMENDATIONS.md (1)

143-179: Add a prefers-reduced-motion note for animations.

This rounds out the accessibility guidance for motion-sensitive users.

✏️ Suggested doc tweak
- - Clear focus outlines.
+ - Clear focus outlines.
+ - Respect prefers-reduced-motion for non-essential animations.

Comment thread .github/workflows/ci.yml
Comment on lines +22 to +27
- name: Install Icecast (for IcecastService / api/system/config)
run: |
sudo apt-get update
sudo apt-get install -y icecast2
# App looks for /usr/bin/icecast; Ubuntu package provides icecast2
sudo ln -sf /usr/bin/icecast2 /usr/bin/icecast || true
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for icecast-related content in the repository
rg -i "icecast" --type yaml --type json --type md -B 2 -A 2

Repository: jerryagenyi/LANStreamer

Length of output: 50382


🌐 Web query:

icecast2 debian package debconf prompts interactive install

💡 Result:

The Debian icecast2 package uses debconf and will only ask questions if you opt into its setup dialog.

What prompts exist (and what triggers them)

The package’s debconf templates are: icecast2/icecast-setup (boolean) and, if that is true, then icecast2/hostname, icecast2/sourcepassword, icecast2/relaypassword, icecast2/adminpassword. Defaults are localhost / hackme. [1]
The debconf config script only asks the password/hostname questions when icecast2/icecast-setup is true. [2]
The postinst applies those answers by editing /etc/icecast2/icecast.xml when icecast2/icecast-setup is true. [3]

Force an interactive prompt run (after install)

sudo dpkg-reconfigure icecast2

This re-runs the debconf dialog for the package.

Ensure apt/dpkg actually goes interactive

Run with an interactive frontend + sufficiently high priority, e.g.:

sudo env DEBIAN_FRONTEND=dialog DEBIAN_PRIORITY=high apt install icecast2

(or DEBIAN_FRONTEND=readline if you don’t want the curses UI).

Noninteractive install (preseed to avoid prompts)

You can preseed answers (and mark them “seen”, preventing interactive questions) with debconf-set-selections. [4]

Example:

sudo debconf-set-selections <<'EOF'
icecast2 icecast2/icecast-setup boolean true
icecast2 icecast2/hostname string stream.example.com
icecast2 icecast2/sourcepassword string S3cretSource
icecast2 icecast2/relaypassword string S3cretRelay
icecast2 icecast2/adminpassword string S3cretAdmin
EOF

sudo apt install icecast2

Key detail: if you leave icecast2/icecast-setup as false, the package won’t ask for the passwords/hostname at install time. [1][2]

Sources: Debian icecast2 debconf templates/config/postinst and debconf documentation. [1][2][3][4]


🌐 Web query:

DEBIAN_FRONTEND=noninteractive apt-get best practice

💡 Result:

What DEBIAN_FRONTEND=noninteractive does

  • It tells debconf to use the noninteractive frontend: it will never prompt, and will use default answers; if you need non-default answers you must preseed debconf. [1]

Best practices (automation, CI, Docker builds)

  1. Prefer -y (Assume-Yes) for apt prompts; add DEBIAN_FRONTEND=noninteractive only when debconf prompts are the problem

    • apt-get -y answers “yes” to apt’s confirmation prompts and runs non-interactively (and will still abort on certain dangerous situations). [2]
  2. Scope it to the single command (don’t set it globally)

    • Good:
      apt-get update
      DEBIAN_FRONTEND=noninteractive apt-get install -y <packages>
    • In Dockerfiles, avoid ENV DEBIAN_FRONTEND=noninteractive (it persists into the final runtime environment); prefer an ARG or inline per RUN step. [5]
  3. If packages ask configuration questions, preseed instead of relying on defaults

    • Use debconf-get-selections / debconf-set-selections (from debconf-utils) to capture/apply answers before installing packages. [3]
  4. Decide how to handle config-file prompts (/etc/... conffiles)

    • In fully noninteractive runs, if you want the run to fail when something tries to prompt (e.g., conffile conflict), one approach is to run with stdin closed:
      DEBIAN_FRONTEND=noninteractive apt-get install -y <pkg> < /dev/null
      This should error out if dpkg needs input. [6]
  5. Avoid dangerous “force yes” behavior

    • Don’t use --force-yes (deprecated and unsafe); prefer explicit allow-* options when you truly mean them. [7]

Common pattern (robust CI/Docker layer):

apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends <packages>
rm -rf /var/lib/apt/lists/*

Sources: debconf frontend behavior [1]; apt-get -y semantics [2]; Debian preseeding workflow [3]; Docker persistence caveat [5]; failing on prompts via /dev/null [6]; --force-yes warning/deprecation [7].


Prevent Icecast install from prompting in CI.
The icecast2 package includes debconf prompts for hostname and passwords during installation, which can hang or fail CI workflows. Use DEBIAN_FRONTEND=noninteractive to skip interactive prompts and apply defaults.

🛠️ Suggested fix
-          sudo apt-get install -y icecast2
+          sudo DEBIAN_FRONTEND=noninteractive apt-get install -y icecast2
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 22 - 27, The CI step that installs the
icecast2 package (the "Install Icecast (for IcecastService / api/system/config)"
run block) can hang on debconf prompts; modify the install command to run
non-interactively by setting DEBIAN_FRONTEND=noninteractive for the apt-get
install step (i.e., ensure the command that installs icecast2 is prefixed or run
with DEBIAN_FRONTEND=noninteractive so prompts are skipped and defaults applied,
keeping the existing sudo ln -sf /usr/bin/icecast2 /usr/bin/icecast line
unchanged).

Comment thread AGENTS.md
Comment on lines +9 to +10
| Situation | Agent |
|-----------|--------|
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix markdownlint MD060 table spacing.
The table separator row at Line 10 trips MD060 (table-column-style). Adjust spacing to match the configured style.

✏️ Suggested fix
-|-----------|--------|
+| --------- | ------ |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| Situation | Agent |
|-----------|--------|
| Situation | Agent |
| --------- | ------ |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)

[warning] 10-10: Table column style
Table pipe is missing space to the right for style "compact"

(MD060, table-column-style)


[warning] 10-10: Table column style
Table pipe is missing space to the left for style "compact"

(MD060, table-column-style)


[warning] 10-10: Table column style
Table pipe is missing space to the right for style "compact"

(MD060, table-column-style)


[warning] 10-10: Table column style
Table pipe is missing space to the left for style "compact"

(MD060, table-column-style)

🤖 Prompt for AI Agents
In `@AGENTS.md` around lines 9 - 10, The table separator row in AGENTS.md violates
markdownlint MD060 due to inconsistent spacing; edit the separator line between
the header row ("| Situation | Agent |") and the data rows so the pipes and
dashes use the same column spacing style (e.g., align separators with single
spaces around column content), ensuring the separator row matches the header
spacing and resolves MD060.

Comment on lines +38 to +52
/**
* Format a stream start/restart error message, appending capacity info when present from the API.
* @param {Object} data - API response body
* @param {string} defaultMsg - Fallback message
* @returns {string}
*/
formatStreamError(data, defaultMsg) {
let msg = (data && (data.error || data.message)) || defaultMsg;
if (data && data.capacity) {
const c = data.capacity;
msg += ` — ${c.activeStreams ?? '?'}/${c.sourceLimit ?? '?'} streams in use`;
if (c.configPath) msg += ` (config: ${c.configPath})`;
}
return msg;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use shortMessage and activeCount when available in stream errors.

Backend now provides shortMessage and capacity.activeCount; ignoring them yields ?/ and loses the concise diagnosis.

🔧 Suggested update
-    let msg = (data && (data.error || data.message)) || defaultMsg;
+    let msg = (data && (data.shortMessage || data.error || data.message)) || defaultMsg;
     if (data && data.capacity) {
         const c = data.capacity;
-        msg += ` — ${c.activeStreams ?? '?'}/${c.sourceLimit ?? '?'} streams in use`;
+        const active = c.activeCount ?? c.activeStreams;
+        msg += ` — ${active ?? '?'}/${c.sourceLimit ?? '?'} streams in use`;
         if (c.configPath) msg += ` (config: ${c.configPath})`;
     }
🤖 Prompt for AI Agents
In `@public/components/FFmpegStreamsManager.js` around lines 38 - 52,
formatStreamError currently builds the message from data.error/data.message and
capacity.activeStreams/sourceLimit, which misses new fields; update
formatStreamError to prefer data.shortMessage over data.error/data.message and
prefer data.capacity.activeCount over data.capacity.activeStreams when present,
falling back to the existing fields or defaultMsg; ensure the capacity text uses
activeCount/sourceLimit (with '?' fallbacks) and still appends (config:
configPath) when data.capacity.configPath exists.

Comment thread src/routes/system.js
Comment on lines +157 to +170
const sourceLimit = icecastService.getSourceLimit();

// Get active streams count
const activeStreams = Object.values(streamingService.activeStreams || {})
.filter(s => s.status === 'running').length;

res.json({
icecast: {
port: actualPort,
host
host,
sourceLimit,
activeStreams,
remaining: sourceLimit - activeStreams,
configPath: icecastService.paths?.config
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Find the system.js file and check the route definition for this endpoint
find . -name "system.js" -path "*/routes/*" -type f

Repository: jerryagenyi/LANStreamer

Length of output: 89


🏁 Script executed:

# Search for the /api/system/config route definition and any middleware
rg -A 10 -B 5 "system.*config|/api/system" --type js

Repository: jerryagenyi/LANStreamer

Length of output: 48271


🏁 Script executed:

# Check for authentication middleware applied to system routes
rg -B 20 "router\.get.*system|system\.js" --type js | head -100

Repository: jerryagenyi/LANStreamer

Length of output: 344


🏁 Script executed:

# Look for any guards or middleware that might protect this endpoint
rg -A 5 "localhost|NODE_ENV.*production|isAuthenticated|requireAuth" src/routes/system.js

Repository: jerryagenyi/LANStreamer

Length of output: 347


🏁 Script executed:

# Check the actual endpoint definition and any security context
sed -n '157,190p' src/routes/system.js

Repository: jerryagenyi/LANStreamer

Length of output: 886


🏁 Script executed:

# Look for any deployment docs or security guidelines
find . -type f \( -name "README*" -o -name "SECURITY*" -o -name "*.md" \) | head -10 | xargs grep -l "localhost\|security\|deployment" 2>/dev/null | head -5

Repository: jerryagenyi/LANStreamer

Length of output: 100


🏁 Script executed:

# Check if there are any comments about intended deployment context
rg -B 5 -A 5 "localhost|LAN|public.*endpoint" src/routes/system.js | head -40

Repository: jerryagenyi/LANStreamer

Length of output: 781


🏁 Script executed:

# Check README for deployment/security context
head -100 README.md | grep -A 10 -B 10 "localhost\|security\|deployment\|LAN" || cat README.md | head -50

Repository: jerryagenyi/LANStreamer

Length of output: 3978


🏁 Script executed:

# Verify that configPath is intentionally returned in tests
cat tests/integration/config-endpoint.test.js | grep -A 5 -B 5 "configPath"

Repository: jerryagenyi/LANStreamer

Length of output: 981


🏁 Script executed:

# Check if there are any environment-specific security considerations
grep -r "NODE_ENV\|production\|deployment" README.md TODO.md AGENTS.md 2>/dev/null | head -20

Repository: jerryagenyi/LANStreamer

Length of output: 49


Remove or conditionally gate configPath from the /api/system/config endpoint.

While LANStreamer is designed for local network deployment, this endpoint is completely public and unconditionally exposes the Icecast configuration file path. If the application is accessible beyond its intended LAN boundary—whether through misconfiguration, port forwarding, or network exposure—this path disclosure could aid reconnaissance. Consider gating it to non-production environments as a defensive practice.

Suggested guard
-        configPath: icecastService.paths?.config
+        ...(process.env.NODE_ENV === 'production'
+          ? {}
+          : { configPath: icecastService.paths?.config })
🤖 Prompt for AI Agents
In `@src/routes/system.js` around lines 157 - 170, The response currently always
returns icecastService.paths?.config under configPath in the /api/system/config
route, which leaks the Icecast config file path; change this so configPath is
either removed or only included under a safe condition (e.g., non-production/dev
environment or when a feature flag is enabled). Locate the JSON response
construction in the route handler (the res.json block referencing
icecastService.paths?.config and the symbol configPath) and wrap the inclusion
in an environment/flag check or omit it entirely for production builds so the
path is not exposed publicly.

Comment on lines +863 to +865
getSourceLimit() {
return this.sourceLimit || 2; // Icecast default
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Preserve explicit 0 values in getSourceLimit.

|| 2 will coerce 0 to the default; use a finite/nullish check instead.

✅ Suggested fix
-  return this.sourceLimit || 2; // Icecast default
+  return Number.isFinite(this.sourceLimit) ? this.sourceLimit : 2; // Icecast default
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
getSourceLimit() {
return this.sourceLimit || 2; // Icecast default
}
getSourceLimit() {
return Number.isFinite(this.sourceLimit) ? this.sourceLimit : 2; // Icecast default
}
🤖 Prompt for AI Agents
In `@src/services/IcecastService.js` around lines 863 - 865, getSourceLimit
currently uses `this.sourceLimit || 2`, which treats 0 as falsy and incorrectly
falls back to 2; update the method (getSourceLimit) to preserve explicit 0 by
performing a nullish/finite check instead (e.g., use the nullish coalescing or
Number.isFinite check on `this.sourceLimit`) so that 0 is returned when
explicitly set, otherwise return 2.

Comment on lines +121 to +141
// Capacity check: ensure config is loaded so sourceLimit is valid; avoid NaN in remaining
await IcecastService.ensureInitialized()
const rawLimit = IcecastService.getSourceLimit()
const sourceLimit = (typeof rawLimit === 'number' && !Number.isNaN(rawLimit)) ? rawLimit : 2
const activeCount = Object.values(this.activeStreams).filter(s => s.status === 'running').length
const remaining = Math.max(0, sourceLimit - activeCount)
const configPath = IcecastService.paths?.config
// Log message includes values so grep / log scans show capacity in one place (no need to read next-line JSON)
logger.info(
`Stream capacity check: sourceLimit=${sourceLimit} activeCount=${activeCount} remaining=${remaining} configPath=${configPath ?? 'n/a'} aboutToStart=${streamConfig.name || streamConfig.id}`,
{ sourceLimit, activeCount, remaining, configPath, aboutToStart: streamConfig.name || streamConfig.id }
)

if (remaining <= 0) {
const err = new Error(
`Stream limit reached. Icecast allows ${sourceLimit} concurrent stream(s); ${activeCount} already running. ` +
`Edit <sources> in icecast.xml and restart Icecast, or stop a stream first. Config: ${configPath || 'unknown'}`
)
err.capacity = { sourceLimit, activeCount, remaining: 0, configPath }
throw err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Capacity check can be bypassed by concurrent starts.

Because no slot is reserved until after FFmpeg starts, parallel start requests can both pass and exceed sourceLimit. Consider reserving a slot (e.g., mark a stream as starting before the async start and count it) or serializing starts.

🤖 Prompt for AI Agents
In `@src/services/StreamingService.js` around lines 121 - 141, The capacity race
happens because we only count streams with status 'running' in the activeCount
calculation, so concurrent starts slip past the remaining check; update the
start flow to reserve a slot by setting this.activeStreams[streamId].status =
'starting' (or similar) before any async work/FFmpeg spawn and change the
activeCount computation in StreamingService to count both 'running' and
'starting' states (e.g., where activeCount is computed from this.activeStreams).
Ensure the slot is cleared on failure (remove entry or set to 'stopped') and
transitioned to 'running' on successful start so the existing remaining check
and thrown capacity Error (err.capacity) reflect reserved slots.

Comment thread TODO.md
- **Listening page:** Play uses same-origin proxy `/api/streams/play/:streamId`; Copy URL gives direct Icecast URL.
- **Tests:** Unit and integration tests in place; see [docs/TEST-PLAN.md](docs/TEST-PLAN.md). Run with `npm test`.
- **Docs:** [docs/TROUBLESHOOTING.md](docs/TROUBLESHOOTING.md), [docs/STARTUP-SEQUENCE.md](docs/STARTUP-SEQUENCE.md), [docs/NETWORK-SETUP.md](docs/NETWORK-SETUP.md).
- [x] **Verify 5+ streams** — Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of “5th fails” was bad device choice, not Icecast limit).
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the stray closing parenthesis.
Line 9 ends with an extra “)”.

✂️ Suggested fix
- - [x] **Verify 5+ streams** — Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of “5th fails” was bad device choice, not Icecast limit).
+ - [x] **Verify 5+ streams** — Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of “5th fails” was bad device choice, not Icecast limit.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- [x] **Verify 5+ streams** — Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of 5th fails was bad device choice, not Icecast limit).
- [x] **Verify 5+ streams** — Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of "5th fails" was bad device choice, not Icecast limit.
🤖 Prompt for AI Agents
In `@TODO.md` at line 9, The line containing the checklist item "Verify 5+ streams
— Confirmed 6/6 live (VB-Cable A/B, mics). Root cause of “5th fails” was bad
device choice, not Icecast limit)" has an extra closing parenthesis at the end;
edit that checklist entry to remove the stray ")" so the sentence ends cleanly
(e.g., after "limit").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant