From fc1c41b8da6253cfe217b33eca4cee43950f00a3 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 19:57:12 -0600 Subject: [PATCH 01/11] Harden what user input and the deploy tooling can reach - Refuse !play links whose host resolves to a private, loopback or link-local address. yt-dlp fetches from the host the bot runs on, which is now a home LAN, so any guild member could make it probe the router. - End yt-dlp's option parsing with -- before the stream target, so a target can never be read as a flag. - Give the CI job a read-only GITHUB_TOKEN. - Stop defaulting launch_ec2.sh to the owner's personal key pair name. - Document why PyNaCl stays at 1.5.0 despite pip-audit (discord.py caps it below 1.6; the advisory is in code voice never calls) and why curl_cffi is required although nothing imports it. - The README asked for the Server Members intent, which the bot never requests. --- .github/workflows/tests.yml | 5 +++++ README.md | 3 ++- cogs/music.py | 3 +++ deploy/launch_ec2.sh | 7 +++++- requirements.txt | 6 +++++ services/media.py | 32 ++++++++++++++++++++++++++- tests/test_audio_stream.py | 11 ++++++++++ tests/test_commands.py | 12 ++++++++++ tests/test_media_helpers.py | 44 +++++++++++++++++++++++++++++++++++++ 9 files changed, 120 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ec5c213..bbf4bbe 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -5,6 +5,11 @@ on: branches: [main] pull_request: +# The job only reads the repo. Without this, GITHUB_TOKEN gets the repository's +# default scopes, which can include write access. +permissions: + contents: read + jobs: pytest: runs-on: ubuntu-latest diff --git a/README.md b/README.md index d1128f6..0f24840 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,8 @@ No installation required — the bot is hosted and always online. ### Requirements - Python 3.11+ - FFmpeg on your PATH -- A Discord bot token (with the **Message Content** and **Server Members** intents enabled) +- A Discord bot token with the **Message Content** intent enabled — the only + privileged intent the bot asks for ### Run locally ```bash diff --git a/cogs/music.py b/cogs/music.py index 183e58c..d852fb1 100644 --- a/cogs/music.py +++ b/cogs/music.py @@ -78,6 +78,9 @@ async def play(self, ctx, *, query: str): query = query.strip() if len(query) > MAX_QUERY_LEN: return await ctx.send(embed=error_embed("That query is too long.")) + if query.startswith("http") and not await media.is_public_url(query): + return await ctx.send(embed=error_embed( + "I can only play links to public websites.")) if not await self._ensure_voice(ctx): return player = self._player(ctx) diff --git a/deploy/launch_ec2.sh b/deploy/launch_ec2.sh index 14f14e1..82f60f9 100644 --- a/deploy/launch_ec2.sh +++ b/deploy/launch_ec2.sh @@ -5,13 +5,18 @@ # # Prereqs: AWS CLI configured, an existing EC2 key pair whose .pem you hold. # +# Usage: +# KEY_NAME=my-key-pair bash deploy/launch_ec2.sh +# set -euo pipefail # ── Config (override via environment) ───────────────────────────────── REGION="${AWS_REGION:-us-east-1}" INSTANCE_TYPE="${INSTANCE_TYPE:-t4g.micro}" # ARM, free-tier eligible AMI_ID="${AMI_ID:-ami-02c4144237becae44}" # Ubuntu 24.04 arm64 (us-east-1) -KEY_NAME="${KEY_NAME:-ils-acc-examplekey-us-east-1}" # existing key pair +# The EC2 key pair to install on the instance. No default: it has to be one +# whose .pem you hold, and guessing wrong launches a box nobody can log in to. +KEY_NAME="${KEY_NAME:?Set KEY_NAME to an existing EC2 key pair name}" SG_NAME="${SG_NAME:-loopify-bot-sg}" NAME_TAG="${NAME_TAG:-loopify-bot}" VOLUME_GB="${VOLUME_GB:-8}" diff --git a/requirements.txt b/requirements.txt index d87f9d8..1ebcc92 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,9 +2,15 @@ # deliberately and re-run the suite; do not let them float. discord.py[voice]==2.7.1 lyricsgenius==3.12.2 +# Capped below 1.6 by discord.py 2.7.1. pip-audit flags 1.5.0 for +# PYSEC-2026-1448 / PYSEC-2026-3002, which are in libsodium's ed25519 point +# validation — voice only uses its secretbox/AEAD ciphers, so the bot never +# reaches that code. Lift the pin once discord.py allows 1.6.2. PyNaCl==1.5.0 python-dotenv==1.2.2 aiohttp==3.14.3 +# Never imported here: yt-dlp uses it for browser impersonation, which some +# extractors require. Removing it breaks those sites, not an import. curl_cffi==0.15.0 # Deliberately NOT pinned. YouTube changes its player, signature challenge and diff --git a/services/media.py b/services/media.py index 5a7d9af..945e708 100644 --- a/services/media.py +++ b/services/media.py @@ -21,12 +21,14 @@ import sys import time import queue +import ipaddress import threading import subprocess import tempfile import asyncio import logging from typing import Optional +from urllib.parse import urlsplit import discord import yt_dlp @@ -180,6 +182,31 @@ async def related(track: dict, *, loop=None) -> Optional[dict]: return None +async def is_public_url(url: str) -> bool: + """ + Whether every address ``url``'s host resolves to is on the public internet. + + yt-dlp fetches whatever it is given, from the host the bot runs on. Without + this, ``!play http://192.168.1.1/`` makes the bot probe the LAN it sits in + (or a cloud metadata endpoint) on behalf of anyone in any guild. A host + that does not resolve is refused too: there is nothing to play there, and + it is not worth letting through to find out. + + This does not follow redirects — yt-dlp does that itself — so it narrows + the exposure rather than closing it. + """ + host = urlsplit(url).hostname + if not host: + return False + try: + infos = await asyncio.get_running_loop().getaddrinfo(host, None) + except OSError: + return False + addresses = {info[4][0].split("%", 1)[0] for info in infos} # drop IPv6 scope + return bool(addresses) and all( + ipaddress.ip_address(address).is_global for address in addresses) + + def _first_entry(info): """Unwrap the first playable entry from a search/playlist result.""" if info and "entries" in info: @@ -339,7 +366,10 @@ def spawn_stream(track: dict) -> AudioStream: cookies = YTDL_OPTIONS.get("cookiefile") if cookies: cmd += ["--cookies", cookies] - cmd.append(_stream_target(track)) + # `--` ends option parsing, so a target that starts with a dash is read as a + # URL and never as a flag. _search_target already prefixes every non-URL + # query, but the argv of a subprocess is not the place to rely on that. + cmd += ["--", _stream_target(track)] return AudioStream.launch(cmd) diff --git a/tests/test_audio_stream.py b/tests/test_audio_stream.py index 7a93593..cae1ed9 100644 --- a/tests/test_audio_stream.py +++ b/tests/test_audio_stream.py @@ -228,3 +228,14 @@ def test_spawn_stream_omits_cookies_when_not_configured(monkeypatch): monkeypatch.delitem(media.YTDL_OPTIONS, "cookiefile", raising=False) media.spawn_stream({"url": "https://example.invalid/x", "title": "T"}) assert "--cookies" not in captured["cmd"] + + +def test_spawn_stream_never_lets_the_target_be_read_as_an_option(monkeypatch): + """A target starting with a dash must reach yt-dlp as a URL, not a flag.""" + captured = {} + monkeypatch.setattr(AudioStream, "launch", classmethod( + lambda cls, cmd: captured.setdefault("cmd", cmd) + )) + media.spawn_stream({"url": "--exec=touch /tmp/pwned", "title": "T"}) + cmd = captured["cmd"] + assert cmd[-2:] == ["--", "--exec=touch /tmp/pwned"] diff --git a/tests/test_commands.py b/tests/test_commands.py index 65b9728..c35940a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -144,6 +144,18 @@ async def test_play_rejects_an_over_length_query(music_cog, ctx): ctx.typing.assert_not_called() # rejected before any yt-dlp work +@pytest.mark.parametrize("url", [ + "http://192.168.1.1/admin", + "http://127.0.0.1:8080/", +]) +async def test_play_refuses_links_into_private_networks(music_cog, ctx, url): + ctx.author.voice.channel = MagicMock() + with patch("cogs.music.media.search", new=AsyncMock()) as search: + await Music.play.callback(music_cog, ctx, query=url) + assert "public" in sent_text(ctx) + search.assert_not_awaited() + + async def test_shuffle_on_an_empty_queue(music_cog, ctx): await Music.shuffle.callback(music_cog, ctx) assert "empty" in sent_text(ctx).lower() diff --git a/tests/test_media_helpers.py b/tests/test_media_helpers.py index a899a91..7967b62 100644 --- a/tests/test_media_helpers.py +++ b/tests/test_media_helpers.py @@ -6,6 +6,8 @@ Process handling lives in ``tests/test_audio_stream.py``. """ +import asyncio + import pytest from services import media @@ -167,3 +169,45 @@ def test_a_client_needing_no_js_runtime_remains_as_a_last_resort(): host where Deno failed to install. """ assert "tv_embedded" in media._PLAYER_CLIENTS + + +# ── which links the bot is willing to fetch ─────────────────────────── +# +# Literal addresses and "localhost" resolve without a network, so these stay +# offline like the rest of the suite. + +@pytest.mark.parametrize("url", [ + "http://127.0.0.1/audio.mp3", + "http://localhost:8080/stream", + "http://192.168.1.1/", # the router of the LAN the bot sits in + "http://10.0.0.5/x", + "http://169.254.169.254/latest/meta-data/", # cloud instance metadata + "http://[::1]/x", + "http://[::ffff:192.168.1.1]/x", # a private IPv4 dressed up as IPv6 + "http:///no-host", +]) +async def test_links_into_private_networks_are_refused(url): + assert await media.is_public_url(url) is False + + +async def test_a_link_to_a_public_address_is_allowed(): + assert await media.is_public_url("https://8.8.8.8/audio.mp3") is True + + +async def test_a_host_that_does_not_resolve_is_refused(monkeypatch): + async def unresolvable(*_args, **_kwargs): + raise OSError("Name or service not known") + + loop = asyncio.get_running_loop() + monkeypatch.setattr(loop, "getaddrinfo", unresolvable) + assert await media.is_public_url("https://nowhere.invalid/x") is False + + +async def test_one_private_address_among_public_ones_is_enough_to_refuse(monkeypatch): + """A name with a public and a private record could be answered with either.""" + async def mixed(*_args, **_kwargs): + return [(2, 1, 6, "", ("8.8.8.8", 0)), (2, 1, 6, "", ("10.0.0.1", 0))] + + loop = asyncio.get_running_loop() + monkeypatch.setattr(loop, "getaddrinfo", mixed) + assert await media.is_public_url("https://mixed.invalid/x") is False From 60fe4c5007fa6ce9c04bfed0366b2afca609ea0a Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 19:59:17 -0600 Subject: [PATCH 02/11] Skip a track yt-dlp cannot start instead of tearing down the player MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If spawning yt-dlp raised (out of processes under TasksMax, say), the exception escaped into the playback loop's catch-all, which destroyed the player: the bot left voice and nobody in the channel was told why. Opening the stream now lives in _open_stream, which announces the failure and returns None, and the loop moves on to the next track. The replay flag is cleared on that path too — left set after a failed effect respawn, it made _advance return a track that was gone, which read as an empty queue. tests/test_player_loop.py is the first test to run _player_loop end to end; both cases fail against the previous loop. Also drop discord.Forbidden from two except clauses that already catch its base class, HTTPException. --- tests/test_player_loop.py | 99 +++++++++++++++++++++++++++++++++++++++ utils/announcer.py | 2 +- utils/errors.py | 2 +- utils/player.py | 35 +++++++++++--- 4 files changed, 130 insertions(+), 8 deletions(-) create mode 100644 tests/test_player_loop.py diff --git a/tests/test_player_loop.py b/tests/test_player_loop.py new file mode 100644 index 0000000..50180af --- /dev/null +++ b/tests/test_player_loop.py @@ -0,0 +1,99 @@ +""" +The playback loop itself, run for real against a fake voice client. + +The other player tests drive ``_advance`` and the controls directly. These run +``_player_loop`` end to end, with yt-dlp, FFmpeg and Discord replaced, to check +what happens *between* tracks — which is where a failure used to take the whole +player down with it. +""" + +import asyncio +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from services import media +from tests.conftest import make_track + + +def _run_inline(_executor, fn, *args): + """``run_in_executor`` without the thread: a future that is already done.""" + future = asyncio.get_running_loop().create_future() + try: + future.set_result(fn(*args)) + except Exception as e: + future.set_exception(e) + return future + + +@pytest.fixture +def looping(player, fake_bot, fake_guild, monkeypatch): + """A player whose loop can run: voice connected, announcer and media faked.""" + fake_bot.wait_until_ready = AsyncMock() + fake_bot.loop.run_in_executor.side_effect = _run_inline + fake_bot.loop.call_soon_threadsafe.side_effect = lambda fn: fn() + + vc = MagicMock() + vc.is_connected.return_value = True + vc.disconnect = AsyncMock() + played: list[dict] = [] + + def play(_source, *, after): + # One track is enough to prove the loop carried on: stop here. + played.append(player.current) + player.destroy() + after(None) + + vc.play.side_effect = play + fake_guild.voice_client = vc + + player.announcer = MagicMock(now_playing=AsyncMock(), load_failed=AsyncMock(), + idle_disconnect=AsyncMock()) + monkeypatch.setattr(media, "make_pipe_source", lambda *a, **k: MagicMock()) + monkeypatch.setattr(media, "prime_source", lambda source: True) + player.played = played + return player + + +async def test_a_track_yt_dlp_cannot_start_is_announced_and_skipped(looping, monkeypatch): + broken, fine = make_track("Broken"), make_track("Fine") + + def spawn(track): + if track is broken: + raise OSError("fork failed: resource temporarily unavailable") + return MagicMock() + + monkeypatch.setattr(media, "spawn_stream", spawn) + looping.add_many([broken, fine]) + + await asyncio.wait_for(looping._player_loop(), timeout=5) + + looping.announcer.load_failed.assert_awaited_once_with(broken) + assert broken["error"] == "unavailable" + assert looping.played == [fine], "the queue must carry on after the failure" + + +async def test_a_failed_respawn_during_an_effect_change_does_not_end_the_session( + looping, monkeypatch): + """ + An effect change replays the current track. If that respawn cannot start, + the replay flag must not survive: _advance would hand back a track that is + gone, and the loop would read that as an empty queue and disconnect. + """ + current, following = make_track("Current"), make_track("Following") + + def spawn(track): + if track is current: + raise OSError("fork failed") + return MagicMock() + + monkeypatch.setattr(media, "spawn_stream", spawn) + looping.current = current + looping._replay = True + looping._resume_at = 42.0 + looping.add(following) + + await asyncio.wait_for(looping._player_loop(), timeout=5) + + assert looping.played == [following] + looping.announcer.idle_disconnect.assert_not_awaited() diff --git a/utils/announcer.py b/utils/announcer.py index 54e48d3..c3a6a72 100644 --- a/utils/announcer.py +++ b/utils/announcer.py @@ -40,5 +40,5 @@ async def _send(self, embed: discord.Embed) -> None: error worth propagating into the playback loop.""" try: await self.channel.send(embed=embed) - except (discord.HTTPException, discord.Forbidden) as e: + except discord.HTTPException as e: log.debug("Could not announce to channel: %s", e) diff --git a/utils/errors.py b/utils/errors.py index abcbf6f..361d61a 100644 --- a/utils/errors.py +++ b/utils/errors.py @@ -80,5 +80,5 @@ async def handle(ctx, error: Exception) -> None: async def _reply(ctx, message: str) -> None: try: await ctx.send(embed=error_embed(message)) - except (discord.HTTPException, discord.Forbidden) as e: + except discord.HTTPException as e: log.debug("Could not report an error to the channel: %s", e) diff --git a/utils/player.py b/utils/player.py index c3ce7bc..30921cf 100644 --- a/utils/player.py +++ b/utils/player.py @@ -361,13 +361,14 @@ async def _player_loop(self) -> None: if not vc or not vc.is_connected(): return self.destroy() - # Stream the audio through yt-dlp → FFmpeg (see services.media). - # A stream fetched while the previous track played starts - # instantly; otherwise pay the 3–8s yt-dlp startup now. - stream = self._take_prefetch(track) + stream = await self._open_stream(track) if stream is None: - stream = await self.bot.loop.run_in_executor( - None, media.spawn_stream, track) + # Move on rather than retry: a replay flag left set would + # make _advance hand back a track that is no longer there. + self._replay = False + self._resume_at = 0.0 + self.current = None + continue self._stream = stream was_replay = self._replay self._replay = False @@ -419,6 +420,28 @@ async def _player_loop(self) -> None: log.exception("Player loop crashed for guild %s", self.guild.id) self.destroy() + async def _open_stream(self, track: dict) -> Optional[media.AudioStream]: + """ + The track's audio: the prefetched stream if there is one, else a new one. + + A stream fetched while the previous track played starts instantly; + otherwise this pays the 3–8s yt-dlp startup. ``None`` means yt-dlp could + not even be started (no processes left, say). That is one track's + failure — it is announced here so the loop can carry on with the queue + instead of tearing the whole player down. + """ + stream = self._take_prefetch(track) + if stream is not None: + return stream + try: + return await self.bot.loop.run_in_executor( + None, media.spawn_stream, track) + except Exception: + log.exception("Could not start streaming in guild %s", self.guild.id) + track["error"] = "unavailable" + await self.announcer.load_failed(track) + return None + def _after_play(self, error: Optional[Exception]) -> None: """Runs in the voice thread — hand control back to the loop safely.""" if error: From fedebf457a0b9499c01a4086891914a290ae4512 Mon Sep 17 00:00:00 2001 From: Ismael Leon Date: Mon, 21 Sep 2026 20:01:25 -0600 Subject: [PATCH 03/11] Name commands with the configured prefix, not a hardcoded ! COMMAND_PREFIX is configurable, but the effects list, the effect help, the !lyrics hint, the Now Playing footer and the YouTube-blocked message all spelled commands with a literal '!'. On a server using another prefix they pointed at commands that do not exist there. Replies to a command use ctx.clean_prefix; embeds built without a context, and help text fixed at import time, use config.COMMAND_PREFIX. !move's help no longer repeats the signature the help already shows. --- cogs/effects.py | 9 ++++++--- cogs/lyrics.py | 3 ++- cogs/music.py | 2 +- tests/test_commands.py | 31 +++++++++++++++++++++++++++++++ utils/embeds.py | 6 ++++-- 5 files changed, 44 insertions(+), 7 deletions(-) diff --git a/cogs/effects.py b/cogs/effects.py index ec669c4..03c0efc 100644 --- a/cogs/effects.py +++ b/cogs/effects.py @@ -4,6 +4,7 @@ import discord from discord.ext import commands +from config import COMMAND_PREFIX from utils.checks import same_voice_channel from utils.embeds import BLURPLE, error_embed, success_embed from utils.player import players @@ -81,7 +82,8 @@ async def _switch_to(self, ctx, name: str | None, filter_str: str, label: str, await ctx.send(embed=error_embed("Nothing is playing.")) @commands.command(name=_EFFECT_NAMES[0], aliases=_EFFECT_NAMES[1:], - help="Apply an audio effect. Use !effects to see them all.") + help=f"Apply an audio effect. Use {COMMAND_PREFIX}effects to see " + "them all.") @same_voice_channel() async def apply_effect(self, ctx): name = ctx.invoked_with.lower() @@ -104,10 +106,11 @@ async def current_effect(self, ctx): @commands.command(name="effects") async def list_effects(self, ctx): """List all available audio effects.""" + prefix = ctx.clean_prefix await ctx.send(embed=discord.Embed( title="🎛️ Available Effects", - description=", ".join(f"`!{name}`" for name in EFFECTS) - + "\n\nUse `!reset` to remove all effects.", + description=", ".join(f"`{prefix}{name}`" for name in EFFECTS) + + f"\n\nUse `{prefix}reset` to remove all effects.", color=BLURPLE, )) diff --git a/cogs/lyrics.py b/cogs/lyrics.py index 1251b36..eb610e9 100644 --- a/cogs/lyrics.py +++ b/cogs/lyrics.py @@ -271,7 +271,8 @@ async def lyrics(self, ctx, *, query: str = None): found = await self._for_track(player.current) else: return await ctx.send(embed=error_embed( - "Nothing is playing. Provide a song name: `!lyrics `" + f"Nothing is playing. Provide a song name: " + f"`{ctx.clean_prefix}lyrics <title>`" )) if found is None: diff --git a/cogs/music.py b/cogs/music.py index d852fb1..1258e19 100644 --- a/cogs/music.py +++ b/cogs/music.py @@ -240,7 +240,7 @@ async def remove(self, ctx, index: int): @commands.command() @same_voice_channel() async def move(self, ctx, from_pos: int, to_pos: int): - """Move a track in the queue: !move <from> <to>""" + """Move a track to another position in the queue.""" player = players.get(ctx.guild.id) if player and player.move(from_pos, to_pos): await ctx.send(embed=success_embed(f"Moved track **{from_pos}** → **{to_pos}**.")) diff --git a/tests/test_commands.py b/tests/test_commands.py index c35940a..9095a28 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -271,3 +271,34 @@ async def test_voice_update_in_a_different_channel_is_ignored(music_cog): music_cog, member, MagicMock(channel=other_channel), MagicMock(channel=None) ) vc.disconnect.assert_not_awaited() + + +# -- the configured prefix, not a hardcoded "!" ------------------------- +# +# COMMAND_PREFIX is configurable, and a reply that names "!reset" to a server +# using "?" points at a command that does not exist there. + +async def test_effects_list_uses_the_prefix_it_was_invoked_with(effects_cog, ctx): + ctx.clean_prefix = "?" + await Effects.list_effects.callback(effects_cog, ctx) + text = sent_text(ctx) + assert "`?bass`" in text and "`?reset`" in text + assert "`!" not in text + + +async def test_lyrics_hint_uses_the_prefix_it_was_invoked_with(ctx): + from cogs.lyrics import Lyrics + + ctx.clean_prefix = "?" + await Lyrics.lyrics.callback(Lyrics(MagicMock()), ctx) + assert "`?lyrics <title>`" in sent_text(ctx) + + +def test_embeds_name_commands_with_the_configured_prefix(monkeypatch): + from utils import embeds + + monkeypatch.setattr(embeds, "COMMAND_PREFIX", "?") + track = {"title": "Song", "url": None, "error": "blocked"} + assert "?play sc: Song" in embeds.load_error_embed(track).description + footer = embeds.now_playing_embed(track, MagicMock()).footer.text + assert "?queue" in footer and "!" not in footer diff --git a/utils/embeds.py b/utils/embeds.py index 0a5e4fd..2b46a8b 100644 --- a/utils/embeds.py +++ b/utils/embeds.py @@ -3,6 +3,8 @@ import discord +from config import COMMAND_PREFIX + GREEN = 0x1DB954 BLURPLE = 0x5865F2 RED = 0xFF4444 @@ -34,7 +36,7 @@ def now_playing_embed(track: dict, requester: discord.abc.User, embed.add_field(name="📺 Channel", value=track["uploader"], inline=True) if track.get("thumbnail"): embed.set_thumbnail(url=track["thumbnail"]) - embed.set_footer(text="🎧 Use !queue to see upcoming tracks") + embed.set_footer(text=f"🎧 Use {COMMAND_PREFIX}queue to see upcoming tracks") return embed @@ -80,7 +82,7 @@ def load_error_embed(track: dict) -> discord.Embed: if track.get("error") == "blocked": return error_embed( f"YouTube is rate-limiting this server, so **{title}** can't be " - f"loaded right now. Try SoundCloud instead — e.g. `!play sc: {title}`." + f"loaded right now. Try SoundCloud instead — e.g. `{COMMAND_PREFIX}play sc: {title}`." ) return error_embed(f"Couldn't load **{title}** — skipping.") From 0aa4ff5e41f175a01dc5427b78f17576f7c02bb0 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:11:38 -0600 Subject: [PATCH 04/11] Type every function, and refuse commands sent in a DM The project asks for type hints on every function; 85 had none, and the track dict that every layer passes around was described only in two docstrings. It is now services.media.Track, a TypedDict, and mypy runs clean with disallow_untyped_defs. Getting there surfaced two real problems: - Nothing limited commands to servers. !play or !queue sent in a DM died on ctx.guild / ctx.author.voice and answered with the generic 'something went wrong' plus a logged traceback. A global guild_only check now refuses DMs, and the error handler tells the user to use a server. Because the check guarantees a guild, utils.context.GuildContext tells the type checker so, without a None test in every command body. - !play resolves a link's host (the SSRF guard) between @user_in_voice and _ensure_voice, so the author can leave voice in between. That now gets the voice-channel message instead of an AttributeError. Mechanical parts: 'return await ctx.send(...)' became a send followed by 'return' so commands can be typed -> None, and one cast each where discord.py's stubs are wider than what this bot does (VoiceProtocol vs VoiceClient, IO[bytes] vs BufferedIOBase), with the reason beside it. --- cogs/effects.py | 23 +++++---- cogs/lyrics.py | 65 +++++++++++++++--------- cogs/music.py | 104 ++++++++++++++++++++++++-------------- config.py | 2 +- main.py | 17 ++++--- services/lyrics_api.py | 6 +-- services/media.py | 81 +++++++++++++++++++---------- services/synced_lyrics.py | 8 +-- tests/test_commands.py | 9 ++++ tests/test_errors.py | 43 ++++++++++++++++ tests/test_lyrics_cog.py | 4 +- utils/announcer.py | 5 +- utils/checks.py | 13 +++-- utils/context.py | 40 +++++++++++++++ utils/embeds.py | 21 ++++---- utils/errors.py | 8 +-- utils/player.py | 47 +++++++++-------- utils/startup.py | 17 ++++--- 18 files changed, 351 insertions(+), 162 deletions(-) create mode 100644 utils/context.py diff --git a/cogs/effects.py b/cogs/effects.py index 03c0efc..e67c396 100644 --- a/cogs/effects.py +++ b/cogs/effects.py @@ -6,6 +6,7 @@ from config import COMMAND_PREFIX from utils.checks import same_voice_channel +from utils.context import GuildContext from utils.embeds import BLURPLE, error_embed, success_embed from utils.player import players @@ -57,24 +58,25 @@ class Effect: class Effects(commands.Cog, name="🎛️ Audio Effects"): - def __init__(self, bot: commands.Bot): + def __init__(self, bot: commands.Bot) -> None: self.bot = bot self._last_change: dict[int, float] = {} # guild_id → monotonic time - def _throttled(self, ctx) -> bool: + def _throttled(self, ctx: GuildContext) -> bool: now = time.monotonic() if now - self._last_change.get(ctx.guild.id, 0.0) < _EFFECT_COOLDOWN: return True self._last_change[ctx.guild.id] = now return False - async def _switch_to(self, ctx, name: str | None, filter_str: str, label: str, + async def _switch_to(self, ctx: GuildContext, name: str | None, filter_str: str, label: str, rate: float = 1.0) -> None: """Throttle, apply, and report — the whole path every effect command takes.""" if self._throttled(ctx): - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( f"Easy — wait {_EFFECT_COOLDOWN:.0f}s between effect changes." )) + return player = players.get(ctx.guild.id) if player and player.apply_effect(name, filter_str, rate): await ctx.send(embed=success_embed(label)) @@ -85,26 +87,27 @@ async def _switch_to(self, ctx, name: str | None, filter_str: str, label: str, help=f"Apply an audio effect. Use {COMMAND_PREFIX}effects to see " "them all.") @same_voice_channel() - async def apply_effect(self, ctx): - name = ctx.invoked_with.lower() + async def apply_effect(self, ctx: GuildContext) -> None: + # Always set here: this callback only runs when invoked by one of its names. + name = (ctx.invoked_with or _EFFECT_NAMES[0]).lower() effect = EFFECTS[name] await self._switch_to(ctx, name, effect.filter, effect.label, effect.rate) @commands.command(name="reset", aliases=["fxreset", "noeffect"]) @same_voice_channel() - async def reset_effect(self, ctx): + async def reset_effect(self, ctx: GuildContext) -> None: """Remove all audio effects.""" await self._switch_to(ctx, None, "", "Audio effects removed ✅") @commands.command(name="effect") - async def current_effect(self, ctx): + async def current_effect(self, ctx: GuildContext) -> None: """Show the active audio effect.""" player = players.get(ctx.guild.id) name = (player.effect_name if player else None) or "none" await ctx.send(embed=success_embed(f"Current effect: **{name}**")) @commands.command(name="effects") - async def list_effects(self, ctx): + async def list_effects(self, ctx: GuildContext) -> None: """List all available audio effects.""" prefix = ctx.clean_prefix await ctx.send(embed=discord.Embed( @@ -115,5 +118,5 @@ async def list_effects(self, ctx): )) -async def setup(bot): +async def setup(bot: commands.Bot) -> None: await bot.add_cog(Effects(bot)) diff --git a/cogs/lyrics.py b/cogs/lyrics.py index eb610e9..e04b917 100644 --- a/cogs/lyrics.py +++ b/cogs/lyrics.py @@ -1,21 +1,23 @@ import asyncio import logging import time -from typing import Awaitable, Callable, Optional +from typing import Awaitable, Callable, Optional, Sequence import aiohttp import discord from discord.ext import commands from services import lyrics_api, synced_lyrics +from services.media import Track # Deliberately not `from ... import Lyrics`: the cog class below is called # Lyrics too, and importing the bare name let it shadow the dataclass. The # Genius fallback then built a Cog instead of a result and raised TypeError, # which the command reported as nothing at all. -from services.synced_lyrics import index_at, search_terms +from services.synced_lyrics import Line, index_at, search_terms +from utils.context import GuildContext from utils.embeds import (error_embed, info_embed, lyrics_embed, lyrics_pages, synced_lyrics_embed) -from utils.player import players +from utils.player import MusicPlayer, players LyricsResult = synced_lyrics.Lyrics @@ -34,7 +36,7 @@ MAX_SLEEP = 2.0 MIN_SLEEP = 0.25 -Loader = Callable[[dict], Awaitable[Optional[LyricsResult]]] +Loader = Callable[[Track], Awaitable[Optional[LyricsResult]]] class LyricsFollower: @@ -46,8 +48,10 @@ class LyricsFollower: simply as a different position on the next wakeup. """ - def __init__(self, message, player, load: Loader, *, - sleep=asyncio.sleep, now=time.monotonic) -> None: + def __init__(self, message: discord.Message, player: MusicPlayer, + load: Loader, *, + sleep: Callable[[float], Awaitable[None]] = asyncio.sleep, + now: Callable[[], float] = time.monotonic) -> None: self.message = message self.player = player self._load = load @@ -60,7 +64,7 @@ def stop(self) -> None: async def run(self) -> None: """Follow the music until the song, the player or the message runs out.""" - track: Optional[dict] = None + track: Optional[Track] = None lyrics: Optional[LyricsResult] = None shown: Optional[int] = None last_edit = float("-inf") @@ -87,12 +91,12 @@ async def run(self) -> None: await self._sleep(self._until_next_line(lyrics.lines, index, position)) async def _show(self, lyrics: LyricsResult, index: int, position: float, - track: Optional[dict]) -> bool: + track: Optional[Track]) -> bool: """Redraw the message. False means it is gone and we should stop.""" try: await self.message.edit(embed=synced_lyrics_embed( lyrics.title, lyrics.artist, lyrics.lines, index, - position, (track or {}).get("duration"), + position, track.get("duration") if track else None, )) return True except (discord.NotFound, discord.Forbidden) as e: @@ -102,7 +106,8 @@ async def _show(self, lyrics: LyricsResult, index: int, position: float, log.warning("Could not update lyrics: %s", e) return True # a rate limit or a blip, not a reason to stop - def _until_next_line(self, lines, index: int, position: float) -> float: + def _until_next_line(self, lines: Sequence[Line], index: int, + position: float) -> float: """ How long until the next line, in real seconds. @@ -139,11 +144,13 @@ async def _turn(self, interaction: discord.Interaction, by: int) -> None: await interaction.response.edit_message(embed=self.embed(), view=self) @discord.ui.button(emoji="◀", style=discord.ButtonStyle.secondary) - async def previous(self, interaction: discord.Interaction, _button) -> None: + async def previous(self, interaction: discord.Interaction, + _button: "discord.ui.Button[LyricsPages]") -> None: await self._turn(interaction, -1) @discord.ui.button(emoji="▶", style=discord.ButtonStyle.secondary) - async def next(self, interaction: discord.Interaction, _button) -> None: + async def next(self, interaction: discord.Interaction, + _button: "discord.ui.Button[LyricsPages]") -> None: await self._turn(interaction, 1) @@ -157,13 +164,13 @@ def __init__(self, on_stop: Callable[[], None]) -> None: @discord.ui.button(label="Stop", emoji="⏹", style=discord.ButtonStyle.secondary) async def stop_following(self, interaction: discord.Interaction, - _button) -> None: + _button: "discord.ui.Button[FollowControls]") -> None: self._on_stop() await interaction.response.edit_message(view=None) class Lyrics(commands.Cog, name="\U0001f3a4 Lyrics"): - def __init__(self, bot): + def __init__(self, bot: commands.Bot) -> None: self.bot = bot self._session: Optional[aiohttp.ClientSession] = None self._following: dict[int, tuple[LyricsFollower, asyncio.Task]] = {} @@ -191,6 +198,8 @@ def _split_query(query: str) -> tuple[str, str]: async def _find(self, title: str, artist: str, duration: Optional[float]) -> Optional[LyricsResult]: """LRCLIB first, since it is the only source with timings, then Genius.""" + if self._session is None: + raise RuntimeError("the Lyrics cog was used before cog_load") found = await synced_lyrics.fetch(self._session, title, artist, duration) if found is not None: return found @@ -214,7 +223,7 @@ async def _search(self, query: str) -> Optional[LyricsResult]: found = await self._find(artist, title, None) return found - async def _for_track(self, track: dict) -> Optional[LyricsResult]: + async def _for_track(self, track: Track) -> Optional[LyricsResult]: """Look a playing track up, with its title tidied into search terms.""" title, artist = search_terms(track.get("title") or "", track.get("uploader") or "") @@ -232,14 +241,16 @@ def stop_following(self, guild_id: int) -> None: if not task.done(): task.cancel() - async def _follow(self, ctx, player, lyrics: LyricsResult) -> None: + async def _follow(self, ctx: GuildContext, player: MusicPlayer, + lyrics: LyricsResult) -> None: """Post the live message and start keeping it up to date.""" self.stop_following(ctx.guild.id) # one per guild; the newest wins index = index_at(lyrics.lines, player.position) message = await ctx.send( embed=synced_lyrics_embed( lyrics.title, lyrics.artist, lyrics.lines, index, - player.position, (player.current or {}).get("duration")), + player.position, + player.current.get("duration") if player.current else None), view=FollowControls(lambda: self.stop_following(ctx.guild.id)), ) follower = LyricsFollower(message, player, self._for_track) @@ -259,7 +270,8 @@ async def _run_follower(self, guild_id: int, follower: LyricsFollower) -> None: # -- The command --------------------------------------------------- @commands.command(aliases=["ly"]) - async def lyrics(self, ctx, *, query: str = None): + async def lyrics(self, ctx: GuildContext, *, + query: Optional[str] = None) -> None: """Follow the lyrics of the current song, or look a song up.""" async with ctx.typing(): player = players.get(ctx.guild.id) @@ -270,20 +282,23 @@ async def lyrics(self, ctx, *, query: str = None): title = player.current.get("title", "") found = await self._for_track(player.current) else: - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( f"Nothing is playing. Provide a song name: " f"`{ctx.clean_prefix}lyrics <title>`" )) + return if found is None: - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( f"Couldn't find lyrics for **{title}**." )) + return if found.instrumental: - return await ctx.send(embed=info_embed( + await ctx.send(embed=info_embed( "\U0001f3b5 Instrumental", f"**{found.title}** has no lyrics to show." )) + return # Only the playing track can be followed: a lyric needs a clock, and # a search result has none. @@ -292,9 +307,11 @@ async def lyrics(self, ctx, *, query: str = None): note = "" if query else "not synced - showing the full lyrics" view = LyricsPages(found.title, found.artist, found.plain, note) - await ctx.send(embed=view.embed(), - view=view if view.total > 1 else None) + if view.total > 1: + await ctx.send(embed=view.embed(), view=view) + else: + await ctx.send(embed=view.embed()) -async def setup(bot): +async def setup(bot: commands.Bot) -> None: await bot.add_cog(Lyrics(bot)) diff --git a/cogs/music.py b/cogs/music.py index 1258e19..bd60e47 100644 --- a/cogs/music.py +++ b/cogs/music.py @@ -1,11 +1,14 @@ import asyncio import logging +from typing import Optional, cast import discord from discord.ext import commands from services import media +from services.media import Track from utils.player import players, MusicPlayer, MAX_QUEUE +from utils.context import GuildContext from utils.embeds import (added_embed, error_embed, now_playing_embed, queue_embed, success_embed) from utils.checks import user_in_voice, same_voice_channel @@ -33,14 +36,20 @@ def _is_playlist_url(query: str) -> bool: class Music(commands.Cog, name="🎵 Music & Queue"): - def __init__(self, bot: commands.Bot): + def __init__(self, bot: commands.Bot) -> None: self.bot = bot # ── Helpers ─────────────────────────────────────────────────────── - async def _ensure_voice(self, ctx) -> bool: + async def _ensure_voice(self, ctx: GuildContext) -> bool: """Connect (or move) the bot to the author's voice channel.""" - dest = ctx.author.voice.channel + # Checked again although @user_in_voice already did: play awaits a + # DNS lookup in between, and the author can leave during it. + dest = ctx.author.voice.channel if ctx.author.voice else None + if dest is None: + await ctx.send(embed=error_embed( + "You must be in a voice channel to use this command.")) + return False perms = dest.permissions_for(ctx.me) if not perms.connect or not perms.speak: await ctx.send(embed=error_embed( @@ -61,7 +70,7 @@ async def _ensure_voice(self, ctx) -> bool: await ctx.send(embed=error_embed("Timed out connecting to voice.")) return False - def _player(self, ctx) -> MusicPlayer: + def _player(self, ctx: GuildContext) -> MusicPlayer: return players.get_or_create(self.bot, ctx.guild, ctx.channel) # ── Playback commands ───────────────────────────────────────────── @@ -70,17 +79,19 @@ def _player(self, ctx) -> MusicPlayer: @commands.cooldown(rate=3, per=5.0, type=commands.BucketType.user) @commands.max_concurrency(1, per=commands.BucketType.user, wait=False) @user_in_voice() - async def play(self, ctx, *, query: str): + async def play(self, ctx: GuildContext, *, query: str) -> None: """Play from YouTube, SoundCloud or a direct link. Accepts URLs or search terms. Tip: prefix a search with `sc:` to search SoundCloud, e.g. `!play sc: lofi`. """ query = query.strip() if len(query) > MAX_QUERY_LEN: - return await ctx.send(embed=error_embed("That query is too long.")) + await ctx.send(embed=error_embed("That query is too long.")) + return if query.startswith("http") and not await media.is_public_url(query): - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( "I can only play links to public websites.")) + return if not await self._ensure_voice(ctx): return player = self._player(ctx) @@ -89,32 +100,37 @@ async def play(self, ctx, *, query: str): if _is_playlist_url(query): tracks = await media.get_playlist(query, loop=self.bot.loop) if not tracks: - return await ctx.send(embed=error_embed("Couldn't load that playlist.")) + await ctx.send(embed=error_embed("Couldn't load that playlist.")) + return return await self._enqueue(ctx, player, tracks, "playlist") track = await media.search(query, loop=self.bot.loop) if not track: - return await ctx.send(embed=error_embed(f"No results found for `{query}`.")) + await ctx.send(embed=error_embed(f"No results found for `{query}`.")) + return await self._enqueue(ctx, player, [track], None) - async def _enqueue(self, ctx, player: MusicPlayer, tracks: list[dict], batch_label): + async def _enqueue(self, ctx: GuildContext, player: MusicPlayer, + tracks: list[Track], batch_label: Optional[str]) -> None: """Add one or many tracks and report to the channel.""" for t in tracks: t["requester"] = ctx.author # who queued it (for Now Playing) was_idle = player.current is None and player.is_empty if len(tracks) == 1: if not player.add(tracks[0]): - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( f"Queue is full (max {MAX_QUEUE} tracks)." )) + return if not was_idle: await ctx.send(embed=added_embed(tracks[0])) else: added = player.add_many(tracks) if added == 0: - return await ctx.send(embed=error_embed( + await ctx.send(embed=error_embed( f"Queue is full (max {MAX_QUEUE} tracks)." )) + return skipped = f" ({len(tracks) - added} skipped — queue full)" if added < len(tracks) else "" await ctx.send(embed=success_embed( f"Added **{added} tracks** from {batch_label} to the queue.{skipped}" @@ -122,7 +138,7 @@ async def _enqueue(self, ctx, player: MusicPlayer, tracks: list[dict], batch_lab @commands.command() @same_voice_channel() - async def pause(self, ctx): + async def pause(self, ctx: GuildContext) -> None: """Pause the current track.""" # Routed through the player so it can stop its playback clock; that # clock is what lets an effect change resume in the right place. @@ -134,7 +150,7 @@ async def pause(self, ctx): @commands.command() @same_voice_channel() - async def resume(self, ctx): + async def resume(self, ctx: GuildContext) -> None: """Resume a paused track.""" player = players.get(ctx.guild.id) if player and player.resume(): @@ -144,7 +160,7 @@ async def resume(self, ctx): @commands.command() @same_voice_channel() - async def skip(self, ctx): + async def skip(self, ctx: GuildContext) -> None: """Skip the current track.""" player = players.get(ctx.guild.id) if player and player.skip(): @@ -154,7 +170,7 @@ async def skip(self, ctx): @commands.command(aliases=["prev"]) @same_voice_channel() - async def previous(self, ctx): + async def previous(self, ctx: GuildContext) -> None: """Go back to the previous track.""" player = players.get(ctx.guild.id) if player and player.go_previous(): @@ -164,7 +180,7 @@ async def previous(self, ctx): @commands.command(aliases=["dc", "leave"]) @same_voice_channel() - async def stop(self, ctx): + async def stop(self, ctx: GuildContext) -> None: """Stop music and disconnect the bot.""" player = players.get(ctx.guild.id) if player: @@ -176,70 +192,78 @@ async def stop(self, ctx): # ── Queue commands ──────────────────────────────────────────────── @commands.command(aliases=["q"]) - async def queue(self, ctx, page: int = 1): + async def queue(self, ctx: GuildContext, page: int = 1) -> None: """Show the current queue.""" player = players.get(ctx.guild.id) if not player: - return await ctx.send(embed=error_embed("Nothing is playing.")) + await ctx.send(embed=error_embed("Nothing is playing.")) + return await ctx.send(embed=queue_embed(player.to_list(), player.current, page=page)) @commands.command(aliases=["np", "current"]) - async def nowplaying(self, ctx): + async def nowplaying(self, ctx: GuildContext) -> None: """Show the currently playing track.""" player = players.get(ctx.guild.id) if not player or not player.current: - return await ctx.send(embed=error_embed("Nothing is playing right now.")) + await ctx.send(embed=error_embed("Nothing is playing right now.")) + return await ctx.send(embed=now_playing_embed(player.current, ctx.author, loop_mode=player.loop_mode)) @commands.command() @same_voice_channel() - async def volume(self, ctx, vol: int): + async def volume(self, ctx: GuildContext, vol: int) -> None: """Set volume (0–100).""" if not 0 <= vol <= 100: - return await ctx.send(embed=error_embed("Volume must be between 0 and 100.")) + await ctx.send(embed=error_embed("Volume must be between 0 and 100.")) + return player = players.get(ctx.guild.id) if not player: - return await ctx.send(embed=error_embed("Nothing is playing.")) + await ctx.send(embed=error_embed("Nothing is playing.")) + return player.set_volume(vol / 100) await ctx.send(embed=success_embed(f"Volume set to **{vol}%** 🔊")) @commands.command() @same_voice_channel() - async def loop(self, ctx, mode: str = "track"): + async def loop(self, ctx: GuildContext, mode: str = "track") -> None: """Set loop mode: track | queue | off""" mode = mode.lower() if mode not in ("track", "queue", "off"): - return await ctx.send(embed=error_embed("Loop mode must be `track`, `queue`, or `off`.")) + await ctx.send(embed=error_embed("Loop mode must be `track`, `queue`, or `off`.")) + return player = players.get(ctx.guild.id) if not player: - return await ctx.send(embed=error_embed("Nothing is playing.")) + await ctx.send(embed=error_embed("Nothing is playing.")) + return player.loop_mode = mode icons = {"track": "🔂", "queue": "🔁", "off": "➡️"} await ctx.send(embed=success_embed(f"Loop mode set to **{mode}** {icons[mode]}")) @commands.command() @same_voice_channel() - async def shuffle(self, ctx): + async def shuffle(self, ctx: GuildContext) -> None: """Shuffle the queue.""" player = players.get(ctx.guild.id) if not player or player.is_empty: - return await ctx.send(embed=error_embed("Queue is empty.")) + await ctx.send(embed=error_embed("Queue is empty.")) + return player.shuffle() await ctx.send(embed=success_embed("Queue shuffled 🔀")) @commands.command() @same_voice_channel() - async def remove(self, ctx, index: int): + async def remove(self, ctx: GuildContext, index: int) -> None: """Remove a track from the queue by its position.""" player = players.get(ctx.guild.id) track = player.remove(index) if player else None if not track: - return await ctx.send(embed=error_embed(f"No track at position {index}.")) + await ctx.send(embed=error_embed(f"No track at position {index}.")) + return await ctx.send(embed=success_embed(f"Removed **{track['title']}** from the queue.")) @commands.command() @same_voice_channel() - async def move(self, ctx, from_pos: int, to_pos: int): + async def move(self, ctx: GuildContext, from_pos: int, to_pos: int) -> None: """Move a track to another position in the queue.""" player = players.get(ctx.guild.id) if player and player.move(from_pos, to_pos): @@ -249,7 +273,7 @@ async def move(self, ctx, from_pos: int, to_pos: int): @commands.command() @same_voice_channel() - async def clear(self, ctx): + async def clear(self, ctx: GuildContext) -> None: """Clear the queue (keeps the current track playing).""" player = players.get(ctx.guild.id) if player: @@ -258,11 +282,12 @@ async def clear(self, ctx): @commands.command() @same_voice_channel() - async def autoplay(self, ctx): + async def autoplay(self, ctx: GuildContext) -> None: """Toggle autoplay (auto-queue related tracks when the queue ends).""" player = players.get(ctx.guild.id) if not player: - return await ctx.send(embed=error_embed("Nothing is playing.")) + await ctx.send(embed=error_embed("Nothing is playing.")) + return player.autoplay = not player.autoplay state = "enabled 🟢" if player.autoplay else "disabled 🔴" await ctx.send(embed=success_embed(f"Autoplay {state}")) @@ -273,11 +298,14 @@ async def autoplay(self, ctx): # centrally in utils.errors, so every command reports them the same way. @commands.Cog.listener() - async def on_voice_state_update(self, member, before, after): + async def on_voice_state_update(self, member: discord.Member, + before: discord.VoiceState, + after: discord.VoiceState) -> None: """Disconnect shortly after the bot is left alone in a channel.""" if member.bot: return - vc = member.guild.voice_client + # Typed as the VoiceProtocol base; this bot only connects with VoiceClient. + vc = cast(Optional[discord.VoiceClient], member.guild.voice_client) if not vc: return if before.channel != vc.channel: @@ -292,5 +320,5 @@ async def on_voice_state_update(self, member, before, after): await vc.disconnect(force=True) -async def setup(bot): +async def setup(bot: commands.Bot) -> None: await bot.add_cog(Music(bot)) diff --git a/config.py b/config.py index aee26f6..3c13094 100644 --- a/config.py +++ b/config.py @@ -65,7 +65,7 @@ def _ffmpeg_version() -> str: return parts[2] if len(parts) > 2 and parts[1] == "version" else "unknown" -def runtime_versions() -> dict: +def runtime_versions() -> dict[str, str]: """ What this instance is actually running. diff --git a/main.py b/main.py index 7f6c04b..22b22c9 100644 --- a/main.py +++ b/main.py @@ -1,5 +1,6 @@ import asyncio import logging +from typing import cast import discord from discord.ext import commands @@ -7,6 +8,7 @@ import config from config import COGS, COMMAND_PREFIX, DISCORD_TOKEN from utils import errors +from utils.context import guild_only from utils.help import build as build_help from utils.startup import serve @@ -26,12 +28,13 @@ help_command=None, # replaced by the generated one below case_insensitive=True, ) +bot.add_check(guild_only) @bot.event -async def on_ready(): +async def on_ready() -> None: log.info("Logged in as %s (ID: %s) — serving %d guild(s)", - bot.user, bot.user.id, len(bot.guilds)) + bot.user, bot.user.id if bot.user else "?", len(bot.guilds)) await bot.change_presence( activity=discord.Activity( type=discord.ActivityType.listening, @@ -41,18 +44,19 @@ async def on_ready(): @bot.event -async def on_command_error(ctx, error): +async def on_command_error(ctx: commands.Context[commands.Bot], + error: commands.CommandError) -> None: # All of it lives in utils.errors so it can be tested without a gateway. await errors.handle(ctx, error) @bot.command(name="help") -async def help_command(ctx): +async def help_command(ctx: commands.Context[commands.Bot]) -> None: """Show this message.""" await ctx.send(embed=build_help(bot, COMMAND_PREFIX)) -async def main(): +async def main() -> None: async with bot: for cog in COGS: try: @@ -62,7 +66,8 @@ async def main(): log.exception("Failed to load cog: %s", cog) # Not bot.start(): the login needs retrying, and voice has to be # released within a bound before close() waits it out. See utils.startup. - await serve(bot, DISCORD_TOKEN) + # config.validate() has already exited if the token is missing. + await serve(bot, cast(str, DISCORD_TOKEN)) if __name__ == "__main__": diff --git a/services/lyrics_api.py b/services/lyrics_api.py index 252511d..2d0dc94 100644 --- a/services/lyrics_api.py +++ b/services/lyrics_api.py @@ -7,7 +7,7 @@ import asyncio import logging from functools import lru_cache -from typing import Optional +from typing import Any, Optional import lyricsgenius @@ -41,7 +41,7 @@ def _client() -> lyricsgenius.Genius: ) -async def fetch(title: str, artist: str = "") -> Optional[dict]: +async def fetch(title: str, artist: str = "") -> Optional[dict[str, str]]: """ Search Genius for lyrics. @@ -60,7 +60,7 @@ async def fetch(title: str, artist: str = "") -> Optional[dict]: # the gateway is up raises, and this never needs a different loop anyway. loop = asyncio.get_running_loop() - def _search(): + def _search() -> Any: client = _client() # An empty artist matches worse than no artist at all. return client.search_song(title, artist) if artist else client.search_song(title) diff --git a/services/media.py b/services/media.py index 945e708..97a70f2 100644 --- a/services/media.py +++ b/services/media.py @@ -12,11 +12,10 @@ signature solving and throttling — see the note above those functions. - Non-blocking: every yt-dlp metadata call runs in a thread executor. -Track dict shape:: - - {title, url, duration, thumbnail, uploader, source, query} +What every lookup returns, and what the queue holds, is a :class:`Track`. """ +import io import os import sys import time @@ -27,7 +26,7 @@ import tempfile import asyncio import logging -from typing import Optional +from typing import IO, Any, NotRequired, Optional, TypedDict, cast from urllib.parse import urlsplit import discord @@ -37,6 +36,21 @@ log = logging.getLogger("loopify.media") + +class Track(TypedDict): + """One playable item, as the queue, the player and the embeds pass it.""" + + title: str + url: Optional[str] # None: streamed by searching `query` or `title` + duration: Optional[float] # seconds; None for a live stream + thumbnail: Optional[str] + uploader: Optional[str] + source: str # the yt-dlp extractor, lowercased + query: str # what was searched for; "" for a playlist entry + requester: NotRequired[discord.abc.User] # set when a command queues it + error: NotRequired[str] # set when it failed to load; see classify_error + + # YouTube player clients, tried in order. ONE definition — both the metadata # options below and the streaming subprocess derive from this, because two # separate lists silently drift and then playback and search disagree about @@ -60,7 +74,7 @@ _PLAYER_CLIENTS = ("web_embedded", "mweb", "tv_embedded") # Base yt-dlp config shared by every call. -YTDL_OPTIONS = { +YTDL_OPTIONS: dict[str, Any] = { "format": "bestaudio/best", "noplaylist": True, "quiet": True, @@ -87,7 +101,7 @@ } -def _build_track(info: dict, *, query: str = "") -> dict: +def _build_track(info: dict[str, Any], *, query: str = "") -> Track: """Convert a yt-dlp info dict into our internal track dict.""" return { "title": info.get("title") or "Unknown Title", @@ -100,14 +114,15 @@ def _build_track(info: dict, *, query: str = "") -> dict: } -def _first_thumb(info: dict) -> Optional[str]: +def _first_thumb(info: dict[str, Any]) -> Optional[str]: thumbs = info.get("thumbnails") or [] return thumbs[-1]["url"] if thumbs else None -def _run(opts: dict, target: str, *, loop): +def _run(opts: dict[str, Any], target: str, *, + loop: asyncio.AbstractEventLoop) -> asyncio.Future[Any]: """Run yt-dlp's blocking extract_info in a thread executor.""" - def _extract(): + def _extract() -> Any: with yt_dlp.YoutubeDL(opts) as ydl: return ydl.extract_info(target, download=False) return loop.run_in_executor(None, _extract) @@ -131,7 +146,8 @@ def _search_target(query: str) -> tuple[str, bool]: # ── Public API ──────────────────────────────────────────────────────── -async def search(query: str, *, loop=None) -> Optional[dict]: +async def search(query: str, *, + loop: Optional[asyncio.AbstractEventLoop] = None) -> Optional[Track]: """Resolve a single track from a search term or any supported URL.""" loop = loop or asyncio.get_event_loop() target, flat_ok = _search_target(query) @@ -145,7 +161,8 @@ async def search(query: str, *, loop=None) -> Optional[dict]: return None -async def search_many(query: str, limit: int = 5, *, loop=None) -> list[dict]: +async def search_many(query: str, limit: int = 5, *, + loop: Optional[asyncio.AbstractEventLoop] = None) -> list[Track]: """Return up to ``limit`` YouTube search results (metadata only).""" loop = loop or asyncio.get_event_loop() opts = {**YTDL_OPTIONS, "extract_flat": True} @@ -158,7 +175,8 @@ async def search_many(query: str, limit: int = 5, *, loop=None) -> list[dict]: return [] -async def get_playlist(url: str, *, loop=None) -> list[dict]: +async def get_playlist(url: str, *, + loop: Optional[asyncio.AbstractEventLoop] = None) -> list[Track]: """Extract every track from a playlist/set/album URL (metadata only).""" loop = loop or asyncio.get_event_loop() opts = {**YTDL_OPTIONS, "noplaylist": False, "extract_flat": True} @@ -171,7 +189,8 @@ async def get_playlist(url: str, *, loop=None) -> list[dict]: return [] -async def related(track: dict, *, loop=None) -> Optional[dict]: +async def related(track: Track, *, + loop: Optional[asyncio.AbstractEventLoop] = None) -> Optional[Track]: """Approximate a 'related' track for autoplay via a themed search.""" seed = track.get("uploader") or track.get("title") or "" if not seed: @@ -202,12 +221,12 @@ async def is_public_url(url: str) -> bool: infos = await asyncio.get_running_loop().getaddrinfo(host, None) except OSError: return False - addresses = {info[4][0].split("%", 1)[0] for info in infos} # drop IPv6 scope + addresses = {str(info[4][0]).split("%", 1)[0] for info in infos} # drop IPv6 scope return bool(addresses) and all( ipaddress.ip_address(address).is_global for address in addresses) -def _first_entry(info): +def _first_entry(info: Optional[dict[str, Any]]) -> Optional[dict[str, Any]]: """Unwrap the first playable entry from a search/playlist result.""" if info and "entries" in info: entries = [e for e in info["entries"] if e] @@ -225,10 +244,11 @@ def _first_entry(info): # # The pipe applies natural backpressure, so memory stays bounded on small hosts. -def _stream_target(track: dict) -> str: +def _stream_target(track: Track) -> str: """The yt-dlp target for streaming a track: its URL, or a search query.""" - if track.get("url"): - return track["url"] + url = track.get("url") + if url: + return url query = track.get("query") or track.get("title", "") target, _ = _search_target(query) return target @@ -246,7 +266,7 @@ def _stream_target(track: dict) -> str: _REAP_TIMEOUT = 5.0 -def _close_quietly(handle) -> None: +def _close_quietly(handle: Optional[IO[bytes]]) -> None: """Close a pipe/file, ignoring anything that goes wrong during teardown.""" if handle is None: return @@ -281,7 +301,7 @@ class AudioStream: __slots__ = ("_proc", "_errfile", "_error", "_closed") - def __init__(self, proc: subprocess.Popen, errfile) -> None: + def __init__(self, proc: subprocess.Popen[bytes], errfile: IO[bytes]) -> None: self._proc = proc self._errfile = errfile self._error: Optional[str] = None @@ -304,7 +324,7 @@ def launch(cls, cmd: list[str]) -> "AudioStream": return cls(proc, errfile) @property - def stdout(self): + def stdout(self) -> Optional[IO[bytes]]: """The audio pipe, or ``None`` once the stream has been closed.""" return None if self._closed else self._proc.stdout @@ -353,7 +373,7 @@ def _read_error(self) -> str: return "" -def spawn_stream(track: dict) -> AudioStream: +def spawn_stream(track: Track) -> AudioStream: """Start streaming a track's best audio through yt-dlp.""" cmd = [ sys.executable, "-m", "yt_dlp", @@ -406,10 +426,11 @@ class BufferedAudioSource(discord.AudioSource): exists to provide. """ - def __init__(self, source, *, seconds: float = READ_AHEAD_SECONDS) -> None: + def __init__(self, source: discord.AudioSource, *, + seconds: float = READ_AHEAD_SECONDS) -> None: self._source = source self.capacity_frames = max(1, int(seconds / FRAME_SECONDS)) - self._queue: "queue.Queue" = queue.Queue(maxsize=self.capacity_frames) + self._queue: queue.Queue[bytes] = queue.Queue(maxsize=self.capacity_frames) self._stop = threading.Event() self._thread = threading.Thread( target=self._fill, name="loopify-readahead", daemon=True, @@ -501,8 +522,9 @@ def cleanup(self) -> None: pass -def make_pipe_source(stdin, *, volume: float = 0.5, ffmpeg_filter: str = "", - seek_seconds: float = 0.0): +def make_pipe_source(stdin: IO[bytes], *, volume: float = 0.5, + ffmpeg_filter: str = "", seek_seconds: float = 0.0, + ) -> discord.PCMVolumeTransformer[BufferedAudioSource]: """ Build a ``discord.PCMVolumeTransformer`` that reads audio from a pipe. @@ -515,7 +537,10 @@ def make_pipe_source(stdin, *, volume: float = 0.5, ffmpeg_filter: str = "", options = f"-vn -af {ffmpeg_filter}" if ffmpeg_filter else "-vn" before = f"-ss {seek_seconds:.3f}" if seek_seconds > 0 else None source = discord.FFmpegPCMAudio( - stdin, pipe=True, before_options=before, options=options, + # Typed as IO[bytes] by typeshed, but Popen with a buffer size hands + # back a BufferedReader, which is what discord.py asks for. + cast(io.BufferedIOBase, stdin), + pipe=True, before_options=before, options=options, ) # Order matters. The read-ahead goes around FFmpeg, which is what stalls, # and the volume transformer stays outermost so `MusicPlayer.set_volume` @@ -526,7 +551,7 @@ def make_pipe_source(stdin, *, volume: float = 0.5, ffmpeg_filter: str = "", ) -def prime_source(source) -> bool: +def prime_source(source: discord.AudioSource) -> bool: """ Wait for a source from :func:`make_pipe_source` to have audio ready. diff --git a/services/synced_lyrics.py b/services/synced_lyrics.py index c42d79b..36f3f3b 100644 --- a/services/synced_lyrics.py +++ b/services/synced_lyrics.py @@ -12,7 +12,7 @@ import re from bisect import bisect_right from dataclasses import dataclass -from typing import Optional +from typing import Any, Optional import aiohttp @@ -129,7 +129,7 @@ def synced(self) -> bool: return bool(self.lines) -async def fetch(session, title: str, artist: str, +async def fetch(session: aiohttp.ClientSession, title: str, artist: str, duration: Optional[float]) -> Optional[Lyrics]: """ Look a track up on LRCLIB. Returns None when there is nothing to show. @@ -138,7 +138,7 @@ async def fetch(session, title: str, artist: str, here raises: LRCLIB being unreachable, slow or wrong is not a reason to interrupt playback, so every failure becomes None and a log line. """ - params = {"track_name": title, "artist_name": artist} + params: dict[str, str | int] = {"track_name": title, "artist_name": artist} if duration: params["duration"] = int(duration) @@ -159,7 +159,7 @@ async def fetch(session, title: str, artist: str, return _build(payload, title, artist) -def _build(payload: dict, title: str, artist: str) -> Optional[Lyrics]: +def _build(payload: dict[str, Any], title: str, artist: str) -> Optional[Lyrics]: """Turn a payload into Lyrics, or None when it holds nothing worth showing.""" lines = parse_lrc(payload.get("syncedLyrics") or "") plain = (payload.get("plainLyrics") or "").strip() diff --git a/tests/test_commands.py b/tests/test_commands.py index 9095a28..2c5d259 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -302,3 +302,12 @@ def test_embeds_name_commands_with_the_configured_prefix(monkeypatch): assert "?play sc: Song" in embeds.load_error_embed(track).description footer = embeds.now_playing_embed(track, MagicMock()).footer.text assert "?queue" in footer and "!" not in footer + + +async def test_play_copes_with_the_author_leaving_voice_mid_command(music_cog, ctx): + """@user_in_voice passed, then the author left while the link resolved.""" + ctx.author.voice = None + with patch("cogs.music.media.search", new=AsyncMock()) as search: + await Music.play.callback(music_cog, ctx, query="bohemian rhapsody") + assert "voice channel" in sent_text(ctx) + search.assert_not_awaited() diff --git a/tests/test_errors.py b/tests/test_errors.py index e56c627..6064016 100644 --- a/tests/test_errors.py +++ b/tests/test_errors.py @@ -217,3 +217,46 @@ def test_no_music_command_keeps_a_private_error_handler(): c.qualified_name for c in Music.__cog_commands__ if c.has_error_handler() ] assert with_handlers == [] + + +# -- commands sent in a DM --------------------------------------------- +# +# Every command acts on a guild. Before the global check, `!play` in a DM died +# on `ctx.author.voice` and got the generic "something went wrong". + +async def test_a_dm_is_refused_by_the_global_check(): + from utils.context import guild_only + + dm = MagicMock(guild=None) + with pytest.raises(commands.NoPrivateMessage): + await guild_only(dm) + + +async def test_a_guild_message_passes_the_global_check(): + from utils.context import guild_only + + assert await guild_only(MagicMock(guild=MagicMock())) is True + + +async def test_a_dm_is_told_to_use_a_server(ctx): + await errors.handle(ctx, commands.NoPrivateMessage()) + assert "server" in sent_text(ctx) + + +def test_the_global_check_is_registered_on_the_bot(monkeypatch): + """Without registration the check is dead code, and DMs reach the commands.""" + import importlib + import sys + + import config + from utils.context import guild_only + + # Importing main runs its startup; keep it offline and credential-free. + monkeypatch.setattr(config, "DISCORD_TOKEN", "not-a-real-token") + monkeypatch.setattr(config, "log_runtime", lambda: None) + sys.modules.pop("main", None) + main = importlib.import_module("main") + try: + assert guild_only in main.bot._checks + finally: + sys.modules.pop("main", None) diff --git a/tests/test_lyrics_cog.py b/tests/test_lyrics_cog.py index 0607b17..11251c5 100644 --- a/tests/test_lyrics_cog.py +++ b/tests/test_lyrics_cog.py @@ -24,7 +24,9 @@ @pytest.fixture def cog(): - return LyricsCog(MagicMock()) + cog = LyricsCog(MagicMock()) + cog._session = MagicMock() # what cog_load opens; every lookup is mocked + return cog @pytest.fixture diff --git a/utils/announcer.py b/utils/announcer.py index c3a6a72..bafa92f 100644 --- a/utils/announcer.py +++ b/utils/announcer.py @@ -11,6 +11,7 @@ import discord +from services.media import Track from utils.embeds import info_embed, load_error_embed, now_playing_embed log = logging.getLogger("loopify.announcer") @@ -22,11 +23,11 @@ class ChannelAnnouncer: def __init__(self, channel: discord.abc.Messageable) -> None: self.channel = channel - async def now_playing(self, track: dict, requester: discord.abc.User, + async def now_playing(self, track: Track, requester: discord.abc.User, loop_mode: str) -> None: await self._send(now_playing_embed(track, requester, loop_mode=loop_mode)) - async def load_failed(self, track: dict) -> None: + async def load_failed(self, track: Track) -> None: await self._send(load_error_embed(track)) async def idle_disconnect(self, after_seconds: float) -> None: diff --git a/utils/checks.py b/utils/checks.py index 09030d9..4d2891f 100644 --- a/utils/checks.py +++ b/utils/checks.py @@ -1,11 +1,16 @@ +from typing import Any, Callable, TypeVar + from discord.ext import commands +from utils.context import GuildContext from utils.embeds import error_embed +T = TypeVar("T") + -def user_in_voice(): +def user_in_voice() -> Callable[[T], T]: """Check: user must be in a voice channel.""" - async def predicate(ctx): + async def predicate(ctx: GuildContext) -> bool: if not ctx.author.voice or not ctx.author.voice.channel: await ctx.send(embed=error_embed( "You must be in a voice channel to use this command.")) @@ -14,9 +19,9 @@ async def predicate(ctx): return commands.check(predicate) -def same_voice_channel(): +def same_voice_channel() -> Callable[[T], T]: """Check: user must be in the same voice channel as the bot.""" - async def predicate(ctx): + async def predicate(ctx: GuildContext) -> bool: if not ctx.author.voice: await ctx.send(embed=error_embed("You must be in a voice channel.")) return False diff --git a/utils/context.py b/utils/context.py new file mode 100644 index 0000000..30eac24 --- /dev/null +++ b/utils/context.py @@ -0,0 +1,40 @@ +""" +Commands only ever run inside a server. + +Every command acts on a guild's voice connection or queue, and a DM has +neither: before :func:`guild_only` was registered, ``!play`` sent in a DM died +on ``ctx.author.voice`` and answered with a generic error. + +Because the check guarantees a guild, :class:`GuildContext` can tell the type +checker so. It exists only for type checking — at runtime it *is* +``commands.Context`` — which is what lets command bodies use ``ctx.guild.id`` +without a ``None`` test the check has already made impossible. +""" + +from typing import TYPE_CHECKING, Any, Optional + +import discord +from discord.ext import commands + +if TYPE_CHECKING: + class GuildContext(commands.Context[commands.Bot]): + @property + def guild(self) -> discord.Guild: ... + + @property + def author(self) -> discord.Member: ... + + @property + def me(self) -> discord.Member: ... + + @property + def voice_client(self) -> Optional[discord.VoiceClient]: ... +else: + GuildContext = commands.Context + + +async def guild_only(ctx: commands.Context[Any]) -> bool: + """Global check: refuse anything sent outside a server.""" + if ctx.guild is None: + raise commands.NoPrivateMessage() + return True diff --git a/utils/embeds.py b/utils/embeds.py index 2b46a8b..3e0f220 100644 --- a/utils/embeds.py +++ b/utils/embeds.py @@ -1,9 +1,11 @@ from datetime import timedelta -from typing import Optional +from typing import Optional, Sequence import discord from config import COMMAND_PREFIX +from services.media import Track +from services.synced_lyrics import Line GREEN = 0x1DB954 BLURPLE = 0x5865F2 @@ -11,18 +13,18 @@ GOLD = 0xFFD700 -def format_duration(seconds: Optional[int]) -> str: +def format_duration(seconds: Optional[float]) -> str: if not seconds: return "🔴 LIVE" return str(timedelta(seconds=seconds)) -def _linked_title(track: dict) -> str: +def _linked_title(track: Track) -> str: url = track.get("url") return f"[{track['title']}]({url})" if url else track["title"] -def now_playing_embed(track: dict, requester: discord.abc.User, +def now_playing_embed(track: Track, requester: discord.abc.User, loop_mode: str = "off") -> discord.Embed: embed = discord.Embed( title="🎵 Now Playing", @@ -40,7 +42,7 @@ def now_playing_embed(track: dict, requester: discord.abc.User, return embed -def added_embed(track: dict) -> discord.Embed: +def added_embed(track: Track) -> discord.Embed: embed = discord.Embed( description=f"➕ Added to queue: **{_linked_title(track)}**", color=BLURPLE) if track.get("thumbnail"): @@ -48,7 +50,7 @@ def added_embed(track: dict) -> discord.Embed: return embed -def queue_embed(queue: list, current: Optional[dict], page: int = 1, +def queue_embed(queue: list[Track], current: Optional[Track], page: int = 1, per_page: int = 10) -> discord.Embed: embed = discord.Embed(title="📋 Music Queue", color=BLURPLE) @@ -76,7 +78,7 @@ def queue_embed(queue: list, current: Optional[dict], page: int = 1, return embed -def load_error_embed(track: dict) -> discord.Embed: +def load_error_embed(track: Track) -> discord.Embed: """Explain why a track produced no audio, per ``AudioStream.classify_error``.""" title = track.get("title", "track") if track.get("error") == "blocked": @@ -109,7 +111,8 @@ def clock(seconds: float) -> str: return f"{seconds // 60}:{seconds % 60:02d}" -def lyrics_window(lines, index: int, context: int = CONTEXT_LINES) -> str: +def lyrics_window(lines: Sequence[Line], index: int, + context: int = CONTEXT_LINES) -> str: """ The line playing now, with a little of what came before and what is next. @@ -154,7 +157,7 @@ def lyrics_pages(text: str, limit: int = EMBED_LIMIT) -> list[str]: return pages -def synced_lyrics_embed(title: str, artist: str, lines, index: int, +def synced_lyrics_embed(title: str, artist: str, lines: Sequence[Line], index: int, position: float, duration: Optional[float]) -> discord.Embed: """The live view: a window on the lyrics plus where the song is.""" embed = discord.Embed( diff --git a/utils/errors.py b/utils/errors.py index 361d61a..b0f2171 100644 --- a/utils/errors.py +++ b/utils/errors.py @@ -21,7 +21,7 @@ log = logging.getLogger("loopify.errors") -def usage(ctx) -> str: +def usage(ctx: commands.Context) -> str: """ How the command should have been invoked, e.g. ``!move <from_pos> <to_pos>``. @@ -46,13 +46,15 @@ def _input_detail(error: commands.UserInputError) -> str: return "I couldn't make sense of that." -async def handle(ctx, error: Exception) -> None: +async def handle(ctx: commands.Context, error: Exception) -> None: """Reply to the user, or log, depending on what went wrong.""" # discord.py wraps exceptions raised inside a command body. error = getattr(error, "original", error) if isinstance(error, commands.CommandNotFound): return + if isinstance(error, commands.NoPrivateMessage): + return await _reply(ctx, "I only take commands in a server, not in DMs.") if isinstance(error, commands.CheckFailure): return # the check already sent its own message if ctx.command is not None and ctx.command.has_error_handler(): @@ -77,7 +79,7 @@ async def handle(ctx, error: Exception) -> None: await _reply(ctx, "Something went wrong on my side. It has been logged.") -async def _reply(ctx, message: str) -> None: +async def _reply(ctx: commands.Context, message: str) -> None: try: await ctx.send(embed=error_embed(message)) except discord.HTTPException as e: diff --git a/utils/player.py b/utils/player.py index 30921cf..fe0a842 100644 --- a/utils/player.py +++ b/utils/player.py @@ -9,9 +9,7 @@ ``stop()`` could fire a callback that advanced the queue at the same time a new source was being played. -Track dict shape (see ``services.media._build_track``):: - - {title, url, duration, thumbnail, uploader, source, query} +Tracks are :class:`services.media.Track`. """ import time @@ -19,11 +17,12 @@ import asyncio import logging from collections import deque -from typing import Callable, Optional +from typing import Callable, Optional, cast import discord from services import media +from services.media import Track from utils.announcer import ChannelAnnouncer log = logging.getLogger("loopify.player") @@ -45,7 +44,7 @@ class MusicPlayer: def __init__(self, bot: discord.Client, guild: discord.Guild, text_channel: discord.abc.Messageable, *, - on_destroy: Optional[Callable[[int], None]] = None): + on_destroy: Optional[Callable[[int], None]] = None) -> None: self.bot = bot self.guild = guild self.announcer = ChannelAnnouncer(text_channel) @@ -54,9 +53,9 @@ def __init__(self, bot: discord.Client, guild: discord.Guild, # and a test can build one without touching process-wide state. self._on_destroy = on_destroy - self.queue: deque[dict] = deque() - self.history: list[dict] = [] - self.current: Optional[dict] = None + self.queue: deque[Track] = deque() + self.history: list[Track] = [] + self.current: Optional[Track] = None self.loop_mode: str = "off" # off | track | queue self.autoplay: bool = False @@ -74,8 +73,8 @@ def __init__(self, bot: discord.Client, guild: discord.Guild, self._seek_base: float = 0.0 # offset the live stream started at self._stream: Optional[media.AudioStream] = None # active yt-dlp stream # Next track's stream, fetched while the current one plays. - self._prefetch: Optional[tuple[dict, media.AudioStream]] = None - self._prefetch_task: Optional[asyncio.Task] = None + self._prefetch: Optional[tuple[Track, media.AudioStream]] = None + self._prefetch_task: Optional[asyncio.Task[None]] = None # Signalling between commands and the playback loop. self._next = asyncio.Event() # set when the current source finishes @@ -88,7 +87,7 @@ def __init__(self, bot: discord.Client, guild: discord.Guild, # ── Queue mutation (called by commands) ─────────────────────────── - def add(self, track: dict) -> bool: + def add(self, track: Track) -> bool: """Append a track. Returns False if the queue is at its hard cap.""" if len(self.queue) >= MAX_QUEUE: return False @@ -96,7 +95,7 @@ def add(self, track: dict) -> bool: self._added.set() return True - def add_many(self, tracks: list[dict]) -> int: + def add_many(self, tracks: list[Track]) -> int: """Append up to the queue cap. Returns how many were actually added.""" room = MAX_QUEUE - len(self.queue) accepted = tracks[:max(0, room)] @@ -105,7 +104,7 @@ def add_many(self, tracks: list[dict]) -> int: self._added.set() return len(accepted) - def remove(self, index: int) -> Optional[dict]: + def remove(self, index: int) -> Optional[Track]: """Remove a 1-based queue position. Returns the removed track or None.""" if not (1 <= index <= len(self.queue)): return None @@ -134,12 +133,14 @@ def clear(self) -> None: def is_empty(self) -> bool: return not self.queue - def to_list(self) -> list[dict]: + def to_list(self) -> list[Track]: return list(self.queue) @property def voice(self) -> Optional[discord.VoiceClient]: - return self.guild.voice_client + # Typed as the VoiceProtocol base; this bot only ever connects with + # discord.py's own VoiceClient. + return cast(Optional[discord.VoiceClient], self.guild.voice_client) @property def text_channel(self) -> discord.abc.Messageable: @@ -230,7 +231,7 @@ def apply_effect(self, name: Optional[str], filter_str: str, # ── Prefetch ────────────────────────────────────────────────────── - def _prefetch_delay(self, track: dict) -> Optional[float]: + def _prefetch_delay(self, track: Track) -> Optional[float]: """ Seconds to wait before prefetching, or ``None`` to start immediately. @@ -258,7 +259,7 @@ def _start_prefetch(self) -> None: upcoming = self.queue[0] self._prefetch_task = self.bot.loop.create_task(self._prefetch_next(upcoming)) - async def _prefetch_next(self, upcoming: dict) -> None: + async def _prefetch_next(self, upcoming: Track) -> None: try: stream = await self.bot.loop.run_in_executor( None, media.spawn_stream, upcoming) @@ -271,7 +272,7 @@ async def _prefetch_next(self, upcoming: dict) -> None: return self._discard(stream) self._prefetch = (upcoming, stream) - def _take_prefetch(self, track: dict) -> Optional[media.AudioStream]: + def _take_prefetch(self, track: Track) -> Optional[media.AudioStream]: """ The prefetched stream for ``track``, or ``None``. @@ -296,7 +297,7 @@ def _discard(self, stream: media.AudioStream) -> None: """Close a stream we are not going to play, off the event loop.""" self.bot.loop.run_in_executor(None, stream.close) - async def _wait_for_end(self, track: dict) -> None: + async def _wait_for_end(self, track: Track) -> None: """Wait for the current track to finish, prefetching before it does.""" delay = self._prefetch_delay(track) if delay is None: @@ -316,7 +317,7 @@ def _seek_target(self) -> float: Live streams report no duration and cannot be seeked, and a position in the last couple of seconds would resume into silence or past the end. """ - duration = (self.current or {}).get("duration") + duration = self.current.get("duration") if self.current else None if not duration: return 0.0 position = self.elapsed @@ -375,6 +376,8 @@ async def _player_loop(self) -> None: seek_to = self._resume_at self._resume_at = 0.0 self._seek_base = seek_to + if stream.stdout is None: + raise RuntimeError("stream was closed before it could play") source = media.make_pipe_source( stream.stdout, volume=self.volume, ffmpeg_filter=self.effect_filter, seek_seconds=seek_to, @@ -420,7 +423,7 @@ async def _player_loop(self) -> None: log.exception("Player loop crashed for guild %s", self.guild.id) self.destroy() - async def _open_stream(self, track: dict) -> Optional[media.AudioStream]: + async def _open_stream(self, track: Track) -> Optional[media.AudioStream]: """ The track's audio: the prefetched stream if there is one, else a new one. @@ -448,7 +451,7 @@ def _after_play(self, error: Optional[Exception]) -> None: log.warning("Playback error in guild %s: %s", self.guild.id, error) self.bot.loop.call_soon_threadsafe(self._next.set) - async def _advance(self) -> tuple[Optional[dict], bool]: + async def _advance(self) -> tuple[Optional[Track], bool]: """ Decide the next track to play. diff --git a/utils/startup.py b/utils/startup.py index 6720458..e1d0c7d 100644 --- a/utils/startup.py +++ b/utils/startup.py @@ -17,6 +17,7 @@ import contextlib import logging import signal +from typing import Any, Awaitable, Callable import aiohttp import discord @@ -47,7 +48,7 @@ def is_transient(error: BaseException) -> bool: asyncio.TimeoutError, discord.GatewayNotFound)) -async def _discard_session(http) -> None: +async def _discard_session(http: discord.http.HTTPClient) -> None: """ Release the aiohttp session a failed login left behind. @@ -67,9 +68,10 @@ async def _discard_session(http) -> None: http.connector = discord.utils.MISSING -async def login_with_retry(bot, token: str, *, +async def login_with_retry(bot: discord.Client, token: str, *, delays: tuple[float, ...] = LOGIN_RETRY_DELAYS, - sleep=asyncio.sleep) -> None: + sleep: Callable[[float], Awaitable[Any]] = asyncio.sleep, + ) -> None: """ Log ``bot`` in, retrying while the failure looks like weather. @@ -89,7 +91,7 @@ async def login_with_retry(bot, token: str, *, await sleep(delay) -async def start(bot, token: str, **retry_options) -> None: +async def start(bot: discord.Client, token: str, **retry_options: Any) -> None: """ Bring the bot online: a retried login, then discord.py's own gateway loop. @@ -109,7 +111,8 @@ async def start(bot, token: str, **retry_options) -> None: _STOP_SIGNALS = ("SIGINT", "SIGTERM") -async def leave_voice(bot, *, timeout: float = VOICE_DISCONNECT_TIMEOUT) -> None: +async def leave_voice(bot: discord.Client, *, + timeout: float = VOICE_DISCONNECT_TIMEOUT) -> None: """ Leave every voice channel, giving each one a bounded chance to confirm. @@ -144,10 +147,10 @@ def _watch_for_stop_signals(stop: asyncio.Event) -> None: pass -async def serve(bot, token: str, *, +async def serve(bot: discord.Client, token: str, *, stop: asyncio.Event | None = None, voice_timeout: float = VOICE_DISCONNECT_TIMEOUT, - **retry_options) -> None: + **retry_options: Any) -> None: """ Run the bot until it is asked to stop, then release voice within a bound. From 05cb22a3b95fd72d7ae5590485338f5fe4b93371 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:12:04 -0600 Subject: [PATCH 05/11] Show track durations in whole seconds yt-dlp reports some durations as floats (SoundCloud: 187.43), and timedelta rendered those in Now Playing and the queue as 0:03:07.430000. --- tests/test_lyrics_view.py | 11 +++++++++++ utils/embeds.py | 4 +++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_lyrics_view.py b/tests/test_lyrics_view.py index 3561a64..d200bcb 100644 --- a/tests/test_lyrics_view.py +++ b/tests/test_lyrics_view.py @@ -132,3 +132,14 @@ def test_a_single_line_longer_than_the_limit_is_split_anyway(): def test_empty_lyrics_give_one_empty_page(): assert lyrics_pages("") == [""] + + +@pytest.mark.parametrize("seconds,expected", [ + (210, "0:03:30"), + (187.43, "0:03:07"), # SoundCloud reports fractional durations + (3725.9, "1:02:05"), +]) +def test_durations_are_shown_in_whole_seconds(seconds, expected): + from utils.embeds import format_duration + + assert format_duration(seconds) == expected diff --git a/utils/embeds.py b/utils/embeds.py index 3e0f220..34fae84 100644 --- a/utils/embeds.py +++ b/utils/embeds.py @@ -16,7 +16,9 @@ def format_duration(seconds: Optional[float]) -> str: if not seconds: return "🔴 LIVE" - return str(timedelta(seconds=seconds)) + # Whole seconds: SoundCloud reports durations like 187.43, which timedelta + # would render as 0:03:07.430000. + return str(timedelta(seconds=int(seconds))) def _linked_title(track: Track) -> str: From 60b850ac7addb2b64352f45946d4eb99e74e887e Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:13:41 -0600 Subject: [PATCH 06/11] Lint and type-check in CI There was no linter and no type checker, so nothing held the type hints in place once written, and an undefined name in a rarely-run branch would only show up as a crash in production. ruff (correctness rules only: pyflakes, syntax errors, bugbear) and mypy (every function typed) now run before the tests. Both are pinned, so a new release cannot fail CI on unchanged code. The first ruff run already caught an unused import from the typing work. Also drops a trailing space and an unused variable in two tests. --- .github/workflows/tests.yml | 9 +++++++++ README.md | 5 ++++- mypy.ini | 18 ++++++++++++++++++ requirements-dev.txt | 5 ++++- ruff.toml | 6 ++++++ tests/test_lyrics_follower.py | 2 +- tests/test_synced_lyrics.py | 2 +- utils/checks.py | 2 +- 8 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 mypy.ini create mode 100644 ruff.toml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index bbf4bbe..edab48d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -41,6 +41,15 @@ jobs: python -m pip install --upgrade pip pip install -r requirements-dev.txt + - name: Lint + # Correctness rules only (see ruff.toml): an undefined name in a + # rarely-run branch otherwise surfaces as a crash in production. + run: ruff check . + + - name: Type check + # Every function must be annotated (see mypy.ini). + run: mypy + - name: Run tests # The suite needs no credentials, no network and no .env. # -ra surfaces anything that skipped, so silent skips stay visible. diff --git a/README.md b/README.md index 0f24840..fd287e6 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,8 @@ adds a fallback for songs LRCLIB does not have. ```bash .venv/bin/pip install -r requirements-dev.txt # Windows: .venv\Scripts\pip pytest +ruff check . # correctness lint (ruff.toml) +mypy # every function must be typed (mypy.ini) ``` The suite needs **no credentials, no network and no `.env`** — it mocks the @@ -118,7 +120,8 @@ A few tests render audio through **real FFmpeg** to measure the pitch and speed filters, since a wrong filter string looks perfectly reasonable and only shows up as the wrong playback speed. Those skip automatically if FFmpeg is not installed. -CI runs everything on every push and pull request against Python 3.11 and 3.12. +CI runs the lint, the type check and the tests on every push and pull request, +against Python 3.11 and 3.12. ### Deploy to a server See **[deploy/README.md](deploy/README.md)** for the full walkthrough: diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 0000000..286ad0f --- /dev/null +++ b/mypy.ini @@ -0,0 +1,18 @@ +[mypy] +python_version = 3.11 +files = main.py, config.py, cogs, services, utils +# The project is a set of top-level packages rather than one installed package. +explicit_package_bases = True +# The project rule: every function carries type hints. +disallow_untyped_defs = True +disallow_incomplete_defs = True +check_untyped_defs = True +warn_unused_ignores = True +warn_redundant_casts = True + +# Neither ships type information. +[mypy-yt_dlp.*] +ignore_missing_imports = True + +[mypy-lyricsgenius.*] +ignore_missing_imports = True diff --git a/requirements-dev.txt b/requirements-dev.txt index f708f29..e742c0f 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,5 +1,8 @@ -# Test-only dependencies. Install with: +# Test and static-analysis dependencies. Install with: # pip install -r requirements.txt -r requirements-dev.txt -r requirements.txt pytest>=8.0 pytest-asyncio>=0.24 +# Pinned: a new release of either can start failing CI on unchanged code. +ruff==0.16.8 +mypy==2.3.1 diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 0000000..a40e950 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,6 @@ +target-version = "py311" + +[lint] +# Correctness only: undefined names, unused imports and variables, syntax +# errors, and bugbear's likely-bug patterns. Formatting is left alone. +select = ["F", "E9", "B"] diff --git a/tests/test_lyrics_follower.py b/tests/test_lyrics_follower.py index d678a40..0d13f94 100644 --- a/tests/test_lyrics_follower.py +++ b/tests/test_lyrics_follower.py @@ -86,7 +86,7 @@ async def load(track): async def test_it_edits_when_the_line_changes(): player = FakePlayer(current={"t": 1}, position=0.0) - follower, conductor, _ = build(player, [15.0, 25.0]) + follower, _, _ = build(player, [15.0, 25.0]) await follower.run() diff --git a/tests/test_synced_lyrics.py b/tests/test_synced_lyrics.py index 375e1d0..8faa5a5 100644 --- a/tests/test_synced_lyrics.py +++ b/tests/test_synced_lyrics.py @@ -85,7 +85,7 @@ def test_an_empty_body_parses_to_nothing(): assert parse_lrc("") == () -def test_the_real_payload_parses(): +def test_the_real_payload_parses(): lines = parse_lrc(REAL) assert len(lines) == 6 assert lines[0] == (0.15, "Is this the real life? Is this just fantasy?") diff --git a/utils/checks.py b/utils/checks.py index 4d2891f..05b0242 100644 --- a/utils/checks.py +++ b/utils/checks.py @@ -1,4 +1,4 @@ -from typing import Any, Callable, TypeVar +from typing import Callable, TypeVar from discord.ext import commands From 6a804337a51577ba0764d522968754fbe1352689 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:14:31 -0600 Subject: [PATCH 07/11] Define yt-dlp's format and source address once The player-client chain was already shared between the metadata options and the streaming command because two copies drift apart. The format and the source address were still written out twice, with the same risk. --- services/media.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/services/media.py b/services/media.py index 97a70f2..fb8a3e5 100644 --- a/services/media.py +++ b/services/media.py @@ -73,14 +73,18 @@ class Track(TypedDict): # chain right avoids needing them at all. _PLAYER_CLIENTS = ("web_embedded", "mweb", "tv_embedded") +# Shared by the metadata options and the streaming command, for the same reason. +_FORMAT = "bestaudio/best" +_SOURCE_ADDRESS = "0.0.0.0" # bind to IPv4; avoids some 403s + # Base yt-dlp config shared by every call. YTDL_OPTIONS: dict[str, Any] = { - "format": "bestaudio/best", + "format": _FORMAT, "noplaylist": True, "quiet": True, "no_warnings": True, "default_search": "ytsearch", - "source_address": "0.0.0.0", # bind to IPv4; avoids some 403s + "source_address": _SOURCE_ADDRESS, "skip_download": True, "extractor_args": { "youtube": {"player_client": list(_PLAYER_CLIENTS)}, @@ -377,11 +381,11 @@ def spawn_stream(track: Track) -> AudioStream: """Start streaming a track's best audio through yt-dlp.""" cmd = [ sys.executable, "-m", "yt_dlp", - "-f", "bestaudio/best", + "-f", _FORMAT, "-o", "-", # write audio to stdout "-q", "--no-warnings", "--no-playlist", "--extractor-args", f"youtube:player_client={','.join(_PLAYER_CLIENTS)}", - "--source-address", "0.0.0.0", + "--source-address", _SOURCE_ADDRESS, ] cookies = YTDL_OPTIONS.get("cookiefile") if cookies: From 4bac2aec62d109989f42e1edbc34e50dffaf1df3 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:14:49 -0600 Subject: [PATCH 08/11] Read a relative COOKIES_PATH from the project root .env is anchored to the project root so the same file is found however the bot is started. A relative COOKIES_PATH was not: resolved against the working directory, cookies.txt was silently missed whenever the bot ran from anywhere else. Under systemd the two coincide, so the deployed bot behaves exactly as before. --- config.py | 15 ++++++++++++++- tests/test_config.py | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/config.py b/config.py index 3c13094..9a7f820 100644 --- a/config.py +++ b/config.py @@ -20,10 +20,23 @@ PROJECT_ROOT = os.path.dirname(os.path.abspath(__file__)) load_dotenv(os.path.join(PROJECT_ROOT, ".env")) + +def project_path(value: Optional[str]) -> Optional[str]: + """A path from .env, with a relative one read from the project root. + + The same anchoring as .env itself, and for the same reason: resolved + against the working directory, `COOKIES_PATH=cookies.txt` silently found + nothing whenever the bot was started from anywhere else. + """ + if not value or os.path.isabs(value): + return value + return os.path.join(PROJECT_ROOT, value) + + DISCORD_TOKEN = os.getenv("DISCORD_TOKEN") COMMAND_PREFIX = os.getenv("COMMAND_PREFIX", "!") GENIUS_TOKEN = os.getenv("GENIUS_TOKEN") -COOKIES_PATH = os.getenv("COOKIES_PATH") +COOKIES_PATH = project_path(os.getenv("COOKIES_PATH")) LOG_LEVEL = os.getenv("LOG_LEVEL", "INFO").upper() COGS = [ diff --git a/tests/test_config.py b/tests/test_config.py index a8d1763..abce662 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -185,3 +185,21 @@ def test_the_bot_unit_is_defined_exactly_once(): if "Description=LoopifyBot Discord Music Bot" in handle.read(): definers.append(path) assert len(definers) == 1, f"the service unit is defined in {len(definers)} places: {definers}" + + +# -- paths from .env ------------------------------------------------------- + +def test_a_relative_path_is_read_from_the_project_root(): + import os + + assert config.project_path("cookies.txt") == os.path.join(config.PROJECT_ROOT, "cookies.txt") + + +def test_an_absolute_path_is_left_alone(tmp_path): + absolute = str(tmp_path / "cookies.txt") + assert config.project_path(absolute) == absolute + + +@pytest.mark.parametrize("unset", [None, ""]) +def test_an_unset_path_stays_unset(unset): + assert config.project_path(unset) == unset From 596ec182a6358a900096d578b45338600c949c52 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:15:45 -0600 Subject: [PATCH 09/11] Trim .gitignore to the rules this project uses It was GitHub's full Python template: Django, Flask, Scrapy, Celery, SageMath, Marimo and a dozen more, with the load-bearing .cache rule sitting under 'Unit test / coverage reports' where it read as test output. It is the runtime cache the systemd unit points XDG_CACHE_HOME and DENO_DIR at, and now says so. The set of ignored files in the checkout is unchanged. --- .gitignore | 219 +++++------------------------------------------------ 1 file changed, 21 insertions(+), 198 deletions(-) diff --git a/.gitignore b/.gitignore index a01eb44..552059d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,218 +1,41 @@ -# Byte-compiled / optimized / DLL files +# Python __pycache__/ *.py[codz] *$py.class - -# C extensions -*.so - -# Distribution / packaging -.Python +*.egg-info/ build/ -develop-eggs/ dist/ -downloads/ -eggs/ -.eggs/ -lib/ -lib64/ -parts/ -sdist/ -var/ -wheels/ -share/python-wheels/ -*.egg-info/ -.installed.cfg -*.egg -MANIFEST -# PyInstaller -# Usually these files are written by a python script from a template -# before PyInstaller builds the exe, so as to inject date/other infos into it. -*.manifest -*.spec - -# Installer logs -pip-log.txt -pip-delete-this-directory.txt +# Virtual environments +.venv/ +venv/ +env/ -# Unit test / coverage reports -htmlcov/ -.tox/ -.nox/ +# Test and static-analysis caches +.pytest_cache/ +.mypy_cache/ +.ruff_cache/ .coverage .coverage.* -.cache -nosetests.xml -coverage.xml -*.cover -*.py.cover -.hypothesis/ -.pytest_cache/ -cover/ - -# Translations -*.mo -*.pot - -# Django stuff: -*.log -local_settings.py -db.sqlite3 -db.sqlite3-journal - -# Flask stuff: -instance/ -.webassets-cache - -# Scrapy stuff: -.scrapy - -# Sphinx documentation -docs/_build/ - -# PyBuilder -.pybuilder/ -target/ - -# Jupyter Notebook -.ipynb_checkpoints - -# IPython -profile_default/ -ipython_config.py - -# pyenv -# For a library or package, you might want to ignore these files since the code is -# intended to run in multiple environments; otherwise, check them in: -# .python-version - -# pipenv -# According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control. -# However, in case of collaboration, if having platform-specific dependencies or dependencies -# having no cross-platform support, pipenv may install dependencies that don't work, or not -# install all needed dependencies. -#Pipfile.lock - -# UV -# Similar to Pipfile.lock, it is generally recommended to include uv.lock in version control. -# This is especially recommended for binary packages to ensure reproducibility, and is more -# commonly ignored for libraries. -#uv.lock - -# poetry -# Similar to Pipfile.lock, it is generally recommended to include poetry.lock in version control. -# This is especially recommended for binary packages to ensure reproducibility, and is more -# commonly ignored for libraries. -# https://python-poetry.org/docs/basic-usage/#commit-your-poetrylock-file-to-version-control -#poetry.lock -#poetry.toml - -# pdm -# Similar to Pipfile.lock, it is generally recommended to include pdm.lock in version control. -# pdm recommends including project-wide configuration in pdm.toml, but excluding .pdm-python. -# https://pdm-project.org/en/latest/usage/project/#working-with-version-control -#pdm.lock -#pdm.toml -.pdm-python -.pdm-build/ - -# pixi -# Similar to Pipfile.lock, it is generally recommended to include pixi.lock in version control. -#pixi.lock -# Pixi creates a virtual environment in the .pixi directory, just like venv module creates one -# in the .venv directory. It is recommended not to include this directory in version control. -.pixi - -# PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm -__pypackages__/ - -# Celery stuff -celerybeat-schedule -celerybeat.pid +htmlcov/ -# SageMath parsed files -*.sage.py +# Runtime caches. Not test output: the systemd unit points XDG_CACHE_HOME and +# DENO_DIR here (deploy/install-units.sh), so on the server this fills up. +.cache # yt-dlp fragment artifacts: streaming to stdout can drop these in the CWD. --Frag* *.part *.ytdl -# Project-specific: AI assistant notes & sensitive runtime files -CLAUDE.md -.claude/ -cookies.txt -*.pem +*.log -# Environments +# Secrets and credentials — never committed. .env .envrc -.venv -env/ -venv/ -ENV/ -env.bak/ -venv.bak/ - -# Spyder project settings -.spyderproject -.spyproject - -# Rope project settings -.ropeproject - -# mkdocs documentation -/site - -# mypy -.mypy_cache/ -.dmypy.json -dmypy.json - -# Pyre type checker -.pyre/ - -# pytype static type analyzer -.pytype/ - -# Cython debug symbols -cython_debug/ - -# PyCharm -# JetBrains specific template is maintained in a separate JetBrains.gitignore that can -# be found at https://github.com/github/gitignore/blob/main/Global/JetBrains.gitignore -# and can be added to the global gitignore or merged into this file. For a more nuclear -# option (not recommended) you can uncomment the following to ignore the entire idea folder. -#.idea/ - -# Abstra -# Abstra is an AI-powered process automation framework. -# Ignore directories containing user credentials, local state, and settings. -# Learn more at https://abstra.io/docs -.abstra/ - -# Visual Studio Code -# Visual Studio Code specific template is maintained in a separate VisualStudioCode.gitignore -# that can be found at https://github.com/github/gitignore/blob/main/Global/VisualStudioCode.gitignore -# and can be added to the global gitignore or merged into this file. However, if you prefer, -# you could uncomment the following to ignore the entire vscode folder -# .vscode/ - -# Ruff stuff: -.ruff_cache/ - -# PyPI configuration file -.pypirc - -# Cursor -# Cursor is an AI-powered code editor. `.cursorignore` specifies files/directories to -# exclude from AI features like autocomplete and code analysis. Recommended for sensitive data -# refer to https://docs.cursor.com/context/ignore-files -.cursorignore -.cursorindexingignore +cookies.txt +*.pem -# Marimo -marimo/_static/ -marimo/_lsp/ -__marimo__/ +# Local assistant notes. +CLAUDE.md +.claude/ From d93083f12e81e22a443b4278644ed6c884fc755c Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:15:45 -0600 Subject: [PATCH 10/11] Move the synced-lyrics design note to docs/design --- .../2026-09-12-synced-lyrics.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/{superpowers/specs/2026-09-12-synced-lyrics-design.md => design/2026-09-12-synced-lyrics.md} (100%) diff --git a/docs/superpowers/specs/2026-09-12-synced-lyrics-design.md b/docs/design/2026-09-12-synced-lyrics.md similarity index 100% rename from docs/superpowers/specs/2026-09-12-synced-lyrics-design.md rename to docs/design/2026-09-12-synced-lyrics.md From 7e6141f6d3990f7e4f81c2d6a1d6f0b49969bce3 Mon Sep 17 00:00:00 2001 From: Ismael Leon <ismaleonsaenz@gmail.com> Date: Mon, 21 Sep 2026 20:16:37 -0600 Subject: [PATCH 11/11] Download Deno into a private temp directory setup.sh fetched Deno to the fixed path /tmp/deno.zip, unpacked it into /tmp and then moved /tmp/deno into /usr/local/bin with sudo. On a shared machine another user could place that file first. mktemp -d gives a directory only this user can write, and install sets the mode in one step. --- deploy/setup.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/deploy/setup.sh b/deploy/setup.sh index 5c21ebe..ea93f0e 100644 --- a/deploy/setup.sh +++ b/deploy/setup.sh @@ -38,12 +38,14 @@ if ! command -v deno >/dev/null 2>&1; then *) echo "!! Unknown arch $ARCH — skipping Deno"; DENO_TARGET="" ;; esac if [[ -n "$DENO_TARGET" ]]; then - curl -fsSL -o /tmp/deno.zip \ + # A private directory, not a fixed name in /tmp: this binary ends up + # in /usr/local/bin via sudo, so nobody else may get to place it first. + DENO_TMP="$(mktemp -d)" + curl -fsSL -o "$DENO_TMP/deno.zip" \ "https://github.com/denoland/deno/releases/latest/download/deno-${DENO_TARGET}.zip" - unzip -o /tmp/deno.zip -d /tmp >/dev/null - sudo mv -f /tmp/deno /usr/local/bin/deno - sudo chmod +x /usr/local/bin/deno - rm -f /tmp/deno.zip + unzip -o "$DENO_TMP/deno.zip" -d "$DENO_TMP" >/dev/null + sudo install -m 755 "$DENO_TMP/deno" /usr/local/bin/deno + rm -rf "$DENO_TMP" fi fi command -v deno >/dev/null 2>&1 && echo "==> Deno: $(deno --version | head -1)"