Compare commits

..
11 Commits
Author SHA1 Message Date
bvandeusen 727f68950e Merge PR #126: missing-file lifecycle, UPnP stall recovery, Android browse parity, Flutter client removed
release / Build signed APK (tag releases only) (push) Skipped
test-web / test (push) Successful in 1m5s
test-go / test (push) Successful in 1m26s
release / Build + push container image (push) Successful in 1m38s
release / Verify release artifacts (tag releases only) (push) Skipped
android / Build + lint + test (push) Successful in 4m54s
test-go / integration (push) Successful in 5m19s
2026-08-17 16:28:13 -04:00
bvandeusen aa9f534f3c Merge pull request 'Genre index: sort A–Z, and repair casing damage at scan time' (#124) from dev into main
test-web / test (push) Successful in 49s
test-go / test (push) Successful in 1m9s
test-go / integration (push) Successful in 5m1s
release / Build signed APK (tag releases only) (push) Successful in 4m2s
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Successful in 2s
2026-08-07 21:40:33 -04:00
bvandeusen 011b4d9a9c Merge pull request 'ci(release): verify a tag release actually shipped its artifacts' (#123) from dev into main
release / Build signed APK (tag releases only) (push) Successful in 3m38s
release / Build + push container image (push) Successful in 1m31s
release / Verify release artifacts (tag releases only) (push) Successful in 2s
2026-08-07 08:32:48 -04:00
bvandeusen d5aa081157 Merge pull request 'Recommendation metrics: publish the margin of error on every delta' (#122) from dev into main
test-web / test (push) Successful in 52s
test-go / test (push) Successful in 1m10s
test-go / integration (push) Successful in 4m53s
release / Build signed APK (tag releases only) (push) Successful in 3m57s
release / Build + push container image (push) Successful in 1m37s
2026-08-06 21:50:41 -04:00
bvandeusen a99f855e98 Merge pull request 'Missing files: detect them, stop offering them, and follow them when they move' (#121) from dev into main
test-go / test (push) Successful in 56s
test-go / integration (push) Successful in 5m0s
release / Build signed APK (tag releases only) (push) Successful in 4m14s
release / Build + push container image (push) Successful in 15s
2026-08-06 20:40:39 -04:00
bvandeusen 7e4727fc49 Merge pull request 'Genre tags: read multi-value frames correctly, and repair existing rows' (#120) from dev into main
test-go / test (push) Successful in 57s
test-go / integration (push) Successful in 4m57s
release / Build signed APK (tag releases only) (push) Successful in 4m23s
release / Build + push container image (push) Successful in 1m39s
2026-08-05 22:10:41 -04:00
bvandeusen 1b7fa635d8 Merge pull request 'Silent self-update, active sessions with real client IPs, genre/year browsing, handoff fix' (#119) from dev into main
test-web / test (push) Successful in 1m3s
test-go / test (push) Successful in 1m13s
test-go / integration (push) Successful in 5m29s
android / Build + lint + test (push) Successful in 5m34s
release / Build signed APK (tag releases only) (push) Successful in 5m5s
release / Build + push container image (push) Successful in 16s
2026-08-05 15:14:48 -04:00
bvandeusen 57d2299180 Merge pull request 'Queue row gestures: album art as grab surface + swipe-to-remove' (#118) from dev into main
test-web / test (push) Successful in 48s
android / Build + lint + test (push) Successful in 5m15s
release / Build signed APK (tag releases only) (push) Successful in 4m38s
release / Build + push container image (push) Successful in 1m48s
2026-08-04 11:37:30 -04:00
bvandeusen fa7ea41ccf Merge pull request 'Minstrel gets a mark — favicon, header lockup, Android adaptive icon' (#117) from dev into main
test-web / test (push) Successful in 47s
android / Build + lint + test (push) Successful in 4m37s
release / Build signed APK (tag releases only) (push) Successful in 8m36s
release / Build + push container image (push) Successful in 1m37s
2026-08-03 20:52:27 -04:00
bvandeusen 324059b2bd Merge pull request 'Discover request surface — taste-aware, rotating, snoozable, tag-targeted (milestone #268)' (#116) from dev into main
test-web / test (push) Successful in 1m5s
test-go / test (push) Successful in 1m30s
android / Build + lint + test (push) Successful in 5m1s
test-go / integration (push) Successful in 5m29s
release / Build signed APK (tag releases only) (push) Successful in 4m21s
release / Build + push container image (push) Successful in 17s
2026-08-03 08:38:24 -04:00
bvandeusen 1138d75a45 Merge pull request 'Playlist-track atomic replace + ci-requirements true-up' (#115) from dev into main
release / Build signed APK (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m33s
android / Build + lint + test (push) Successful in 4m30s
2026-08-01 12:23:37 -04:00
208 changed files with 1052 additions and 14779 deletions
+5 -19
View File
@@ -6,20 +6,9 @@
**/build
web/build
# The Android client — built by its own job, never from this context. The APK
# reaches the image through client/, downloaded as a CI artifact, so nothing
# here reads android/ sources.
#
# This block named `flutter_client/` until 2026-09-10 and lost its PATTERN when
# that tree was deleted, leaving a comment describing an exclusion that was no
# longer happening. android/ never took its place, so 4.1 MB of Gradle project
# has been entering the context and busting the `COPY . .` layer on every
# Android-only change.
android/
# Local `make build` output — an 18 MB binary the image never uses, since the
# builder stage compiles its own.
bin/
# Flutter mobile client — built separately on developer machines / Flutter CI.
# Including it in the Go build context wastes ~70 files and invalidates the
# `COPY . .` layer cache on every Flutter-only change.
# Docs and IDE noise
docs/
@@ -37,8 +26,5 @@ docs/
!.env.example
# CI workflow files don't need to ship in the image.
#
# This said `.forgejo/` and `.github/` — neither of which this repo has. Gitea
# Actions reads `.gitea/`, so the one directory that actually exists was the
# one not excluded, and every workflow edit invalidated the context.
.gitea/
.forgejo/
.github/
+9 -6
View File
@@ -80,12 +80,15 @@ jobs:
- name: Upload debug APK
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
# Stock action: it works on this forge since the runner moved to
# gitea/runner 3.x, which edits upload-artifact's client-side GHES refusal
# out of the action bundle (Scribe snippet #2271). Never @v3 — it reports
# success while Gitea serves artifacts back only through the v4 API, and
# it is what left 72 unreachable artifacts on this repo (Scribe 2270).
uses: actions/upload-artifact@v7
# Mirrored action, never actions/upload-artifact. @v4+ throws
# GHESNotSupportedError client-side on the hostname (no server setting
# reaches that check), and @v3 is worse — it reports success while Gitea
# serves artifacts back only through the v4 API, so the upload is stored
# and invisible to every retrieval path. @v3 is what left 72 unreachable
# artifacts on this repo. Pinned by SHA because the mirror auto-syncs;
# full URL because DEFAULT_ACTIONS_URL sends bare owner/repo to github.com.
# See Scribe issues 2255 / 2270.
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
with:
name: minstrel-android-debug-${{ github.sha }}
path: android/app/build/outputs/apk/debug/app-debug.apk
+91 -289
View File
@@ -2,71 +2,15 @@ name: release
# Builds and pushes the minstrel container image to the Gitea registry.
#
# push to dev → :dev (freshly-built dev APK bundled)
# push to main → :latest + :<sha> (latest-release APK bundled)
# push tag vYYYY.MM.DD.HHMM → :latest (fresh APK bundled)
# workflow_dispatch → manual trigger (same rules based on the ref)
# push to main → :main and :latest (latest-release APK bundled)
# push tag vYYYY.MM.DD → :vYYYY.MM.DD and :latest (freshly-built APK bundled)
# workflow_dispatch → manual trigger (same rules based on the ref)
#
# That is the whole tag map, and it is family rule 145 + 147 as written.
#
# :<sha> on main is the ROLLBACK UNIT — every production commit addressable
# without a release ceremony. It is minted only on main, where rollback is
# actually worth having: merges are gated (rule 2) so they number in the dozens
# per year, while on dev they would be one per push, forever, for a channel
# whose entire contract is that it moves.
#
# There are NO :<version> image tags. This repo published :vYYYY.MM.DD.HHMM
# until 2026-09-10 and it was the inverse of the rule on both counts — minting
# a version tag nobody pinned while the rollback unit the rule names did not
# exist here at all. Git and the build's own self-reported version answer
# "which build is this"; a third name for the same thing is upkeep for a model
# we do not run. Operator, 2026-09-10: "only things like the APK need that kind
# of versioning for their update process."
#
# There is no :main either. :latest tracks main's tip with no gate between them
# (rule 147), so a second name for the same image sends readers looking for a
# distinction that does not exist.
#
# The dev channel exists so testing a build does not require shipping one.
# Before it, the only way to get an APK onto a phone was to cut a release,
# which made `main` the staging area by default. `:dev` carries its own
# freshly-built APK, signed with the SAME key as release builds — a different
# key cannot install over the stable app, so anyone crossing channels would
# have to uninstall and lose their data.
#
# :dev is published ALONE, with no per-commit tag. A rolling channel is
# rolling by definition; a commit-addressable image for it would be a
# rollback target nobody ever pulls, kept forever. Recovery on dev is to fix
# forward.
#
# Note what this repo does NOT need: a cross-repo dispatch to refresh the
# channel when its bundled APK is rebuilt. That mechanism exists elsewhere in
# the family because the app and the server live in separate repos. Minstrel
# is a monorepo — one push builds the APK and the image in the same run from
# the same commit, so the channel cannot go stale against its own artifact.
# The requirement is satisfied structurally; copying the mechanism would add
# a moving part to fix a problem that does not exist here.
#
# Release model: the tag IS the artifact's version name with a `v` in front.
# `v2026.09.10.1432` and `2026.09.10.1432` are the same string, derived from
# the tagged commit's UTC timestamp — so there is no mismatch to reconcile
# between what the tag says and what the APK reports, and nothing to look up
# when minting one.
#
# TAGS ARE IMMUTABLE. Never move, retarget or delete a published tag. A
# same-day second release is not a collision — HHMM makes every tag unique
# by construction, so the answer is simply another tag.
#
# This block used to say the opposite: that the per-day tag was
# "intentionally mutable" and that a same-day re-cut should
# `git push -f origin vYYYY.MM.DD`. That instruction is what the family
# rulebook now forbids outright, and it has incidents behind it — moving a
# same-day tag forward once took a published release down with it. Anyone
# installing from a tag is holding something the tag no longer points at,
# which is a worse failure than an extra row in the tag list.
#
# :latest is updated by every main push AND every tag push, so it always
# reflects the newest blessed image.
# Release model: per-day CalVer tags (no trailing patch digit). The day's
# tag is intentionally mutable — if a second release happens the same day,
# move the tag with `git push -f origin vYYYY.MM.DD` and the image tag of
# the same name gets overwritten. :latest is updated by every main push
# AND every tag push, so it always reflects the newest blessed image.
#
# APK pipeline: on tag pushes the android-release job builds + signs the
# Android APK and uploads it as a workflow artifact. The image-release
@@ -80,37 +24,33 @@ name: release
# :latest (not just tags), a main build with no APK would silently strip
# the in-app update channel off :latest until the next release. So on
# non-tag builds image-release pulls the MOST RECENT release's signed APK
# AND the version sidecar published beside it — the recorded values, not
# recomputed ones — so no rebuild is needed, just a rebundle. Tag builds
# keep bundling their own freshly-built APK.
# and reconstructs its exact versionName (tag + commit-count, the same
# formula android-release bakes in) for the version sidecar — no rebuild,
# just rebundle. Tag builds keep bundling their own freshly-built APK.
#
# Android testing (lint + detekt + unit tests, debug APK upload on main)
# lives in android.yml and runs independently on every push.
on:
push:
branches: [main, dev]
branches: [main]
tags: ['v*']
paths-ignore:
- 'docs/**'
- '**/*.md'
workflow_dispatch:
# A rapid re-push to main should supersede the in-flight build — the
# operator explicitly wants the later commit to win. Tags no longer enter
# into this: they are immutable and unique, so no tag build can ever be
# superseded by another run on the same ref.
# Force-moving the per-day tag (or rapidly re-pushing to main) should
# supersede the in-flight build — the operator explicitly wants the
# later commit to win.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
jobs:
android-release:
name: Build signed APK (releases and dev)
# Also builds on `dev`, which is what makes a test channel possible at
# all. Without it the only way to get a build onto a phone was to cut a
# release, which quietly turns `main` into the staging area.
if: startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev'
name: Build signed APK (tag releases only)
if: startsWith(github.ref, 'refs/tags/v')
runs-on: flutter-ci
container:
image: git.fabledsword.com/bvandeusen/ci-android:36
@@ -135,18 +75,14 @@ jobs:
outputs:
version_name: ${{ steps.ver.outputs.name }}
version_code: ${{ steps.ver.outputs.code }}
channel: ${{ steps.ver.outputs.channel }}
steps:
- name: Checkout
uses: actions/checkout@v4
with:
# Full history. The version name now reads only the tip commit's
# timestamp, so a shallow clone would technically serve — but this
# job derives a value that ships to devices, and a shallow checkout
# changes what git-derived values resolve to WITHOUT failing. The
# whole failure class here is a green build carrying a wrong
# version, so the cheap guarantee is worth keeping.
# fetch-depth: 0 retrieves full history; default shallow clone
# would return 1 for `git rev-list --count HEAD`, breaking the
# iteration suffix.
fetch-depth: 0
- name: Compute release version
@@ -155,23 +91,12 @@ jobs:
working-directory: ${{ github.workspace }}
run: |
set -euo pipefail
# The derivation lives in ci/version.sh, not here, so it can be
# executed by a test on every push. Anything inline in this file is
# unverifiable until a release is already running.
out="$(ci/version.sh HEAD)"
printf '%s\n' "${out}" >> "$GITHUB_OUTPUT"
# The channel is a property of the LANE, not of the commit, which is
# why it is derived here rather than in version.sh. Same commit built
# on dev and on main reports the same NAME and differs only here —
# that is the whole point of separating the two values.
if [ "${GITHUB_REF}" = "refs/heads/dev" ]; then
channel=dev
else
channel=stable
fi
echo "channel=${channel}" >> "$GITHUB_OUTPUT"
echo "::notice::APK $(printf '%s' "${out}" | tr '\n' ' ') channel=${channel}"
TAG="${GITHUB_REF#refs/tags/v}"
COMMIT_COUNT=$(git rev-list --count HEAD)
VERSION_NAME="${TAG}.${COMMIT_COUNT}"
echo "name=${VERSION_NAME}" >> "$GITHUB_OUTPUT"
echo "code=${COMMIT_COUNT}" >> "$GITHUB_OUTPUT"
echo "::notice::APK version: ${VERSION_NAME} (code=${COMMIT_COUNT})"
# Checked BEFORE the expensive work, not after it. "Attach APK to gitea
# Release" below resolves the release by tag and fails if it is absent —
@@ -183,7 +108,6 @@ jobs:
# the release together, so this passes). A bare `git push origin vX` is the
# case this catches.
- name: Release must exist for this tag
if: startsWith(github.ref, 'refs/tags/v')
shell: bash
working-directory: ${{ github.workspace }}
env:
@@ -232,12 +156,13 @@ jobs:
-PMINSTREL_VERSION_CODE=${{ steps.ver.outputs.code }}
- name: Upload APK as workflow artifact
# Stock action (snippet #2271) — never @v3, which uploads something Gitea
# will never serve back. This is the producing half of a pair:
# image-release downloads `minstrel-apk` below. Any upload v4+ pairs with
# any download v4+ on this forge (every combination tested 2026-09-10,
# Scribe spike #3843), so the two pins need not move together.
uses: actions/upload-artifact@v7
# Mirrored action, never actions/upload-artifact — @v4+ refuses on the
# hostname, @v3 uploads something Gitea will never serve back. This is
# the producing half of a pair: image-release downloads `minstrel-apk`
# below with the matching download-artifact mirror. Both must stay on
# the v4 protocol — mixing a v3 upload with a v4 download (or the
# reverse) yields an empty listing, not an error. See Scribe 2255 / 2270.
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
with:
name: minstrel-apk
path: android/app/build/outputs/apk/release/app-release.apk
@@ -246,15 +171,9 @@ jobs:
if-no-files-found: error
- name: Attach APK to gitea Release
# Tag releases only. A dev build has no Release to hang assets on and
# does not need one — the :dev image bundles the APK, and the server
# serves it from /api/client/apk like any other.
if: startsWith(github.ref, 'refs/tags/v')
shell: bash
env:
CI_TOKEN: ${{ secrets.CI_TOKEN }}
VERSION_NAME: ${{ steps.ver.outputs.name }}
VERSION_CODE: ${{ steps.ver.outputs.code }}
run: |
set -euxo pipefail
TAG="${GITHUB_REF#refs/tags/}"
@@ -262,20 +181,6 @@ jobs:
APK_PATH="app/build/outputs/apk/release/app-release.apk"
ls -lh "${APK_PATH}"
# Publish the version sidecar as a release asset next to the APK.
#
# This is what lets a later :latest build stop RECONSTRUCTING the
# bundled APK's version and simply read what was recorded. The
# ordering key in particular cannot be re-derived after the fact —
# it is build-time minutes, so once this job ends the value exists
# nowhere else. Reconstruction could only ever recover the name,
# and only by duplicating a formula that then has to be kept in
# step across two files.
SIDECAR_PATH="/tmp/minstrel.apk.version"
printf '{"name":"%s","code":%s,"channel":"stable"}\n' \
"${VERSION_NAME}" "${VERSION_CODE}" > "${SIDECAR_PATH}"
cat "${SIDECAR_PATH}"
RELEASE_JSON="$(curl -fsSL \
-H "Authorization: token ${CI_TOKEN}" \
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}")"
@@ -297,20 +202,6 @@ jobs:
exit 1
fi
# Same treatment for the sidecar. Named `.apk.version` so the
# downloader's `\.apk$` match cannot pick it up by mistake.
SIDECAR_HTTP=$(curl -sS -L -o /tmp/upload-sidecar.out -w '%{http_code}' \
-H "Authorization: token ${CI_TOKEN}" \
-F "attachment=@${SIDECAR_PATH}" \
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=minstrel-${TAG}.apk.version")
echo "sidecar_upload_http=${SIDECAR_HTTP}"
cat /tmp/upload-sidecar.out || true
echo
if [ "${SIDECAR_HTTP}" -lt 200 ] || [ "${SIDECAR_HTTP}" -ge 300 ]; then
echo "::error::version sidecar upload returned HTTP ${SIDECAR_HTTP}"
exit 1
fi
image-release:
name: Build + push container image
# `needs:` waits for android-release. For tag pushes android-release
@@ -331,16 +222,11 @@ jobs:
- name: Checkout
uses: actions/checkout@v4
with:
# Full history, and rule 149 names this specifically: any job that
# DERIVES the version name needs it, because a shallow clone changes
# what git-derived values resolve to WITHOUT failing — a too-low
# value, silently, with every lane green.
#
# This job was depth-1 while it took the version from GITHUB_REF. It
# now runs ci/version.sh itself, because with :<version> image tags
# gone the server's self-reported version is the only thing that says
# which build an image is.
# Full history + tags so non-tag :latest builds can resolve the
# latest release tag's commit count and reconstruct the bundled
# APK's exact versionName (see "Bundle latest release APK" below).
fetch-depth: 0
fetch-tags: true
- name: Detect buildable project
id: guard
@@ -358,68 +244,21 @@ jobs:
if: steps.guard.outputs.ready == 'true'
shell: bash
run: |
set -euo pipefail
# THE VERSION, and it is derived the same way on every ref — the
# branch decides the CHANNEL, never the version (family rule 149).
#
# This used to be three different things: the literal string "main"
# on main, "dev" on dev, and the tag name on a tag. None of them
# ordered, and the first two were the same string forever — two dev
# images eight weeks apart were indistinguishable in the UI. That
# mattered little while :vYYYY.MM.DD.HHMM existed to identify a
# build; with version image tags gone, this IS how an operator tells
# which build a container is running.
#
# `sed -n s///p` rather than `grep`: it exits 0 when nothing matches,
# so the empty check below is actually reachable. A grep here would
# kill the step at the assignment under the runner's pipefail — the
# exact bug that took down the first main build after the version
# rework.
VERSION="$(ci/version.sh HEAD | sed -n 's/^name=//p')"
if [ -z "${VERSION}" ]; then
echo "::error::could not derive a build version from ci/version.sh"
exit 1
fi
if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then
# A release refreshes the CHANNEL and mints nothing else.
#
# The tag build exists to produce the signed APK and attach it to
# the release; the image it rebuilds is the SAME SOURCE as the main
# build minutes earlier, differing only in which APK is baked in.
# Rule 145 is explicit about that case: when the same source is
# rebuilt with different contents, publish the moving channel tag
# and never a commit-addressable one.
#
# :latest must move here rather than waiting for the next main
# push, or the channel would carry the PREVIOUS release's APK
# indefinitely — a channel that cannot refresh itself (rule 146).
CHANNEL=stable
echo "args=-t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
echo "::notice::Release build ${VERSION}: refreshing :latest around the new APK"
elif [[ "${GITHUB_REF}" == "refs/heads/dev" ]]; then
# The rolling test channel, and :dev ALONE — deliberately no
# per-commit tag. A rolling channel is rolling by definition, so a
# commit-addressable image here would be a rollback target nobody
# has ever pulled, accumulating in the registry forever. Recovery
# on dev is to fix forward.
CHANNEL=dev
echo "args=-t ${IMAGE}:dev" >> "$GITHUB_OUTPUT"
echo "::notice::Dev-branch build ${VERSION}: :dev"
VERSION="${GITHUB_REF#refs/tags/}"
echo "args=-t ${IMAGE}:${VERSION} -t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
echo "::notice::Release build: ${VERSION} + latest"
else
# The production line: :latest tracks main's tip (rule 147) and
# :<sha> is the rollback unit (rule 145). Full 40-char SHA, matching
# the family's other repos, so a rollback target is addressable
# straight from the commit anyone is reading.
CHANNEL=stable
echo "args=-t ${IMAGE}:latest -t ${IMAGE}:${GITHUB_SHA}" >> "$GITHUB_OUTPUT"
echo "::notice::Main-branch build ${VERSION}: :latest + :${GITHUB_SHA}"
# Main is the protected, post-PR-merge branch. Treat it as the
# rolling stable channel — every main push moves :latest.
# Pinned consumers can target :vYYYY.MM.DD; everyone else
# gets the newest main.
echo "args=-t ${IMAGE}:main -t ${IMAGE}:latest" >> "$GITHUB_OUTPUT"
echo "version=main" >> "$GITHUB_OUTPUT"
echo "::notice::Main-branch build: :main + :latest"
fi
echo "version=${VERSION}" >> "$GITHUB_OUTPUT"
echo "channel=${CHANNEL}" >> "$GITHUB_OUTPUT"
- name: Registry login
if: steps.guard.outputs.ready == 'true'
shell: bash
@@ -428,57 +267,54 @@ jobs:
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
- name: Download signed APK artifact
# Tag and dev pushes — android-release just produced this. Only `main`
# takes the "Bundle latest release APK" path below, because it is the
# one ref that moves a channel without building an APK of its own.
if: >-
steps.guard.outputs.ready == 'true' &&
(startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev')
# Consuming half of the pair: stock download-artifact, which works here for
# the same reason as the upload (gitea/runner 3.x edits the GHES refusal
# out of the bundle; snippet #2271). v8 runs on node24, which every
# CI-runner image carries — the runner uses the image's own node.
uses: actions/download-artifact@v8
# Tag pushes only — android-release just produced this. Non-tag
# builds take the "Bundle latest release APK" path below instead.
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
# Consuming half of the pair — never actions/download-artifact. Same fork,
# same reason: upstream's client-side GHES check rejects this hostname
# before it connects. bvandeusen/download-artifact mirrors
# code.forgejo.org/forgejo/download-artifact.
#
# SHA below is that fork's `v6` tag. Match on @actions/artifact, NOT on
# the action's own version number — the two actions release on unrelated
# cadences, and download v5 would pair a ^2.3.2 client with this file's
# ^4.0.0 uploader. v6 is the tag whose bundled library major (^4.0.0) is
# the same one proven against this instance by the upload side.
# Deliberately NOT v7: it moves to node24 and upstream requires runner
# >= 2.327.1 for it, which act_runner does not claim to satisfy.
# Pinned, not tagged — the mirror auto-syncs every 8h.
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
with:
name: minstrel-apk
path: client/
- name: Stage bundled APK + version sidecar
if: >-
steps.guard.outputs.ready == 'true' &&
(startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/dev')
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
shell: bash
env:
# All three pulled from android-release's outputs so the sidecar the
# server hands clients matches exactly what is baked into the APK
# they are comparing against.
# Pulled from android-release.outputs.version_name so the
# sidecar string the server hands clients matches the
# versionName baked into the APK they're comparing against.
APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }}
APK_VERSION_CODE: ${{ needs.android-release.outputs.version_code }}
APK_CHANNEL: ${{ needs.android-release.outputs.channel }}
run: |
set -euxo pipefail
# The artifact lands as `app-release.apk` (the original Gradle
# output name). The Dockerfile COPYs client/* into /app/client/
# and the server reads minstrel.apk + minstrel.apk.version.
mv client/app-release.apk client/minstrel.apk
printf '{"name":"%s","code":%s,"channel":"%s"}\n' \
"${APK_VERSION_NAME}" "${APK_VERSION_CODE}" "${APK_CHANNEL}" \
> client/minstrel.apk.version
cat client/minstrel.apk.version
echo "${APK_VERSION_NAME}" > client/minstrel.apk.version
ls -lh client/
- name: Bundle latest release APK (non-tag :latest builds)
# Main pushes don't build an APK, but they DO move :latest — so
# without this the in-app update channel would vanish from :latest
# until the next tag. Pull the most-recent release's signed APK and
# the sidecar published beside it, so what the server reports is what
# that build actually recorded rather than something re-derived here.
# reconstruct its exact versionName (${TAG#v}.$(git rev-list --count
# TAG) — identical to android-release's formula) so the version
# sidecar the server hands clients matches the installed build.
# Degrades to an empty client/ (404 update channel) — never a wrong
# version — if no release or APK asset can be resolved. That
# degradation only actually works because the greps below carry
# `|| true`; under the runner's default pipefail a non-matching grep
# kills the step instead of falling through to the empty-case branch.
if: steps.guard.outputs.ready == 'true' && github.ref == 'refs/heads/main'
# version — if no release / APK asset / tag-count can be resolved.
if: steps.guard.outputs.ready == 'true' && !startsWith(github.ref, 'refs/tags/v')
shell: bash
env:
CI_TOKEN: ${{ secrets.CI_TOKEN }}
@@ -490,40 +326,19 @@ jobs:
if [ -z "${REL_JSON}" ]; then
echo "::notice::no published release — image ships without bundled APK"; exit 0
fi
# `|| true` on every one of these, and it is load-bearing rather
# than defensive habit. The runner already invokes this shell as
# `bash -e -o pipefail`, so a pipeline whose grep matches NOTHING
# exits non-zero even though `head` succeeded — and the step dies at
# the assignment, before ever reaching the `if` written to handle the
# empty case. Every "degrades gracefully" branch below is unreachable
# without this.
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)" || true
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)" || true
TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)"
APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)"
if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then
echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0
fi
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}"
# Take the version the release RECORDED rather than recomputing it.
# This used to re-derive the name from the tagged commit, which meant
# the formula lived in two files that had to be kept in step, and it
# could only ever recover the name — the ordering key is build-time
# minutes and does not exist anywhere after that build ends.
SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)" || true
if [ -n "${SIDECAR_URL}" ]; then
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk.version "${SIDECAR_URL}"
cat client/minstrel.apk.version
else
# Releases published before sidecars were attached. Their name is
# still recoverable from the tag, but their ordering key genuinely
# is not — so it is reported ABSENT rather than guessed. A wrong
# key is an install the platform refuses; an absent one just tells
# the client to fall back to comparing names, which is exactly
# what those builds already do.
echo "::notice::release ${TAG} predates the version sidecar — bundling with name only, no ordering key"
printf '{"name":"%s","code":null,"channel":"stable"}\n' "${TAG#v}" > client/minstrel.apk.version
COUNT="$(git rev-list --count "${TAG}" 2>/dev/null || true)"
if [ -z "${COUNT}" ]; then
echo "::notice::could not resolve commit count for ${TAG} (tag not fetched?) — skipping APK bundle"; exit 0
fi
echo "::notice::bundled release APK from ${TAG}"
VERSION_NAME="${TAG#v}.${COUNT}"
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}"
echo "${VERSION_NAME}" > client/minstrel.apk.version
echo "::notice::bundled release APK ${TAG} as version ${VERSION_NAME}"
ls -lh client/
- name: Build and push
@@ -531,7 +346,6 @@ jobs:
run: |
docker buildx build \
--build-arg MINSTREL_VERSION="${{ steps.tags.outputs.version }}" \
--build-arg MINSTREL_CHANNEL="${{ steps.tags.outputs.channel }}" \
--push ${{ steps.tags.outputs.args }} .
# Verifies a tag release actually ended up complete, and names the specific
@@ -542,8 +356,8 @@ jobs:
# `failure` with none executed and image-release showed `skipped`. The run was
# red, but the *release page rendered fine*, and `main`'s own push build had
# already moved `:latest`, so the code was deployable and nothing looked
# obviously wrong. The release was simply missing its APK and its image,
# which is easy to skim past.
# obviously wrong. The release was simply missing its APK and its immutable
# `:vYYYY.MM.DD` image, which is easy to skim past.
#
# This job cannot prevent that (the cause was a runner failing to launch, not
# anything in this file). What it does is turn an incomplete release into an
@@ -594,30 +408,18 @@ jobs:
# missing when v2026.08.07 had to be re-cut. `always()` on this job means
# it runs even when image-release failed, so without this the guard would
# cheerfully verify an incomplete release.
#
# This asserted `:${TAG}` — the :vYYYY.MM.DD.HHMM image — until
# 2026-09-10. Version image tags are no longer published (rule 145), so
# that assertion would now fail every release for a tag nothing mints.
# The rollback target it was really protecting is the :<sha> image, which
# main's own build published for this same commit before the tag was cut.
#
# Checking it here earns its keep twice over: it still catches an image
# push that silently did not happen, and it additionally proves the
# ORDERING — a tag cut on a commit whose main build never completed has
# no rollback target, and that is worth failing on rather than
# discovering during an incident.
- name: Rollback image must exist for the tagged commit
- name: Immutable image tag must exist
shell: bash
run: |
set -euo pipefail
TAG="${GITHUB_REF#refs/tags/}"
IMAGE="git.fabledsword.com/bvandeusen/minstrel"
echo "${{ secrets.CI_TOKEN }}" \
| docker login git.fabledsword.com -u "${{ github.actor }}" --password-stdin
if ! docker manifest inspect "${IMAGE}:${GITHUB_SHA}" > /dev/null 2>&1; then
echo "::error::image ${IMAGE}:${GITHUB_SHA} does not exist — this commit has no rollback target."
echo "::error::That image is published by the MAIN build of this commit, not by the tag build. If main's build never ran or failed, fix that first; a release whose commit cannot be rolled back to is the thing this check exists to refuse."
if ! docker manifest inspect "${IMAGE}:${TAG}" > /dev/null 2>&1; then
echo "::error::image ${IMAGE}:${TAG} was never pushed — the release tag has no immutable image, so there is nothing to pin or roll back to. Re-run this workflow run."
exit 1
fi
echo "::notice::rollback target verified: ${IMAGE}:${GITHUB_SHA}"
echo "::notice::image verified: ${IMAGE}:${TAG}"
-6
View File
@@ -32,12 +32,6 @@ on:
- 'cmd/**'
- '.golangci.yml'
- '.gitea/workflows/test-go.yml'
# The release lane's own trigger is `main` + tags, so nothing it
# contains is exercised until a release is already running. These two
# entries are what let internal/server/release_version_test.go guard
# the version derivation on ordinary dev pushes instead.
- 'ci/**'
- '.gitea/workflows/release.yml'
# pull_request trigger intentionally omitted — see test-web.yml for
# the rationale (single-author repo, push covers PR-merge equivalent).
-5
View File
@@ -12,11 +12,6 @@
# Test binary, built with `go test -c`
*.test
# `make build` output. bin/minstrel was tracked until 2026-09-10 — an 18 MB
# binary committed by accident, last refreshed by a commit about web test
# mocks, and re-dirtied by every local build since.
bin/
# Bundled Android APK + version sidecar (#397). Populated by CI for
# tag releases; never committed. README in client/ explains the flow.
client/minstrel.apk
+5 -20
View File
@@ -15,32 +15,17 @@ COPY . .
# Overwrite the committed placeholder with the freshly-built SPA assets.
COPY --from=web /web/build ./web/build
ENV CGO_ENABLED=0
# Version stamping. release.yml passes the DERIVED version name
# (YYYY.MM.DD.HHMM) and the lane's channel; a local `docker build` falls back
# to "dev"/"local". Both are surfaced at /healthz.
#
# These are two values on purpose (family rule 149): the same commit built on
# dev and on main reports the same NAME and differs only in CHANNEL. Folding
# the channel into the version string is what the rule forbids — the version
# used to BE the channel word here ("main"/"dev"), which meant two dev images
# eight weeks apart were indistinguishable.
# Version stamping: release.yml passes the git tag via MINSTREL_VERSION
# build-arg; local `docker build` falls back to "dev". Surfaced at
# /healthz for operator-side image-version verification.
ARG MINSTREL_VERSION=dev
ARG MINSTREL_CHANNEL=local
RUN go build -trimpath \
-ldflags="-s -w \
-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=${MINSTREL_VERSION}' \
-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerChannel=${MINSTREL_CHANNEL}'" \
-ldflags="-s -w -X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=${MINSTREL_VERSION}'" \
-o /out/minstrel ./cmd/minstrel
FROM debian:bookworm-slim
# ffmpeg: duration probes and the exact-tier audio hash (a SHA-256 of the
# encoded audio packets, so no decode). libchromaprint-tools: fpcalc, the
# acoustic fingerprint that tells the same recording at two bitrates apart
# from two different recordings (M400). Both are baked in at build time so a
# deployed instance never fetches either (rule 164); fpcalc is shelled out
# rather than bound because CGO_ENABLED=0 above rules out cgo.
RUN apt-get update \
&& apt-get install -y --no-install-recommends ca-certificates ffmpeg libchromaprint-tools \
&& apt-get install -y --no-install-recommends ca-certificates ffmpeg \
&& rm -rf /var/lib/apt/lists/*
RUN groupadd --system --gid 1000 minstrel \
+8 -22
View File
@@ -37,12 +37,8 @@ services:
ports: ['4533:4533']
volumes:
# Your music library. Point ./music at wherever your audio files
# live. Writable, because Minstrel deletes a file when an admin asks
# it to (for example, quarantine's "Delete file"). It never moves,
# renames or retags anything. The container runs as uid 1000, so that
# user needs write access to the folders. Mount it :ro to forbid even
# deletes: those actions then refuse, say why, and delete nothing.
- ./music:/music
# live. Mounted read-only — Minstrel never writes to your library.
- ./music:/music:ro
# Generated data: playlist cover collages, artist art, caches.
# The path must match MINSTREL_STORAGE_DATA_DIR, which the image
# sets to /app/data — keep this mount on /app/data or your cache
@@ -51,7 +47,7 @@ services:
environment:
MINSTREL_DATABASE_URL: postgres://minstrel:minstrel@db:5432/minstrel?sslmode=disable
# Colon-separated library roots to scan; must match the container
# path of the music mount above (/music here).
# path of the read-only music mount above (/music here).
MINSTREL_LIBRARY_SCAN_PATHS: /music
depends_on: [db]
@@ -116,21 +112,11 @@ Most operational keys have a `MINSTREL_<SECTION>_<FIELD>` env override. Recommen
Image tags (`git.fabledsword.com/bvandeusen/minstrel:<tag>`):
- `:latest`production. Tracks `main`'s tip and moves on every `main` push and every release. What most operators should run.
- `:<commit-sha>` — the rollback unit. Every `main` push publishes one, so any production commit is addressable without a release ceremony. Immutable: a given SHA tag is never re-pushed. Pin one if you need a deployment that cannot change under you, and use it to roll back.
- `:dev` — the rolling test channel, rebuilt on every push to `dev` and carrying its own freshly-built Android APK. Run this to try something before it ships. It moves constantly, has no per-commit tag, and its only recovery path is forward — if a `:dev` image is broken, the fix is the next push, not a rollback.
- `:latest`the newest blessed image. Moves on every `main` push **and** every release. Recommended for most operators.
- `:vYYYY.MM.DD` — immutable per-day release tags. Pin one of these for a deployment you don't want moving under you. (Per-day CalVer — no trailing patch digit; a same-day re-cut moves the tag forward.)
- `:main` — the rolling post-merge tip. Same image as `:latest` at push time; choose it if you want to track `main` explicitly rather than the release line.
That is the whole tag map. **There are no version-numbered image tags**, and no `:main`. Git and the build's own self-reported version answer "which build is this" — the Settings page shows it, and so does `/healthz`. Release *tags* in git are still `vYYYY.MM.DD.HHMM`; they name a changelog entry and the APK attached to it, not an image.
Rolling back to `:<commit-sha>` pins the **server code** at that commit — not the server-and-app pair. The Android APK is baked in at image build time, so a SHA image carries whichever app was current when that commit was built, which may be older than what `:latest` bundles now. If both halves matter, check what the image bundles rather than trusting the tag's name.
Every `:latest`, `:<commit-sha>` and `:dev` bundles a signed Android APK, so the in-app update channel is always live. All are signed with the same key, so a phone can move between the stable and dev channels without uninstalling — point it at a `:dev` server and the in-app updater offers that channel's build.
The app reports which channel it is on alongside its version, and decides whether an update is available using the build's ordering key rather than its displayed name — the same value Android installs by, so an offer it makes is one the platform will accept.
Database migrations run automatically at startup; rollbacks require restoring a Postgres dump.
Releases up to 2026-09-10 also published a `:vYYYY.MM.DD[.HHMM]` image tag. Those images still exist and still work — they are simply not extended.
Every `:latest` and every `:vYYYY.MM.DD` bundles the current signed Android APK, so the in-app update channel is always live. Database migrations run automatically at startup; rollbacks require restoring a Postgres dump.
## Specs
@@ -164,7 +150,7 @@ Two concurrent dev processes:
- Day-to-day work happens on `dev` (or feature branches merged into `dev`).
- `main` is **protected** — changes land via PR from `dev`.
- Releases are cut by tagging `v*` off `main`; the release workflow builds the signed APK, attaches it to the release, and refreshes `:latest` around it.
- Releases are cut by tagging `v*` off `main`; the release workflow builds and pushes the container image to the Gitea registry.
Task and milestone tracking: Fable (`Minstrel` project, id 12).
+8 -18
View File
@@ -21,24 +21,13 @@ android {
applicationId = "com.fabledsword.minstrel"
minSdk = 26
targetSdk = 36
// versionName / versionCode are released-build values injected by CI.
// Local / debug builds fall back to "dev" so the About card reads
// honestly.
//
// versionName is "YYYY.MM.DD.HHMM" from the COMMIT's timestamp, so
// every lane building this source reports the same string and the
// channel is the only thing that differs between them.
//
// versionCode is minutes since 2020-01-01 at BUILD time. It is the
// value the platform decides installs by, so it must be monotonic by
// construction.
//
// This comment used to say versionCode was a commit count and that it
// was "monotonic forever". It was neither — a commit count runs ahead
// on `dev`, so a dev build outranked the `main` release meant to
// replace it and Android refused the install as a downgrade. Worth
// knowing the claim was here, stated as a reassurance, while the bug
// it denied was live.
// versionName / versionCode are released-build values injected by
// CI from the git tag + commit count. Local / debug builds fall
// back to "dev" so the About card reads honestly. Releases ship
// versionName="YYYY.MM.DD.<commits>" (e.g. "2026.06.02.142") and
// versionCode=<commits>, which is monotonic forever and lets the
// shared isVersionNewer comparator distinguish two same-day
// re-cuts (the iteration suffix differs).
val versionNameOverride =
(project.findProperty("MINSTREL_VERSION_NAME") as String?)?.takeIf { it.isNotBlank() }
val versionCodeOverride =
@@ -161,6 +150,7 @@ dependencies {
implementation(libs.compose.ui)
implementation(libs.compose.ui.graphics)
implementation(libs.compose.material3)
implementation(libs.compose.ui.text.google.fonts)
debugImplementation(libs.compose.ui.tooling)
implementation(libs.compose.ui.tooling.preview)
@@ -15,14 +15,10 @@ import androidx.compose.material3.HorizontalDivider
import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.OutlinedButton
import androidx.compose.material3.Scaffold
import androidx.compose.material3.SnackbarHost
import androidx.compose.material3.SnackbarHostState
import androidx.compose.material3.Text
import androidx.compose.material3.TextButton
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.remember
import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.text.style.TextOverflow
@@ -46,12 +42,6 @@ fun AdminQuarantineScreen(
viewModel: AdminQuarantineViewModel = hiltViewModel(),
) {
val state by viewModel.uiState.collectAsStateWithLifecycle()
val snackbarHostState = remember { SnackbarHostState() }
LaunchedEffect(Unit) {
viewModel.transientMessages.collect { msg ->
snackbarHostState.showSnackbar(msg)
}
}
Scaffold(
contentWindowInsets = ShellContentWindowInsets,
modifier = Modifier.fillMaxSize(),
@@ -63,7 +53,6 @@ fun AdminQuarantineScreen(
onBack = { navController.popBackStack() },
)
},
snackbarHost = { SnackbarHost(snackbarHostState) },
) { inner ->
PullToRefreshScaffold(
onRefresh = { viewModel.refresh().join() },
@@ -10,13 +10,10 @@ import com.fabledsword.minstrel.events.EventsStream
import com.fabledsword.minstrel.models.AdminQuarantineItemRef
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.Job
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.receiveAsFlow
import kotlinx.coroutines.launch
import javax.inject.Inject
@@ -37,15 +34,6 @@ class AdminQuarantineViewModel @Inject constructor(
private val internal = MutableStateFlow<AdminQuarantineUiState>(AdminQuarantineUiState.Loading)
val uiState: StateFlow<AdminQuarantineUiState> = internal.asStateFlow()
/**
* One-shot messages for the screen's snackbar. A failed action has to say
* why: the row quietly reappearing reads as a glitch, and for a Delete
* file refused by a read-only library it hides the one thing the
* operator can fix (#3918).
*/
private val transientMessagesChannel = Channel<String>(Channel.BUFFERED)
val transientMessages: Flow<String> = transientMessagesChannel.receiveAsFlow()
init {
refresh()
viewModelScope.launch {
@@ -98,9 +86,8 @@ class AdminQuarantineViewModel @Inject constructor(
try {
action(trackId)
} catch (
@Suppress("TooGenericExceptionCaught") e: Throwable,
@Suppress("TooGenericExceptionCaught", "SwallowedException") e: Throwable,
) {
transientMessagesChannel.trySend(ErrorCopy.fromThrowable(e))
refresh()
}
}
@@ -37,35 +37,18 @@ object ErrorCopy {
* as connection failures.
*/
fun fromThrowable(t: Throwable): String = when (t) {
is HttpException -> fromHttp(t)
is HttpException -> messageFor(codeFromHttp(t))
is IOException -> messageFor("connection_refused")
else -> TABLE.getValue("unknown")
}
/**
* Codes whose server message carries specifics the operator needs in
* order to act — which directory, which uid — that fixed copy cannot say.
* For these the message follows the copy (#3918). Kept to a named set on
* purpose: most server messages are internal detail. Mirrors web's
* errors.ts.
*/
private val DETAIL_CODES = setOf("library_not_writable", "file_delete_failed")
private fun fromHttp(e: HttpException): String {
val body = bodyFromHttp(e)
val copy = messageFor(body.code.ifEmpty { "unknown" })
return if (body.code in DETAIL_CODES && body.message.isNotBlank()) {
"$copy ${body.message}"
} else {
copy
}
}
private fun bodyFromHttp(e: HttpException): Body {
private fun codeFromHttp(e: HttpException): String {
val raw = runCatching { e.response()?.errorBody()?.string() }.getOrNull()
?: return Body()
return runCatching { json.decodeFromString<Envelope>(raw).error }
.getOrNull() ?: Body()
?: return "unknown"
val code = runCatching { json.decodeFromString<Envelope>(raw).error?.code }
.getOrNull()
.orEmpty()
return code.ifEmpty { "unknown" }
}
private val TABLE: Map<String, String> = mapOf(
@@ -116,8 +99,6 @@ object ErrorCopy {
"request_not_pending" to "This request is no longer pending.",
"request_not_found" to "That request no longer exists.",
"track_not_found" to "That track no longer exists.",
"library_not_writable" to "The music library isn't writable by the server.",
"file_delete_failed" to "The file couldn't be deleted.",
"album_not_found" to "That album no longer exists.",
"artist_not_found" to "That artist no longer exists.",
"playlist_not_found" to "That playlist no longer exists.",
@@ -18,7 +18,6 @@ import com.fabledsword.minstrel.connectivity.NetworkStatusController
import com.fabledsword.minstrel.di.ApplicationScope
import com.fabledsword.minstrel.player.PlayerController
import com.fabledsword.minstrel.player.RemotePlayerState
import com.fabledsword.minstrel.player.TransportObservation
import com.fabledsword.minstrel.player.output.OutputPickerController
import com.fabledsword.minstrel.player.output.OutputRoute
import dagger.hilt.android.qualifiers.ApplicationContext
@@ -104,7 +103,6 @@ class DiagnosticsReporter @Inject constructor(
launch { collectUpnpDrops() }
launch { collectPlayerState() }
launch { collectTrackChanges() }
launch { collectTransportFlap() }
launch { collectRoutes() }
launch { heartbeatLoop() }
}
@@ -193,67 +191,6 @@ class DiagnosticsReporter @Inject constructor(
}
}
/**
* Catch the renderer rapidly leaving and re-entering PLAYING.
*
* The operator reports the Sonos "play pause play pause, like someone
* pressing it every half second", usually as a track starts, cleared by a
* manual pause or skip. Nothing here could see that: `player_state`
* carries source/loading/error but not playing, `track_change` needs the
* index to move, and the heartbeat samples once per 45s. The symptom fell
* through every existing collector, which is why it has only ever been
* described and never measured.
*
* Records every raw transport change (cheap — steady playback produces
* a couple per track) and, when they come in a burst, one summary event
* carrying the whole sequence. The summary is the useful artefact: it
* pairs the renderer's states with local-vs-Sonos track and position, so
* an episode says whether the app and the renderer disagreed about which
* track was playing, or agreed while the renderer rebuffered.
*
* See [TransportObservation] on the 1 Hz sampling limit.
*/
private suspend fun collectTransportFlap() {
val detector = TransportFlapDetector()
playerController.transportEvents.collect { obs ->
record("upnp_sync", buildJsonObject {
put("event", "transport")
put("state", obs.state)
put("status_ok", obs.statusOk)
put("sonos_track", obs.trackNumber)
put("sonos_pos_ms", obs.positionMs)
put("play_intent", obs.playIntent)
})
detector.onChange(obs)?.let { recordFlapSummary(it) }
}
}
private suspend fun recordFlapSummary(recent: List<TransportObservation>) {
val ui = playerController.uiState.value
val casting = outputPicker.routesState.value.current.protocol !=
OutputRoute.Protocol.SYSTEM
val spanMs = recent.last().atElapsedMs - recent.first().atElapsedMs
record("upnp_sync", buildJsonObject {
put("event", "transport_flap")
put("changes", recent.size)
put("window_ms", spanMs)
// The sequence itself, e.g. "PLAYING>TRANSITIONING>STOPPED>PLAYING".
// Whether STOPPED appears at all is the first question to ask of a
// captured episode.
put("sequence", recent.joinToString(">") { it.state })
put("sonos_positions_ms", recent.joinToString(",") { it.positionMs.toString() })
put("sonos_tracks", recent.joinToString(",") { it.trackNumber.toString() })
put("local_index", ui.queueIndex)
put("local_track_id", ui.currentTrack?.id ?: "")
put("local_pos_ms", ui.positionMs)
putSonos(this, casting)
put("upnp_loading", ui.isUpnpLoading)
put("server_health", networkStatus.state.value.name)
put("route", outputPicker.routesState.value.current.name)
addPowerFields(this)
})
}
private suspend fun collectRoutes() {
// 'playback' — route changes happen for all outputs. This only ever
// logs the ACTIVE route (routesState.current), so no "connected" flag.
@@ -1,75 +0,0 @@
package com.fabledsword.minstrel.diagnostics
import com.fabledsword.minstrel.player.TransportObservation
/**
* Decides when a run of renderer transport changes is a *flap* — the renderer
* repeatedly failing to settle — rather than an ordinary track transition.
*
* The operator reports the Sonos "play pause play pause, like someone pressing
* it every half second", usually as a track starts. No diagnostic event could
* see it, so it has been described several times and measured never. This is
* the rule that decides when an episode is worth writing down.
*
* Pure decision state, like [com.fabledsword.minstrel.player.RemoteStallWatchdog]:
* the caller owns the flow and the recording, this only answers "is this an
* episode, and which readings make it up". Keeps the windowing and the
* one-episode-one-summary rule testable without a renderer or a clock.
*/
class TransportFlapDetector(
private val windowMs: Long = FLAP_WINDOW_MS,
private val minChanges: Int = FLAP_MIN_CHANGES,
private val summaryCooldownMs: Long = FLAP_SUMMARY_COOLDOWN_MS,
) {
private val recent = ArrayDeque<TransportObservation>()
private var lastSummaryAtMs: Long? = null
/**
* Feed one transport change. Returns the readings making up an episode
* worth recording, or null when there is nothing to say.
*
* The returned list is a copy: the caller may hold it while more readings
* arrive.
*/
fun onChange(observation: TransportObservation): List<TransportObservation>? {
recent.addLast(observation)
dropReadingsOlderThan(observation.atElapsedMs)
if (!isEpisode(observation.atElapsedMs)) return null
lastSummaryAtMs = observation.atElapsedMs
return recent.toList()
}
private fun dropReadingsOlderThan(nowMs: Long) {
while (recent.isNotEmpty() && nowMs - recent.first().atElapsedMs > windowMs) {
recent.removeFirst()
}
}
/**
* Enough changes packed together, and far enough from the last thing we
* wrote down. The cooldown is what keeps one episode to one summary: a
* sustained fault produces a change every poll, and a summary per reading
* would bury the per-change events underneath them.
*/
private fun isEpisode(nowMs: Long): Boolean {
val since = lastSummaryAtMs
val cooled = since == null || nowMs - since >= summaryCooldownMs
return recent.size >= minChanges && cooled
}
/** Forget everything — call when the route changes or casting ends. */
fun reset() {
recent.clear()
lastSummaryAtMs = null
}
companion object {
// Readings arrive at the 1 Hz poll cadence, and a normal track
// transition is 2-3 changes (PLAYING -> TRANSITIONING -> PLAYING).
// Four inside six seconds is not a track change, and it is not a
// person at the Sonos app either; it is the renderer not settling.
const val FLAP_WINDOW_MS = 6_000L
const val FLAP_MIN_CHANGES = 4
const val FLAP_SUMMARY_COOLDOWN_MS = 60_000L
}
}
@@ -1,26 +1,15 @@
package com.fabledsword.minstrel.models
/**
* The server-bundled APK, as reported by `GET /api/client/version`.
* Wire shape returned by `GET /api/client/version`. Mirrors
* the Flutter client's `UpdateInfo`.
*
* Three values that are deliberately kept apart:
*
* - [version] is a LABEL for people — "YYYY.MM.DD.HHMM", derived from the
* build's commit, so two channels carrying the same code read the same.
* Display this; never decide on it when [code] is present.
* - [code] is the ORDERING KEY, and is the same value Android itself
* installs by. It answers "may this be installed over that?", which the
* name cannot. Null when the server predates the field.
* - [channel] is a SIBLING FIELD, never a suffix inside the name. Reported
* verbatim rather than validated, so an unexpected value is shown rather
* than dropped.
*
* [apkUrl] is server-relative (e.g. `/api/client/apk`).
* `version` is the server-bundled APK version (may have a leading
* "v" from the git tag); `apkUrl` is server-relative (e.g.
* `/api/client/apk`); `sizeBytes` is the download size.
*/
data class UpdateInfo(
val version: String,
val code: Long?,
val channel: String?,
val apkUrl: String,
val sizeBytes: Long,
)
@@ -4,26 +4,12 @@ import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
/**
* Wire shape for `GET /api/client/version`.
*
* `apkUrl` falls back to `/api/client/apk` if the server omits it.
*
* [code] MUST stay nullable, and this is not a style preference. The app's
* Json is configured with `coerceInputValues = true`, which replaces a JSON
* null with the declared default for a NON-nullable property — so writing
* `val code: Long = 0` would turn "this server reports no ordering key" into
* "this build's ordering key is 0", silently, with no error anywhere. A
* nullable type is what keeps absent distinguishable from zero, and the
* distinction is the whole reason the field exists.
*
* A server predating the ordering key sends neither [code] nor [channel];
* both arrive null and the caller falls back to comparing names.
* Wire shape for `GET /api/client/version`. Defaults match Flutter:
* apk_url falls back to `/api/client/apk` if the server omits it.
*/
@Serializable
data class UpdateInfoWire(
val version: String = "",
val code: Long? = null,
val channel: String? = null,
@SerialName("apk_url") val apkUrl: String = "/api/client/apk",
@SerialName("size_bytes") val sizeBytes: Long = 0,
)
@@ -14,7 +14,6 @@ import com.fabledsword.minstrel.connectivity.NetworkStatusController
import com.fabledsword.minstrel.connectivity.ServerHealth
import com.fabledsword.minstrel.player.output.ActiveUpnp
import com.fabledsword.minstrel.player.output.ActiveUpnpHolder
import com.fabledsword.minstrel.player.output.upnp.PositionInfo
import com.fabledsword.minstrel.player.output.upnp.SoapFaultException
import com.fabledsword.minstrel.player.output.upnp.TransportInfo
import com.fabledsword.minstrel.player.output.upnp.TransportState
@@ -49,12 +48,12 @@ import timber.log.Timber
*
* Drop heuristic: the 1 Hz poll loop is the *sole* arbiter of route
* liveness -- [RemotePlayerState.recordPollFailure]'s rolling threshold
* (DROP_THRESHOLD consecutive failures) fires [RemoteEvents.onDrop]. A failed transport
* (DROP_THRESHOLD consecutive failures) fires [onDrop]. A failed transport
* command (play/pause/seek/next) does NOT drop on its own: a locked phone's
* WiFi power-save can stall a single command's socket I/O for a second or
* two while the renderer is perfectly reachable, so commands retry on
* transient IO failure and otherwise defer to the poll loop. The factory
* wraps that callback into a SharedFlow consumed by the NowPlaying
* wraps the [onDrop] callback into a SharedFlow consumed by the NowPlaying
* surface as a snackbar.
*
* Queue mode: OutputPickerController loads the full queue into Sonos's
@@ -72,29 +71,10 @@ class MinstrelForwardingPlayer(
private val remoteState: RemotePlayerState,
private val castNetworkLock: CastNetworkLock,
private val networkStatus: NetworkStatusController,
private val events: RemoteEvents = RemoteEvents(),
private val onDrop: (routeName: String) -> Unit,
private val onStalled: (trackId: String) -> Unit = {},
) : ForwardingPlayer(delegate) {
/**
* The ways remote playback reports trouble outward. Grouped rather than
* passed as three more constructor lambdas: they share a lifetime, they
* all end up as flows on [PlayerFactory], and the list grows every time
* the renderer finds a new way to disappoint us.
*/
data class RemoteEvents(
/** A route stopped answering and playback fell back to the phone. */
val onDrop: (routeName: String) -> Unit = {},
/** A track could not be got playing again; surfaces to the user. */
val onStalled: (trackId: String) -> Unit = {},
/** The renderer's queue is short of ours and needs rebuilding. */
val onQueueTruncated: () -> Unit = {},
/**
* A raw poll reading, emitted only when it differs from the previous
* one. Diagnostics-only; see [TransportObservation].
*/
val onTransport: (TransportObservation) -> Unit = {},
)
private val scope = CoroutineScope(SupervisorJob() + Dispatchers.IO)
private val handler = Handler(delegate.applicationLooper)
private var pollJob: Job? = null
@@ -117,16 +97,6 @@ class MinstrelForwardingPlayer(
// visibly jumps backwards immediately after a drag, then forwards again.
@Volatile private var lastSeekIssuedAtMs: Long = 0L
// Last-read renderer queue length + when we read it. See [queueStateFor]:
// a stopped renderer is polled once a second and its queue does not change
// by itself, so re-asking every tick is pure round-trips.
@Volatile private var cachedNrTracks: Int = 0
@Volatile private var lastMediaInfoAtMs: Long = 0L
// Previous raw transport reading, so [TransportObservation]s are emitted
// on change rather than once a second forever. Null until the first poll.
@Volatile private var lastObservedTransport: Pair<TransportState, Boolean>? = null
// Wake channel for the poll loop. requestImmediatePoll() trySend's a Unit;
// pollLoop's select{} races the delay against this channel so the next
// pollOnce can fire immediately instead of waiting up to POLL_INTERVAL_MS.
@@ -518,35 +488,17 @@ class MinstrelForwardingPlayer(
// without this the radio power-saves on a locked screen and the
// poll below starves -- see [CastNetworkLock].
castNetworkLock.acquire()
// STOP the wrapped ExoPlayer -- not pause. pause() is only
// playWhenReady=false: ExoPlayer's LoadControl keeps loading, so a
// paused-but-prepared player goes on downloading the current track
// (~50s of buffer). During a cast that means the phone pulls the
// same file the renderer is streaming, over the same WiFi, and
// re-arms on every track change via syncLocalCursorToRemote's
// seekTo. At FLAC bitrates that is a second full-rate download
// competing with the speaker for air, starting exactly when a new
// track does. stop() ends the loading; Media3 keeps the media
// items, the current index and the position, so cursor sync and
// the handoff back are unaffected, and getPlaybackState() already
// reports STATE_READY while remote so no external reader sees IDLE.
//
// handler.post targets the application looper, so this runs on the
// same thread that processes our override calls -- no race with the
// pause() override branching to SOAP (holder.active is already
// non-null by the time this post fires, but delegate.stop()
// bypasses the override entirely).
handler.post { delegate.stop() }
// Pause the wrapped ExoPlayer so we are not playing local audio
// simultaneously with the remote renderer. handler.post targets the
// application looper, so this runs on the same thread that processes
// our override calls -- no race with the pause() override branching
// to SOAP (holder.active is already non-null by the time this post
// fires, but delegate.pause() bypasses the override entirely).
handler.post { delegate.pause() }
pollJob = scope.launch { pollLoop(active) }
} else {
castNetworkLock.release()
remoteState.reset()
lastObservedTransport = null
// The delegate was stopped for the cast, so it is IDLE and would
// ignore a play(). Re-prepare it for local playback. Safe when the
// queue is empty, and it does not start playback on its own --
// playWhenReady is still false until something calls play().
handler.post { delegate.prepare() }
// The next cast starts with a clean attempt budget; a stall on the
// route we just left says nothing about the next one.
stallWatchdog.reset()
@@ -564,7 +516,7 @@ class MinstrelForwardingPlayer(
} else if (remoteState.recordPollFailure()) {
if (networkStatus.state.value == ServerHealth.Healthy) {
Timber.w("UPnP drop threshold tripped for %s", active.routeName)
handler.post { events.onDrop(active.routeName) }
handler.post { onDrop(active.routeName) }
return
}
networkDropSuppressed = suppressDropForNetwork(active, networkDropSuppressed)
@@ -642,7 +594,6 @@ class MinstrelForwardingPlayer(
}
TransportState.TRANSITIONING, TransportState.UNKNOWN -> Unit
}
observeTransport(transport, info)
checkForStall(active, info.trackUri, transport)
notifyRemoteStateChanged()
}
@@ -662,15 +613,12 @@ class MinstrelForwardingPlayer(
transport: TransportInfo,
) {
val decision = stallWatchdog.onPoll(
RemoteStallWatchdog.Poll(
trackUri = trackUri,
state = transport.state,
statusOk = transport.statusOk,
playIntent = remoteState.lastPlayIntent,
positionMs = remoteState.positionMs,
nowMs = SystemClock.elapsedRealtime(),
queue = queueStateFor(active, transport),
),
trackUri = trackUri,
state = transport.state,
statusOk = transport.statusOk,
playIntent = remoteState.lastPlayIntent,
positionMs = remoteState.positionMs,
nowMs = SystemClock.elapsedRealtime(),
)
when (decision) {
is RemoteStallWatchdog.Decision.Resume -> {
@@ -690,17 +638,6 @@ class MinstrelForwardingPlayer(
Timber.w(it, "UPnP stall: resume attempt failed on %s", active.routeName)
}
}
is RemoteStallWatchdog.Decision.RepairQueue -> {
Timber.w(
"UPnP queue truncated on %s: renderer ended at its last track " +
"while %d local tracks remain; repair attempt %d",
active.routeName, delegate.mediaItemCount, decision.attempt,
)
// The renderer isn't broken -- it played everything it was
// given. Rebuilding the queue is the fix; the controller owns
// queue loading, so ask it rather than duplicating that here.
handler.post { events.onQueueTruncated() }
}
RemoteStallWatchdog.Decision.GiveUp -> {
Timber.w(
"UPnP stall on %s: giving up after repeated resume attempts",
@@ -708,80 +645,12 @@ class MinstrelForwardingPlayer(
)
// Tell the user and the admin inbox. Silence here would be the
// original bug: playback simply ends and nobody finds out.
trackIdFromStreamUri(trackUri)?.let { handler.post { events.onStalled(it) } }
trackIdFromStreamUri(trackUri)?.let { handler.post { onStalled(it) } }
}
RemoteStallWatchdog.Decision.None -> Unit
}
}
/**
* Publish this poll's raw transport reading if it differs from the last.
*
* Change-gated on purpose: steady playback is one reading repeated once a
* second, which is worth nothing and would fill the ring buffer. What is
* worth capturing is the renderer LEAVING a state — which during normal
* playback happens a couple of times per track, and during the fault the
* operator describes should happen repeatedly within a few seconds.
*/
private fun observeTransport(transport: TransportInfo, info: PositionInfo) {
val key = transport.state to transport.statusOk
if (key == lastObservedTransport) return
lastObservedTransport = key
events.onTransport(
TransportObservation(
state = transport.state.name,
statusOk = transport.statusOk,
trackNumber = info.track,
positionMs = info.relTimeMs,
playIntent = remoteState.lastPlayIntent,
atElapsedMs = SystemClock.elapsedRealtime(),
),
)
}
/**
* How the renderer's queue compares to ours, for [RemoteStallWatchdog].
*
* Only asked when the transport is actually stopped or reporting an error:
* while it plays, the answer changes nothing and GetMediaInfo would be a
* third SOAP round-trip every second. Even then the result is cached for
* [MEDIA_INFO_TTL_MS], because a stopped renderer gets polled once a
* second and its queue length does not change on its own.
*
* A renderer that reports NrTracks=0 is telling us nothing usable (some
* don't implement it) -- that reads as UNKNOWN, never as "empty queue",
* so an unhelpful renderer keeps the old resume-and-seek behaviour rather
* than being told its queue is broken.
*/
@Suppress("ReturnCount") // one early return per verdict reads better than nesting
private suspend fun queueStateFor(
active: ActiveUpnp,
transport: TransportInfo,
): RemoteStallWatchdog.QueueState {
val stalled = transport.state == TransportState.STOPPED || !transport.statusOk
if (!stalled) return RemoteStallWatchdog.QueueState.UNKNOWN
val now = SystemClock.elapsedRealtime()
if (now - lastMediaInfoAtMs > MEDIA_INFO_TTL_MS) {
lastMediaInfoAtMs = now
cachedNrTracks = runCatching { active.avTransport.getMediaInfo().nrTracks }
.onFailure { Timber.w(it, "UPnP GetMediaInfo failed on %s", active.routeName) }
.getOrDefault(0)
}
val nrTracks = cachedNrTracks
val rendererTrack = remoteState.trackNumber
if (nrTracks <= 0 || rendererTrack <= 0) return RemoteStallWatchdog.QueueState.UNKNOWN
if (rendererTrack < nrTracks) return RemoteStallWatchdog.QueueState.HAS_MORE
// On its last track. Whether that is a problem depends entirely on
// whether we have tracks it never received.
return if (delegate.mediaItemCount > nrTracks) {
RemoteStallWatchdog.QueueState.TRUNCATED
} else {
RemoteStallWatchdog.QueueState.COMPLETE
}
}
/**
* Align the paused local delegate cursor to the track the renderer is
* actually playing, so the un-overridden current-item getters
@@ -874,10 +743,6 @@ class MinstrelForwardingPlayer(
const val POLL_INTERVAL_MS = 1_000L
const val NON_PLAYING_CONFIRM = 2
const val SEEK_ACK_WINDOW_MS = 2_000L
// How long a GetMediaInfo queue-length reading stays good for. The
// watchdog needs three agreeing polls (~3s) before it acts, so one
// read comfortably covers a decision without asking every tick.
const val MEDIA_INFO_TTL_MS = 5_000L
// Safety upper bound on how long the polling tick will wait for
// Sonos to ack a user transport. The common case clears event-driven
// when Sonos's reported Track matches the wrapped player; this only
@@ -77,13 +77,6 @@ class PlayerController @Inject constructor(
* during UPnP playback shows "Disconnected from <name>" to the user.
*/
val dropEvents: SharedFlow<String> = playerFactory.dropEvents
/**
* Raw UPnP transport readings from [PlayerFactory.transportEvents], for
* the diagnostics reporter. Read-only tap — nothing in the playback path
* consumes it.
*/
val transportEvents: SharedFlow<TransportObservation> = playerFactory.transportEvents
private val sessionToken =
SessionToken(context, ComponentName(context, MinstrelPlayerService::class.java))
@@ -84,29 +84,6 @@ class PlayerFactory @Inject constructor(
)
val stallEvents: SharedFlow<String> = stallEventsInternal.asSharedFlow()
// Fires when the renderer is found to have reached the end of a queue
// shorter than ours -- i.e. part of the queue load never landed. The
// controller owns queue loading, so it collects this and rebuilds.
// Same one-is-enough buffering: repeated notices are the same problem.
private val queueRepairInternal = MutableSharedFlow<Unit>(
replay = 0,
extraBufferCapacity = 1,
onBufferOverflow = BufferOverflow.DROP_OLDEST,
)
val queueRepairEvents: SharedFlow<Unit> = queueRepairInternal.asSharedFlow()
// Raw renderer transport readings, change-gated. Unlike the flows above
// this one carries a SEQUENCE — the diagnostics flap detector needs
// several readings in a row to tell oscillation from a normal track
// transition — so it buffers more than one and drops oldest under
// pressure rather than collapsing to the latest.
private val transportInternal = MutableSharedFlow<TransportObservation>(
replay = 0,
extraBufferCapacity = TRANSPORT_EVENT_BUFFER,
onBufferOverflow = BufferOverflow.DROP_OLDEST,
)
val transportEvents: SharedFlow<TransportObservation> = transportInternal.asSharedFlow()
fun build(): Player {
val exo = buildExoPlayer()
return MinstrelForwardingPlayer(
@@ -115,12 +92,8 @@ class PlayerFactory @Inject constructor(
remoteState = remoteState,
castNetworkLock = CastNetworkLock(context),
networkStatus = serverHealth,
events = MinstrelForwardingPlayer.RemoteEvents(
onDrop = { name -> emitDrop(name) },
onStalled = { trackId -> stallEventsInternal.tryEmit(trackId) },
onQueueTruncated = { queueRepairInternal.tryEmit(Unit) },
onTransport = { transportInternal.tryEmit(it) },
),
onDrop = { name -> emitDrop(name) },
onStalled = { trackId -> stallEventsInternal.tryEmit(trackId) },
)
}
@@ -173,12 +146,6 @@ class PlayerFactory @Inject constructor(
.build(),
)
private companion object {
// Enough readings to hold a whole flap episode plus the normal
// transitions around it; the detector's window is only a few seconds.
const val TRANSPORT_EVENT_BUFFER = 32
}
private fun emitDrop(routeName: String) {
dropEventsInternal.tryEmit(routeName)
}
@@ -29,23 +29,6 @@ import com.fabledsword.minstrel.player.output.upnp.TransportState
* [RETRY_SPACING_MS]. A genuinely unplayable file must not become an
* infinite retry loop against the renderer.
*
* A stop is not always a fault, and not always the same fault. Three
* different things arrive here looking identical — the transport says
* STOPPED and we wanted to be playing:
*
* 1. The stream died mid-track. Re-play and seek back. ([Decision.Resume])
* 2. The renderer reached the end of a queue *shorter than ours*, because
* part of the load never landed. Nothing is broken; it is playing
* exactly what it was given. Repairing the queue is the fix, and
* re-playing the finished track is not. ([Decision.RepairQueue])
* 3. The renderer reached the end of the queue and so did we. Playback is
* simply over. ([Decision.None])
*
* Case 3 matters as much as the others: without [QueueState] every cast
* session would end with the watchdog retrying the last track three times
* and then reporting a `stalled` error for a listening session that
* finished perfectly normally.
*
* Pure decision state, no coroutines and no SOAP: the caller owns the poll
* loop and performs the transport calls, this only says what should happen.
* That keeps the awkward part — counting, keying and giving up — testable
@@ -53,46 +36,6 @@ import com.fabledsword.minstrel.player.output.upnp.TransportState
*/
class RemoteStallWatchdog {
/**
* What the renderer's queue looks like relative to ours, as of this poll.
* The caller derives it from GetMediaInfo's NrTracks against the local
* queue; it only needs to be accurate when the transport is not playing.
*/
enum class QueueState {
/**
* The renderer didn't report a usable count, or it is playing and the
* question is moot. Treated as "assume a real stall" — the old
* behaviour, which is right when we know nothing.
*/
UNKNOWN,
/** The renderer still has tracks after the current one. */
HAS_MORE,
/**
* The renderer is on its last track but our queue has tracks it never
* received — the load was truncated.
*/
TRUNCATED,
/** Renderer is on its last track and so are we: playback is over. */
COMPLETE,
}
/**
* One poll's worth of observation. Grouped into a type rather than passed
* as a long parameter list so adding a fact doesn't reshuffle call sites.
*/
data class Poll(
val trackUri: String,
val state: TransportState,
val statusOk: Boolean,
val playIntent: Boolean,
val positionMs: Long,
val nowMs: Long,
val queue: QueueState = QueueState.UNKNOWN,
)
sealed interface Decision {
/** Nothing to do. */
data object None : Decision
@@ -104,14 +47,6 @@ class RemoteStallWatchdog {
*/
data class Resume(val attempt: Int, val resumeAtMs: Long) : Decision
/**
* The renderer ran off the end of a queue we failed to fully load.
* The caller should append the tail it never got and resume at the
* next track — re-playing the current one would just replay a track
* the listener already heard.
*/
data class RepairQueue(val attempt: Int) : Decision
/** Attempts are exhausted. Report it and stop trying for this track. */
data object GiveUp : Decision
}
@@ -124,22 +59,36 @@ class RemoteStallWatchdog {
private var gaveUp: Boolean = false
/**
* Feed one poll result in, get the action out. See [Poll] for the inputs;
* `nowMs` is a monotonic clock (SystemClock.elapsedRealtime), passed in so
* tests can drive time.
* Feed one poll result in, get the action out.
*
* @param trackUri the renderer's current track URI — identity for the
* per-track attempt budget, so moving to the next track forgives a
* previous one's failures.
* @param statusOk the transport's own status flag: false means the
* renderer is reporting an error rather than merely being stopped.
* @param playIntent the operator's last play/pause intent.
* @param nowMs a monotonic clock (SystemClock.elapsedRealtime), passed in
* so tests can drive time.
*/
@Suppress("ReturnCount") // early returns per state are clearer than nesting
fun onPoll(poll: Poll): Decision {
if (poll.trackUri != trackKey) {
fun onPoll(
trackUri: String,
state: TransportState,
statusOk: Boolean,
playIntent: Boolean,
positionMs: Long,
nowMs: Long,
): Decision {
if (trackUri != trackKey) {
// New track: a fresh attempt budget, and no inherited stall state.
trackKey = poll.trackUri
trackKey = trackUri
resetStall()
attempts = 0
gaveUp = false
lastPlayingPositionMs = 0L
}
if (!poll.playIntent) {
if (!playIntent) {
// Stopped because we asked. Not a stall, and the next genuine one
// should start from a clean budget.
resetStall()
@@ -148,8 +97,8 @@ class RemoteStallWatchdog {
return Decision.None
}
if (poll.state == TransportState.PLAYING && poll.statusOk) {
lastPlayingPositionMs = poll.positionMs
if (state == TransportState.PLAYING && statusOk) {
lastPlayingPositionMs = positionMs
resetStall()
// A track that recovered and is playing again has earned back its
// budget; a later, unrelated stall on the same track should get
@@ -158,7 +107,7 @@ class RemoteStallWatchdog {
return Decision.None
}
val stalled = poll.state == TransportState.STOPPED || !poll.statusOk
val stalled = state == TransportState.STOPPED || !statusOk
if (!stalled) {
// PAUSED (someone else's doing) or TRANSITIONING/UNKNOWN (in
// flight). Neither is a stall; drop the streak so a mid-track
@@ -167,14 +116,6 @@ class RemoteStallWatchdog {
return Decision.None
}
// The queue simply ended. Not a fault, so it must not consume the
// attempt budget or raise an error — the listener heard everything
// they queued.
if (poll.queue == QueueState.COMPLETE) {
resetStall()
return Decision.None
}
stoppedStreak += 1
if (stoppedStreak < STALL_CONFIRM_POLLS) return Decision.None
if (gaveUp) return Decision.None
@@ -183,15 +124,11 @@ class RemoteStallWatchdog {
gaveUp = true
return Decision.GiveUp
}
if (attempts > 0 && poll.nowMs - lastAttemptAtMs < RETRY_SPACING_MS) return Decision.None
if (attempts > 0 && nowMs - lastAttemptAtMs < RETRY_SPACING_MS) return Decision.None
attempts += 1
lastAttemptAtMs = poll.nowMs
return if (poll.queue == QueueState.TRUNCATED) {
Decision.RepairQueue(attempt = attempts)
} else {
Decision.Resume(attempt = attempts, resumeAtMs = lastPlayingPositionMs)
}
lastAttemptAtMs = nowMs
return Decision.Resume(attempt = attempts, resumeAtMs = lastPlayingPositionMs)
}
/** Forget everything — call when the route changes or playback is torn down. */
@@ -1,38 +0,0 @@
package com.fabledsword.minstrel.player
/**
* One reading of what a UPnP renderer says it is doing, taken by the poll
* loop and emitted only when it differs from the previous reading.
*
* Exists for diagnostics. The operator reports the Sonos rapidly
* play-pause-play-pausing at the start of a track, and nothing in the
* diagnostics could see it: `player_state` records source / loading / error
* but not whether we are playing, `track_change` needs the queue index to
* move, and the heartbeat samples once every 45 seconds. A symptom that
* lasts a few seconds and changes no index fell straight through all three.
*
* This is the closest observation point we have to the renderer's own truth
* — the raw GetTransportInfo reading, before the two-poll confirmation and
* the UI's smoothing have had a chance to hide the wobble.
*
* **It samples at the poll cadence (1 Hz).** If the real oscillation is
* faster than that, what lands here is an aliased jagged sequence rather
* than the true waveform. That still answers the question that matters —
* whether the renderer is steadily PLAYING or repeatedly leaving that state
* — but it cannot measure the true period. If a captured episode comes back
* looking clean, the next instrument is burst sampling, not this one.
*/
data class TransportObservation(
/** [com.fabledsword.minstrel.player.output.upnp.TransportState] name. */
val state: String,
/** CurrentTransportStatus: false means the renderer reports an error. */
val statusOk: Boolean,
/** The renderer's 1-based queue position at this reading. */
val trackNumber: Int,
/** The renderer's reported position within the track. */
val positionMs: Long,
/** Whether the operator's last intent was to be playing. */
val playIntent: Boolean,
/** Monotonic stamp, so a consumer can measure gaps between readings. */
val atElapsedMs: Long,
)
@@ -10,9 +10,11 @@ import com.fabledsword.minstrel.models.TrackRef
import com.fabledsword.minstrel.player.PlayerController
import com.fabledsword.minstrel.player.PlayerFactory
import com.fabledsword.minstrel.player.RemotePlayerState
import com.fabledsword.minstrel.player.StreamTokenProvider
import com.fabledsword.minstrel.player.output.upnp.AVTransportClient
import com.fabledsword.minstrel.player.output.upnp.RenderingControlClient
import com.fabledsword.minstrel.player.output.upnp.SoapClient
import com.fabledsword.minstrel.player.output.upnp.SoapFaultException
import com.fabledsword.minstrel.player.output.upnp.TransportState
import com.fabledsword.minstrel.player.output.upnp.UpnpDiscoveryController
import com.fabledsword.minstrel.player.output.upnp.bareUdn
@@ -59,7 +61,7 @@ data class RouteSnapshot(
* - [OutputRoute.Protocol.SYSTEM] — MediaRouter.selectRoute (built-in,
* wired, Bluetooth)
* - [OutputRoute.Protocol.UPNP] — mint a signed stream token via
* [SonosQueueLoader], drive the discovered renderer with
* [StreamTokenProvider.mint], drive the discovered renderer with
* AVTransport.SetAVTransportURI + Play, pause local playback so
* audio yields to the network speaker
* - [OutputRoute.Protocol.CAST] / [OutputRoute.Protocol.SONOS] —
@@ -77,7 +79,7 @@ class OutputPickerController @Inject constructor(
private val upnpDiscovery: UpnpDiscoveryController,
private val playerController: PlayerController,
private val playerFactory: PlayerFactory,
private val sonosQueue: SonosQueueLoader,
private val streamTokens: StreamTokenProvider,
private val activeUpnpHolder: ActiveUpnpHolder,
private val remoteState: RemotePlayerState,
private val okHttp: OkHttpClient,
@@ -171,7 +173,6 @@ class OutputPickerController @Inject constructor(
playerFactory.dropEvents.collect { handleRemoteDrop() }
}
scope.launch { observeQueueChangesForSonosResync() }
scope.launch { observeQueueRepairRequests() }
scope.launch { observeIdleRevertWhileUpnp() }
scope.launch { observeSelectedRouteDisappearance() }
}
@@ -254,7 +255,7 @@ class OutputPickerController @Inject constructor(
* setMediaItems override clears holder.active + sets target so the
* imminent play() call drops (drops via isLoadingUpnp() = true). Then
* this collector observes the uiState.queue change and re-runs
* SonosQueueLoader.load to push the new tracks to Sonos.
* loadQueueOnSonos to push the new tracks to Sonos.
*
* Discrimination: selectUpnp's initial-load path doesn't change
* uiState.queue (the queue was already populated before route
@@ -285,71 +286,6 @@ class OutputPickerController @Inject constructor(
}
}
/**
* Rebuild the renderer's queue when playback stopped because the renderer
* ran off the end of a queue shorter than ours.
*
* [SonosQueueLoader] tolerates individual AddURIToQueue failures and
* gives up appending after a few consecutive ones
* -- Sonos rate-limits burst adds. Until this existed that left a short
* queue on the renderer and nothing to notice it: the renderer played what
* it had and stopped, and the app went on believing there were forty
* tracks left. [MinstrelForwardingPlayer] now compares GetMediaInfo's
* NrTracks against the local queue and asks for this.
*
* A full reload, not an incremental diff: the renderer's copy is known to
* be wrong, and the diff path reasons from what we *think* it holds, which
* is exactly the assumption that failed. The load re-seeks to the
* current track and plays, so recovery lands where the listener was.
*/
private suspend fun observeQueueRepairRequests() {
playerFactory.queueRepairEvents.collect {
val routeId = selectedUpnpRouteIdInternal.value
?: activeUpnpHolder.active.value?.routeId
if (routeId == null) {
Timber.w("Sonos queue repair skipped: no UPnP route selected")
return@collect
}
val state = playerController.uiState.value
if (state.queue.isEmpty()) {
Timber.w("Sonos queue repair skipped: local queue is empty")
return@collect
}
// Resume on the track AFTER the current one. The renderer stopped
// because it finished the last track it had; the local cursor is
// synced to that track, so reloading at it would replay something
// the listener just heard. The next one is what they never got.
val resumeAt = (state.queueIndex + 1).coerceAtMost(state.queue.size - 1)
repairSonosQueue(routeId, state.queue, resumeAt)
}
}
private suspend fun repairSonosQueue(
routeId: String,
queue: List<TrackRef>,
currentIndex: Int,
) = selectUpnpMutex.withLock {
val upnpRoute = upnpDiscovery.routes.value.firstOrNull { it.id == routeId }
val transport = upnpDiscovery.transportFor(routeId)
if (upnpRoute == null || transport == null) {
Timber.w("Sonos queue repair: route or transport gone for %s", routeId)
return@withLock
}
val outputRoute = OutputRoute.fromUpnpRoute(upnpRoute)
Timber.w(
"Sonos queue repair: reloading %d tracks on %s (resuming at index %d)",
queue.size, outputRoute.name, currentIndex,
)
runCatching {
sonosQueue.load(transport, outputRoute, queue, currentIndex)
}.onFailure { e ->
// Leave the route active: the renderer is reachable enough to have
// told us its queue length, so dropping to local would be a harsher
// remedy than letting the next stall re-decide.
Timber.w(e, "Sonos queue repair failed on %s", outputRoute.name)
}
}
/**
* Bring Sonos's native queue back in sync with the local queue after a
* mutation. Tries an incremental SOAP diff first (RemoveTrackRangeFromQueue
@@ -377,7 +313,7 @@ class OutputPickerController @Inject constructor(
return@withLock
}
val handledIncrementally = runCatching {
sonosQueue.tryIncrementalResync(transport, oldIds, newQueue)
tryIncrementalResync(transport, oldIds, newQueue)
}.getOrElse { e ->
Timber.w(e, "Sonos incremental resync errored; falling back to full reload")
false
@@ -401,7 +337,7 @@ class OutputPickerController @Inject constructor(
val rendering = renderingClientFor(routeId)
Timber.w("Sonos resync: full reload of %d tracks on %s", newQueue.size, outputRoute.name)
runCatching {
sonosQueue.load(transport, outputRoute, newQueue, newCurrentIndex)
loadQueueOnSonos(transport, outputRoute, newQueue, newCurrentIndex)
activeUpnpHolder.set(
ActiveUpnp(
routeId = routeId,
@@ -418,6 +354,98 @@ class OutputPickerController @Inject constructor(
}
}
/**
* Diff-based incremental Sonos queue sync. Returns true when the new
* queue can be produced from the old one with a remove-then-insert at
* the same middle slice -- the common-prefix and common-suffix portions
* stay untouched, and the current Sonos track must lie in the preserved
* prefix (otherwise the diff would orphan playback). Returns false to
* signal the caller to fall back to a full reload.
*/
private suspend fun tryIncrementalResync(
transport: AVTransportClient,
oldIds: List<String>,
newQueue: List<TrackRef>,
): Boolean {
val newIds = newQueue.map { it.id }
if (oldIds == newIds) return true
val prefixLen = commonPrefixLength(oldIds, newIds)
val suffixLen = commonSuffixLength(
oldIds.subList(prefixLen, oldIds.size),
newIds.subList(prefixLen, newIds.size),
)
val removedCount = oldIds.size - prefixLen - suffixLen
val addedCount = newIds.size - prefixLen - suffixLen
// Sonos's current track number is 1-based; compare against the
// preserved-prefix range as 0-based. If the current track is in
// the removed slice, incremental can't preserve playback -- caller
// falls back to full rebuild.
val currentSonosIdx0 = remoteState.trackNumber - 1
val canApply = currentSonosIdx0 in 0 until prefixLen
if (canApply) {
applyQueueDiff(transport, newQueue, prefixLen, removedCount, addedCount)
} else {
Timber.w(
"Sonos incremental: current track %d not in preserved prefix [0,%d); full rebuild",
currentSonosIdx0,
prefixLen,
)
}
return canApply
}
private suspend fun applyQueueDiff(
transport: AVTransportClient,
newQueue: List<TrackRef>,
prefixLen: Int,
removedCount: Int,
addedCount: Int,
) {
if (removedCount > 0) {
Timber.w(
"Sonos incremental: RemoveTrackRangeFromQueue start=%d count=%d",
prefixLen + 1,
removedCount,
)
transport.removeTrackRangeFromQueue(
startingIndex = prefixLen + 1,
numberOfTracks = removedCount,
)
}
if (addedCount == 0) return
Timber.w(
"Sonos incremental: AddURIToQueue x%d starting at position %d",
addedCount,
prefixLen + 1,
)
for (i in 0 until addedCount) {
val ref = newQueue[prefixLen + i]
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = prefixLen + i + 1,
)
if (i > 0) delay(EXTEND_THROTTLE_MS)
}
}
private fun commonPrefixLength(a: List<String>, b: List<String>): Int {
val limit = minOf(a.size, b.size)
for (i in 0 until limit) {
if (a[i] != b[i]) return i
}
return limit
}
private fun commonSuffixLength(a: List<String>, b: List<String>): Int {
val limit = minOf(a.size, b.size)
for (i in 0 until limit) {
if (a[a.size - 1 - i] != b[b.size - 1 - i]) return i
}
return limit
}
/**
* Called when the active UPnP route drops unexpectedly (the poll loop's
@@ -511,7 +539,7 @@ class OutputPickerController @Inject constructor(
* 1. Pause local so the user doesn't keep hearing local audio.
* 2. Set target early so ForwardingPlayer drops transport taps
* while the 17-second queue load is in progress.
* 3. Wire active LAST (after the queue load) so SOAP commands
* 3. Wire active LAST (after loadQueueOnSonos) so SOAP commands
* are never routed to a half-loaded Sonos queue.
*/
private suspend fun selectUpnp(route: OutputRoute) = selectUpnpMutex.withLock {
@@ -552,7 +580,7 @@ class OutputPickerController @Inject constructor(
// taps don't hit Sonos's stale state from a prior session.
activeUpnpHolder.setTarget(effectiveRoute.id)
runCatching {
sonosQueue.load(transport, effectiveRoute, uiState.queue, uiState.queueIndex)
loadQueueOnSonos(transport, effectiveRoute, uiState.queue, uiState.queueIndex)
// Wire active LAST -- SOAP path is now safe to use.
activeUpnpHolder.set(
ActiveUpnp(
@@ -669,6 +697,108 @@ class OutputPickerController @Inject constructor(
return if (i >= 0) segments.getOrNull(i + 1) else null
}
private suspend fun loadQueueOnSonos(
transport: AVTransportClient,
route: OutputRoute,
queue: List<TrackRef>,
currentIndex: Int,
) {
Timber.w("UPnP select: clear queue on %s", route.name)
transport.removeAllTracksFromQueue()
val initialEnd = (currentIndex + 1).coerceAtMost(queue.size)
val initialBatch = queue.subList(0, initialEnd)
Timber.w(
"UPnP select: add %d initial tracks (currentIndex=%d, totalQueue=%d)",
initialBatch.size, currentIndex, queue.size,
)
initialBatch.forEachIndexed { idx, ref ->
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = idx + 1,
)
}
val coordinatorUdn = route.id.bareUdn()
val queueUri = "x-rincon-queue:$coordinatorUdn#0"
Timber.w("UPnP select: SetAVTransportURI %s", queueUri)
transport.setAVTransportURI(queueUri, "")
Timber.w("UPnP select: Seek to track %d", currentIndex + 1)
transport.seekToTrack(currentIndex + 1)
Timber.w("UPnP select: Play")
transport.play()
Timber.w("UPnP select: initial done; backgrounding remainder")
val remaining = queue.drop(initialEnd)
if (remaining.isNotEmpty()) {
scope.launch { extendQueueOnSonos(transport, route, remaining, initialEnd) }
}
}
/**
* Background-append tracks after activation. Runs concurrently with
* Sonos playback. Cancels if the user disconnects from this route
* (active.routeId changes or becomes null). Tolerates individual
* AddURIToQueue failures — log and continue so some tracks loaded
* is better than zero tracks loaded.
*/
private suspend fun extendQueueOnSonos(
transport: AVTransportClient,
route: OutputRoute,
tracks: List<TrackRef>,
startPosition: Int,
) {
Timber.w(
"UPnP extend: appending %d tracks starting at position %d",
tracks.size, startPosition + 1,
)
var consecutiveFailures = 0
var succeeded = 0
var aborted = false
for ((i, ref) in tracks.withIndex()) {
if (aborted) break
if (activeUpnpHolder.active.value?.routeId != route.id) {
Timber.w("UPnP extend: cancelled at offset %d (route changed)", i)
aborted = true
} else {
val outcome = runCatching {
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = startPosition + i + 1,
)
}
if (outcome.isSuccess) {
consecutiveFailures = 0
succeeded += 1
// Throttle the burst so we don't tickle Sonos's burst-add
// rejection -- logcat 2026-06-04 showed 33 consecutive
// failures clustered at ~10ms intervals once offset 39 was
// reached, which looks like a rate-limit kicking in. The
// delay is small enough that extending 100 tracks adds
// only ~5s to background work that's already async.
delay(EXTEND_THROTTLE_MS)
} else {
consecutiveFailures += 1
val e = outcome.exceptionOrNull()
val detail = (e as? SoapFaultException)?.let {
"code=${it.code} desc=${it.description}"
} ?: e?.message
Timber.w(e, "UPnP extend: append failed at offset %d -- %s", i, detail)
if (consecutiveFailures >= EXTEND_ABORT_AFTER_FAILURES) {
Timber.w(
"UPnP extend: aborting after %d consecutive failures",
consecutiveFailures,
)
aborted = true
}
}
}
}
Timber.w("UPnP extend: done (%d / %d appended)", succeeded, tracks.size)
}
private fun renderingClientFor(routeId: String): RenderingControlClient? {
val rcUrl = upnpDiscovery.routes.value
@@ -700,6 +830,9 @@ class OutputPickerController @Inject constructor(
}
private companion object {
const val EXTEND_ABORT_AFTER_FAILURES = 3
const val EXTEND_THROTTLE_MS = 50L
// 5 minutes of continuous non-playing on a UPnP route before we
// revert to the phone speaker, so a stale Sonos selection can't make
// a later "tap play" do nothing.
@@ -1,316 +0,0 @@
package com.fabledsword.minstrel.player.output
import com.fabledsword.minstrel.di.ApplicationScope
import com.fabledsword.minstrel.models.TrackRef
import com.fabledsword.minstrel.player.RemotePlayerState
import com.fabledsword.minstrel.player.StreamTokenProvider
import com.fabledsword.minstrel.player.output.upnp.AVTransportClient
import com.fabledsword.minstrel.player.output.upnp.SoapFaultException
import com.fabledsword.minstrel.player.output.upnp.bareUdn
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
import javax.inject.Singleton
/**
* Owns the shape of a Sonos renderer's native queue: loading it, growing it,
* diffing it against local queue mutations, and — the part that was missing —
* confirming the renderer actually took what we sent.
*
* Split out of [OutputPickerController], which is about *which route is
* selected*. How many tracks the renderer is holding is a separate concern
* with its own failure modes, and it had grown large enough to hide one:
* every write here is a SOAP call that can fail individually, and until
* [verifyQueueLength] nothing ever read the result back.
*/
@Singleton
class SonosQueueLoader @Inject constructor(
@ApplicationScope private val scope: CoroutineScope,
private val streamTokens: StreamTokenProvider,
private val activeUpnpHolder: ActiveUpnpHolder,
private val remoteState: RemotePlayerState,
) {
suspend fun load(
transport: AVTransportClient,
route: OutputRoute,
queue: List<TrackRef>,
currentIndex: Int,
) {
Timber.w("UPnP select: clear queue on %s", route.name)
transport.removeAllTracksFromQueue()
val initialEnd = (currentIndex + 1).coerceAtMost(queue.size)
val initialBatch = queue.subList(0, initialEnd)
Timber.w(
"UPnP select: add %d initial tracks (currentIndex=%d, totalQueue=%d)",
initialBatch.size, currentIndex, queue.size,
)
initialBatch.forEachIndexed { idx, ref ->
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = idx + 1,
)
}
val coordinatorUdn = route.id.bareUdn()
val queueUri = "x-rincon-queue:$coordinatorUdn#0"
Timber.w("UPnP select: SetAVTransportURI %s", queueUri)
transport.setAVTransportURI(queueUri, "")
Timber.w("UPnP select: Seek to track %d", currentIndex + 1)
transport.seekToTrack(currentIndex + 1)
Timber.w("UPnP select: Play")
transport.play()
Timber.w("UPnP select: initial done; backgrounding remainder")
val remaining = queue.drop(initialEnd)
// Verify even when there is no tail to append: the initial batch is
// sent the same way and can be dropped the same way.
scope.launch {
if (remaining.isNotEmpty()) {
extendQueueOnSonos(transport, route, remaining, initialEnd)
}
verifyQueueLength(transport, route, queue)
}
}
/**
* Background-append tracks after activation. Runs concurrently with Sonos
* playback. Cancels if the user disconnects from this route (active.routeId
* changes or becomes null). Correctness of the result is [verifyQueueLength]'s
* job, not this function's.
*/
private suspend fun extendQueueOnSonos(
transport: AVTransportClient,
route: OutputRoute,
tracks: List<TrackRef>,
startPosition: Int,
) {
Timber.w(
"UPnP extend: appending %d tracks starting at position %d",
tracks.size, startPosition + 1,
)
val succeeded = appendTracksToQueue(transport, route, tracks, startPosition)
Timber.w("UPnP extend: done (%d / %d appended)", succeeded, tracks.size)
}
/**
* Confirm the renderer holds as many tracks as we sent, and append the
* tail it dropped.
*
* [appendTracksToQueue] tolerates individual failures and gives up after
* [EXTEND_ABORT_AFTER_FAILURES] consecutive ones, because Sonos rate-limits
* burst adds (logcat 2026-06-04: 33 consecutive failures once offset 39 was
* reached). That is the right call in the moment — some tracks loaded beats
* none — but it used to be the end of the story, and the renderer was left
* holding a queue shorter than ours with nothing aware of it. It then
* played what it had and stopped, which looked exactly like playback dying
* for no reason.
*
* Sonos appends sequentially, so a short queue means a missing tail: taking
* `fullQueue.drop(nrTracks)` is the gap. Bounded at [VERIFY_ROUNDS] passes
* so a renderer that refuses to grow can't spin here forever.
*/
@Suppress("ReturnCount") // each bail-out is a distinct reason to stop verifying
private suspend fun verifyQueueLength(
transport: AVTransportClient,
route: OutputRoute,
fullQueue: List<TrackRef>,
) {
repeat(VERIFY_ROUNDS) { round ->
if (activeUpnpHolder.active.value?.routeId != route.id) return
val nrTracks = runCatching { transport.getMediaInfo().nrTracks }
.getOrElse { e ->
Timber.w(e, "UPnP verify: GetMediaInfo failed on %s", route.name)
return
}
// 0 means the renderer told us nothing usable, not that its queue
// is empty. Guessing "empty" here would re-send the whole queue to
// a renderer that is playing it perfectly well.
if (nrTracks <= 0) {
Timber.w("UPnP verify: no usable NrTracks from %s; skipping", route.name)
return
}
if (nrTracks >= fullQueue.size) {
Timber.w("UPnP verify: renderer holds %d tracks, queue intact", nrTracks)
return
}
val missing = fullQueue.drop(nrTracks)
Timber.w(
"UPnP verify: %s holds %d of %d tracks; appending %d missing (round %d)",
route.name, nrTracks, fullQueue.size, missing.size, round + 1,
)
appendTracksToQueue(transport, route, missing, nrTracks)
}
Timber.w("UPnP verify: gave up repairing queue length on %s", route.name)
}
/**
* Append [tracks] at [startPosition] (0-based), returning how many landed.
* Tolerates individual AddURIToQueue failures — log and continue so some
* tracks loaded is better than zero tracks loaded — and stops early after
* [EXTEND_ABORT_AFTER_FAILURES] consecutive ones.
*/
private suspend fun appendTracksToQueue(
transport: AVTransportClient,
route: OutputRoute,
tracks: List<TrackRef>,
startPosition: Int,
): Int {
var consecutiveFailures = 0
var succeeded = 0
var aborted = false
for ((i, ref) in tracks.withIndex()) {
if (aborted) break
if (activeUpnpHolder.active.value?.routeId != route.id) {
Timber.w("UPnP extend: cancelled at offset %d (route changed)", i)
aborted = true
} else {
val outcome = runCatching {
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = startPosition + i + 1,
)
}
if (outcome.isSuccess) {
consecutiveFailures = 0
succeeded += 1
// Throttle the burst so we don't tickle Sonos's burst-add
// rejection -- logcat 2026-06-04 showed 33 consecutive
// failures clustered at ~10ms intervals once offset 39 was
// reached, which looks like a rate-limit kicking in. The
// delay is small enough that extending 100 tracks adds
// only ~5s to background work that's already async.
delay(EXTEND_THROTTLE_MS)
} else {
consecutiveFailures += 1
val e = outcome.exceptionOrNull()
val detail = (e as? SoapFaultException)?.let {
"code=${it.code} desc=${it.description}"
} ?: e?.message
Timber.w(e, "UPnP extend: append failed at offset %d -- %s", i, detail)
if (consecutiveFailures >= EXTEND_ABORT_AFTER_FAILURES) {
Timber.w(
"UPnP extend: aborting after %d consecutive failures",
consecutiveFailures,
)
aborted = true
}
}
}
}
return succeeded
}
/**
* Diff-based incremental Sonos queue sync. Returns true when the new
* queue can be produced from the old one with a remove-then-insert at
* the same middle slice -- the common-prefix and common-suffix portions
* stay untouched, and the current Sonos track must lie in the preserved
* prefix (otherwise the diff would orphan playback). Returns false to
* signal the caller to fall back to a full reload.
*/
suspend fun tryIncrementalResync(
transport: AVTransportClient,
oldIds: List<String>,
newQueue: List<TrackRef>,
): Boolean {
val newIds = newQueue.map { it.id }
if (oldIds == newIds) return true
val prefixLen = commonPrefixLength(oldIds, newIds)
val suffixLen = commonSuffixLength(
oldIds.subList(prefixLen, oldIds.size),
newIds.subList(prefixLen, newIds.size),
)
val removedCount = oldIds.size - prefixLen - suffixLen
val addedCount = newIds.size - prefixLen - suffixLen
// Sonos's current track number is 1-based; compare against the
// preserved-prefix range as 0-based. If the current track is in
// the removed slice, incremental can't preserve playback -- caller
// falls back to full rebuild.
val currentSonosIdx0 = remoteState.trackNumber - 1
val canApply = currentSonosIdx0 in 0 until prefixLen
if (canApply) {
applyQueueDiff(transport, newQueue, prefixLen, removedCount, addedCount)
} else {
Timber.w(
"Sonos incremental: current track %d not in preserved prefix [0,%d); full rebuild",
currentSonosIdx0,
prefixLen,
)
}
return canApply
}
private suspend fun applyQueueDiff(
transport: AVTransportClient,
newQueue: List<TrackRef>,
prefixLen: Int,
removedCount: Int,
addedCount: Int,
) {
if (removedCount > 0) {
Timber.w(
"Sonos incremental: RemoveTrackRangeFromQueue start=%d count=%d",
prefixLen + 1,
removedCount,
)
transport.removeTrackRangeFromQueue(
startingIndex = prefixLen + 1,
numberOfTracks = removedCount,
)
}
if (addedCount == 0) return
Timber.w(
"Sonos incremental: AddURIToQueue x%d starting at position %d",
addedCount,
prefixLen + 1,
)
for (i in 0 until addedCount) {
val ref = newQueue[prefixLen + i]
val token = streamTokens.mint(ref.id)
transport.addURIToQueue(
uri = token.url,
mime = token.mime,
title = token.title,
enqueuedURIPosition = prefixLen + i + 1,
)
if (i > 0) delay(EXTEND_THROTTLE_MS)
}
}
private fun commonPrefixLength(a: List<String>, b: List<String>): Int {
val limit = minOf(a.size, b.size)
for (i in 0 until limit) {
if (a[i] != b[i]) return i
}
return limit
}
private fun commonSuffixLength(a: List<String>, b: List<String>): Int {
val limit = minOf(a.size, b.size)
for (i in 0 until limit) {
if (a[a.size - 1 - i] != b[b.size - 1 - i]) return i
}
return limit
}
private companion object {
// Abort the append loop after this many consecutive AddURIToQueue
// failures; Sonos rate-limits burst adds and a wall of failures means
// it has stopped accepting, not that the next one might land.
const val EXTEND_ABORT_AFTER_FAILURES = 3
const val EXTEND_THROTTLE_MS = 50L
// Verify/repair passes after a queue load. Two: one to catch the
// common case (a rate-limit burst dropped a chunk), one to catch a
// repair that itself got rate-limited. Beyond that the renderer is
// refusing for a reason retrying won't fix, and the stall watchdog
// becomes the backstop.
const val VERIFY_ROUNDS = 2
}
}
@@ -211,34 +211,6 @@ class AVTransportClient(
)
}
/**
* What the renderer believes it is holding: how many tracks are in its
* queue, and the URI the transport is pointed at.
*
* We load the queue with AddURIToQueue and, until this existed, never
* read it back — so a partially-applied load was invisible. Sonos
* rate-limits burst adds (logcat 2026-06-04: 33 consecutive failures once
* offset 39 was reached), and [OutputPickerController]'s extend loop gives
* up after a few of those and leaves a short queue behind. The renderer
* then plays what it actually has and stops, correctly, at an end the app
* did not know existed.
*
* NrTracks is the cheap authoritative answer, so queue truncation becomes
* something we can detect and repair rather than infer.
*/
suspend fun getMediaInfo(): MediaInfo {
val result = soap.call(
controlUrl = controlUrl,
serviceType = SERVICE_TYPE,
action = "GetMediaInfo",
args = mapOf("InstanceID" to "0"),
)
return MediaInfo(
nrTracks = result["NrTracks"]?.toIntOrNull() ?: 0,
currentUri = result["CurrentURI"].orEmpty(),
)
}
suspend fun getTransportInfo(): TransportInfo {
val result = soap.call(
controlUrl = controlUrl,
@@ -326,14 +298,6 @@ data class PositionInfo(
val trackDurationMs: Long,
)
/**
* [nrTracks] is the renderer's own count of its queue — 0 when it reports
* nothing, which callers must read as "unknown", never as "empty". A
* renderer that does not implement GetMediaInfo usefully must not be
* mistaken for one with an empty queue.
*/
data class MediaInfo(val nrTracks: Int, val currentUri: String)
enum class TransportState { PLAYING, PAUSED, STOPPED, TRANSITIONING, UNKNOWN }
/**
@@ -109,11 +109,8 @@ private fun MiniCover(coverUrl: String, contentDescription: String) {
* NowPlayingScreen via [onExpandClick].
*
* Layout (Column):
* - Slim seek slider pinned at the top (4dp track)
* - Row: cover | title/artist column | like | prev | play/pause | next.
* Weighted so it fills the rest of the fixed-height bar and centres its
* own content; otherwise the row keeps its intrinsic 48dp and the
* leftover height collects at the bottom as dead surface.
* - Slim seek slider at the top (4dp track)
* - Row: cover | title/artist column | like | prev | play/pause | next
*
* No kebab on the mini bar (operator 2026-06-01): the full kebab
* surface lives on NowPlayingScreen, and dropping it from the mini
@@ -167,12 +164,6 @@ fun MiniPlayer(
durationMs = state.durationMs,
)
MiniRow(
// Take whatever the progress fill leaves. Without this the
// Column stacks 4dp + the row's intrinsic 48dp from the top
// and the remaining 28dp of an 80dp bar sits empty
// underneath — the content looked top-aligned rather than
// centred, with a dead strip above the gesture bar.
modifier = Modifier.weight(1f),
track = track,
isPlaying = state.isPlaying,
isUpnpLoading = state.isUpnpLoading,
@@ -214,7 +205,6 @@ private fun MiniProgressFill(positionMs: Long, durationMs: Long) {
@Composable
@Suppress("LongParameterList")
private fun MiniRow(
modifier: Modifier,
track: TrackRef,
isPlaying: Boolean,
isUpnpLoading: Boolean,
@@ -226,7 +216,7 @@ private fun MiniRow(
onToggleLike: () -> Unit,
) {
Row(
modifier = modifier
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 12.dp),
verticalAlignment = Alignment.CenterVertically,
@@ -9,7 +9,7 @@ import com.fabledsword.minstrel.update.data.ApkInstaller
import com.fabledsword.minstrel.update.data.InstallStage
import com.fabledsword.minstrel.update.data.UpdateRepository
import com.fabledsword.minstrel.update.data.isBusy
import com.fabledsword.minstrel.update.data.isUpdateAvailable
import com.fabledsword.minstrel.update.data.isVersionNewer
import com.fabledsword.minstrel.update.data.message
import com.fabledsword.minstrel.update.data.stage
import dagger.hilt.android.lifecycle.HiltViewModel
@@ -37,10 +37,6 @@ sealed interface UpdateCheckResult {
data class AboutUiState(
val installedVersion: String = BuildConfig.VERSION_NAME,
// The value the platform installs by, and therefore the one the update
// check must decide on. Held in state rather than read inline so a test
// can drive the comparison without a BuildConfig.
val installedCode: Long = BuildConfig.VERSION_CODE.toLong(),
val isChecking: Boolean = false,
val installStage: InstallStage = InstallStage.IDLE,
val installMessage: String? = null,
@@ -49,9 +45,8 @@ data class AboutUiState(
/**
* Backs the About card's update controls. "Check for updates" calls
* [UpdateRepository.getLatest], compares versus this build via
* [isUpdateAvailable] — on the ordering key where the server reports one,
* on the name otherwise — and reports the terminal state.
* [UpdateRepository.getLatest], compares versus the build's
* VERSION_NAME via [isVersionNewer], and reports the terminal state.
* When an update is available, [install] downloads the APK via
* [ApkInstaller] and installs it — routing the user to the "install
* unknown apps" settings page first when that permission hasn't been
@@ -71,17 +66,9 @@ class AboutCardViewModel @Inject constructor(
viewModelScope.launch {
internal.update { it.copy(isChecking = true, installMessage = null) }
val installed = internal.value.installedVersion
val installedCode = internal.value.installedCode
val result = runCatching { repository.getLatest() }
.map { latest ->
if (
isUpdateAvailable(
serverCode = latest.code,
serverName = latest.version,
installedCode = installedCode,
installedName = installed,
)
) {
if (isVersionNewer(latest.version, installed)) {
UpdateCheckResult.UpdateAvailable(latest)
} else {
UpdateCheckResult.Latest
@@ -2,45 +2,72 @@ package com.fabledsword.minstrel.theme
import androidx.compose.material3.Typography
import androidx.compose.ui.text.TextStyle
import androidx.compose.ui.text.font.Font
import androidx.compose.ui.text.font.FontFamily
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.text.font.FontWeight
import androidx.compose.ui.text.googlefonts.Font
import androidx.compose.ui.text.googlefonts.GoogleFont
import androidx.compose.ui.unit.sp
import com.fabledsword.minstrel.R
/**
* Bundled typefaces, vendored into res/font by tools/vendor-fonts.py.
*
* These were fetched at runtime through the Play Services font provider until
* 2026-09-09. That is a network dependency for rendering, and a deployed
* instance is not guaranteed one — the provider is also absent entirely on
* devices without Play Services, where the app silently fell back to the
* platform default and stopped looking like Minstrel. Bundling costs ~0.86 MB
* of APK and removes both failure modes.
* Google Fonts provider — fetches font files via Play Services Fonts at
* runtime, caches them across launches. Matches the Flutter client's
* `google_fonts` package behaviour (no bundled .ttf files in either tree).
*
* Per FabledSword design system:
* - Fraunces — display + headline (mythic serif)
* - Inter — body + label (clean sans for UI text)
* - JetBrains Mono — technical / monospace
* Weights are restricted to 400 (regular) and 500 (medium) only.
*
* Each res/font entry is a single static instance, not a variable font: the
* weight declared beside it here must match the file's own OS/2
* usWeightClass, which the vendoring script asserts on download.
*/
private val GoogleFontProvider = GoogleFont.Provider(
providerAuthority = "com.google.android.gms.fonts",
providerPackage = "com.google.android.gms",
certificates = R.array.com_google_android_gms_fonts_certs,
)
private val FrauncesFont = GoogleFont("Fraunces")
private val InterFont = GoogleFont("Inter")
private val JetBrainsMonoFont = GoogleFont("JetBrains Mono")
private val Fraunces = FontFamily(
Font(R.font.fraunces_regular, FontWeight.W400, FontStyle.Normal),
Font(R.font.fraunces_medium, FontWeight.W500, FontStyle.Normal),
Font(
googleFont = FrauncesFont,
fontProvider = GoogleFontProvider,
weight = FontWeight.W400,
style = FontStyle.Normal,
),
Font(
googleFont = FrauncesFont,
fontProvider = GoogleFontProvider,
weight = FontWeight.W500,
style = FontStyle.Normal,
),
)
private val Inter = FontFamily(
Font(R.font.inter_regular, FontWeight.W400, FontStyle.Normal),
Font(R.font.inter_medium, FontWeight.W500, FontStyle.Normal),
Font(
googleFont = InterFont,
fontProvider = GoogleFontProvider,
weight = FontWeight.W400,
style = FontStyle.Normal,
),
Font(
googleFont = InterFont,
fontProvider = GoogleFontProvider,
weight = FontWeight.W500,
style = FontStyle.Normal,
),
)
private val JetBrainsMono = FontFamily(
Font(R.font.jetbrains_mono_regular, FontWeight.W400, FontStyle.Normal),
Font(
googleFont = JetBrainsMonoFont,
fontProvider = GoogleFontProvider,
weight = FontWeight.W400,
style = FontStyle.Normal,
),
)
/**
@@ -19,8 +19,7 @@ private const val POLL_INTERVAL_MS = 24 * 60 * 60 * 1000L
/**
* Drives the shell's soft "update available" banner. Polls
* `/api/client/version` at launch + every 24h and, when the bundled
* APK outranks this build — by ordering key where the server reports one,
* by name otherwise — exposes its [UpdateInfo] so
* APK is strictly newer than this build, exposes its [UpdateInfo] so
* [com.fabledsword.minstrel.update.ui.UpdateBanner] can nudge an
* install. Mirrors Flutter's `ClientUpdateController`.
*
@@ -59,13 +58,6 @@ class UpdateBannerController @Inject constructor(
private suspend fun runOnce() {
val info = runCatching { repository.getLatest() }.getOrNull() ?: return
latest.value = info.takeIf {
isUpdateAvailable(
serverCode = it.code,
serverName = it.version,
installedCode = BuildConfig.VERSION_CODE.toLong(),
installedName = BuildConfig.VERSION_NAME,
)
}
latest.value = info.takeIf { isVersionNewer(it.version, BuildConfig.VERSION_NAME) }
}
}
@@ -21,39 +21,10 @@ class UpdateRepository @Inject constructor(retrofit: Retrofit) {
private fun UpdateInfoWire.toDomain(): UpdateInfo = UpdateInfo(
version = version,
code = code,
channel = channel,
apkUrl = apkUrl,
sizeBytes = sizeBytes,
)
/**
* True when [server] should be offered over the installed build.
*
* **Decide on the ordering key whenever the server sends one.** That is the
* same value Android's package installer compares, so an offer made this way
* implies an install the platform will actually accept. The app used to
* compare NAMES while the platform installed by `versionCode`, with nothing
* keeping the two orderings consistent — so it could offer a build Android
* then refused as a downgrade, or stay quiet about one it would have taken.
*
* Name comparison survives only as the fallback for a server that predates
* the field. A null code means "this server cannot tell me" — never "zero" —
* because treating absent as zero would rank every such server as infinitely
* old and offer its build to everyone, forever.
*/
fun isUpdateAvailable(
serverCode: Long?,
serverName: String,
installedCode: Long,
installedName: String,
): Boolean =
if (serverCode != null) {
serverCode > installedCode
} else {
isVersionNewer(serverName, installedName)
}
/**
* True when [server] is strictly newer than [installed]. Mirrors
* Flutter's `isVersionNewer` — splits both strings on `.`, parses
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.

Before

Width:  |  Height:  |  Size: 5.1 KiB

After

Width:  |  Height:  |  Size: 3.1 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 7.9 KiB

After

Width:  |  Height:  |  Size: 3.9 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 3.1 KiB

After

Width:  |  Height:  |  Size: 2.0 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 4.8 KiB

After

Width:  |  Height:  |  Size: 2.6 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 7.2 KiB

After

Width:  |  Height:  |  Size: 4.0 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 11 KiB

After

Width:  |  Height:  |  Size: 5.2 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 12 KiB

After

Width:  |  Height:  |  Size: 6.1 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 18 KiB

After

Width:  |  Height:  |  Size: 8.1 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 16 KiB

After

Width:  |  Height:  |  Size: 8.3 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 25 KiB

After

Width:  |  Height:  |  Size: 11 KiB

@@ -0,0 +1,16 @@
<?xml version="1.0" encoding="utf-8"?>
<!-- Google Fonts provider certificate hashes for downloadable fonts via
androidx.compose.ui.text.googlefonts.GoogleFont.Provider. Standard
values published by Google; copied verbatim from the AndroidX docs. -->
<resources>
<array name="com_google_android_gms_fonts_certs">
<item>@array/com_google_android_gms_fonts_certs_dev</item>
<item>@array/com_google_android_gms_fonts_certs_prod</item>
</array>
<string-array name="com_google_android_gms_fonts_certs_dev">
<item>MIIEqDCCA5CgAwIBAgIJANWFuGx90071MA0GCSqGSIb3DQEBBAUAMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTAeFw0wODA0MTUyMzM2NTZaFw0zNTA5MDEyMzM2NTZaMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbTCCASAwDQYJKoZIhvcNAQEBBQADggENADCCAQgCggEBANbOLggKv+IxTdGNs8/TGFy0PTP6DHThvbbR24kT9ixcOd9W+EaBPWW+wPPKQmsHxajtWjmQwWfna8mZuSeJS48LIgAZlKkpoyLcfobBPv6yyz8x1IxWWmF9c1IGN3vSL6BLNJEUyMEPzC2WZdwT4ZG2cuJTtzeETl6jWFKx68ETtZxNVHe9Iy9NMxEljDqVZ4y6+FlHaiYJqq3LcJpJVuKYz4kvOcyf3M0nDA8mUlVdfsOlw/H4uoNQ7VrAQUKB4kAyfxsKp/RZmnZSJ7+8Ag9aTC+oguTd1iFNuMqDUlpePo6CGuh73iKuq8mYvtdQQ0Yz+mF4j2YWB7Gj0R1k2cCAQOjgfwwgfkwHQYDVR0OBBYEFI0cxb6VTEM8YYY6FbBMvAPyT+CyMIHJBgNVHSMEgcEwgb6AFI0cxb6VTEM8YYY6FbBMvAPyT+CyoYGapIGXMIGUMQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEWMBQGA1UEBxMNTW91bnRhaW4gVmlldzEQMA4GA1UEChMHQW5kcm9pZDEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDEiMCAGCSqGSIb3DQEJARYTYW5kcm9pZEBhbmRyb2lkLmNvbYIJANWFuGx90071MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQADggEBABnTDPEF+3iSP0wNfdIjIz1AlnrPzgAIHVvXxunW7SBrDhEglQZBbKJEk5kT0mtKoOD1JMrSu1xuTKEBahWRbqHsXclaXjoBADb0kkjVEJu/Lh5hgYZnOjvlba8Ld7HCKePCVePoTJBdI4fvugnL8TsgK05aIskyY0hKI9L8KfqfGTl1lzOv2KoWD0KWwtAWPoGChZxmQ+nBli+gwYMzM1vAkP+aayLe0a1EQimlOalO762r0GXO0ks+UeXde2Z4e+8S/pf7pITEI/tP+MxJTALw9QUWEv9lKTk+jkbqxbsh8nfBUapfKqYn0eidpwq2AzVp3juYl7//fKnaPhJD9gs=</item>
</string-array>
<string-array name="com_google_android_gms_fonts_certs_prod">
<item>MIIEQzCCAyugAwIBAgIJAMLgh0ZkSjCNMA0GCSqGSIb3DQEBBAUAMHQxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDAeFw0wODA4MjEyMzEzMzRaFw0zNjAxMDcyMzEzMzRaMHQxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKtWLgDYO6IIrgqWbxJOKdoR8qtW0I9Y4sypEwPpt1TTcvZApxsdyxMJZ2JORland2qSGT2y5b+3JKkedxiLDmpHpDsz2WCbdxgxRczfey5YZnTJ4VZbH0xqWVW/8lGmPav5xVwnIiJS6HXk+BVKZF+JcWjAsb/GEuq/eFdpuzSqeYTcfi6idkyugwfYwXFU1+5fZKUaRKYCwkkFQVfcAs1fXA5V+++FGfvjJ/CxURaSxaBvGdGDhfXE28LWuT9ozCl5xw4Yq5OGazvV24mZVSoOO0yZ31j7kYvtwYK6NeADwbSxDdJEqO4k//0zOHKrUiGYXtqw/A0LFFtqoZKFjnkCAwEAAaOB1zCB1DAdBgNVHQ4EFgQUhzkS9E6G+x8U7eIYZVgWyN4j2u4wgaQGA1UdIwSBnDCBmYAUhzkS9E6G+x8U7eIYZVgWyN4j2u6heKR2MHQxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpDYWxpZm9ybmlhMRYwFAYDVQQHEw1Nb3VudGFpbiBWaWV3MRQwEgYDVQQKEwtHb29nbGUgSW5jLjEQMA4GA1UECxMHQW5kcm9pZDEQMA4GA1UEAxMHQW5kcm9pZIIJAMLgh0ZkSjCNMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEEBQADggEBABywqUAtNkXf2EVQuRGiI3pnNvIYx7N5xj4LMtloEdEqMpEcMa6Qe87qDx2hsArOR1nzQAFGsT/8YIIfX0fAJjQuP1lAcExSxVKbFICEvFBaWuhGgOOZ7CYzfHB6tEzJFLR2DQHQrXLT2HKDDhxhe9hKzqIRDSc5Hjr3jY5MMzfYM5lFvKK9pLqEsP6/Ad9SDhupcVoOWVrSCNKfRb6jpJbZuxJhCnq8tmlV4iy5tEW0a3VBYzpRoBdAaORWqHQTUlt+iL3aH7C5OxhgN/JuxvxXBL/3kkc0wK1ZNuk+sb4lNXmHnVqQYTcyowQHRPCRsPzCCl4ANULRpZjxAd0xUgg=</item>
</string-array>
</resources>
@@ -1,60 +0,0 @@
package com.fabledsword.minstrel.api
import okhttp3.MediaType.Companion.toMediaType
import okhttp3.ResponseBody.Companion.toResponseBody
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Test
import retrofit2.HttpException
import retrofit2.Response
import java.io.IOException
class ErrorCopyTest {
private fun httpError(status: Int, body: String): HttpException =
HttpException(
Response.error<Unit>(status, body.toResponseBody("application/json".toMediaType())),
)
@Test
fun libraryNotWritableAppendsTheServerDetail() {
val detail = "Minstrel runs as uid 1000, gid 1000 and cannot delete from /music/A " +
"(read-only file system). The library mount must be writable by that user. " +
"Nothing was deleted."
val e = httpError(409, """{"error":{"code":"library_not_writable","message":"$detail"}}""")
assertEquals(
"${ErrorCopy.messageFor("library_not_writable")} $detail",
ErrorCopy.fromThrowable(e),
)
}
@Test
fun detailCodeWithoutAMessageShowsTheCopyAlone() {
val e = httpError(409, """{"error":{"code":"library_not_writable","message":""}}""")
assertEquals(ErrorCopy.messageFor("library_not_writable"), ErrorCopy.fromThrowable(e))
}
// Server messages are usually internal detail; appending them for every
// code would leak driver errors into snackbars. This pins the scope.
@Test
fun otherCodesNeverCarryTheServerMessage() {
val e = httpError(404, """{"error":{"code":"track_not_found","message":"pgx: no rows"}}""")
assertEquals(ErrorCopy.messageFor("track_not_found"), ErrorCopy.fromThrowable(e))
}
@Test
fun anUnparseableBodyFallsBackToUnknown() {
val e = httpError(500, "not json")
assertEquals(ErrorCopy.messageFor("unknown"), ErrorCopy.fromThrowable(e))
}
@Test
fun transportFailureMapsToConnectionRefused() {
assertEquals(
ErrorCopy.messageFor("connection_refused"),
ErrorCopy.fromThrowable(IOException("refused")),
)
}
}
@@ -1,128 +0,0 @@
package com.fabledsword.minstrel.diagnostics
import com.fabledsword.minstrel.player.TransportObservation
import org.junit.jupiter.api.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertNull
/**
* The rule deciding when renderer transport changes are worth recording as an
* episode. Worth pinning because both failure directions are costly: too eager
* and every track transition writes a summary that buries the real one, too
* shy and the operator's stutter goes unmeasured for another month.
*/
class TransportFlapDetectorTest {
private fun obs(state: String, atMs: Long, track: Int = 1, posMs: Long = 0L) =
TransportObservation(
state = state,
statusOk = true,
trackNumber = track,
positionMs = posMs,
playIntent = true,
atElapsedMs = atMs,
)
/**
* PLAYING -> TRANSITIONING -> PLAYING is what a queue advance looks like.
* It happens on every single track and must never be recorded as a fault.
*/
@Test
fun `an ordinary track transition is not an episode`() {
val d = TransportFlapDetector()
assertNull(d.onChange(obs("PLAYING", 0)))
assertNull(d.onChange(obs("TRANSITIONING", 1_000)))
assertNull(d.onChange(obs("PLAYING", 2_000)))
}
@Test
fun `four changes inside the window is an episode`() {
val d = TransportFlapDetector()
d.onChange(obs("PLAYING", 0))
d.onChange(obs("STOPPED", 500))
d.onChange(obs("PLAYING", 1_000))
// assertNotNull returns the value, so the asserts below need no cast.
val episode = assertNotNull(d.onChange(obs("STOPPED", 1_500)))
assertEquals(4, episode.size)
assertEquals(
listOf("PLAYING", "STOPPED", "PLAYING", "STOPPED"),
episode.map { it.state },
)
}
/**
* Changes spread thinly are normal listening — a few track advances over
* a couple of minutes must not accumulate into a false episode.
*/
@Test
fun `changes spread beyond the window never accumulate`() {
val d = TransportFlapDetector()
repeat(20) { i ->
assertNull(d.onChange(obs("PLAYING", i * 10_000L)))
}
}
/** The window slides: old readings age out rather than counting forever. */
@Test
fun `readings older than the window are dropped`() {
val d = TransportFlapDetector()
d.onChange(obs("PLAYING", 0))
d.onChange(obs("STOPPED", 1_000))
// Long gap — the two above are now stale.
assertNull(d.onChange(obs("PLAYING", 30_000)))
assertNull(d.onChange(obs("STOPPED", 30_500)))
// Only three fresh readings so far.
assertNull(d.onChange(obs("PLAYING", 31_000)))
assertNotNull(d.onChange(obs("STOPPED", 31_500)))
}
/**
* A fault that persists produces a change every poll. Without the cooldown
* every one of them would write a summary, which is exactly the noise that
* makes a diagnostics dump unreadable.
*/
@Test
fun `a sustained fault reports one episode, not one per reading`() {
val d = TransportFlapDetector()
var episodes = 0
repeat(40) { i ->
if (d.onChange(obs(if (i % 2 == 0) "PLAYING" else "STOPPED", i * 500L)) != null) {
episodes++
}
}
assertEquals(1, episodes)
}
/** Past the cooldown, a fresh episode is worth recording again. */
@Test
fun `a later episode reports again once the cooldown has passed`() {
val d = TransportFlapDetector()
repeat(4) { d.onChange(obs("PLAYING", it * 500L)) }
val second = (0 until 4).map { d.onChange(obs("STOPPED", 90_000 + it * 500L)) }
assertEquals(1, second.count { it != null })
}
@Test
fun `reset forgets the window and the cooldown`() {
val d = TransportFlapDetector()
repeat(4) { d.onChange(obs("PLAYING", it * 500L)) }
d.reset()
repeat(3) { d.onChange(obs("PLAYING", 3_000 + it * 500L)) }
// A 4th change after reset is a new episode, cooldown notwithstanding.
assertNotNull(d.onChange(obs("STOPPED", 5_000)))
}
/** The episode is a snapshot — later readings must not mutate it. */
@Test
fun `a returned episode is not mutated by later readings`() {
val d = TransportFlapDetector()
d.onChange(obs("PLAYING", 0))
d.onChange(obs("STOPPED", 500))
d.onChange(obs("PLAYING", 1_000))
val episode = assertNotNull(d.onChange(obs("STOPPED", 1_500)))
val sizeAtCapture = episode.size
repeat(5) { d.onChange(obs("PLAYING", 2_000 + it * 500L)) }
assertEquals(sizeAtCapture, episode.size)
}
}
@@ -19,35 +19,15 @@ class RemoteStallWatchdogTest {
playIntent: Boolean = true,
positionMs: Long = 0L,
nowMs: Long = 0L,
queue: RemoteStallWatchdog.QueueState = RemoteStallWatchdog.QueueState.UNKNOWN,
) = onPoll(
RemoteStallWatchdog.Poll(
trackUri = trackUri,
state = state,
statusOk = statusOk,
playIntent = playIntent,
positionMs = positionMs,
nowMs = nowMs,
queue = queue,
),
)
) = onPoll(trackUri, state, statusOk, playIntent, positionMs, nowMs)
/** Drive [n] stopped polls and return the last decision. */
private fun RemoteStallWatchdog.stopFor(
n: Int,
nowMs: Long = 0L,
queue: RemoteStallWatchdog.QueueState = RemoteStallWatchdog.QueueState.UNKNOWN,
trackUri: String = uri,
): RemoteStallWatchdog.Decision {
var last: RemoteStallWatchdog.Decision = RemoteStallWatchdog.Decision.None
repeat(n) {
last = poll(
trackUri = trackUri,
state = TransportState.STOPPED,
nowMs = nowMs,
queue = queue,
)
}
repeat(n) { last = poll(state = TransportState.STOPPED, nowMs = nowMs) }
return last
}
@@ -194,106 +174,6 @@ class RemoteStallWatchdogTest {
assertEquals(60_000L, again.resumeAtMs)
}
// A stopped renderer can mean three different things. Before QueueState
// they were indistinguishable, and all three were treated as a stall.
/**
* The regression that mattered most: reaching the end of the queue is how
* every listening session ends. Treating it as a stall meant retrying the
* last track three times and then raising a `stalled` error for playback
* that finished perfectly normally.
*/
@Test
fun `reaching the end of the queue is not a stall`() {
val w = RemoteStallWatchdog()
repeat(20) {
assertIs<RemoteStallWatchdog.Decision.None>(
w.stopFor(1, nowMs = it * 1_000L, queue = RemoteStallWatchdog.QueueState.COMPLETE),
)
}
}
/** And it must not quietly spend the budget it never needed. */
@Test
fun `a completed queue leaves the attempt budget untouched`() {
val w = RemoteStallWatchdog()
w.stopFor(5, queue = RemoteStallWatchdog.QueueState.COMPLETE)
// Same track, now genuinely stalled: full budget, first attempt.
val decision = w.stopFor(3, nowMs = 30_000L)
assertIs<RemoteStallWatchdog.Decision.Resume>(decision)
assertEquals(1, decision.attempt)
}
/**
* The bug behind all of this: the renderer stopped because it reached the
* end of a queue we failed to fully load. Re-playing the finished track is
* the wrong remedy — the queue is what's broken.
*/
@Test
fun `running off the end of a truncated queue asks for a repair`() {
val w = RemoteStallWatchdog()
val decision = w.stopFor(3, queue = RemoteStallWatchdog.QueueState.TRUNCATED)
assertIs<RemoteStallWatchdog.Decision.RepairQueue>(decision)
assertEquals(1, decision.attempt)
}
/** A renderer with tracks left that stopped anyway really has stalled. */
@Test
fun `stopping mid-queue is still a stall`() {
val w = RemoteStallWatchdog()
val decision = w.stopFor(3, queue = RemoteStallWatchdog.QueueState.HAS_MORE)
assertIs<RemoteStallWatchdog.Decision.Resume>(decision)
}
/**
* A renderer that doesn't report NrTracks usefully must keep the old
* behaviour rather than being told its queue is fine or broken.
*/
@Test
fun `an unknown queue state falls back to resuming`() {
val w = RemoteStallWatchdog()
assertIs<RemoteStallWatchdog.Decision.Resume>(
w.stopFor(3, queue = RemoteStallWatchdog.QueueState.UNKNOWN),
)
}
/** Repairs are bounded by the same budget, so a renderer that will not
* grow its queue stops being asked. */
@Test
fun `repairs are capped and then it gives up`() {
val w = RemoteStallWatchdog()
val truncated = RemoteStallWatchdog.QueueState.TRUNCATED
assertIs<RemoteStallWatchdog.Decision.RepairQueue>(
w.stopFor(3, nowMs = 0L, queue = truncated),
)
assertIs<RemoteStallWatchdog.Decision.RepairQueue>(
w.stopFor(1, nowMs = 5_000L, queue = truncated),
)
assertIs<RemoteStallWatchdog.Decision.RepairQueue>(
w.stopFor(1, nowMs = 10_000L, queue = truncated),
)
assertIs<RemoteStallWatchdog.Decision.GiveUp>(
w.stopFor(1, nowMs = 15_000L, queue = truncated),
)
}
/**
* A successful repair adds tracks, so the renderer moves on to one it had
* never seen. That is a new track, which restores the budget by the same
* rule any other track change does.
*/
@Test
fun `a repair that works hands the next track a full budget`() {
val w = RemoteStallWatchdog()
assertIs<RemoteStallWatchdog.Decision.RepairQueue>(
w.stopFor(3, queue = RemoteStallWatchdog.QueueState.TRUNCATED),
)
w.poll(trackUri = other, state = TransportState.PLAYING, nowMs = 6_000L)
val next = w.stopFor(3, nowMs = 30_000L, trackUri = other)
assertIs<RemoteStallWatchdog.Decision.Resume>(next)
assertEquals(1, next.attempt)
}
@Test
fun `reset forgets everything`() {
val w = RemoteStallWatchdog()
@@ -183,52 +183,6 @@ class AVTransportClientTest {
}
}
@Test
fun `getMediaInfo parses NrTracks and CurrentURI`() = runTest {
server.enqueue(
MockResponse().setBody(
"""<?xml version="1.0"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
<s:Body>
<u:GetMediaInfoResponse
xmlns:u="urn:schemas-upnp-org:service:AVTransport:1">
<NrTracks>42</NrTracks>
<MediaDuration>0:00:00</MediaDuration>
<CurrentURI>x-rincon-queue:RINCON_ABC#0</CurrentURI>
</u:GetMediaInfoResponse>
</s:Body>
</s:Envelope>""".trimIndent(),
),
)
val info = client.getMediaInfo()
assertEquals(42, info.nrTracks)
assertEquals("x-rincon-queue:RINCON_ABC#0", info.currentUri)
}
/**
* A renderer that omits NrTracks reads as 0, which callers must treat as
* "unknown". Parsing it as anything else would let an unhelpful renderer
* be mistaken for one with an empty queue — and the repair path would
* then re-send the whole queue to a device playing it perfectly well.
*/
@Test
fun `getMediaInfo reports zero when the renderer omits NrTracks`() = runTest {
server.enqueue(
MockResponse().setBody(
"""<?xml version="1.0"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
<s:Body>
<u:GetMediaInfoResponse
xmlns:u="urn:schemas-upnp-org:service:AVTransport:1">
<CurrentURI>http://x/y.mp3</CurrentURI>
</u:GetMediaInfoResponse>
</s:Body>
</s:Envelope>""".trimIndent(),
),
)
assertEquals(0, client.getMediaInfo().nrTracks)
}
private fun emptyResponse(action: String): MockResponse = MockResponse().setBody(
"""<?xml version="1.0"?>
<s:Envelope xmlns:s="http://schemas.xmlsoap.org/soap/envelope/">
@@ -1,125 +0,0 @@
package com.fabledsword.minstrel.theme
import org.junit.jupiter.api.Test
import java.io.File
import kotlin.test.assertEquals
import kotlin.test.assertTrue
/**
* Guards that the typefaces ship inside the APK instead of being fetched at
* runtime.
*
* Until 2026-09-09 these were resolved through the Play Services font
* provider. That needs a network the deployed app is not guaranteed, and a
* provider that devices without Play Services do not have at all. Both
* failures are silent — text just renders in the platform default, which
* reads as a styling regression rather than a missing dependency.
*
* Expectations are read out of Typography.kt itself rather than hardcoded, so
* this cannot drift away from what the app actually declares: adding a face
* without vendoring its file fails here, and so does changing a declared
* weight without refetching the matching static instance.
*/
class BundledFontsTest {
@Test
fun `typography builds its families from bundled resources`() {
val source = typographySource()
assertTrue(
source.contains("R.font."),
"Typography.kt should build its families from res/font resources",
)
FORBIDDEN.forEach { symbol ->
assertTrue(
!source.contains(symbol),
"Typography.kt must not reference $symbol — fonts are bundled, not fetched",
)
}
}
@Test
fun `every declared face is vendored as TrueType at its declared weight`() {
val declared = FACE_PATTERN.findAll(typographySource()).toList()
assertTrue(
declared.isNotEmpty(),
"no Font(R.font.…, FontWeight.W…) declarations found — the guard would pass vacuously",
)
declared.forEach { match ->
val (name, weight) = match.destructured
val file = File(appDir(), "src/main/res/font/$name.ttf")
assertTrue(file.isFile, "res/font/$name.ttf is missing — run tools/vendor-fonts.py")
val bytes = file.readBytes()
assertTrue(
bytes.copyOfRange(0, TTF_MAGIC.size).contentEquals(TTF_MAGIC),
"$name.ttf is not TrueType — res/font cannot load a woff2 or an eot",
)
// The decisive check. Google's css2 endpoint silently collapses a
// multi-weight request to 400 for legacy clients, so Medium can
// come back as Regular: a valid TrueType file that renders at the
// wrong weight everywhere. usWeightClass is the only field that
// tells the two apart.
assertEquals(
weight.toInt(),
weightClass(bytes),
"$name.ttf carries a different OS/2 usWeightClass than the FontWeight declared beside it",
)
}
}
/** Typography.kt with comments removed, so prose naming the forbidden
* symbols cannot satisfy — or trip — the absence check above. */
private fun typographySource(): String =
File(appDir(), TYPOGRAPHY)
.readText()
.replace(BLOCK_COMMENT, "")
.replace(LINE_COMMENT, "")
/** Gradle's working directory for tests is the module dir, but don't rely
* on it: walk up until the module is found, and say so if it isn't. */
private fun appDir(): File {
var dir: File? = File("").absoluteFile
while (dir != null) {
if (File(dir, TYPOGRAPHY).isFile) return dir
if (File(dir, "app/$TYPOGRAPHY").isFile) return File(dir, "app")
dir = dir.parentFile
}
error("could not locate the app module from ${File("").absolutePath}")
}
private fun weightClass(bytes: ByteArray): Int {
val tables = readU16(bytes, NUM_TABLES)
for (i in 0 until tables) {
val record = TABLE_DIRECTORY + i * TABLE_RECORD
if (String(bytes, record, TAG_LENGTH, Charsets.US_ASCII) == "OS/2") {
return readU16(bytes, readU32(bytes, record + OFFSET_FIELD) + WEIGHT_FIELD)
}
}
error("no OS/2 table in the font")
}
private fun readU16(bytes: ByteArray, at: Int): Int =
((bytes[at].toInt() and BYTE_MASK) shl Byte.SIZE_BITS) or (bytes[at + 1].toInt() and BYTE_MASK)
private fun readU32(bytes: ByteArray, at: Int): Int =
(readU16(bytes, at) shl Short.SIZE_BITS) or readU16(bytes, at + 2)
private companion object {
const val TYPOGRAPHY = "src/main/java/com/fabledsword/minstrel/theme/Typography.kt"
val FORBIDDEN = listOf("GoogleFont", "googlefonts")
val FACE_PATTERN = Regex("""R\.font\.(\w+)\s*,\s*FontWeight\.W(\d+)""")
val BLOCK_COMMENT = Regex("""/\*[\s\S]*?\*/""")
val LINE_COMMENT = Regex("""//.*""")
val TTF_MAGIC = byteArrayOf(0x00, 0x01, 0x00, 0x00)
// Offsets into the TrueType table directory, per the OpenType spec.
const val NUM_TABLES = 4
const val TABLE_DIRECTORY = 12
const val TABLE_RECORD = 16
const val TAG_LENGTH = 4
const val OFFSET_FIELD = 8
const val WEIGHT_FIELD = 4
const val BYTE_MASK = 0xFF
}
}
@@ -1,163 +0,0 @@
package com.fabledsword.minstrel.update.data
import org.junit.jupiter.api.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue
/**
* The update channel had no tests at all before this. That is worth saying
* out loud, because the thing it decides — whether anyone is ever offered an
* update — fails silently in both directions: an update nobody is offered
* looks exactly like being up to date, and nobody files a bug about a prompt
* they never saw.
*/
class UpdateVersioningTest {
@Test
fun `decides on the ordering key when the server reports one`() {
assertTrue(
isUpdateAvailable(
serverCode = 3523847, serverName = "2026.09.10.1432",
installedCode = 3519456, installedName = "2026.09.09.1828",
),
)
assertFalse(
isUpdateAvailable(
serverCode = 3519456, serverName = "2026.09.09.1828",
installedCode = 3523847, installedName = "2026.09.10.1432",
),
)
}
@Test
fun `an equal ordering key is not an update`() {
assertFalse(
isUpdateAvailable(
serverCode = 3523847, serverName = "2026.09.10.1432",
installedCode = 3523847, installedName = "2026.09.10.1432",
),
)
}
/**
* The property the whole rework exists for: the offer must agree with what
* the platform will actually install. Where the two disagree, the ordering
* key wins, because that is the value Android compares.
*/
@Test
fun `the ordering key wins even when the name disagrees`() {
// Name looks older, key is newer — e.g. an older commit rebuilt later.
assertTrue(
isUpdateAvailable(
serverCode = 9_000_000, serverName = "2020.01.01.0000",
installedCode = 1, installedName = "2099.12.31.2359",
),
)
// Name looks newer, key is not. Offering this would be offering an
// install the platform then refuses as a downgrade.
assertFalse(
isUpdateAvailable(
serverCode = 1, serverName = "2099.12.31.2359",
installedCode = 9_000_000, installedName = "2020.01.01.0000",
),
)
}
@Test
fun `falls back to the name when the server reports no ordering key`() {
assertTrue(
isUpdateAvailable(
serverCode = null, serverName = "2026.09.10.1432",
installedCode = 3519456, installedName = "2026.09.09.1828",
),
)
assertFalse(
isUpdateAvailable(
serverCode = null, serverName = "2026.09.09.1828",
installedCode = 3519456, installedName = "2026.09.10.1432",
),
)
}
/**
* A null code must never be read as zero. Zero would rank every
* older server as infinitely behind and offer its build to everyone,
* forever — so this asserts the fallback runs instead of a comparison
* against 0 succeeding by accident.
*/
@Test
fun `a null ordering key is absent, not zero`() {
// installedCode is 0 here: if null coerced to 0, "0 > 0" would be
// false and this would wrongly report no update despite a newer name.
assertTrue(
isUpdateAvailable(
serverCode = null, serverName = "2026.09.10.1432",
installedCode = 0, installedName = "2026.09.09.1828",
),
)
}
/**
* The recorded migration constraint, pinned so it cannot be forgotten:
* the old scheme's fourth segment was a commit count (~1895), the new
* one is HHMM. Across a day boundary the date decides and all is well.
*/
@Test
fun `new-scheme name outranks an old-scheme name on a later day`() {
assertTrue(isVersionNewer("2026.09.10.1432", "2026.09.09.1895"))
}
/**
* ...but on the SAME day the comparison comes down to HHMM against a
* commit count, and any build before ~19:00 UTC reads as older. This is
* why the first new-scheme release had to be cut on a later calendar day.
* Asserting the trap so nobody "fixes" it by accident.
*/
@Test
fun `same-day new-scheme name can read older than an old-scheme name`() {
assertFalse(isVersionNewer("2026.09.09.1828", "2026.09.09.1895"))
}
@Test
fun `name comparison degrades per segment rather than discarding`() {
// The string is still compared rather than rejected outright: an
// earlier segment decides and the unparseable tail never matters.
assertTrue(isVersionNewer("2026.09.10.1432-dev", "2026.09.09.1828"))
// A shorter name pads with zeros instead of being refused.
assertTrue(isVersionNewer("2026.09.10", "2026.09.09.9999"))
assertFalse(isVersionNewer("2026.09.10", "2026.09.10.0"))
}
/**
* What "costs that segment's precision" actually means, and it is worth
* pinning because it is a real edge rather than a nicety: when the
* unparseable segment is the DECIDING one, it reads as 0 and loses. So a
* `-dev` suffixed build compares as older than an unsuffixed one from the
* same minute.
*
* That is the correct behaviour for a degrading parser — it is bounded
* loss rather than a discarded string — but it is exactly why the channel
* belongs in its own field and never in the name.
*/
@Test
fun `an unparseable deciding segment reads as zero and loses`() {
assertFalse(isVersionNewer("2026.09.10.1432-dev", "2026.09.10.1000"))
}
/**
* Both sides unparseable (branch-name builds) falls back to string
* inequality, so a dev build still surfaces rather than comparing equal
* and going silent.
*/
@Test
fun `two unparseable names fall back to string inequality`() {
assertTrue(isVersionNewer("main", "dev"))
assertFalse(isVersionNewer("dev", "dev"))
}
@Test
fun `a leading v is ignored on either side`() {
assertTrue(isVersionNewer("v2026.09.10.1432", "2026.09.09.1828"))
assertFalse(isVersionNewer("v2026.09.10.1432", "v2026.09.10.1432"))
}
}
+1
View File
@@ -53,6 +53,7 @@ compose-ui-graphics = { module = "androidx.compose.ui:ui-graphics" }
compose-ui-tooling = { module = "androidx.compose.ui:ui-tooling" }
compose-ui-tooling-preview = { module = "androidx.compose.ui:ui-tooling-preview" }
compose-material3 = { module = "androidx.compose.material3:material3" }
compose-ui-text-google-fonts = { module = "androidx.compose.ui:ui-text-google-fonts" }
hilt-android = { module = "com.google.dagger:hilt-android", version.ref = "hilt" }
hilt-compiler = { module = "com.google.dagger:hilt-compiler", version.ref = "hilt" }
room-runtime = { module = "androidx.room:room-runtime", version.ref = "room" }
Executable
BIN
View File
Binary file not shown.
+47 -20
View File
@@ -62,30 +62,57 @@ None.
- **Go toolchain pin.** `go.mod` is on `go 1.25.0` because `golang.org/x/crypto v0.51.0` declares 1.25 as its minimum. `ci-go:1.26` satisfies this with headroom. Future `x/crypto` bumps that move the Go floor should be paired with an image-tag bump in this file + the workflows.
- **In-app update channel — `needs:`, not polling.** `release.yml`'s `image-release` job declares `needs: [android-release]`, so on tag pushes the signed APK is guaranteed present before the image build starts — no polling window, no race. (The old cross-workflow polling against `flutter.yml` is gone with that workflow.) On non-tag `main` pushes `android-release` is skipped and `image-release` instead pulls the most recent release's APK and reconstructs its exact `versionName`, so `:latest` never ships without an update channel. It degrades to an empty `client/` — never a wrong version — if no release, asset, or tag commit-count can be resolved.
- **Cache server reachability.** `test-web.yml` does NOT use `cache: 'npm'` on `actions/setup-node` — the Gitea Actions cache server isn't reachable from this runner's container network and `setup-node` was burning ~4m41s on ETIMEDOUT before failing open. With the migration to `ci-go:1.26`, `setup-node` is removed entirely (Node is in the image). The cache concern reappears if a future change re-introduces a network-dependent action.
- **Artifacts — stock `actions/upload-artifact@v7` and `actions/download-artifact@v8`; never `@v3`.**
- **Artifacts — use the mirrored actions, never `actions/{upload,download}-artifact`.**
```yaml
uses: actions/upload-artifact@v7
uses: actions/download-artifact@v8
uses: https://git.fabledsword.com/bvandeusen/upload-artifact@cb8afe72b42edc798abfb8fcb556cf660d894245
uses: https://git.fabledsword.com/bvandeusen/download-artifact@8d4e9521a5f7e5f8b6351f341f719f9f45a92a3a
```
Stock works on this forge since the runner moved to gitea/runner 3.x, which
edits the actions' client-side `isGhes()` refusal out of their bundles. Proven
on 2026-09-10 for upload v4v7 and download v4v8 (Scribe spike #3843). Until
then this repo pinned SHA mirrors of the Forgejo project's forks, because
upstream threw on the hostname before it opened a connection (Scribe 2255).
Upstream's `@v4+` cannot work against this instance and no server-side change
will help: `isGhes()` rejects any hostname that isn't `github.com` /
`*.ghe.com` / `*.localhost` and throws before it opens a connection, so the
server is never asked what it supports. `@v3` is worse — it reports success,
and Gitea then serves artifacts back only through the v4 API
(`content_encoding = application/zip`), so a v3 upload is stored but invisible
to every retrieval path. A green job producing nothing retrievable; that is how
72 unreachable artifacts accumulated on this repo. Scribe issues 2255 / 2270.
`@v3` is still broken: it reports success, and Gitea serves artifacts back only
through the v4 API (`content_encoding = application/zip`), so a v3 upload is
stored but invisible to every retrieval path. That is how 72 unreachable
artifacts accumulated on this repo (Scribe 2270).
Both are pull mirrors of the Forgejo project's forks
(`code.forgejo.org/forgejo/{upload,download}-artifact`, one commit on upstream
disabling that check), mirrored so CI depends on commits we hold and pinned by
SHA because the mirrors auto-sync every 8h — a moved upstream tag would
otherwise silently change what runs.
**Pairing no longer needs managing.** This entry used to pin upload v5 against
download v6 so both bundled `@actions/artifact` ^4.0.0, warning that a mismatch
across `release.yml`'s producer/consumer pair would list empty. Tested, and not
true on this instance: every download major v4v8 read the artifacts of every
upload major v4v7, by name and by pattern (CI-runner run 6312). The only real
protocol break is v3 → v4. node24 is no longer a concern either — every
CI-runner image carries Node 24 and the runner runs actions with the image's
`node`.
**Match the pins on `@actions/artifact`, not on the actions' own version
numbers.** The two actions release on unrelated cadences, so equal version
numbers do NOT mean a compatible pair — upload `v5` bundles `@actions/artifact`
^4.0.0 while download `v5` bundles ^2.3.2. The pins above are upload **v5** and
download **v6**, which is the pairing that puts ^4.0.0 on both sides. This
matters because `release.yml` is a producer/consumer pair — `android-release`
uploads `minstrel-apk`, `image-release` downloads it — and a protocol mismatch
across it yields an empty listing rather than an error, exactly the silent
failure this entry exists to prevent.
| tag | `@actions/artifact` | runtime |
|---|---|---|
| upload v4 | ^2.1.1 | node20 |
| **upload v5** ← pinned | **^4.0.0** | node20 |
| download v4 | ^2.1.1 | node20 |
| download v5 | ^2.3.2 | node20 |
| **download v6** ← pinned | **^4.0.0** | node20 |
| download v7 | ^5.0.0 | **node24** |
The only true protocol break in this history was **v3 → v4** (upstream:
"Downloading artifacts that were created from `actions/upload-artifact@v3` and
below are not supported"); v4-and-up are one family. Later majors are mostly
ergonomics and runtime — upload v4 forbids re-uploading a name and caps a job
at 500 artifacts; download v5 made by-ID extraction match by-name.
**Do not jump the download pin to v7.** That major is a runner requirement, not
a feature change: it moves to `runs.using: node24` and upstream states it
"requires a minimum Actions Runner version of 2.327.1 … if you are using
self-hosted runners, ensure they are updated before upgrading." act_runner is
not GitHub's runner and makes no such version claim, so node24 is unverified
here. Everything currently pinned is node20.
Upload steps set `if-no-files-found: error` rather than the default `warn`, so
an upload that matches nothing fails its own job instead of failing the
-159
View File
@@ -1,159 +0,0 @@
#!/usr/bin/env bash
#
# Derives the three values a build is stamped with, and the tag that names it.
#
# name=YYYY.MM.DD.HHMM label for people, from the timestamp of the newest
# commit that CHANGED SOMETHING SHIPPED (see SHIPPED)
# code=<int> ordering key, minutes since 2020-01-01 at BUILD time
# tag=v<name> what a release of this commit must be called
#
# Usage: ci/version.sh [<commit-ish>] (default HEAD)
#
# This exists as a script rather than inline workflow YAML for one reason:
# release.yml only runs on `main` and on tags, so anything living inside it is
# unverifiable until a release is already happening — which is the worst
# possible moment to discover the version is wrong, because the failure mode
# is silent (an update nobody is offered looks exactly like being current).
# As a script it can be executed by a test on every push instead.
#
# The two clocks are deliberate and are NOT interchangeable:
#
# The NAME answers "is this the same code?" — so it must read identically on
# every lane that builds this commit. Commit time does that; build time
# prints two different strings for one thing.
#
# The CODE answers "may this be installed over that?" — so it must be
# monotonic BY CONSTRUCTION. Build time is; commit time is not (rebuild an
# older commit and it goes down, which on a phone is a refused install), and
# a commit COUNT is worse still, because it runs ahead on `dev` and inverts
# against `main`.
set -euo pipefail
readonly EPOCH_2020=1577836800 # 2020-01-01T00:00:00Z
readonly REF="${1:-HEAD}"
# Both clocks are overridable so a test can pin them. Nothing but tests should
# set these — the defaults are the real derivation.
# The paths that do NOT ship, in either artifact. Everything else counts.
#
# A DENYLIST, and the direction is the whole point. As an allowlist, the list
# has to be updated by whoever adds a directory and nothing fails if they
# don't — so the failure mode is a changed artifact keeping its old version,
# silently, on a green run. That is a build lying about what it is. Inverted,
# new content counts by default and the only way to wrongly EXCLUDE something
# is to name it here deliberately.
#
# The two error directions are not symmetric, which is why this is not taste:
# wrongly excluded → changed artifact, unchanged version. A silent lie.
# wrongly included → version moves when nothing shipped. Cosmetic noise in
# a string nobody sorts.
#
# THIS REPO SHIPS TWO ARTIFACTS FROM ONE DERIVATION, and that is why the list
# is shorter than it looks like it should be. The server image ships cmd/,
# internal/, shared/, web/, config.example.yaml and client/; the APK ships
# android/. Neither ships the other's sources — but excluding android/ here
# would stop an Android-only commit from moving the APK's OWN version, which
# is the dangerous direction. So this is the union: exclude only what ships in
# NEITHER, and accept that an Android commit also nudges the server's reported
# version. Over-inclusion across the two, which is the harmless direction.
#
# The family's other repos (roundtable / roundtable-android) each keep a
# tighter list because they are separate repos with one artifact apiece. Do
# not copy theirs onto this one.
readonly SHIPPED=(
.
':!.gitea' # CI workflows — including this script's own caller
':!ci' # CI scripts — including this script
':!docs'
':!tools' # asset/font generators; their OUTPUT ships, they do not
':!deploy' # test-database bootstrap SQL
':!bin' # local `make build` output
':!*.md'
':!Makefile'
':!docker-compose.yml'
':!.env.example'
':!.gitignore'
':!.dockerignore'
':!renovate.json'
':!.golangci.yml'
# TESTS DO NOT SHIP, so they must not re-version an artifact.
#
# Named as globs rather than a directory because this repo has no tests/
# tree to exclude: Go tests sit inline beside the code they cover, and the
# web suite sits beside its modules. `go build` drops *_test.go outright and
# the Vite build never imports a .test.ts, so neither reaches an artifact.
#
# A commit touching a test AND its source still moves the version — the
# source path matches on its own. Only a test-ONLY commit is inert, which is
# the whole intent.
#
# Patterns match what exists today and nothing speculative: there are no
# .spec.* files, no __tests__/ directories and no androidTest/ tree. If any
# appear they will re-version until named here, which is the harmless
# direction and the reason this list is a denylist.
':!*_test.go' # 158 files, inline beside the code
':!*.test.ts' # 114 files
':!*.test.js'
':!android/app/src/test' # JVM unit tests; no androidTest tree exists
':!web/vitest.config.ts' # test-harness config, not build config
':!web/vitest.setup.ts'
)
commit_epoch="${MINSTREL_COMMIT_EPOCH:-}"
if [ -z "${commit_epoch}" ]; then
commit_epoch="$(git log --format=%ct -1 "${REF}" -- "${SHIPPED[@]}")"
# Loudly, on purpose. A silent fallback here is the landmine this whole
# script exists to avoid: a plausible-looking version that is quietly wrong,
# on a green run. Realistically this means a shallow clone (no commit in
# range touches the shipped set) rather than a repo of pure CI config.
if [ -z "${commit_epoch}" ]; then
echo "version.sh: no commit under '${REF}' touches the shipped file set — shallow clone? (needs fetch-depth: 0)" >&2
exit 1
fi
fi
now_epoch="${MINSTREL_NOW_EPOCH:-$(date -u +%s)}"
if ! name="$(date -u -d "@${commit_epoch}" +%Y.%m.%d.%H%M 2>/dev/null)"; then
echo "version.sh: could not read a commit timestamp from '${commit_epoch}'" >&2
exit 1
fi
if ! [ "${now_epoch}" -eq "${now_epoch}" ] 2>/dev/null; then
echo "version.sh: build timestamp '${now_epoch}' is not a number" >&2
exit 1
fi
code=$(( (now_epoch - EPOCH_2020) / 60 ))
# Assert the shape here, at the source. A malformed name builds, signs and
# publishes perfectly happily; it only surfaces later as an update channel
# that has quietly stopped offering anything.
if [[ ! "${name}" =~ ^[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}$ ]]; then
echo "version.sh: name '${name}' is not YYYY.MM.DD.HHMM" >&2
exit 1
fi
# A non-positive key means the build clock is set before 2020, and every
# comparison downstream would be nonsense.
if [ "${code}" -le 0 ]; then
echo "version.sh: ordering key '${code}' is not positive — build clock wrong?" >&2
exit 1
fi
# Android's versionCode is a signed 32-bit int and the platform refuses an APK
# whose code exceeds it. At ~525k minutes a year this is four thousand years
# away in normal operation, so the realistic cause is a build machine with a
# badly wrong clock — which produces a code that is not merely too large but
# also unreachably high, permanently blocking every real build that follows
# from ever outranking it. Cheaper to refuse the build than to discover that
# from a phone that will not update.
readonly VERSION_CODE_CEILING=2147483647
if [ "${code}" -gt "${VERSION_CODE_CEILING}" ]; then
echo "version.sh: ordering key '${code}' exceeds versionCode's int32 ceiling — build clock wrong?" >&2
exit 1
fi
# KEY=VALUE, which is also exactly $GITHUB_OUTPUT's format.
echo "name=${name}"
echo "code=${code}"
echo "tag=v${name}"
+1 -24
View File
@@ -122,15 +122,7 @@ func run() error {
}
defer pool.Close()
// Fingerprinting settings (M400 #3913): one instance, shared by the scanner,
// the fingerprint backfill, the duplicate sweep and the admin API, so a save
// reaches all of them without a restart. A load failure is logged, not fatal:
// the service falls back to the shipped defaults.
fpSettings, fpErr := library.NewFingerprintSettingsService(ctx, pool)
if fpErr != nil {
logger.Warn("fingerprint settings: using defaults", "err", fpErr)
}
scanner := library.New(pool, logger, cfg.Library.ScanPaths, fpSettings)
scanner := library.New(pool, logger, cfg.Library.ScanPaths)
contact := cfg.Library.ContactEmail
if contact == "" {
@@ -222,17 +214,6 @@ func run() error {
// SQL, no external calls; empty on single-user servers.
go coplay.NewWorker(pool, logger.With("component", "coplay")).Run(ctx)
// Fingerprint backfill (M400 #3908): fingerprints the tracks the scan never
// will — everything imported before fingerprinting existed, and rows derived
// by an older method. A worker of its own rather than a scan stage; see
// internal/library/fingerprint_backfill.go for why.
go library.NewFingerprintBackfillWorker(pool, logger.With("component", "fingerprint_backfill"), fpSettings).Run(ctx)
// Duplicate sweep (M400 #3910): proposes groups of tracks holding one
// recording, from the fingerprints above. Sweeps only when fingerprints have
// changed since the last sweep.
go library.NewDuplicateSweepWorker(pool, logger.With("component", "duplicate_sweep"), fpSettings).Run(ctx)
// Start the tag-enrichment worker (#1490). Reconciles the compiled-in
// tag providers with tag_provider_settings, bumps the sources version if
// the provider set changed (re-opening settled rows), then drains tracks
@@ -376,10 +357,6 @@ func run() error {
srv.PlaylistScheduler = playlistScheduler
srv.RecSettings = recSettings
srv.TagSettings = tagSettings
srv.FingerprintSettings = fpSettings
// The sweeper above holds this same instance, so a save from the admin
// card changes what it does on its next tick (#3936).
srv.ReacqSettings = reacqSettings
srv.StreamSecret = cfg.StreamSecret
httpServer := &http.Server{
Addr: cfg.Server.Address,
Binary file not shown.

Before

Width:  |  Height:  |  Size: 1.1 MiB

-45
View File
@@ -1,12 +1,10 @@
package api
import (
"context"
"net/http"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
// coverageRollupResp is the wire shape for GET /api/admin/library/coverage.
@@ -38,46 +36,3 @@ func (h *handlers) handleGetLibraryCoverage(w http.ResponseWriter, r *http.Reque
PendingNoMbid: row.PendingNoMbid,
})
}
// fingerprintCoverageResp is the wire shape for GET /api/admin/library/fingerprints.
// fingerprinted + rejected + pending = total. Missing tracks are not counted:
// there is no file to fingerprint. Enabled travels with the counts because with
// fingerprinting off (#3913) pending never shrinks, and a gauge that implies
// progress would be promising work nothing is doing.
type fingerprintCoverageResp struct {
Total int64 `json:"total"`
Fingerprinted int64 `json:"fingerprinted"`
Rejected int64 `json:"rejected"`
Pending int64 `json:"pending"`
Enabled bool `json:"enabled"`
}
// fingerprintCoverage reads the gauge against the current settings: a print at
// another length counts as pending, because the backfill will re-derive it.
func (h *handlers) fingerprintCoverage(ctx context.Context) (fingerprintCoverageResp, error) {
cfg := h.fingerprintSettings.Get()
row, err := library.FingerprintCoverage(ctx, h.pool, cfg)
if err != nil {
return fingerprintCoverageResp{}, err
}
return fingerprintCoverageResp{
Total: row.Total,
Fingerprinted: row.Fingerprinted,
Rejected: row.Rejected,
Pending: row.Pending,
Enabled: cfg.Enabled,
}, nil
}
// handleGetFingerprintCoverage implements GET /api/admin/library/fingerprints:
// how far the fingerprint backfill (#3908) has got. The backfill is its own
// worker spanning many passes, with no scan run to attach a tally to, so its
// progress is read live here. Always 200; zeros on an empty library.
func (h *handlers) handleGetFingerprintCoverage(w http.ResponseWriter, r *http.Request) {
cov, err := h.fingerprintCoverage(r.Context())
if err != nil {
writeErrWithLog(w, h.logger, "admin: get fingerprint coverage", apierror.InternalMsg("lookup failed", err))
return
}
writeJSON(w, http.StatusOK, cov)
}
-316
View File
@@ -1,316 +0,0 @@
package api
import (
"context"
"encoding/json"
"errors"
"io"
"net/http"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
// duplicateMemberView is one copy in a proposed duplicate group. LikeCount and
// PlayCount span every user: the report is admin-only, and what a copy carries
// is the fact the operator weighs when choosing which to keep.
type duplicateMemberView struct {
TrackID string `json:"track_id"`
Title string `json:"title"`
ArtistName string `json:"artist_name"`
AlbumID string `json:"album_id"`
AlbumTitle string `json:"album_title"`
FilePath string `json:"file_path"`
FileFormat string `json:"file_format"`
FileSize int64 `json:"file_size"`
DurationSec int32 `json:"duration_sec"`
AddedAt string `json:"added_at"`
LikeCount int64 `json:"like_count"`
PlayCount int64 `json:"play_count"`
}
// duplicateGroupView is one proposal. SurvivorTrackID and SurvivorReason are
// the copy the report proposes keeping and the rule that chose it
// (library.ProposeSurvivor) — a default the merge (#3911) lets the operator
// override.
type duplicateGroupView struct {
ID string `json:"id"`
Tier string `json:"tier"`
WorstBitErrorRate *float32 `json:"worst_bit_error_rate"`
DetectedAt string `json:"detected_at"`
SurvivorTrackID string `json:"survivor_track_id"`
SurvivorReason string `json:"survivor_reason"`
Members []duplicateMemberView `json:"members"`
}
// duplicateSweepView is the latest sweep. State is "never" when none has run,
// which is what lets the page tell an empty report apart from a sweep that
// found nothing.
type duplicateSweepView struct {
State string `json:"state"`
StartedAt *string `json:"started_at"`
FinishedAt *string `json:"finished_at"`
Candidates *int32 `json:"candidates"`
GroupsFound *int32 `json:"groups_found"`
OversizeClusters *int32 `json:"oversize_clusters"`
ErrorMessage *string `json:"error_message"`
}
// adminDuplicatesResponse is the paged report. Total counts groups.
type adminDuplicatesResponse struct {
Sweep duplicateSweepView `json:"sweep"`
Fingerprints fingerprintCoverageResp `json:"fingerprints"`
Total int64 `json:"total"`
Limit int `json:"limit"`
Offset int `json:"offset"`
Groups []duplicateGroupView `json:"groups"`
}
// handleListDuplicates implements GET /api/admin/library/duplicates (#3912).
//
// Read-only. The sweep's state and the fingerprint backfill's progress travel
// with the groups because an empty report means three different things — still
// fingerprinting, never swept, or swept and clean — and the page has to say which.
func (h *handlers) handleListDuplicates(w http.ResponseWriter, r *http.Request) {
limit, offset, err := parsePaging(r.URL.Query())
if err != nil {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_paging")
return
}
ctx := r.Context()
q := dbq.New(h.pool)
sweep := duplicateSweepView{State: "never"}
last, err := q.GetLatestDuplicateSweep(ctx)
switch {
case err == nil:
sweep = duplicateSweepViewOf(last)
case !errors.Is(err, pgx.ErrNoRows):
h.logger.Error("admin: latest duplicate sweep", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
cov, err := h.fingerprintCoverage(ctx)
if err != nil {
h.logger.Error("admin: fingerprint coverage", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
total, err := q.CountPendingDuplicateGroups(ctx)
if err != nil {
h.logger.Error("admin: count duplicate groups", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
rows, err := q.ListPendingDuplicateGroupMembers(ctx, dbq.ListPendingDuplicateGroupMembersParams{
PageLimit: int32(limit), PageOffset: int32(offset),
})
if err != nil {
h.logger.Error("admin: list duplicate groups", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
writeJSON(w, http.StatusOK, adminDuplicatesResponse{
Sweep: sweep,
Fingerprints: cov,
Total: total,
Limit: limit,
Offset: offset,
Groups: foldDuplicateGroups(rows),
})
}
func duplicateSweepViewOf(s dbq.DuplicateSweep) duplicateSweepView {
v := duplicateSweepView{
State: "running",
Candidates: s.Candidates,
GroupsFound: s.GroupsFound,
OversizeClusters: s.OversizeClusters,
ErrorMessage: s.ErrorMessage,
}
started := formatTimestamp(s.StartedAt)
v.StartedAt = &started
if s.FinishedAt.Valid {
finished := formatTimestamp(s.FinishedAt)
v.FinishedAt = &finished
v.State = "finished"
}
return v
}
// foldDuplicateGroups folds the one-row-per-member query result into groups and
// proposes each group's survivor. It relies on the query ordering members of a
// group together, so a run-length fold is enough and the page order holds.
func foldDuplicateGroups(rows []dbq.ListPendingDuplicateGroupMembersRow) []duplicateGroupView {
groups := make([]duplicateGroupView, 0, 8)
var candidates [][]library.SurvivorCandidate
for _, row := range rows {
id := uuidToString(row.GroupID)
if n := len(groups); n == 0 || groups[n-1].ID != id {
groups = append(groups, duplicateGroupView{
ID: id,
Tier: row.Tier,
WorstBitErrorRate: row.WorstBitErrorRate,
DetectedAt: formatTimestamp(row.DetectedAt),
})
candidates = append(candidates, nil)
}
n := len(groups) - 1
trackID := uuidToString(row.TrackID)
groups[n].Members = append(groups[n].Members, duplicateMemberView{
TrackID: trackID,
Title: row.Title,
ArtistName: row.ArtistName,
AlbumID: uuidToString(row.AlbumID),
AlbumTitle: row.AlbumTitle,
FilePath: row.FilePath,
FileFormat: row.FileFormat,
FileSize: row.FileSize,
DurationSec: row.DurationMs / 1000,
AddedAt: formatTimestamp(row.AddedAt),
LikeCount: row.LikeCount,
PlayCount: row.PlayCount,
})
candidates[n] = append(candidates[n], library.SurvivorCandidate{
TrackID: trackID, FileFormat: row.FileFormat, FileSize: row.FileSize, AddedAt: row.AddedAt.Time,
})
}
for i := range groups {
groups[i].SurvivorTrackID, groups[i].SurvivorReason = library.ProposeSurvivor(candidates[i])
}
return groups
}
// handleRunDuplicateSweep implements POST /api/admin/library/duplicates/sweep:
// 202 when a sweep starts, 409 sweep_in_progress when one is already running.
// The sweep outlives the request, so it runs on a background context, as
// handleTriggerScan's scan does.
func (h *handlers) handleRunDuplicateSweep(w http.ResponseWriter, _ *http.Request) {
// Runs whatever the sweep interval says: the interval paces the automatic
// sweep, and an operator pressing the button has already decided.
started, err := library.TryStartDuplicateSweep(
context.Background(), h.pool, h.logger.With("source", "manual"), h.fingerprintSettings.Get(),
)
if err != nil {
h.logger.Error("admin: start duplicate sweep", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
if !started {
writeAdminJSONErr(w, http.StatusConflict, "sweep_in_progress")
return
}
writeJSON(w, http.StatusAccepted, map[string]bool{"started": true})
}
// handleDismissDuplicateGroup implements POST
// /api/admin/library/duplicates/{id}/dismiss: "these are not duplicates". The
// sweep keeps the dismissal and will not propose that set of tracks again. 404
// duplicate_group_not_pending when the group was already resolved or is gone.
func (h *handlers) handleDismissDuplicateGroup(w http.ResponseWriter, r *http.Request) {
id, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id")
return
}
n, err := dbq.New(h.pool).DismissDuplicateGroup(r.Context(), id)
if err != nil {
h.logger.Error("admin: dismiss duplicate group", "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
return
}
if n == 0 {
writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending")
return
}
writeJSON(w, http.StatusOK, map[string]string{"status": "dismissed"})
}
// mergeDuplicateRequest chooses the copy to keep. An empty survivor_track_id
// keeps the report's proposal.
type mergeDuplicateRequest struct {
SurvivorTrackID string `json:"survivor_track_id"`
Unmonitor bool `json:"unmonitor"`
}
// mergeDuplicateResponse reports what the merge removed. RemovedPaths are files
// deleted from disk; the operator reads them to know exactly what went.
type mergeDuplicateResponse struct {
SurvivorTrackID string `json:"survivor_track_id"`
RemovedPaths []string `json:"removed_paths"`
LidarrUnmonitorFailed *bool `json:"lidarr_unmonitor_failed,omitempty"`
}
// mergeRequestBodyLimit bounds the request body. It holds one id and a flag.
const mergeRequestBodyLimit = 1 << 16
// handleMergeDuplicateGroup implements POST /api/admin/library/duplicates/{id}/merge
// (#3911): keep one copy, move the others' likes, plays and playlist entries onto
// it, and delete the others' files and rows.
//
// Errors:
// - 409 library_not_writable / 500 file_delete_failed when a file could not be
// removed — nothing was changed (fileRemoveAPIError)
// - 404 duplicate_group_not_pending when the group was already resolved
// - 400 survivor_not_in_group, invalid_id, invalid_body
func (h *handlers) handleMergeDuplicateGroup(w http.ResponseWriter, r *http.Request) {
admin, ok := requireUser(w, r)
if !ok {
return
}
groupID, ok := parseUUID(chi.URLParam(r, "id"))
if !ok {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id")
return
}
var body mergeDuplicateRequest
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, mergeRequestBodyLimit)).Decode(&body); err != nil && !errors.Is(err, io.EOF) {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_body")
return
}
var survivorID pgtype.UUID // invalid: keep the proposal
if body.SurvivorTrackID != "" {
if survivorID, ok = parseUUID(body.SurvivorTrackID); !ok {
writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id")
return
}
}
res, unmonitorFailed, err := h.tracks.MergeDuplicates(r.Context(), groupID, survivorID, admin.ID, body.Unmonitor)
if err != nil {
if apiErr, ok := fileRemoveAPIError(err); ok {
logFileRemoveFailure(h.logger, apiErr, "group_id", uuidToString(groupID))
writeErr(w, apiErr)
return
}
switch {
case errors.Is(err, library.ErrDuplicateGroupNotPending):
writeAdminJSONErr(w, http.StatusNotFound, "duplicate_group_not_pending")
case errors.Is(err, library.ErrSurvivorNotInGroup):
writeAdminJSONErr(w, http.StatusBadRequest, "survivor_not_in_group")
default:
h.logger.Error("admin: merge duplicate group", "group_id", uuidToString(groupID), "err", err)
writeAdminJSONErr(w, http.StatusInternalServerError, "server_error")
}
return
}
resp := mergeDuplicateResponse{
SurvivorTrackID: uuidToString(res.Survivor.TrackID),
RemovedPaths: make([]string, 0, len(res.Removed)),
}
for _, c := range res.Removed {
resp.RemovedPaths = append(resp.RemovedPaths, c.FilePath)
}
if body.Unmonitor && unmonitorFailed {
failed := true
resp.LidarrUnmonitorFailed = &failed
}
writeJSON(w, http.StatusOK, resp)
}
-65
View File
@@ -1,65 +0,0 @@
package api
import (
"testing"
"time"
"github.com/jackc/pgx/v5/pgtype"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
)
func dupUUID(b byte) pgtype.UUID {
var u pgtype.UUID
u.Bytes[15] = b
u.Valid = true
return u
}
func dupTS(t time.Time) pgtype.Timestamptz { return pgtype.Timestamptz{Time: t, Valid: true} }
// Rows arrive one per member, members of a group together. The fold must keep
// groups apart, keep the query's order, and propose each group's survivor from
// its own members only.
func TestFoldDuplicateGroups(t *testing.T) {
older := time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC)
newer := older.Add(48 * time.Hour)
ber := float32(0.04)
rows := []dbq.ListPendingDuplicateGroupMembersRow{
// Group 1: identical audio, sizes tie, the older copy should be kept.
{GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(10),
Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(newer), PlayCount: 3},
{GroupID: dupUUID(1), Tier: "exact", DetectedAt: dupTS(newer), TrackID: dupUUID(11),
Title: "WWW", FileFormat: "mp3", FileSize: 6_900_000, DurationMs: 215_400, AddedAt: dupTS(older), LikeCount: 1},
// Group 2: the same recording, FLAC against MP3.
{GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(20),
Title: "Lovesick", FileFormat: "mp3", FileSize: 9_000_000, DurationMs: 198_000, AddedAt: dupTS(older)},
{GroupID: dupUUID(2), Tier: "acoustic", WorstBitErrorRate: &ber, DetectedAt: dupTS(older), TrackID: dupUUID(21),
Title: "Lovesick", FileFormat: "flac", FileSize: 30_000_000, DurationMs: 198_000, AddedAt: dupTS(newer)},
}
got := foldDuplicateGroups(rows)
if len(got) != 2 {
t.Fatalf("folded %d groups, want 2", len(got))
}
g1, g2 := got[0], got[1]
if g1.ID != uuidToString(dupUUID(1)) || len(g1.Members) != 2 || g1.WorstBitErrorRate != nil {
t.Fatalf("group 1 = %+v, want the exact pair with no score", g1)
}
if g1.SurvivorTrackID != uuidToString(dupUUID(11)) || g1.SurvivorReason != "in the library longest" {
t.Errorf("group 1 survivor = (%s, %q), want the older copy", g1.SurvivorTrackID, g1.SurvivorReason)
}
if g1.Members[0].DurationSec != 215 || g1.Members[0].PlayCount != 3 || g1.Members[1].LikeCount != 1 {
t.Errorf("group 1 members lost their facts: %+v", g1.Members)
}
if g2.Tier != "acoustic" || g2.WorstBitErrorRate == nil || *g2.WorstBitErrorRate != ber {
t.Fatalf("group 2 = %+v, want the acoustic pair with its score", g2)
}
// Chosen from group 2's own members: a survivor leaking across groups is
// exactly what a wrong fold boundary would produce.
if g2.SurvivorTrackID != uuidToString(dupUUID(21)) || g2.SurvivorReason != "lossless (flac)" {
t.Errorf("group 2 survivor = (%s, %q), want the FLAC copy", g2.SurvivorTrackID, g2.SurvivorReason)
}
}
@@ -1,67 +0,0 @@
package api
import (
"encoding/json"
"errors"
"net/http"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
// fingerprintSettingsBody is the wire shape for GET and PUT
// /api/admin/library/fingerprint-settings (M400 #3913). The threshold travels as
// the bit-error rate the matcher uses; the card presents it as a match percentage.
type fingerprintSettingsBody struct {
Enabled bool `json:"enabled"`
ChromaprintLengthSec int32 `json:"chromaprint_length_sec"`
AcousticMaxBitErrorRate float64 `json:"acoustic_max_bit_error_rate"`
BackfillConcurrency int32 `json:"backfill_concurrency"`
SweepIntervalHours int32 `json:"sweep_interval_hours"`
}
func fingerprintSettingsBodyOf(s library.FingerprintSettings) fingerprintSettingsBody {
return fingerprintSettingsBody{
Enabled: s.Enabled,
ChromaprintLengthSec: s.ChromaprintLengthSec,
AcousticMaxBitErrorRate: s.AcousticMaxBitErrorRate,
BackfillConcurrency: s.BackfillConcurrency,
SweepIntervalHours: s.SweepIntervalHours,
}
}
// handleGetFingerprintSettings implements GET /api/admin/library/fingerprint-settings.
func (h *handlers) handleGetFingerprintSettings(w http.ResponseWriter, _ *http.Request) {
writeJSON(w, http.StatusOK, fingerprintSettingsBodyOf(h.fingerprintSettings.Get()))
}
// handleUpdateFingerprintSettings implements PUT /api/admin/library/fingerprint-settings.
//
// A whole-row write. A body that leaves a field out decodes it as zero, which no
// field accepts, so a partial save is refused rather than zeroing what it omitted.
// The saved settings reach the scanner and both workers at once: they share the
// service instance.
func (h *handlers) handleUpdateFingerprintSettings(w http.ResponseWriter, r *http.Request) {
var req fingerprintSettingsBody
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON"))
return
}
saved, err := h.fingerprintSettings.Set(r.Context(), library.FingerprintSettings{
Enabled: req.Enabled,
ChromaprintLengthSec: req.ChromaprintLengthSec,
AcousticMaxBitErrorRate: req.AcousticMaxBitErrorRate,
BackfillConcurrency: req.BackfillConcurrency,
SweepIntervalHours: req.SweepIntervalHours,
})
if err != nil {
// Validation mirrors migration 0061's CHECKs and names the field.
if errors.Is(err, library.ErrFingerprintSettingOutOfRange) {
writeErr(w, apierror.BadRequest("invalid_setting", err.Error()))
return
}
writeErrWithLog(w, h.logger, "admin fingerprint settings: update failed", apierror.Internal(err))
return
}
writeJSON(w, http.StatusOK, fingerprintSettingsBodyOf(saved))
}
@@ -1,55 +0,0 @@
package api
import (
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
func TestGetFingerprintSettings_ServesDefaultsWithoutAService(t *testing.T) {
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
rec := httptest.NewRecorder()
h.handleGetFingerprintSettings(rec, httptest.NewRequest(http.MethodGet, "/api/admin/library/fingerprint-settings", nil))
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want 200", rec.Code)
}
var got fingerprintSettingsBody
if err := json.NewDecoder(rec.Body).Decode(&got); err != nil {
t.Fatalf("decode: %v", err)
}
if want := fingerprintSettingsBodyOf(library.DefaultFingerprintSettings); got != want {
t.Fatalf("body = %+v, want the defaults %+v", got, want)
}
}
func TestUpdateFingerprintSettings_Rejects(t *testing.T) {
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
for name, tc := range map[string]struct {
body string
code string
mentions string
}{
"a value out of range, naming the field": {
body: `{"enabled":true,"chromaprint_length_sec":5,"acoustic_max_bit_error_rate":0.15,"backfill_concurrency":2,"sweep_interval_hours":1}`,
code: "invalid_setting",
mentions: "chromaprint_length_sec",
},
// A partial body would otherwise zero every field it left out.
"a body missing fields": {body: `{"enabled":false}`, code: "invalid_setting"},
"malformed JSON": {body: `{"enabled":`, code: "invalid_body"},
} {
rec := httptest.NewRecorder()
h.handleUpdateFingerprintSettings(rec, httptest.NewRequest(
http.MethodPut, "/api/admin/library/fingerprint-settings", strings.NewReader(tc.body)))
body := rec.Body.String()
if rec.Code != http.StatusBadRequest || !strings.Contains(body, `"`+tc.code+`"`) || !strings.Contains(body, tc.mentions) {
t.Errorf("%s: status %d body %s; want 400 %s mentioning %q", name, rec.Code, body, tc.code, tc.mentions)
}
}
}
-7
View File
@@ -133,13 +133,6 @@ func (h *handlers) handleDeleteQuarantineFile(w http.ResponseWriter, r *http.Req
}
action, err := h.lidarrQuarantine.DeleteFile(r.Context(), id, admin.ID)
if err != nil {
// Written in the enveloped shape, not writeAdminJSONErr's bare code: the
// message is the part that tells the operator which directory and uid.
if apiErr, ok := fileRemoveAPIError(err); ok {
logFileRemoveFailure(h.logger, apiErr, "track_id", uuidToString(id))
writeErr(w, apiErr)
return
}
switch {
case errors.Is(err, lidarrquarantine.ErrTrackNotFound):
writeAdminJSONErr(w, http.StatusNotFound, "track_not_found")
+1 -1
View File
@@ -69,7 +69,7 @@ func installQuarantineClientFn(t *testing.T, h *handlers) {
}
return lidarr.NewClient(c.BaseURL, c.APIKey)
}
h.lidarrQuarantine = lidarrquarantine.NewService(h.pool, cfg, clientFn, h.dataDir)
h.lidarrQuarantine = lidarrquarantine.NewService(h.pool, cfg, clientFn)
}
// flagDirect bypasses the HTTP handler to seed a quarantine row via the
+4 -6
View File
@@ -97,16 +97,14 @@ type tuningSnapshot struct {
func (h *handlers) tuningSnapshot() tuningSnapshot {
var out tuningSnapshot
out.Profiles = map[string]weightsResp{
recsettings.ScopeRadio: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeRadio)),
recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)),
recsettings.ScopeSongsLike: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeSongsLike)),
recsettings.ScopeRadio: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeRadio)),
recsettings.ScopeDailyMix: weightsRespFrom(h.recSettings.Weights(recsettings.ScopeDailyMix)),
}
out.Taste = tasteRespFrom(h.recSettings.Taste())
out.Discover = discoverRespFrom(h.recSettings.Discover())
out.Shipped.Profiles = map[string]weightsResp{
recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()),
recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()),
recsettings.ScopeSongsLike: weightsRespFrom(recsettings.ShippedSongsLikeWeights()),
recsettings.ScopeRadio: weightsRespFrom(recsettings.ShippedRadioWeights()),
recsettings.ScopeDailyMix: weightsRespFrom(recsettings.ShippedDailyMixWeights()),
}
out.Shipped.Taste = tasteRespFrom(recsettings.ShippedTasteTuning())
out.Shipped.Discover = discoverRespFrom(recsettings.ShippedDiscoverTuning())
+4 -11
View File
@@ -23,17 +23,15 @@ type removeTrackResponse struct {
// handleRemoveTrack implements DELETE /api/admin/tracks/{id}?unmonitor=true|false.
//
// Admin-only (gated by auth.RequireAdmin on the /admin route group). Deletes the
// file, then the DB row, and runs the album/artist cascade tidy-up — and deletes
// nothing at all when the file cannot be removed (#3918). When
// Admin-only (gated by auth.RequireAdmin on the /admin route group). Always
// deletes the file + DB row and runs the album/artist cascade tidy-up. When
// unmonitor=true and the track has an mbid, also calls Lidarr.UnmonitorTrack
// — failure there is non-fatal (the destructive part already completed) and
// surfaces as `lidarr_unmonitor_failed: true` in the success envelope.
//
// Per spec §5, Lidarr-side errors during the unmonitor step do NOT map to
// wire error codes. The codes this handler emits are not_found,
// library_not_writable (409) and file_delete_failed when the file could not be
// removed, server_error, plus the auth codes the middleware emits upstream.
// wire error codes; the only error codes this handler emits are not_found,
// server_error, plus the auth codes the middleware emits upstream.
func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
idStr := chi.URLParam(r, "id")
trackID, ok := parseUUID(idStr)
@@ -68,11 +66,6 @@ func (h *handlers) handleRemoveTrack(w http.ResponseWriter, r *http.Request) {
writeErr(w, &apierror.Error{Status: http.StatusNotFound, Code: "not_found", Message: "track not found"})
return
}
if apiErr, ok := fileRemoveAPIError(err); ok {
logFileRemoveFailure(h.logger, apiErr, "track_id", idStr)
writeErr(w, apiErr)
return
}
h.logger.Error("api: remove track failed", "err", err, "track_id", idStr)
writeErr(w, apierror.InternalMsg("remove failed", err))
return
+26 -47
View File
@@ -24,7 +24,6 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/playevents"
"git.fabledsword.com/bvandeusen/minstrel/internal/playlists"
"git.fabledsword.com/bvandeusen/minstrel/internal/reacquisition"
"git.fabledsword.com/bvandeusen/minstrel/internal/recommendation"
"git.fabledsword.com/bvandeusen/minstrel/internal/recsettings"
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
"git.fabledsword.com/bvandeusen/minstrel/internal/tracks"
@@ -33,31 +32,29 @@ import (
// Mount attaches /api/* handlers to r. Public endpoints (login) are outside
// RequireUser; everything else is gated by the middleware. The events writer
// is shared with the Subsonic mount so /rest/scrobble feeds the same store.
func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService) {
func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService) {
rng := rand.New(rand.NewSource(rand.Int63()))
h := &handlers{
pool: pool, logger: logger, events: events, recCfg: recCfg,
recSettings: recSettings,
rng: rng.Float64,
lidarrCfg: lidarrCfg,
lidarrRequests: lidarrReqs,
lidarrQuarantine: lidarrQuar,
tracks: tracksSvc,
playlists: playlistsSvc,
coverart: coverEnricher,
coverSettings: coverSettings,
tagSettings: tagSettings,
scanner: scanner,
scanCfg: scanCfg,
dataDir: dataDir,
mailer: sender,
eventbus: bus,
playlistScheduler: playlistScheduler,
streamSecret: streamSecret,
netSettings: netSettings,
reacqSettings: reacqSettings,
fingerprintSettings: fpSettings,
librarySize: recommendation.NewLibrarySize(nil),
recSettings: recSettings,
rng: rng.Float64,
lidarrCfg: lidarrCfg,
lidarrRequests: lidarrReqs,
lidarrQuarantine: lidarrQuar,
tracks: tracksSvc,
playlists: playlistsSvc,
coverart: coverEnricher,
coverSettings: coverSettings,
tagSettings: tagSettings,
scanner: scanner,
scanCfg: scanCfg,
dataDir: dataDir,
mailer: sender,
eventbus: bus,
playlistScheduler: playlistScheduler,
streamSecret: streamSecret,
netSettings: netSettings,
reacqSettings: reacqSettings,
}
r.Route("/api", func(api chi.Router) {
@@ -216,16 +213,6 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
admin.Get("/library/missing", h.handleListMissingTracks)
admin.Get("/library/coverage", h.handleGetLibraryCoverage)
admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage)
admin.Get("/library/fingerprint-settings", h.handleGetFingerprintSettings)
admin.Put("/library/fingerprint-settings", h.handleUpdateFingerprintSettings)
// Duplicates report (#3912): proposals from the duplicate sweep, a
// trigger to sweep now, dismissal, and the merge (#3911), which deletes
// the removed copies' files after moving their history onto the kept one.
admin.Get("/library/duplicates", h.handleListDuplicates)
admin.Post("/library/duplicates/sweep", h.handleRunDuplicateSweep)
admin.Post("/library/duplicates/{id}/dismiss", h.handleDismissDuplicateGroup)
admin.Post("/library/duplicates/{id}/merge", h.handleMergeDuplicateGroup)
admin.Get("/invites", h.handleListInvites)
admin.Post("/invites", h.handleCreateInvite)
@@ -281,16 +268,12 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
}
type handlers struct {
pool *pgxpool.Pool
logger *slog.Logger
events *playevents.Writer
recCfg config.RecommendationConfig
recSettings *recsettings.Service
rng func() float64
// librarySize memoises the track count that sizes the candidate pool
// (#3880). Held here rather than counted per request: the count is a
// full table scan, and library size only moves when a scan runs.
librarySize *recommendation.LibrarySize
pool *pgxpool.Pool
logger *slog.Logger
events *playevents.Writer
recCfg config.RecommendationConfig
recSettings *recsettings.Service
rng func() float64
lidarrCfg *lidarrconfig.Service
lidarrRequests *lidarrrequests.Service
lidarrQuarantine *lidarrquarantine.Service
@@ -309,10 +292,6 @@ type handlers struct {
// missing files (milestone #290) — grace window, backoff, attempt caps.
// Cached in the service, so the admin card reads it without a query.
reacqSettings *reacquisition.SettingsService
// fingerprintSettings is the fingerprinting policy (M400 #3913), the same
// instance the scanner and the fingerprint workers read, so a save from the
// admin card reaches them without a restart. Nil serves the defaults.
fingerprintSettings *library.FingerprintSettingsService
// netSettings caches the trusted reverse-proxy depth read by the auth
// middleware on every request and edited from the admin network card.
netSettings *netsettings.Service
+1 -1
View File
@@ -65,7 +65,7 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) {
}
lidarrCfg := lidarrconfig.New(pool)
lidarrReqs := lidarrrequests.NewService(pool, lidarrCfg, nil, nil)
lidarrQuar := lidarrquarantine.NewService(pool, lidarrCfg, nil, "")
lidarrQuar := lidarrquarantine.NewService(pool, lidarrCfg, nil)
// tracks.Service has no Lidarr unmonitorer in tests by default; the
// admin-tracks tests below override h.tracks via installTracksLidarrStub
// when they need a stubbed Lidarr.
+7 -54
View File
@@ -6,21 +6,19 @@ package api
// /app/client/ at image build time.
//
// Both endpoints are authenticated — the bandwidth cost of the APK
// (~30-60 MB) makes anonymous access an abuse vector. The client only
// polls after login, so this gate is invisible to the actual update flow.
// (~30-60 MB) makes anonymous access an abuse vector. The Flutter
// client's polling only fires after login (banner mounts in the post-
// login shell), so this gate is invisible to the actual update flow.
//
// /api/client/apk additionally rate-limits per user to a single
// download every 60s. Real install flows fire one download per
// update; anything tighter is scripted/abusive.
//
// Returns 404 gracefully when the APK isn't present (dev environments,
// pre-CI-wiring); the client treats 404 as "no update channel available."
//
// (These paragraphs said "the Flutter client" until 2026-09-10. That client
// was deleted in v2026.08.18 — the Android app is the only one now.)
// pre-CI-wiring); the Flutter client treats 404 as "no update channel
// available."
import (
"encoding/json"
"errors"
"net/http"
"os"
@@ -86,36 +84,8 @@ func clientAPKAllowDownload(userID string, now time.Time) time.Duration {
return 0
}
// clientVersionSidecar is the JSON written beside the bundled APK by
// release.yml. It carries three values that are deliberately separate:
//
// - Name is a LABEL for people, "YYYY.MM.DD.HHMM" from the commit's
// timestamp. Two channels carrying the same code report the same name.
// - Code is the ORDERING KEY, minutes since 2020-01-01 at build time, and
// is the value Android itself installs by. It answers "may this be
// installed over that?" — the name never does.
// - Channel is a SIBLING FIELD, never a suffix inside the name.
//
// JSON rather than a positional line on purpose. The obvious growth path for
// the old one-value file was "<name> <code>", which a first-space split
// silently mangles the moment a third field appears: the code stops parsing,
// and the reader falls back to name comparison WITHOUT erroring.
type clientVersionSidecar struct {
Name string `json:"name"`
// Pointer, not int64: absent must stay distinguishable from zero. An
// artifact published before codes were recorded genuinely has no code —
// zero would claim it is infinitely old rather than unknown.
Code *int64 `json:"code"`
Channel string `json:"channel"`
}
type clientVersionResponse struct {
Version string `json:"version"`
// omitempty on both: the client must be able to tell "this server does
// not report a code" from "this build's code is 0", because those call
// for different behaviour on the other end.
Code *int64 `json:"code,omitempty"`
Channel string `json:"channel,omitempty"`
Version string `json:"version"`
APKURL string `json:"apk_url"`
SizeBytes int64 `json:"size_bytes"`
}
@@ -147,25 +117,8 @@ func (h *handlers) handleClientVersion(w http.ResponseWriter, _ *http.Request) {
return
}
var sidecar clientVersionSidecar
if err := json.Unmarshal(versionBytes, &sidecar); err != nil {
// Fail LOUDLY rather than serving a blank version. The failure mode
// this avoids is the one that never gets reported: if an unreadable
// sidecar produced an empty name, every client would compare against
// nothing, conclude it was current, and go quiet — "I cannot read
// this" and "there is nothing newer" would be the same answer.
writeErrWithLog(w, h.logger, "client_version: sidecar is not valid JSON", err)
return
}
if sidecar.Name == "" {
http.Error(w, `{"error":{"code":"bad_client_version","message":"version sidecar has no name"}}`, http.StatusInternalServerError)
return
}
writeJSON(w, http.StatusOK, clientVersionResponse{
Version: strings.TrimSpace(sidecar.Name),
Code: sidecar.Code,
Channel: strings.TrimSpace(sidecar.Channel),
Version: strings.TrimSpace(string(versionBytes)),
APKURL: "/api/client/apk",
SizeBytes: stat.Size(),
})
+6 -75
View File
@@ -77,32 +77,18 @@ func TestClientVersion_404WhenAPKButNoVersion(t *testing.T) {
}
}
// writeClientAssets stages an APK plus a raw sidecar body, and returns the
// APK's size so callers can assert size_bytes without recomputing it.
func writeClientAssets(t *testing.T, sidecar string) int64 {
t.Helper()
func TestClientVersion_200WithBothFiles(t *testing.T) {
dir := withClientAPKDir(t)
body := []byte("fake apk content")
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte(sidecar), 0o644); err != nil {
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte("v2026.05.10\n"), 0o644); err != nil {
t.Fatal(err)
}
return int64(len(body))
}
func getClientVersion(t *testing.T) *httptest.ResponseRecorder {
t.Helper()
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
rr := httptest.NewRecorder()
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
return rr
}
func TestClientVersion_200WithBothFiles(t *testing.T) {
size := writeClientAssets(t, `{"name":"2026.09.10.1432","code":3523847,"channel":"stable"}`+"\n")
rr := getClientVersion(t)
if rr.Code != http.StatusOK {
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
}
@@ -110,69 +96,14 @@ func TestClientVersion_200WithBothFiles(t *testing.T) {
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatal(err)
}
if resp.Version != "2026.09.10.1432" {
t.Errorf("version: want 2026.09.10.1432, got %q", resp.Version)
}
if resp.Code == nil {
t.Fatal("code: want 3523847, got absent — the client decides on this, so absent means it silently falls back to name comparison")
}
if *resp.Code != 3523847 {
t.Errorf("code: want 3523847, got %d", *resp.Code)
}
if resp.Channel != "stable" {
t.Errorf("channel: want stable, got %q", resp.Channel)
if resp.Version != "v2026.05.10" {
t.Errorf("version: want trimmed v2026.05.10, got %q", resp.Version)
}
if resp.APKURL != "/api/client/apk" {
t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL)
}
if resp.SizeBytes != size {
t.Errorf("size_bytes: want %d, got %d", size, resp.SizeBytes)
}
}
// A release published before ordering keys were recorded has a name and
// genuinely no code. That must arrive as ABSENT, not as 0 — zero would claim
// the build is infinitely old and offer an update to everyone forever.
func TestClientVersion_CodeAbsentIsOmittedNotZero(t *testing.T) {
writeClientAssets(t, `{"name":"2026.09.09","code":null,"channel":"stable"}`)
rr := getClientVersion(t)
if rr.Code != http.StatusOK {
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
}
var resp clientVersionResponse
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatal(err)
}
if resp.Code != nil {
t.Errorf("code: want absent, got %d", *resp.Code)
}
// The wire must omit the key entirely, so a client can distinguish
// "this server reports no code" from "this build's code is 0".
var raw map[string]any
if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil {
t.Fatal(err)
}
if _, present := raw["code"]; present {
t.Errorf("code key should be omitted entirely, body was %s", rr.Body.String())
}
}
// The failure this guards is the one nobody reports: if an unreadable sidecar
// produced an empty version, every client would compare against nothing,
// decide it was current, and go quiet. "I cannot read this" and "there is
// nothing newer" must not be the same answer.
func TestClientVersion_MalformedSidecarErrorsRatherThanReportingNothing(t *testing.T) {
for _, sidecar := range []string{
"2026.09.10.1432", // the OLD plain-text format
`{"name":"x",`, // truncated JSON
`{"code":123,"channel":"dev"}`, // valid JSON, no name
"",
} {
writeClientAssets(t, sidecar)
rr := getClientVersion(t)
if rr.Code == http.StatusOK {
t.Errorf("sidecar %q: want an error status, got 200 with body %s", sidecar, rr.Body.String())
}
if resp.SizeBytes != int64(len(body)) {
t.Errorf("size_bytes: want %d, got %d", len(body), resp.SizeBytes)
}
}
-58
View File
@@ -1,58 +0,0 @@
package api
import (
"errors"
"fmt"
"log/slog"
"net/http"
"git.fabledsword.com/bvandeusen/minstrel/internal/apierror"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
// fileRemoveAPIError answers a delete that could not reach the track's file
// (#3918). Both delete endpoints use it, so the operator gets the same
// explanation from the admin remove-track action and from quarantine's Delete
// file.
//
// The unwritable case is a 409 rather than a 500 because nothing is broken: the
// request conflicts with how the library is mounted, and the fix is the
// operator's. The message names the directory — removal writes to the parent,
// not the file — and the uid/gid the process runs as, which is the half of a
// permission problem invisible from the host. Every case says nothing was
// deleted, because that is exactly what the operator will be worried about.
func fileRemoveAPIError(err error) (*apierror.Error, bool) {
var fre *library.FileRemoveError
if !errors.As(err, &fre) {
return nil, false
}
if fre.NotWritable() {
return &apierror.Error{
Status: http.StatusConflict,
Code: "library_not_writable",
Message: fmt.Sprintf(
"Minstrel runs as uid %d, gid %d and cannot delete from %s (%s). "+
"The library mount must be writable by that user. Nothing was deleted.",
fre.UID, fre.GID, fre.Dir(), fre.Reason()),
Cause: err,
}, true
}
return &apierror.Error{
Status: http.StatusInternalServerError,
Code: "file_delete_failed",
Message: fmt.Sprintf("Could not delete %s (%s). Nothing was deleted.", fre.Path, fre.Reason()),
Cause: err,
}, true
}
// logFileRemoveFailure records a delete that could not reach its file. An
// unwritable library is an environment fact the operator can fix, so it is a
// Warn; anything else is a real fault.
func logFileRemoveFailure(logger *slog.Logger, apiErr *apierror.Error, attrs ...any) {
attrs = append(attrs, "code", apiErr.Code, "err", apiErr.Cause)
if apiErr.Status == http.StatusConflict {
logger.Warn("api: track file could not be deleted", attrs...)
return
}
logger.Error("api: track file could not be deleted", attrs...)
}
-96
View File
@@ -1,96 +0,0 @@
package api
import (
"errors"
"fmt"
"io/fs"
"net/http"
"strings"
"syscall"
"testing"
"git.fabledsword.com/bvandeusen/minstrel/internal/library"
)
const removeTestPath = "/music/Moe Shop/WWW (2020)/01 - WWW.mp3"
// removeFailure builds the error a delete service returns when the file would
// not go, wrapped the way lidarrquarantine.DeleteFile and tracks.RemoveTrack
// wrap it — the mapping has to see through that.
func removeFailure(errno syscall.Errno) error {
return fmt.Errorf("delete file: %w", &library.FileRemoveError{
Path: removeTestPath, UID: 1000, GID: 1000,
Err: &fs.PathError{Op: "remove", Path: removeTestPath, Err: errno},
})
}
func TestFileRemoveAPIError(t *testing.T) {
cases := []struct {
name string
errno syscall.Errno
wantStatus int
wantCode string
wantIn []string
}{
{
name: "read-only mount", errno: syscall.EROFS,
wantStatus: http.StatusConflict, wantCode: "library_not_writable",
wantIn: []string{"uid 1000, gid 1000", "/music/Moe Shop/WWW (2020)", "read-only file system", "Nothing was deleted"},
},
{
name: "permission denied", errno: syscall.EACCES,
wantStatus: http.StatusConflict, wantCode: "library_not_writable",
wantIn: []string{"permission denied", "Nothing was deleted"},
},
{
name: "operation not permitted", errno: syscall.EPERM,
wantStatus: http.StatusConflict, wantCode: "library_not_writable",
wantIn: []string{"operation not permitted"},
},
{
name: "i/o error", errno: syscall.EIO,
wantStatus: http.StatusInternalServerError, wantCode: "file_delete_failed",
wantIn: []string{removeTestPath, "input/output error", "Nothing was deleted"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
apiErr, ok := fileRemoveAPIError(removeFailure(tc.errno))
if !ok {
t.Fatal("a wrapped *library.FileRemoveError was not recognised")
}
if apiErr.Status != tc.wantStatus || apiErr.Code != tc.wantCode {
t.Fatalf("got %d %s, want %d %s", apiErr.Status, apiErr.Code, tc.wantStatus, tc.wantCode)
}
for _, want := range tc.wantIn {
if !strings.Contains(apiErr.Message, want) {
t.Errorf("message %q lacks %q", apiErr.Message, want)
}
}
})
}
}
// The unwritable answer must name the DIRECTORY. Removal needs write access to
// the parent, so a message naming the file would send the operator to fix the
// wrong permissions. The directory is a prefix of the file path, which is why a
// plain "contains the directory" check could never catch that regression.
func TestFileRemoveAPIError_NotWritableNamesTheDirectoryNotTheFile(t *testing.T) {
apiErr, _ := fileRemoveAPIError(removeFailure(syscall.EROFS))
if strings.Contains(apiErr.Message, "01 - WWW.mp3") {
t.Fatalf("message names the file rather than its directory: %q", apiErr.Message)
}
}
func TestFileRemoveAPIError_IgnoresOtherErrors(t *testing.T) {
for name, err := range map[string]error{
"nil": nil,
"plain error": errors.New("delete track: connection reset"),
"path error": &fs.PathError{Op: "remove", Path: removeTestPath, Err: syscall.EROFS},
"not found": library.ErrTrackNotFound,
} {
if _, ok := fileRemoveAPIError(err); ok {
t.Errorf("%s: mapped as a file-remove failure", name)
}
}
}
+1 -4
View File
@@ -465,7 +465,7 @@ func TestRoutesRegisteredInMount(t *testing.T) {
r := chi.NewRouter()
w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)),
30*time.Minute, 0.5, 30000)
Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil, nil)
Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil)
paths := []string{
"/api/artists",
@@ -484,9 +484,6 @@ func TestRoutesRegisteredInMount(t *testing.T) {
// wired.
"/api/admin/library/missing",
"/api/admin/library/reacquisition",
"/api/admin/library/fingerprints",
"/api/admin/library/fingerprint-settings",
"/api/admin/library/duplicates",
}
for _, p := range paths {
req := httptest.NewRequest(http.MethodGet, p, nil)
+2 -24
View File
@@ -87,24 +87,10 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
currentVec.DeviceClass = latestDeviceClass(r.Context(), q, user.ID, h.logger)
exclude := parseExcludeParam(r.URL.Query().Get("exclude"))
// Size the pool to the library (#3880). A fixed ~170 candidates samples a
// shrinking fraction of a growing collection, which is what made the
// recommendations feel less relevant as the library grew. Degrades to the
// base limits if the count is unavailable — never fails the request over a
// sizing hint.
librarySize := h.librarySize.Get(r.Context(), func(ctx context.Context) (int64, error) {
return recommendation.CountLibraryTracks(ctx, q)
})
limits := recommendation.ScaleForLibrary(
recommendation.DefaultCandidateSourceLimits(), librarySize,
)
limits := recommendation.DefaultCandidateSourceLimits()
candidates, err := recommendation.LoadCandidatesFromSimilarity(
r.Context(), q, user.ID, seedID,
h.recCfg.RecentlyPlayedHours, currentVec, exclude, limits,
// A fresh seed per request (#3889): radio is a new session each time
// and SHOULD draw differently. The system mixes are the surfaces that
// promise repeatability; this is not one of them.
strconv.FormatInt(time.Now().UnixNano(), 36),
)
if err != nil {
h.logger.Warn("api: radio: similarity-pool failed; falling back to whole-library", "err", err)
@@ -122,15 +108,7 @@ func (h *handlers) handleRadio(w http.ResponseWriter, r *http.Request) {
// Scoring weights come from the DB-backed tuning lab (#1250) —
// read per request so an admin change takes effect live.
weights := h.recSettings.Weights(recsettings.ScopeRadio)
// Diversity caps (#3882). Radio had none while every sibling surface did,
// which is how a whole session could come back from one artist. Scaled to
// the requested length so a 20-track radio and a 200-track one are capped
// alike; Shuffle relaxes them rather than returning a short radio.
//
// limit-1 because the seed track occupies the first slot and is prepended
// below — the caps govern the tracks that FOLLOW it.
caps := recommendation.RadioDiversityCaps(limit - 1)
picks := recommendation.Shuffle(candidates, weights, time.Now().UTC(), h.rng, limit-1, caps)
picks := recommendation.Shuffle(candidates, weights, time.Now().UTC(), h.rng, limit-1)
out := make([]TrackRef, 0, len(picks)+1)
out = append(out, trackRefFrom(track, album.Title, artist.Name))
-5
View File
@@ -55,11 +55,6 @@ const (
// exercised.
ActionSessionRevoke Action = "session_revoke"
ActionSessionRevokeOthers Action = "session_revoke_others"
// Duplicate merge (#3911). Irreversible: a copy's file and row are removed
// and its history moved onto the copy kept. The metadata names both, so the
// log can answer "where did that file go" long after the report is gone.
ActionDuplicateMerge Action = "duplicate_merge"
)
// Write inserts one audit_log row. metadata is marshaled as JSON;
-1
View File
@@ -168,7 +168,6 @@ func TestWrite_AllActionConstantsArePersisted(t *testing.T) {
audit.ActionTokenRegenerate,
audit.ActionForgotPasswordInit,
audit.ActionPasswordResetByEmail,
audit.ActionDuplicateMerge,
}
for _, a := range actions {
if err := audit.Write(context.Background(), pool, nilUUID, nilUUID, a, nil); err != nil {
-457
View File
@@ -1,457 +0,0 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.31.1
// source: duplicates.sql
package dbq
import (
"context"
"github.com/jackc/pgx/v5/pgtype"
)
const addDuplicateGroupMember = `-- name: AddDuplicateGroupMember :exec
INSERT INTO duplicate_group_members (group_id, track_id)
VALUES ($1, $2)
ON CONFLICT DO NOTHING
`
type AddDuplicateGroupMemberParams struct {
GroupID pgtype.UUID
TrackID pgtype.UUID
}
func (q *Queries) AddDuplicateGroupMember(ctx context.Context, arg AddDuplicateGroupMemberParams) error {
_, err := q.db.Exec(ctx, addDuplicateGroupMember, arg.GroupID, arg.TrackID)
return err
}
const countPendingDuplicateGroups = `-- name: CountPendingDuplicateGroups :one
SELECT count(*)::bigint
FROM duplicate_groups g
WHERE g.status = 'pending'
AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2
`
// Proposals awaiting review. A group left with one member — its other tracks
// deleted since the sweep — is no proposal at all and is not counted; the next
// sweep retires it.
func (q *Queries) CountPendingDuplicateGroups(ctx context.Context) (int64, error) {
row := q.db.QueryRow(ctx, countPendingDuplicateGroups)
var column_1 int64
err := row.Scan(&column_1)
return column_1, err
}
const deleteStalePendingDuplicateGroups = `-- name: DeleteStalePendingDuplicateGroups :execrows
DELETE FROM duplicate_groups g
WHERE g.status = 'pending'
AND g.last_seen_sweep_id IS DISTINCT FROM $1
AND (g.last_seen_sweep_id IS NULL
OR (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = g.last_seen_sweep_id)
< (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = $1))
`
// A pending proposal this sweep did not find again no longer describes the
// library: a member was re-fingerprinted, merged away or went missing. Dismissed
// groups are kept regardless — they are the memory of a decision.
//
// Only proposals last confirmed by an EARLIER sweep go. Should two sweeps ever
// overlap (a manual trigger racing the worker), neither may delete what the other
// has just found.
func (q *Queries) DeleteStalePendingDuplicateGroups(ctx context.Context, sweepID pgtype.UUID) (int64, error) {
result, err := q.db.Exec(ctx, deleteStalePendingDuplicateGroups, sweepID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const dismissDuplicateGroup = `-- name: DismissDuplicateGroup :execrows
UPDATE duplicate_groups
SET status = 'dismissed', resolved_at = now()
WHERE id = $1 AND status = 'pending'
`
// "These are not duplicates." Only a pending group can be dismissed; zero rows
// means it was already resolved or no longer exists.
func (q *Queries) DismissDuplicateGroup(ctx context.Context, id pgtype.UUID) (int64, error) {
result, err := q.db.Exec(ctx, dismissDuplicateGroup, id)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const finishDuplicateSweep = `-- name: FinishDuplicateSweep :exec
UPDATE duplicate_sweeps
SET finished_at = now(),
candidates = $1,
groups_found = $2,
oversize_clusters = $3,
error_message = NULLIF($4::text, '')
WHERE id = $5
`
type FinishDuplicateSweepParams struct {
Candidates *int32
GroupsFound *int32
OversizeClusters *int32
ErrorMessage string
ID pgtype.UUID
}
func (q *Queries) FinishDuplicateSweep(ctx context.Context, arg FinishDuplicateSweepParams) error {
_, err := q.db.Exec(ctx, finishDuplicateSweep,
arg.Candidates,
arg.GroupsFound,
arg.OversizeClusters,
arg.ErrorMessage,
arg.ID,
)
return err
}
const getInFlightDuplicateSweep = `-- name: GetInFlightDuplicateSweep :one
SELECT id, started_at
FROM duplicate_sweeps
WHERE finished_at IS NULL
ORDER BY started_at DESC
LIMIT 1
`
type GetInFlightDuplicateSweepRow struct {
ID pgtype.UUID
StartedAt pgtype.Timestamptz
}
// The guard against two sweeps at once: "in flight" is finished_at IS NULL.
func (q *Queries) GetInFlightDuplicateSweep(ctx context.Context) (GetInFlightDuplicateSweepRow, error) {
row := q.db.QueryRow(ctx, getInFlightDuplicateSweep)
var i GetInFlightDuplicateSweepRow
err := row.Scan(&i.ID, &i.StartedAt)
return i, err
}
const getLatestDuplicateSweep = `-- name: GetLatestDuplicateSweep :one
SELECT id, started_at, finished_at, candidates, groups_found, oversize_clusters, error_message
FROM duplicate_sweeps
ORDER BY started_at DESC
LIMIT 1
`
func (q *Queries) GetLatestDuplicateSweep(ctx context.Context) (DuplicateSweep, error) {
row := q.db.QueryRow(ctx, getLatestDuplicateSweep)
var i DuplicateSweep
err := row.Scan(
&i.ID,
&i.StartedAt,
&i.FinishedAt,
&i.Candidates,
&i.GroupsFound,
&i.OversizeClusters,
&i.ErrorMessage,
)
return i, err
}
const getLatestFingerprintComputedAt = `-- name: GetLatestFingerprintComputedAt :one
SELECT max(computed_at)::timestamptz AS latest FROM track_fingerprints
`
// Whether a sweep has anything new to look at: fingerprints written since the
// last sweep started.
func (q *Queries) GetLatestFingerprintComputedAt(ctx context.Context) (pgtype.Timestamptz, error) {
row := q.db.QueryRow(ctx, getLatestFingerprintComputedAt)
var latest pgtype.Timestamptz
err := row.Scan(&latest)
return latest, err
}
const listDismissedDuplicateMemberSets = `-- name: ListDismissedDuplicateMemberSets :many
SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids
FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id
WHERE g.status = 'dismissed'
GROUP BY g.id
`
type ListDismissedDuplicateMemberSetsRow struct {
ID pgtype.UUID
TrackIds []pgtype.UUID
}
// What the operator has already said are not duplicates. A new proposal whose
// every member sat together in one of these is not proposed again.
func (q *Queries) ListDismissedDuplicateMemberSets(ctx context.Context) ([]ListDismissedDuplicateMemberSetsRow, error) {
rows, err := q.db.Query(ctx, listDismissedDuplicateMemberSets)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListDismissedDuplicateMemberSetsRow
for rows.Next() {
var i ListDismissedDuplicateMemberSetsRow
if err := rows.Scan(&i.ID, &i.TrackIds); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listDuplicateCandidates = `-- name: ListDuplicateCandidates :many
SELECT t.id, t.duration_ms, f.audio_stream_sha256, f.chromaprint
FROM tracks t
JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL
AND f.fingerprint_version >= $1
AND f.chromaprint IS NOT NULL
-- Only chromaprints taken at the current length: prints at two lengths are not
-- comparable, and after a length change the backfill is still re-deriving the
-- rest (#3913).
AND f.chromaprint_length_sec = $2
AND (t.duration_ms, t.id) > ($3::integer, $4::uuid)
ORDER BY t.duration_ms, t.id
LIMIT $5
`
type ListDuplicateCandidatesParams struct {
CurrentVersion int16
ChromaprintLengthSec int32
AfterDurationMs int32
AfterID pgtype.UUID
PageLimit int32
}
type ListDuplicateCandidatesRow struct {
ID pgtype.UUID
DurationMs int32
AudioStreamSha256 []byte
Chromaprint []int32
}
// The acoustic tier's input, one page at a time in (duration_ms, id) order so the
// sweep holds only a sliding window of durations. Tracks without a chromaprint
// cannot be compared acoustically and are left out; any exact duplicates among
// them come from ListExactDuplicateHashes.
func (q *Queries) ListDuplicateCandidates(ctx context.Context, arg ListDuplicateCandidatesParams) ([]ListDuplicateCandidatesRow, error) {
rows, err := q.db.Query(ctx, listDuplicateCandidates,
arg.CurrentVersion,
arg.ChromaprintLengthSec,
arg.AfterDurationMs,
arg.AfterID,
arg.PageLimit,
)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListDuplicateCandidatesRow
for rows.Next() {
var i ListDuplicateCandidatesRow
if err := rows.Scan(
&i.ID,
&i.DurationMs,
&i.AudioStreamSha256,
&i.Chromaprint,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listExactDuplicateHashes = `-- name: ListExactDuplicateHashes :many
SELECT f.audio_stream_sha256,
array_agg(t.id ORDER BY t.id)::uuid[] AS track_ids
FROM track_fingerprints f
JOIN tracks t ON t.id = f.track_id
WHERE t.missing_since IS NULL
AND f.fingerprint_version >= $1
AND f.audio_stream_sha256 IS NOT NULL
GROUP BY f.audio_stream_sha256
HAVING count(*) > 1
`
type ListExactDuplicateHashesRow struct {
AudioStreamSha256 []byte
TrackIds []pgtype.UUID
}
// The exact tier, library-wide in one pass: identical encoded audio shared by
// more than one present track.
func (q *Queries) ListExactDuplicateHashes(ctx context.Context, currentVersion int16) ([]ListExactDuplicateHashesRow, error) {
rows, err := q.db.Query(ctx, listExactDuplicateHashes, currentVersion)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListExactDuplicateHashesRow
for rows.Next() {
var i ListExactDuplicateHashesRow
if err := rows.Scan(&i.AudioStreamSha256, &i.TrackIds); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listPendingDuplicateGroupMembers = `-- name: ListPendingDuplicateGroupMembers :many
WITH page AS (
SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at
FROM duplicate_groups g
WHERE g.status = 'pending'
AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2
ORDER BY g.detected_at DESC, g.id
LIMIT $2 OFFSET $1
)
SELECT p.id AS group_id,
p.tier,
p.worst_bit_error_rate,
p.detected_at,
t.id AS track_id,
t.title,
artists.name AS artist_name,
albums.id AS album_id,
albums.title AS album_title,
t.file_path,
t.file_format,
t.file_size,
t.duration_ms,
t.added_at,
(SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count,
(SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count
FROM page p
JOIN duplicate_group_members m ON m.group_id = p.id
JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id
ORDER BY p.detected_at DESC, p.id, t.id
`
type ListPendingDuplicateGroupMembersParams struct {
PageOffset int32
PageLimit int32
}
type ListPendingDuplicateGroupMembersRow struct {
GroupID pgtype.UUID
Tier string
WorstBitErrorRate *float32
DetectedAt pgtype.Timestamptz
TrackID pgtype.UUID
Title string
ArtistName string
AlbumID pgtype.UUID
AlbumTitle string
FilePath string
FileFormat string
FileSize int64
DurationMs int32
AddedAt pgtype.Timestamptz
LikeCount int64
PlayCount int64
}
// One page of proposals, newest first, flattened to one row per member so the
// handler folds them without a query per group. What each copy carries — likes
// and plays from every user — is here because it is what the operator weighs
// when deciding which copy to keep.
func (q *Queries) ListPendingDuplicateGroupMembers(ctx context.Context, arg ListPendingDuplicateGroupMembersParams) ([]ListPendingDuplicateGroupMembersRow, error) {
rows, err := q.db.Query(ctx, listPendingDuplicateGroupMembers, arg.PageOffset, arg.PageLimit)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListPendingDuplicateGroupMembersRow
for rows.Next() {
var i ListPendingDuplicateGroupMembersRow
if err := rows.Scan(
&i.GroupID,
&i.Tier,
&i.WorstBitErrorRate,
&i.DetectedAt,
&i.TrackID,
&i.Title,
&i.ArtistName,
&i.AlbumID,
&i.AlbumTitle,
&i.FilePath,
&i.FileFormat,
&i.FileSize,
&i.DurationMs,
&i.AddedAt,
&i.LikeCount,
&i.PlayCount,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const startDuplicateSweep = `-- name: StartDuplicateSweep :one
INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at
`
type StartDuplicateSweepRow struct {
ID pgtype.UUID
StartedAt pgtype.Timestamptz
}
func (q *Queries) StartDuplicateSweep(ctx context.Context) (StartDuplicateSweepRow, error) {
row := q.db.QueryRow(ctx, startDuplicateSweep)
var i StartDuplicateSweepRow
err := row.Scan(&i.ID, &i.StartedAt)
return i, err
}
const upsertDuplicateGroup = `-- name: UpsertDuplicateGroup :one
INSERT INTO duplicate_groups (member_key, tier, worst_bit_error_rate, last_seen_sweep_id)
VALUES ($1, $2, $3, $4)
ON CONFLICT (member_key) DO UPDATE
SET tier = EXCLUDED.tier,
worst_bit_error_rate = EXCLUDED.worst_bit_error_rate,
last_seen_sweep_id = EXCLUDED.last_seen_sweep_id
WHERE duplicate_groups.status = 'pending'
RETURNING id
`
type UpsertDuplicateGroupParams struct {
MemberKey string
Tier string
WorstBitErrorRate *float32
SweepID pgtype.UUID
}
// Proposes a group, or refreshes one already pending. A group already dismissed
// or merged is left exactly as it is: the WHERE on the update makes the conflict
// a no-op, and the caller sees no row.
func (q *Queries) UpsertDuplicateGroup(ctx context.Context, arg UpsertDuplicateGroupParams) (pgtype.UUID, error) {
row := q.db.QueryRow(ctx, upsertDuplicateGroup,
arg.MemberKey,
arg.Tier,
arg.WorstBitErrorRate,
arg.SweepID,
)
var id pgtype.UUID
err := row.Scan(&id)
return id, err
}
@@ -1,72 +0,0 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.31.1
// source: fingerprint_settings.sql
package dbq
import (
"context"
)
const getFingerprintSettings = `-- name: GetFingerprintSettings :one
SELECT id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at FROM fingerprint_settings WHERE id = true
`
func (q *Queries) GetFingerprintSettings(ctx context.Context) (FingerprintSetting, error) {
row := q.db.QueryRow(ctx, getFingerprintSettings)
var i FingerprintSetting
err := row.Scan(
&i.ID,
&i.Enabled,
&i.ChromaprintLengthSec,
&i.AcousticMaxBitErrorRate,
&i.BackfillConcurrency,
&i.SweepIntervalHours,
&i.UpdatedAt,
)
return i, err
}
const updateFingerprintSettings = `-- name: UpdateFingerprintSettings :one
UPDATE fingerprint_settings
SET enabled = $1,
chromaprint_length_sec = $2,
acoustic_max_bit_error_rate = $3,
backfill_concurrency = $4,
sweep_interval_hours = $5,
updated_at = now()
WHERE id = true
RETURNING id, enabled, chromaprint_length_sec, acoustic_max_bit_error_rate, backfill_concurrency, sweep_interval_hours, updated_at
`
type UpdateFingerprintSettingsParams struct {
Enabled bool
ChromaprintLengthSec int32
AcousticMaxBitErrorRate float64
BackfillConcurrency int32
SweepIntervalHours int32
}
// Whole-row write from the admin card; migration 0061's CHECKs are the backstop
// behind the service's own validation.
func (q *Queries) UpdateFingerprintSettings(ctx context.Context, arg UpdateFingerprintSettingsParams) (FingerprintSetting, error) {
row := q.db.QueryRow(ctx, updateFingerprintSettings,
arg.Enabled,
arg.ChromaprintLengthSec,
arg.AcousticMaxBitErrorRate,
arg.BackfillConcurrency,
arg.SweepIntervalHours,
)
var i FingerprintSetting
err := row.Scan(
&i.ID,
&i.Enabled,
&i.ChromaprintLengthSec,
&i.AcousticMaxBitErrorRate,
&i.BackfillConcurrency,
&i.SweepIntervalHours,
&i.UpdatedAt,
)
return i, err
}
-172
View File
@@ -1,172 +0,0 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.31.1
// source: fingerprints.sql
package dbq
import (
"context"
"github.com/jackc/pgx/v5/pgtype"
)
const deleteTrackFingerprint = `-- name: DeleteTrackFingerprint :exec
DELETE FROM track_fingerprints WHERE track_id = $1
`
// A file changed but could not be fingerprinted, for a reason unrelated to the
// file. The stored row describes the OLD bytes, so it goes and the backfill
// re-derives it — nothing may keep trusting a stale identity.
func (q *Queries) DeleteTrackFingerprint(ctx context.Context, trackID pgtype.UUID) error {
_, err := q.db.Exec(ctx, deleteTrackFingerprint, trackID)
return err
}
const getFingerprintCoverage = `-- name: GetFingerprintCoverage :one
SELECT count(*)::bigint AS total,
count(*) FILTER (
WHERE f.fingerprint_version >= $1
AND f.chromaprint_length_sec = $2
AND f.audio_stream_sha256 IS NOT NULL AND f.chromaprint IS NOT NULL
)::bigint AS fingerprinted,
count(*) FILTER (
WHERE f.fingerprint_version >= $1
AND f.chromaprint_length_sec = $2
AND (f.audio_stream_sha256 IS NULL OR f.chromaprint IS NULL)
)::bigint AS rejected,
count(*) FILTER (
WHERE f.track_id IS NULL
OR f.fingerprint_version < $1
OR f.chromaprint_length_sec <> $2
)::bigint AS pending
FROM tracks t
LEFT JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL
`
type GetFingerprintCoverageParams struct {
CurrentVersion int16
ChromaprintLengthSec int32
}
type GetFingerprintCoverageRow struct {
Total int64
Fingerprinted int64
Rejected int64
Pending int64
}
// The admin gauge for the backfill. fingerprinted + rejected + pending = total.
// "Current" means derived by the current method AT the current length: a row at
// another length is pending, because the backfill will re-derive it. rejected is
// a current row with a NULL half: a tool ran and refused the file, which is
// settled rather than waiting. Missing tracks are excluded, or the gauge could
// never reach the end.
func (q *Queries) GetFingerprintCoverage(ctx context.Context, arg GetFingerprintCoverageParams) (GetFingerprintCoverageRow, error) {
row := q.db.QueryRow(ctx, getFingerprintCoverage, arg.CurrentVersion, arg.ChromaprintLengthSec)
var i GetFingerprintCoverageRow
err := row.Scan(
&i.Total,
&i.Fingerprinted,
&i.Rejected,
&i.Pending,
)
return i, err
}
const listTracksNeedingFingerprint = `-- name: ListTracksNeedingFingerprint :many
SELECT t.id, t.file_path
FROM tracks t
LEFT JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL
-- A row taken at another length is as stale as one from an older method:
-- chromaprints at two lengths cannot be compared (#3913).
AND (f.track_id IS NULL
OR f.fingerprint_version < $1
OR f.chromaprint_length_sec <> $2)
AND t.id > $3
ORDER BY t.id
LIMIT $4
`
type ListTracksNeedingFingerprintParams struct {
CurrentVersion int16
ChromaprintLengthSec int32
AfterID pgtype.UUID
BatchLimit int32
}
type ListTracksNeedingFingerprintRow struct {
ID pgtype.UUID
FilePath string
}
// The backfill's work queue (#3908): tracks with no fingerprint, or one derived
// by an older method. Keyset-paged on id so a pass visits each track at most
// once. That cursor is load-bearing: an inconclusive attempt writes no row, so
// without it a file that keeps timing out would be listed again straight away
// and retried in a tight loop. Missing tracks are skipped — there is no file to
// read.
func (q *Queries) ListTracksNeedingFingerprint(ctx context.Context, arg ListTracksNeedingFingerprintParams) ([]ListTracksNeedingFingerprintRow, error) {
rows, err := q.db.Query(ctx, listTracksNeedingFingerprint,
arg.CurrentVersion,
arg.ChromaprintLengthSec,
arg.AfterID,
arg.BatchLimit,
)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListTracksNeedingFingerprintRow
for rows.Next() {
var i ListTracksNeedingFingerprintRow
if err := rows.Scan(&i.ID, &i.FilePath); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const upsertTrackFingerprint = `-- name: UpsertTrackFingerprint :exec
INSERT INTO track_fingerprints (
track_id, audio_stream_sha256, chromaprint, fingerprint_version, chromaprint_length_sec
) VALUES (
$1, $2, $3,
$4, $5
)
ON CONFLICT (track_id) DO UPDATE SET
audio_stream_sha256 = EXCLUDED.audio_stream_sha256,
chromaprint = EXCLUDED.chromaprint,
fingerprint_version = EXCLUDED.fingerprint_version,
chromaprint_length_sec = EXCLUDED.chromaprint_length_sec,
computed_at = now()
`
type UpsertTrackFingerprintParams struct {
TrackID pgtype.UUID
AudioStreamSha256 []byte
Chromaprint []int32
FingerprintVersion int16
ChromaprintLengthSec int32
}
// Written whenever a track's fingerprint is derived: by the scan when a file is
// new or its bytes changed, and by the backfill (#3908) for rows derived by an
// older method. Replaces the row wholesale — a fingerprint of the old bytes has
// no standing once the file has changed.
func (q *Queries) UpsertTrackFingerprint(ctx context.Context, arg UpsertTrackFingerprintParams) error {
_, err := q.db.Exec(ctx, upsertTrackFingerprint,
arg.TrackID,
arg.AudioStreamSha256,
arg.Chromaprint,
arg.FingerprintVersion,
arg.ChromaprintLengthSec,
)
return err
}
-339
View File
@@ -1,339 +0,0 @@
// Code generated by sqlc. DO NOT EDIT.
// versions:
// sqlc v1.31.1
// source: merge.sql
package dbq
import (
"context"
"github.com/jackc/pgx/v5/pgtype"
)
const listDuplicateGroupMergeMembers = `-- name: ListDuplicateGroupMergeMembers :many
SELECT t.id, t.file_path, t.file_format, t.file_size, t.added_at, t.album_id,
t.mbid, albums.mbid AS album_mbid
FROM duplicate_group_members m
JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
WHERE m.group_id = $1
ORDER BY t.id
`
type ListDuplicateGroupMergeMembersRow struct {
ID pgtype.UUID
FilePath string
FileFormat string
FileSize int64
AddedAt pgtype.Timestamptz
AlbumID pgtype.UUID
Mbid *string
AlbumMbid *string
}
func (q *Queries) ListDuplicateGroupMergeMembers(ctx context.Context, groupID pgtype.UUID) ([]ListDuplicateGroupMergeMembersRow, error) {
rows, err := q.db.Query(ctx, listDuplicateGroupMergeMembers, groupID)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListDuplicateGroupMergeMembersRow
for rows.Next() {
var i ListDuplicateGroupMergeMembersRow
if err := rows.Scan(
&i.ID,
&i.FilePath,
&i.FileFormat,
&i.FileSize,
&i.AddedAt,
&i.AlbumID,
&i.Mbid,
&i.AlbumMbid,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const lockDuplicateGroupForMerge = `-- name: LockDuplicateGroupForMerge :one
SELECT id, tier, status
FROM duplicate_groups
WHERE id = $1
FOR UPDATE
`
type LockDuplicateGroupForMergeRow struct {
ID pgtype.UUID
Tier string
Status string
}
// Duplicate merge (Scribe #3911). Every statement here runs inside the one
// transaction library.MergeDuplicateGroup opens, after the removed copy's file
// is already gone. The loser's own track row is deleted last with DeleteTrack;
// what these do is move everything it carries onto the survivor first, so that
// delete's CASCADE finds nothing left to destroy.
// Locks the group for the rest of the transaction, so two merges of one group
// cannot run at once.
func (q *Queries) LockDuplicateGroupForMerge(ctx context.Context, id pgtype.UUID) (LockDuplicateGroupForMergeRow, error) {
row := q.db.QueryRow(ctx, lockDuplicateGroupForMerge, id)
var i LockDuplicateGroupForMergeRow
err := row.Scan(&i.ID, &i.Tier, &i.Status)
return i, err
}
const markDuplicateGroupMerged = `-- name: MarkDuplicateGroupMerged :execrows
UPDATE duplicate_groups
SET status = 'merged', resolved_at = now()
WHERE id = $1 AND status = 'pending'
`
func (q *Queries) MarkDuplicateGroupMerged(ctx context.Context, id pgtype.UUID) (int64, error) {
result, err := q.db.Exec(ctx, markDuplicateGroupMerged, id)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeCopyGeneralLikes = `-- name: MergeCopyGeneralLikes :many
INSERT INTO general_likes (user_id, track_id, liked_at)
SELECT user_id, $1::uuid, liked_at
FROM general_likes
WHERE track_id = $2::uuid
ON CONFLICT (user_id, track_id) DO UPDATE
SET liked_at = LEAST(general_likes.liked_at, EXCLUDED.liked_at)
RETURNING user_id
`
type MergeCopyGeneralLikesParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
// Collision-safe merges: a unique key includes track_id, so the survivor may
// already hold a matching row. Copy what it lacks; DeleteTrack's CASCADE then
// removes the loser's originals.
// One like per user. A user who liked both copies keeps a single like, dated to
// the earlier of the two.
func (q *Queries) MergeCopyGeneralLikes(ctx context.Context, arg MergeCopyGeneralLikesParams) ([]pgtype.UUID, error) {
rows, err := q.db.Query(ctx, mergeCopyGeneralLikes, arg.SurvivorID, arg.LoserID)
if err != nil {
return nil, err
}
defer rows.Close()
var items []pgtype.UUID
for rows.Next() {
var user_id pgtype.UUID
if err := rows.Scan(&user_id); err != nil {
return nil, err
}
items = append(items, user_id)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const mergeCopyTrackSimilarity = `-- name: MergeCopyTrackSimilarity :execrows
INSERT INTO track_similarity (track_a_id, track_b_id, score, source, fetched_at)
SELECT CASE WHEN track_a_id = $1::uuid THEN $2::uuid ELSE track_a_id END,
CASE WHEN track_b_id = $1::uuid THEN $2::uuid ELSE track_b_id END,
score, source, fetched_at
FROM track_similarity
WHERE (track_a_id = $1::uuid OR track_b_id = $1::uuid)
AND (CASE WHEN track_a_id = $1::uuid THEN $2::uuid ELSE track_a_id END)
<> (CASE WHEN track_b_id = $1::uuid THEN $2::uuid ELSE track_b_id END)
ON CONFLICT (track_a_id, track_b_id, source) DO NOTHING
`
type MergeCopyTrackSimilarityParams struct {
LoserID pgtype.UUID
SurvivorID pgtype.UUID
}
// Rewrites the loser to the survivor on either side of an edge. An edge between
// the two copies would become a track similar to itself — the table forbids
// that, and it means nothing — so it is dropped. An edge the survivor already
// has from the same source is kept as it is.
func (q *Queries) MergeCopyTrackSimilarity(ctx context.Context, arg MergeCopyTrackSimilarityParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeCopyTrackSimilarity, arg.LoserID, arg.SurvivorID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeCopyTrackTags = `-- name: MergeCopyTrackTags :execrows
INSERT INTO track_tags (track_id, tag, weight)
SELECT $1::uuid, tag, weight
FROM track_tags
WHERE track_id = $2::uuid
ON CONFLICT (track_id, tag) DO NOTHING
`
type MergeCopyTrackTagsParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
func (q *Queries) MergeCopyTrackTags(ctx context.Context, arg MergeCopyTrackTagsParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeCopyTrackTags, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeInheritTrackMbid = `-- name: MergeInheritTrackMbid :exec
UPDATE tracks AS survivor
SET mbid = loser.mbid
FROM tracks AS loser
WHERE survivor.id = $1::uuid
AND loser.id = $2::uuid
AND survivor.mbid IS NULL
AND loser.mbid IS NOT NULL
`
type MergeInheritTrackMbidParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
// A recording MBID is what the similarity pipeline keys on. If only the removed
// copy carried one, the survivor takes it rather than going dark to similarity.
func (q *Queries) MergeInheritTrackMbid(ctx context.Context, arg MergeInheritTrackMbidParams) error {
_, err := q.db.Exec(ctx, mergeInheritTrackMbid, arg.SurvivorID, arg.LoserID)
return err
}
const mergeRepointContextualLikes = `-- name: MergeRepointContextualLikes :execrows
UPDATE contextual_likes SET track_id = $1::uuid WHERE track_id = $2::uuid
`
type MergeRepointContextualLikesParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
func (q *Queries) MergeRepointContextualLikes(ctx context.Context, arg MergeRepointContextualLikesParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeRepointContextualLikes, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeRepointLidarrRequests = `-- name: MergeRepointLidarrRequests :execrows
UPDATE lidarr_requests SET matched_track_id = $1::uuid
WHERE matched_track_id = $2::uuid
`
type MergeRepointLidarrRequestsParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
func (q *Queries) MergeRepointLidarrRequests(ctx context.Context, arg MergeRepointLidarrRequestsParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeRepointLidarrRequests, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeRepointPlayEvents = `-- name: MergeRepointPlayEvents :execrows
UPDATE play_events SET track_id = $1::uuid WHERE track_id = $2::uuid
`
type MergeRepointPlayEventsParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
// Plain repoints: no unique key involves track_id, so moving rows cannot collide.
func (q *Queries) MergeRepointPlayEvents(ctx context.Context, arg MergeRepointPlayEventsParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeRepointPlayEvents, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeRepointPlaybackErrors = `-- name: MergeRepointPlaybackErrors :execrows
UPDATE playback_errors SET track_id = $1::uuid WHERE track_id = $2::uuid
`
type MergeRepointPlaybackErrorsParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
func (q *Queries) MergeRepointPlaybackErrors(ctx context.Context, arg MergeRepointPlaybackErrorsParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeRepointPlaybackErrors, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
const mergeRepointPlaylistTracks = `-- name: MergeRepointPlaylistTracks :many
UPDATE playlist_tracks SET track_id = $1::uuid
WHERE track_id = $2::uuid
RETURNING playlist_id
`
type MergeRepointPlaylistTracksParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
// playlist_tracks is keyed by (playlist_id, position), so repointing keeps every
// entry exactly where it was. A playlist that held both copies simply holds the
// survivor twice — the user put two entries there, and both stay.
func (q *Queries) MergeRepointPlaylistTracks(ctx context.Context, arg MergeRepointPlaylistTracksParams) ([]pgtype.UUID, error) {
rows, err := q.db.Query(ctx, mergeRepointPlaylistTracks, arg.SurvivorID, arg.LoserID)
if err != nil {
return nil, err
}
defer rows.Close()
var items []pgtype.UUID
for rows.Next() {
var playlist_id pgtype.UUID
if err := rows.Scan(&playlist_id); err != nil {
return nil, err
}
items = append(items, playlist_id)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const mergeRepointSkipEvents = `-- name: MergeRepointSkipEvents :execrows
UPDATE skip_events SET track_id = $1::uuid WHERE track_id = $2::uuid
`
type MergeRepointSkipEventsParams struct {
SurvivorID pgtype.UUID
LoserID pgtype.UUID
}
func (q *Queries) MergeRepointSkipEvents(ctx context.Context, arg MergeRepointSkipEventsParams) (int64, error) {
result, err := q.db.Exec(ctx, mergeRepointSkipEvents, arg.SurvivorID, arg.LoserID)
if err != nil {
return 0, err
}
return result.RowsAffected(), nil
}
-45
View File
@@ -297,42 +297,6 @@ type DiscoverTuning struct {
UpdatedAt pgtype.Timestamptz
}
type DuplicateGroup struct {
ID pgtype.UUID
MemberKey string
Tier string
WorstBitErrorRate *float32
Status string
DetectedAt pgtype.Timestamptz
LastSeenSweepID pgtype.UUID
ResolvedAt pgtype.Timestamptz
}
type DuplicateGroupMember struct {
GroupID pgtype.UUID
TrackID pgtype.UUID
}
type DuplicateSweep struct {
ID pgtype.UUID
StartedAt pgtype.Timestamptz
FinishedAt pgtype.Timestamptz
Candidates *int32
GroupsFound *int32
OversizeClusters *int32
ErrorMessage *string
}
type FingerprintSetting struct {
ID bool
Enabled bool
ChromaprintLengthSec int32
AcousticMaxBitErrorRate float64
BackfillConcurrency int32
SweepIntervalHours int32
UpdatedAt pgtype.Timestamptz
}
type GeneralLike struct {
UserID pgtype.UUID
TrackID pgtype.UUID
@@ -703,15 +667,6 @@ type Track struct {
MissingSince pgtype.Timestamptz
}
type TrackFingerprint struct {
TrackID pgtype.UUID
AudioStreamSha256 []byte
Chromaprint []int32
FingerprintVersion int16
ComputedAt pgtype.Timestamptz
ChromaprintLengthSec int32
}
type TrackSimilarity struct {
TrackAID pgtype.UUID
TrackBID pgtype.UUID
+16 -32
View File
@@ -829,7 +829,7 @@ similar_artists AS (
JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
WHERE asim.source = 'listenbrainz'
AND t.id NOT IN (SELECT id FROM excluded_ids)
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
ORDER BY asim.score DESC, random()
LIMIT $6
),
tag_overlap AS (
@@ -857,7 +857,7 @@ likes_overlap AS (
WHERE t.id = gl.track_id
AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
)
ORDER BY md5(gl.track_id::text || $12::text)
ORDER BY random()
LIMIT $8
),
taste_overlap AS (
@@ -884,7 +884,7 @@ coplay_artists AS (
WHERE asim.source = 'user_cooccurrence'
AND t.id NOT IN (SELECT id FROM excluded_ids)
AND t.id <> $2
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
ORDER BY asim.score DESC, random()
LIMIT $11
),
random_fill AS (
@@ -900,7 +900,7 @@ random_fill AS (
UNION SELECT track_id FROM taste_overlap
UNION SELECT track_id FROM coplay_artists
)
ORDER BY md5(t.id::text || $12::text)
ORDER BY random()
LIMIT $9
)
SELECT
@@ -938,18 +938,17 @@ GROUP BY t.id, t.title, t.album_id, t.artist_id, t.duration_ms, t.file_path,
`
type LoadRadioCandidatesV2Params struct {
UserID pgtype.UUID
ID pgtype.UUID
Column3 interface{}
Column4 []pgtype.UUID
Limit int32
Limit_2 int32
Limit_3 int32
Limit_4 int32
Limit_5 int32
Limit_6 int32
Limit_7 int32
Column12 string
UserID pgtype.UUID
ID pgtype.UUID
Column3 interface{}
Column4 []pgtype.UUID
Limit int32
Limit_2 int32
Limit_3 int32
Limit_4 int32
Limit_5 int32
Limit_6 int32
Limit_7 int32
}
type LoadRadioCandidatesV2Row struct {
@@ -972,22 +971,8 @@ type LoadRadioCandidatesV2Row struct {
// enter the pool even when the similarity/random arms miss them; scored
// in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
// $11 coplay_artists K (#1533 — tracks by artists co-played across the
// instance with the seed's artist; source='user_cooccurrence'),
// $12 order_seed (text) — see below.
// instance with the seed's artist; source='user_cooccurrence').
//
// $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
// a stable set only while their LIMIT exceeded the rows eligible for them: at
// that point they returned all of them and the order stopped mattering,
// because the caller sorts by track id before scoring. Below that threshold
// they returned a random SUBSET, and two builds on the same day drew
// different ones — so "daily determinism" held by accident, and only for
// libraries smaller than the limits.
//
// md5(id || seed) keeps the intent — an arbitrary spread that changes when
// the seed does — while making it reproducible for a given seed. The CALLER
// decides what that means: system mixes pass a per-(user, day) string and get
// the determinism they promise; radio passes a fresh value per request and
// keeps varying, which is what a radio should do.
// Returns same shape as LoadRadioCandidates plus similarity_score column.
func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandidatesV2Params) ([]LoadRadioCandidatesV2Row, error) {
rows, err := q.db.Query(ctx, loadRadioCandidatesV2,
@@ -1002,7 +987,6 @@ func (q *Queries) LoadRadioCandidatesV2(ctx context.Context, arg LoadRadioCandid
arg.Limit_5,
arg.Limit_6,
arg.Limit_7,
arg.Column12,
)
if err != nil {
return nil, err
+19 -16
View File
@@ -148,36 +148,39 @@ func (q *Queries) DeleteTrack(ctx context.Context, id pgtype.UUID) (DeleteTrackR
return i, err
}
const findMissingTrackByAudioHash = `-- name: FindMissingTrackByAudioHash :many
SELECT t.id, t.file_path
FROM tracks t
JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NOT NULL
AND f.audio_stream_sha256 = $1
const findMissingTrackByFingerprint = `-- name: FindMissingTrackByFingerprint :many
SELECT id, file_path FROM tracks
WHERE missing_since IS NOT NULL
AND file_size = $1
AND duration_ms = $2
LIMIT 2
`
type FindMissingTrackByAudioHashRow struct {
type FindMissingTrackByFingerprintParams struct {
FileSize int64
DurationMs int32
}
type FindMissingTrackByFingerprintRow struct {
ID pgtype.UUID
FilePath string
}
// Move detection fallback for files with no MBID (#2528, #3914). The audio stream
// hash identifies the encoded audio itself, so it survives a rename, a move and a
// retag — anything short of a re-encode. It replaced (file_size, duration_ms),
// which could pair two unrelated files that happened to share a byte count and a
// duration, and missed a file retagged in place, whose size changes.
// Move detection fallback for files with no MBID (#2528). Exact byte size AND
// exact decoded duration is a strong pair: a plain move or rename preserves
// both, while a re-encode changes at least one — and a re-encode genuinely is a
// different file, so failing to match there is correct rather than a gap.
//
// Same missing-only constraint and same LIMIT 2 rationale as the MBID variant.
func (q *Queries) FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]FindMissingTrackByAudioHashRow, error) {
rows, err := q.db.Query(ctx, findMissingTrackByAudioHash, audioStreamSha256)
func (q *Queries) FindMissingTrackByFingerprint(ctx context.Context, arg FindMissingTrackByFingerprintParams) ([]FindMissingTrackByFingerprintRow, error) {
rows, err := q.db.Query(ctx, findMissingTrackByFingerprint, arg.FileSize, arg.DurationMs)
if err != nil {
return nil, err
}
defer rows.Close()
var items []FindMissingTrackByAudioHashRow
var items []FindMissingTrackByFingerprintRow
for rows.Next() {
var i FindMissingTrackByAudioHashRow
var i FindMissingTrackByFingerprintRow
if err := rows.Scan(&i.ID, &i.FilePath); err != nil {
return nil, err
}
@@ -1,16 +0,0 @@
-- Drop the rows the narrower constraints are about to forbid, or re-adding
-- them fails against existing data (the 0051 down-migration pattern).
DELETE FROM recommendation_weight_profiles WHERE profile = 'songs_like';
DELETE FROM recommendation_tuning_audit WHERE scope = 'songs_like';
ALTER TABLE recommendation_tuning_audit
DROP CONSTRAINT recommendation_tuning_audit_scope_check;
ALTER TABLE recommendation_tuning_audit
ADD CONSTRAINT recommendation_tuning_audit_scope_check
CHECK (scope IN ('radio', 'daily_mix', 'taste', 'discover'));
ALTER TABLE recommendation_weight_profiles
DROP CONSTRAINT recommendation_weight_profiles_profile_check;
ALTER TABLE recommendation_weight_profiles
ADD CONSTRAINT recommendation_weight_profiles_profile_check
CHECK (profile IN ('radio', 'daily_mix'));
@@ -1,37 +0,0 @@
-- 0057_songs_like_tuning.up.sql — a THIRD weight profile, for Songs-like
-- (Scribe #3881, milestone #398).
--
-- Songs-like shared the `daily_mix` profile with For-You, and that is the bug.
-- The two surfaces want opposite things: For-You is a broad "what will they
-- enjoy today", Songs-like answers "what sounds like THIS", and under one set
-- of weights the broad answer wins. Operator, 2026-09-10: "I'm expecting to
-- get a consistent sound and style from the experience... I was getting a
-- seeming wide variety of music from each one when I was hoping to stay in a
-- certain neighborhood."
--
-- Under the shared daily_mix weights, an UNRELATED track the user had liked
-- and not played recently scored 1.0 + 2.0 + 1.0 = 4.0 before taste, while a
-- PERFECT similarity match they had not liked scored 1.0 + 1.5 = 2.5. Liking
-- something outranked sounding like the seed. Splitting the profile is what
-- lets similarity dominate here without making For-You narrow.
--
-- Rows are seeded by the recsettings boot reconcile, not here, so shipped
-- defaults live in exactly one place (Go) — same as 0040.
-- Rule #36: a new value for a CHECK-gated column needs the constraint
-- rewritten in the SAME change, or the first row written under the new
-- profile fails at runtime rather than at migrate time.
ALTER TABLE recommendation_weight_profiles
DROP CONSTRAINT recommendation_weight_profiles_profile_check;
ALTER TABLE recommendation_weight_profiles
ADD CONSTRAINT recommendation_weight_profiles_profile_check
CHECK (profile IN ('radio', 'daily_mix', 'songs_like'));
-- The audit table gates the same name on a separate constraint. Missing this
-- one would let the profile be seeded and then fail on the first knob turn —
-- green at boot, 500 on first use.
ALTER TABLE recommendation_tuning_audit
DROP CONSTRAINT recommendation_tuning_audit_scope_check;
ALTER TABLE recommendation_tuning_audit
ADD CONSTRAINT recommendation_tuning_audit_scope_check
CHECK (scope IN ('radio', 'daily_mix', 'taste', 'discover', 'songs_like'));
@@ -1 +0,0 @@
DROP TABLE track_fingerprints;
@@ -1,38 +0,0 @@
-- 0058_track_fingerprints.up.sql — an acoustic identity per track (Scribe
-- milestone #400: #3905, #3906).
--
-- A table of its own rather than columns on tracks, for the hot path's sake:
-- tracks is read with SELECT * by eight queries, among them ListTracksByAlbum,
-- SearchTracks and GetTracksByIDs — album pages, search, the Subsonic surface.
-- A ~4 KB chromaprint column on tracks would be de-TOASTed on every one of
-- those reads to carry a value only the duplicate sweep ever looks at.
--
-- What a row means, which the backfill depends on:
-- no row never fingerprinted
-- fingerprint_version < current derived by an older method; re-derive it
-- fingerprint_version = current attempted; a NULL value means that tool
-- failed on this file, and it is not retried
-- until the file changes
-- A failure that says nothing about the file — a timeout, a cancelled scan, a
-- missing binary — writes no row at all, so the backfill tries again.
CREATE TABLE track_fingerprints (
-- CASCADE is right here, unlike for the likes and play history M400's
-- merge has to carry across: a fingerprint describes one file's bytes and
-- means nothing once that file's row is gone.
track_id uuid PRIMARY KEY REFERENCES tracks (id) ON DELETE CASCADE,
-- SHA-256 of the ENCODED audio packets (ffmpeg -c:a copy -f hash), not of
-- decoded samples. internal/library/fingerprint.go says why.
audio_stream_sha256 bytea
CHECK (audio_stream_sha256 IS NULL OR octet_length(audio_stream_sha256) = 32),
-- fpcalc -raw -signed: the same 32 bits per item, stored signed because
-- integer is.
chromaprint integer[],
fingerprint_version smallint NOT NULL,
computed_at timestamptz NOT NULL DEFAULT now()
);
-- The exact duplicate tier is an equality match on this column. Partial
-- because a NULL is never looked up — it only means the hash was not taken.
CREATE INDEX track_fingerprints_audio_stream_sha256
ON track_fingerprints (audio_stream_sha256)
WHERE audio_stream_sha256 IS NOT NULL;
@@ -1,4 +0,0 @@
DROP INDEX IF EXISTS tracks_duration_id_idx;
DROP TABLE duplicate_group_members;
DROP TABLE duplicate_groups;
DROP TABLE duplicate_sweeps;
@@ -1,54 +0,0 @@
-- 0059_duplicate_groups.up.sql — proposed duplicates and the sweeps that find
-- them (Scribe milestone #400: #3910).
--
-- The sweep compares fingerprints (track_fingerprints, 0058) and proposes groups
-- of tracks that hold one recording. Nothing here merges anything: a group is a
-- proposal the operator reviews, and the merge (#3911) is a separate act.
-- One row per sweep. Lets the report tell "the sweep has never run" apart from
-- "it ran and found nothing", and gives the in-flight guard something to check,
-- the same way scan_runs does for the library scan.
CREATE TABLE duplicate_sweeps (
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
started_at timestamptz NOT NULL DEFAULT now(),
finished_at timestamptz,
candidates integer,
groups_found integer,
oversize_clusters integer,
error_message text
);
CREATE INDEX duplicate_sweeps_started_at_idx ON duplicate_sweeps (started_at DESC);
CREATE TABLE duplicate_groups (
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
-- The group's identity: its member track ids, sorted and joined. A sweep
-- that finds the same tracks again updates this row rather than proposing
-- them twice, and a dismissal stays attached to the set it was made about.
member_key text NOT NULL UNIQUE,
-- Rule 36: a new value for either CHECK swaps the constraint in the same
-- migration.
tier text NOT NULL CHECK (tier IN ('exact', 'acoustic')),
-- Largest disagreement between any two members; NULL for exact groups,
-- which have no score.
worst_bit_error_rate real,
status text NOT NULL DEFAULT 'pending'
CHECK (status IN ('pending', 'dismissed', 'merged')),
detected_at timestamptz NOT NULL DEFAULT now(),
last_seen_sweep_id uuid REFERENCES duplicate_sweeps (id) ON DELETE SET NULL,
resolved_at timestamptz
);
CREATE INDEX duplicate_groups_status_idx ON duplicate_groups (status);
CREATE TABLE duplicate_group_members (
group_id uuid NOT NULL REFERENCES duplicate_groups (id) ON DELETE CASCADE,
-- CASCADE is right here: a track that genuinely leaves the library has no
-- place in a proposal about its duplicates.
track_id uuid NOT NULL REFERENCES tracks (id) ON DELETE CASCADE,
PRIMARY KEY (group_id, track_id)
);
CREATE INDEX duplicate_group_members_track_idx ON duplicate_group_members (track_id);
-- The sweep streams candidates in (duration_ms, id) order, keyset-paged, so it
-- only ever holds a few seconds' worth of durations in memory. Without this each
-- page would sort the whole library again.
CREATE INDEX tracks_duration_id_idx ON tracks (duration_ms, id);
@@ -1 +0,0 @@
DROP INDEX IF EXISTS play_events_track_idx;
@@ -1,8 +0,0 @@
-- 0060_play_events_track_index.up.sql — play_events by track (Scribe #3912, #3911).
--
-- play_events is indexed by (user_id, started_at) and (user_id, track_id), both
-- led by user. Nothing reached it by track alone until the duplicates report,
-- which shows each copy's play count — a scan of the whole table per copy — and
-- the merge (#3911), which repoints a duplicate's play history onto the copy
-- being kept. Both ask "every play of this track", whoever played it.
CREATE INDEX play_events_track_idx ON play_events (track_id);
@@ -1,2 +0,0 @@
ALTER TABLE track_fingerprints DROP COLUMN chromaprint_length_sec;
DROP TABLE fingerprint_settings;
@@ -1,50 +0,0 @@
-- 0061_fingerprint_settings.up.sql — fingerprinting's knobs, in admin Settings
-- (Scribe #3913, milestone #400). Rule 25: anything an operator might tune is a
-- database row, changed without a restart. Singleton in the style of
-- reacquisition_settings (0056).
CREATE TABLE fingerprint_settings (
id boolean PRIMARY KEY DEFAULT true,
-- Fingerprinting new files, the backfill, and the duplicate sweep. Off stops
-- the decode work entirely — the reason to turn it off is a slow NAS, and
-- that is the operator's call. On by default: a library that cannot tell its
-- duplicates apart is what milestone #400 exists to end.
enabled boolean NOT NULL DEFAULT true,
-- Seconds of audio fpcalc fingerprints. Chromaprints taken at different
-- lengths cannot be compared, which is why track_fingerprints records the
-- length each row was taken at (below): change this and every chromaprint is
-- re-derived, and until then only rows at the new length are compared.
chromaprint_length_sec integer NOT NULL DEFAULT 120,
-- The most disagreement two aligned fingerprints may show and still be
-- proposed as one recording. Unrelated audio sits near 0.5, so the ceiling
-- stays well clear of it.
acoustic_max_bit_error_rate double precision NOT NULL DEFAULT 0.15,
-- Files the backfill decodes at once. Decoding competes with playback
-- transcoding for CPU and with streaming for the mount.
backfill_concurrency integer NOT NULL DEFAULT 2,
-- The least time between duplicate sweeps. A sweep still runs only when
-- fingerprints have changed since the last one.
sweep_interval_hours integer NOT NULL DEFAULT 1,
-- When the settings were last saved. A new threshold or length can change
-- what a sweep finds, so a save makes a sweep due.
updated_at timestamptz NOT NULL DEFAULT now(),
CONSTRAINT fingerprint_settings_singleton CHECK (id = true),
CONSTRAINT fingerprint_settings_length_range
CHECK (chromaprint_length_sec >= 30 AND chromaprint_length_sec <= 600),
CONSTRAINT fingerprint_settings_threshold_range
CHECK (acoustic_max_bit_error_rate >= 0.01 AND acoustic_max_bit_error_rate <= 0.35),
CONSTRAINT fingerprint_settings_concurrency_range
CHECK (backfill_concurrency >= 1 AND backfill_concurrency <= 8),
CONSTRAINT fingerprint_settings_sweep_interval_range
CHECK (sweep_interval_hours >= 1 AND sweep_interval_hours <= 168)
);
INSERT INTO fingerprint_settings (id) VALUES (true) ON CONFLICT (id) DO NOTHING;
-- Every row written so far was taken at fpcalc's default length.
ALTER TABLE track_fingerprints ADD COLUMN chromaprint_length_sec integer NOT NULL DEFAULT 120;
-156
View File
@@ -1,156 +0,0 @@
-- name: StartDuplicateSweep :one
INSERT INTO duplicate_sweeps DEFAULT VALUES RETURNING id, started_at;
-- name: FinishDuplicateSweep :exec
UPDATE duplicate_sweeps
SET finished_at = now(),
candidates = sqlc.arg(candidates),
groups_found = sqlc.arg(groups_found),
oversize_clusters = sqlc.arg(oversize_clusters),
error_message = NULLIF(sqlc.arg(error_message)::text, '')
WHERE id = sqlc.arg(id);
-- name: GetInFlightDuplicateSweep :one
-- The guard against two sweeps at once: "in flight" is finished_at IS NULL.
SELECT id, started_at
FROM duplicate_sweeps
WHERE finished_at IS NULL
ORDER BY started_at DESC
LIMIT 1;
-- name: GetLatestDuplicateSweep :one
SELECT id, started_at, finished_at, candidates, groups_found, oversize_clusters, error_message
FROM duplicate_sweeps
ORDER BY started_at DESC
LIMIT 1;
-- name: GetLatestFingerprintComputedAt :one
-- Whether a sweep has anything new to look at: fingerprints written since the
-- last sweep started.
SELECT max(computed_at)::timestamptz AS latest FROM track_fingerprints;
-- name: ListExactDuplicateHashes :many
-- The exact tier, library-wide in one pass: identical encoded audio shared by
-- more than one present track.
SELECT f.audio_stream_sha256,
array_agg(t.id ORDER BY t.id)::uuid[] AS track_ids
FROM track_fingerprints f
JOIN tracks t ON t.id = f.track_id
WHERE t.missing_since IS NULL
AND f.fingerprint_version >= sqlc.arg(current_version)
AND f.audio_stream_sha256 IS NOT NULL
GROUP BY f.audio_stream_sha256
HAVING count(*) > 1;
-- name: ListDuplicateCandidates :many
-- The acoustic tier's input, one page at a time in (duration_ms, id) order so the
-- sweep holds only a sliding window of durations. Tracks without a chromaprint
-- cannot be compared acoustically and are left out; any exact duplicates among
-- them come from ListExactDuplicateHashes.
SELECT t.id, t.duration_ms, f.audio_stream_sha256, f.chromaprint
FROM tracks t
JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL
AND f.fingerprint_version >= sqlc.arg(current_version)
AND f.chromaprint IS NOT NULL
-- Only chromaprints taken at the current length: prints at two lengths are not
-- comparable, and after a length change the backfill is still re-deriving the
-- rest (#3913).
AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec)
AND (t.duration_ms, t.id) > (sqlc.arg(after_duration_ms)::integer, sqlc.arg(after_id)::uuid)
ORDER BY t.duration_ms, t.id
LIMIT sqlc.arg(page_limit);
-- name: ListDismissedDuplicateMemberSets :many
-- What the operator has already said are not duplicates. A new proposal whose
-- every member sat together in one of these is not proposed again.
SELECT g.id, array_agg(m.track_id ORDER BY m.track_id)::uuid[] AS track_ids
FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id
WHERE g.status = 'dismissed'
GROUP BY g.id;
-- name: UpsertDuplicateGroup :one
-- Proposes a group, or refreshes one already pending. A group already dismissed
-- or merged is left exactly as it is: the WHERE on the update makes the conflict
-- a no-op, and the caller sees no row.
INSERT INTO duplicate_groups (member_key, tier, worst_bit_error_rate, last_seen_sweep_id)
VALUES (sqlc.arg(member_key), sqlc.arg(tier), sqlc.narg(worst_bit_error_rate), sqlc.arg(sweep_id))
ON CONFLICT (member_key) DO UPDATE
SET tier = EXCLUDED.tier,
worst_bit_error_rate = EXCLUDED.worst_bit_error_rate,
last_seen_sweep_id = EXCLUDED.last_seen_sweep_id
WHERE duplicate_groups.status = 'pending'
RETURNING id;
-- name: AddDuplicateGroupMember :exec
INSERT INTO duplicate_group_members (group_id, track_id)
VALUES (sqlc.arg(group_id), sqlc.arg(track_id))
ON CONFLICT DO NOTHING;
-- name: DeleteStalePendingDuplicateGroups :execrows
-- A pending proposal this sweep did not find again no longer describes the
-- library: a member was re-fingerprinted, merged away or went missing. Dismissed
-- groups are kept regardless — they are the memory of a decision.
--
-- Only proposals last confirmed by an EARLIER sweep go. Should two sweeps ever
-- overlap (a manual trigger racing the worker), neither may delete what the other
-- has just found.
DELETE FROM duplicate_groups g
WHERE g.status = 'pending'
AND g.last_seen_sweep_id IS DISTINCT FROM sqlc.arg(sweep_id)
AND (g.last_seen_sweep_id IS NULL
OR (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = g.last_seen_sweep_id)
< (SELECT s.started_at FROM duplicate_sweeps s WHERE s.id = sqlc.arg(sweep_id)));
-- name: CountPendingDuplicateGroups :one
-- Proposals awaiting review. A group left with one member — its other tracks
-- deleted since the sweep — is no proposal at all and is not counted; the next
-- sweep retires it.
SELECT count(*)::bigint
FROM duplicate_groups g
WHERE g.status = 'pending'
AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2;
-- name: ListPendingDuplicateGroupMembers :many
-- One page of proposals, newest first, flattened to one row per member so the
-- handler folds them without a query per group. What each copy carries — likes
-- and plays from every user — is here because it is what the operator weighs
-- when deciding which copy to keep.
WITH page AS (
SELECT g.id, g.tier, g.worst_bit_error_rate, g.detected_at
FROM duplicate_groups g
WHERE g.status = 'pending'
AND (SELECT count(*) FROM duplicate_group_members m WHERE m.group_id = g.id) >= 2
ORDER BY g.detected_at DESC, g.id
LIMIT sqlc.arg(page_limit) OFFSET sqlc.arg(page_offset)
)
SELECT p.id AS group_id,
p.tier,
p.worst_bit_error_rate,
p.detected_at,
t.id AS track_id,
t.title,
artists.name AS artist_name,
albums.id AS album_id,
albums.title AS album_title,
t.file_path,
t.file_format,
t.file_size,
t.duration_ms,
t.added_at,
(SELECT count(*) FROM general_likes l WHERE l.track_id = t.id)::bigint AS like_count,
(SELECT count(*) FROM play_events e WHERE e.track_id = t.id)::bigint AS play_count
FROM page p
JOIN duplicate_group_members m ON m.group_id = p.id
JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
JOIN artists ON artists.id = t.artist_id
ORDER BY p.detected_at DESC, p.id, t.id;
-- name: DismissDuplicateGroup :execrows
-- "These are not duplicates." Only a pending group can be dismissed; zero rows
-- means it was already resolved or no longer exists.
UPDATE duplicate_groups
SET status = 'dismissed', resolved_at = now()
WHERE id = sqlc.arg(id) AND status = 'pending';
@@ -1,15 +0,0 @@
-- name: GetFingerprintSettings :one
SELECT * FROM fingerprint_settings WHERE id = true;
-- name: UpdateFingerprintSettings :one
-- Whole-row write from the admin card; migration 0061's CHECKs are the backstop
-- behind the service's own validation.
UPDATE fingerprint_settings
SET enabled = sqlc.arg(enabled),
chromaprint_length_sec = sqlc.arg(chromaprint_length_sec),
acoustic_max_bit_error_rate = sqlc.arg(acoustic_max_bit_error_rate),
backfill_concurrency = sqlc.arg(backfill_concurrency),
sweep_interval_hours = sqlc.arg(sweep_interval_hours),
updated_at = now()
WHERE id = true
RETURNING *;
-70
View File
@@ -1,70 +0,0 @@
-- name: UpsertTrackFingerprint :exec
-- Written whenever a track's fingerprint is derived: by the scan when a file is
-- new or its bytes changed, and by the backfill (#3908) for rows derived by an
-- older method. Replaces the row wholesale — a fingerprint of the old bytes has
-- no standing once the file has changed.
INSERT INTO track_fingerprints (
track_id, audio_stream_sha256, chromaprint, fingerprint_version, chromaprint_length_sec
) VALUES (
sqlc.arg(track_id), sqlc.narg(audio_stream_sha256), sqlc.narg(chromaprint),
sqlc.arg(fingerprint_version), sqlc.arg(chromaprint_length_sec)
)
ON CONFLICT (track_id) DO UPDATE SET
audio_stream_sha256 = EXCLUDED.audio_stream_sha256,
chromaprint = EXCLUDED.chromaprint,
fingerprint_version = EXCLUDED.fingerprint_version,
chromaprint_length_sec = EXCLUDED.chromaprint_length_sec,
computed_at = now();
-- name: DeleteTrackFingerprint :exec
-- A file changed but could not be fingerprinted, for a reason unrelated to the
-- file. The stored row describes the OLD bytes, so it goes and the backfill
-- re-derives it — nothing may keep trusting a stale identity.
DELETE FROM track_fingerprints WHERE track_id = $1;
-- name: ListTracksNeedingFingerprint :many
-- The backfill's work queue (#3908): tracks with no fingerprint, or one derived
-- by an older method. Keyset-paged on id so a pass visits each track at most
-- once. That cursor is load-bearing: an inconclusive attempt writes no row, so
-- without it a file that keeps timing out would be listed again straight away
-- and retried in a tight loop. Missing tracks are skipped — there is no file to
-- read.
SELECT t.id, t.file_path
FROM tracks t
LEFT JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL
-- A row taken at another length is as stale as one from an older method:
-- chromaprints at two lengths cannot be compared (#3913).
AND (f.track_id IS NULL
OR f.fingerprint_version < sqlc.arg(current_version)
OR f.chromaprint_length_sec <> sqlc.arg(chromaprint_length_sec))
AND t.id > sqlc.arg(after_id)
ORDER BY t.id
LIMIT sqlc.arg(batch_limit);
-- name: GetFingerprintCoverage :one
-- The admin gauge for the backfill. fingerprinted + rejected + pending = total.
-- "Current" means derived by the current method AT the current length: a row at
-- another length is pending, because the backfill will re-derive it. rejected is
-- a current row with a NULL half: a tool ran and refused the file, which is
-- settled rather than waiting. Missing tracks are excluded, or the gauge could
-- never reach the end.
SELECT count(*)::bigint AS total,
count(*) FILTER (
WHERE f.fingerprint_version >= sqlc.arg(current_version)
AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec)
AND f.audio_stream_sha256 IS NOT NULL AND f.chromaprint IS NOT NULL
)::bigint AS fingerprinted,
count(*) FILTER (
WHERE f.fingerprint_version >= sqlc.arg(current_version)
AND f.chromaprint_length_sec = sqlc.arg(chromaprint_length_sec)
AND (f.audio_stream_sha256 IS NULL OR f.chromaprint IS NULL)
)::bigint AS rejected,
count(*) FILTER (
WHERE f.track_id IS NULL
OR f.fingerprint_version < sqlc.arg(current_version)
OR f.chromaprint_length_sec <> sqlc.arg(chromaprint_length_sec)
)::bigint AS pending
FROM tracks t
LEFT JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NULL;
-101
View File
@@ -1,101 +0,0 @@
-- Duplicate merge (Scribe #3911). Every statement here runs inside the one
-- transaction library.MergeDuplicateGroup opens, after the removed copy's file
-- is already gone. The loser's own track row is deleted last with DeleteTrack;
-- what these do is move everything it carries onto the survivor first, so that
-- delete's CASCADE finds nothing left to destroy.
-- name: LockDuplicateGroupForMerge :one
-- Locks the group for the rest of the transaction, so two merges of one group
-- cannot run at once.
SELECT id, tier, status
FROM duplicate_groups
WHERE id = sqlc.arg(id)
FOR UPDATE;
-- name: ListDuplicateGroupMergeMembers :many
SELECT t.id, t.file_path, t.file_format, t.file_size, t.added_at, t.album_id,
t.mbid, albums.mbid AS album_mbid
FROM duplicate_group_members m
JOIN tracks t ON t.id = m.track_id
JOIN albums ON albums.id = t.album_id
WHERE m.group_id = sqlc.arg(group_id)
ORDER BY t.id;
-- Plain repoints: no unique key involves track_id, so moving rows cannot collide.
-- name: MergeRepointPlayEvents :execrows
UPDATE play_events SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid;
-- name: MergeRepointSkipEvents :execrows
UPDATE skip_events SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid;
-- name: MergeRepointContextualLikes :execrows
UPDATE contextual_likes SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid;
-- name: MergeRepointPlaybackErrors :execrows
UPDATE playback_errors SET track_id = sqlc.arg(survivor_id)::uuid WHERE track_id = sqlc.arg(loser_id)::uuid;
-- name: MergeRepointLidarrRequests :execrows
UPDATE lidarr_requests SET matched_track_id = sqlc.arg(survivor_id)::uuid
WHERE matched_track_id = sqlc.arg(loser_id)::uuid;
-- name: MergeRepointPlaylistTracks :many
-- playlist_tracks is keyed by (playlist_id, position), so repointing keeps every
-- entry exactly where it was. A playlist that held both copies simply holds the
-- survivor twice — the user put two entries there, and both stay.
UPDATE playlist_tracks SET track_id = sqlc.arg(survivor_id)::uuid
WHERE track_id = sqlc.arg(loser_id)::uuid
RETURNING playlist_id;
-- Collision-safe merges: a unique key includes track_id, so the survivor may
-- already hold a matching row. Copy what it lacks; DeleteTrack's CASCADE then
-- removes the loser's originals.
-- name: MergeCopyGeneralLikes :many
-- One like per user. A user who liked both copies keeps a single like, dated to
-- the earlier of the two.
INSERT INTO general_likes (user_id, track_id, liked_at)
SELECT user_id, sqlc.arg(survivor_id)::uuid, liked_at
FROM general_likes
WHERE track_id = sqlc.arg(loser_id)::uuid
ON CONFLICT (user_id, track_id) DO UPDATE
SET liked_at = LEAST(general_likes.liked_at, EXCLUDED.liked_at)
RETURNING user_id;
-- name: MergeCopyTrackTags :execrows
INSERT INTO track_tags (track_id, tag, weight)
SELECT sqlc.arg(survivor_id)::uuid, tag, weight
FROM track_tags
WHERE track_id = sqlc.arg(loser_id)::uuid
ON CONFLICT (track_id, tag) DO NOTHING;
-- name: MergeCopyTrackSimilarity :execrows
-- Rewrites the loser to the survivor on either side of an edge. An edge between
-- the two copies would become a track similar to itself — the table forbids
-- that, and it means nothing — so it is dropped. An edge the survivor already
-- has from the same source is kept as it is.
INSERT INTO track_similarity (track_a_id, track_b_id, score, source, fetched_at)
SELECT CASE WHEN track_a_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_a_id END,
CASE WHEN track_b_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_b_id END,
score, source, fetched_at
FROM track_similarity
WHERE (track_a_id = sqlc.arg(loser_id)::uuid OR track_b_id = sqlc.arg(loser_id)::uuid)
AND (CASE WHEN track_a_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_a_id END)
<> (CASE WHEN track_b_id = sqlc.arg(loser_id)::uuid THEN sqlc.arg(survivor_id)::uuid ELSE track_b_id END)
ON CONFLICT (track_a_id, track_b_id, source) DO NOTHING;
-- name: MergeInheritTrackMbid :exec
-- A recording MBID is what the similarity pipeline keys on. If only the removed
-- copy carried one, the survivor takes it rather than going dark to similarity.
UPDATE tracks AS survivor
SET mbid = loser.mbid
FROM tracks AS loser
WHERE survivor.id = sqlc.arg(survivor_id)::uuid
AND loser.id = sqlc.arg(loser_id)::uuid
AND survivor.mbid IS NULL
AND loser.mbid IS NOT NULL;
-- name: MarkDuplicateGroupMerged :execrows
UPDATE duplicate_groups
SET status = 'merged', resolved_at = now()
WHERE id = sqlc.arg(id) AND status = 'pending';
+5 -20
View File
@@ -45,22 +45,7 @@ WHERE t.id <> $2
-- enter the pool even when the similarity/random arms miss them; scored
-- in Go via TasteMatch, so sim_score here is 0 pool-inclusion),
-- $11 coplay_artists K (#1533 — tracks by artists co-played across the
-- instance with the seed's artist; source='user_cooccurrence'),
-- $12 order_seed (text) — see below.
--
-- $12 REPLACES `ORDER BY random()` IN FOUR ARMS (#3889). Those arms returned
-- a stable set only while their LIMIT exceeded the rows eligible for them: at
-- that point they returned all of them and the order stopped mattering,
-- because the caller sorts by track id before scoring. Below that threshold
-- they returned a random SUBSET, and two builds on the same day drew
-- different ones — so "daily determinism" held by accident, and only for
-- libraries smaller than the limits.
--
-- md5(id || seed) keeps the intent — an arbitrary spread that changes when
-- the seed does — while making it reproducible for a given seed. The CALLER
-- decides what that means: system mixes pass a per-(user, day) string and get
-- the determinism they promise; radio passes a fresh value per request and
-- keeps varying, which is what a radio should do.
-- instance with the seed's artist; source='user_cooccurrence').
-- Returns same shape as LoadRadioCandidates plus similarity_score column.
WITH
@@ -102,7 +87,7 @@ similar_artists AS (
JOIN seed_artist sa ON asim.artist_a_id = sa.artist_id
WHERE asim.source = 'listenbrainz'
AND t.id NOT IN (SELECT id FROM excluded_ids)
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
ORDER BY asim.score DESC, random()
LIMIT $6
),
tag_overlap AS (
@@ -130,7 +115,7 @@ likes_overlap AS (
WHERE t.id = gl.track_id
AND trim(g_overlap.g) IN (SELECT tag FROM seed_tags)
)
ORDER BY md5(gl.track_id::text || $12::text)
ORDER BY random()
LIMIT $8
),
taste_overlap AS (
@@ -157,7 +142,7 @@ coplay_artists AS (
WHERE asim.source = 'user_cooccurrence'
AND t.id NOT IN (SELECT id FROM excluded_ids)
AND t.id <> $2
ORDER BY asim.score DESC, md5(t.id::text || $12::text)
ORDER BY asim.score DESC, random()
LIMIT $11
),
random_fill AS (
@@ -173,7 +158,7 @@ random_fill AS (
UNION SELECT track_id FROM taste_overlap
UNION SELECT track_id FROM coplay_artists
)
ORDER BY md5(t.id::text || $12::text)
ORDER BY random()
LIMIT $9
)
SELECT
+9 -11
View File
@@ -155,19 +155,17 @@ SELECT id, file_path FROM tracks
AND mbid = sqlc.arg(mbid)::text
LIMIT 2;
-- name: FindMissingTrackByAudioHash :many
-- Move detection fallback for files with no MBID (#2528, #3914). The audio stream
-- hash identifies the encoded audio itself, so it survives a rename, a move and a
-- retag — anything short of a re-encode. It replaced (file_size, duration_ms),
-- which could pair two unrelated files that happened to share a byte count and a
-- duration, and missed a file retagged in place, whose size changes.
-- name: FindMissingTrackByFingerprint :many
-- Move detection fallback for files with no MBID (#2528). Exact byte size AND
-- exact decoded duration is a strong pair: a plain move or rename preserves
-- both, while a re-encode changes at least one — and a re-encode genuinely is a
-- different file, so failing to match there is correct rather than a gap.
--
-- Same missing-only constraint and same LIMIT 2 rationale as the MBID variant.
SELECT t.id, t.file_path
FROM tracks t
JOIN track_fingerprints f ON f.track_id = t.id
WHERE t.missing_since IS NOT NULL
AND f.audio_stream_sha256 = sqlc.arg(audio_stream_sha256)
SELECT id, file_path FROM tracks
WHERE missing_since IS NOT NULL
AND file_size = sqlc.arg(file_size)
AND duration_ms = sqlc.arg(duration_ms)
LIMIT 2;
-- name: AdoptTrackPath :execrows
-15
View File
@@ -87,10 +87,6 @@ var dataTables = []string{
// pristine Discover knobs rather than whatever a previous test tuned.
"discover_tuning",
"recommendation_tuning_audit",
"duplicate_group_members", // M400
"duplicate_groups",
"duplicate_sweeps",
"track_fingerprints", // M400
"tracks",
"albums",
"artists",
@@ -130,15 +126,4 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) {
); err != nil {
t.Fatalf("dbtest.ResetDB reset tag-sources version: %v", err)
}
// Fingerprinting settings (M400 #3913), a singleton like the counters above.
// Every column goes back to its migration default rather than to literals
// written here, so a test can pin the Go defaults to the migration's.
if _, err := pool.Exec(ctx, `
UPDATE fingerprint_settings
SET enabled = DEFAULT, chromaprint_length_sec = DEFAULT,
acoustic_max_bit_error_rate = DEFAULT, backfill_concurrency = DEFAULT,
sweep_interval_hours = DEFAULT, updated_at = DEFAULT`,
); err != nil {
t.Fatalf("dbtest.ResetDB reset fingerprint settings: %v", err)
}
}
+32 -161
View File
@@ -5,16 +5,12 @@ import (
"errors"
"fmt"
"io/fs"
"log/slog"
"os"
"path/filepath"
"syscall"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/coverart"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync"
)
@@ -23,179 +19,54 @@ import (
// that has no row in tracks.
var ErrTrackNotFound = errors.New("library: track not found")
// removeFile is os.Remove behind a variable so a test can make removal fail the
// way a read-only mount or a wrongly-owned directory does. A chmod-based test
// cannot stand in for that: root ignores permission bits, so in a CI container
// running as root it would pass without ever exercising the failure.
var removeFile = os.Remove
// FileRemoveError reports that a track's file exists but could not be removed.
// When DeleteTrackFile returns one, NOTHING was deleted: the row, its likes, its
// play history and its playlist memberships are all intact.
type FileRemoveError struct {
Path string
// UID and GID are the identity the server process runs as — the half of a
// permission problem the operator cannot see from the host side.
UID, GID int
Err error
}
func (e *FileRemoveError) Error() string { return fmt.Sprintf("remove track file: %v", e.Err) }
func (e *FileRemoveError) Unwrap() error { return e.Err }
// Dir is the directory removal needs write access to. Unlinking a file writes to
// its PARENT, so a world-writable file inside a read-only directory still cannot
// be removed — naming the file's own permissions would send the operator to the
// wrong place.
func (e *FileRemoveError) Dir() string { return filepath.Dir(e.Path) }
// NotWritable reports whether the library is unwritable for this process — a
// read-only mount or a permission denial — rather than an I/O fault. It is the
// case the operator can fix, so callers answer it differently.
func (e *FileRemoveError) NotWritable() bool {
return errors.Is(e.Err, fs.ErrPermission) || errors.Is(e.Err, syscall.EROFS)
}
// Reason is the underlying cause without the path os.Remove already wrapped
// around it, for messages that name the directory themselves.
func (e *FileRemoveError) Reason() string {
var pathErr *fs.PathError
if errors.As(e.Err, &pathErr) {
return pathErr.Err.Error()
}
return e.Err.Error()
}
// DeletedTrack reports what a delete tidied away beyond the track itself.
type DeletedTrack struct {
// AlbumID is set when the track was its album's last, so the album went too.
AlbumID *pgtype.UUID
// ArtistID is set when that album was its artist's last, so the artist went too.
ArtistID *pgtype.UUID
}
// DeleteTrackFile removes a track's file from disk and then its row, tidying
// away an album or artist the delete leaves empty. It is the ONLY path that
// deletes a track file: the admin remove-track endpoint and quarantine's Delete
// file both come through here (#3918).
// DeleteTrackFile removes a track file from disk and its row from the
// tracks table. Album and artist rows are left untouched.
//
// Order is the whole contract. The file goes first, and if it cannot go — a
// read-only mount, a permission denial, an I/O error — nothing else happens and
// a *FileRemoveError comes back. Proceeding past that failure is how #3918 lost
// history: tracks CASCADEs to play_events, general_likes, contextual_likes,
// playlist_tracks, track_tags and playback_errors, so the row and everything
// hanging off it were destroyed while the file survived, and the next scan
// re-imported it as a brand-new track with none of it.
// Steps:
// 1. Look up the track to get its file_path.
// 2. Remove the file from disk. fs.ErrNotExist is OK — already gone.
// 3. Delete the tracks row.
//
// A file that is already gone (fs.ErrNotExist) is not a failure; the row is
// removed as asked.
// Order matters: file first, then DB. If the file delete fails (permission,
// I/O error), we leave the DB row alone so the admin can retry.
//
// This is NOT the missing-file path. That lifecycle is deliberately
// non-destructive: reconcile stamps missing_since (#2523), selection paths
// filter on it, and a returning file is un-marked or adopted (#2528). This is the
// explicit, irreversible "remove this recording", never the way to tidy up a row
// whose file merely went away.
// The reverse failure mode — file gone, DB row still present — IS reconciled
// now, and not by this function: the scan's reconcile pass stamps
// tracks.missing_since (#2523), every selection path filters on it, and a file
// that returns is un-marked or adopted at its new path (#2528). That is the
// normal life of a vanished file and it is deliberately non-destructive: the
// row, its play history and its likes survive, because a missing file is a
// track Minstrel still knows about (#2527).
//
// dataDir, when set, also clears the cached art of an artist the delete removed.
// logger may be nil.
func DeleteTrackFile(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, trackID pgtype.UUID,
) (DeletedTrack, error) {
if logger == nil {
logger = slog.Default()
}
// So this function is NOT the missing-file path. It is the explicit admin
// action "remove this recording from disk and from the library", and it is
// irreversible: tracks CASCADEs to play_events, general_likes_tracks,
// contextual_likes, track_tags and playback_errors. Reach for it when the
// operator means to destroy the record, never to tidy up a row whose file
// merely went away.
func DeleteTrackFile(ctx context.Context, pool *pgxpool.Pool, trackID pgtype.UUID) error {
q := dbq.New(pool)
track, err := q.GetTrackByID(ctx, trackID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return DeletedTrack{}, ErrTrackNotFound
return ErrTrackNotFound
}
return DeletedTrack{}, fmt.Errorf("get track: %w", err)
return fmt.Errorf("get track: %w", err)
}
if err := removeTrackFileOnDisk(track.FilePath); err != nil {
return DeletedTrack{}, err
if err := os.Remove(track.FilePath); err != nil && !errors.Is(err, fs.ErrNotExist) {
return fmt.Errorf("remove file: %w", err)
}
// The row and any album or artist it empties go together, so a failure
// partway cannot leave a deleted track with a ghost album behind it.
tx, err := pool.Begin(ctx)
if err != nil {
return DeletedTrack{}, fmt.Errorf("begin tx: %w", err)
if _, err := pool.Exec(ctx, "DELETE FROM tracks WHERE id = $1", trackID); err != nil {
return fmt.Errorf("delete row: %w", err)
}
defer func() { _ = tx.Rollback(ctx) }()
tq := dbq.New(tx)
deleted, err := tq.DeleteTrack(ctx, trackID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
// Removed by someone else between the lookup and here.
return DeletedTrack{}, ErrTrackNotFound
}
return DeletedTrack{}, fmt.Errorf("delete track: %w", err)
}
out, err := tidyEmptiedAlbum(ctx, tq, deleted.AlbumID)
if err != nil {
return DeletedTrack{}, err
}
if err := tx.Commit(ctx); err != nil {
return DeletedTrack{}, fmt.Errorf("commit: %w", err)
}
// Both of these run after the delete has committed, so neither may fail
// it: the recording is gone either way. An unlogged change leaves the track
// in offline clients' caches until the next scan touches its album; a
// leftover art directory is only disk.
// Log the change after the delete succeeds. Best-effort: a Warn-level
// failure here would leave the cache index orphaned on offline clients
// until the next scan touches the surrounding album.
if err := syncpkg.LogChange(ctx, pool, syncpkg.EntityTrack,
syncpkg.FormatUUID(trackID), syncpkg.OpDelete); err != nil {
logger.Warn("track delete: LogChange failed", "track_id", syncpkg.FormatUUID(trackID), "err", err)
}
if out.ArtistID != nil && dataDir != "" {
if err := coverart.CleanupArtistArt(dataDir, *out.ArtistID); err != nil {
logger.Warn("track delete: artist-art cleanup failed",
"artist_id", syncpkg.FormatUUID(*out.ArtistID), "err", err)
}
}
return out, nil
}
// removeTrackFileOnDisk is the one rule for removing a track's file, shared by
// DeleteTrackFile and the duplicate merge. A file already gone is fine; anything
// else comes back as a *FileRemoveError, and the caller must then change nothing
// in the database (#3918).
func removeTrackFileOnDisk(path string) error {
if err := removeFile(path); err != nil && !errors.Is(err, fs.ErrNotExist) {
return &FileRemoveError{Path: path, UID: os.Getuid(), GID: os.Getgid(), Err: err}
return fmt.Errorf("log change: %w", err)
}
return nil
}
// tidyEmptiedAlbum removes an album a track delete left with no tracks, and its
// artist if that album was the artist's last. It runs on the caller's
// transaction, so the tidy-up commits or rolls back with the delete itself.
func tidyEmptiedAlbum(ctx context.Context, tq *dbq.Queries, albumID pgtype.UUID) (DeletedTrack, error) {
var out DeletedTrack
album, err := tq.DeleteAlbumIfEmpty(ctx, albumID)
switch {
case err == nil:
id := album.ID
out.AlbumID = &id
artistID, aerr := tq.DeleteArtistIfEmpty(ctx, album.ArtistID)
switch {
case aerr == nil:
out.ArtistID = &artistID
case errors.Is(aerr, pgx.ErrNoRows):
// The artist still has other albums or stray tracks.
default:
return DeletedTrack{}, fmt.Errorf("delete artist if empty: %w", aerr)
}
case errors.Is(err, pgx.ErrNoRows):
// The album still has other tracks.
default:
return DeletedTrack{}, fmt.Errorf("delete album if empty: %w", err)
}
return out, nil
}

Some files were not shown because too many files have changed in this diff Show More