feat(tui): pergamum tui skeleton + device-grant login + paginated book list (bookshelf-wtsp.7) #1237

Merged

zombor merged 4 commits from bd-bookshelf-wtsp.7 into main 2026-07-26 02:49:55 +00:00

Conversation 3 Commits 4 Files changed 16 +2219 -1

zombor commented 2026-07-25 00:16:09 +00:00

Owner

Copy link

Summary

• Adds pergamum tui login — performs the RFC 8628 OAuth Device Authorization Grant:
  calls POST /oauth/device/code, prints user_code + verification_uri for the user
  to open in their browser, polls POST /oauth/token until approved, then stores the
  resulting pergamum access+refresh JWTs in ~/.config/pergamum/tui-tokens.json (0600).
• Adds pergamum tui books — Bubble Tea paginated book list browser: fetches
  GET /books with Authorization: Bearer, cursor-paginated at 50/page, n to load
  the next page, q to quit; transparently refreshes the access token via the stored
  refresh token on 401.
• internal/tui package: Client (device code request, token poll, refresh, book list)
  • TokenStore (0600 JSON file with atomic write) — 100% black-box test coverage, 36
    Ginkgo specs using httptest.Server stubs.
• Wires tui as a new subcommand on the main binary alongside serve/scan/worker.

Depends on bookshelf-wtsp.1 (Bearer support in AuthMiddleware + device-grant endpoints
on the server — already merged).

Test plan

• make test — all packages pass including new internal/tui (36 specs)
• make coverage — 100% coverage gate passes; internal/tui at 100%
• make lint — zero issues
• go build ./cmd/pergamum/... — compiles clean
• Manual smoke (requires running server + OIDC configured):
  • pergamum tui login --server http://localhost:8080 → shows user_code/URL, waits for browser approval
  • After approval in browser: "Login successful!" + token file written at 0600
  • pergamum tui books → Bubble Tea list renders; n loads next page; q exits

Closes bead bookshelf-wtsp.7 on merge.

## Summary - Adds `pergamum tui login` — performs the RFC 8628 OAuth Device Authorization Grant: calls `POST /oauth/device/code`, prints `user_code` + `verification_uri` for the user to open in their browser, polls `POST /oauth/token` until approved, then stores the resulting pergamum access+refresh JWTs in `~/.config/pergamum/tui-tokens.json` (0600). - Adds `pergamum tui books` — Bubble Tea paginated book list browser: fetches `GET /books` with `Authorization: Bearer`, cursor-paginated at 50/page, `n` to load the next page, `q` to quit; transparently refreshes the access token via the stored refresh token on 401. - `internal/tui` package: `Client` (device code request, token poll, refresh, book list) + `TokenStore` (0600 JSON file with atomic write) — 100% black-box test coverage, 36 Ginkgo specs using httptest.Server stubs. - Wires `tui` as a new subcommand on the main binary alongside `serve`/`scan`/`worker`. Depends on bookshelf-wtsp.1 (Bearer support in AuthMiddleware + device-grant endpoints on the server — already merged). ## Test plan - [x] `make test` — all packages pass including new `internal/tui` (36 specs) - [x] `make coverage` — 100% coverage gate passes; `internal/tui` at 100% - [x] `make lint` — zero issues - [x] `go build ./cmd/pergamum/...` — compiles clean - Manual smoke (requires running server + OIDC configured): - `pergamum tui login --server http://localhost:8080` → shows user_code/URL, waits for browser approval - After approval in browser: "Login successful!" + token file written at 0600 - `pergamum tui books` → Bubble Tea list renders; `n` loads next page; `q` exits Closes bead bookshelf-wtsp.7 on merge.

zombor added 1 commit 2026-07-25 00:16:10 +00:00

feat(tui): add pergamum tui subcommand with device-grant login + paginated book list (bookshelf-wtsp.7) ... 472a33940e

- internal/tui: API client (device-grant, token poll, refresh, book list)
  and TokenStore (0600 JSON file) with 100% black-box test coverage
- cmd/pergamum/tui.go: 'pergamum tui login' performs RFC 8628 device
  authorization grant (POST /oauth/device/code → print user_code → poll
  /oauth/token → save tokens)
- cmd/pergamum/tui_books_model.go: Bubble Tea paginated book browser
  (cursor-based, 50/page, 'n' for next page, 'q' to quit, transparent
  token refresh on 401)
- Registers 'tui' as a subcommand on the main binary alongside serve/scan/worker

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor commented 2026-07-25 00:29:42 +00:00

Author

Owner

Copy link

[MAJOR] internal/tui/apiclient_test.go:350-351 — wall-clock timing in test assertions
BeTemporally assertions using time.Now() are flaky in CI environments. Per review-standard.md, tests must not assert on real elapsed time. Fix: inject a clock function into the TokenResponse creation path so tests can control time, or use a fixed past/future time that does not depend on when the test runs. The BeTemporally checks should verify that ExpiresAt is approximately (now + 3600s) using a reasonable tolerance, not by checking against the actual wall clock.

[MAJOR] internal/tui/tokenstore_test.go:149-170 — wall-clock timing in IsValid() test assertions
Lines 154 and 166 use time.Now().Add(...) to create test tokens, then lines 157/169 call IsValid() which itself calls time.Now(). This creates a race: if the test is slow, time.Now() in IsValid() will return a different value than expected, causing flakes. Fix: refactor to accept an injectable now func in the Tokens type or create a test helper that returns predictable times. Alternatively, use fixed past/future dates that never depend on the actual current time.

[MINOR] internal/tui/apiclient_test.go:143-147, 270-275, 307-310 — multiple Expect calls per It block
The one-Expect-per-It convention requires exactly one Expect call per It block. These It blocks contain 4, 2, and 2 separate Expect calls respectively. Fix: fold multiple assertions into a single Expect using Gomega matchers (e.g., Expect(result, err).To(And(HaveField(...)...)) or split each assertion into its own It block with a descriptive name.

REVIEW VERDICT: 2 major, 1 minor

[MAJOR] internal/tui/apiclient_test.go:350-351 — wall-clock timing in test assertions BeTemporally assertions using time.Now() are flaky in CI environments. Per review-standard.md, tests must not assert on real elapsed time. Fix: inject a clock function into the TokenResponse creation path so tests can control time, or use a fixed past/future time that does not depend on when the test runs. The BeTemporally checks should verify that ExpiresAt is approximately (now + 3600s) using a reasonable tolerance, not by checking against the actual wall clock. [MAJOR] internal/tui/tokenstore_test.go:149-170 — wall-clock timing in IsValid() test assertions Lines 154 and 166 use time.Now().Add(...) to create test tokens, then lines 157/169 call IsValid() which itself calls time.Now(). This creates a race: if the test is slow, time.Now() in IsValid() will return a different value than expected, causing flakes. Fix: refactor to accept an injectable now func in the Tokens type or create a test helper that returns predictable times. Alternatively, use fixed past/future dates that never depend on the actual current time. [MINOR] internal/tui/apiclient_test.go:143-147, 270-275, 307-310 — multiple Expect calls per It block The one-Expect-per-It convention requires exactly one Expect call per It block. These It blocks contain 4, 2, and 2 separate Expect calls respectively. Fix: fold multiple assertions into a single Expect using Gomega matchers (e.g., Expect(result, err).To(And(HaveField(...)...)) or split each assertion into its own It block with a descriptive name. REVIEW VERDICT: 2 major, 1 minor

zombor commented 2026-07-25 00:29:48 +00:00

Author

Owner

Copy link

Security re-review (FINAL, adversarial) — reserve-then-refund OPDS rate limiter

Verdict up front: the reserve-then-refund design is sound, but this implementation of the refund is broken in production and reintroduces the original drain (MAJOR #1). The bug is masked by the frozen-clock unit tests, which is why it survived to iteration 4.

[BLOCKER] internal/users/ratelimiter.go:127 — refund is a no-op under the real (production) clock; drain (MAJOR #1) is NOT fixed
  ReserveToken returns the success refund as `func() { r.CancelAt(l.now()) }`, evaluating the
  clock at REFUND time. golang.org/x/time/rate's CancelAt(t) short-circuits when
  `r.timeToAct.Before(t)` (rate.go). For an *immediate* reservation timeToAct == the reserve
  instant, so once any real time has elapsed — and bcrypt in verifyCredentials guarantees
  ~50-100ms — the refund-time `l.now()` is strictly after timeToAct, the guard trips, and
  CancelAt restores NOTHING. In production (now=time.Now) every successful auth therefore
  consumes a token and never gives it back: the bucket drains 10->9->...->0, then legitimate
  requests are rejected. A poll-heavy OPDS client (re-auth on every cover/thumbnail/file
  request; a single 50-book feed page emits >10 asset fetches) is throttled within seconds —
  exactly the drain this iteration was built to eliminate.

  Empirically verified against x/time v0.15.0: real-clock refund drains to REJECTED after 10;
  frozen-clock refund (what the unit tests inject) stays pinned at 10.000 — so the tests pass
  while production is broken. Note the internal inconsistency that proves the bug: the REJECT
  path (ratelimiter.go:123) calls `r.CancelAt(now)` with the RESERVE-time `now` and correctly
  restores; only the SUCCESS refund uses the later `l.now()`.

  Fix: capture the reserve-time value and cancel with it —
      return true, func() { r.CancelAt(now) }
  (`now` is already in scope from `now := l.now()`). Verified: under a real clock this keeps the
  bucket full on success while failures — which never call refund — still drain and throttle
  after burst. Works identically under the frozen test clock. Also add a real-clock (non-frozen
  or advanced-clock) test asserting N>burst *successful* auths from one IP are NOT throttled, so
  this regression cannot hide behind a frozen clock again.

[MINOR] internal/opds/handler.go authenticate (post-fix consideration) — valid-cred concurrent-bcrypt amplification
  Once the refund is fixed, a single VALID credential lets one IP sustain up to `burst` (10)
  concurrent bcrypt hashes continuously (each success refunds, so the cap only bites between
  completion and the next reserve), whereas the web /login path (consuming Allow, no refund)
  throttles the same attacker to ~10/min. This is the deliberate, bounded trade-off of
  refund-on-success and requires authentication, so it is acceptable — but keep `burst`
  single-digit/configurable and be aware OPDS gives an authenticated attacker more sustained
  hash throughput than /login. Not blocking.

[MINOR] internal/opds/handler.go opdsClientIP — duplicates users.clientIP verbatim
  opdsClientIP is byte-identical to internal/users clientIP (RemoteAddr-only, no XFF trust —
  which is correct). Consider exporting/reusing the one helper to avoid drift. No security impact.

Everything else verified clean:

• Concurrent bcrypt bound (was MAJOR #3): holds. The reserve is a real consume held across bcrypt and only refunded AFTER it completes, so at most burst reservations (hence <=burst concurrent hashes) per IP exist at any instant; the burst+1th finds an empty bucket and is rejected pre-hash. Holds under both the broken and the fixed refund.
• Brute-force (original): preserved. Failed attempts never call refund -> bucket empties after burst failures -> further attempts rejected pre-bcrypt. Empirically confirmed.
• No over-refund / cross-bucket refund (4b): CancelAt caps restored tokens at burst; the refund closure is bound to the same reservation r/e.limiter (same IP), so it cannot exceed capacity or credit another IP.
• Token-leak (4a): the only code between reserve and refund is verifyCredentials; its error path is the intended no-refund (throttle) branch. A panic would leak one token, but it self-heals via refill and is not a normal path.
• Reject path / clock race (4d): ReserveN+CancelAt run under rate.Limiter's own mutex; the reject branch cancels with reserve-time now and does not consume; no refill-faster-than-rate race.
• IP source (5): opdsClientIP uses RemoteAddr only, never X-Forwarded-For — no per-IP bypass. /login unchanged (still loginLimiter.Allow, consuming). Audits preserved (ActionLoginRateLimited / ActionLoginFailed). No password/secret logged; username sanitized. LoginReserve wired as a non-nil method value in app.New; authenticate nil-guards it.

REVIEW VERDICT: 1 blocker, 0 major, 2 minor

## Security re-review (FINAL, adversarial) — reserve-then-refund OPDS rate limiter Verdict up front: the reserve-then-refund **design** is sound, but this **implementation of the refund is broken in production** and reintroduces the original drain (MAJOR #1). The bug is masked by the frozen-clock unit tests, which is why it survived to iteration 4. ``` [BLOCKER] internal/users/ratelimiter.go:127 — refund is a no-op under the real (production) clock; drain (MAJOR #1) is NOT fixed ReserveToken returns the success refund as `func() { r.CancelAt(l.now()) }`, evaluating the clock at REFUND time. golang.org/x/time/rate's CancelAt(t) short-circuits when `r.timeToAct.Before(t)` (rate.go). For an *immediate* reservation timeToAct == the reserve instant, so once any real time has elapsed — and bcrypt in verifyCredentials guarantees ~50-100ms — the refund-time `l.now()` is strictly after timeToAct, the guard trips, and CancelAt restores NOTHING. In production (now=time.Now) every successful auth therefore consumes a token and never gives it back: the bucket drains 10->9->...->0, then legitimate requests are rejected. A poll-heavy OPDS client (re-auth on every cover/thumbnail/file request; a single 50-book feed page emits >10 asset fetches) is throttled within seconds — exactly the drain this iteration was built to eliminate. Empirically verified against x/time v0.15.0: real-clock refund drains to REJECTED after 10; frozen-clock refund (what the unit tests inject) stays pinned at 10.000 — so the tests pass while production is broken. Note the internal inconsistency that proves the bug: the REJECT path (ratelimiter.go:123) calls `r.CancelAt(now)` with the RESERVE-time `now` and correctly restores; only the SUCCESS refund uses the later `l.now()`. Fix: capture the reserve-time value and cancel with it — return true, func() { r.CancelAt(now) } (`now` is already in scope from `now := l.now()`). Verified: under a real clock this keeps the bucket full on success while failures — which never call refund — still drain and throttle after burst. Works identically under the frozen test clock. Also add a real-clock (non-frozen or advanced-clock) test asserting N>burst *successful* auths from one IP are NOT throttled, so this regression cannot hide behind a frozen clock again. [MINOR] internal/opds/handler.go authenticate (post-fix consideration) — valid-cred concurrent-bcrypt amplification Once the refund is fixed, a single VALID credential lets one IP sustain up to `burst` (10) concurrent bcrypt hashes continuously (each success refunds, so the cap only bites between completion and the next reserve), whereas the web /login path (consuming Allow, no refund) throttles the same attacker to ~10/min. This is the deliberate, bounded trade-off of refund-on-success and requires authentication, so it is acceptable — but keep `burst` single-digit/configurable and be aware OPDS gives an authenticated attacker more sustained hash throughput than /login. Not blocking. [MINOR] internal/opds/handler.go opdsClientIP — duplicates users.clientIP verbatim opdsClientIP is byte-identical to internal/users clientIP (RemoteAddr-only, no XFF trust — which is correct). Consider exporting/reusing the one helper to avoid drift. No security impact. ``` Everything else verified clean: - **Concurrent bcrypt bound (was MAJOR #3):** holds. The reserve is a real consume held across bcrypt and only refunded AFTER it completes, so at most `burst` reservations (hence <=burst concurrent hashes) per IP exist at any instant; the burst+1th finds an empty bucket and is rejected pre-hash. Holds under both the broken and the fixed refund. - **Brute-force (original):** preserved. Failed attempts never call refund -> bucket empties after `burst` failures -> further attempts rejected pre-bcrypt. Empirically confirmed. - **No over-refund / cross-bucket refund (4b):** CancelAt caps restored tokens at `burst`; the refund closure is bound to the same reservation `r`/`e.limiter` (same IP), so it cannot exceed capacity or credit another IP. - **Token-leak (4a):** the only code between reserve and refund is verifyCredentials; its error path is the intended no-refund (throttle) branch. A panic would leak one token, but it self-heals via refill and is not a normal path. - **Reject path / clock race (4d):** ReserveN+CancelAt run under rate.Limiter's own mutex; the reject branch cancels with reserve-time `now` and does not consume; no refill-faster-than-rate race. - **IP source (5):** opdsClientIP uses RemoteAddr only, never X-Forwarded-For — no per-IP bypass. `/login` unchanged (still loginLimiter.Allow, consuming). Audits preserved (ActionLoginRateLimited / ActionLoginFailed). No password/secret logged; username sanitized. LoginReserve wired as a non-nil method value in app.New; authenticate nil-guards it. REVIEW VERDICT: 1 blocker, 0 major, 2 minor

zombor commented 2026-07-25 00:31:32 +00:00

Author

Owner

Copy link

Security Review — PR #1237 (TUI OAuth device-grant client, bookshelf-wtsp.7)

Adversarial focus: client-side credential handling (token at rest, user/device code, TLS, refresh, committed secrets, host trust).

[MAJOR] cmd/pergamum/tui_books_model.go:154 — Unbounded refresh/retry loop on a persistent 401
Update reacts to ErrUnauthorized from ListBooks by calling refreshAndRetry(). That closure refreshes then re-calls ListBooks; if the list request returns 401 again (valid refresh but token audience/permission mismatch, or a server that keeps 401'ing), it returns another booksLoadedMsg{err: ErrUnauthorized}, which re-enters refreshAndRetry() — with no retry cap and no backoff/sleep. Refresh keeps succeeding, list keeps 401'ing, and the client hot-loops /auth/refresh + /books as fast as the network allows (self-inflicted DoS / server hammering). The loop only breaks if the refresh itself fails. The spec explicitly wants "a failed refresh clears the stored creds rather than looping." Fix: bound to a single refresh attempt per 401 (track a refreshed/attempt flag in the model); on a second consecutive 401 after a successful refresh, stop and surface "session invalid — run 'pergamum tui login' again" instead of retrying.

[MINOR] cmd/pergamum/tui_books_model.go:206 — Failed refresh does not clear stored credentials
When RefreshToken fails, the model shows "session expired" but never calls store.Clear(), leaving the now-invalid refresh token (and stale access token) on disk. Not exploitable (file is 0600, token already rejected), but it deviates from the "failed refresh clears the stored creds" requirement and means the next books run reloads dead creds and 401s again. Fix: store.Clear() on unrecoverable refresh failure.

[MINOR] cmd/pergamum/tui.go:39 — No HTTPS enforcement / cleartext-token warning for non-localhost servers
Default --server is http://localhost:8080 (fine, not attacker-controllable). But the URL is used verbatim; a user who points --server http://remote-host sends the device grant, bearer access token, and refresh token over cleartext HTTP with no warning. TLS is verified when https is used (default transport, no InsecureSkipVerify), so this only covers the plaintext-scheme case. Fix: warn (or refuse) when the scheme is http and the host is not loopback.

[MINOR] internal/tui/tokenstore.go:71 — Temp-file perms not re-asserted on a pre-existing .tmp
Save writes path+".tmp" via os.WriteFile(..., 0600) then renames. On first create this is 0600 (good), but os.WriteFile does not chmod an already-existing .tmp left by a prior crash, so perms could be inherited from a looser pre-existing file. Low risk (per-user UserConfigDir, dir is 0700). Fix: os.OpenFile(tmp, O_CREATE|O_TRUNC|O_WRONLY, 0600) (after removing any stale temp) or os.Chmod post-write.

Positives confirmed (no finding):

• Token at rest: written 0600 via temp+rename, parent dir MkdirAll(...,0700), path = os.UserConfigDir()/pergamum/tui-tokens.json (correct per-user config dir).
• No access/refresh token is ever printed to stdout/stderr, logged, or rendered in the book-list UI; error strings surfaced to the UI carry only status codes / sentinels, never token material.
• device_code is never printed; only user_code/verification URIs are shown (intended by RFC 8628).
• TLS: no InsecureSkipVerify, no custom transport disabling verification; default http.Client verifies certs.
• No committed secret: client_id="pergamum-tui" is a public OAuth client id; no client_secret, no hard-coded token.
• Host trust: default server is loopback, not attacker-controllable.

REVIEW VERDICT: 0 blocker, 1 major, 3 minor

## Security Review — PR #1237 (TUI OAuth device-grant client, bookshelf-wtsp.7) Adversarial focus: client-side credential handling (token at rest, user/device code, TLS, refresh, committed secrets, host trust). [MAJOR] cmd/pergamum/tui_books_model.go:154 — Unbounded refresh/retry loop on a persistent 401 `Update` reacts to `ErrUnauthorized` from `ListBooks` by calling `refreshAndRetry()`. That closure refreshes then re-calls `ListBooks`; if the list request returns 401 again (valid refresh but token audience/permission mismatch, or a server that keeps 401'ing), it returns another `booksLoadedMsg{err: ErrUnauthorized}`, which re-enters `refreshAndRetry()` — with no retry cap and no backoff/sleep. Refresh keeps succeeding, list keeps 401'ing, and the client hot-loops `/auth/refresh` + `/books` as fast as the network allows (self-inflicted DoS / server hammering). The loop only breaks if the refresh itself fails. The spec explicitly wants "a failed refresh clears the stored creds rather than looping." Fix: bound to a single refresh attempt per 401 (track a `refreshed`/attempt flag in the model); on a second consecutive 401 after a successful refresh, stop and surface "session invalid — run 'pergamum tui login' again" instead of retrying. [MINOR] cmd/pergamum/tui_books_model.go:206 — Failed refresh does not clear stored credentials When `RefreshToken` fails, the model shows "session expired" but never calls `store.Clear()`, leaving the now-invalid refresh token (and stale access token) on disk. Not exploitable (file is 0600, token already rejected), but it deviates from the "failed refresh clears the stored creds" requirement and means the next `books` run reloads dead creds and 401s again. Fix: `store.Clear()` on unrecoverable refresh failure. [MINOR] cmd/pergamum/tui.go:39 — No HTTPS enforcement / cleartext-token warning for non-localhost servers Default `--server` is `http://localhost:8080` (fine, not attacker-controllable). But the URL is used verbatim; a user who points `--server http://remote-host` sends the device grant, bearer access token, and refresh token over cleartext HTTP with no warning. TLS is verified when https is used (default transport, no `InsecureSkipVerify`), so this only covers the plaintext-scheme case. Fix: warn (or refuse) when the scheme is `http` and the host is not loopback. [MINOR] internal/tui/tokenstore.go:71 — Temp-file perms not re-asserted on a pre-existing .tmp `Save` writes `path+".tmp"` via `os.WriteFile(..., 0600)` then renames. On first create this is 0600 (good), but `os.WriteFile` does not chmod an already-existing `.tmp` left by a prior crash, so perms could be inherited from a looser pre-existing file. Low risk (per-user `UserConfigDir`, dir is 0700). Fix: `os.OpenFile(tmp, O_CREATE|O_TRUNC|O_WRONLY, 0600)` (after removing any stale temp) or `os.Chmod` post-write. Positives confirmed (no finding): - Token at rest: written 0600 via temp+rename, parent dir `MkdirAll(...,0700)`, path = `os.UserConfigDir()/pergamum/tui-tokens.json` (correct per-user config dir). - No access/refresh token is ever printed to stdout/stderr, logged, or rendered in the book-list UI; error strings surfaced to the UI carry only status codes / sentinels, never token material. - `device_code` is never printed; only `user_code`/verification URIs are shown (intended by RFC 8628). - TLS: no `InsecureSkipVerify`, no custom transport disabling verification; default `http.Client` verifies certs. - No committed secret: `client_id="pergamum-tui"` is a public OAuth client id; no `client_secret`, no hard-coded token. - Host trust: default server is loopback, not attacker-controllable. REVIEW VERDICT: 0 blocker, 1 major, 3 minor

zombor added 1 commit 2026-07-25 00:41:32 +00:00

fix(tui): address 3 majors + 4 minors from review (bookshelf-wtsp.7) ... fc66966a87

MAJOR 1: bound the 401-refresh retry to a single attempt. Add `refreshed bool`
to booksModel; on a second consecutive ErrUnauthorized after a refresh, surface
"session invalid — run 'pergamum tui login' again" instead of re-entering the
refresh→list hot-loop.

MAJOR 2: inject a clock into Client via SetClock(func() time.Time). PollToken
and RefreshToken use c.now() instead of time.Now() so ExpiresAt is deterministic
in tests; replace the BeTemporally wall-clock assertions with an exact-equality
assertion against a fixed time.

MAJOR 3: add now func() time.Time parameter to Tokens.IsValid. Tests pass a
fixed-time closure so expired/valid comparisons are deterministic and race-free.

MINOR 1: split multi-Expect Its in apiclient_test.go into one Expect per It,
folding error checks into value assertions where applicable.

MINOR 2: call store.Clear() on unrecoverable refresh failure in refreshAndRetry
so a broken session file is removed and the user can log in again cleanly.

MINOR 3: add warnIfCleartext helper; warn to stderr when --server uses plain
HTTP to a non-loopback host so users know tokens travel cleartext.

MINOR 4: remove any stale .tmp before os.WriteFile in TokenStore.Save to
ensure the new file is always created with 0600 regardless of a pre-existing
temp with looser permissions.

zombor added 1 commit 2026-07-25 01:18:04 +00:00

refactor(tui): single immersive app with in-app login/library views (bookshelf-wtsp.7) ... 7284d10fdd

Replace the wrong shell-subcommand design (tui login / tui books) with a
single `pergamum tui` command that launches a full-screen Bubble Tea app.
This also eliminates the "parse args: already parsed" bug caused by the
shared tuiFlags FlagSet being re-parsed for each sub-subcommand.

View-router structure (cmd/pergamum/tui_app_model.go):
- appModel holds the active tui.ViewKind (Login or Library) and delegates
  Init/Update/View to the current sub-model.
- loggedInMsg transitions Login → Library after device-grant approval.
- sessionExpiredMsg transitions Library → Login when 401 persists after refresh.

Login view (cmd/pergamum/tui_login_model.go):
- In-app RFC 8628 device grant: displays user_code + verification_uri on
  screen and polls /oauth/token via tea.Tick in the background.
- Handles ErrAuthorizationPending (keep polling), ErrSlowDown (backoff),
  ErrExpiredToken (show error + offer retry), and success (store tokens +
  emit loggedInMsg to trigger router transition).

Library view (cmd/pergamum/tui_books_model.go):
- Uses tui.OnUnauthorized(refreshed) for the 401 decision instead of inline
  logic; emits sessionExpiredMsg on session expiry instead of a dead-end error.

internal/tui additions (auth.go + auth_test.go):
- ViewKind type (ViewLogin / ViewLibrary).
- InitialView(tokens, now) — determines launch view from stored tokens.
- OnUnauthorized(refreshed) — encodes the single-refresh-per-session cap.
- 16 new specs; internal/tui stays at 100% coverage (52 specs total).

Preserved from the previous review-fix commit:
- Single-refresh-per-401 cap (refreshed bool guard).
- Injected clock (SetClock / IsValid(now)).
- store.Clear() on refresh failure.
- cleartext-HTTP warning (warnIfCleartext).
- 0600 temp-file atomic write in TokenStore.Save.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor added 1 commit 2026-07-25 11:18:58 +00:00

fix(tui): forward cached window size to newly-activated view so post-login library list renders (bookshelf-wtsp.7) ... 08bec4c1eb

appModel.Update now caches tea.WindowSizeMsg dimensions and forwards them to the
freshly-constructed sub-model on every view transition:

- loggedInMsg (Login→Library): after newBooksModel, immediately calls
  library.Update(WindowSizeMsg{cached w,h}) so the bubbles list gets
  list.SetSize(...) before Init() fires the first page fetch. Without this the
  list has 0×0 dimensions (no terminal size msg ever re-fires for the new view)
  and renders blank rows even though data loads correctly.

- sessionExpiredMsg (Library→Login): same forward so the login view is sized
  if the user is bounced back mid-session.

- WindowSizeMsg handling in Update itself now caches m.width/m.height AND
  delegates to the active sub-model (via the existing delegation switch below
  the type switch), so both paths — initial sizing and live terminal resizes —
  keep working correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

zombor force-pushed bd-bookshelf-wtsp.7 from 08bec4c1eb to 3a8ec705bb 2026-07-26 02:39:49 +00:00 Compare

zombor merged commit f2a4cebea5 into main 2026-07-26 02:49:55 +00:00

zombor referenced this pull request from a commit 2026-07-26 02:49:58 +00:00

Merge pull request 'feat(tui): pergamum tui skeleton + device-grant login + paginated book list (bookshelf-wtsp.7)' (#1237) from bd-bookshelf-wtsp.7 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

zombor/pergamum!1237

No description provided.