Repository navigation
feat: install and run the Jellyfin media server on the gateway host - #355
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBftoc1JCMLow9LDFZMADs
|
|
||
| # Gateway-only (mac-mini): same hostname match as is_gateway_host in bootstrap.sh | ||
| require "socket" | ||
| if Socket.gethostname.split(".").first.match?(/\Amac-mini(-\d+)?\z/) |
There was a problem hiding this comment.
[Suggestion] The gateway regex now lives in two languages: is_gateway_host in bootstrap.sh and this line. The comment points at the bash version, but nothing makes the two stay in step. If one is widened later (for example to a new hostname), the other drifts silently. A small check in tests/test-bootstrap.sh could catch that. It would assert that the Brewfile contains mac-mini(-\d+)? and bootstrap.sh contains mac-mini(-[0-9]+)?, or extract both and run the same sample hostnames through each.
| # Gateway-only (mac-mini): same hostname match as is_gateway_host in bootstrap.sh | ||
| require "socket" | ||
| if Socket.gethostname.split(".").first.match?(/\Amac-mini(-\d+)?\z/) | ||
| cask "jellyfin" # Media server (http://localhost:8096) |
There was a problem hiding this comment.
[Suggestion] The cask only installs Jellyfin.app. Nothing starts it at login, so after a reboot the server is down until someone launches it by hand. If it is meant to be always-on like the other gateway services, consider adding it as a Login Item, or adding a LaunchAgents/ plist gated by is_gateway_host like com.evansenter.obsidian-mcp. It could also go in the CLAUDE.md Infrastructure Services table (port 8096). That can be a follow-up if you would rather keep this PR install-only.
There was a problem hiding this comment.
Code Review — Round 1
Summary
Adds a jellyfin cask to the Brewfile, gated on the hostname. The Ruby check (Socket.gethostname.split(".").first against \Amac-mini(-\d+)?\z) is equivalent to is_gateway_host (${HOSTNAME%%.*} against ^mac-mini(-[0-9]+)?$). Both read gethostname(2), strip at the first dot, and anchor the same pattern, so mac-mini, mac-mini.local and mac-mini-2 match, and macbook.local and mac-mini-old do not. This follows the existing Xcode conditional pattern in the Brewfile, and none of the Brewfile ban-list tests in tests/test-bootstrap.sh are affected.
Note: the round-count command (--paginate --slurp --jq) is rejected by this gh version, which says --slurp is not supported with --jq. I counted with --paginate --jq per review instead. That found zero prior automated reviews, so this is round 1.
Verdict
APPROVE - No blocking findings. Two non-blocking suggestions are posted inline: keeping the two copies of the gateway regex in step, and auto-starting Jellyfin at login.
Automated review by Claude Code
…atcher On macOS 27, .NET's FSEvents-backed config watcher blocks forever at startup, so the server never logs or binds 8096. Run the cask's server binary from a gateway-only LaunchAgent with DOTNET_USE_POLLING_FILE_WATCHER=1. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBftoc1JCMLow9LDFZMADs
| <string>__HOME__/.local/log/jellyfin.err</string> | ||
|
|
||
| <key>ProcessType</key> | ||
| <string>Background</string> |
There was a problem hiding this comment.
[Suggestion] ProcessType=Background is the right choice for sysload and obsidian-mcp. For Jellyfin it is less clear. launchd applies Background-class CPU and I/O throttling, and live transcoding and streaming are user-facing work. When the box is under contention, that throttling can show up as playback buffering. Consider Standard, which is the launchd default, or dropping the key. That is only worth doing if you see stutter while transcoding.
| <integer>30</integer> | ||
|
|
||
| <key>StandardOutPath</key> | ||
| <string>__HOME__/.local/log/jellyfin.stdout</string> |
There was a problem hiding this comment.
[Suggestion] Jellyfin already writes rotated logs under its own log dir, and it also writes to the console. As a result, jellyfin.stdout gets every console log line and nothing ever rotates or truncates it, so on an always-on server it grows without bound. Consider pointing StandardOutPath at /dev/null, or logging less to the console (e.g. via logging.json), and keeping only stderr for crash output.
There was a problem hiding this comment.
Code Review — Round 2
Summary
This round answers the round-1 suggestion about auto-start. It adds a gateway-only com.evansenter.jellyfin LaunchAgent that runs the cask server binary headless with DOTNET_USE_POLLING_FILE_WATCHER=1. It also adds a regression test that pins the env var, and documents the service in CLAUDE.md. The install gate in install_launch_agents() is is_gateway_host plus -x on the binary, which matches how obsidian-mcp is gated. install_launch_agent substitutes __HOME__, and the data dir path contains a space, but it is a single ProgramArguments element, so that is safe. KeepAlive/SuccessfulExit=false plus ThrottleInterval match the existing agents.
Round count: --paginate --slurp --jq is rejected by this gh version, so I counted with --paginate --jq per page. That found 1 prior automated review, so this is round 2.
Previously Addressed (Filtered)
Brewfile:76: nothing starts Jellyfin at login. This round adds the LaunchAgent.Brewfile:75: the regex is duplicated across bash and Ruby. Not re-raised.
Verdict
APPROVE - No blocking findings. Two non-blocking suggestions are posted inline: ProcessType=Background throttling for transcoding, and the unbounded stdout log.
Automated review by Claude Code
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LBftoc1JCMLow9LDFZMADs
There was a problem hiding this comment.
Code Review — Round 3
Summary
The new commit (7ca3214) handles both round-2 suggestions in LaunchAgents/com.evansenter.jellyfin.plist:
ProcessTypeis nowStandard, so launchd no longer caps CPU and IO during transcoding.- stdout now goes to
/dev/null, because Jellyfin already writes rotating logs under its datadir. stderr is still kept in~/.local/log/jellyfin.err, which catches crashes that happen before logging starts.
Both changes have a comment explaining why. The DOTNET_USE_POLLING_FILE_WATCHER regression test (tests/test-bootstrap.sh:707) still holds. No tests depend on the stdout path. install_launch_agent still creates ~/.local/log for the stderr file. Nothing outside the plist changed in this round.
Round count: --paginate --slurp --jq is rejected by this gh version, so I counted with --paginate --jq. That found 2 prior automated reviews, so this is round 3.
Previously Addressed (Filtered)
Brewfile: nothing started Jellyfin at login. Fixed in round 2 by the LaunchAgent.Brewfile: the gateway regex is duplicated across bash and Ruby. Not re-raised.com.evansenter.jellyfin.plist:ProcessType=Backgroundthrottled transcoding. Fixed in7ca3214.com.evansenter.jellyfin.plist: the stdout log grew without limit. Fixed in7ca3214.
Verdict
APPROVE - No blocking findings and no new suggestions.
Automated review by Claude Code
Adds
cask "jellyfin"to the Brewfile (gated to mac-mini with the same hostname match asis_gateway_host) and a gateway-onlycom.evansenter.jellyfinLaunchAgent that runs the server binary headless.Why a LaunchAgent instead of Jellyfin.app: on macOS 27.0.1 the server hangs forever at startup — it writes
logging.default.jsonand then blocks starting .NET's FSEvents-backed config watcher, never logging or binding 8096 (reproduced with a fresh data dir and with--nonetchange).DOTNET_USE_POLLING_FILE_WATCHER=1fixes it (up in <1s); the app wrapper can't be given that environment. Matches dotnet/runtime#131323 and Macro-Deck-App/Macro-Deck#1445.Verified: agent installed via
install_launch_agent,/web/returns 200; new regression test pins the env var;make check67/67.🤖 Generated with Claude Code
https://claude.ai/code/session_01LBftoc1JCMLow9LDFZMADs