Melodix

Ownership

These contracts define who can change playback and voice state, how long that state lives, and which checks protect it. Use the numbered rules when changing a player, stream, session or status message. Each rule names its enforcement and any known gap.

  • Rules 1–5: track values, stream mutation, locks and playback runs.
  • Rules 6–9: session lifetime, connection liveness and protected audio frames.
  • Rules 10–13: dispatch, status messages, reply visibility and button middleware.
  • Known gaps and adding a rule.

Enforcement grades

A rule’s grade is the strongest thing that would stop a change violating it.

Grade Means
compiler The violation does not typecheck. Needs no vigilance.
refused It compiles and fails at runtime with an error naming the mistake.
checked A test reads the source and fails on the shape. Survives refactors that behavioural tests do not.
tested A test fails, usually under -race. Depends on the test driving the right interleaving.
documented A comment says so. Review is the only enforcement — treat these as rules that will eventually be broken.

go test -race ./... runs all of it.

1. A Track leaves the engine by value

Player.CurrentTrack returns parsers.Track. No exported player method hands out a *parsers.Track.

Because a playing Track is written while it plays: the parser that actually opened it, whether the stream is passthrough or cached, a title the media knew and the resolver did not. Renderers read it from their own goroutines. A pointer shares the writes; a value cannot.

Enforced by compiler — a caller holding a value has nothing to write back into the engine — and checked: player.TestNoExportedMethodHandsOutATrackPointer.

2. A Track goes down by value; findings come back as values

Streamer.Open takes the Track by value and returns what the parser learned in a parsers.Opened. RecoveryStream is built from a Track value, applies each Opened to its own copy, and reports what it learned as a stream.OpenInfo – from Start and from the confirmation callback – which the player applies under its own lock.

Because Open used to take a *Track as an out-parameter, and five parser packages wrote through it. Whoever owned that pointer had its state written by a parser, from whichever goroutine was reading the stream.

Enforced by compiler – no parser or stream is handed a pointer it could write through – and checked: parsers.TestStreamerOpenTakesTheTrackByValue pins the shape of the interface. stream.TestRecoveryStreamDoesNotWriteTheCallersTrack still covers the one thing a value shares, the parser list’s backing array.

3. A stream’s run state has one writer, and it is whoever reads it

Parser index, position, retry counts, cache writer, Track copy: all belong to the goroutine pulling packets. Start opens once, before a reader exists. After that the only way in is RequestReopen, which raises a flag and closes the source to unblock a parked read; the reader performs the reopen itself.

There is no exported accessor for what a stream is playing. Copying the answer out does not help — the read itself races the writer.

Because recovery rewrites every one of those fields, and one of them is a map. A concurrent map write is a runtime crash, not an error you can handle.

Enforced by compiler — open is unexported, so no other package can call it — refused — a second Start returns an error rather than performing it — and tested: stream.TestRequestReopenDoesNotRaceTheReadAheadProducer, TestAStreamCannotBeStartedTwice.

4. p.mu is never held across I/O

Capture what the call needs under the lock, release, then call.

Because the player’s outward edges are a voice join (15s), a voice leave (10s) and a stream open (a fetch, or an ffmpeg process). The lock guards one guild’s queue, so anything held across those makes every reader of that queue wait for a network round trip.

Enforced by checked: player.TestLockIsNeverHeldAcrossIO scans player.go for the outward interfaces’ method names inside a locked region. It matches method names rather than receivers, because moving a call out from under a lock starts by capturing the receiver into a local — so a receiver-keyed check goes quiet exactly when someone puts one back.

Limit: it does not follow calls. A helper called under the lock that itself does I/O passes this and is still wrong.

5. A run changes only its own state

A playback run is a *run: its stop and done channels, its track, its phase, the parser the UI was told about and whether history has a row. The player holds the latest one as p.run. The playback goroutine, the confirmation callback and the queue-end teardown are each handed their run, and whether it is still in charge is p.run == r.

Because Stop waits up to ten seconds for the playback goroutine to exit. A track can end and the next begin inside that wait, so the run that is current when it finishes is not the one it was called for. When those fields lived on the player, every run wrote the same ones, and a generation counter compared before each write was all that kept a late run off a newer run’s state.

A run ending now writes only that run: finish(r) sets r’s phase and nothing else, and a Stop that was overtaken ends the run it captured, not the one that replaced it. What still needs the identity check is what belongs to the player rather than to a run – the queue and the voice channel – so a teardown aimed at one run does nothing once another is in charge.

Enforced by compiler – per-run state is not on Player, and a run’s goroutines are passed their *run – and tested: player.TestStopDoesNotResetANewerRun, TestASupersededRunDoesNotClearTheCurrentOne, TestAStopNeverResetsARunItDidNotAskAbout, and TestAQueueEndTeardownDoesNotStopTheTrackThatFollowedIt, which fails when the teardown’s identity check is removed.

Run identity has one blind spot, and the queue-end teardown covers it separately: /play enqueues before it starts a run, so between the two the run still matches while the queue is no longer empty. A teardown that finds tracks queued stands down rather than clearing them. Tested: TestAQueueEndTeardownKeepsATrackQueuedAfterTheQueueEmptied.

6. Nothing reachable from a Player has session lifetime

A voicesink.Provider holds no voice manager and no DAVE registry. It resolves both per acquisition through a Resources function.

Because a Player outlives every gateway session, and both of those die with one: a voice manager belongs to a single bot.Client and closes over that client’s gateway, and a DAVE registry is built fresh per session. Held across a reconnect, a join is addressed to a gateway that has been shut — which fails instantly and permanently, and which invalidating the sink does not fix, because what is stale is the thing that makes connections.

Enforced by checked — voicesink.TestAProviderStoresNothingThatDiesWithTheSession reflects over the struct — and tested: TestAProviderUsesTheLiveSessionAfterARestart.

The violation to watch for is “resolve once in the constructor and keep it”. It reads as a tidy-up.

7. Connection liveness is the voice manager’s answer

The provider keeps the connection it opened only to recognise it again, and compares it by identity. It never reads protocol state off it.

Because disgo removes a connection on a voice websocket close it cannot resume from, and on closing the client, and announces neither. A field of ours can only hold a memory of an answer. Acquiring without asking hands the next track a sink over a dead socket; releasing without asking spends the close budget waiting on a gateway that is not there. Reading protocol state off the connection races the gateway goroutine, which writes it without a lock.

Enforced by checked — voicesink.TestEveryPathThatDecidesLivenessAsksTheManager — and tested: TestAConnRemovedByTheLibraryForcesARejoin, TestReleasingAConnTheLibraryAlreadyTookDoesNotCloseIt.

8. A sink never emits a frame its transport cannot protect

While the guild’s DAVE session reports no live epoch, the frame provider withholds frames, without consuming the packets it holds — so the hold is a pause rather than a gap. A hold that outlasts its budget ends the track as a transport failure.

Because dave-go forwards a frame unmodified rather than failing when no ratchet is selectable, and disgo’s send path never asks. An unprotected frame on an end-to-end encrypted channel is dropped by every receiver expecting encryption, and reaches the server with no end-to-end layer.

Enforced by tested — voicesink.TestHoldStopsSendingWithoutAnEpoch, TestHoldThatNeverResolvesEndsTheTrack — and documented in pkg/music/sink’s AudioSink contract, whose silence is what let this property disappear during a library swap.

9. A sink detects its own transport dying

Sink.Stream watches three things it can observe: the audio sender has stopped asking for frames, the pull it is inside has outlasted its budget, and the connection it was built on is no longer the guild’s.

Because disgo offers one signal for a connection dying under a running track, OpusFrameProvider.Close, and v0.19.6 never calls it. A sink that waits for it blocks forever: the track never ends, the queue never advances, and the player’s transport recovery sits behind an error nothing produces. The pull budget is the same failure arriving from the other side — a source that stops delivering parks the sender inside one ReadPacket, where every other signal reads healthy, so without a bound a stalled read is indistinguishable from a slow one and lasts forever.

Enforced by tested — voicesink.TestClosedSocketEndsTheTrackAsTransportFailure, TestAConnRemovedBehindOurBackEndsTheTrack, TestASourceThatStopsDeliveringEndsTheTrack, all against disgo’s real audio sender, with TestASlowReadIsNotAStalledOne for the other edge — and documented in the AudioSink contract.

10. The gateway read loop dispatches and returns

Command bodies run on a per-guild worker. Ready and GuildJoin stay inline; their ordering is load-bearing.

Because disgo dispatches events synchronously. A command body on that goroutine holds the socket unread for its whole duration, and Discord’s acknowledgement deadline is three seconds from the interaction’s creation, not from its delivery.

Enforced by tested: discord.TestDispatchDoesNotRunTheCommandOnTheCallersGoroutine, TestTwoGuildsRunAtTheSameTime, TestOneGuildStaysSequential.

11. A guild’s status message has one owner

The voice service is the only thing that registers, replaces or drops a guild’s status message, and PlayNextAndAnnounce is the only way one comes to exist. Commands hand it the interaction; they do not start playback and announce it as two steps of their own.

Because the player announces Playing while PlayNext is still running, before the command’s message exists, and the watcher renders Playing into whatever is registered at that moment. With the start split between the commands and the service, that was the previous status message: it got the new track written into it and stayed on that track for good. And a /search pick, announcing through its own interaction, registered the ephemeral chooser — visible to one person, and out of reach of every later edit.

Enforced by compiler — VoiceAPI offers commands no way to register a message except through PlayNextAndAnnounce — and tested: voice.TestStartingPlaybackLeavesThePreviousStatusMessageAlone, TestAPickStartsAPublicStatusMessage, TestSkippingMarksTheOldStatusMessageSkipped.

12. A private reply stays private

The Responder records whether it deferred publicly. An ephemeral followup that would be the first thing to replace a public placeholder deletes the placeholder first, then posts.

Because the first followup after a deferral replaces the placeholder and takes the deferral’s visibility, not its own. Every command that defers publicly and then reports an error ephemerally — the voice errors of /play and /next, /stop’s, the dispatcher’s own — posted that error to the whole channel. Deleting the placeholder after the followup instead would take the followup with it, since by then it is the original response.

Enforced by tested: reply.TestAnEphemeralReplyAfterAPublicDeferStaysPrivate, TestOnlyAPublicPlaceholderIsRemovedForAPrivateReply, which pin the order of calls against a fake REST client. Discord’s behaviour itself is outside any test; the release matrix checks it live.

13. A click is an invocation of its command

A component interaction runs the command that owns it through that command’s middleware chain, like a slash invocation does. The Adapter at the bottom of the chain hands it to the component handler.

Because clicks used to go straight to the handler. Nothing a command was registered with applied to its buttons: no click was audited, and disabling a command group left its choosers working. The group check had a branch for components that nothing could reach.

Enforced by tested: discord.TestAComponentRunsThroughItsCommandsMiddleware drives the dispatcher; middleware.TestAClickIsAudited and TestAClickInADisabledGroupIsRefused drive the production chain.

Not enforced

These limits need review or runtime verification beyond the named checks.

  • Rule 4 does not follow calls into helpers invoked under the lock.
  • Reply errors are ignored at every call site in internal/discord/command, which is defensible — the interaction is gone by the time one fails — but means no command can act on one. What was the actual defect, the silence, is fixed: adapter reports every failed reply as reply_failed before returning it.
  • kkdai’s init() rewrites a third-party package’s global for the whole process on import. The library offers no narrower knob.
  • A UDP write failing with anything but a closed socket is logged by disgo and ignored, so a track can drain into a socket delivering nothing while everything above reports normal playback. Melodix cannot observe it. The log bridge names it voice_audio_send_failed, which is a mitigation, not a fix.

Adding a rule

A rule that cannot name its enforcement does not belong here yet.

Write the check first — it is usually twenty lines of reading your own source — then verify it fails when you reintroduce the shape it forbids. Every check above was confirmed that way, and rule 4’s first version did not catch the defect it was written for.