git/intake/main

fix: an album can sit in the archive with no journal record

The journal is the only thing that knows an album exists. An album is
promoted into the archive before its record is written, so a process killed
in between -- OOM, power loss, SIGKILL -- leaves a complete album that
verify says nothing about, audit does not count, and a later run happily
files a second copy beside. Neither command looked at the archive at all,
so neither could notice.

library verify now lists every album directory in the archive that no
journal record names, and library audit counts them as needing attention.
Both report the count in --json.

The same commit fixes the name search in the "both" collision path: it
treated any error from os.Stat as "the name is free" unless it was
NotExist, so an unreadable parent directory would have spun looking for a
name it could never check.

Closes a70d500
diff --git a/internal/cli/cli.go b/internal/cli/cli.go
index 52a12b4..de11d33 100644
--- a/internal/cli/cli.go
+++ b/internal/cli/cli.go
@@ -456,8 +456,18 @@ func (a *App) Audit(ctx context.Context) error {
// suggestion to rebuild it -- which rebuilds it identically.
ext := library.Ext(a.Cfg.Codec)
+ dirs, err := a.archiveDirs()
+ if err != nil {
+ return err
+ }
+ orphans := a.unrecorded(j, dirs)
+
var checked, ok, problems, missingArchives int
var findings []auditFinding
+ for _, dir := range orphans {
+ problems++
+ findings = append(findings, auditFinding{"(not in the journal)", dir, "this album is in the archive but no journal record names it"})
+ }
for _, k := range keys {
rec := j.Albums[k]
if rec.Phase != state.PhaseArchived {
@@ -520,6 +530,7 @@ func (a *App) Audit(ctx context.Context) error {
out := map[string]any{
"checked": checked, "ok": ok, "problems": problems,
"missing_archives": missingArchives,
+ "unrecorded": len(orphans),
}
if findings != nil {
out["albums"] = findings
@@ -539,6 +550,9 @@ func (a *App) Audit(ctx context.Context) error {
if missingArchives > 0 {
a.logf("%d archived album(s) have no audio in their archive directory.", missingArchives)
}
+ if len(orphans) > 0 {
+ a.logf("%d album(s) are in the archive with no journal record; they are invisible to verify and can be filed again.", len(orphans))
+ }
if problems > 0 {
a.logf("Rebuild one with: intake library transcode <album>")
}
@@ -563,6 +577,60 @@ func archiveMasters(dir string) ([]string, []string, error) {
return masters, names, nil
}
+// archiveDirs lists the album directories actually present in the archive, two
+// levels down.
+//
+// The journal is the only thing that knows an album exists, so an album that
+// reached the archive without a record -- the process was killed between the
+// promotion and the journal write, or somebody copied it in by hand -- is
+// invisible: verify says nothing about it and a later run happily files a second
+// copy beside it. Comparing the two lists is what makes that visible.
+func (a *App) archiveDirs() ([]string, error) {
+ root := a.Cfg.Archive()
+ artists, err := os.ReadDir(root)
+ if err != nil {
+ if os.IsNotExist(err) {
+ return nil, nil
+ }
+ return nil, err
+ }
+ var out []string
+ for _, artist := range artists {
+ if !artist.IsDir() || strings.HasPrefix(artist.Name(), ".") {
+ continue
+ }
+ albums, err := os.ReadDir(filepath.Join(root, artist.Name()))
+ if err != nil {
+ continue
+ }
+ for _, album := range albums {
+ if !album.IsDir() || strings.HasPrefix(album.Name(), ".") {
+ continue
+ }
+ out = append(out, filepath.Join(root, artist.Name(), album.Name()))
+ }
+ }
+ sort.Strings(out)
+ return out, nil
+}
+
+// unrecorded returns the archive directories the journal does not name.
+func (a *App) unrecorded(j *state.Journal, dirs []string) []string {
+ known := map[string]bool{}
+ for _, rec := range j.Albums {
+ if rec.ArchiveDir != "" {
+ known[filepath.Clean(rec.ArchiveDir)] = true
+ }
+ }
+ var out []string
+ for _, dir := range dirs {
+ if !known[filepath.Clean(dir)] {
+ out = append(out, dir)
+ }
+ }
+ return out
+}
+
func (a *App) journalPath() string { return filepath.Join(a.Cfg.StateDir, "state.json") }
func (a *App) journal() (*state.Journal, error) {
@@ -662,6 +730,12 @@ func (a *App) Verify(ctx context.Context) error {
}
sort.Strings(keys)
+ dirs, err := a.archiveDirs()
+ if err != nil {
+ return err
+ }
+ orphans := a.unrecorded(j, dirs)
+
var archived, missing, placeholders, noLibrary int
var placeholderRows []state.Record
var missingRows []string
@@ -713,12 +787,16 @@ func (a *App) Verify(ctx context.Context) error {
"missing": missing,
"placeholders": placeholders,
"no_library": noLibrary,
+ "unrecorded": orphans,
})
}
for _, dir := range missingRows {
a.logf("MISSING %s (journaled but not on disk)", dir)
}
+ for _, dir := range orphans {
+ a.logf("UNRECORDED %s (in the archive but no journal record names it)", dir)
+ }
if len(placeholderRows) > 0 {
a.logf("")
@@ -739,8 +817,8 @@ func (a *App) Verify(ctx context.Context) error {
}
a.logf("")
- a.logf("journal: %d archived, %d missing on disk, %d with placeholder covers, %d without a library copy",
- archived, missing, placeholders, noLibrary)
+ a.logf("journal: %d archived, %d missing on disk, %d with placeholder covers, %d without a library copy, %d unrecorded in the archive",
+ archived, missing, placeholders, noLibrary, len(orphans))
if noLibrary > 0 {
a.logf("Build one with: intake library transcode <album>")
}
diff --git a/internal/cli/journal_test.go b/internal/cli/journal_test.go
index 792e04f..44a6b90 100644
--- a/internal/cli/journal_test.go
+++ b/internal/cli/journal_test.go
@@ -219,3 +219,54 @@ func TestJSONOutputIsTheOnlyThingOnStdout(t *testing.T) {
t.Errorf("missing = %v, want 1", verifyOut["missing"])
}
}
+
+// The journal is the only thing that knows an album exists. An album that reached
+// the archive without a record -- the process was killed between the promotion
+// and the journal write -- is invisible: verify says nothing and a later run
+// files a second copy beside it. Comparing the archive against the journal is
+// what makes that visible.
+func TestUnrecordedArchiveDirectoriesAreReported(t *testing.T) {
+ root := t.TempDir()
+ a, out := cliApp(t, root)
+
+ recorded := filepath.Join(root, "archive", "artist", "known")
+ orphan := filepath.Join(root, "archive", "artist", "orphan")
+ for _, d := range []string{recorded, orphan} {
+ if err := os.MkdirAll(d, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ }
+ seed(t, a, state.Record{Source: "/staging/one", Phase: state.PhaseArchived, AlbumArtist: "A", Album: "Known", ArchiveDir: recorded, LibraryDir: recorded})
+
+ if err := a.Verify(t.Context()); err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(out.String(), "UNRECORDED") || !strings.Contains(out.String(), orphan) {
+ t.Errorf("verify did not report the album with no record:\n%s", out.String())
+ }
+ if strings.Contains(out.String(), "UNRECORDED "+recorded) {
+ t.Errorf("verify reported a recorded album as unrecorded:\n%s", out.String())
+ }
+
+ out.Reset()
+ if err := a.Audit(t.Context()); err != nil {
+ t.Fatal(err)
+ }
+ if !strings.Contains(out.String(), "1 album(s) are in the archive with no journal record") {
+ t.Errorf("audit did not count the album with no record:\n%s", out.String())
+ }
+
+ out.Reset()
+ a.JSON = true
+ if err := a.Verify(t.Context()); err != nil {
+ t.Fatal(err)
+ }
+ var parsed map[string]any
+ if err := json.Unmarshal(out.Bytes(), &parsed); err != nil {
+ t.Fatalf("library verify --json is not JSON: %v\n%s", err, out.String())
+ }
+ list, _ := parsed["unrecorded"].([]any)
+ if len(list) != 1 {
+ t.Errorf("unrecorded = %v, want one entry", parsed["unrecorded"])
+ }
+}
diff --git a/internal/execute/execute.go b/internal/execute/execute.go
index 704dc2b..c9a69ca 100644
--- a/internal/execute/execute.go
+++ b/internal/execute/execute.go
@@ -321,9 +321,19 @@ func promote(target, tmp, collision string, logf func(string, ...any)) (string,
case "both":
alt := target + " (2)"
for i := 3; ; i++ {
- if _, err := os.Stat(alt); os.IsNotExist(err) {
+ _, err := os.Stat(alt)
+ if os.IsNotExist(err) {
break
}
+ if err != nil {
+ // Anything other than "it is not there" -- an unreadable
+ // parent directory, say -- would otherwise spin here for
+ // ever picking names that can never be checked.
+ return "", fmt.Errorf("looking for a free name beside %s: %w", target, err)
+ }
+ if i > 1000 {
+ return "", fmt.Errorf("could not find a free name beside %s", target)
+ }
alt = fmt.Sprintf("%s (%d)", target, i)
}
logf("destination exists; keeping both as %s", filepath.Base(alt))