[rianjs]

Closes #11

What changed

• add sptfy tracks get, albums get, and artists get
• accept strict raw Spotify IDs, matching Spotify URIs, and canonical open.spotify.com URLs
• render cli-common single-resource text blocks with album/artist breadcrumb IDs
• support --id, --fields, --extended, and --include-artwork
• remove the now-deprecated artist GENRES output and avoid deprecated batch endpoints
• extend the behavioral spec, README, and hermetic/live smoke coverage

Security and design

References are parsed locally and validated before credential/session access. Provider requests use only fixed single-resource paths; supplied URLs are never followed. The command package owns a narrow injected session interface, while concrete construction remains in the root.

Verification

• make check — passed, including lint, normal/no-1Password/static tests, and hermetic live harness
• make snapshot — passed for macOS, Linux, Windows, Homebrew, deb/rpm, and winget rendering
• live encrypted-file credential smoke — passed for track, album, and artist reads across raw ID, URI, and canonical URL forms using only user-read-private

Spotify reference confirmation: current single-resource APIs remain supported, while batch metadata endpoints and artist genres are deprecated.

[rianjs]

Architect PR review — approved

Reviewed exact head ff8f25f4c1e6a3de149253f4cb98950a4c2e30f4 against #11 and cli-common.

• composition root owns the concrete authenticated session and injects a command-owned narrow interface
• reference parsing is local, standard-library-only, typed, and occurs before credential/session access
• provider methods validate canonical IDs at their own boundary and call only fixed single-resource paths
• output reuses the established projections/sanitization and carries track → album/artist plus album → artist breadcrumbs
• no batch abstraction, URL following, new dependency, or speculative cache was introduced
• deprecated artist genres and batch metadata behavior are not carried forward

Verification: make check, make snapshot, and live encrypted-file smokes across all three reference forms passed.

[rianjs]

Independent TDD review — findings resolved

Terra reviewed ff8f25f and found two actionable gaps:

1. empty ? / # delimiters could bypass canonical-URL rejection
2. album/artist detail flag and sanitization coverage was thinner than track coverage

Resolved at 35896ab:

• reject any raw query/fragment delimiter before session access, including empty delimiters
• add regression cases for both forms
• add exact album/artist --fields, --extended, --include-artwork, and sanitized detail-output tests
• rerun make check successfully with zero lint findings and all build/test variants green

[rianjs-bot]

Automated PR Review

Reviewed commit: 35896ab1e68e
Profile: codex-rianjs-bot - Posting as: rianjs-bot[bot]

Summary

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

structure:repo-health (1 finding)

Minor - internal/client/client.go:155

Single-resource reads should preserve the request/response identity contract, but all three methods only reject an empty response ID. A malformed or misrouted provider response containing a different non-empty valid ID is rendered as a successful result (and --id emits that different ID), despite the command having requested another resource. Require the decoded ID to equal the requested ID in GetTrack, GetAlbum, and GetArtist, and add mismatched-ID fixtures alongside the existing missing-ID test.

automation:ci-release (1 finding)

Minor - scripts/live-smoke-test.sh:67

The hermetic fake accepts any tracks get, albums get, or artists get invocation and the live script does not inspect their output. Therefore this merge-gate test will still pass if a future edit drops or swaps the intended raw-ID/URI/canonical-URL coverage while retaining any successful get call. Make each fake branch assert its expected reference argument (and ideally make the live script validate the returned resource ID) so the test verifies the advertised catalog-read forms.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/client/client.go, internal/client/client_test.go, internal/cmd/catalogcmd/catalog.go, internal/cmd/catalogcmd/catalog_test.go, internal/cmd/root/root.go, internal/cmd/root/root_test.go, internal/cmd/searchcmd/search_test.go, internal/output/catalog.go, internal/output/catalog_test.go, internal/output/track.go, internal/session/session.go, internal/session/session_test.go, internal/spotifyref/spotifyref.go, internal/spotifyref/spotifyref_test.go	unavailable	Review limited to Go implementation quality and behavioral test coverage. Full test execution is blocked by sandbox restrictions on loopback listeners and the local C toolchain path.
policies:conventions	complete_broad	README.md, internal/cmd/catalogcmd/catalog.go, internal/cmd/root/root.go, scripts/live-smoke-test.sh, scripts/live-smoke.sh, spotify-cli-behavior.md	unavailable	Shared cli-common and .github convenience copies were not present locally; review used the available repo-local development guide and behavior specification.
structure:repo-health	complete_broad	internal/client/client.go, internal/cmd/catalogcmd/catalog.go, internal/cmd/root/root.go, internal/output/catalog.go, internal/session/session.go, internal/spotifyref/spotifyref.go	unavailable	Focused on the assigned changed files and their directly related tests/specification. Targeted Go tests could not run in this sandbox because local listener creation is denied and the toolchain cannot resolve this workspace path.
documentation:docs	complete_broad	README.md, spotify-cli-behavior.md	unavailable	Documentation-only review; targeted Go test invocation was blocked by the local clang toolchain mishandling the workspace path containing spaces.
automation:ci-release	complete_broad	scripts/live-smoke-test.sh, scripts/live-smoke.sh	unavailable	Review limited to the two assigned smoke scripts and their automation contract.

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, documentation:docs, automation:ci-release
Engine	codex_cli · gpt-5.6-terra
Reviewed by	cr · rianjs-bot[bot]
Duration	2m 02s wall · 4m 50s compute
Cost	unavailable
Tokens	962.8k in / 11.7k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	16.6k	500	11.0k	unavailable	unavailable	12s
go:implementation-tests	gpt-5.6-terra	415.6k	3.3k	353.0k	unavailable	unavailable	1m 30s
policies:conventions	gpt-5.6-terra	108.3k	1.4k	76.8k	unavailable	unavailable	30s
structure:repo-health	gpt-5.6-terra	194.5k	2.6k	146.7k	unavailable	unavailable	1m 04s
documentation:docs	gpt-5.6-terra	117.6k	1.5k	90.6k	unavailable	unavailable	47s
automation:ci-release	gpt-5.6-terra	74.3k	1.8k	54.5k	unavailable	unavailable	40s
orchestrator-rollup	gpt-5.6-terra	35.9k	610	27.1k	unavailable	unavailable	5s

[rianjs]

Final architect review — approved

Reviewed exact head cdd5c4602a17678b611bf9c931150d1ec43cc2ad after the independent TDD and automated-review fixes.

• the post-review change is confined to the live smoke and its hermetic fake; it now proves the exact raw-ID, URI, and canonical-URL invocations and validates returned IDs
• the composition root, narrow command-owned session contract, fixed provider paths, local reference parsing, and text projection remain unchanged
• make check passes, and read-only live checks returned the expected track, album, and artist IDs

The automated suggestion to require every response ID to equal the requested ID was intentionally not applied. Spotify's Track Relinking contract permits Get Track to return a substituted playable track with a different ID, with the original exposed through linked_from; the Get Track reference likewise documents replacement based on availability. Enforcing equality would reject valid Spotify responses.

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

[monit-reviewer]

Automated PR Review

Reviewed commit: cdd5c4602a17
Profile: codex-monit-reviewer - Posting as: monit-reviewer

Summary

Reviewer	Findings
go:implementation-tests	0
policies:conventions	0
architecture:solid-reviewer-agnostic	2
structure:repo-health	0

architecture:solid-reviewer-agnostic (2 findings)

Blocking - internal/client/client.go:155

U-L1: GetTrack promises the resource requested by id, but this guard accepts any nonblank response ID; GetAlbum and GetArtist repeat the issue at lines 170 and 185. A misrouted or stale upstream response can therefore make get X successfully render resource Y, including emitting Y under --id. Require each returned ID to equal the requested ID and return ErrInvalidResponse otherwise, with mismatch tests for all three getters.

Minor - internal/cmd/catalogcmd/catalog.go:86

U-L2: this defer discards Session.Close errors, as do the album and artist branches at lines 118 and 149. Because the session retains a credential-store backend and Close explicitly reports failures, cleanup/release failure is silently presented as command success. Propagate and wrap the close error via a named return/helper, or add an explicit comment documenting why close is intentionally best-effort.

Reviewer Coverage

Reviewer	Status	Inspected	Skipped	Constraints
go:implementation-tests	complete_broad	internal/client/client.go, internal/client/client_test.go, internal/cmd/catalogcmd/catalog.go, internal/cmd/catalogcmd/catalog_test.go, internal/cmd/root/root.go, internal/cmd/root/root_test.go, internal/cmd/searchcmd/search_test.go, internal/output/catalog.go, internal/output/catalog_test.go, internal/output/track.go, internal/session/session.go, internal/session/session_test.go, internal/spotifyref/spotifyref.go, internal/spotifyref/spotifyref_test.go	unavailable	Focused Go implementation and behavioral-test review. Full package tests could not run in this sandbox because local listener creation is denied and several CGO builds cannot resolve the workspace path.
policies:conventions	complete_broad	README.md, internal/cmd/catalogcmd/catalog.go, internal/cmd/catalogcmd/catalog_test.go, internal/cmd/root/root.go, internal/cmd/root/root_test.go, scripts/live-smoke-test.sh, scripts/live-smoke.sh, spotify-cli-behavior.md	unavailable	No local checkout of the canonical cli-common shared-conventions docs was available. Targeted Go tests could not build because the sandboxed workspace path contains spaces and the CGO compiler invocation failed; the hermetic live-smoke script passed.
architecture:solid-reviewer-agnostic	complete_broad	internal/client/client.go, internal/cmd/catalogcmd/catalog.go, internal/cmd/root/root.go, internal/output/catalog.go, internal/session/session.go, internal/spotifyref/spotifyref.go	unavailable	Full make check could not complete in the sandbox: macOS CGO preprocessing failed during lint, and broader tests requiring loopback sockets were blocked. Targeted tests for catalogcmd, output, session, and spotifyref passed.; Review was limited to the six assigned changed files, with related tests and project guidance inspected for context.
structure:repo-health	complete_broad	README.md, internal/client/client.go, internal/cmd/catalogcmd/catalog.go, internal/cmd/root/root.go, internal/output/catalog.go, internal/session/session.go, internal/spotifyref/spotifyref.go, scripts/live-smoke-test.sh, scripts/live-smoke.sh, spotify-cli-behavior.md	unavailable	Focused on the assigned structural surfaces; targeted Go tests could not complete because the sandbox forbids listener binding and some cgo build paths.

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

---

Completed in 3m 15s | 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, policies:conventions, architecture:solid-reviewer-agnostic, structure:repo-health
Engine	codex_cli · gpt-5.6-terra, gpt-5.6-sol
Reviewed by	cr · monit-reviewer
Duration	3m 15s wall · 6m 57s compute
Cost	unavailable
Tokens	2.0M in / 15.9k out

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost	Duration
orchestrator-selection	gpt-5.6-terra	19.9k	591	11.0k	unavailable	unavailable	13s
go:implementation-tests	gpt-5.6-terra	458.8k	3.2k	399.4k	unavailable	unavailable	1m 28s
policies:conventions	gpt-5.6-terra	409.7k	2.8k	356.4k	unavailable	unavailable	1m 19s
architecture:solid-reviewer-agnostic	gpt-5.6-sol	733.0k	6.1k	667.1k	unavailable	unavailable	2m 37s
structure:repo-health	gpt-5.6-terra	268.9k	2.3k	215.3k	unavailable	unavailable	1m 04s
orchestrator-rollup	gpt-5.6-terra	72.2k	1.0k	54.5k	unavailable	unavailable	13s

[rianjs]

These are low-value, please approve the PR

[monit-reviewer]

Approving after an explicit PR author override request following a prior codereview pass.