Skip to content

feat(tui): in-TUI Spotify auth trigger from Settings tab#60

Merged
vxcozy merged 1 commit into
mainfrom
feat/in-tui-auth
Apr 18, 2026
Merged

feat(tui): in-TUI Spotify auth trigger from Settings tab#60
vxcozy merged 1 commit into
mainfrom
feat/in-tui-auth

Conversation

@vxcozy
Copy link
Copy Markdown
Owner

@vxcozy vxcozy commented Apr 18, 2026

Summary

  • New Verb::StartAuth + Event::AuthStarted|Completed|Failed round-trip lets users kick off the Spotify OAuth PKCE flow from inside the TUI instead of dropping to a second shell for clitunes auth.
  • Keybind: press a on the Settings tab. While pending the tab says "Opening browser… waiting for Spotify"; on success the status badge flips to "Logged in" automatically via a follow-up Verb::ReadConfig; on failure the reason is shown inline until the next retry.
  • 5-minute timeout on the daemon-side flow so walked-away users don't end up with a zombie pending state.

Why

PR #58 surfaced auth state on the Settings tab but still asked users to drop to another terminal to actually authenticate. Single-pane users (tmux-less, SSH with one pane, iTerm tabs-only) hit that friction every first-run; the taped-over hint "Run clitunes auth in another terminal to sign in" felt like a TODO we'd shipped.

The daemon already owned the credential-cache write path via load_or_authenticate. The gap was a verb that triggered it and an event topic that streamed progress. This PR closes that gap without touching the existing clitunes auth subcommand — both entry points coexist.

Phase 1 findings (auth-flow daemon-drivability)

  • librespot-oauth 0.8's get_access_token is cleanly daemon-drivable when the redirect URI carries a port (ours: http://127.0.0.1:8898/login). It binds its own HTTP listener and never reads stdin in that path.
  • The browser is opened via librespot's bundled open::that_in_background; SSH / no-browser environments still get a functional auth URL printed (today that print is lost to the daemon's /dev/null stdout — surfacing the URL to the TUI requires a librespot-oauth public API change and is out of scope for this PR).
  • ensure_consent in auth.rs reads from stdin, so the daemon can't call load_or_authenticate directly. This PR adds a sibling authenticate_from_daemon that skips the stdin prompt (the a keypress counts as consent at the UI layer) and reuses run_oauth_flow + save_cached unchanged.

Wire shape

Verb (no payload, idempotent):

Verb::StartAuth  // "start_auth" on the wire

Events (topic "auth"; clients subscribe alongside config):

Event::AuthStarted { url: Option<String> }  // url reserved; None today
Event::AuthCompleted
Event::AuthFailed   { reason: String }

Daemon-side, dispatch_verbs owns an Arc<AtomicBool> guard so overlapping StartAuth verbs coalesce; subsequent verbs ack ok: true but emit no duplicate events. The flow itself runs under tokio::task::spawn_blocking wrapped in tokio::time::timeout(5min); all three terminal states (ok / err / elapsed) map to a single AuthCompleted or AuthFailed { reason } event.

Tests

  • New state.rs tests: a starts auth when logged out, is ignored while pending or already logged in, and retries cleanly after a failure (the stale error banner clears).
  • New paint.rs tests: pending-state message replaces the sign-in hint; failure reason renders in a Last attempt failed: banner.
  • New verbs.rs / events.rs roundtrip tests for the four new wire types.
  • Existing settings-tab tests updated for the new "Press `a` to sign in" copy and the a auth footer hint.

Test plan

  • cargo fmt --check clean
  • cargo clippy --workspace --all-targets -- -D warnings clean
  • cargo test --workspace --all-features — all suites green; default-features build + tests green too
  • Manual smoke: open picker, 4 → Settings, a → browser opens + pending copy shown, complete auth → status flips to "Logged in" on its own
  • Manual smoke: a with no network → ~5min later, AuthFailed { reason: "timeout" } surfaces inline

Scope discipline

  • No config-write path (device name / Connect toggle remain editor-only) — that's a future slice.
  • No changes to the existing clitunes auth subcommand.
  • Visualiser reactivity files untouched (CLI-91 is in a parallel worktree).

Closes clitunes-y53 / CLI-92

Before this change, users on the Settings tab saw a static hint telling
them to run `clitunes auth` in a second terminal. Single-pane users
had to leave the TUI (or open a tmux split) just to authenticate.

This wires an in-TUI trigger: pressing `a` on the Settings tab fires
a new `Verb::StartAuth` at the daemon, which runs the PKCE flow in a
spawn_blocking task and streams progress back on a new `auth` topic.

Verb + events

- `Verb::StartAuth` (no payload; idempotent — in-flight re-entries are
  dropped silently via an `AtomicBool` guard inside `dispatch_verbs`).
- `Event::AuthStarted { url: Option<String> }` — `url` is reserved for
  a future refactor; librespot-oauth 0.8 does not expose the authorize
  URL through its public API, so today the field is always `None` and
  the browser is opened via the built-in `open::that_in_background`
  inside librespot.
- `Event::AuthCompleted` — the render loop follows with a fresh
  `Verb::ReadConfig` so the status badge flips to "Logged in".
- `Event::AuthFailed { reason }` — surfaced inline under the status
  line until the user retries. A 5-minute tokio::time::timeout maps
  to `reason: "timeout"` so a walked-away user isn't stuck pending.

Keybind + paint

- `a`/`A` on the Settings tab returns `PickerAction::StartAuth`,
  ignored when already logged in or while a flow is pending.
- The tab instruction row swaps to "Opening browser… waiting for
  Spotify to complete sign-in." while pending, and surfaces
  `Last attempt failed: <reason>` on failure. Footer advertises the
  new `a auth` binding alongside the existing numeric jumps.

Daemon-drivability

librespot-oauth's `get_access_token` is fully daemon-drivable as long
as the redirect URI has a port (ours is `127.0.0.1:8898/login`) —
it binds its own HTTP listener and never touches stdin. A new
`authenticate_from_daemon` in `sources::spotify::auth` wraps
`run_oauth_flow(open_browser=true)` + `save_cached` without the
stdin-reading consent prompt that the `clitunes auth` subcommand uses.
The existing `clitunes auth` path is untouched.

Closes clitunes-y53 / CLI-92
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@vxcozy vxcozy merged commit adb84d1 into main Apr 18, 2026
12 checks passed
vxcozy added a commit that referenced this pull request Apr 18, 2026
PR #60 left the in-TUI Spotify auth flow vulnerable to two stuck-state
bugs: if the daemon crashed mid-flow the `auth_in_progress` flag
survived indefinitely, and a control-socket reconnect would leave the
client convinced a flow was still running against a daemon that had
no memory of it. Fix both:

- `PickerState::tick_auth_deadline` trips after 5m30s — longer than
  the daemon's own 5m ceiling so the daemon's `AuthFailed { "timeout" }`
  normally wins with its specific reason, and this is the last-resort
  unstick when the daemon is simply gone. Called every render tick.
- `ReconnectingSession` fires a reconnect notice via a sender the TUI
  installs on startup. The render loop drains the channel each tick,
  calls `handle_daemon_reconnected` (which clears any pending auth
  flag with a benign "daemon reconnected — retry if needed" banner),
  and re-issues `Verb::ReadConfig` so the Settings tab resyncs with
  the new daemon's actual auth state.

Fix 1 from the original scope — capturing the OAuth authorize URL so
the TUI can render it for SSH/headless users — turned out to be
structurally impossible from outside librespot-oauth. Both the CSRF
state and the PKCE challenge are generated inside
`OAuthClient::set_auth_url` with no public hook. Any URL we'd build
ourselves would pair a different state + challenge with the verifier
librespot's listener is waiting to exchange — the user's browser
redirect would then fail PKCE validation. The task's stop-and-report
clause applies. Added a TODO in auth.rs pointing at the upstream fix
(expose the URL through the public API) and softened the pending-row
message to point headless users at the sibling `clitunes auth` CLI,
which runs in their terminal and can print the URL directly.

Tests:
- tick_auth_deadline: noop when idle, noop before timeout, trips
  after timeout with a client-side reason, and doesn't double-fail.
- handle_daemon_reconnected: clears pending flag + surfaces benign
  note; noop when not in progress.
- set_auth_completed/failed clear the deadline clock.
- `a` press populates `auth_started_at`.
- paint: pending state renders the new "run clitunes auth" fallback.

Local-CI triple clean: `cargo fmt --check`, `cargo clippy --workspace
--all-targets -- -D warnings`, `cargo test --workspace --all-features`.

Closes clitunes-lsn / CLI-93
vxcozy added a commit that referenced this pull request Apr 18, 2026
…uth (#61)

PR #60 left the in-TUI Spotify auth flow vulnerable to two stuck-state
bugs: if the daemon crashed mid-flow the `auth_in_progress` flag
survived indefinitely, and a control-socket reconnect would leave the
client convinced a flow was still running against a daemon that had
no memory of it. Fix both:

- `PickerState::tick_auth_deadline` trips after 5m30s — longer than
  the daemon's own 5m ceiling so the daemon's `AuthFailed { "timeout" }`
  normally wins with its specific reason, and this is the last-resort
  unstick when the daemon is simply gone. Called every render tick.
- `ReconnectingSession` fires a reconnect notice via a sender the TUI
  installs on startup. The render loop drains the channel each tick,
  calls `handle_daemon_reconnected` (which clears any pending auth
  flag with a benign "daemon reconnected — retry if needed" banner),
  and re-issues `Verb::ReadConfig` so the Settings tab resyncs with
  the new daemon's actual auth state.

Fix 1 from the original scope — capturing the OAuth authorize URL so
the TUI can render it for SSH/headless users — turned out to be
structurally impossible from outside librespot-oauth. Both the CSRF
state and the PKCE challenge are generated inside
`OAuthClient::set_auth_url` with no public hook. Any URL we'd build
ourselves would pair a different state + challenge with the verifier
librespot's listener is waiting to exchange — the user's browser
redirect would then fail PKCE validation. The task's stop-and-report
clause applies. Added a TODO in auth.rs pointing at the upstream fix
(expose the URL through the public API) and softened the pending-row
message to point headless users at the sibling `clitunes auth` CLI,
which runs in their terminal and can print the URL directly.

Tests:
- tick_auth_deadline: noop when idle, noop before timeout, trips
  after timeout with a client-side reason, and doesn't double-fail.
- handle_daemon_reconnected: clears pending flag + surfaces benign
  note; noop when not in progress.
- set_auth_completed/failed clear the deadline clock.
- `a` press populates `auth_started_at`.
- paint: pending state renders the new "run clitunes auth" fallback.

Local-CI triple clean: `cargo fmt --check`, `cargo clippy --workspace
--all-targets -- -D warnings`, `cargo test --workspace --all-features`.

Closes clitunes-lsn / CLI-93
@vxcozy vxcozy deleted the feat/in-tui-auth branch April 18, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant