feat(auth): add Spotify OAuth PKCE

[roboclaw-bot]

Summary

• add Spotify Authorization Code with PKCE as an opt-in official Web API authentication path
• refresh and securely persist per-profile OAuth tokens with cross-process locking
• serialize OAuth login and clear as one profile-scoped lifecycle transition, reloading profile state after lock acquisition
• preserve cookie authentication as the default and keep Connect cookie-backed
• make OAuth clearing failure-safe by persisting the cookie fallback before deleting the token cache
• rebase the original feature commit from feat(auth): add Spotify OAuth PKCE #57 onto current main and drop the search fix already landed in fix(search): decode Spotify plural result containers #59

This replaces #57 because the managed maintainer identity could not write to the contributor fork. The original OAuth feature commit retains VACInc's authorship; the state-consistency repairs are separate commits.

Product decision

Maintainer direction on 2026-09-02 accepted this bounded OAuth mode for spogo's purpose: it is optional, local-only, limited to the existing Web API surface, and does not change the cookie-first default or make Connect depend on a hosted credential service. This satisfies the explicit product-decision requirement for new persistent credential flows in VISION.md.

Security

• uses PKCE with a cryptographically random verifier and state
• binds callbacks only to explicit IPv4 or IPv6 loopback addresses and validates callback host, method, path, and state
• never accepts or stores a Spotify client secret
• stores token caches and lifecycle locks in a 0700 directory with 0600 files on POSIX systems
• coordinates concurrent token readers and refreshes with a per-profile cache lock
• coordinates login token/profile commits and clear profile/token commits with a shared per-profile lifecycle lock
• reloads the persisted profile while holding the lifecycle lock so stale command contexts cannot recreate an inconsistent state
• uses same-directory atomic replacement on POSIX and MoveFileEx replacement with write-through on Windows
• never emits access or refresh token values in status output
• does not follow redirects from Spotify's token endpoint

Compatibility

Cookie authentication remains the default. OAuth selects the token provider for public Web API operations; Spotify Connect and internal endpoints still require browser cookies. Existing command names, output formats, and cookie-auth profiles remain unchanged.

Verification

Validated on exact head 3d11f9762853b0e7639aedc3d8ddb594d3faa200:

• go test ./...
• go test -race ./...
• focused regressions repeated 20 times under the race detector:
  • login pauses after saving a new token while a stale-profile clear starts concurrently
  • clear remains blocked until login commits the profile, reloads that committed profile, restores cookie auth, and removes the token
  • a failed profile write leaves OAuth selected and preserves the token cache for a safe retry
• go run golang.org/x/tools/cmd/deadcode@v0.49.0 -test ./...
• golangci-lint v2.13.2 run (0 issues)
• ./scripts/check-coverage.sh 90 (90.0%)
• node scripts/build-docs-site.mjs
• VCS-stamped go build ./cmd/spogo

Built-CLI OAuth authority proof

The built CLI was run against its normal loopback callback and Spotify's real accounts token endpoint with dummy public values only:

• a callback with an invalid state returned HTTP 400; the CLI remained waiting, proving no token exchange was triggered
• the same run then received a callback with the generated state, returned HTTP 200, and reached Spotify's real token endpoint
• Spotify rejected the dummy exchange as expected and the CLI exited 3; no credential or real token was used, printed, or persisted

A successful authorization, persisted real token, and authenticated Web API request still require a human Spotify sign-in and remain the sole unfulfilled live-proof item.

Worked on by

• @VACInc

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 2, 2026, 7:18 PM ET / 23:18 UTC.

ClawSweeper review

What this changes

The PR adds opt-in Spotify Authorization Code with PKCE authentication for Web API requests, with per-profile token storage, refresh, CLI commands, tests, and documentation while retaining cookie authentication by default.

Merge readiness

⛔ Blocked before merge - 6 items remain

Keep open: the earlier lifecycle-race finding is addressed on the current head, but this new persistent OAuth credential flow still needs attributable maintainer confirmation and successful real OAuth/Web API proof before merge.

Priority: P2
Reviewed head: 3d11f9762853b0e7639aedc3d8ddb594d3faa200
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The implementation and focused lifecycle repair are substantial, but merge readiness is limited by missing successful real OAuth proof and an unverified product decision record.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: the OAuth callback produces a persisted token that the Web API client later consumes. The supplied narrative covers a rejected dummy exchange but provides no captured after-fix successful login, persisted-token, authenticated-request result, or final-effect proof for the rejected callback; add redacted terminal/live output and redact private endpoints, IPs, and credentials. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: the OAuth callback produces a persisted token that the Web API client later consumes. The supplied narrative covers a rejected dummy exchange but provides no captured after-fix successful login, persisted-token, authenticated-request result, or final-effect proof for the rejected callback; add redacted terminal/live output and redact private endpoints, IPs, and credentials. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	5 items	Persistent credential policy: The repository vision requires an explicit product decision and an end-to-end verification path for credential flows and user-visible changes. Prior concurrency finding addressed: Both OAuth login and clear now acquire the profile lifecycle lock and reload persisted profile state before committing their profile/token transitions. OAuth authority path: The new provider reads a per-profile token, refreshes it when needed, and returns the resulting bearer token to the Web API client; cache writes are protected by a file lock.
Findings	None	None.
Security	None	None.

How this fits together

Spogo is a script-first Spotify CLI whose commands select Connect or the public Web API. This change adds a local OAuth token provider for Web API calls, while Connect continues to obtain its session from browser cookies.

flowchart LR
  Command[CLI command] --> Choice[Engine and auth selection]
  Choice --> Cookies[Browser cookie provider]
  Choice --> Login[OAuth loopback login]
  Login --> Cache[Per-profile token cache]
  Cookies --> Web[Web API client]
  Cache --> Web
  Web --> Spotify[Spotify Web API]
  Choice --> Connect[Cookie-backed Connect protocol]

Loading

Decision needed

Question	Recommendation
Should spogo accept this optional persistent OAuth credential flow as part of its supported CLI contract?	Sponsor the bounded OAuth mode: Confirm the local-only, Web-API-only OAuth contract and require the remaining real behavior proof before merge.

Why: VISION.md requires an explicit product decision for credential flows, and the supplied snapshot has an author assertion of approval but no attributable maintainer record to audit.

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the OAuth callback produces a persisted token that the Web API client later consumes. The supplied narrative covers a rejected dummy exchange but provides no captured after-fix successful login, persisted-token, authenticated-request result, or final-effect proof for the rejected callback; add redacted terminal/live output and redact private endpoints, IPs, and credentials. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve merge risk (P1) - The review snapshot asserts prior product acceptance but contains no attributable maintainer decision for the new persistent credential flow required by VISION.md.
• Resolve merge risk (P1) - Successful OAuth authorization and use of the persisted credential for an authenticated Web API call have not been demonstrated on the built CLI.
• Resolve merge risk (P1) - The new OAuth authority path persists refresh credentials and sends bearer tokens to Spotify; the supplied evidence does not prove the allowed and rejected callback paths through their final effects.
• Complete next step (P2) - Obtain maintainer confirmation of the OAuth credential contract and add redacted built-CLI proof of successful login plus an authenticated Web API request, including invalid-state no-exchange evidence.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Changed surface	31 files, +2,824/-156 lines	The feature spans CLI behavior, token persistence, engine wiring, tests, and user documentation.
Production versus tests	production/docs +1,313, tests +1,511	The substantial test addition supports the new lifecycle and token-provider behavior, but does not replace live OAuth proof.

Root-cause cluster

Relationship: canonical
Canonical: #61
Summary: This clean, updated PR is the intended landing candidate for the OAuth work formerly proposed in the older conflicting branch.

Members:

• superseded: feat(auth): add Spotify OAuth PKCE #57 - This PR retains the OAuth feature work while adding the lifecycle repairs and rebasing it onto current main; the older branch is conflict-marked in the supplied context.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Confirm contract and prove the real flow (recommended)
   Record maintainer approval, then add redacted built-CLI evidence of successful OAuth login, authenticated Web API use, and invalid-state rejection before merge.
2. Defer OAuth support
   Pause or close the PR if maintainers do not want a persistent OAuth credential lifecycle in the supported CLI contract.

Technical review

Best possible solution:

Land a narrowly approved, cookie-default OAuth mode only after redacted built-CLI evidence shows successful login, token persistence, one authenticated Web API request, and rejected-state behavior without token exchange.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR introduces a new OAuth capability rather than reporting an existing-behavior defect. The supplied tests exercise simulated OAuth paths, but no successful real account flow is evidenced.

Is this the best way to solve the issue?

Unclear: the implementation preserves cookie defaults and scopes OAuth to the Web API, but it is the best solution only if a maintainer explicitly accepts the new credential contract and real built-CLI proof validates it.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 87a990da7e29.

Labels

Label justifications:

• P2: This is a bounded new authentication capability with no demonstrated current-user outage.
• merge-risk: 🚨 auth-provider: Merging adds a new OAuth provider, stored refresh credentials, and Web API token-selection behavior.
• merge-risk: 🚨 security-boundary: The change accepts OAuth callback authority and persists credentials, while final-effect proof remains incomplete.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the OAuth callback produces a persisted token that the Web API client later consumes. The supplied narrative covers a rejected dummy exchange but provides no captured after-fix successful login, persisted-token, authenticated-request result, or final-effect proof for the rejected callback; add redacted terminal/live output and redact private endpoints, IPs, and credentials. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Persistent credential policy: The repository VISION.md requires an explicit product decision and an end-to-end verification path for credential flows and user-visible changes. (VISION.md:47, 3d11f9762853)
• Prior concurrency finding addressed: Both OAuth login and clear now acquire the profile lifecycle lock and reload persisted profile state before committing their profile/token transitions. (internal/cli/auth_oauth.go:171, 3d11f9762853)
• OAuth authority path: The new provider reads a per-profile token, refreshes it when needed, and returns the resulting bearer token to the Web API client; cache writes are protected by a file lock. (internal/spotify/oauth.go:123, 3d11f9762853)
• Current-head provenance: The latest PR commit is the lifecycle-transition repair and has the prior reviewed head as its recorded parent. (internal/cli/auth_oauth.go:171, 3d11f9762853)
• Real-proof gap: The PR body describes a built-CLI callback run ending in an expected dummy-token rejection; it does not provide captured evidence of a successful authorization, persisted token, and authenticated Web API request. (3d11f9762853)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• VACInc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Obtain attributable maintainer confirmation of the persistent OAuth contract.
• Add redacted built-CLI proof of successful OAuth login followed by one authenticated --engine web --auth oauth request.
• Add final-effect proof that an invalid callback state reaches no token exchange.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-02T22:26:13.578Z sha 0ba7ca7 :: needs real behavior proof before merge. :: [P2] Serialize OAuth login and clear state changes

[steipete]

Closing this repair PR as superseded by #57. The current #57 head,
62f34e8, already contains the failure-safe
clear and serialized OAuth lifecycle repairs from this PR, preserves
@VACInc's contribution, and adds shared-config locking, profile-path
containment, and successful real-account proof.

The OAuth lifecycle implementation is identical between the two heads;
#57 also validates callback port ranges. The remaining product decision
and maintainer-owned repairs are tracked in #57. Thank you for the repair
work.