| Age | Commit message (Collapse) | Author |
|---|
|
Existing databases created before the finished column was added to the
base schema still satisfied CREATE TABLE IF NOT EXISTS and never got the
column, causing GET /api/v1/media/{id} to fail with
"SQL logic error: no such column: finished" and break media detail view.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
Server: expose a public countUsers endpoint (GET /api/v1/auth/count)
so mobile clients can detect first-run (count=0) without a session.
Android: wire countUsers via DioPlayerApiClient, add firstRunProvider
(FutureProvider), update go_router redirect to drive /bootstrap vs /login
based on the count, rework LoginScreen to handle loading/error states, and
add widget tests for the new login screen and smoke-test updates.
Fix review issues: correct FutureProvider cache-lifetime comment in
first_run_provider.dart; add TestServer_CountUsers covering zero-users
and users-exist cases to handlers_test.go.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
Move all dependency wiring, background worker startup, server lifecycle
(Wire, StartBackgroundWorkers, RunServer, RunWithSignal, BuildLogger) into
internal/app so cmd/player/main.go becomes thin: parse flags, load config,
delegate to app.RunWithSignal. Updated main_test.go to call app.Wire and
app.StartBackgroundWorkers directly.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
l9: Add mediaRoot field to mediaStreamer. NewMediaStreamer now takes a
mediaRoot string parameter; when non-empty, Open() rejects any path that
does not reside under that directory (filepath.Clean prefix check), returning
ErrForbidden to prevent filepath-traversal via a compromised AbsPath in the
DB. Production wiring passes cfg.MediaRoot; tests that don't exercise path
traversal pass "". Added TestMediaStreamerOpenRejectsPathOutsideRoot to
cover the rejection path.
k9: Add a defer-based cleanup guard in DownloadEpisode. After the enclosure
file is written, a succeeded flag gates a deferred closure that calls
dbCleanup() (undoes DB row + removes file) when persistDownloadedEpisode
succeeded, or removeAndLog(path) when it did not. This ensures that any
failure after the file is written — including UpdateEpisodeMedia — leaves no
orphaned files on disk. The guard is disarmed by setting succeeded=true on
the happy path.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
- escapeLike() helper in repository/media.go for LIKE search safety
- ErrWeakPassword sentinel in service.go (min 8 chars, HTTP 400)
- CreateUser rejects empty/short passwords in service/user.go
- ErrWeakPassword auto-dispatched via HTTPStatuser (no switch needed)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
serveBootstrap now calls CountUsers() and redirects to /login.html
when the user count is non-zero, preventing the bootstrap form from
being reachable on an already-configured instance.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
handleError previously hard-coded a switch mapping every service sentinel to
its HTTP status. Adding a new sentinel required editing handleError (OCP
violation). Move the status onto the sentinel itself: introduce
api.HTTPStatuser { HTTPStatus() int }, convert the 10 dispatched service
sentinels to a typed *apiError that implements it, and turn handleError into
a thin errors.As-based dispatcher. ErrShareExpired stays a plain sentinel
because it is handled inline by share handlers and never reaches
handleError.
Sentinel messages are aligned with what handleError used to emit (e.g.
ErrForbidden is now "forbidden" instead of "access denied",
ErrAlreadyBootstrapped is "bootstrap already complete") so the dispatcher
can use err.Error() uniformly. Wrapped errors (fmt.Errorf("%w: ...",
ErrNotFound, ...)) keep their context in the response body, which is a
minor improvement over the previous behaviour that collapsed everything to
the bare sentinel message. errors.Is keeps working via pointer equality on
the *apiError sentinels.
|
|
handlers_share.go (handleCreateShare share-expiry) and handlers_auth.go
(setSessionCookie and apiTokenExpiresAt) previously called time.Now() directly,
which made time-dependent semantics impossible to assert deterministically in
tests. They now use s.clk.Now(), where s.clk is a clock.Clock injected through
ServerDeps.Clock (nil-default to clock.RealClock{} so existing callers keep
working unchanged). apiTokenExpiresAt is promoted to a method on *Server so it
can reach the injected clock. Production wiring in cmd/player/main.go passes
deps.clk so handlers share the same time source as the rest of the services.
Two new unit tests (handlers_share_test.go) use clock.MockClock to assert
handleCreateShare and setSessionCookie compute their expiry timestamps from
the injected clock rather than the wall clock.
While propagating, this commit also includes a mechanical fix-up for the
pathID(...) signature change (now returns int64 + error) across the API
handler files so the package still builds; the additional err-check makes
malformed path variables produce 400 instead of silently parsing as zero.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
The cleanup paths in writeService.UploadMedia, copyFile, and the podcast
episode download/persist flow previously swallowed os.Remove errors.
When unlink fails due to permission or I/O issues, operators had no
breadcrumb — only an already-gone file is benign. Each call now goes
through a small helper that logs a warning unless the error is
fs.ErrNotExist.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
The deferred out.Close() silently dropped disk-full, quota, and I/O
errors surfaced when the OS flushes buffered writes. Capture the Close
error explicitly, remove the partial destination on failure, and add
copyFile success / missing-source unit tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Previously writeJSON wrote the status header before encoding, so an
encode failure produced a misleading 200 (or other caller status) with
a truncated body. Marshal into a bytes.Buffer first; on error emit 500
with a JSON error envelope instead.
|
|
Silently discarding UPDATE failures left the per-feed
consecutive_failures counter stale, so a transient DB error caused the
checker to keep hammering the failing feed every interval. setFeedBackoff
now returns the error and callers log it at warn level so operators see
the failure.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
browseService.ListMedia and ListSets each contained the same
"compute permitted set IDs for this non-admin user" logic. Promote
it to an exported method (h.AllowedSetIDs) on accessHelper so the
permission resolution lives in one place, then call it from both
sites in browse.go. Tests still pass via the existing service
suite — no new unit test was needed since accessHelper is exercised
through its callers.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Several per-service Store interfaces declared the same four-repo
block (UserRepo + SetRepo + SetPermissionRepo + MediaRepo) verbatim.
Extract that into a CoreStore interface and have MediaServiceStore,
AdminServiceStore, AccessHelperStore, BrowseServiceStore, and
WriteServiceStore embed it. Repository structurally still satisfies
every interface; tests unaffected.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Calling Start a second time used to spawn a duplicate goroutine,
leaking the first one and double-counting tick events. sync.Once
matches the existing lifecycle (Stop is also one-shot — the worker
is not designed to be restarted after Stop), so the simplest fix is
a startOnce.Do guard. Added a unit test that calls Start twice and
asserts only one goroutine runs.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
browseService.GetThumbnail previously called os.Stat directly, coupling
the service layer to the filesystem and forcing tests to write temporary
files. Introduce a thumb.Resolver interface plus an FSResolver default,
inject it into browseService, and wire NewMediaServiceWithDeps so
production constructs the resolver explicitly. Tests can now swap in a
fake Resolver without touching disk.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
The scanner (thumbnailForVideo, thumbnailForImage), the service importer
(generateThumbnail), and writeService.RegenerateThumbnail all reinvented
the same ".thumbnails/<base>.jpg" layout inline. Centralise the
convention in internal/thumb/path.go (ThumbnailDir, ThumbnailNameFor,
ThumbnailPathFor + DirName) so changing the on-disk layout is a one-line
change and the three callers cannot drift apart. Pure path math, no I/O —
callers retain their existing MkdirAll mechanism (scanner FS, os.MkdirAll
in service) for testability.
Covered by table-driven unit tests for trailing slashes, missing
extensions, dotfiles, and multi-dot filenames.
|
|
The middleware previously took the full service.AuthService, but only
called three methods on it. Define a narrow api.Authenticator interface
(AuthenticateBearer, CountUsers, GetUserByID) and inject that instead,
fixing the ISP violation. service.AuthService satisfies the new
interface structurally, so callers and tests need no changes.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
The serveFileResult handler used to fabricate a default MediaStreamer at
request time when s.streamer was nil. That silently masked wiring
mistakes and violated the Dependency Inversion Principle — the handler
was inventing its own dependency instead of demanding one from the
caller. Removed the fallback so the handler now uses s.streamer directly.
Construction-time validation of deps.MediaStreamer was added to
api.NewServerWithLogger in commit 4215db6, which makes the runtime nil
case unreachable. Mirrors the explicit-deps pattern set in commits
622827c (http.Client in podcast service) and 92edb83 (TokenManager in
auth service).
Test helpers updated to inject a service.NewMediaStreamer(nil) when one
isn't otherwise supplied so the existing suite still constructs Servers
through the validating constructor.
Refs agent task 6a.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Wraps the feed checker's HTTP GET in a bounded retry loop (3 attempts,
exponential 500ms->1s->2s) and adds a per-host failure tracker so a
flaky host is skipped for 5 minutes instead of being hammered on every
scheduled tick. Skipped fetches still bump the existing
consecutive_failures counter so feed-level scheduling continues to
honour persistent failures.
5xx and transport errors are retried; 4xx is treated as terminal so we
do not retry on 404/410/403. Retry params and host-backoff window are
configurable via a FeedFetchPolicy struct field with exported defaults.
|
|
Previously internal/api/middleware.go contained an isBootstrapPublic
function with a hardcoded switch over public paths plus three /css/ /js/
/images/ prefix checks. Whenever a new public route was added in
server.go a developer also had to remember to extend the switch — easy
to miss, and the symptom is a silent 307 to /bootstrap.html.
Public-route metadata now lives on the Middleware itself:
* Middleware gets publicPaths map[string]bool and publicPrefixes []string
* RegisterPublic(path) / RegisterPublicPrefix(prefix) populate the registry
* BootstrapRedirect consults isPublic(path) instead of a hardcoded list
Server.routes() registers each public route through new helpers
(handlePublic / handlePublicFunc / handlePublicPrefix) so the mux pattern
and the bypass set are declared together — there is no separate whitelist
to keep in sync.
Routes migrated: 14 exact (bootstrap/login {/api,/api/v1/auth} variants,
/healthz, /readyz, /login.html, /bootstrap.html, /favicon.{ico,svg},
/logo.{png,svg}, /manifest.json, /sw.js) and 4 prefixes (/css/, /js/,
/images/, /s/ for tokenised share URLs).
TestMiddleware_BootstrapRedirect now seeds the registry explicitly
(middleware-level unit test, no full server). A new TestServer_PublicRouteRegistry
asserts the full set of public paths is registered after NewServer().
Refs: agent task ha.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
extracted
Two related refactors that converged on internal/api/server.go.
s9 — NewServerWithLogger no longer panics on nil deps.Config. Returns
(*Server, error) so cmd/player/main.go can log a clear message and exit
cleanly when wiring is misconfigured. All callers updated, including
api unit tests that construct a Server directly.
aa — Share-page HTML rendering moved out of handlers_share.go into a
new internal/web package. internal/web/sharepage.go owns
SharePageRenderer and the private injectShareMedia helper; the api
handler now calls s.shareRenderer.Render(...) and routes errors via
http.Error. injectShareMedia coverage was moved alongside it in
internal/web/sharepage_test.go; the duplicate tests in
handlers_more_test.go were removed (placeholder comment left so the
reference is greppable). Server gained one new field (shareRenderer
*web.SharePageRenderer) and one new constructor line; production
wiring uses deps.StaticFS.
Background: both tasks were initially worked by separate sub-agents
that stopped mid-edit when usage limits hit; their WIP was preserved
in a stash and finalized in this commit. Tests rerun from a clean
state: full Go unit suite green, 25/25 LLM e2e, 22/22 Playwright.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Remove the silent nil-fallback in NewAuthService that fabricated a
default auth.TokenManager when callers passed nil. The service now
panics on nil, forcing callers to inject their own TokenManager and
keeping construction at the composition root.
Production wiring (cmd/player/main.go) already constructs the
TokenManager explicitly via auth.NewTokenManager(); test call sites in
internal/api/handlers_test.go and handlers_podcast_test.go that used to
pass nil now pass auth.NewTokenManager() to honour the new contract.
Mirrors the pattern set in commit 622827c (http.Client in podcast
service). Refs task 7a.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
cmd/player/main.go had recoverBackgroundWorkerPanic and
internal/service/recover.go had handleWorkerPanic implementing the same
log-and-stack panic recovery. Export the service variant as RecoverWorker
(unchanged signature), use it from main.go, and drop the duplicate
helper plus the now-unused runtime/debug import.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Adds `var _ Interface = (*concrete)(nil)` assertions so the build
breaks if the concrete type drifts from its interface. Only
adminService and mediaStreamer have matching interfaces; scanService
and GCWorker have no interface defined in the package, so no
assertion was added for them.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
probeWorkerLoop was passing the parent ctx into probeFile, which in turn
passed it to prober.Probe (ffprobe) and thumbGen.Generate (ffmpeg). When
scanCtx was cancelled — because another worker failed or TriggerRescan
restarted the scan — those subprocesses kept running until the parent
ctx was cancelled. Switching to scanCtx makes them stop promptly.
Kept writerLoop's store.CreateMedia on the parent ctx: the scanCtx.Err()
guard at the top of the loop already prevents new writes after cancel,
and any in-flight CreateMedia should complete so the DB stays consistent
with what was probed.
|
|
A crypto/rand.Read failure is a process-level emergency, but the previous
implementation aborted the entire server. Propagate the error through
the TokenManager interface so callers (CreateAPIToken) can surface it
via the normal HTTP 500 path instead of crashing.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Remove the silent nil-fallback in NewPodcastService /
NewPodcastServiceWithLogger that fabricated a default http.Client when
callers passed nil. The service now panics on nil, forcing callers to
inject their own client and own the HTTP timeout / transport policy
explicitly. DefaultHTTPClientTimeout remains exported so production
wiring (cmd/player/main.go) keeps a sensible default at the composition
root, not buried in the service.
Updated TestPodcastService_NilHTTPClient_Defaults to
TestPodcastService_NilHTTPClient_Panics to document the new contract.
All production call sites already pass a non-nil client.
Refs task 8a.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Change fmt.Errorf calls at streamer.go:30 and :36 from %v to %w
for the underlying err so callers can use errors.Is/errors.As to
inspect the wrapped OS error in addition to ErrNotFound.
Go 1.20+ (project is on 1.25) supports multiple %w verbs in a
single fmt.Errorf, so both ErrNotFound and the underlying error
are now part of the error chain.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Replace time.After(delay) with time.NewTimer inside the retry backoff
select in FFProber.Probe. time.After leaks the underlying timer until
it fires, so when ctx.Done() wins the select the timer would otherwise
linger for up to 30s. NewTimer + Stop() on the ctx.Done branch lets the
runtime release it immediately.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
1. tagService.AssignTag and RemoveTag now use verifyModifyAccess
(owner role required) instead of verifyAccess. Tags are global
state visible to every user with access to a media item, so a
viewer must not be able to add or remove them. Favorites and
notes stay on verifyAccess because they're per-user data
(favorites.user_id, media_notes.user_id) and don't affect anyone
else. Verified via curl: viewer POST /media/{id}/tags now 403,
admin still 200.
2. serveFileResult now emits a strong ETag header
("<size>-<mtime-nanos>") before calling http.ServeContent. Go's
ServeContent honours If-None-Match when ETag is set, so iOS
audio clients and podcast apps can revalidate cached downloads
with conditional GETs. Verified via curl: ETag present on
/stream; If-None-Match matching the ETag returns 304.
3. MediaFilter gains IncludeDeleted flag; ListMedia skips the
implicit `deleted_at IS NULL` predicate when it is set.
FSScanner.loadExistingMedia now passes IncludeDeleted=true so
the dedup map includes soft-deleted rows. Previously a re-scan
of a soft-deleted file tried to CreateMedia and hit the
UNIQUE(set_id, rel_path) constraint, failing the whole scan and
setting progress.last_error. Now the rescan skips the row
cleanly; soft-delete sticks.
4. FSScanner.reconcileOrphans soft-deletes media rows whose
underlying file disappeared between scans. The scanner used to
only walk files that exist and never compare against the DB,
leaving phantom rows in GET /api/v1/media that 404'd on stream.
Verified via curl: rm /testdata/.../orphan.mp3, rescan, row
now has deleted_at != NULL.
Scenarios updated to lock in the fixed behaviour:
S19 step 15 — viewer tag-add now asserts 403, not 200.
S20 step 14 — asserts ETag is present and If-None-Match → 304.
S24 step 13 — asserts clean rescan (no last_error from UNIQUE).
S24 step 20 — asserts orphan rows are soft-deleted by rescan.
Verified: full Go unit suite passes; 25/25 LLM e2e scenarios;
22/22 Playwright e2e-web.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Bug fix: POST /api/v1/progress and POST /api/v1/progress/batch did not
verify that the supplied media_id belonged to a media row the caller
could see. Two failure modes:
- Missing media_id triggered an FK violation in UpsertProgress, which
fell through handleError to HTTP 500 instead of 404.
- Soft-deleted media_id (row still exists, deleted_at != nil) was
accepted silently with HTTP 200, recording progress on an item the
user could no longer reach.
Both now route through accessHelper.verifyAccess in progressService,
which returns ErrNotFound (404) for missing/soft-deleted rows and
ErrForbidden (403) for unauthorized sets. Verified via curl:
POST /progress media_id=999999999 → 404; POST /progress/batch with a
bad id → 404.
Tests: progress_test.go and no_rows_test.go now seed MediaRepo and
UserRepo so the verifyAccess branch finds a real (admin) caller and a
real media row. All other tests untouched.
S16 covers media list pagination, filtering, sort, and the parser's
intentional fail-open behaviour for malformed limit/offset/type.
S17 covers podcast list endpoints (GET /podcasts, GET /podcasts/{id}/
episodes) and the admin-only subscribe gate. S18 covers single
POST /progress + GET /in-progress, including the new 404 path for
missing/forbidden media. Full LLM e2e suite passes 18/18.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Three more handlers had the same pattern as the RevokeShare bug fixed
in 0848488: the service returned a plain errors.New(...) for an
expected condition, so handleError fell through to its default 500
branch instead of mapping to 404/400.
- browse.GetThumbnail → ErrNotFound (404 instead of 500) when the
media row has no thumbnail path.
- tag.RemoveTag → ErrNotFound (404 instead of 500) when the tag name
does not exist. DELETE /media/{id}/tags/{unknown} now returns 404.
- write.RegenerateSetCover → new sentinel ErrEmptySetForCover, mapped
to 400 in handleError, when the target set contains no media files
eligible to be promoted to a cover.
S15 (auth-boundary negatives) is tightened: the share-revoke step used
to accept {404, 500} as a workaround for the bug; it now requires 404.
A new section C2 locks in the three regressions above so they cannot
silently return 500 again.
Verified end-to-end via curl: DELETE /media/{id}/tags/unknown → 404,
DELETE /shares/unknown-token → 404. Full LLM e2e suite 15/15 still
passes against the rebuilt server.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
scenarios
Bug fix: shareService.RevokeShare returned errors.New("share not found")
instead of the sentinel ErrShareNotFound, so handleError fell through to
HTTP 500 instead of 404 for DELETE /api/v1/shares/{unknown-token}. The
sister method ValidateShareToken already used the sentinel; this aligns
them. Verified with curl: DELETE on a missing token now returns 404.
The bug surfaced while drafting S15 (auth-boundary negative tests),
which exercises 401 unauthenticated, 403 non-admin → admin routes, and
404 unknown resources. S14 covers admin rescan + scan-progress polling.
All 15 LLM e2e scenarios pass against the fixed server.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
