diff --git a/.github/workflows/release-podcast-tests.yml b/.github/workflows/release-podcast-tests.yml new file mode 100644 index 000000000..6d93ebb63 --- /dev/null +++ b/.github/workflows/release-podcast-tests.yml @@ -0,0 +1,29 @@ +name: Release podcast offline tests +on: + pull_request: + paths: + - release/podcast/** + - .github/workflows/release-podcast*.yml + push: + branches: [master] + paths: + - release/podcast/** + - .github/workflows/release-podcast*.yml +permissions: + contents: read +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version-file: go.mod + - name: Install tools for the offline MP3 decode test + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends ffmpeg + - run: go test -race -count=1 -v ./release/podcast diff --git a/.github/workflows/release-podcast.yml b/.github/workflows/release-podcast.yml new file mode 100644 index 000000000..d25597e34 --- /dev/null +++ b/.github/workflows/release-podcast.yml @@ -0,0 +1,126 @@ +name: Release podcast + +on: + release: + types: [published] + workflow_dispatch: + inputs: + from: + description: Published version, or inclusive first version of a range + default: v0.64.0 + required: true + type: string + to: + description: Optional inclusive last version (at most three releases) + default: v0.64.2 + required: false + type: string + mode: + description: Validate is free; script and audio use OpenAI + default: validate + type: choice + options: [validate, script, audio] + include_prereleases: + description: Allow published prereleases (manual only) + default: false + type: boolean + force_regenerate: + description: Allow another paid attempt even if reserved before + default: false + type: boolean + +permissions: {} + +# Serialize the artifact lookup and reservation, including overlapping rollups. +# GitHub may replace an older pending run; recover that run with manual dispatch. +concurrency: + group: release-podcast-${{ github.repository }} + cancel-in-progress: false + +jobs: + generate: + if: >- + github.repository == 'navidrome/navidrome' && + ((github.event_name == 'release' && !github.event.release.draft && !github.event.release.prerelease && vars.RELEASE_AUDIO_ENABLED == 'true') || + (github.event_name == 'workflow_dispatch' && github.ref == format('refs/heads/{0}', github.event.repository.default_branch))) + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + actions: read + env: + AUDIO_MODE: ${{ github.event_name == 'release' && 'audio' || inputs.mode }} + AUDIO_ENABLED: ${{ vars.RELEASE_AUDIO_ENABLED }} + AUDIO_TEXT_MODEL: ${{ vars.RELEASE_AUDIO_TEXT_MODEL }} + AUDIO_TTS_MODEL: ${{ vars.RELEASE_AUDIO_TTS_MODEL }} + AUDIO_VOICE: ${{ vars.RELEASE_AUDIO_VOICE }} + steps: + # Always execute the reviewed default-branch helper, never release-note data. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6 + with: + go-version-file: go.mod + - name: Build the shared Go CLI before exposing credentials + run: go build -o "$RUNNER_TEMP/release-podcast" ./release/podcast + - name: Install MP3 inspection tools before generation + if: env.AUDIO_MODE == 'audio' && env.AUDIO_ENABLED == 'true' + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends ffmpeg + - name: Resolve sources and check limits and duplicate attempts + id: prepare + env: + GH_TOKEN: ${{ github.token }} + AUDIO_KEY_CONFIGURED: ${{ secrets.OPENAI_API_KEY != '' }} + run: '"$RUNNER_TEMP/release-podcast" --github-stage prepare' + - name: Reserve this paid attempt before contacting OpenAI + if: steps.prepare.outputs.generate == 'true' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + # Exact deterministic name supports API filtering across runs. + # Keep immutable within a run; force requires a fresh dispatch. + name: ${{ steps.prepare.outputs.reservation }} + path: release-podcast/manifest.json + if-no-files-found: error + retention-days: 90 + - name: Generate and validate the grounded script + if: steps.prepare.outputs.generate == 'true' + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + GH_TOKEN: ${{ github.token }} + run: '"$RUNNER_TEMP/release-podcast" --github-stage script' + - name: Checkpoint the validated script + if: steps.prepare.outputs.generate == 'true' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: release-podcast-script-${{ github.run_id }}-${{ github.run_attempt }} + path: | + release-podcast/transcript.txt + release-podcast/sources.json + release-podcast/evidence.json + release-podcast/manifest.json + if-no-files-found: error + retention-days: 30 + - name: Synthesize and inspect the MP3 + if: steps.prepare.outputs.generate == 'true' && env.AUDIO_MODE == 'audio' + env: + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + GH_TOKEN: ${{ github.token }} + run: '"$RUNNER_TEMP/release-podcast" --github-stage speech' + - name: Upload review artifacts + if: always() && steps.prepare.outputs.prepared == 'true' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: release-podcast-${{ github.run_id }}-${{ github.run_attempt }} + path: | + release-podcast/transcript.txt + release-podcast/release-podcast.mp3 + release-podcast/sources.json + release-podcast/evidence.json + release-podcast/manifest.json + if-no-files-found: error + retention-days: 30 + compression-level: 0 diff --git a/README.md b/README.md index e1a2955a1..4bc85e6a6 100644 --- a/README.md +++ b/README.md @@ -35,18 +35,11 @@ See instructions on the [project's website](https://www.navidrome.org/docs/insta ## Cloud Hosting -Several cloud hosting providers partner with us to offer [officially supported, cloud-hosted solutions](https://www.navidrome.org/docs/installation/managed). If you sign up with any of these providers, a share of the revenue funds the development of Navidrome at no additional cost for you. - -Run on PikaPods -
-Deploy with Zenith -
-Deploy on ElfHosted - - - - +[PikaPods](https://www.pikapods.com) has partnered with us to offer you an +[officially supported, cloud-hosted solution](https://www.navidrome.org/docs/installation/managed/#pikapods). +A share of the revenue helps fund the development of Navidrome at no additional cost for you. +[![PikaPods](https://www.pikapods.com/static/run-button.svg)](https://www.pikapods.com/pods?run=navidrome) ## Features diff --git a/adapters/gotaglib/end_to_end_test.go b/adapters/gotaglib/end_to_end_test.go index 0f9a90d94..e7dd18ac1 100644 --- a/adapters/gotaglib/end_to_end_test.go +++ b/adapters/gotaglib/end_to_end_test.go @@ -90,7 +90,7 @@ var _ = Describe("Extractor", func() { info.FileInfo = testFileInfo{FileInfo: fileInfo} metadata := metadata.New(path, info) - return new(metadata.ToMediaFile(model.Library{ID: 1}, "folderID")) + return new(metadata.ToMediaFile(1, "folderID")) } BeforeEach(func() { diff --git a/cmd/inspect.go b/cmd/inspect.go index 05f569f3e..5e88793cc 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -1,17 +1,13 @@ package cmd import ( - "context" "encoding/json" "fmt" - "path/filepath" "strings" "github.com/navidrome/navidrome/core" - "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/persistence" "github.com/pelletier/go-toml/v2" "github.com/spf13/cobra" "gopkg.in/yaml.v3" @@ -32,7 +28,7 @@ var inspectCmd = &cobra.Command{ Long: "Show file tags as seen by Navidrome", Args: cobra.MinimumNArgs(1), Run: func(cmd *cobra.Command, args []string) { - runInspector(cmd.Context(), args) + runInspector(args) }, } @@ -59,24 +55,18 @@ func prettyMarshal(v any) ([]byte, error) { return []byte(res.String()), nil } -func runInspector(ctx context.Context, args []string) { +func runInspector(args []string) { marshal := marshalers[format] if marshal == nil { log.Fatal("Invalid format", "format", format) } - libs := loadLibraries(ctx) - matcher := model.NewLibraryMatcher(libs) var out []core.InspectOutput for _, filePath := range args { if !model.IsAudioFile(filePath) { log.Warn("Not an audio file", "file", filePath) continue } - lib, ok := libraryForFile(matcher, filePath) - if !ok && len(libs) > 0 { - log.Warn("File is not in any library, using the global PID config", "file", filePath) - } - output, err := core.Inspect(filePath, lib, "") + output, err := core.Inspect(filePath, 1, "") if err != nil { log.Warn("Unable to process file", "file", filePath, "error", err) continue @@ -87,33 +77,3 @@ func runInspector(ctx context.Context, args []string) { data, _ := marshal(out) fmt.Println(string(data)) } - -// loadLibraries reads the libraries, so each file gets its library's PID config. It never creates a DB. -func loadLibraries(ctx context.Context) model.Libraries { - if dbFile, ok := existingDBFile(); !ok { - log.Warn(ctx, "No database found, using the global PID config", "path", dbFile) - return nil - } - defer db.Init(ctx)() - libs, err := persistence.New(db.Db()).Library().GetAll(ctx) - if err != nil { - log.Warn(ctx, "Could not load libraries, using the global PID config", err) - return nil - } - for i := range libs { - if absPath, err := filepath.Abs(libs[i].Path); err == nil { - libs[i].Path = absPath - } - } - return libs -} - -// libraryForFile falls back to the default library with no overrides, which uses the global PID config. -func libraryForFile(matcher *model.LibraryMatcher, filePath string) (model.Library, bool) { - if absPath, err := filepath.Abs(filePath); err == nil { - if lib, ok := matcher.FindLibrary(absPath); ok { - return lib, true - } - } - return model.Library{ID: model.DefaultLibraryID}, false -} diff --git a/cmd/inspect_test.go b/cmd/inspect_test.go deleted file mode 100644 index 728dc8770..000000000 --- a/cmd/inspect_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package cmd - -import ( - "os" - "path/filepath" - - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" - "github.com/navidrome/navidrome/model" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("inspect", func() { - Describe("libraryForFile", func() { - var matcher *model.LibraryMatcher - var root string - - BeforeEach(func() { - root = GinkgoT().TempDir() - cwd, err := os.Getwd() - Expect(err).ToNot(HaveOccurred()) - matcher = model.NewLibraryMatcher(model.Libraries{ - {ID: 1, Path: filepath.Join(root, "music")}, - {ID: 2, Path: filepath.Join(cwd, "loose"), PIDAlbum: "folder"}, - }) - }) - - It("returns the library that contains an absolute path", func() { - lib, ok := libraryForFile(matcher, filepath.Join(root, "music", "album", "track.mp3")) - Expect(ok).To(BeTrue()) - Expect(lib.ID).To(Equal(1)) - }) - - It("resolves a relative path against the working directory", func() { - lib, ok := libraryForFile(matcher, filepath.Join("loose", "track.mp3")) - Expect(ok).To(BeTrue()) - Expect(lib.PIDAlbum).To(Equal("folder")) - }) - - It("falls back to the default library without overrides", func() { - lib, ok := libraryForFile(matcher, filepath.Join(root, "elsewhere", "track.mp3")) - Expect(ok).To(BeFalse()) - Expect(lib).To(Equal(model.Library{ID: model.DefaultLibraryID})) - }) - }) - - Describe("loadLibraries", func() { - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - }) - - It("does not create a database when there is none", func() { - dbFile := filepath.Join(GinkgoT().TempDir(), "navidrome.db") - conf.Server.DbPath = dbFile + "?_journal_mode=WAL" - - Expect(loadLibraries(GinkgoT().Context())).To(BeNil()) - Expect(dbFile).ToNot(BeAnExistingFile()) - }) - }) -}) diff --git a/cmd/root.go b/cmd/root.go index e39c55365..089f09472 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -190,20 +190,16 @@ func schedulePeriodicScan(ctx context.Context) func() error { } } -// librariesWithChangedPID returns the names of the libraries whose effective PID config differs from -// the one used by their last finished scan -func librariesWithChangedPID(ctx context.Context, ds model.DataStore) ([]string, error) { - libs, err := ds.Library().GetAll(ctx) +func pidHashChanged(ds model.DataStore) (bool, error) { + pidAlbum, err := ds.Property().DefaultGet(context.Background(), consts.PIDAlbumKey, "") if err != nil { - return nil, err + return false, err } - var names []string - for _, lib := range libs { - if lib.PIDChanged() { - names = append(names, lib.Name) - } + pidTrack, err := ds.Property().DefaultGet(context.Background(), consts.PIDTrackKey, "") + if err != nil { + return false, err } - return names, nil + return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || !strings.EqualFold(pidTrack, conf.Server.PID.Track), nil } // runInitialScan runs an initial scan of the music library if needed. @@ -218,12 +214,12 @@ func runInitialScan(ctx context.Context) func() error { if err != nil { return err } - pidChangedLibs, err := librariesWithChangedPID(ctx, ds) + pidHasChanged, err := pidHashChanged(ds) if err != nil { return err } scanOnStartup := conf.Server.Scanner.Enabled && conf.Server.Scanner.ScanOnStartup - scanNeeded := scanOnStartup || inProgress || fullScanRequired == "1" || len(pidChangedLibs) > 0 + scanNeeded := scanOnStartup || inProgress || fullScanRequired == "1" || pidHasChanged time.Sleep(2 * time.Second) // Wait 2 seconds before the initial scan if scanNeeded { s := CreateScanner(ctx) @@ -231,9 +227,9 @@ func runInitialScan(ctx context.Context) func() error { case fullScanRequired == "1": log.Warn(ctx, "Full scan required after migration") _ = ds.Property().Delete(ctx, consts.FullScanAfterMigrationFlagKey) - case len(pidChangedLibs) > 0: - // Includes never-scanned libraries. The scanner rescans in full only the ones that need it - log.Warn(ctx, "Libraries with a new or changed PID config, scanning", "libraries", pidChangedLibs) + case pidHasChanged: + log.Warn(ctx, "PID config changed, performing full scan") + fullScanRequired = "1" case inProgress: log.Warn(ctx, "Resuming interrupted scan") default: diff --git a/cmd/root_test.go b/cmd/root_test.go index 423cd2a8f..af8d44e7e 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -1,7 +1,6 @@ package cmd import ( - "errors" "net/http" "net/http/httptest" "path" @@ -10,8 +9,6 @@ import ( "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -47,30 +44,3 @@ var _ = Describe("profilerHandler", func() { Entry("with a trailing-slash BasePath", "/music/"), ) }) - -var _ = Describe("librariesWithChangedPID", func() { - var ds *tests.MockDataStore - var libs *tests.MockLibraryRepo - - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - libs = &tests.MockLibraryRepo{} - ds = &tests.MockDataStore{MockedLibrary: libs} - }) - - It("returns only the libraries whose PID config changed", func() { - pid := model.Library{}.EffectivePID() - libs.SetData(model.Libraries{ - {ID: 1, Name: "Same", ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track}, - {ID: 2, Name: "Changed", PIDAlbum: "folder", ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track}, - {ID: 3, Name: "Never scanned"}, - }) - Expect(librariesWithChangedPID(GinkgoT().Context(), ds)).To(ConsistOf("Changed", "Never scanned")) - }) - - It("returns the error from the repository", func() { - libs.Err = errors.New("db down") - _, err := librariesWithChangedPID(GinkgoT().Context(), ds) - Expect(err).To(MatchError("db down")) - }) -}) diff --git a/cmd/utils.go b/cmd/utils.go index f35a31fb1..72ec67f90 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -18,16 +18,11 @@ import ( "github.com/navidrome/navidrome/persistence" ) -// existingDBFile returns the database file (DbPath minus DSN params), and whether it exists. -func existingDBFile() (string, bool) { - path, _, _ := strings.Cut(conf.Server.DbPath, "?") - _, err := os.Stat(path) - return path, err == nil -} - -// requireExistingDB aborts the command when the database file does not exist. +// requireExistingDB aborts the command when the database file (DbPath minus DSN +// params) does not exist. func requireExistingDB() { - if path, ok := existingDBFile(); !ok { + path, _, _ := strings.Cut(conf.Server.DbPath, "?") + if _, err := os.Stat(path); os.IsNotExist(err) { log.Fatal("No existing database", "path", path) } } diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 2a396689d..19f92d9d5 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -95,9 +95,8 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { artworkArtwork := artwork.NewArtwork(dataStore, fileCache, imageStore, fFmpeg) transcodingCache := stream.GetTranscodingCache() mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) - transcodeDecider := stream.NewTranscodeDecider(dataStore, fFmpeg) share := core.NewShare(dataStore) - archiver := core.NewArchiver(mediaStreamer, transcodeDecider, dataStore, share, artworkArtwork) + archiver := core.NewArchiver(mediaStreamer, dataStore, share, artworkArtwork) players := core.NewPlayers(dataStore) broker := events.GetBroker() metricsMetrics := metrics.GetPrometheusInstance(dataStore) @@ -111,6 +110,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { playTracker := scrobbler.GetPlayTracker(dataStore, broker, manager) playbackServer := playback.GetInstance(dataStore) lyricsLyrics := lyrics.NewLyrics(dataStore, manager) + transcodeDecider := stream.NewTranscodeDecider(dataStore, fFmpeg) sonicSonic := sonic.New(dataStore, manager, matcherMatcher) router := subsonic.New(dataStore, artworkArtwork, mediaStreamer, archiver, players, provider, modelScanner, broker, playlistsPlaylists, playTracker, share, playbackServer, metricsMetrics, lyricsLyrics, transcodeDecider, sonicSonic) return router @@ -159,10 +159,9 @@ func CreatePublicRouter() *public.Router { artworkArtwork := artwork.NewArtwork(dataStore, fileCache, imageStore, fFmpeg) transcodingCache := stream.GetTranscodingCache() mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) - transcodeDecider := stream.NewTranscodeDecider(dataStore, fFmpeg) share := core.NewShare(dataStore) - archiver := core.NewArchiver(mediaStreamer, transcodeDecider, dataStore, share, artworkArtwork) - router := public.New(dataStore, artworkArtwork, mediaStreamer, transcodeDecider, share, archiver) + archiver := core.NewArchiver(mediaStreamer, dataStore, share, artworkArtwork) + router := public.New(dataStore, artworkArtwork, mediaStreamer, share, archiver) return router } diff --git a/consts/consts.go b/consts/consts.go index 42e9ec42f..9bdac9125 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -156,6 +156,8 @@ const ( //DefaultAlbumPID = "album_legacy" DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate" DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title" + PIDAlbumKey = "PIDAlbum" + PIDTrackKey = "PIDTrack" ) const ( diff --git a/core/archiver.go b/core/archiver.go index 60eb44858..6f362322a 100644 --- a/core/archiver.go +++ b/core/archiver.go @@ -35,14 +35,13 @@ type Archiver interface { ZipPlaylist(ctx context.Context, id string, format string, bitrate int, w io.Writer) error } -func NewArchiver(ms stream.MediaStreamer, decider stream.TranscodeDecider, ds model.DataStore, shares Share, artwork artwork.Artwork) Archiver { - return &archiver{ds: ds, ms: ms, decider: decider, shares: shares, artwork: artwork} +func NewArchiver(ms stream.MediaStreamer, ds model.DataStore, shares Share, artwork artwork.Artwork) Archiver { + return &archiver{ds: ds, ms: ms, shares: shares, artwork: artwork} } type archiver struct { ds model.DataStore ms stream.MediaStreamer - decider stream.TranscodeDecider shares Share artwork artwork.Artwork } @@ -79,9 +78,8 @@ func (a *archiver) zipAlbums(ctx context.Context, id string, format string, bitr log.Debug(ctx, "Zipping album", "name", album[0].Album, "artist", album[0].AlbumArtist, "folder", folder, "format", format, "bitrate", bitrate, "isMultiDisc", isMultiDisc, "numTracks", len(album)) for _, mf := range album { - req := a.resolveRequest(ctx, &mf, format, bitrate) - file := a.albumFilename(mf, req.Format, isMultiDisc, folder) - if addErr := a.addFileToZip(ctx, z, mf, req, file); errors.Is(addErr, stream.ErrTooManyTranscodes) { + file := a.albumFilename(mf, format, isMultiDisc, folder) + if addErr := a.addFileToZip(ctx, z, mf, format, bitrate, file); errors.Is(addErr, stream.ErrTooManyTranscodes) { // Stop iterating: continuing would just rack up more // rejections from the limiter. Close finalises whatever // tracks were already written; the rejected one is not @@ -206,9 +204,8 @@ func (a *archiver) zipMediaFiles(ctx context.Context, id, name string, format st zippedMfs := make(model.MediaFiles, len(mfs)) for idx, mf := range mfs { - req := a.resolveRequest(ctx, &mf, format, bitrate) - file := a.playlistFilename(mf, req.Format, idx) - if addErr := a.addFileToZip(ctx, z, mf, req, file); errors.Is(addErr, stream.ErrTooManyTranscodes) { + file := a.playlistFilename(mf, format, idx) + if addErr := a.addFileToZip(ctx, z, mf, format, bitrate, file); errors.Is(addErr, stream.ErrTooManyTranscodes) { // Abort the whole archive: continuing would silently emit // empty zip entries since the headers are already written. _ = z.Close() @@ -254,14 +251,7 @@ func (a *archiver) playlistFilename(mf model.MediaFile, format string, idx int) return fmt.Sprintf("%02d - %s - %s.%s", idx+1, str.SanitizeFilename(mf.Artist), str.SanitizeFilename(mf.Title), ext) } -func (a *archiver) resolveRequest(ctx context.Context, mf *model.MediaFile, format string, bitrate int) stream.Request { - if format == "" || format == "raw" { - return stream.Request{Format: "raw"} - } - return a.decider.ResolveRequest(ctx, mf, format, bitrate, 0) -} - -func (a *archiver) addFileToZip(ctx context.Context, z *zip.Writer, mf model.MediaFile, req stream.Request, filename string) error { +func (a *archiver) addFileToZip(ctx context.Context, z *zip.Writer, mf model.MediaFile, format string, bitrate int, filename string) error { path := mf.AbsolutePath() // Open the source before writing the zip entry header so a rejection @@ -269,13 +259,13 @@ func (a *archiver) addFileToZip(ctx context.Context, z *zip.Writer, mf model.Med // archive. var r io.ReadCloser var err error - if req.Format != "raw" { - r, err = a.ms.NewStream(ctx, &mf, req) + if format != "raw" && format != "" { + r, err = a.ms.NewStream(ctx, &mf, stream.Request{Format: format, BitRate: bitrate}) } else { r, err = os.Open(path) } if err != nil { - log.Error(ctx, "Error opening file for zipping", "file", path, "format", req.Format, err) + log.Error(ctx, "Error opening file for zipping", "file", path, "format", format, err) return err } defer func() { diff --git a/core/archiver_test.go b/core/archiver_test.go index 178d1b6b9..4e00ce78c 100644 --- a/core/archiver_test.go +++ b/core/archiver_test.go @@ -26,7 +26,6 @@ var _ = Describe("Archiver", func() { var ( arch core.Archiver ms *mockMediaStreamer - dc *fakeDecider ds *mockDataStore sh *mockShare ca *mockCoverArt @@ -34,11 +33,10 @@ var _ = Describe("Archiver", func() { BeforeEach(func() { ms = &mockMediaStreamer{} - dc = &fakeDecider{} sh = &mockShare{} ds = &mockDataStore{} ca = &mockCoverArt{images: map[string][]byte{}} - arch = core.NewArchiver(ms, dc, ds, sh, ca) + arch = core.NewArchiver(ms, ds, sh, ca) }) Context("ZipAlbum", func() { @@ -68,23 +66,6 @@ var _ = Describe("Archiver", func() { Expect(zr.File[0].Name).To(Equal("Album_Promo/01 - track1.mp3")) Expect(zr.File[1].Name).To(Equal("Album_Promo/02 - track2.mp3")) }) - - It("streams the request resolved by the transcode decider and names the entry after its format", func() { - mfRepo := &mockMediaFileRepository{} - mfRepo.On("GetAll", mock.Anything).Return(model.MediaFiles{{Path: "test_data/01 - track1.flac", Suffix: "flac", AlbumID: "1"}}, nil) - ds.On("MediaFile").Return(mfRepo) - resolved := stream.Request{Format: "opus", BitRate: 128, SampleRate: 48000, Channels: 2} - dc.resolved = &resolved - ms.On("NewStream", mock.Anything, mock.Anything, resolved).Return(io.NopCloser(strings.NewReader("test")), nil).Once() - - out := new(bytes.Buffer) - Expect(arch.ZipAlbum(GinkgoT().Context(), "1", "mp3", 128, out)).To(Succeed()) - ms.AssertExpectations(GinkgoT()) - - zr, err := zip.NewReader(bytes.NewReader(out.Bytes()), int64(out.Len())) - Expect(err).ToNot(HaveOccurred()) - Expect(zr.File[0].Name).To(HaveSuffix("01 - track1.opus")) - }) }) Context("ZipArtist", func() { @@ -315,30 +296,6 @@ var _ = Describe("Archiver", func() { }) Context("ZipPlaylist", func() { - It("names the entries and the M3U lines after the resolved format", func() { - pls := &model.Playlist{ID: "1", Name: "Test Playlist", Tracks: []model.PlaylistTrack{ - {MediaFile: model.MediaFile{Path: "test_data/01 - track1.flac", Suffix: "flac", Artist: "Artist 1", Title: "track1"}}, - }} - plRepo := &mockPlaylistRepository{} - plRepo.On("GetWithTracks", "1", true, false).Return(pls, nil) - ds.On("Playlist").Return(plRepo) - dc.resolved = &stream.Request{Format: "opus", BitRate: 128} - ms.On("NewStream", mock.Anything, mock.Anything, *dc.resolved).Return(io.NopCloser(strings.NewReader("test")), nil) - - out := new(bytes.Buffer) - Expect(arch.ZipPlaylist(GinkgoT().Context(), "1", "mp3", 128, out)).To(Succeed()) - - zr, err := zip.NewReader(bytes.NewReader(out.Bytes()), int64(out.Len())) - Expect(err).ToNot(HaveOccurred()) - Expect(zr.File[0].Name).To(Equal("01 - Artist 1 - track1.opus")) - m3u, err := zr.File[1].Open() - Expect(err).ToNot(HaveOccurred()) - defer m3u.Close() - content, err := io.ReadAll(m3u) - Expect(err).ToNot(HaveOccurred()) - Expect(string(content)).To(ContainSubstring("01 - Artist 1 - track1.opus")) - }) - It("zips a playlist correctly", func() { tracks := []model.PlaylistTrack{ {MediaFile: model.MediaFile{Path: "test_data/01 - track1.mp3", Suffix: "mp3", AlbumID: "1", Album: "Album 1", DiscNumber: 1, Artist: "AC/DC", Title: "track1"}}, @@ -614,19 +571,6 @@ func (m *mockMediaStreamer) NewStream(ctx context.Context, mf *model.MediaFile, return &stream.Stream{ReadCloser: args.Get(0).(io.ReadCloser)}, nil } -// fakeDecider echoes the legacy format/bitrate unless a resolved request is set. -type fakeDecider struct { - stream.TranscodeDecider - resolved *stream.Request -} - -func (f *fakeDecider) ResolveRequest(_ context.Context, _ *model.MediaFile, format string, bitRate int, offset int) stream.Request { - if f.resolved != nil { - return *f.resolved - } - return stream.Request{Format: format, BitRate: bitRate, Offset: offset} -} - type mockShare struct { mock.Mock core.Share diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index fb07332fe..40baa2495 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -374,11 +374,8 @@ func (r *resolver) resolvePlaylist(ctx context.Context, playlistID string) (reso } } - tracks := r.ds.Playlist().Tracks(ctx, pl.ID, false) - if tracks == nil { - return resolution{}, fmt.Errorf("resolvePlaylist: could not load tracks for playlist %s", pl.ID) - } - albumIDs, err := tracks.GetAlbumIDs(ctx, model.QueryOptions{Max: PlaylistGridSamples, Sort: "random()"}) + albumIDs, err := r.ds.Playlist().Tracks(ctx, pl.ID, false). + GetAlbumIDs(ctx, model.QueryOptions{Max: PlaylistGridSamples, Sort: "random()"}) if err != nil { return resolution{}, err } diff --git a/core/artwork/resolve_test.go b/core/artwork/resolve_test.go index 2a36531bb..da144d8e2 100644 --- a/core/artwork/resolve_test.go +++ b/core/artwork/resolve_test.go @@ -707,16 +707,6 @@ var _ = Describe("resolveItem", func() { Expect(err).To(HaveOccurred()) Expect(res).To(Equal(resolution{})) }) - - It("returns an error when the playlist tracks cannot be loaded", func() { - plRepo := tests.CreateMockPlaylistRepo() - plRepo.SetData(model.Playlists{{ID: "pl4", Name: "Playlist"}}) - ds.MockedPlaylist = plRepo - - res, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "pl", ItemID: "pl4"}) - Expect(err).To(HaveOccurred()) - Expect(res).To(Equal(resolution{})) - }) }) }) diff --git a/core/artwork/worker.go b/core/artwork/worker.go index 4be99f92e..28e51958c 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -4,11 +4,9 @@ import ( "bytes" "cmp" "context" - "fmt" "io" "math" "math/rand/v2" - "runtime/debug" "sync" "time" @@ -246,7 +244,7 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc item.ImageType = cmp.Or(item.ImageType, model.ImageTypePrimary) trace := &ChainTrace{} ctx = withTrace(ctx, trace) - out, got, retryIn := w.safeAcquire(ctx, item) + out, got, retryIn := w.proc.acquire(ctx, item) queue := w.proc.ds.ArtworkQueue() switch out { @@ -288,20 +286,6 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) (outc return out, got } -// safeAcquire turns a panic into a failed attempt: the drain runs on a bare goroutine, so an -// unrecovered panic would crash the server, and the still-queued row would crash it again on restart. -func (w *Worker) safeAcquire(ctx context.Context, item model.ArtworkQueueItem) (out outcome, got *acquired, retryIn time.Duration) { - defer func() { - if r := recover(); r != nil { - log.Error(ctx, "Artwork: Panic while processing item", "kind", item.ItemKind, "id", item.ItemID, - "imageType", item.ImageType, "attempts", item.Attempts, "panic", r, "stack", string(debug.Stack())) - traceStage(ctx, "panic", fmt.Errorf("%v", r)) - out, got, retryIn = outcomeFailed, nil, 0 - } - }() - return w.proc.acquire(ctx, item) -} - // recordGiveUp keeps the last failure on the state row after the queue row is deleted. An item // that never resolved has no row to update, and creating one would settle it absent. func (w *Worker) recordGiveUp(ctx context.Context, item model.ArtworkQueueItem, trace string) { diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index 80ca68bc3..a6c07b763 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -142,18 +142,6 @@ func (v *visibilityPlaylistRepo) Get(ctx context.Context, id string) (*model.Pla return v.MockPlaylistRepo.Get(ctx, id) } -type panickingAlbumRepo struct { - *tests.MockAlbumRepo - panicID string -} - -func (r *panickingAlbumRepo) Get(ctx context.Context, id string) (*model.Album, error) { - if id == r.panicID { - panic("boom") - } - return r.MockAlbumRepo.Get(ctx, id) -} - func adminUserRepo() *tests.MockedUserRepo { repo := tests.CreateMockUserRepo() Expect(repo.Put(GinkgoT().Context(), &model.User{ID: "admin", UserName: "admin", IsAdmin: true})).To(Succeed()) @@ -290,36 +278,6 @@ var _ = Describe("Worker", func() { Expect(err).To(MatchError(model.ErrNotFound), "a timeout must never settle on absent") }) - It("fails an item that panics, without stopping the rest of the batch", func() { - folderRepo.result = []model.Folder{{ - Path: "tests/fixtures/artist/an-album", - ImageFiles: []string{"cover.jpg"}, - }} - albums := tests.CreateMockAlbumRepo() - albums.SetData(model.Albums{ - {ID: "alboom", Name: "Album", FolderIDs: []string{"f1"}}, - {ID: "alok", Name: "Album", FolderIDs: []string{"f1"}}, - }) - ds.MockedAlbum = &panickingAlbumRepo{MockAlbumRepo: albums, panicID: "alboom"} - Expect(queueRepo.Enqueue(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alboom"})).To(Succeed()) - Expect(queueRepo.Enqueue(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "alok"})).To(Succeed()) - - n, err := w.drain(ctx, 1) - Expect(err).ToNot(HaveOccurred()) - Expect(n).To(Equal(2)) - - it := findQueued(queueRepo, "al", "alboom") - Expect(it).ToNot(BeNil(), "a panicking item must be rescheduled, not dropped") - Expect(it.Attempts).To(Equal(1)) - Expect(it.RetryAt).To(BeTemporally(">", time.Now())) - Expect(it.Trace).To(ContainSubstring("boom")) - - Expect(findQueued(queueRepo, "al", "alok")).To(BeNil()) - ia, err := artRepo.GetItemArtwork(ctx, model.KindAlbumArtwork, "alok", model.ImageTypePrimary) - Expect(err).ToNot(HaveOccurred()) - Expect(ia.Source).To(Equal("folder")) - }) - It("reschedules past the provider's requested delay when it exceeds the backoff", func() { conf.Server.CoverArtPriority = "external" ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{{ID: "al9", Name: "Album"}}) diff --git a/core/inspect.go b/core/inspect.go index c60459b88..01ec33760 100644 --- a/core/inspect.go +++ b/core/inspect.go @@ -15,7 +15,7 @@ type InspectOutput struct { MappedTags *model.MediaFile `json:"mappedTags,omitempty"` } -func Inspect(filePath string, lib model.Library, folderId string) (*InspectOutput, error) { +func Inspect(filePath string, libraryId int, folderId string) (*InspectOutput, error) { path, file := filepath.Split(filePath) s, err := storage.For(path) @@ -39,22 +39,12 @@ func Inspect(filePath string, lib model.Library, folderId string) (*InspectOutpu return nil, model.ErrNotFound } - md := metadata.New(scannerPath(lib, filePath), tag) + md := metadata.New(path, tag) result := &InspectOutput{ File: filePath, RawTags: tags[file].Tags, - MappedTags: new(md.ToMediaFile(lib, folderId)), + MappedTags: new(md.ToMediaFile(libraryId, folderId)), } return result, nil } - -// scannerPath returns the path the scanner uses for the file (relative to its library), so -// folder-based PIDs match the DB. Files outside the library keep their absolute path. -func scannerPath(lib model.Library, filePath string) string { - absPath, err := filepath.Abs(filePath) - if err != nil || lib.Path == "" { - return filePath - } - return model.LibraryRelativePath(lib.Path, absPath) -} diff --git a/core/inspect_test.go b/core/inspect_test.go deleted file mode 100644 index 0ac90990c..000000000 --- a/core/inspect_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package core_test - -import ( - "path/filepath" - - "github.com/navidrome/navidrome/core" - "github.com/navidrome/navidrome/model" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Inspect", func() { - var fixtures string - - BeforeEach(func() { - var err error - fixtures, err = filepath.Abs(filepath.Join("tests", "fixtures")) - Expect(err).ToNot(HaveOccurred()) - }) - - It("maps the file with the library-relative path the scanner uses", func() { - lib := model.Library{ID: 2, Path: filepath.Dir(fixtures), PIDAlbum: "folder"} - out, err := core.Inspect(filepath.Join(fixtures, "test.mp3"), lib, "") - Expect(err).ToNot(HaveOccurred()) - Expect(out.MappedTags.Path).To(Equal("fixtures/test.mp3")) - Expect(out.MappedTags.LibraryID).To(Equal(2)) - }) - - It("gives the same IDs for relative and absolute paths", func() { - lib := model.Library{ID: 2, Path: filepath.Dir(fixtures), PIDAlbum: "folder"} - abs, err := core.Inspect(filepath.Join(fixtures, "test.mp3"), lib, "") - Expect(err).ToNot(HaveOccurred()) - rel, err := core.Inspect(filepath.Join("tests", "fixtures", "test.mp3"), lib, "") - Expect(err).ToNot(HaveOccurred()) - Expect(rel.MappedTags.AlbumID).To(Equal(abs.MappedTags.AlbumID)) - Expect(rel.MappedTags.PID).To(Equal(abs.MappedTags.PID)) - }) - - It("keeps the given path for a file outside the library", func() { - filePath := filepath.Join(fixtures, "test.mp3") - out, err := core.Inspect(filePath, model.Library{ID: model.DefaultLibraryID}, "") - Expect(err).ToNot(HaveOccurred()) - Expect(out.MappedTags.Path).To(Equal(filePath)) - }) -}) diff --git a/core/library.go b/core/library.go index f1153da26..628ee4b7b 100644 --- a/core/library.go +++ b/core/library.go @@ -2,12 +2,10 @@ package core import ( "context" - "errors" "fmt" "io/fs" "os" "path/filepath" - "slices" "strconv" "strings" "time" @@ -17,7 +15,6 @@ import ( "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/metadata" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/utils/slice" @@ -203,22 +200,23 @@ func (r *libraryRepositoryWrapper) Update(ctx context.Context, id string, entity } pathChanged := originalLib.Path != lib.Path - pidChanged := (updatesColumn(cols, "pidAlbum") && originalLib.PIDAlbum != lib.PIDAlbum) || - (updatesColumn(cols, "pidTrack") && originalLib.PIDTrack != lib.PIDTrack) err = r.LibraryRepository.Put(ctx, lib, cols...) if err != nil { return r.mapError(err) } - if pathChanged && r.watcher != nil { - if err := r.watcher.Watch(ctx, lib); err != nil { - log.Warn(ctx, "Failed to restart watcher for updated library", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path, err) + // Restart watcher and trigger scan if path was updated + if pathChanged { + if r.watcher != nil { + if err := r.watcher.Watch(ctx, lib); err != nil { + log.Warn(ctx, "Failed to restart watcher for updated library", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path, err) + } } - } - if (pathChanged || pidChanged) && r.scanner != nil { - go r.triggerScan(ctx, lib, "updated") + if r.scanner != nil { + go r.triggerScan(ctx, lib, "updated") + } } // Send library refresh event to all clients @@ -327,15 +325,6 @@ func (r *libraryRepositoryWrapper) validateLibrary(ctx context.Context, library } } - library.PIDAlbum = strings.TrimSpace(library.PIDAlbum) - library.PIDTrack = strings.TrimSpace(library.PIDTrack) - if err := metadata.ValidatePIDSpec(library.PIDAlbum, true); err != nil { - validationErrors["pidAlbum"] = err.Error() - } - if err := metadata.ValidatePIDSpec(library.PIDTrack, false); err != nil { - validationErrors["pidTrack"] = err.Error() - } - if len(validationErrors) > 0 { return &rest.ValidationError{Errors: validationErrors} } @@ -343,11 +332,6 @@ func (r *libraryRepositoryWrapper) validateLibrary(ctx context.Context, library return nil } -// updatesColumn reports whether an update with these columns writes col. No columns means all of them. -func updatesColumn(cols []string, col string) bool { - return len(cols) == 0 || slices.Contains(cols, col) -} - func (r *libraryRepositoryWrapper) validateLibraryPath(ctx context.Context, library *model.Library) error { // Validate path format if !filepath.IsAbs(library.Path) { @@ -415,27 +399,11 @@ func (s *libraryService) validateLibraryIDs(ctx context.Context, libraryIDs []in return nil } -var scanWaitInterval = time.Second - func (r *libraryRepositoryWrapper) triggerScan(ctx context.Context, lib *model.Library, action string) { - // Runs in its own goroutine and outlives the HTTP request - ctx = context.WithoutCancel(ctx) - - // A running scan loaded the libraries before this change, and would reject a new request - for { - status, err := r.scanner.Status(ctx) - if err != nil || !status.Scanning { - break - } - time.Sleep(scanWaitInterval) - } - log.Info(ctx, fmt.Sprintf("Triggering scan for %s library", action), "libraryID", lib.ID, "name", lib.Name, "path", lib.Path) start := time.Now() - warnings, err := r.scanner.ScanAll(ctx, false) // Quick scan: the scanner rescans libraries with a changed PID config in full - if errors.Is(err, model.ErrAlreadyScanning) { - log.Debug(ctx, "Scan already running, it covers this change", "libraryID", lib.ID, "name", lib.Name) - } else if err != nil { + warnings, err := r.scanner.ScanAll(ctx, false) // Quick scan for new library + if err != nil { log.Error(ctx, fmt.Sprintf("Error scanning %s library", action), "libraryID", lib.ID, "name", lib.Name, err) } else { log.Info(ctx, fmt.Sprintf("Scan completed for %s library", action), "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start)) diff --git a/core/library_test.go b/core/library_test.go index e6ebb1974..5402eac22 100644 --- a/core/library_test.go +++ b/core/library_test.go @@ -322,37 +322,6 @@ var _ = Describe("Library Service", func() { }) }) - Describe("PID validation", func() { - pidError := func(err error, field string) string { - var validationErr *rest.ValidationError - Expect(errors.As(err, &validationErr)).To(BeTrue()) - return validationErr.Errors[field] - } - - It("rejects an unknown attribute in the album PID", func() { - _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: "albmversion"}) - Expect(pidError(err, "pidAlbum")).To(ContainSubstring(`unknown attribute "albmversion"`)) - }) - - It("rejects albumid in the album PID", func() { - _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: "albumid"}) - Expect(pidError(err, "pidAlbum")).To(ContainSubstring("albumid")) - }) - - It("rejects an unknown attribute in the track PID", func() { - _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDTrack: "nosuchtag"}) - Expect(pidError(err, "pidTrack")).To(ContainSubstring(`unknown attribute "nosuchtag"`)) - }) - - It("trims spaces", func() { - library := &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: " folder ", PIDTrack: " "} - _, err := repo.Save(ctx, library) - Expect(err).ToNot(HaveOccurred()) - Expect(library.PIDAlbum).To(Equal("folder")) - Expect(library.PIDTrack).To(BeEmpty()) - }) - }) - Describe("Path Validation", func() { Context("Create operation", func() { It("fails when path is not absolute", func() { @@ -710,48 +679,6 @@ var _ = Describe("Library Service", func() { }, "100ms", "10ms").Should(Equal(0)) }) - It("triggers scan when updating the library PID config", func() { - libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir}}) - - library := model.Library{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"} - Expect(repo.Update(ctx, "1", library)).To(Succeed()) - - Eventually(func() int { - return scanner.GetScanAllCallCount() - }, "1s", "10ms").Should(Equal(1)) - // A quick scan: the scanner itself rescans this library in full - Expect(scanner.GetScanAllCalls()[0].FullScan).To(BeFalse()) - }) - - It("does not trigger scan when the PID fields were not sent", func() { - libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"}}) - - // The REST layer decodes a missing pidAlbum as "". Only the sent fields count. - library := model.Library{ID: 1, Name: "Renamed", Path: tempDir} - Expect(repo.Update(ctx, "1", library, "name", "path")).To(Succeed()) - - Consistently(func() int { - return scanner.GetScanAllCallCount() - }, "100ms", "10ms").Should(Equal(0)) - }) - - It("waits for a running scan before triggering a new one", func() { - libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir}}) - scanner.SetScanning(true) - - library := model.Library{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"} - Expect(repo.Update(ctx, "1", library)).To(Succeed()) - - Consistently(func() int { - return scanner.GetScanAllCallCount() - }, "200ms", "20ms").Should(Equal(0)) - - scanner.SetScanning(false) - Eventually(func() int { - return scanner.GetScanAllCallCount() - }, "3s", "20ms").Should(Equal(1)) - }) - It("does not trigger scan when library creation fails", func() { // Try to create library with invalid data (empty name) library := &model.Library{Path: tempDir} diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 1d2ff5df4..4a78a7f3f 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -10,7 +10,6 @@ import ( "path/filepath" "runtime" "runtime/debug" - "slices" "strings" "sync" "sync/atomic" @@ -270,13 +269,9 @@ func (c *insightsCollector) collect(ctx context.Context) []byte { if err != nil { log.Trace(ctx, "Error reading radios count", err) } - libs, err := c.ds.Library().GetAll(ctx) + data.Library.Libraries, err = c.ds.Library().CountAll(ctx) if err != nil { - log.Trace(ctx, "Error reading libraries", err) - } - data.Library.Libraries = int64(len(libs)) - if slices.ContainsFunc(libs, func(lib model.Library) bool { return lib.PIDAlbum != "" || lib.PIDTrack != "" }) { - data.Config.HasCustomPID = true + log.Trace(ctx, "Error reading libraries count", err) } data.Library.ActiveUsers, err = c.ds.User().CountAll(ctx, model.QueryOptions{ Filters: squirrel.Gt{"last_access_at": time.Now().Add(-7 * 24 * time.Hour)}, diff --git a/core/playlists/import.go b/core/playlists/import.go index b5991b095..658bd92dc 100644 --- a/core/playlists/import.go +++ b/core/playlists/import.go @@ -78,8 +78,8 @@ func (s *playlists) resolveFolder(ctx context.Context, dir string) (*model.Folde if err != nil { return nil, err } - matcher := model.NewLibraryMatcher(libs) - lib, ok := matcher.FindLibrary(dir) + matcher := newLibraryMatcher(libs) + lib, ok := matcher.findLibrary(dir) if !ok { return nil, fmt.Errorf("%w: %s", errNotInLibrary, dir) } diff --git a/core/playlists/parse_m3u.go b/core/playlists/parse_m3u.go index ab95b8850..9610e9dbb 100644 --- a/core/playlists/parse_m3u.go +++ b/core/playlists/parse_m3u.go @@ -1,11 +1,13 @@ package playlists import ( + "cmp" "context" "fmt" "io" "net/url" "path/filepath" + "slices" "strings" "time" @@ -154,9 +156,61 @@ func (r pathResolution) ToQualifiedString() (string, error) { return fmt.Sprintf("%d:%s", r.libraryID, filepath.ToSlash(relativePath)), nil } +// libraryMatcher holds sorted libraries with cleaned paths for efficient path matching. +type libraryMatcher struct { + libraries model.Libraries + cleanedPaths []string +} + +// findLibraryForPath finds which library contains the given absolute path. +// Returns library ID and path, or 0 and empty string if not found. +func (lm *libraryMatcher) findLibraryForPath(absolutePath string) (int, string) { + lib, ok := lm.findLibrary(absolutePath) + if !ok { + return 0, "" + } + return lib.ID, filepath.Clean(lib.Path) +} + +// findLibrary checks if the absolute path is under any of the library paths. +func (lm *libraryMatcher) findLibrary(absolutePath string) (model.Library, bool) { + // Check sorted libraries (longest path first) to find the best match + for i, cleanLibPath := range lm.cleanedPaths { + // Check if absolutePath is under this library path + if strings.HasPrefix(absolutePath, cleanLibPath) { + // Ensure it's a proper path boundary (not just a prefix) + if len(absolutePath) == len(cleanLibPath) || absolutePath[len(cleanLibPath)] == filepath.Separator { + return lm.libraries[i], true + } + } + } + return model.Library{}, false +} + +// newLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first). +// This ensures correct matching when library paths are prefixes of each other. +// Example: /music-classical must be checked before /music +// Otherwise, /music-classical/track.mp3 would match /music instead of /music-classical +func newLibraryMatcher(libs model.Libraries) *libraryMatcher { + // Sort libraries by path length (descending) to ensure longest paths match first. + slices.SortFunc(libs, func(i, j model.Library) int { + return cmp.Compare(len(j.Path), len(i.Path)) // Reverse order for descending + }) + + // Pre-clean all library paths once for efficient matching + cleanedPaths := make([]string, len(libs)) + for i, lib := range libs { + cleanedPaths[i] = filepath.Clean(lib.Path) + } + return &libraryMatcher{ + libraries: libs, + cleanedPaths: cleanedPaths, + } +} + // pathResolver handles path resolution logic for playlist imports. type pathResolver struct { - matcher *model.LibraryMatcher + matcher *libraryMatcher } // newPathResolver creates a pathResolver with libraries loaded from the datastore. @@ -165,7 +219,7 @@ func newPathResolver(ctx context.Context, ds model.DataStore) (*pathResolver, er if err != nil { return nil, err } - matcher := model.NewLibraryMatcher(libs) + matcher := newLibraryMatcher(libs) return &pathResolver{matcher: matcher}, nil } @@ -192,14 +246,14 @@ func (r *pathResolver) resolvePath(line string, folder *model.Folder) pathResolu // a pathResolution with the library information. Returns an invalid resolution if // the path is not found in any library. func (r *pathResolver) findInLibraries(absolutePath string) pathResolution { - lib, ok := r.matcher.FindLibrary(absolutePath) - if !ok { + libID, libPath := r.matcher.findLibraryForPath(absolutePath) + if libID == 0 { return pathResolution{valid: false} } return pathResolution{ absolutePath: absolutePath, - libraryPath: filepath.Clean(lib.Path), - libraryID: lib.ID, + libraryPath: libPath, + libraryID: libID, valid: true, } } @@ -234,7 +288,7 @@ func (r *pathResolver) resolvePaths(ctx context.Context, folder *model.Folder, l // HTTP(S) URLs are stored as-is (gated by EnableM3UExternalAlbumArt). // Local paths (file://, absolute, or relative) are resolved to an absolute path // and validated against known library boundaries via matcher. -func resolveImageURL(value string, folder *model.Folder, matcher *model.LibraryMatcher, owner model.User) string { +func resolveImageURL(value string, folder *model.Folder, matcher *libraryMatcher, owner model.User) string { value = strings.TrimSpace(value) if value == "" { return "" @@ -254,7 +308,7 @@ func resolveImageURL(value string, folder *model.Folder, matcher *model.LibraryM return "" } - lib, ok := matcher.FindLibrary(localPath) + lib, ok := matcher.findLibrary(localPath) // A playlist without a folder (API upload, or CLI import from outside all libraries) may only use the owner's libraries. if !ok || (folder == nil && !owner.HasLibraryAccess(lib.ID)) { return "" diff --git a/core/playlists/parse_m3u_test.go b/core/playlists/parse_m3u_test.go index ced6c2b16..b6a3a96f9 100644 --- a/core/playlists/parse_m3u_test.go +++ b/core/playlists/parse_m3u_test.go @@ -9,6 +9,187 @@ import ( . "github.com/onsi/gomega" ) +var _ = Describe("libraryMatcher", func() { + var ds *tests.MockDataStore + var mockLibRepo *tests.MockLibraryRepo + ctx := context.Background() + + BeforeEach(func() { + tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)") + mockLibRepo = &tests.MockLibraryRepo{} + ds = &tests.MockDataStore{ + MockedLibrary: mockLibRepo, + } + }) + + // Helper function to create a libraryMatcher from the mock datastore + createMatcher := func(ds model.DataStore) *libraryMatcher { + libs, err := ds.Library().GetAll(ctx) + Expect(err).ToNot(HaveOccurred()) + return newLibraryMatcher(libs) + } + + Describe("Longest library path matching", func() { + It("matches the longest library path when multiple libraries share a prefix", func() { + // Setup libraries with prefix conflicts + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/music"}, + {ID: 2, Path: "/music-classical"}, + {ID: 3, Path: "/music-classical/opera"}, + }) + + matcher := createMatcher(ds) + + // Test that longest path matches first and returns correct library ID + testCases := []struct { + path string + expectedLibID int + expectedLibPath string + }{ + {"/music-classical/opera/track.mp3", 3, "/music-classical/opera"}, + {"/music-classical/track.mp3", 2, "/music-classical"}, + {"/music/track.mp3", 1, "/music"}, + {"/music-classical/opera/subdir/file.mp3", 3, "/music-classical/opera"}, + } + + for _, tc := range testCases { + libID, libPath := matcher.findLibraryForPath(tc.path) + Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d, but got %d", tc.path, tc.expectedLibID, libID) + Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s, but got %s", tc.path, tc.expectedLibPath, libPath) + } + }) + + It("handles libraries with similar prefixes but different structures", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/home/user/music"}, + {ID: 2, Path: "/home/user/music-backup"}, + }) + + matcher := createMatcher(ds) + + // Test that music-backup library is matched correctly + libID, libPath := matcher.findLibraryForPath("/home/user/music-backup/track.mp3") + Expect(libID).To(Equal(2)) + Expect(libPath).To(Equal("/home/user/music-backup")) + + // Test that music library is still matched correctly + libID, libPath = matcher.findLibraryForPath("/home/user/music/track.mp3") + Expect(libID).To(Equal(1)) + Expect(libPath).To(Equal("/home/user/music")) + }) + + It("matches path that is exactly the library root", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/music"}, + {ID: 2, Path: "/music-classical"}, + }) + + matcher := createMatcher(ds) + + // Exact library path should match + libID, libPath := matcher.findLibraryForPath("/music-classical") + Expect(libID).To(Equal(2)) + Expect(libPath).To(Equal("/music-classical")) + }) + + It("handles complex nested library structures", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/media"}, + {ID: 2, Path: "/media/audio"}, + {ID: 3, Path: "/media/audio/classical"}, + {ID: 4, Path: "/media/audio/classical/baroque"}, + }) + + matcher := createMatcher(ds) + + testCases := []struct { + path string + expectedLibID int + expectedLibPath string + }{ + {"/media/audio/classical/baroque/bach/track.mp3", 4, "/media/audio/classical/baroque"}, + {"/media/audio/classical/mozart/track.mp3", 3, "/media/audio/classical"}, + {"/media/audio/rock/track.mp3", 2, "/media/audio"}, + {"/media/video/movie.mp4", 1, "/media"}, + } + + for _, tc := range testCases { + libID, libPath := matcher.findLibraryForPath(tc.path) + Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID) + Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s", tc.path, tc.expectedLibPath) + } + }) + }) + + Describe("Edge cases", func() { + It("handles empty library list", func() { + mockLibRepo.SetData([]model.Library{}) + + matcher := createMatcher(ds) + Expect(matcher).ToNot(BeNil()) + + // Should not match anything + libID, libPath := matcher.findLibraryForPath("/music/track.mp3") + Expect(libID).To(Equal(0)) + Expect(libPath).To(BeEmpty()) + }) + + It("handles single library", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/music"}, + }) + + matcher := createMatcher(ds) + + libID, libPath := matcher.findLibraryForPath("/music/track.mp3") + Expect(libID).To(Equal(1)) + Expect(libPath).To(Equal("/music")) + }) + + It("handles libraries with special characters in paths", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/music[test]"}, + {ID: 2, Path: "/music(backup)"}, + }) + + matcher := createMatcher(ds) + Expect(matcher).ToNot(BeNil()) + + // Special characters should match literally + libID, libPath := matcher.findLibraryForPath("/music[test]/track.mp3") + Expect(libID).To(Equal(1)) + Expect(libPath).To(Equal("/music[test]")) + }) + }) + + Describe("Path matching order", func() { + It("ensures longest paths match first", func() { + mockLibRepo.SetData([]model.Library{ + {ID: 1, Path: "/a"}, + {ID: 2, Path: "/ab"}, + {ID: 3, Path: "/abc"}, + }) + + matcher := createMatcher(ds) + + // Verify that longer paths match correctly (not cut off by shorter prefix) + testCases := []struct { + path string + expectedLibID int + }{ + {"/abc/file.mp3", 3}, + {"/ab/file.mp3", 2}, + {"/a/file.mp3", 1}, + } + + for _, tc := range testCases { + libID, _ := matcher.findLibraryForPath(tc.path) + Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID) + } + }) + }) +}) + var _ = Describe("pathResolver", func() { var ds *tests.MockDataStore var mockLibRepo *tests.MockLibraryRepo diff --git a/core/storage/local/local.go b/core/storage/local/local.go index e2ce00a1b..686838565 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -76,17 +76,6 @@ func (lfs *localFS) ResolveSymlink(name string) (string, error) { return filepath.EvalSymlinks(filepath.Join(lfs.root, filepath.FromSlash(name))) } -// ReadLink and Lstat implement fs.ReadLinkFS, so callers can detect symlinks without following them. -var _ fs.ReadLinkFS = (*localFS)(nil) - -func (lfs *localFS) ReadLink(name string) (string, error) { - return fs.ReadLink(lfs.FS, name) -} - -func (lfs *localFS) Lstat(name string) (fs.FileInfo, error) { - return fs.Lstat(lfs.FS, name) -} - func (lfs *localFS) ReadTags(path ...string) (map[string]metadata.Info, error) { res, err := lfs.extractor.Parse(path...) if err != nil { diff --git a/db/migrations/20260929221042_add_library_pid_columns.sql b/db/migrations/20260929221042_add_library_pid_columns.sql deleted file mode 100644 index 487512287..000000000 --- a/db/migrations/20260929221042_add_library_pid_columns.sql +++ /dev/null @@ -1,18 +0,0 @@ --- +goose Up --- +goose StatementBegin -alter table library add column pid_album varchar default '' not null; -alter table library add column pid_track varchar default '' not null; -alter table library add column scanned_pid_album varchar default '' not null; -alter table library add column scanned_pid_track varchar default '' not null; - --- Every library was scanned with the global PID config, so seed it as their scanned config. --- This way the upgrade does not trigger a full rescan. -update library set - scanned_pid_album = coalesce((select value from property where id = 'PIDAlbum'), ''), - scanned_pid_track = coalesce((select value from property where id = 'PIDTrack'), ''); - -delete from property where id in ('PIDAlbum', 'PIDTrack'); --- +goose StatementEnd - --- +goose Down -SELECT 1; diff --git a/model/library.go b/model/library.go index e80d22c89..1e33222ac 100644 --- a/model/library.go +++ b/model/library.go @@ -1,13 +1,10 @@ package model import ( - "cmp" "context" - "strings" "time" "github.com/deluan/rest" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/utils/slice" ) @@ -30,38 +27,6 @@ type Library struct { TotalSize int64 `json:"totalSize" db:"total_size"` TotalDuration float64 `json:"totalDuration" db:"total_duration"` DefaultNewUsers bool `json:"defaultNewUsers" db:"default_new_users"` - PIDAlbum string `json:"pidAlbum" db:"pid_album"` - PIDTrack string `json:"pidTrack" db:"pid_track"` - ScannedPIDAlbum string `json:"-" db:"scanned_pid_album"` - ScannedPIDTrack string `json:"-" db:"scanned_pid_track"` -} - -// PIDConfig holds the persistent ID specs used to compute track and album IDs. -type PIDConfig struct { - Track string - Album string -} - -// EffectivePID returns the PID specs in effect for this library: its own overrides, falling back to -// the global config. -func (l Library) EffectivePID() PIDConfig { - return PIDConfig{ - Track: cmp.Or(l.PIDTrack, conf.Server.PID.Track), - Album: cmp.Or(l.PIDAlbum, conf.Server.PID.Album), - } -} - -// PIDChanged reports whether the effective PID specs differ from the ones used by the last finished -// scan of this library. A library that was never scanned counts as changed. -func (l Library) PIDChanged() bool { - pid := l.EffectivePID() - return !strings.EqualFold(l.ScannedPIDAlbum, pid.Album) || !strings.EqualFold(l.ScannedPIDTrack, pid.Track) -} - -// NeedsPIDRescan reports whether the library has content imported with an old PID config, so it must be -// rescanned in full. A library that never finished a scan has nothing to regroup. -func (l Library) NeedsPIDRescan() bool { - return !l.LastScanAt.IsZero() && l.PIDChanged() } const ( @@ -94,8 +59,6 @@ type LibraryRepository interface { // TODO These methods should be moved to a core service ScanBegin(ctx context.Context, id int, fullScan bool) error ScanEnd(ctx context.Context, id int) error - // SetScannedPID records the PID specs used by the last finished scan of the library - SetScannedPID(ctx context.Context, id int, pid PIDConfig) error ScanInProgress(ctx context.Context) (bool, error) RefreshStats(ctx context.Context, id int) error } diff --git a/model/library_matcher.go b/model/library_matcher.go deleted file mode 100644 index 83af96f9f..000000000 --- a/model/library_matcher.go +++ /dev/null @@ -1,57 +0,0 @@ -package model - -import ( - "cmp" - "path/filepath" - "slices" - "strings" -) - -// LibraryMatcher finds the library that contains an absolute path. -type LibraryMatcher struct { - libraries Libraries - cleanedPaths []string -} - -// NewLibraryMatcher sorts the libraries longest path first, so /music-classical is checked before /music. -func NewLibraryMatcher(libs Libraries) *LibraryMatcher { - libs = slices.Clone(libs) - slices.SortFunc(libs, func(i, j Library) int { - return cmp.Compare(len(j.Path), len(i.Path)) - }) - cleanedPaths := make([]string, len(libs)) - for i, lib := range libs { - cleanedPaths[i] = filepath.Clean(lib.Path) - } - return &LibraryMatcher{libraries: libs, cleanedPaths: cleanedPaths} -} - -// FindLibrary returns the library whose path contains absolutePath. -func (lm *LibraryMatcher) FindLibrary(absolutePath string) (Library, bool) { - for i, libPath := range lm.cleanedPaths { - // A cleaned path only ends with a separator when it is a filesystem root - if strings.HasPrefix(absolutePath, libPath) && (len(absolutePath) == len(libPath) || - absolutePath[len(libPath)] == filepath.Separator || strings.HasSuffix(libPath, string(filepath.Separator))) { - return lm.libraries[i], true - } - } - return Library{}, false -} - -// LibraryRelativePath rebases an absolute path onto the library root, as the scanner's io/fs sees it -// (forward slashes). Relative paths, and absolute paths outside the library root, are returned unchanged. -func LibraryRelativePath(libPath, path string) string { - if !filepath.IsAbs(path) { - return path - } - // The library root may be relative (e.g. the default "./music"); it resolves against the same cwd - absLib, err := filepath.Abs(libPath) - if err != nil { - return path - } - rel, err := filepath.Rel(absLib, path) - if err != nil || !filepath.IsLocal(rel) { - return path - } - return filepath.ToSlash(rel) -} diff --git a/model/library_matcher_test.go b/model/library_matcher_test.go deleted file mode 100644 index 09e6f7e33..000000000 --- a/model/library_matcher_test.go +++ /dev/null @@ -1,91 +0,0 @@ -package model_test - -import ( - "os" - "path/filepath" - - "github.com/navidrome/navidrome/model" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("LibraryMatcher", func() { - // Paths are written Unix-style and converted, so they use the OS separator, as filepath.Abs output does - find := func(libs model.Libraries, path string) int { - for i := range libs { - libs[i].Path = filepath.FromSlash(libs[i].Path) - } - lib, ok := model.NewLibraryMatcher(libs).FindLibrary(filepath.FromSlash(path)) - if !ok { - return 0 - } - return lib.ID - } - - DescribeTable("matches the longest library path", - func(libs model.Libraries, path string, expectedID int) { - Expect(find(libs, path)).To(Equal(expectedID)) - }, - Entry("nested library", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}, {ID: 3, Path: "/music-classical/opera"}}, "/music-classical/opera/subdir/track.mp3", 3), - Entry("sibling with a shared prefix", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music-classical/track.mp3", 2), - Entry("shorter library", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music/track.mp3", 1), - Entry("exact library root", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music-classical", 2), - Entry("deeply nested libraries", model.Libraries{{ID: 1, Path: "/media"}, {ID: 2, Path: "/media/audio"}, {ID: 3, Path: "/media/audio/classical"}, {ID: 4, Path: "/media/audio/classical/baroque"}}, "/media/audio/classical/mozart/track.mp3", 3), - Entry("prefix that is not a path boundary", model.Libraries{{ID: 1, Path: "/a"}, {ID: 2, Path: "/ab"}, {ID: 3, Path: "/abc"}}, "/ab/file.mp3", 2), - Entry("special characters match literally", model.Libraries{{ID: 1, Path: "/music[test]"}, {ID: 2, Path: "/music(backup)"}}, "/music[test]/track.mp3", 1), - Entry("library path with a trailing slash", model.Libraries{{ID: 1, Path: "/music/"}}, "/music/track.mp3", 1), - Entry("library at the filesystem root", model.Libraries{{ID: 1, Path: "/"}}, "/music/track.mp3", 1), - Entry("nested library under a root library", model.Libraries{{ID: 1, Path: "/"}, {ID: 2, Path: "/music"}}, "/music/track.mp3", 2), - ) - - It("does not match a path outside every library", func() { - Expect(find(model.Libraries{{ID: 1, Path: "/music"}}, "/music-backup/track.mp3")).To(BeZero()) - }) - - It("does not match anything without libraries", func() { - Expect(find(nil, "/music/track.mp3")).To(BeZero()) - }) - - It("does not reorder the caller's libraries", func() { - libs := model.Libraries{{ID: 1, Path: "/a"}, {ID: 2, Path: "/abc"}} - model.NewLibraryMatcher(libs) - Expect(libs.IDs()).To(Equal([]int{1, 2})) - }) -}) - -var _ = Describe("LibraryRelativePath", func() { - // Paths are built with filepath so the "absolute" cases stay absolute on every OS - // (a Unix-style "/foo" is not absolute on Windows). - libRoot, _ := filepath.Abs(filepath.Join("jukebox", "collection")) - outside, _ := filepath.Abs(filepath.Join("somewhere", "else")) - - It("returns a relative path unchanged", func() { - Expect(model.LibraryRelativePath(libRoot, "_Collection")).To(Equal("_Collection")) - }) - - It("rebases an absolute target when the library root is relative", func() { - cwd, err := os.Getwd() - Expect(err).ToNot(HaveOccurred()) - Expect(model.LibraryRelativePath(filepath.Join("music", "library"), filepath.Join(cwd, "music", "library", "rock"))).To(Equal("rock")) - }) - - It("rebases an absolute path that equals the library root to '.'", func() { - Expect(model.LibraryRelativePath(libRoot, libRoot)).To(Equal(".")) - }) - - It("rebases an absolute path under the library root", func() { - Expect(model.LibraryRelativePath(libRoot, filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection")) - }) - - It("handles a trailing slash on the library path", func() { - Expect(model.LibraryRelativePath(libRoot+string(filepath.Separator), filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection")) - }) - - It("leaves an absolute path outside the library root unchanged", func() { - Expect(model.LibraryRelativePath(libRoot, outside)).To(Equal(outside)) - }) - - It("returns an empty path unchanged", func() { - Expect(model.LibraryRelativePath(libRoot, "")).To(Equal("")) - }) -}) diff --git a/model/library_test.go b/model/library_test.go deleted file mode 100644 index 4e799e13f..000000000 --- a/model/library_test.go +++ /dev/null @@ -1,73 +0,0 @@ -package model_test - -import ( - "encoding/json" - "time" - - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" - "github.com/navidrome/navidrome/model" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Library PID config", func() { - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - conf.Server.PID.Album = "global_album" - conf.Server.PID.Track = "global_track" - }) - - Describe("EffectivePID", func() { - It("falls back to the global config", func() { - Expect(model.Library{}.EffectivePID()).To(Equal(model.PIDConfig{Track: "global_track", Album: "global_album"})) - }) - It("uses the library overrides", func() { - lib := model.Library{PIDAlbum: "folder", PIDTrack: "title"} - Expect(lib.EffectivePID()).To(Equal(model.PIDConfig{Track: "title", Album: "folder"})) - }) - }) - - Describe("PIDChanged", func() { - It("is false when the scanned specs match, ignoring case", func() { - lib := model.Library{ScannedPIDAlbum: "GLOBAL_ALBUM", ScannedPIDTrack: "global_track"} - Expect(lib.PIDChanged()).To(BeFalse()) - }) - It("is true when the album override differs from the scanned spec", func() { - lib := model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track"} - Expect(lib.PIDChanged()).To(BeTrue()) - }) - It("is true when only the track spec changed", func() { - lib := model.Library{PIDTrack: "title", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track"} - Expect(lib.PIDChanged()).To(BeTrue()) - }) - It("is true when the global config changed for a library without overrides", func() { - lib := model.Library{ScannedPIDAlbum: "old_album", ScannedPIDTrack: "global_track"} - Expect(lib.PIDChanged()).To(BeTrue()) - }) - It("is true for a library that was never scanned", func() { - Expect(model.Library{}.PIDChanged()).To(BeTrue()) - }) - }) - - Describe("NeedsPIDRescan", func() { - It("is false for a library that never finished a scan", func() { - Expect(model.Library{PIDAlbum: "folder"}.NeedsPIDRescan()).To(BeFalse()) - }) - It("is true for a scanned library whose PID config changed", func() { - lib := model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track", LastScanAt: time.Now()} - Expect(lib.NeedsPIDRescan()).To(BeTrue()) - }) - It("is false for a scanned library whose PID config did not change", func() { - lib := model.Library{ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track", LastScanAt: time.Now()} - Expect(lib.NeedsPIDRescan()).To(BeFalse()) - }) - }) - - It("does not expose the scanned specs in JSON", func() { - data, err := json.Marshal(model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "secret_album", ScannedPIDTrack: "secret_track"}) - Expect(err).ToNot(HaveOccurred()) - Expect(string(data)).To(ContainSubstring(`"pidAlbum":"folder"`)) - Expect(string(data)).ToNot(ContainSubstring("secret_")) - }) -}) diff --git a/model/metadata/map_mediafile.go b/model/metadata/map_mediafile.go index 2135824d2..6d12feba9 100644 --- a/model/metadata/map_mediafile.go +++ b/model/metadata/map_mediafile.go @@ -8,14 +8,15 @@ import ( "math" "strconv" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/str" ) -func (md Metadata) ToMediaFile(lib model.Library, folderID string) model.MediaFile { +func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile { mf := model.MediaFile{ - LibraryID: lib.ID, + LibraryID: libID, FolderID: folderID, Tags: maps.Clone(md.tags), } @@ -83,9 +84,8 @@ func (md Metadata) ToMediaFile(lib model.Library, folderID string) model.MediaFi mf.AlbumArtist = md.mapDisplayAlbumArtist(mf) // Persistent IDs - pid := lib.EffectivePID() - mf.PID = md.trackPID(mf, pid) - mf.AlbumID = md.albumID(mf, pid.Album) + mf.PID = md.trackPID(mf) + mf.AlbumID = md.albumID(mf, conf.Server.PID.Album) // BFR These IDs will go away once the UI handle multiple participants. // BFR For Legacy Subsonic compatibility, we will set them in the API handlers diff --git a/model/metadata/map_mediafile_test.go b/model/metadata/map_mediafile_test.go index c19398841..baaf8fab5 100644 --- a/model/metadata/map_mediafile_test.go +++ b/model/metadata/map_mediafile_test.go @@ -30,23 +30,9 @@ var _ = Describe("ToMediaFile", func() { var toMediaFile = func(tags model.RawTags) model.MediaFile { props.Tags = tags md = metadata.New("filepath", props) - return md.ToMediaFile(model.Library{ID: 1}, "folderID") + return md.ToMediaFile(1, "folderID") } - Describe("Persistent IDs", func() { - It("uses the library PID config for the album ID and for albumid in the track spec", func() { - props.Tags = model.RawTags{"ALBUM": {"Kind of Blue"}, "TITLE": {"So What"}} - md = metadata.New("Jazz/Loose/01.mp3", props) - - byTags := md.ToMediaFile(model.Library{ID: 1, PIDAlbum: "album", PIDTrack: "albumid,title"}, "folderID") - byFolder := md.ToMediaFile(model.Library{ID: 1, PIDAlbum: "folder", PIDTrack: "albumid,title"}, "folderID") - - Expect(byFolder.AlbumID).ToNot(Equal(byTags.AlbumID)) - Expect(byFolder.AlbumID).To(Equal(md.AlbumID(byFolder, "folder"))) - Expect(byFolder.PID).ToNot(Equal(byTags.PID)) - }) - }) - Describe("Dates", func() { It("should parse properly tagged dates ", func() { mf = toMediaFile(model.RawTags{ diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go index db652fb8b..ec66e12b9 100644 --- a/model/metadata/map_participants_test.go +++ b/model/metadata/map_participants_test.go @@ -38,7 +38,7 @@ var _ = Describe("Participants", func() { var toMediaFile = func(tags model.RawTags) model.MediaFile { props.Tags = tags md = metadata.New("filepath", props) - return md.ToMediaFile(model.Library{ID: 1}, "folderID") + return md.ToMediaFile(1, "folderID") } Describe("ARTIST(S) tags", func() { diff --git a/model/metadata/metadata_test.go b/model/metadata/metadata_test.go index a1a675006..c84d93981 100644 --- a/model/metadata/metadata_test.go +++ b/model/metadata/metadata_test.go @@ -323,7 +323,7 @@ var _ = Describe("Metadata", func() { tag: {tagValue}, } md = metadata.New(filePath, props) - return md.ToMediaFile(model.Library{}, "0") + return md.ToMediaFile(0, "0") } DescribeTable("Gain", diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index b66ce824a..db315dc6b 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -2,11 +2,11 @@ package metadata import ( "cmp" - "errors" "fmt" "path/filepath" "strings" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -22,13 +22,12 @@ type hashFunc = func(...string) string // attributes. Attributes can be either tags or processed values like folder, // albumid, albumartistid, etc. For each field, it gets all its attribute values // and concatenates them, then hashes the result. If a field is empty, it is -// skipped and the function looks for the next field. albumSpec is the album PID -// spec used to resolve the `albumid` attribute. +// skipped and the function looks for the next field. // // Taking hash as a parameter (instead of closing over it in a factory) keeps // mf on the stack: closing over mf would force the whole ~1KB MediaFile to the // heap on every call. -func computePID(mf model.MediaFile, md Metadata, spec, albumSpec string, prependLibId bool, hash hashFunc) string { +func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool, hash hashFunc) string { switch spec { case "track_legacy": return legacyTrackID(mf, prependLibId) @@ -42,7 +41,7 @@ func computePID(mf model.MediaFile, md Metadata, spec, albumSpec string, prepend values := make([]string, len(attributes)) hasValue := false for i, attr := range attributes { - v := getPIDAttr(mf, md, attr, prependLibId, spec, albumSpec, hash) + v := getPIDAttr(mf, md, attr, prependLibId, spec, hash) if v != "" { hasValue = true } @@ -59,15 +58,15 @@ func computePID(mf model.MediaFile, md Metadata, spec, albumSpec string, prepend return hash(pid) } -func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec, albumSpec string, hash hashFunc) string { +func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string, hash hashFunc) string { attr = strings.TrimSpace(strings.ToLower(attr)) switch attr { case "albumid": - if spec == albumSpec { + if spec == conf.Server.PID.Album { log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec) return "" } - return computePID(mf, md, albumSpec, albumSpec, prependLibId, hash) + return computePID(mf, md, conf.Server.PID.Album, prependLibId, hash) case "folder": return filepath.Dir(mf.Path) case "albumartistid": @@ -80,50 +79,18 @@ func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, return md.String(model.TagName(attr)) } -// ValidatePIDSpec checks a PID override before it is stored; empty means "use the global config". -// Aliases resolve to empty at scan time: accepted only in track specs, because the default one uses them. -func ValidatePIDSpec(spec string, isAlbum bool) error { - switch { - case spec == "", isAlbum && spec == "album_legacy", !isAlbum && spec == "track_legacy": - return nil - } - for field := range strings.SplitSeq(spec, "|") { - for attr := range strings.SplitSeq(field, ",") { - attr = strings.TrimSpace(strings.ToLower(attr)) - switch attr { - case "": - return fmt.Errorf("empty attribute in %q", spec) - case "albumid": - if isAlbum { - return errors.New("albumid cannot be used in an album PID") - } - case "folder", "albumartistid": - default: - name, ok := model.CanonicalTagName(attr) - if !ok { - return fmt.Errorf("unknown attribute %q", attr) - } - if isAlbum && string(name) != attr { - return fmt.Errorf("use the tag name %q instead of its alias %q", name, attr) - } - } - } - } - return nil -} - -func (md Metadata) trackPID(mf model.MediaFile, pid model.PIDConfig) string { - return computePID(mf, md, pid.Track, pid.Album, true, id.NewHash) +func (md Metadata) trackPID(mf model.MediaFile) string { + return computePID(mf, md, conf.Server.PID.Track, true, id.NewHash) } func (md Metadata) albumID(mf model.MediaFile, pidConf string) string { - return computePID(mf, md, pidConf, pidConf, true, id.NewHash) + return computePID(mf, md, pidConf, true, id.NewHash) } // BFR Must be configurable? func (md Metadata) artistID(name string) string { mf := model.MediaFile{AlbumArtist: name} - return computePID(mf, md, "albumartistid", "", false, id.NewHash) + return computePID(mf, md, "albumartistid", false, id.NewHash) } func (md Metadata) mapTrackTitle() string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 9f6eaf1f4..8e38bbd42 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -3,7 +3,8 @@ package metadata import ( "strings" - "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -12,18 +13,16 @@ import ( var _ = Describe("getPID", func() { var ( - md Metadata - mf model.MediaFile - sum hashFunc - albumSpec string + md Metadata + mf model.MediaFile + sum hashFunc ) getPID := func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { - return computePID(mf, md, spec, albumSpec, prependLibId, sum) + return computePID(mf, md, spec, prependLibId, sum) } BeforeEach(func() { sum = func(s ...string) string { return "(" + strings.Join(s, ",") + ")" } - albumSpec = consts.DefaultAlbumPID }) Context("attributes are tags", func() { @@ -67,7 +66,8 @@ var _ = Describe("getPID", func() { Context("calculated attributes", func() { BeforeEach(func() { - albumSpec = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate" + DeferCleanup(configtest.SetupConfig()) + conf.Server.PID.Album = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate" }) When("field is title", func() { It("should return the pid", func() { @@ -121,8 +121,8 @@ var _ = Describe("getPID", func() { When("albumid configuration refers to albumid recursively", func() { It("should avoid infinite recursion", func() { // Reproduce the issue from #4920 - albumSpec = "albumid,album,albumversion,releasedate" - spec := albumSpec + conf.Server.PID.Album = "albumid,album,albumversion,releasedate" + spec := conf.Server.PID.Album md.tags = map[model.TagName][]string{ "album": {"Album Name"}, "albumversion": {"Version"}, @@ -205,7 +205,8 @@ var _ = Describe("getPID", func() { }) When("prependLibId is true with nested albumid", func() { It("should handle nested albumid calls correctly", func() { - albumSpec = "album" + DeferCleanup(configtest.SetupConfig()) + conf.Server.PID.Album = "album" spec := "albumid" md.tags = map[model.TagName][]string{"album": {"Test Album"}} mf.AlbumArtist = "Test Artist" @@ -305,34 +306,3 @@ var _ = Describe("getPID", func() { }) }) }) - -var _ = Describe("ValidatePIDSpec", func() { - DescribeTable("accepts valid specs", - func(spec string, isAlbum bool) { - Expect(ValidatePIDSpec(spec, isAlbum)).To(Succeed()) - }, - Entry("empty, meaning the global config", "", true), - Entry("default album spec", consts.DefaultAlbumPID, true), - Entry("default track spec, which uses tag aliases", consts.DefaultTrackPID, false), - Entry("folder", "folder", true), - Entry("album legacy", "album_legacy", true), - Entry("track legacy", "track_legacy", false), - Entry("computed attributes", "albumartistid,album|title", true), - Entry("albumid in a track spec", "albumid,title", false), - Entry("spaces and mixed case", "MusicBrainz_AlbumID | Folder", true), - ) - - DescribeTable("rejects invalid specs", - func(spec string, isAlbum bool, msg string) { - Expect(ValidatePIDSpec(spec, isAlbum)).To(MatchError(ContainSubstring(msg))) - }, - Entry("unknown tag", "albmversion", true, `unknown attribute "albmversion"`), - Entry("empty field", "album||title", true, "empty attribute"), - Entry("empty attribute", "album,,title", true, "empty attribute"), - Entry("trailing separator", "album|", true, "empty attribute"), - Entry("albumid in an album spec", "albumid,album", true, "albumid"), - Entry("tag alias in an album spec", "talb", true, `use the tag name "album" instead of its alias "talb"`), - Entry("track legacy in an album spec", "track_legacy", true, `unknown attribute "track_legacy"`), - Entry("album legacy in a track spec", "album_legacy", false, `unknown attribute "album_legacy"`), - ) -}) diff --git a/model/scanner.go b/model/scanner.go index d22c3d0d6..36c9007fb 100644 --- a/model/scanner.go +++ b/model/scanner.go @@ -2,15 +2,12 @@ package model import ( "context" - "errors" "fmt" "strconv" "strings" "time" ) -var ErrAlreadyScanning = errors.New("already scanning") - // ScanTarget represents a specific folder within a library to be scanned. // NOTE: This struct is used as a map key, so it should only contain comparable types. type ScanTarget struct { diff --git a/model/tag_mappings.go b/model/tag_mappings.go index 5a8168754..ce7d2f37b 100644 --- a/model/tag_mappings.go +++ b/model/tag_mappings.go @@ -195,28 +195,6 @@ func TagMappings() map[TagName]TagConf { return mappings } -// CanonicalTagName returns the mapped tag that name is, or is an alias of. Tags are stored under this name. -func CanonicalTagName(name string) (TagName, bool) { - tagName, ok := tagNameIndex()[TagName(name).ToLower()] - return tagName, ok -} - -// tagNameIndex maps every tag name and alias to its tag name. Names are added last, so they win over aliases -// (musicbrainz_trackid is a tag and also an alias of musicbrainz_recordingid). -var tagNameIndex = sync.OnceValue(func() map[TagName]TagName { - mappings := TagMappings() - index := make(map[TagName]TagName, len(mappings)) - for name, tag := range mappings { - for _, alias := range tag.Aliases { - index[TagName(alias)] = name - } - } - for name := range mappings { - index[name] = name - } - return index -}) - func TagRolesConf() TagConf { _, cfg := parseMappings() return cfg.Roles diff --git a/model/tag_mappings_test.go b/model/tag_mappings_test.go index 91e54e5d4..e582c3f2f 100644 --- a/model/tag_mappings_test.go +++ b/model/tag_mappings_test.go @@ -192,22 +192,3 @@ var _ = Describe("TagConf", func() { }) }) }) - -var _ = Describe("CanonicalTagName", func() { - DescribeTable("resolves tag names and aliases", - func(name string, expected TagName) { - tagName, ok := CanonicalTagName(name) - Expect(ok).To(BeTrue()) - Expect(tagName).To(Equal(expected)) - }, - Entry("tag name", "album", TagAlbum), - Entry("alias", "talb", TagAlbum), - Entry("mixed case alias", "TALB", TagAlbum), - Entry("tag name that is also an alias of another tag", "musicbrainz_trackid", TagMusicBrainzTrackID), - ) - - It("does not resolve an unknown name", func() { - _, ok := CanonicalTagName("nosuchtag") - Expect(ok).To(BeFalse()) - }) -}) diff --git a/persistence/library_repository.go b/persistence/library_repository.go index 85da65cbc..bf6b8995e 100644 --- a/persistence/library_repository.go +++ b/persistence/library_repository.go @@ -93,8 +93,6 @@ func (r *libraryRepository) Put(ctx context.Context, l *model.Library, colsToUpd "path": l.Path, "remote_path": l.RemotePath, "default_new_users": l.DefaultNewUsers, - "pid_album": l.PIDAlbum, - "pid_track": l.PIDTrack, }, colsToUpdate...) cols["updated_at"] = l.UpdatedAt sq := Update(r.tableName).SetMap(cols).Where(Eq{"id": l.ID}) @@ -178,15 +176,6 @@ func (r *libraryRepository) ScanEnd(ctx context.Context, id int) error { return err } -func (r *libraryRepository) SetScannedPID(ctx context.Context, id int, pid model.PIDConfig) error { - sq := Update(r.tableName). - Set("scanned_pid_album", pid.Album). - Set("scanned_pid_track", pid.Track). - Where(Eq{"id": id}) - _, err := r.executeSQL(ctx, sq) - return err -} - func (r *libraryRepository) ScanInProgress(ctx context.Context) (bool, error) { query := r.newSelect(ctx).Where(NotEq{"last_scan_started_at": time.Time{}}) count, err := r.count(ctx, query) diff --git a/persistence/library_repository_test.go b/persistence/library_repository_test.go index bf485a06f..0ff470861 100644 --- a/persistence/library_repository_test.go +++ b/persistence/library_repository_test.go @@ -270,38 +270,6 @@ var _ = Describe("LibraryRepository", func() { }) }) - Describe("PID config", func() { - It("stores the overrides, and Put never touches the scanned specs", func() { - lib := &model.Library{Name: "PID Library", Path: "/music/pid", PIDAlbum: "folder", PIDTrack: "title"} - Expect(repo.Put(ctx, lib)).To(Succeed()) - Expect(repo.SetScannedPID(ctx, lib.ID, model.PIDConfig{Album: "folder", Track: "title"})).To(Succeed()) - - // An update coming from the REST API has no scanned specs. It must not clear them - update := &model.Library{ID: lib.ID, Name: "PID Library", Path: "/music/pid", PIDTrack: "title"} - Expect(repo.Put(ctx, update)).To(Succeed()) - - saved, err := repo.Get(ctx, lib.ID) - Expect(err).ToNot(HaveOccurred()) - Expect(saved.PIDAlbum).To(BeEmpty()) - Expect(saved.PIDTrack).To(Equal("title")) - Expect(saved.ScannedPIDAlbum).To(Equal("folder")) - Expect(saved.ScannedPIDTrack).To(Equal("title")) - }) - - It("keeps the overrides when a partial update does not send them", func() { - lib := &model.Library{Name: "Partial", Path: "/music/partial", PIDAlbum: "folder", PIDTrack: "title"} - Expect(repo.Put(ctx, lib)).To(Succeed()) - - Expect(repo.Put(ctx, &model.Library{ID: lib.ID, Name: "Renamed"}, "name")).To(Succeed()) - - saved, err := repo.Get(ctx, lib.ID) - Expect(err).ToNot(HaveOccurred()) - Expect(saved.Name).To(Equal("Renamed")) - Expect(saved.PIDAlbum).To(Equal("folder")) - Expect(saved.PIDTrack).To(Equal("title")) - }) - }) - Describe("Delete", func() { var adminRepo model.LibraryRepository var artistRepo model.ArtistRepository diff --git a/release/podcast/README.md b/release/podcast/README.md new file mode 100644 index 000000000..f651977a6 --- /dev/null +++ b/release/podcast/README.md @@ -0,0 +1,246 @@ +# Release podcast + +A standalone Go CLI turns **published Navidrome release notes** into a grounded +English recap and an MP3. The local CLI and GitHub workflow share the same source +resolution, evidence checks, cost limits and generation stages. Outputs are +`transcript.txt`, `release-podcast.mp3`, `sources.json`, `evidence.json` and +`manifest.json`. Nothing is uploaded to release assets or social media. + +## Local use, including before merge + +From a checkout of this PR, with the Go version from `go.mod`: + +```sh +go run ./release/podcast --from 0.64.0 --to 0.64.2 \ + --dry-run --output /tmp/navidrome-podcast-validation +``` + +This resolves the inclusive range of published releases, writes exact sources +and a manifest, and makes **zero OpenAI requests**. It needs GitHub network +access; `GH_TOKEN` is optional for public notes and increases the rate limit. +It needs neither an OpenAI key nor repository variables, and does not spoof +GitHub context. To select one published release, use `--from` alone: + +```sh +go run ./release/podcast --from v0.64.2 \ + --mode validate --output /tmp/navidrome-podcast-validation +``` + +Select one release or an inclusive range of at most three releases. Versions +may omit the `v` prefix. There is no comma-separated tag-list option. +Ranges require ordered stable-version endpoints that both exist as published +releases. Listing is bounded to 1,000 release records; larger listings require +single-release selection with `--from` alone. Published prereleases require +`--include-prereleases`; select a prerelease endpoint with `--from` alone. +Drafts are discarded. +Prereleases use SemVer precedence, including numeric identifiers: `rc.2` +precedes `rc.10`, and both precede the corresponding stable release. Thus a +stable lower range bound excludes its own RCs; a stable upper bound includes +its RCs only with `--include-prereleases`. + +When you separately decide to incur API usage, make `OPENAI_API_KEY` available +in your local process environment through your own secure setup. **Never put +the key in a CLI argument, log, commit, or transcript.** A repository Actions +secret is not a local environment variable; this tool does not retrieve it. +Choose supported values for the nonsecret `TEXT_MODEL`, `TTS_MODEL` and `VOICE` +variables used in this example: + +```sh +go run ./release/podcast --from 0.64.0 --to 0.64.2 \ + --mode audio --allow-paid \ + --text-model "$TEXT_MODEL" --tts-model "$TTS_MODEL" --voice "$VOICE" \ + --output /tmp/navidrome-podcast-preview +``` + +Install `ffmpeg` (including `ffprobe`) yourself before local audio mode. Media +preflight runs before any paid request. Then listen to +`/tmp/navidrome-podcast-preview/release-podcast.mp3` and compare the transcript +and evidence with the notes. Local preview does **not** require merging the PR +or setting `RELEASE_AUDIO_ENABLED`. + +`--mode script --allow-paid --text-model MODEL` generates only the transcript +and evidence. `--dry-run` always forces validation, even if `--mode audio` or +`--allow-paid` is also present. CLI model/voice flags override `AUDIO_TEXT_MODEL`, +`AUDIO_TTS_MODEL` and `AUDIO_VOICE` environment values; there are no model/voice +defaults. No API host, repository, key, shell command or pricing override is +accepted as a CLI option. + +Use a fresh output directory for each intentional preview. Paid runs acquire +an exclusive local `.release-podcast.lock` and persist a reservation in +`.attempts/` **before** contacting OpenAI. Failed attempts also remain reserved. +`--force` permits another paid attempt and replacement of generated files; +it cannot bypass an active lock. After a killed process, review the output and +ledger before manually removing a stale lock. Validation refuses directories +containing generated script/audio files so it cannot relabel older audio. +Local deduplication is scoped to the output directory; a new directory or deleted +ledger can permit another charge. Script and audio modes have distinct keys. + +## Models and voice + +These are supported choices, **not finalized user defaults**: + +| Setting | Reviewed options | +| --- | --- | +| Text model | `gpt-6-luna` (low reasoning), `gpt-4.1-mini-2025-04-14` | +| Speech model | `gpt-4o-mini-tts-2025-12-15`, `tts-1`, `tts-1-hd` | +| Legacy voices | `alloy`, `echo`, `fable`, `onyx`, `nova`, `shimmer` | +| Mini TTS voices | Legacy voices plus `ash`, `ballad`, `coral`, `sage`, `verse`, `marin`, `cedar` | + +Onyx and cedar are candidates to audition for a male-style delivery; perceived +voice gender is subjective. Mini TTS receives fixed calm English instructions; +legacy TTS does not. Unsupported models fail before paid requests. A new model +requires a code/pricing review. Luna is an API alias whose behavior can change; +the other script model and Mini TTS use pinned snapshots. + +## GitHub Actions setup and publication + +The workflow is `.github/workflows/release-podcast.yml`, named **Release +podcast**. Automatic generation is disabled by default. The secret and variable +names already communicated during planning are retained, so no configuration +rename is required: + +1. The maintainer adds repository Actions secret `OPENAI_API_KEY` personally. + One project key covers Responses and speech when its model/endpoint access + allows both. Use a dedicated project key and usage alerts. +2. Choose repository variables `RELEASE_AUDIO_TEXT_MODEL`, + `RELEASE_AUDIO_TTS_MODEL` and `RELEASE_AUDIO_VOICE`. +3. Set `RELEASE_AUDIO_ENABLED` to literal `true` only when paid workflow runs + are authorized. Blank variables are fine for offline tests/manual validation. +4. After merge, dispatch **Release podcast** from `master`, starting with + `validate`, `from=v0.64.0` and `to=v0.64.2`. Clear `to` to select only `from`. + Models and voice come from repository + variables, not manual inputs. No named GitHub Environment is configured. + +The automatic trigger is **`release: published`**, stable releases only. It +fetches the event's exact release ID, never `/releases/latest`. Tags, edits, +drafts and prereleases do not automatically generate audio. Promotion of an +already-published prerelease may require manual dispatch; manual prereleases +need explicit opt-in. + +The existing `pipeline.yml` uses `GITHUB_TOKEN` for GoReleaser and +`release/goreleaser.yml` sets `draft: true`. Those behaviors are unchanged. +Maintainer publication of the draft through GitHub can trigger this workflow; +publication with `GITHUB_TOKEN` generally suppresses downstream release events. +Use manual dispatch after such publication, or separately review an explicit +dispatch integration with narrow Actions permissions. Do not add a broad PAT +or change draft publishing to solve this integration. + +Land the workflow before the next tag. Release events are associated with the +tagged commit; historical tags cannot be assumed to contain a new workflow. +Manual dispatch requires the workflow on the default branch and deliberately +rejects other branches. Checkout executes the reviewed default-branch helper, +with credentials not persisted. **Use the first-class local CLI to test the PR +before merge**, rather than bypassing this Actions guard. + +The workflow builds the shared Go binary and installs media tools in +credential-free steps. It exposes `OPENAI_API_KEY` only to generation steps, +grants `contents: read` and `actions: read`, and pins actions to verified SHAs. +No PR event can run the paid workflow. The offline PR test workflow has no key. + +| Mode | Maximum OpenAI requests | Outputs | +| --- | --- | --- | +| `validate` / `--dry-run` | None | Sources and manifest; modeled text estimate if configured | +| `script` | One Responses request | Transcript, evidence, sources and manifest | +| `audio` | One Responses plus one speech request | Script outputs and validated MP3 | + +## Limits, grounding and costs + +Target about two minutes: 250–280 total words and 105–145 seconds. Small releases +may be shorter; correctness takes priority over filler. There are no application +or SDK retries/repair calls. Timeouts may already be billed. HTTP requests have +a one-minute timeout, media inspection 30 seconds, and the CLI/workflow ten +minutes. Sources and the complete prompt each have separate 64 KiB caps; +oversized material fails without truncating warnings. Text output is capped at +3,000 tokens including reasoning/evidence. Narration is at most 280 words and +2,500 characters. Mini TTS additionally uses a conservative 2,000 UTF-8-byte +input ceiling including instructions, to stay below its input-token limit. + +The model has no tools and receives only public notes as untrusted evidence, +not executable instructions. No embedded links or draft advisories are fetched. +Strict JSON maps sentences to exact source excerpts and required cautions. +IDs/excerpts and migration/security coverage must match; lexical checks retain +the prototype's backup, client resync, experimental/opt-in, plugin networking, +Docker discovery, and slow-storage/32-bit scan cautions. These checks do not +prove semantic entailment or classify language perfectly. **Human factual and +listening review remains required.** Every transcript discloses the AI voice. +`store: false` does not imply zero provider retention. + +Reviewed prices, USD per million units, checked 2026-10-01: + +| Model | Input | Output | +| --- | ---: | ---: | +| GPT-6 Luna | $0.10/text token | $0.50/text token | +| GPT-4.1 mini | $0.40/text token | $1.60/text token | +| TTS-1 | $15/character | — | +| TTS-1 HD | $30/character | — | +| GPT-4o mini TTS | $0.60/text token | $12/audio token | + +Preflight rejects a **modeled allowance above $0.10**, using all serialized +prompt bytes plus 1,024 framing tokens and maximum text output. Legacy TTS uses +the character cap. Mini TTS uses a conservative **6,000 audio-token allowance** +plus input; its API offers no enforceable output-token/dollar cap, so this is +an estimate, **not a billing guarantee**. Prices, anomalous audio duration, +taxes, GitHub usage and deliberate later runs can change costs. Project budget +alerts are soft thresholds. The original approximately $0.03 example applied +to illustrative 4.1 mini + TTS-1 inputs, not every supported model combination. +Neither this PR nor an estimate authorizes a paid prototype run. + +## Actions duplicate attempts and recovery + +Jobs serialize separately from the build pipeline. Before paid requests, the +GitHub API's exact `name` filter looks up the deterministic reservation name; +unrelated repository artifacts do not enter pagination. A bounded lookup of up +to 10,000 matching reservations fails closed on errors or overflow. A reservation +artifact is uploaded first, keyed by repository, sorted release IDs and mode. +Matching attempts are skipped even after failure or +source/model edits; explicit manual `force_regenerate=true` permits another +paid attempt. Rollups and single releases are different source sets. +Use a fresh manual dispatch for forced generation. Reservation names repeat +across runs but are immutable within a run: a rerun of an already-reserved +forced attempt fails at upload before any paid request, preserving the ledger. + +Reservations last 90 days, review outputs/script checkpoints 30 days, limited +by repository policy. Deleted/expired artifacts permit another attempt, so +this is best-effort deduplication, not a permanent billing ledger. GitHub can +replace an older pending concurrency run; dispatch it manually if needed. + +Both paid stages re-fetch notes and stop if changed/withdrawn. Scripts are +checkpointed before TTS. Invalid scripts never reach speech, and error JSON +never becomes MP3. `ffprobe` checks codec/duration and `ffmpeg` decodes the full +file, restricted to MP3 and local file/pipe protocols. Duration deviations are +flagged without regeneration. Manifests record source/prompt/script/audio hashes, +the generation fingerprint, commits, config, request counts, text usage and +duration. Child media/git commands do not inherit API credentials. + +Do not force regeneration to fix delivery. If the runner/checkpoint is lost, +automatic cross-run checkpoint recovery is not implemented: review the saved +script and decide whether a new deliberate attempt is warranted. It may be +billed. Actions artifacts expire and require signed-in repository read access; +they are not permanent anonymous podcast URLs. + +## Offline verification + +```sh +go test -race -count=1 -v ./release/podcast +``` + +HTTP transports are mocked and unexpected requests fail. Public v0.64.0/.1/.2 +fixtures test omissions, unsafe versions/output, inclusive ranges, paid guards, +local locking/reservations, changed sources/checkpoints, Actions trust/config, +artifact expiry/lookup limits, budget caps, redirects and no retries. Real MP3 +decoding uses a local synthetic tone with mocked OpenAI. CI installs media +tools so that test runs rather than skips. The tool uses Go's standard library; +no Python implementation or additional Go module dependency is required. + +## References + +- [GitHub release events](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#release) +- [GITHUB_TOKEN event suppression](https://docs.github.com/en/actions/how-tos/writing-workflows/choosing-when-your-workflow-runs/triggering-a-workflow) +- [GitHub exact-name artifact filter](https://docs.github.com/en/rest/actions/artifacts#list-artifacts-for-a-repository) +- [SemVer prerelease precedence](https://semver.org/) +- [GPT-6 Luna](https://developers.openai.com/api/docs/models/gpt-6-luna) +- [GPT-4.1 mini](https://developers.openai.com/api/docs/models/gpt-4.1-mini) +- [Mini TTS](https://developers.openai.com/api/docs/models/gpt-4o-mini-tts) +- [TTS-1](https://developers.openai.com/api/docs/models/tts-1) +- [TTS-1 HD](https://developers.openai.com/api/docs/models/tts-1-hd) +- [Speech guide](https://developers.openai.com/api/docs/guides/text-to-speech) diff --git a/release/podcast/actions.go b/release/podcast/actions.go new file mode 100644 index 000000000..66f890663 --- /dev/null +++ b/release/podcast/actions.go @@ -0,0 +1,177 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/url" + "os" +) + +type event struct { + Action string `json:"action"` + Release releaseRecord `json:"release"` + Repository struct { + DefaultBranch string `json:"default_branch"` + } `json:"repository"` + Inputs struct { + From string `json:"from"` + To string `json:"to"` + Mode string `json:"mode"` + IncludePrereleases string `json:"include_prereleases"` + Force string `json:"force_regenerate"` + } `json:"inputs"` +} + +func githubEvent() (event, error) { + var ev event + if os.Getenv("GITHUB_ACTIONS") != "true" || os.Getenv("GITHUB_REPOSITORY") != repository { + return ev, errors.New("Actions entrypoint requires the navidrome repository context") + } + data, err := os.ReadFile(os.Getenv("GITHUB_EVENT_PATH")) // #nosec G703 -- event path is provided by the verified GitHub runner context, never release data. + if err != nil || len(data) > 8<<20 || json.Unmarshal(data, &ev) != nil { + return ev, errors.New("invalid Actions event") + } + switch os.Getenv("GITHUB_EVENT_NAME") { + case "workflow_dispatch": + if ev.Repository.DefaultBranch == "" || os.Getenv("GITHUB_REF") != "refs/heads/"+ev.Repository.DefaultBranch { + return ev, errors.New("manual Actions generation must use the default branch") + } + case "release": + if ev.Action != "published" { + return ev, errors.New("expected a release published event") + } + if _, err := normalizeRelease(ev.Release, false); err != nil { + return ev, err + } + default: + return ev, errors.New("unsupported Actions event") + } + return ev, nil +} + +func (e *engine) duplicateArtifact(ctx context.Context, reservation string) (bool, error) { + for page := 1; page <= 100; page++ { + var response struct { + Artifacts []struct { + Name string `json:"name"` + Expired bool `json:"expired"` + } `json:"artifacts"` + } + if err := e.github(ctx, fmt.Sprintf("/actions/artifacts?name=%s&per_page=100&page=%d", url.QueryEscape(reservation), page), &response); err != nil { + return false, err + } + for _, a := range response.Artifacts { + if !a.Expired && a.Name == reservation { + return true, nil + } + } + if len(response.Artifacts) < 100 { + return false, nil + } + } + return false, errors.New("matching reservation ledger exceeds lookup limit; manual review required") +} + +func actionsOutput(key, value string) error { + f, err := os.OpenFile(os.Getenv("GITHUB_OUTPUT"), os.O_APPEND|os.O_WRONLY, 0600) // #nosec G703 -- append only to the runner-provided Actions output file. + if err != nil { + return errors.New("Actions output file unavailable") + } + defer f.Close() + if _, err := fmt.Fprintf(f, "%s=%s\n", key, value); err != nil { + return errors.New("cannot write Actions output") + } + return nil +} + +func (e *engine) runGitHub(ctx context.Context, stage string) error { + ev, err := githubEvent() + if err != nil { + return err + } + switch stage { + case "script": + return e.generateScript(ctx) + case "speech": + return e.generateSpeech(ctx) + case "prepare": + return e.prepareGitHub(ctx, ev) + default: + return errors.New("invalid Actions stage") + } +} + +func (e *engine) prepareGitHub(ctx context.Context, ev event) error { + manual := os.Getenv("GITHUB_EVENT_NAME") == "workflow_dispatch" + mode := "audio" + allow, force := false, false + if manual { + mode = ev.Inputs.Mode + if mode == "" { + mode = "validate" + } + allow = ev.Inputs.IncludePrereleases == "true" + force = ev.Inputs.Force == "true" + } + if mode != "validate" && mode != "script" && mode != "audio" { + return errors.New("invalid manual mode") + } + if mode != "validate" && (os.Getenv("AUDIO_ENABLED") != "true" || os.Getenv("AUDIO_KEY_CONFIGURED") != "true") { + return errors.New("paid Actions modes require explicit enablement and OPENAI_API_KEY in Secrets") + } + c, err := reviewedConfig(os.Getenv("AUDIO_TEXT_MODEL"), os.Getenv("AUDIO_TTS_MODEL"), os.Getenv("AUDIO_VOICE"), mode) + if err != nil { + return err + } + if mode == "audio" { + if err := e.mediaTools(ctx); err != nil { + return err + } + } + var sources []source + if manual { + sources, err = e.resolveLocal(ctx, options{from: ev.Inputs.From, to: ev.Inputs.To, includePrereleases: allow}) + } else { + var record releaseRecord + err = e.github(ctx, fmt.Sprintf("/releases/%d", ev.Release.ID), &record) + if err == nil { + var s source + s, err = normalizeRelease(record, false) + if err == nil && (s.ID != ev.Release.ID || s.Tag != ev.Release.Tag) { + err = errors.New("release identity changed") + } + sources = []source{s} + } + } + if err != nil { + return err + } + m, err := e.prepare(ctx, sources, c, mode, "github", allow, force) + if err != nil { + return err + } + m.RunID = os.Getenv("GITHUB_RUN_ID") + m.RunAttempt = os.Getenv("GITHUB_RUN_ATTEMPT") + m.EventSHA = os.Getenv("GITHUB_SHA") + if mode != "validate" { + repeated, err := e.duplicateArtifact(ctx, m.Reservation) + if err != nil { + return err + } + if repeated && !force { + m.Status = "duplicate" + } + } + if err := e.saveSources(sources, m); err != nil { + return err + } + if err := actionsOutput("reservation", m.Reservation); err != nil { + return err + } + if err := actionsOutput("prepared", "true"); err != nil { + return err + } + return actionsOutput("generate", fmt.Sprint(mode != "validate" && m.Status != "duplicate")) +} diff --git a/release/podcast/main.go b/release/podcast/main.go new file mode 100644 index 000000000..942e88d73 --- /dev/null +++ b/release/podcast/main.go @@ -0,0 +1,181 @@ +// release-podcast is a standalone, artifact-only release narration tool. +package main + +import ( + "context" + "embed" + "errors" + "flag" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +//go:embed prompt.txt +var promptFiles embed.FS + +const repository = "navidrome/navidrome" + +type options struct { + from, to, mode, textModel, ttsModel, voice, output, githubStage string + allowPaid, dryRun, includePrereleases, force bool +} + +func parseOptions(args []string, stderr io.Writer) (options, error) { + var o options + f := flag.NewFlagSet("release-podcast", flag.ContinueOnError) + f.SetOutput(stderr) + f.StringVar(&o.from, "from", "", "Published version to select, or inclusive first version of a range (v prefix optional)") + f.StringVar(&o.to, "to", "", "Optional inclusive last published version of a range") + f.StringVar(&o.mode, "mode", "validate", "validate (free), script, or audio") + f.StringVar(&o.textModel, "text-model", os.Getenv("AUDIO_TEXT_MODEL"), "Reviewed script model; no default") + f.StringVar(&o.ttsModel, "tts-model", os.Getenv("AUDIO_TTS_MODEL"), "Reviewed speech model; no default") + f.StringVar(&o.voice, "voice", os.Getenv("AUDIO_VOICE"), "Compatible stock voice; no default") + f.StringVar(&o.output, "output", "release-podcast", "Output directory (sources, manifest, script and MP3)") + f.BoolVar(&o.dryRun, "dry-run", false, "Force validate mode; never contact OpenAI") + f.BoolVar(&o.allowPaid, "allow-paid", false, "Explicitly permit bounded OpenAI requests for this local invocation") + f.BoolVar(&o.includePrereleases, "include-prereleases", false, "Include published prereleases") + f.BoolVar(&o.force, "force", false, "Permit another paid attempt and replacement of generated files") + f.StringVar(&o.githubStage, "github-stage", "", "Actions entrypoint: prepare, script, or speech") + if err := f.Parse(args); err != nil { + return o, err + } + if f.NArg() != 0 { + return o, errors.New("unexpected positional arguments") + } + if o.dryRun { + o.mode = "validate" + } + if o.mode != "validate" && o.mode != "script" && o.mode != "audio" { + return o, errors.New("mode must be validate, script, or audio") + } + if o.githubStage != "" && (o.from != "" || o.to != "" || o.allowPaid || o.dryRun || o.force || o.includePrereleases) { + return o, errors.New("Actions stages use trusted event/environment configuration, not local selection flags") + } + return o, nil +} + +type engine struct { + client *http.Client + githubToken, openAIKey, output, prompt string + command func(context.Context, string, ...string) ([]byte, error) +} + +func newEngine(output string) (*engine, error) { + abs, err := filepath.Abs(output) + if err != nil { + return nil, errors.New("invalid output directory") + } + prompt, err := promptFiles.ReadFile("prompt.txt") + if err != nil { + return nil, errors.New("embedded prompt unavailable") + } + return &engine{output: abs, prompt: string(prompt), githubToken: os.Getenv("GH_TOKEN"), openAIKey: os.Getenv("OPENAI_API_KEY"), + client: &http.Client{Timeout: time.Minute, CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }}, + command: func(ctx context.Context, name string, args ...string) ([]byte, error) { + if name != "git" && name != "ffprobe" && name != "ffmpeg" { + return nil, errors.New("unsupported local tool") + } + cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 -- fixed tool allowlist; arguments are data, never shell source. + for _, item := range os.Environ() { + if !strings.HasPrefix(item, "OPENAI_API_KEY=") && !strings.HasPrefix(item, "GH_TOKEN=") { + cmd.Env = append(cmd.Env, item) + } + } + return cmd.Output() + }, + }, nil +} + +func run(ctx context.Context, args []string, stdout, stderr io.Writer) error { + o, err := parseOptions(args, stderr) + if err != nil { + return err + } + e, err := newEngine(o.output) + if err != nil { + return err + } + if o.githubStage != "" { + return e.runGitHub(ctx, o.githubStage) + } + if os.Getenv("GITHUB_ACTIONS") == "true" { + return errors.New("Actions must use the guarded github-stage entrypoint") + } + return e.runLocal(ctx, o, stdout) +} + +func (e *engine) runLocal(ctx context.Context, o options, stdout io.Writer) error { + if o.mode != "validate" && (!o.allowPaid || e.openAIKey == "") { + return errors.New("paid local modes require --allow-paid and OPENAI_API_KEY in the environment") + } + cfg, err := reviewedConfig(o.textModel, o.ttsModel, o.voice, o.mode) + if err != nil { + return err + } + if o.mode == "audio" { + if err := e.mediaTools(ctx); err != nil { + return err + } + } + sources, err := e.resolveLocal(ctx, o) + if err != nil { + return err + } + m, err := e.prepare(ctx, sources, cfg, o.mode, "local", o.includePrereleases, o.force) + if err != nil { + return err + } + if o.mode == "validate" { + if err := e.checkGeneratedFiles(); err != nil { + return err + } + if err := e.saveSources(sources, m); err != nil { + return err + } + _, err = fmt.Fprintln(stdout, "Validated published sources; OpenAI requests: 0. Outputs:", e.output) + return err + } + releaseLock, err := e.reserveLocal(m, o.force) + if err != nil { + return err + } + defer releaseLock() + if o.force { + for _, name := range []string{"transcript.txt", "release-podcast.mp3", "evidence.json"} { + if err := os.Remove(filepath.Join(e.output, name)); err != nil && !errors.Is(err, os.ErrNotExist) { + return errors.New("cannot replace previous generated outputs") + } + } + } + if err := e.saveSources(sources, m); err != nil { + return err + } + if err := e.generateScript(ctx); err != nil { + return err + } + if o.mode == "audio" { + if err := e.generateSpeech(ctx); err != nil { + return err + } + } + _, err = fmt.Fprintln(stdout, "Generated review outputs:", e.output) + return err +} + +func main() { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + if err := run(ctx, os.Args[1:], os.Stdout, os.Stderr); err != nil { + if errors.Is(err, flag.ErrHelp) { + return + } + fmt.Fprintln(os.Stderr, "Release podcast:", err) + os.Exit(1) + } +} diff --git a/release/podcast/pipeline.go b/release/podcast/pipeline.go new file mode 100644 index 000000000..f77b5f9a4 --- /dev/null +++ b/release/podcast/pipeline.go @@ -0,0 +1,471 @@ +package main + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "math" + "os" + "path/filepath" + "slices" + "sort" + "strconv" + "strings" + "time" +) + +const ( + maxPromptBytes = 65536 + maxOutputTokens = 3000 + maxScriptChars = 2500 + miniInputBytes = 2000 + modeledAudioTokens = 6000 + maxCostUSD = 0.10 + speechInstructions = "Speak in clear, calm English with a warm, measured delivery. Read API as letters." + intro = "Welcome to the Navidrome release recap. This narration uses an AI-generated voice." + closing = "For the full details and upgrade guidance, read the official release notes linked alongside this transcript." +) + +type modelConfig struct { + TextModel string `json:"text_model"` + TTSModel string `json:"tts_model"` + Voice string `json:"voice"` + Speed float64 `json:"speed"` + Instructions string `json:"speech_instructions"` +} + +var textRates = map[string][2]float64{"gpt-6-luna": {0.10, 0.50}, "gpt-4.1-mini-2025-04-14": {0.40, 1.60}} +var speechRates = map[string]float64{"tts-1": 15, "tts-1-hd": 30, "gpt-4o-mini-tts-2025-12-15": 0} + +func reviewedConfig(text, tts, voice, mode string) (modelConfig, error) { + c := modelConfig{TextModel: text, TTSModel: tts, Voice: voice, Speed: 1} + if _, ok := textRates[text]; !ok && (mode != "validate" || text != "") { + return c, errors.New("select a reviewed text model; no default is selected") + } + if _, ok := speechRates[tts]; !ok && (mode == "audio" || tts != "") { + return c, errors.New("select a reviewed speech model") + } + voices := ",alloy,echo,fable,onyx,nova,shimmer," + if tts == "gpt-4o-mini-tts-2025-12-15" { + voices += "ash,ballad,coral,sage,verse,marin,cedar," + c.Instructions = speechInstructions + } + if (voice != "" || mode == "audio") && (voice == "" || !slices.Contains(strings.Split(strings.Trim(voices, ","), ","), voice)) { + return c, errors.New("select a compatible reviewed stock voice") + } + return c, nil +} + +func narrationByteLimit(c modelConfig) int { + if c.Instructions != "" { + return miniInputBytes - len(c.Instructions) + } + return maxScriptChars * 4 // Character-priced models; Unicode still checked separately. +} + +type manifest struct { + Version int `json:"version"` + Mode string `json:"mode"` + Origin string `json:"origin"` + Config modelConfig `json:"config"` + Cost *float64 `json:"modeled_cost_usd"` + SourceHash string `json:"source_sha256"` + PromptHash string `json:"prompt_sha256"` + GenerationHash string `json:"generation_sha256"` + ImplementationSHA string `json:"implementation_sha"` + EventSHA string `json:"event_sha,omitempty"` + RunID string `json:"run_id,omitempty"` + RunAttempt string `json:"run_attempt,omitempty"` + CreatedAt string `json:"created_at"` + Reservation string `json:"reservation"` + TextRequests int `json:"text_requests"` + SpeechRequests int `json:"speech_requests"` + IncludePrereleases bool `json:"include_prereleases"` + Force bool `json:"force_regenerate"` + Status string `json:"status"` + ScriptHash string `json:"script_sha256,omitempty"` + AudioHash string `json:"audio_sha256,omitempty"` + Words int `json:"word_count,omitempty"` + Characters int `json:"character_count,omitempty"` + Duration float64 `json:"duration_seconds,omitempty"` + DurationNeedsReview bool `json:"duration_needs_review,omitempty"` + TextUsage json.RawMessage `json:"text_usage,omitempty"` +} + +func hashBytes(data []byte) string { h := sha256.Sum256(data); return hex.EncodeToString(h[:]) } +func hashJSON(value any) string { data, _ := json.Marshal(value); return hashBytes(data) } + +func (e *engine) textPayload(sources []source, c modelConfig) map[string]any { + input, _ := json.Marshal(map[string]any{"sources": promptSources(sources), "required_cautions": cautions(sources), + "introduction": intro, "closing": closing, "narration_limits": map[string]int{"max_words": 280, "max_characters": maxScriptChars, "max_utf8_bytes": narrationByteLimit(c)}}) + p := map[string]any{"model": c.TextModel, "store": false, "max_output_tokens": maxOutputTokens, "instructions": e.prompt, "input": string(input), + "text": map[string]any{"format": map[string]any{"type": "json_schema", "name": "release_narration", "strict": true, "schema": narrationSchema()}}} + if c.TextModel == "gpt-6-luna" { + p["reasoning"] = map[string]string{"effort": "low"} + } + return p +} + +func modeledCost(payload any, c modelConfig, mode string) (*float64, error) { + data, err := json.Marshal(payload) + if err != nil { + return nil, errors.New("cannot encode prompt") + } + if len(data) > maxPromptBytes { + return nil, errors.New("complete prompt exceeds byte limit; never truncate warnings") + } + rate, ok := textRates[c.TextModel] + if !ok { + return nil, nil + } + // Conservative byte-level input-token count, plus protocol framing. + cost := (float64(len(data)+1024)*rate[0] + maxOutputTokens*rate[1]) / 1e6 + if mode == "audio" { + if c.Instructions != "" { + cost += (float64(miniInputBytes)*0.60 + modeledAudioTokens*12) / 1e6 + } else { + cost += maxScriptChars * speechRates[c.TTSModel] / 1e6 + } + } + if cost > maxCostUSD { + return nil, errors.New("modeled cost exceeds $0.10; review source size or models") + } + return &cost, nil +} + +func (e *engine) prepare(ctx context.Context, sources []source, c modelConfig, mode, origin string, allow, force bool) (manifest, error) { + var m manifest + seen := map[int64]bool{} + total := 0 + ids := []int64{} + if len(sources) < 1 || len(sources) > 3 { + return m, errors.New("select one to three releases") + } + for _, s := range sources { + if seen[s.ID] { + return m, errors.New("duplicate release IDs") + } + seen[s.ID] = true + ids = append(ids, s.ID) + total += len(s.Body) + } + if total > maxSourceBytes { + return m, errors.New("combined notes exceed byte limit") + } + cost, err := modeledCost(e.textPayload(sources, c), c, mode) + if err != nil { + return m, err + } + sha, err := e.command(ctx, "git", "rev-parse", "HEAD") + if err != nil { + sha = []byte("unknown") + } + sort.Slice(ids, func(i, j int) bool { return ids[i] < ids[j] }) + m = manifest{Version: 2, Mode: mode, Origin: origin, Config: c, Cost: cost, SourceHash: hashJSON(sources), PromptHash: hashBytes([]byte(e.prompt)), + ImplementationSHA: strings.TrimSpace(string(sha)), CreatedAt: time.Now().UTC().Format(time.RFC3339), IncludePrereleases: allow, Force: force, Status: "prepared", + Reservation: "release-podcast-attempt-" + hashJSON(map[string]any{"repo": repository, "ids": ids, "mode": mode})} + m.GenerationHash = hashJSON(map[string]any{"sources": sources, "config": c, "prompt": e.prompt, "implementation": m.ImplementationSHA}) + return m, nil +} + +func (e *engine) writeFile(name string, data []byte) error { + if err := os.MkdirAll(e.output, 0750); err != nil { + return errors.New("cannot create output directory") + } + f, err := os.CreateTemp(e.output, ".release-podcast-*") + if err != nil { + return errors.New("cannot create output file") + } + defer os.Remove(f.Name()) + if _, err := f.Write(data); err != nil { + f.Close() + return errors.New("cannot write output") + } + if err := f.Close(); err != nil { + return errors.New("cannot close output") + } + if err := os.Rename(f.Name(), filepath.Join(e.output, name)); err != nil { + return errors.New("cannot install output") + } + return nil +} + +func (e *engine) writeJSON(name string, value any) error { + data, err := json.MarshalIndent(value, "", " ") + if err != nil { + return errors.New("cannot encode output") + } + return e.writeFile(name, append(data, '\n')) +} + +func (e *engine) readJSON(name string, target any) error { + data, err := os.ReadFile(filepath.Join(e.output, name)) + if err != nil { + return errors.New("required output checkpoint is unavailable") + } + if len(data) > 8<<20 || json.Unmarshal(data, target) != nil { + return errors.New("invalid output checkpoint") + } + return nil +} + +func (e *engine) saveSources(sources []source, m manifest) error { + if err := e.writeJSON("sources.json", sources); err != nil { + return err + } + return e.writeJSON("manifest.json", m) +} + +func (e *engine) reserveLocal(m manifest, force bool) (func(), error) { + if err := os.MkdirAll(e.output, 0750); err != nil { + return nil, errors.New("cannot create output directory") + } + lockPath := filepath.Join(e.output, ".release-podcast.lock") + lock, err := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) + if err != nil { + return nil, errors.New("output directory is locked by another attempt; review a stale lock before removal") + } + lock.Close() + release := func() { _ = os.Remove(lockPath) } + fail := func(err error) (func(), error) { release(); return nil, err } + ledger := filepath.Join(e.output, ".attempts") + if err := os.MkdirAll(ledger, 0750); err != nil { + return fail(errors.New("cannot create local attempt ledger")) + } + attempt := filepath.Join(ledger, m.Reservation+".json") + if !force { + if err := e.checkGeneratedFiles(); err != nil { + return fail(err) + } + } + flags := os.O_CREATE | os.O_EXCL | os.O_WRONLY + if force { + flags = os.O_CREATE | os.O_TRUNC | os.O_WRONLY + } + f, err := os.OpenFile(attempt, flags, 0600) + if err != nil { + return fail(errors.New("paid attempt already reserved or ledger unavailable; explicit --force permits another charge")) + } + data, _ := json.Marshal(m) + _, writeErr := f.Write(data) + closeErr := f.Close() + if writeErr != nil || closeErr != nil { + return fail(errors.New("cannot persist attempt reservation; no paid request made")) + } + return release, nil +} + +func (e *engine) checkGeneratedFiles() error { + for _, name := range []string{"transcript.txt", "release-podcast.mp3", "evidence.json"} { + _, err := os.Lstat(filepath.Join(e.output, name)) + if err == nil { + return errors.New("generated files already exist; use a fresh output directory or explicit --force in a paid mode") + } + if !errors.Is(err, os.ErrNotExist) { + return errors.New("cannot inspect existing output files") + } + } + return nil +} + +func (e *engine) stage(ctx context.Context, kind string) (manifest, []source, error) { + var m manifest + var sources []source + if err := e.readJSON("manifest.json", &m); err != nil { + return m, nil, err + } + if err := e.readJSON("sources.json", &sources); err != nil { + return m, nil, err + } + if e.openAIKey == "" || m.Mode == "validate" { + return m, nil, errors.New("paid stage is not authorized or key is missing") + } + if (m.Origin != "local" && m.Origin != "github") || (os.Getenv("GITHUB_ACTIONS") == "true") != (m.Origin == "github") { + return m, nil, errors.New("checkpoint origin does not match the execution context") + } + if m.Origin == "github" { + if _, err := githubEvent(); err != nil { + return m, nil, err + } + if os.Getenv("AUDIO_ENABLED") != "true" { + return m, nil, errors.New("Actions paid generation is disabled") + } + if m.RunID != os.Getenv("GITHUB_RUN_ID") || m.RunAttempt != os.Getenv("GITHUB_RUN_ATTEMPT") { + return m, nil, errors.New("checkpoint belongs to another Actions attempt") + } + c, err := reviewedConfig(os.Getenv("AUDIO_TEXT_MODEL"), os.Getenv("AUDIO_TTS_MODEL"), os.Getenv("AUDIO_VOICE"), m.Mode) + if err != nil || c != m.Config { + return m, nil, errors.New("configuration changed after preparation") + } + } + if (kind == "text" && (m.TextRequests != 0 || m.Status != "prepared")) || (kind == "speech" && (m.SpeechRequests != 0 || m.Status != "script_validated" || m.Mode != "audio")) { + return m, nil, errors.New("stage already attempted or checkpoint not ready") + } + if hashJSON(sources) != m.SourceHash { + return m, nil, errors.New("source checkpoint changed") + } + if err := e.recheck(ctx, m, sources); err != nil { + return m, nil, err + } + return m, sources, nil +} + +func (e *engine) generateScript(ctx context.Context) error { + m, sources, err := e.stage(ctx, "text") + if err != nil { + return err + } + payload := e.textPayload(sources, m.Config) + if _, err := modeledCost(payload, m.Config, m.Mode); err != nil { + return err + } + m.TextRequests = 1 + m.Status = "script_requested" + if err := e.writeJSON("manifest.json", m); err != nil { + return err + } + data, ct, err := e.request(ctx, "https://api.openai.com/v1/responses", e.openAIKey, payload, 1<<20) + if err != nil { + return err + } + if ct != "application/json" { + return errors.New("text API returned an unexpected content type") + } + result, usage, err := parseResponse(data) + if err != nil { + return err + } + text, err := validateNarration(result, sources) + if err != nil { + return err + } + if len(strings.TrimSuffix(text, "\n")) > narrationByteLimit(m.Config) { + return errors.New("narration exceeds configured speech input limit; shorten optional highlights") + } + if err := e.writeFile("transcript.txt", []byte(text)); err != nil { + return err + } + if err := e.writeJSON("evidence.json", result); err != nil { + return err + } + m.Status = "script_validated" + m.ScriptHash = hashBytes([]byte(text)) + m.Words = len(strings.Fields(text)) + m.Characters = len([]rune(strings.TrimSuffix(text, "\n"))) + m.TextUsage = usage + return e.writeJSON("manifest.json", m) +} + +func (e *engine) mediaTools(ctx context.Context) error { + for _, tool := range []string{"ffprobe", "ffmpeg"} { + toolCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + _, err := e.command(toolCtx, tool, "-version") + cancel() + if err != nil { + return fmt.Errorf("%s is required before paid audio generation", tool) + } + } + return nil +} + +func (e *engine) generateSpeech(ctx context.Context) error { + m, sources, err := e.stage(ctx, "speech") + if err != nil { + return err + } + text, err := os.ReadFile(filepath.Join(e.output, "transcript.txt")) + if err != nil { + return errors.New("script checkpoint unavailable") + } + var result narration + if err := e.readJSON("evidence.json", &result); err != nil { + return err + } + validated, err := validateNarration(result, sources) + if err != nil || validated != string(text) || hashBytes(text) != m.ScriptHash { + return errors.New("script checkpoint changed or is invalid") + } + if err := e.mediaTools(ctx); err != nil { + return err + } + input := strings.TrimSuffix(string(text), "\n") + payload := map[string]any{"model": m.Config.TTSModel, "voice": m.Config.Voice, "input": input, "response_format": "mp3", "speed": m.Config.Speed} + if m.Config.Instructions != "" { + if len(input)+len(m.Config.Instructions) > miniInputBytes { + return errors.New("mini TTS conservative input-token limit exceeded") + } + payload["instructions"] = m.Config.Instructions + } + m.SpeechRequests = 1 + m.Status = "speech_requested" + if err := e.writeJSON("manifest.json", m); err != nil { + return err + } + data, ct, err := e.request(ctx, "https://api.openai.com/v1/audio/speech", e.openAIKey, payload, 10<<20) + if err != nil { + return err + } + if (ct != "audio/mpeg" && ct != "audio/mp3" && ct != "application/octet-stream") || len(data) == 0 { + return errors.New("speech API did not return audio") + } + if err := e.writeFile("audio.tmp", data); err != nil { + return err + } + temporary := filepath.Join(e.output, "audio.tmp") + defer os.Remove(temporary) + mediaCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + probe, err := e.command(mediaCtx, "ffprobe", "-v", "error", "-f", "mp3", "-protocol_whitelist", "file,pipe", "-show_entries", "format=duration:stream=codec_name", "-of", "json", temporary) + if err != nil { + return errors.New("speech response cannot be inspected as MP3") + } + var info struct { + Format struct { + Duration string `json:"duration"` + } `json:"format"` + Streams []struct { + Codec string `json:"codec_name"` + } `json:"streams"` + } + if json.Unmarshal(probe, &info) != nil || len(info.Streams) == 0 { + return errors.New("invalid MP3 metadata") + } + duration, err := strconv.ParseFloat(info.Format.Duration, 64) + if err != nil || math.IsNaN(duration) || math.IsInf(duration, 0) || duration <= 0 { + return errors.New("invalid MP3 duration") + } + for _, stream := range info.Streams { + if stream.Codec != "mp3" { + return errors.New("speech response is not MP3") + } + } + if _, err := e.command(mediaCtx, "ffmpeg", "-v", "error", "-xerror", "-f", "mp3", "-protocol_whitelist", "file,pipe", "-i", temporary, "-f", "null", "-"); err != nil { + return errors.New("MP3 failed complete decoding") + } + if err := os.Rename(temporary, filepath.Join(e.output, "release-podcast.mp3")); err != nil { + return errors.New("cannot install validated MP3") + } + m.Status = "audio_validated" + m.Duration = duration + m.DurationNeedsReview = duration < 105 || duration > 145 + m.AudioHash = hashBytes(data) + return e.writeJSON("manifest.json", m) +} + +func strictDecode(data []byte, target any) error { + decoder := json.NewDecoder(bytes.NewReader(data)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(target); err != nil { + return errors.New("invalid structured narration schema") + } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + return errors.New("unexpected data after narration") + } + return nil +} diff --git a/release/podcast/podcast_test.go b/release/podcast/podcast_test.go new file mode 100644 index 000000000..dfe05672e --- /dev/null +++ b/release/podcast/podcast_test.go @@ -0,0 +1,852 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "io" + "net/http" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" +) + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +func fixtures(t *testing.T) ([]releaseRecord, []source) { + t.Helper() + data, err := os.ReadFile("testdata/releases.json") + if err != nil { + t.Fatal(err) + } + var records []releaseRecord + if err := json.Unmarshal(data, &records); err != nil { + t.Fatal(err) + } + var sources []source + for _, r := range records { + s, err := normalizeRelease(r, false) + if err != nil { + t.Fatal(err) + } + sources = append(sources, s) + } + return records, sources +} + +func exampleNarration(t *testing.T, sources []source) narration { + t.Helper() + texts := []struct { + index int + text, quote string + }{ + {0, "Version 0.64.0 introduced experimental Jellyfin music support, which must be explicitly enabled.", "Enable it with `Jellyfin.Enabled = true`."}, + {0, "Back up your database before upgrading because internal IDs change; clients may need to resync cached IDs.", "back up your database before upgrading"}, + {0, "Plugin authors must migrate to the host HTTP service and review restrictions on private or loopback network addresses.", "Plugins must use the host HTTP service instead"}, + {0, "Shares now belong to their creator, and admins cannot create them for another user.", "Shares are always owned by the user who creates them."}, + {0, "Negative configuration durations are rejected at startup, and unknown options produce warnings.", "Negative values are rejected at startup."}, + {0, "Security fixes protect library access and plugin networking, alongside improvements to artwork, sorting and playlist imports.", "This release fixes several reported vulnerabilities."}, + {0, "Database restore also avoids wiping existing data when the backup file is missing.", "wiping the database when the backup file does not exist"}, + {1, "Version 0.64.1 is a security release fixing five vulnerabilities; upgrade as soon as practical.", "This is a security release. It fixes five vulnerabilities"}, + {1, "Jellyfin client compatibility improves, and Quick Connect makes signing in easier.", "Add Quick Connect sign-in."}, + {1, "Local discovery is opt-in, and Docker users need host networking for discovery broadcasts.", "Docker users need host networking for the UDP broadcast to reach the container."}, + {1, "Smart playlists can reference another playlist by path, while the interface follows your selected language for dates.", "Smart playlists can reference another playlist by path"}, + {2, "Version 0.64.2 fixes scan failures and database lock contention on slow storage.", "Fix `database is locked` errors, UI freezes and failed scans on slow storage."}, + {2, "It also fixes scans on 32-bit builds with invalid track metadata.", "32-bit builds (armv5/6/7, 386)"}, + {2, "Security fixes sanitize download names and prevent an admin password from reaching logs.", "Stop writing the admin password to the log"}, + } + r := narration{Cautions: []coverage{}} + for _, item := range texts { + if !strings.Contains(sources[item.index].Body, item.quote) { + t.Fatalf("fixture quote absent: %s", item.quote) + } + r.Sentences = append(r.Sentences, sentence{Text: item.text, SourceID: sources[item.index].SourceID, Excerpt: item.quote}) + } + for _, c := range cautions(sources) { + index := 0 + for i, s := range r.Sentences { + if s.SourceID == c.SourceID { + index = i + break + } + } + r.Cautions = append(r.Cautions, coverage{CautionID: c.ID, SentenceIndex: &index}) + } + return r +} + +func response(status int, ct string, data []byte) *http.Response { + return &http.Response{StatusCode: status, Header: http.Header{"Content-Type": []string{ct}}, Body: io.NopCloser(bytes.NewReader(data))} +} + +func testEngine(t *testing.T) (*engine, []releaseRecord, []source) { + t.Helper() + // Local fixtures must not inherit the hosting CI runner's Actions context. + // Actions-specific tests establish their own context with actionsEnvironment. + t.Setenv("GITHUB_ACTIONS", "false") + t.Setenv("GH_TOKEN", "") + t.Setenv("OPENAI_API_KEY", "") + e, err := newEngine(t.TempDir()) + if err != nil { + t.Fatal(err) + } + e.openAIKey = "offline-test-key" + records, sources := fixtures(t) + e.command = func(_ context.Context, name string, args ...string) ([]byte, error) { + if name == "git" { + return []byte("test-commit"), nil + } + if len(args) == 1 && args[0] == "-version" { + return nil, nil + } + if name == "ffprobe" { + return []byte(`{"format":{"duration":"120.5"},"streams":[{"codec_name":"mp3"}]}`), nil + } + if name == "ffmpeg" { + return nil, nil + } + return nil, errors.New("unexpected command") + } + e.client.Transport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Host == "api.github.com" { + if req.URL.Path == "/repos/"+repository+"/releases" { + data, _ := json.Marshal(records) + return response(200, "application/json", data), nil + } + if strings.Contains(req.URL.Path, "/actions/artifacts") { + return response(200, "application/json", []byte(`{"artifacts":[]}`)), nil + } + for _, record := range records { + if req.URL.Path == "/repos/"+repository+"/releases/tags/"+record.Tag || req.URL.Path == "/repos/"+repository+"/releases/"+strconv.FormatInt(record.ID, 10) { + data, _ := json.Marshal(record) + return response(200, "application/json", data), nil + } + } + } + if req.URL.Host == "api.openai.com" { + if req.URL.Path == "/v1/responses" { + data, _ := json.Marshal(exampleNarration(t, sources)) + body, _ := json.Marshal(map[string]any{"status": "completed", "usage": map[string]int{"input_tokens": 1000}, "output": []any{map[string]any{"type": "message", "content": []any{map[string]string{"type": "output_text", "text": string(data)}}}}}) + return response(200, "application/json", body), nil + } + if req.URL.Path == "/v1/audio/speech" { + return response(200, "audio/mpeg", []byte("mock-mp3-data")), nil + } + } + t.Fatalf("unmocked network request: %s", req.URL.Host+req.URL.Path) + return nil, errors.New("unmocked request") + }) + return e, records, sources +} + +func audioOptions() options { + return options{from: "v0.64.0", to: "v0.64.2", mode: "audio", textModel: "gpt-6-luna", ttsModel: "gpt-4o-mini-tts-2025-12-15", voice: "onyx", allowPaid: true} +} + +func TestVersionSelection(t *testing.T) { + for _, raw := range []string{"", "v0.64.0,", "v0.64.0,0.64.0", "v1.0.0,v2.0.0,v3.0.0,v4.0.0", "$(touch secret)", "../../foo", "v1.0.0\nmalicious", "v01.2.3", "v1.2.3١", "v99999999999.0.0"} { + t.Run(raw, func(t *testing.T) { + if _, err := normalizeTag(raw); err == nil { + t.Fatal("unsafe tags accepted") + } + }) + } + tag, err := normalizeTag("0.64.2") + if err != nil || tag != "v0.64.2" { + t.Fatal(tag, err) + } +} + +func TestSingleReleaseSelection(t *testing.T) { + e, records, _ := testEngine(t) + original := e.client.Transport + e.client.Transport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Path != "/repos/"+repository+"/releases/tags/v0.64.2" { + t.Fatal("single release must use its exact tag endpoint", req.URL) + } + return original.RoundTrip(req) + }) + for _, from := range []string{"0.64.2", "v0.64.2"} { + sources, err := e.resolveLocal(t.Context(), options{from: from}) + if err != nil || len(sources) != 1 || sources[0].Tag != "v0.64.2" { + t.Fatal(sources, err) + } + } + if _, err := parseOptions([]string{"--tags", "v0.64.0,v0.64.1"}, io.Discard); err == nil { + t.Fatal("removed --tags flag remains accepted") + } + r := records[0] + r.Tag += "-rc.1" + r.Prerelease = true + data, _ := json.Marshal(r) + e.client.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { return response(200, "application/json", data), nil }) + if _, err := e.resolveLocal(t.Context(), options{from: r.Tag}); err == nil { + t.Fatal("single prerelease accepted without opt-in") + } + if sources, err := e.resolveLocal(t.Context(), options{from: r.Tag, includePrereleases: true}); err != nil || len(sources) != 1 { + t.Fatal(sources, err) + } +} + +func TestReleaseEligibility(t *testing.T) { + records, _ := fixtures(t) + cases := map[string]func(*releaseRecord){"draft": func(r *releaseRecord) { r.Draft = true }, "prerelease": func(r *releaseRecord) { r.Prerelease = true }, "empty": func(r *releaseRecord) { r.Body = " " }, "unpublished": func(r *releaseRecord) { r.PublishedAt = "" }, "id": func(r *releaseRecord) { r.ID = 0 }, "oversized": func(r *releaseRecord) { r.Body = strings.Repeat("x", maxSourceBytes+1) }, "multiple tags": func(r *releaseRecord) { r.Tag = "v0.64.0,v0.64.1" }} + for name, mutate := range cases { + t.Run(name, func(t *testing.T) { + r := records[0] + mutate(&r) + if _, err := normalizeRelease(r, false); err == nil { + t.Fatal("ineligible release accepted") + } + }) + } + r := records[0] + r.Prerelease = true + r.Tag = "v0.64.0-rc.1" + if _, err := normalizeRelease(r, true); err != nil { + t.Fatal(err) + } +} + +func TestInclusiveRangeSelection(t *testing.T) { + e, _, _ := testEngine(t) + sources, err := e.resolveLocal(t.Context(), options{from: "0.64.0", to: "v0.64.2"}) + if err != nil || len(sources) != 3 || sources[0].Tag != "v0.64.0" || sources[2].Tag != "v0.64.2" { + t.Fatal(sources, err) + } + for _, o := range []options{{from: "v0.64.2", to: "v0.64.0"}, {from: "v0.63.0", to: "v0.64.2"}, {}, {to: "v0.64.2"}, {from: "v0.64.0,v0.64.1"}, {from: "v0.64.0-rc.1", to: "v0.64.2"}} { + if _, err := e.resolveLocal(t.Context(), o); err == nil { + t.Fatal("invalid range accepted", o) + } + } +} + +func TestSemVerPrereleasePrecedence(t *testing.T) { + ordered := []string{"v1.0.0-alpha", "v1.0.0-alpha.1", "v1.0.0-alpha.beta", "v1.0.0-beta", "v1.0.0-beta.2", "v1.0.0-beta.11", "v1.0.0-rc.1", "v1.0.0"} + for i, a := range ordered { + for j, b := range ordered { + order := compareVersion(a, b) + if (i < j && order >= 0) || (i == j && order != 0) || (i > j && order <= 0) { + t.Fatalf("incorrect precedence for %s and %s: %d", a, b, order) + } + } + } + for _, pair := range [][2]string{{"v1.0.0-rc.2", "v1.0.0-rc.10"}, {"v1.0.0-99999999999999999999", "v1.0.0-100000000000000000000"}, {"v1.0.0-9", "v1.0.0-alpha"}, {"v1.0.0-alpha.beta", "v1.0.0-alpha-beta"}, {"v1.0.0", "v1.0.1-alpha"}} { + if _, err := version(pair[0]); err != nil { + t.Fatal(err) + } + if compareVersion(pair[0], pair[1]) >= 0 { + t.Fatal("incorrect numeric/identifier precedence", pair) + } + } + for _, tag := range []string{"v1.0.0-01", "v1.0.0-rc.01", "v1.0.0-rc..1", "v1.0.0-"} { + if _, err := normalizeTag(tag); err == nil { + t.Fatal("invalid SemVer prerelease accepted", tag) + } + } +} + +func TestRangePrereleaseBoundaries(t *testing.T) { + e, records, _ := testEngine(t) + selected := []releaseRecord{records[1], records[0]} + for i := range 2 { + r := records[i] + r.ID += 100 + r.Tag += "-rc.1" + r.Prerelease = true + selected = append(selected, r) + } + data, _ := json.Marshal(selected) + e.client.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { return response(200, "application/json", data), nil }) + sources, err := e.resolveLocal(t.Context(), options{from: "v0.64.0", to: "v0.64.1", includePrereleases: true}) + if err != nil || len(sources) != 3 || sources[0].Tag != "v0.64.0" || sources[1].Tag != "v0.64.1-rc.1" || sources[2].Tag != "v0.64.1" { + t.Fatal("prerelease at lower bound must be excluded, upper-bound RC included", sources, err) + } + sources, err = e.resolveLocal(t.Context(), options{from: "v0.64.0", to: "v0.64.1"}) + if err != nil || len(sources) != 2 { + t.Fatal("prerelease opt-in was bypassed", sources, err) + } +} + +func TestRangeDiscardsDraftsAndFailsClosedAtLimit(t *testing.T) { + e, records, _ := testEngine(t) + draft := records[0] + draft.ID = 999 + draft.Draft = true + draft.Tag = "v0.64.0-rc.1" + draft.Body = "private advisory never sent" + data, _ := json.Marshal(append(records, draft)) + e.client.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { return response(200, "application/json", data), nil }) + sources, err := e.resolveLocal(t.Context(), options{from: "v0.64.0", to: "v0.64.2", includePrereleases: true}) + if err != nil || len(sources) != 3 { + t.Fatal(sources, err) + } + page := make([]releaseRecord, 100) + for i := range page { + page[i] = releaseRecord{Draft: true} + } + data, _ = json.Marshal(page) + calls := 0 + e.client.Transport = roundTripFunc(func(*http.Request) (*http.Response, error) { + calls++ + return response(200, "application/json", data), nil + }) + if _, err := e.resolveLocal(t.Context(), options{from: "v0.64.0", to: "v0.64.2"}); err == nil || calls != 10 { + t.Fatal("unbounded/incomplete range accepted", calls, err) + } +} + +func TestConfigurationAndCostLimits(t *testing.T) { + e, _, sources := testEngine(t) + cfg, err := reviewedConfig("gpt-6-luna", "gpt-4o-mini-tts-2025-12-15", "cedar", "audio") + if err != nil { + t.Fatal(err) + } + payload := e.textPayload(sources, cfg) + cost, err := modeledCost(payload, cfg, "audio") + if err != nil || cost == nil || *cost > maxCostUSD { + t.Fatal(cost, err) + } + if payload["store"] != false || payload["tools"] != nil || payload["reasoning"] == nil { + t.Fatal("unsafe text configuration") + } + if _, err := modeledCost(strings.Repeat("x", maxPromptBytes+1), cfg, "audio"); err == nil { + t.Fatal("oversized prompt accepted") + } + hd, err := reviewedConfig("gpt-4.1-mini-2025-04-14", "tts-1-hd", "onyx", "audio") + if err != nil { + t.Fatal(err) + } + if _, err := modeledCost(strings.Repeat("x", 60000), hd, "audio"); err == nil { + t.Fatal("modeled dollar ceiling ignored") + } + for _, values := range [][3]string{{"", "", ""}, {"unreviewed", "tts-1", "onyx"}, {"gpt-6-luna", "unreviewed", "onyx"}, {"gpt-6-luna", "tts-1", "cedar"}, {"gpt-6-luna", "tts-1", "echo,fable"}} { + if _, err := reviewedConfig(values[0], values[1], values[2], "audio"); err == nil { + t.Fatal("invalid configuration accepted", values) + } + } + if _, err := reviewedConfig("", "", "", "validate"); err != nil { + t.Fatal(err) + } + legacy, err := reviewedConfig("gpt-6-luna", "tts-1", "onyx", "audio") + if err != nil || legacy.Instructions != "" { + t.Fatal(legacy, err) + } +} + +func TestCLIDryRunAndPaidGuards(t *testing.T) { + t.Setenv("AUDIO_TEXT_MODEL", "gpt-6-luna") + o, err := parseOptions([]string{"--from", "0.64.2", "--text-model", "gpt-4.1-mini-2025-04-14", "--mode", "audio", "--dry-run", "--output", "out"}, io.Discard) + if err != nil || o.mode != "validate" || o.textModel != "gpt-4.1-mini-2025-04-14" || o.output != "out" { + t.Fatal(o, err) + } + if _, err := parseOptions([]string{"--openai-key", "never-pass-a-key"}, io.Discard); err == nil { + t.Fatal("key argument accepted") + } + e, _, _ := testEngine(t) + paid := audioOptions() + paid.allowPaid = false + if err := e.runLocal(t.Context(), paid, io.Discard); err == nil { + t.Fatal("paid mode needs explicit flag") + } + paid.allowPaid = true + e.openAIKey = "" + if err := e.runLocal(t.Context(), paid, io.Discard); err == nil { + t.Fatal("paid mode needs local environment key") + } +} + +func TestLocalValidationNeverContactsOpenAI(t *testing.T) { + e, _, _ := testEngine(t) + original := e.client.Transport + e.client.Transport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Host == "api.openai.com" { + t.Fatal("validation contacted OpenAI") + } + return original.RoundTrip(req) + }) + if err := e.runLocal(t.Context(), options{from: "v0.64.0", to: "v0.64.2", mode: "validate"}, io.Discard); err != nil { + t.Fatal(err) + } + var m manifest + if err := e.readJSON("manifest.json", &m); err != nil || m.TextRequests != 0 || m.SpeechRequests != 0 || m.Origin != "local" { + t.Fatal(m, err) + } +} + +func TestLocalPaidPipelineAndDuplicateProtection(t *testing.T) { + e, _, _ := testEngine(t) + if err := e.runLocal(t.Context(), audioOptions(), io.Discard); err != nil { + t.Fatal(err) + } + var m manifest + if err := e.readJSON("manifest.json", &m); err != nil { + t.Fatal(err) + } + if m.Status != "audio_validated" || m.TextRequests != 1 || m.SpeechRequests != 1 || m.AudioHash == "" || m.ScriptHash == "" || m.Duration != 120.5 { + t.Fatal(m) + } + for _, name := range []string{"release-podcast.mp3", "transcript.txt", "evidence.json", "sources.json", "manifest.json"} { + if _, err := os.Stat(filepath.Join(e.output, name)); err != nil { + t.Fatal(err) + } + } + if err := e.runLocal(t.Context(), audioOptions(), io.Discard); err == nil { + t.Fatal("duplicate attempt permitted") + } + o := audioOptions() + o.force = true + if err := e.runLocal(t.Context(), o, io.Discard); err != nil { + t.Fatal(err) + } +} + +func TestLocalLedgerPersistsFailuresAndLocksConcurrentAttempts(t *testing.T) { + e, _, sources := testEngine(t) + cfg, _ := reviewedConfig("gpt-6-luna", "tts-1", "onyx", "audio") + m, err := e.prepare(t.Context(), sources, cfg, "audio", "local", false, false) + if err != nil { + t.Fatal(err) + } + release, err := e.reserveLocal(m, false) + if err != nil { + t.Fatal(err) + } + if _, err := e.reserveLocal(m, true); err == nil { + t.Fatal("force bypassed active lock") + } + release() + if _, err := e.reserveLocal(m, false); err == nil { + t.Fatal("reserved failed attempt retried") + } + release, err = e.reserveLocal(m, true) + if err != nil { + t.Fatal(err) + } + release() +} + +func TestEvidenceAndConsequentialOmissions(t *testing.T) { + _, sources := fixtures(t) + if _, err := validateNarration(exampleNarration(t, sources), sources); err != nil { + t.Fatal(err) + } + for _, term := range []string{"Back up", "resync", "experimental", "host HTTP", "opt-in", "host networking", "32-bit", "slow storage"} { + t.Run(term, func(t *testing.T) { + r := exampleNarration(t, sources) + for i := range r.Sentences { + r.Sentences[i].Text = strings.ReplaceAll(r.Sentences[i].Text, term, "some detail") + } + if _, err := validateNarration(r, sources); err == nil { + t.Fatal("consequential qualifier omitted") + } + }) + } + for name, mutate := range map[string]func(*narration){"source": func(r *narration) { r.Sentences[0].SourceID = "unknown" }, "excerpt": func(r *narration) { r.Sentences[0].Excerpt = "fabricated unsupported evidence" }, "coverage": func(r *narration) { r.Cautions = r.Cautions[1:] }, "index": func(r *narration) { n := 999; r.Cautions[0].SentenceIndex = &n }, "duplicate": func(r *narration) { r.Cautions = append(r.Cautions, r.Cautions[0]) }} { + t.Run(name, func(t *testing.T) { + r := exampleNarration(t, sources) + mutate(&r) + if _, err := validateNarration(r, sources); err == nil { + t.Fatal("invalid evidence accepted") + } + }) + } +} + +func TestUnsafeModelOutputAndSourceMarkup(t *testing.T) { + _, sources := fixtures(t) + for _, suffix := range []string{" https://evil.example", " `shell`", " $(cat secret)", "