refactor(#345): エラーログを DI された ErrorLog 契約に統一する

AGENTS.md

@@ -85,6 +85,23 @@ Shared conventions:
   `AudioTapGateway`, #313).
 - Handlers, use cases, and repositories return data. `StandardOutput` formats
   and prints it. Never pass handler instances into output methods.
+- There are exactly three output paths, all Domain protocols resolved through
+  DI, and every one of them is a write-only sink -- never a DataStore.
+  `StandardOutput` carries CLI results, `DeveloperLog` carries config-gated
+  decision traces (#331), and `ErrorLog` carries always-on operational errors
+  to stderr (#345). **Never write `fputs` (or `print`, or `os.Logger`) directly
+  from a Source module** -- the only two live `fputs` calls are the injected
+  printers inside `PrintStandardOutput` and `StandardErrorLog`, and a fourth
+  hand-rolled path is what #345 removed. Report errors with
+  `errorLog.record(.subsystem, "<operation> failed: \(error)")`: the `lyra:`
+  prefix, the subsystem spelling, and the newline belong to the sink, and the
+  subsystem vocabulary is the closed `ErrorSubsystem` enum, so adding a source
+  means adding a case rather than inventing a string.
+- Suppressing a specific error is a rule that needs a test, not a comment.
+  LRCLIB's 404 means "no lyrics for this track" and must stay unreported so it
+  is distinguishable from a broken fetch (#318); that guard lives at the call
+  site (it is LRCLIB's contract, not the sink's business) and is pinned in both
+  directions by `LyricsErrorReportingTests`.
 - Views do not own business logic. Keep orchestration in Presenters and
   Interactors. The existing rendering-only dependency access patterns may stay,
   but do not add feature logic to SwiftUI views.

Package.swift

@@ -150,6 +150,14 @@ let package = Package(
             ]
         ),
 
+        // ── ErrorLog ──
+        .target(
+            name: "ErrorLog",
+            dependencies: [
+                "Domain"
+            ]
+        ),
+
         // ── BenchmarkHandler ──
         .target(
             name: "BenchmarkHandler",
@@ -250,6 +258,7 @@ let package = Package(
                 "Domain",
                 "RandomSource",
                 "DeveloperLog",
+                "ErrorLog",
                 "TrackInteractor",
                 "ScreenInteractor",
                 "ConfigInteractor",
@@ -445,6 +454,7 @@ let package = Package(
                 "Domain",
                 "ScopedAPISession",
                 .product(name: "Papyrus", package: "papyrus"),
+                .product(name: "Dependencies", package: "swift-dependencies"),
             ]
         ),
         .target(
@@ -636,6 +646,13 @@ let package = Package(
         ),
         .testTarget(name: "AppKitScreenProviderTests", dependencies: ["AppKitScreenProvider", "Domain"]),
         .testTarget(name: "RandomSourceTests", dependencies: ["RandomSource", "Domain"]),
+        .testTarget(
+            name: "ErrorLogTests",
+            dependencies: [
+                "ErrorLog",
+                "Domain",
+            ]
+        ),
         .testTarget(
             name: "DeveloperLogTests",
             dependencies: [

Sources/DependencyInjection/ErrorLogRegistration.swift

@@ -0,0 +1,7 @@
+import Dependencies
+import Domain
+import ErrorLog
+
+extension ErrorLogKey: DependencyKey {
+    public static let liveValue: any Domain.ErrorLog = StandardErrorLog()
+}

Sources/Domain/Misc/ErrorLog.swift

@@ -0,0 +1,38 @@
+import Dependencies
+
+/// A write-only, always-on error sink. A caller reports that one operation failed and
+/// says which subsystem it belongs to; the live implementation renders that as a line
+/// on stderr. Like `DeveloperLog` this is a *StandardOutput-family* contract — an
+/// output sink, never a DataStore: nothing reads it back as domain data.
+///
+/// It is deliberately separate from `DeveloperLog` (config-gated decision traces, file
+/// output, contains listening history) and from `StandardOutput` (CLI results, which a
+/// DataSource has no business reaching for). Sharing either would have made the three
+/// purposes one switch.
+///
+/// The `lyra: ` prefix, the subsystem rendering, and the trailing newline all belong to
+/// the implementation, so the convention lives in one place instead of being restated
+/// at every call site as it was before #345.
+public protocol ErrorLog: Sendable {
+    /// Report one failed operation. `message` describes what failed and why, without
+    /// the prefix or the subsystem — `"search failed: \(error)"`, not
+    /// `"lyra: MusicBrainz search failed: \(error)"`.
+    func record(_ subsystem: ErrorSubsystem, _ message: String)
+}
+
+public enum ErrorLogKey: TestDependencyKey {
+    /// Silent under test: a suite that does not care about error reporting should not
+    /// spray stderr, and one that does overrides `$0.errorLog` with a spy.
+    public static let testValue: any ErrorLog = SilentErrorLog()
+}
+
+extension DependencyValues {
+    public var errorLog: any ErrorLog {
+        get { self[ErrorLogKey.self] }
+        set { self[ErrorLogKey.self] = newValue }
+    }
+}
+
+private struct SilentErrorLog: ErrorLog {
+    func record(_ subsystem: ErrorSubsystem, _ message: String) {}
+}

Sources/Entity/ErrorSubsystem.swift

@@ -0,0 +1,17 @@
+/// Where an error report came from, as it appears in the daemon's stderr line (#345).
+///
+/// The set is closed on purpose. Before this existed the subsystem was a bare string
+/// baked into each `fputs` call, so nothing stopped the four sites from disagreeing —
+/// and they did (`LRCLIB` / `MusicBrainz` / `AI` / `spectrum:`, mixing case and an
+/// extra colon). A raw-value enum makes the vocabulary a compile-time fact and gives
+/// the naming rule something to be tested against.
+public enum ErrorSubsystem: String, Sendable, CaseIterable {
+    /// The LRCLIB lyrics catalog.
+    case lrclib = "LRCLIB"
+    /// The MusicBrainz metadata catalog.
+    case musicBrainz = "MusicBrainz"
+    /// The user-configured OpenAI-compatible metadata extractor.
+    case ai = "AI"
+    /// Audio capture and analysis for the spectrum overlay.
+    case spectrum = "Spectrum"
+}

Sources/ErrorLog/StandardErrorLog.swift

@@ -0,0 +1,27 @@
+import Darwin.POSIX
+import Domain
+
+/// Live `ErrorLog`: one line per report on stderr, which is where all four call sites
+/// were already writing by hand before #345. stderr rather than a file because these
+/// are always-on operational errors — the daemon's stderr is captured by whichever
+/// supervisor started it (brew service, LaunchAgent, or a foreground `lyra daemon`),
+/// so the report lands wherever the user is already looking.
+public struct StandardErrorLog: Sendable {
+    private let printer: @Sendable (String) -> Void
+
+    public init() {
+        self.init { fputs($0, stderr) }
+    }
+
+    /// Test seam, mirroring `PrintStandardOutput`'s injected printers: the rendering is
+    /// the part worth asserting on, and it is not observable through a real `fputs`.
+    init(printer: @escaping @Sendable (String) -> Void) {
+        self.printer = printer
+    }
+}
+
+extension StandardErrorLog: ErrorLog {
+    public func record(_ subsystem: ErrorSubsystem, _ message: String) {
+        printer("lyra: \(subsystem.rawValue) \(message)\n")
+    }
+}

Sources/LyricsDataSource/LyricsDataSourceImpl.swift

@@ -1,9 +1,11 @@
+import Dependencies
 import Domain
 import Foundation
 @preconcurrency import Papyrus
 import ScopedAPISession
 
 public struct LyricsDataSourceImpl {
+    @Dependency(\.errorLog) private var errorLog
     private let apiSession: ScopedAPISession<any LRCLib>
 
     public init() {
@@ -62,8 +64,13 @@ extension LyricsDataSourceImpl {
     // 404 is LRCLIB's regular "no lyrics for this track" answer; only transport
     // and server failures are worth surfacing so "no lyrics" and "fetch broken"
     // stay distinguishable in the daemon log (#318).
+    //
+    // The guard stays here rather than moving into the sink: that a 404 means "no
+    // lyrics" is LRCLIB's own contract, not something a general error log could know.
+    // What #345 changed is that it is now guarding a *dependency* — so the rule can
+    // finally be pinned by a test instead of resting on this comment.
     private func log(_ error: some Error, operation: String) {
         if let papyrusError = error as? PapyrusError, papyrusError.response?.statusCode == 404 { return }
-        fputs("lyra: LRCLIB \(operation) failed: \(error)\n", stderr)
+        errorLog.record(.lrclib, "\(operation) failed: \(error)")
     }
 }

Sources/MetadataDataSource/LLMMetadataDataSourceImpl.swift

@@ -6,6 +6,7 @@ import ScopedAPISession
 
 public struct LLMMetadataDataSourceImpl {
     @Dependency(\.configDataSource) private var configDataSource
+    @Dependency(\.errorLog) private var errorLog
     private let sessionFactory: @Sendable (AIEndpoint) -> ScopedAPISession<any OpenAICompatible>
 
     public init() {
@@ -51,7 +52,7 @@ extension LLMMetadataDataSourceImpl {
         do {
             response = try await sessionFactory(config).withAPI { try await $0.chatCompletion(request: request) }
         } catch {
-            fputs("lyra: AI extraction failed: \(error)\n", stderr)
+            errorLog.record(.ai, "extraction failed: \(error)")
             return nil
         }
 

Sources/MetadataDataSource/MusicBrainzMetadataDataSourceImpl.swift

@@ -1,9 +1,11 @@
+import Dependencies
 import Domain
 import Foundation
 @preconcurrency import Papyrus
 import ScopedAPISession
 
 public struct MusicBrainzMetadataDataSourceImpl {
+    @Dependency(\.errorLog) private var errorLog
     private let apiSession: ScopedAPISession<any MusicBrainz>
 
     public init() {
@@ -40,7 +42,7 @@ extension MusicBrainzMetadataDataSourceImpl: MetadataDataSource {
                 guard !candidates.isEmpty else { continue }
                 return candidates
             } catch {
-                fputs("lyra: MusicBrainz search failed: \(error)\n", stderr)
+                errorLog.record(.musicBrainz, "search failed: \(error)")
             }
         }
 

Sources/SpectrumInteractor/SpectrumInteractorImpl.swift

@@ -34,6 +34,7 @@ public final class SpectrumInteractorImpl: @unchecked Sendable {
     @Dependency(\.configUseCase) private var configService
     @Dependency(\.playbackUseCase) private var playbackService
     @Dependency(\.spectrumUseCase) private var spectrumService
+    @Dependency(\.errorLog) private var errorLog
     private let capturingSubject = CurrentValueSubject<Bool, Never>(false)
     private let processor = OSAllocatedUnfairLock(initialState: Processor.idle)
 
@@ -100,13 +101,13 @@ extension SpectrumInteractorImpl: SpectrumInteractor {
                 failedAttempts = started ? 0 : failedAttempts + 1
                 guard started else {
                     let giveUp = failedAttempts >= maxCaptureAttempts
-                    fputs(
-                        "lyra: spectrum: startCapture(pid: \(pid)) failed "
+                    errorLog.record(
+                        .spectrum,
+                        "startCapture(pid: \(pid)) failed "
                             + "(attempt \(failedAttempts)/\(maxCaptureAttempts)); "
                             + (giveUp
-                                ? "giving up until the source changes\n"
-                                : "retrying on next now-playing tick\n"),
-                        stderr)
+                                ? "giving up until the source changes"
+                                : "retrying on next now-playing tick"))
                     continue
                 }
             }

Sources/VersionHandler/Resources/version.txt

@@ -1 +1 @@
-2.28.5
+2.28.6

Tests/ErrorLogTests/StandardErrorLogTests.swift

@@ -0,0 +1,72 @@
+import Domain
+import Testing
+import os
+
+@testable import ErrorLog
+
+@Suite("StandardErrorLog (#345)")
+struct StandardErrorLogTests {
+    @Test("default init() wires the live stderr printer")
+    func defaultInitInstantiates() {
+        // Exercising init() covers the production wiring; the real fputs is not
+        // observable, which is exactly why the printer seam exists.
+        _ = StandardErrorLog()
+    }
+
+    @Test("a report renders as one prefixed, subsystem-tagged, newline-terminated line")
+    func rendersOneLine() {
+        let sink = LineRecorder()
+
+        StandardErrorLog(printer: { sink.append($0) }).record(.musicBrainz, "search failed: boom")
+
+        #expect(sink.lines == ["lyra: MusicBrainz search failed: boom\n"])
+    }
+
+    // The point of the contract: the caller hands over the *message*, and the
+    // prefix / subsystem / newline convention lives in one place instead of being
+    // restated (and mis-stated) at each call site as it was before #345.
+    @Test("every subsystem renders the same shape", arguments: ErrorSubsystem.allCases)
+    func everySubsystemRendersTheSameShape(subsystem: ErrorSubsystem) {
+        let sink = LineRecorder()
+
+        StandardErrorLog(printer: { sink.append($0) }).record(subsystem, "op failed: x")
+
+        #expect(sink.lines == ["lyra: \(subsystem.rawValue) op failed: x\n"])
+    }
+
+    // The naming rule as a test rather than a convention. Before #345 the subsystem
+    // was a bare string per call site, and they had already drifted — `spectrum:`
+    // carried a lowercase name and an extra colon that the other three did not.
+    @Test("subsystem names are uniformly shaped", arguments: ErrorSubsystem.allCases)
+    func subsystemNamesAreUniform(subsystem: ErrorSubsystem) {
+        let name = subsystem.rawValue
+
+        #expect(!name.isEmpty)
+        #expect(name.first?.isUppercase == true)
+        #expect(!name.contains(":"))
+        #expect(!name.contains(" "))
+    }
+
+    @Test("each report is one call to the sink — nothing is buffered or merged")
+    func reportsAreNotBatched() {
+        let sink = LineRecorder()
+        let log = StandardErrorLog(printer: { sink.append($0) })
+
+        log.record(.lrclib, "get failed: a")
+        log.record(.ai, "extraction failed: b")
+
+        #expect(sink.lines == ["lyra: LRCLIB get failed: a\n", "lyra: AI extraction failed: b\n"])
+    }
+}
+
+/// Collects what the sink printed. A lock rather than an actor because `record` is
+/// synchronous — an actor could not be read from the nonisolated printer closure.
+private final class LineRecorder: Sendable {
+    private let state = OSAllocatedUnfairLock(initialState: [String]())
+
+    var lines: [String] { state.withLock { $0 } }
+
+    func append(_ line: String) {
+        state.withLock { $0.append(line) }
+    }
+}