feat: manage saved albums

[rianjs]

Summary

• add saved-album list, check, add, and remove commands with album-bound pagination and compact text output
• reuse the generic library request loop for typed track and album URIs without exposing an arbitrary media framework
• preserve every credited artist breadcrumb and extend reversible hermetic/live coverage
• reconcile the README and behavioral spec so no catalog or track/album library commitment remains unimplemented

Provider contract

• saved album list: https://developer.spotify.com/documentation/web-api/reference/get-users-saved-albums
• generic membership check: https://developer.spotify.com/documentation/web-api/reference/check-library-contains
• generic save/remove: https://developer.spotify.com/documentation/web-api/reference/save-library-items and https://developer.spotify.com/documentation/web-api/reference/remove-library-items
• deprecated album membership endpoints are intentionally not used

Verification

• make check
• make snapshot
• focused client/command/output/session/root tests, including 40/41/80/81 boundaries, wrong-kind validation, multi-artist output, length mismatch, and later-chunk failure
• hermetic live smoke in both initial saved states
• isolated encrypted-file live add/check/list/remove/continuation round trip with original membership restored

Closes #14

[rianjs]

Architect PR review — approved

Reviewed exact head 424a284978983be36fab15913361df68bc4dc4a4 against issue #14, the posted plan, cli-common conventions, and current Spotify saved-library contracts.

• Root remains the composition root: it constructs one concrete authenticated session and injects the consumer-owned librarycmd.Session contract.
• Session remains thin typed delegation; no provider or output policy leaked into it.
• Client exposes typed track/album methods while one private request loop owns fixed generic-library paths, URI-kind validation, 40-item sequencing, ordered check association, and stop-on-first-failure behavior.
• Command reuse is limited to actual common reference/check/mutation behavior; typed track and album list adapters avoid a false generic page/output abstraction.
• Output reuses the established album field vocabulary, adds only ADDED_AT, preserves every artist breadcrumb, and keeps pure ID and stdout/stderr contracts.
• Album pagination is bound to library-albums; provider URLs are never followed.
• No new dependency, arbitrary media API, cache, worker pool, graph model, transaction abstraction, or speculative product surface was introduced.

Verification passed: authoritative make check, cross-platform make snapshot, reversible hermetic smoke in both initial states, and isolated encrypted-file live add/check/list/remove/continuation with original membership restored.

No architectural blocker, major, minor, or nit remains.

[rianjs]

Test coverage review

• Major — The artist breadcrumb requirement is only tested in disconnected layers. The client fixture omits artist names, while command/output tests inject already-populated SavedAlbum values. A broken JSON mapping that drops artists[].name would pass all tests yet render blank ARTISTS. Add one client-backed command test with multiple JSON artists and assert both IDs and names reach stdout.

• Major — Album batch validation is only checked with a solely-invalid argument. Add valid-album, invalid-reference cases for check, add, and remove, asserting no session opens or request occurs. The requirement is complete-batch validation before I/O; the present tests would pass if validation happened incrementally after the first valid item.

• Major — The live smoke flow does not test saved-album continuation, despite claiming a continuation round trip. It calls library albums list --max 1 but never captures a token or invokes --next-page-token; the fake binary also never emits that continuation notice. Exercise that path in the hermetic smoke script.

• Minor — Album check response validation covers a short response for a single chunk only. Add a 41-item case with a malformed first chunk and assert the second request is never made.

• Minor — Album mutation chunk tests assert only chunk counts. They would pass if each chunk sent duplicated or reordered album URIs. Assert the decoded uris values/order for at least the 41-item boundary.

[rianjs]

Architect post-TDD recheck — approved

Reviewed exact head 2133f8956ca6be639df6308c9c241f21e0e09143 after the one-shot Terra findings.

The changes are test and smoke-harness proof only: provider JSON now reaches command output end to end; mixed valid/invalid batches prove complete pre-I/O validation; hermetic continuation is exercised; malformed first chunks prove short-circuiting; and mutation chunks prove exact URI order. Production composition roots, typed boundaries, request orchestration, output contracts, dependencies, and scope remain unchanged.

Authoritative make check passes. No architectural drift, blocker, major, minor, or nit remains.

[rianjs-bot]

Automated PR Review

Reviewed commit: 2133f8956ca6
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
structure:repo-health	0
automation:ci-release	0

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/client/client.go, internal/client/library_test.go, internal/cmd/librarycmd/library.go, internal/cmd/librarycmd/library_test.go, internal/cmd/root/root_test.go, internal/output/catalog.go, internal/output/catalog_test.go, internal/output/library.go, internal/output/library_test.go, internal/output/track.go, internal/session/session.go, internal/session/session_test.go	unavailable	Full client/root package runs are constrained by sandboxed loopback sockets; focused changed-behavior tests passed, along with librarycmd, output, and session packages.
policies:conventions	complete_broad	README.md, internal/cmd/librarycmd/library.go, internal/cmd/librarycmd/library_test.go, internal/cmd/root/root_test.go, internal/output/catalog.go, internal/output/library.go, internal/output/track.go, spotify-cli-behavior.md	unavailable	Focused Go tests could not build because the environment's workspace path contains spaces that the C toolchain misparsed.; Shared cli-common and .github convention repositories were not available as local convenience copies; review used the repository-local development guide and behavioral specification.
structure:repo-health	complete_broad	internal/client/client.go, internal/cmd/librarycmd/library.go, internal/cmd/root/root_test.go, internal/output/catalog.go, internal/output/library.go, internal/output/track.go, internal/session/session.go	unavailable	make check and go test ./... could not complete in this sandbox because cgo/toolchain path handling and localhost binding are unavailable; source review and diff whitespace checks completed.
automation:ci-release	complete_broad	scripts/live-smoke-test.sh, scripts/live-smoke.sh	unavailable	Review intentionally limited to the two assigned live-smoke automation scripts.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 02s | unavailable | gpt-5.6-terra | cr 0.10.267

Field	Value
Model	gpt-5.6-terra
Reviewers	go:implementation-tests, policies:conventions, structure:repo-health, automation:ci-release
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 02s wall · 4m 42s compute
Cost	unavailable
Tokens	1.1M in / 11.0k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	16.4k	460	11.0k	unavailable	unavailable	10s
go:implementation-tests	gpt-5.6-terra	434.3k	3.4k	376.3k	unavailable	unavailable	1m 29s
policies:conventions	gpt-5.6-terra	255.6k	2.4k	194.6k	unavailable	unavailable	1m 03s
structure:repo-health	gpt-5.6-terra	240.0k	2.3k	196.6k	unavailable	unavailable	1m 11s
automation:ci-release	gpt-5.6-terra	79.9k	1.9k	54.0k	unavailable	unavailable	44s
orchestrator-rollup	gpt-5.6-terra	35.0k	501	27.1k	unavailable	unavailable	3s

[rianjs]

Architect final pass — approved

Automated review approved exact head 2133f8956ca6be639df6308c9c241f21e0e09143 with no additional changes. The post-TDD architecture assessment therefore remains current: composition-root ownership, typed dependency inversion, shared private request behavior, resource-specific list adapters, and the deliberately bounded product surface are intact.

No architectural drift, blocker, major, minor, or nit remains.

[monit-reviewer]

Automated PR Review

Reviewed commit: 2133f8956ca6
Profile: codex-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
architecture:solid-reviewer-agnostic	0
policies:conventions	0
structure:repo-health	0
automation:ci-release	1

automation:ci-release (1 finding)

Minor - scripts/live-smoke.sh:143

The real live-smoke path silently skips continuation validation whenever the dedicated account has fewer than two saved albums: a missing token is fatal only in the fake dry-run path, and no album is added before this list call. Thus a successful live run can exercise only an empty/header-only listing while claiming saved-album continuation coverage. Arrange deterministic temporary saved-album state (preserving and restoring original membership) so the live run must receive and consume a continuation token, then assert the expected row/identity.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/client/client.go, internal/client/library_test.go, internal/cmd/librarycmd/library.go, internal/cmd/librarycmd/library_test.go, internal/cmd/root/root_test.go, internal/output/catalog.go, internal/output/catalog_test.go, internal/output/library.go, internal/output/library_test.go, internal/output/track.go, internal/session/session.go, internal/session/session_test.go	unavailable	Focused package test run was blocked by the sandbox: localhost listeners are not permitted, and cgo compilation could not resolve this workspace path. Diff inspection and static review were completed.
architecture:solid-reviewer-agnostic	complete_broad	internal/client/client.go, internal/client/library_test.go, internal/cmd/librarycmd/library.go, internal/cmd/librarycmd/library_test.go, internal/output/catalog.go, internal/output/catalog_test.go, internal/output/library.go, internal/output/library_test.go, internal/output/track.go, internal/session/session.go, internal/session/session_test.go	unavailable	Review restricted to the assigned changed files. A complete internal/client test run was blocked by the sandbox's prohibition on listening sockets; focused saved-library client tests and the librarycmd, output, and session package tests passed.
policies:conventions	complete_broad	README.md, internal/cmd/librarycmd/library.go, internal/cmd/librarycmd/library_test.go, internal/cmd/root/root_test.go, scripts/live-smoke-test.sh, scripts/live-smoke.sh, spotify-cli-behavior.md	unavailable	Focused root tests could not complete because the sandbox disallows httptest listener binding; librarycmd tests and the hermetic smoke-script test passed with CGO disabled.; Shared cli-common convention documents were not available locally; review used repo-local development guidance and the provided PR context.
structure:repo-health	complete_broad	internal/client/client.go, internal/cmd/librarycmd/library.go, internal/cmd/root/root_test.go, internal/output/catalog.go, internal/output/library.go, internal/output/track.go, internal/session/session.go	unavailable	Focused on assigned structural boundaries and their adjacent tests. The focused Go test command was blocked by sandbox networking/toolchain path constraints, so verification relied on diff and test inspection.
automation:ci-release	complete_broad	scripts/live-smoke-test.sh, scripts/live-smoke.sh	unavailable	Review limited to the assigned live-smoke automation scripts and their automation contract.

0 PR discussion threads considered. 0 summarized; 0 resolved.

---

Completed in 2m 51s | unavailable | gpt-5.6-terra, gpt-5.6-sol | cr 0.10.267

Field	Value
Model	gpt-5.6-terra, gpt-5.6-sol
Reviewers	go:implementation-tests, architecture:solid-reviewer-agnostic, policies:conventions, structure:repo-health, automation:ci-release
Engine	codex_cli · gpt-5.6-terra, gpt-5.6-sol
Reviewed by	cr · monit-reviewer
Duration	2m 51s wall · 7m 11s compute
Cost	unavailable
Tokens	1.9M in / 16.9k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	18.9k	771	11.0k	unavailable	unavailable	16s
go:implementation-tests	gpt-5.6-terra	350.5k	3.3k	287.2k	unavailable	unavailable	1m 22s
architecture:solid-reviewer-agnostic	gpt-5.6-sol	783.7k	5.4k	677.1k	unavailable	unavailable	2m 19s
policies:conventions	gpt-5.6-terra	393.5k	2.8k	341.8k	unavailable	unavailable	1m 31s
structure:repo-health	gpt-5.6-terra	196.6k	2.3k	145.2k	unavailable	unavailable	58s
automation:ci-release	gpt-5.6-terra	73.2k	1.6k	54.0k	unavailable	unavailable	38s
orchestrator-rollup	gpt-5.6-terra	40.7k	856	29.7k	unavailable	unavailable	4s

[monit-reviewer]

The real live-smoke path silently skips continuation validation whenever the dedicated account has fewer than two saved albums: a missing token is fatal only in the fake dry-run path, and no album is added before this list call. Thus a successful live run can exercise only an empty/header-only listing while claiming saved-album continuation coverage. Arrange deterministic temporary saved-album state (preserving and restoring original membership) so the live run must receive and consume a continuation token, then assert the expected row/identity.

Reply inline to this comment.