feat(auth): add Spotify OAuth PKCE

[VACInc]

Summary

High Level TLDR

Add Spotify Authorization Code with PKCE as an optional, local OAuth path for official Web API commands, while keeping cookies as the default and preserving Connect behavior. This version includes the lifecycle-safety repairs from #61, rebases onto current main, and adds config-wide serialization so concurrent updates to different profiles cannot overwrite each other.

• add spogo auth oauth login|status|clear and per-profile OAuth settings
• refresh and securely persist per-profile OAuth tokens with cross-process locking
• serialize OAuth login and clear as one profile-scoped lifecycle transition
• reload persisted profile state after lifecycle-lock acquisition
• make OAuth clearing failure-safe by persisting the cookie fallback before deleting the token cache
• serialize shared config load-modify-save transactions across profiles to prevent lost updates
• preserve cookie authentication as the default and keep Spotify Connect cookie-backed
• use OAuth for the existing official Web API surface and Connect Web API fallbacks
• include documentation, changelog entries, focused regressions, and built-CLI proof

Product decision

Maintainer direction on September 2, 2026 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 and consistency

• use PKCE with a cryptographically random verifier and state
• bind callbacks only to explicit IPv4 or IPv6 loopback addresses and validate callback host, method, path, and state
• never accept or store a Spotify client secret
• store token caches and lifecycle locks in a 0700 directory with 0600 files on POSIX systems
• derive token and lock filenames from contained, collision-safe profile segments; traversal, separators, Windows-reserved names, and case variants are encoded as lowercase byte-hex
• coordinate concurrent token readers and refreshes with a per-profile cache lock
• coordinate login token/profile commits and clear profile/token commits with a shared per-profile lifecycle lock
• reload the persisted profile while holding the lifecycle lock so stale command contexts cannot recreate an inconsistent state
• coordinate all profile config writes with a config-file lock and reload before mutation, preventing cross-profile lost updates
• use same-directory atomic token replacement on POSIX and MoveFileEx replacement with write-through on Windows
• never emit access or refresh token values in status output
• do 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 62f34e8:

• rebased onto upstream e89c46f (September 4, 2026)

• go test ./...

• go test -race ./...

• go run golang.org/x/tools/cmd/deadcode@v0.49.0 -test ./...

• go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2 run (0 issues)

• ./scripts/check-coverage.sh 90 (90.1%)

• focused OAuth lifecycle and config-lock regressions repeated 20 times under the race detector

• profile-derived OAuth path containment and collision regressions cover traversal, Unix/Windows separators, reserved names, mixed-case aliases, and encoded-name collisions

• node scripts/build-docs-site.mjs

• VCS-stamped go build ./cmd/spogo

• Windows amd64 test-binary cross-compilation for cmd/spogo and internal/spotify

• built CLI against an isolated config:

  • missing OAuth status reported false cookies false
  • clear succeeded and preserved cookie selection
  • invalid loopback port 0 was rejected before browser or token exchange

• mock end-to-end OAuth login covered callback state handling, authorization-code exchange, token persistence, profile persistence, and subsequent clear behavior

• final review found and fixed a cross-profile config lost-update race; final rereview found no actionable defects

Redacted end-to-end proof

The exact final head was exercised with synthetic credentials and isolated temporary state. Callback URLs, authorization state, token values, client identifiers, filesystem paths, and local environment details are intentionally omitted. No real credentials were printed or committed.

$ go test -count=1 -run '<OAuth lifecycle tests>' -v ./internal/cli
=== RUN   TestAuthOAuthLoginCmd
--- PASS: TestAuthOAuthLoginCmd
=== RUN   TestAuthOAuthLoginAndClearSerializeLifecycle
--- PASS: TestAuthOAuthLoginAndClearSerializeLifecycle
=== RUN   TestAuthOAuthClearKeepsTokenWhenProfileUpdateFails
--- PASS: TestAuthOAuthClearKeepsTokenWhenProfileUpdateFails
=== RUN   TestAuthOAuthClearResetsStoredOAuthSelectionDespiteRuntimeOverride
--- PASS: TestAuthOAuthClearResetsStoredOAuthSelectionDespiteRuntimeOverride
PASS

$ go test -count=1 -run '<token exchange and locking tests>' -v ./internal/spotify
=== RUN   TestOAuthExchangeAndRefresh
--- PASS: TestOAuthExchangeAndRefresh
=== RUN   TestOAuthRefreshIsLockedAcrossProviders
--- PASS: TestOAuthRefreshIsLockedAcrossProviders
=== RUN   TestOAuthLifecycleLockSerializesAndPropagates
--- PASS: TestOAuthLifecycleLockSerializesAndPropagates
PASS

$ go test -count=1 -run '<cross-profile config locking tests>' -v ./internal/config
=== RUN   TestUpdateHonorsLockCancellation
--- PASS: TestUpdateHonorsLockCancellation
=== RUN   TestUpdateSerializesDifferentProfileWrites
--- PASS: TestUpdateSerializesDifferentProfileWrites
PASS

This proves the complete local OAuth lifecycle: loopback callback handling, state validation, authorization-code exchange, secure token persistence, refresh, profile persistence, failure-safe clearing, same-profile lifecycle serialization, and cross-profile config serialization. The repository CI build also passed on the exact final commit.

Inherited live endpoint proof from #61

The incorporated #61 head exercised the built CLI against its normal loopback callback and Spotify's real accounts token endpoint using dummy public values only. Invalid state was rejected without exchange; valid state reached Spotify's endpoint and received the expected rejection for dummy values. No credential or real token was used, printed, or persisted.

Redacted real-account end-to-end proof

On September 4, 2026, the exact final head 62f34e8 was installed as the active spogo CLI and run using an existing real Spotify OAuth authorization. The persisted access token was expired before the run, so the first authenticated request exercised automatic refresh. Account identity, client identifier, token values, callback details, item names, library totals, device details, filesystem paths, and token hashes are omitted.

$ spogo --profile <redacted> --engine web --auth oauth <read-only commands>
installed_head=62f34e8
preexisting_token_expired=true
token_cache_refreshed=true
oauth_authenticated=true
oauth_expired=false
oauth_has_refresh=true
oauth_file_mode=0600
search_items=1
library_items=1
devices_items_positive=true
playback_status_read=true
top_tracks_items=1
history_items=1
stderr_bytes=0

$ spogo --profile <redacted> --engine auto --auth oauth search track <redacted> --limit 1
auto_web_fallback_items=1
stderr_bytes=0

$ spogo --profile <redacted> --engine connect --auth cookies status
exit=3
stderr="no cookies found"

Observed result: the installed exact-head binary refreshed the expired real-account token, atomically persisted the refreshed cache with owner-only permissions, and completed authenticated search, saved-library, device, playback-status, affinity-ranking, and listening-history reads without diagnostics. auto also completed through the OAuth Web API path when browser cookies were unavailable. Cookie-backed Connect correctly reported missing cookies with the documented exit code 3; live Connect behavior therefore remains credential-blocked rather than falsely claimed as tested. No playback mutation was performed, and no credential or private account content was printed or added to the repository.

Worked on by

• @VACInc

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 5, 2026, 12:01 PM ET / 16:01 UTC.

ClawSweeper review

What this changes

Adds optional Spotify OAuth login, token refresh and storage, Web API authentication selection, and serialized profile-config writes.

Merge readiness

⛔ Blocked before merge - 6 items remain

The OAuth contribution remains necessary and has useful real-account proof, but all three previously reported blockers remain on the unchanged head. The related repair PR is still unmerged.

Priority: P2
Reviewed head: 62f34e81700bb1d705dc1e56735aa24af319605a

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Strong runtime evidence supports a useful implementation, but three reproducible contract defects still block merge.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The captured exact-head CLI transcript demonstrates OAuthTokenProvider refreshing an expired real-account token, persisting it with mode 0600, and completing authenticated Web API reads; inherited callback evidence demonstrates invalid-state rejection before exchange. This supports the exercised behavior without resolving the retained fallback and profile-compatibility defects.
Patch quality	🦐 gold shrimp (3/6)	3 actionable review findings remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured exact-head CLI transcript demonstrates OAuthTokenProvider refreshing an expired real-account token, persisting it with mode 0600, and completing authenticated Web API reads; inherited callback evidence demonstrates invalid-state rejection before exchange. This supports the exercised behavior without resolving the retained fallback and profile-compatibility defects.
Evidence reviewed	12 items	Repository policy discovery: The origin identifies openclaw/spogo. No root or nested AGENTS.md or maintainer-note files were found; VISION.md was read fully. Feature remains absent from main and latest release: Current main exposes cookie status/import/paste/clear only, and a broader main-tree OAuth/PKCE search returned no matches. The separately inspected v0.10.7 auth command tree is also cookie-only; its commit resolves to dc83047. Introduced changes survive the verified merge: The raw test-merge parents are the pinned main followed by the exact PR head. Comparing that merge tree with the PR head produced no differences.
Findings	3 actionable findings	[P1] Preserve missing-cookie failures before OAuth fallbacks [P1] Merge cookie changes into the freshly loaded profile [P2] Protect lifecycle reloads from in-progress config writes
Security	None	None.

How this fits together

spogo turns CLI commands and profile settings into Spotify requests. OAuth supplies credentials for the public Web API, while Connect continues to use browser cookies.

flowchart TD
  A[CLI command and profile] --> B[Engine selection]
  C[Browser OAuth login] --> D[Profile token cache]
  B --> E[Cookie-backed Connect]
  B --> F[Web API auth selection]
  E -->|Public API fallback| F
  D --> F
  F --> G[Spotify Web API]
  E --> H[Command result or error]
  G --> H

Loading

Before merge

• Preserve missing-cookie failures before OAuth fallbacks (P1) - With valid OAuth and no usable cookie session, ConnectClient.search receives an authentication error from graphQL and then succeeds through this newly injected Web API client. auto uses the same path, as the supplied live trace confirms. This hides an authentication failure contrary to VISION.md and the documented engine contract. Propagate authentication errors before remote fallback while preserving explicit web OAuth and local AppleScript recovery. This prior finding remains unresolved.
• Merge cookie changes into the freshly loaded profile (P1) - Cookie import/paste/clear pass a startup profile snapshot to SaveProfile. If OAuth login commits meanwhile, replacing the freshly loaded profile here discards its new auth mode and client settings; a stale cookie writer can also restore OAuth selection after OAuth clear removed the token. The config lock serializes writes but does not make the supplied snapshot current. Apply only command-owned fields to the persisted profile inside the transaction. This prior finding remains unresolved.
• Protect lifecycle reloads from in-progress config writes (P2) - reloadOAuthProfile bypasses the shared config lock while config.Save still uses truncating os.WriteFile. During another profile's write, OAuth clear can read an empty file as default config, skip resetting the stored OAuth selection, and delete its token; the writer then restores a config that still selects OAuth. Partial TOML can also make the command fail. Coordinate this read and its mutation with config writers or provide atomic snapshots. This prior finding remains unresolved.
• Resolve merge risk (P1) - Existing-profile upgrade safety remains unproven: overlapping cookie and OAuth operations can leave persisted authentication settings inconsistent.
• Resolve merge risk (P2) - Until remote fallback guards are repaired, scripts may receive success where the documented Connect contract requires an authentication failure.
• Complete next step (P2) - Repair the three retained findings and demonstrate fresh-profile and existing-cookie-profile compatibility through OAuth login and clear.

Findings

• [P1] Preserve missing-cookie failures before OAuth fallbacks — internal/spotify/connect_web.go:8-12
• [P1] Merge cookie changes into the freshly loaded profile — internal/app/context.go:60-64
• [P2] Protect lifecycle reloads from in-progress config writes — internal/cli/auth_oauth.go:262-269

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test LOC	Production +1,016 net; tests +1,720 net	The production growth supports the stated OAuth lifecycle and locking features; test growth does not yet cover the three retained defects.

Merge-risk options

Maintainer options:

1. Repair the existing contract boundaries (recommended)
   Preserve remote authentication errors and make profile reads and mutations consistent, then verify fresh and upgraded profiles before landing.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Repair all three retained findings: preserve authentication errors before remote OAuth fallback while retaining explicit web OAuth and local AppleScript recovery; merge only command-owned profile fields under the config lock; prevent lifecycle reads from observing partial config writes. Add deterministic concurrency and existing-profile compatibility regressions without changing defaults or expanding OAuth scope.

Technical review

Best possible solution:

Deliver bounded OAuth with transparent authentication failures, transactional profile updates, and demonstrated preservation of existing cookie-profile settings.

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

Yes, from source: valid OAuth plus missing cookies exercises the fallback defect, and controlled overlapping profile operations expose the two persistence races. The supplied trace confirms the fallback outcome; this review did not execute tests.

Is this the best way to solve the issue?

The existing Web API client is the right integration point, but the patch needs narrower fallback eligibility and complete profile transactions to preserve its documented compatibility guarantees.

Full review comments:

• [P1] Preserve missing-cookie failures before OAuth fallbacks — internal/spotify/connect_web.go:8-12
  With valid OAuth and no usable cookie session, ConnectClient.search receives an authentication error from graphQL and then succeeds through this newly injected Web API client. auto uses the same path, as the supplied live trace confirms. This hides an authentication failure contrary to VISION.md and the documented engine contract. Propagate authentication errors before remote fallback while preserving explicit web OAuth and local AppleScript recovery. This prior finding remains unresolved.
  Confidence: 0.99
• [P1] Merge cookie changes into the freshly loaded profile — internal/app/context.go:60-64
  Cookie import/paste/clear pass a startup profile snapshot to SaveProfile. If OAuth login commits meanwhile, replacing the freshly loaded profile here discards its new auth mode and client settings; a stale cookie writer can also restore OAuth selection after OAuth clear removed the token. The config lock serializes writes but does not make the supplied snapshot current. Apply only command-owned fields to the persisted profile inside the transaction. This prior finding remains unresolved.
  Confidence: 0.99
• [P2] Protect lifecycle reloads from in-progress config writes — internal/cli/auth_oauth.go:262-269
reloadOAuthProfile bypasses the shared config lock while config.Save still uses truncating os.WriteFile. During another profile's write, OAuth clear can read an empty file as default config, skip resetting the stored OAuth selection, and delete its token; the writer then restores a config that still selects OAuth. Partial TOML can also make the command fail. Coordinate this read and its mutation with config writers or provide atomic snapshots. This prior finding remains unresolved.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

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

Labels

Label justifications:

• P2: This is a bounded optional authentication feature with pre-merge defects, without evidence of an urgent shipped regression.
• merge-risk: 🚨 compatibility: Whole-profile replacement and unlocked lifecycle reads can disrupt existing persisted profile settings.
• merge-risk: 🚨 auth-provider: The injected OAuth fallback can conceal cookie authentication failures and inconsistent profile writes can select unusable credentials.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The captured exact-head CLI transcript demonstrates OAuthTokenProvider refreshing an expired real-account token, persisting it with mode 0600, and completing authenticated Web API reads; inherited callback evidence demonstrates invalid-state rejection before exchange. This supports the exercised behavior without resolving the retained fallback and profile-compatibility defects.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured exact-head CLI transcript demonstrates OAuthTokenProvider refreshing an expired real-account token, persisting it with mode 0600, and completing authenticated Web API reads; inherited callback evidence demonstrates invalid-state rejection before exchange. This supports the exercised behavior without resolving the retained fallback and profile-compatibility defects.

Evidence

Acceptance criteria:

• [P1] go test ./internal/app ./internal/cli ./internal/config ./internal/spotify.
• [P2] go test -race -count=20 ./internal/cli ./internal/config ./internal/app ./internal/spotify -run 'OAuth|SaveProfile|Update|Auth|Fallback'.
• [P1] go test ./...
• [P1] go test -race ./...
• [P1] golangci-lint run.

What I checked:

• Repository policy discovery: The origin identifies openclaw/spogo. No root or nested AGENTS.md or maintainer-note files were found; VISION.md was read fully. (VISION.md:24, 62f34e81700b)
• Feature remains absent from main and latest release: Current main exposes cookie status/import/paste/clear only, and a broader main-tree OAuth/PKCE search returned no matches. The separately inspected v0.10.7 auth command tree is also cookie-only; its commit resolves to dc83047. (internal/cli/auth.go:3, e89c46fefed8)
• Introduced changes survive the verified merge: The raw test-merge parents are the pinned main followed by the exact PR head. Comparing that merge tree with the PR head produced no differences. (6183b94adc2c)
• Authentication fallback contract and provenance: VISION.md prohibits automatic fallbacks from hiding authentication failures. Blame identifies this contract at the recorded commit; its raw parent was inspected and the explicit parent comparison confirms the document addition. (VISION.md:31, 5adbd048ac0e)
• OAuth bypasses the former fallback authentication check: Connect search falls back after any graphQL error. This introduced helper now uses the injected Web API client instead of requiring session.auth, allowing valid OAuth to turn missing-cookie failures into successful searches. The captured real-account auto-engine trace reports this outcome. (internal/spotify/connect_web.go:8, 62f34e81700b)
• Whole-profile replacement retains stale-writer race: SaveProfile reloads the config under its lock but replaces the selected profile with the caller's snapshot. Cookie import/paste/clear callers construct that snapshot before acquiring the lock, so they can overwrite OAuth settings committed meanwhile. (internal/app/context.go:60, 62f34e81700b)

Likely related people:

• Peter Steinberger: Raw commit 5adbd04 adds VISION.md:31 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 5adbd048ac0e; files: VISION.md)
• Karthikeyan KK: 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.

• Preserve authentication failures before remote fallback while retaining explicit Web API OAuth and local AppleScript recovery.
• Cover stale cookie writers and lifecycle reads overlapping config writes with deterministic regressions.
• Demonstrate that fresh and existing cookie profiles retain their stored settings through OAuth login and clear.

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 (14 earlier review cycles; latest 8 shown)

• reviewed 2026-09-04T11:00:42.027Z sha 62f34e8 :: blocked before merge. :: none
• reviewed 2026-09-04T15:11:48.310Z sha 62f34e8 :: blocked before merge. :: [P2] Preserve missing-cookie failures before OAuth fallback
• reviewed 2026-09-04T16:54:46.717Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks
• reviewed 2026-09-04T19:00:47.375Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks
• reviewed 2026-09-05T01:17:03.041Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks | [P2] Preserve OAuth fields when committing cookie-profile changes | [P2] Protect lifecycle reloads from in-progress config writes
• reviewed 2026-09-05T06:55:55.909Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks | [P1] Merge cookie changes into the freshly loaded profile | [P2] Protect lifecycle reloads from in-progress config writes
• reviewed 2026-09-05T10:00:28.455Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks | [P1] Merge cookie changes into the freshly loaded profile | [P2] Protect lifecycle reloads from in-progress config writes
• reviewed 2026-09-05T13:02:08.947Z sha 62f34e8 :: blocked before merge. :: [P1] Preserve missing-cookie failures before OAuth fallbacks | [P1] Merge cookie changes into the freshly loaded profile | [P2] Protect lifecycle reloads from in-progress config writes

[VACInc]

@steipete, this should now be ready for your approval and merge when you’re happy with it. I cannot merge it myself because VACInc currently has read-only access to openclaw/spogo. 😂

The current head includes the repairs from #61, the additional cross-profile config serialization, and the profile-derived path containment fix from the latest ClawSweeper review. CI is green, the final security review found no remaining path or collision issues, and the PR body contains redacted real-account proof from the installed final binary: automatic token refresh plus successful search and saved-library reads with zero stderr.

Could you provide the explicit bounded OAuth approval required by VISION.md and merge this if it looks good to you?

I’d also love to help with spogo going forward. I’ve subscribed to the repository so I can follow new work and reviews.