feat(login): OAuth device-flow login + token refresh; submit via token route - #4
Merged
Conversation
…n route Add a browser-based OAuth device-authorization grant to `civitai login` so `civitai login` (no flags) authenticates via the browser and stores auto-refreshing tokens that work for `app submit` and `whoami`. Implements the civitai server contract from PR #2644. - `civitai login` (no --token): device init -> print verification URL + user code (best-effort browser open of verification_uri_complete; --no-browser to skip) -> poll /api/auth/oauth/device-token every `interval`s (respects slow_down, stops at expires_in) -> store tokens. `--token <key>` still stores a personal API key unchanged. - config: stores access/refresh tokens, absolute access-token expiry, scope, and an auth_kind marker (oauth vs token) alongside the legacy single-`token` personal key (backward compatible, 0600). CIVITAI_TOKEN env is always treated as a personal key. - internal/auth.Source bridges config <-> api.TokenSource: refreshes an expired OAuth access token before a request (and once on a 401), persists the rotated refresh token, retries once. A personal key is served as-is with no refresh. - app submit now uploads by default to the v1 token route /api/v1/blocks/submit-version (CIVITAI_SUBMIT_PATH still overrides); stale "not yet automated" copy updated. Client id `civitai-cli` is a public (PKCE/device) client, no secret; scope 33554433 = UserRead|AppBlocksSubmit. Tests: device happy path (init -> 2 pending -> success, tokens persisted 0600), slow_down interval bump, expired_token/access_denied terminal errors, overall timeout, refresh rotation + persistence, 401-refresh-retry, personal-key no- refresh, and app submit posts to /api/v1/blocks/submit-version with the Bearer + bundleBase64 body. `go build ./... && go test ./...` green. Tokens/device-code are never logged. Not verified: real end-to-end against prod (needs PR #2644 merged + the civitai-cli OAuth client row provisioned). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… fallback path, expires_in guard The token/refresh route returns the @node-oauth/oauth2-server shape where `scope` is a JSON ARRAY of strings (e.g. ["33554433"]), while the device-token (login) route returns a plain string. `TokenResponse.Scope` was a plain `string`, so json.Unmarshal of the refresh body failed the entire struct and Refresh() returned "unexpected refresh response" — after the 1h access-token TTL every authenticated command failed and never self-healed. - C1: new `Scope` string type with `UnmarshalJSON` accepting BOTH a JSON string and a JSON array of strings (array normalized to a space-joined string per OAuth convention); applied to `TokenResponse.Scope`. Both server shapes now parse cleanly. Call sites convert via `.String()`. - M1: real-contract tests — replay the actual array-shaped refresh body (`"scope":["33554433"]`) and the string-shaped device-token body; the array test fails without C1 and passes with it. Plus source-level round-trip + expires_in-guard tests. - M2: cap the RFC 8628 +5s-per-slow_down backoff at 60s (maxPollInterval) so a repeated/misbehaving slow_down can't stretch the poll unboundedly. - L1: fix the verification_uri fallback `/oauth/device` -> `/login/oauth/device`. - L4: floor-guard refresh `expires_in` — reject a non-positive value instead of persisting a token that expires ~now (refresh-on-every-call). - M3 (not fixed, documented): concurrent-process refresh race vs server revoke-on-refresh needs a cross-process file lock; left as a code-comment follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
ZacxDev
added a commit
that referenced
this pull request
Jun 19, 2026
…#5) Refresh() POSTed a JSON body with Content-Type: application/json to /api/auth/oauth/token, but that endpoint is the @node-oauth/oauth2-server token handler, which REQUIRES application/x-www-form-urlencoded and rejects JSON with "content must be application/x-www-form-urlencoded". The custom device-flow endpoints (/device, /device-token) accept JSON, so LOGIN worked — but every refresh failed, so OAuth auth silently died after the 1h access-token TTL (observed live: `whoami` → "device login failed: ... must be application/x-www-form-urlencoded"). Fix: add postForm() (x-www-form-urlencoded) and send the refresh grant through it. Verified live: an expired session refreshes cleanly against prod. The bug slipped #4's tests because TestRefreshRotatesToken's mock decoded the request body as JSON (accepting the wrong content-type). Hardened it to assert Content-Type: application/x-www-form-urlencoded + ParseForm — it now FAILS on a JSON refresh and PASSES on the form-encoded one. Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
ZacxDev
added a commit
that referenced
this pull request
Aug 3, 2026
…s a to-do list The app-analytics handoff has been sitting on this branch, unmerged, describing three open PRs and a re-gate in flight. All of that shipped — 8 PRs across both repos, plus follow-ups #1, #2 and #4 — so as written the doc's most prominent content is a set of live-sounding directives for work that is done. A stranded doc that is also stale is worse than no doc. Changes: - A RESOLVED banner up top with the full merged-PR table, replacing "nothing merged". - Neutralised the "Do not merge until that re-gate reports" imperative, and recorded that the rebase it anticipated WAS needed, for a different reason: #3566 landed later and edited the same `detail: {}` object, turning #3561 CONFLICTING after its gate had passed. - Struck follow-ups #1, #2 and #4 with what actually happened, including the two places this doc was WRONG: * #1's entry missed a second proc with the identical live defect — `getMyForgejoCloneInfo`, which `civitai app pull` drives. It was found by an audit, not by the list, which is worth knowing about ranked follow-up lists in general: the list is not a survey. * #2's suggested fix (reuse `humaniseScopeEndpoint`) would have shipped a bug. Measured against the real function it returns '(no workflow id)' for `workflow:submit` and '' for `user-settings:write`, because it is the per-ROW labeller and an aggregate bucket has no `detail`. - Recorded #1's scope decision with the prod evidence that later confirmed it: 331 live tokens unblocked, 30 of which lack bit 26 — so copying the nearest precedent (AppBlocksDevTunnel) would have left those 30 still 403ing. Plus the measurement trap: `(mask & Full) = Full` is also true when `mask == Full` and reports 145 false hits; the strict-superset form needs `AND mask <> Full`. - New "Still open — start here" section: the CI `component`-tier gap (three PRs shipped browser tests that have never run on a canonical browser), the stale-node_modules trap that silently removes ~1,126 tests, the unverified `addCollaborator` downgrade lead, and a note that `installs: 0` should be assumed broken until a positive control exists. - "What actually caught the bugs": across 12 adversarial audit rounds every fix round found a defect in the previous fix, and the mechanical gate caught none of them — the suite and typecheck were green at every tip. Doc-only. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
ZacxDev
added a commit
that referenced
this pull request
Aug 4, 2026
* docs: handoff for app-analytics CLI command + two platform fixes Captures the state of #190, civitai/civitai#3557 and #3561, the stale-integration-gate blocker, the measured per-app analytics, ranked follow-ups, and the session's reusable lessons. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01858ymA3tEJQi83435u7npi * docs: re-gate cleared the stale-gate blocker; #3557 prettier fixed Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01858ymA3tEJQi83435u7npi * docs: record the three docs PRs and the #191-after-#190 merge constraint Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01858ymA3tEJQi83435u7npi * docs(handoff): record the shipped outcomes so the doc stops reading as a to-do list The app-analytics handoff has been sitting on this branch, unmerged, describing three open PRs and a re-gate in flight. All of that shipped — 8 PRs across both repos, plus follow-ups #1, #2 and #4 — so as written the doc's most prominent content is a set of live-sounding directives for work that is done. A stranded doc that is also stale is worse than no doc. Changes: - A RESOLVED banner up top with the full merged-PR table, replacing "nothing merged". - Neutralised the "Do not merge until that re-gate reports" imperative, and recorded that the rebase it anticipated WAS needed, for a different reason: #3566 landed later and edited the same `detail: {}` object, turning #3561 CONFLICTING after its gate had passed. - Struck follow-ups #1, #2 and #4 with what actually happened, including the two places this doc was WRONG: * #1's entry missed a second proc with the identical live defect — `getMyForgejoCloneInfo`, which `civitai app pull` drives. It was found by an audit, not by the list, which is worth knowing about ranked follow-up lists in general: the list is not a survey. * #2's suggested fix (reuse `humaniseScopeEndpoint`) would have shipped a bug. Measured against the real function it returns '(no workflow id)' for `workflow:submit` and '' for `user-settings:write`, because it is the per-ROW labeller and an aggregate bucket has no `detail`. - Recorded #1's scope decision with the prod evidence that later confirmed it: 331 live tokens unblocked, 30 of which lack bit 26 — so copying the nearest precedent (AppBlocksDevTunnel) would have left those 30 still 403ing. Plus the measurement trap: `(mask & Full) = Full` is also true when `mask == Full` and reports 145 false hits; the strict-superset form needs `AND mask <> Full`. - New "Still open — start here" section: the CI `component`-tier gap (three PRs shipped browser tests that have never run on a canonical browser), the stale-node_modules trap that silently removes ~1,126 tests, the unverified `addCollaborator` downgrade lead, and a note that `installs: 0` should be assumed broken until a positive control exists. - "What actually caught the bugs": across 12 adversarial audit rounds every fix round found a defect in the previous fix, and the mechanical gate caught none of them — the suite and typecheck were green at every tip. Doc-only. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * docs(handoff): retract the "CI does not run the component project" claim — it was false The "Still open" section asserted that CI does not run the `component` (browser) project at all, and used that to frame the browser tests shipped today as having no CI coverage. **That was wrong.** The `preview / component-tests` external check runs exactly that project. Recording HOW the error happened, because the mechanism is more useful than the fact: every status query written during the session filtered for `Unit tests|Typecheck|ESLint|event-engine`, so `preview / component-tests` was never in a result set — and its absence from those results was read as evidence it did not exist. The evidence was selected and then the selection was treated as the finding. The claim was then repeated in three PR comments and this doc without ever being checked directly. What the check actually shows on #3574, the PR that shipped 7 browser tests: 075519d380 (before the audit-fix round) success 8e75826616 (first fix round) FAILURE 928273e4dd (second fix round) FAILURE It is report-only, which is why the merge was not blocked. Cutting the other way: PR 3591 PASSES component-tests on a base that contains the #3574 merge, while PR 3594 fails on the same base — green for some PRs, red for others, which looks like flakiness or content-dependence rather than a defect #3574 introduced. Genuinely unresolved. Also recorded: this cannot be settled from a NixOS host. The full-project component run does not complete locally either WITH the merge (crashes, "Browser connection was closed") or WITHOUT it (times out at 25 minutes) — measured both ways specifically to check whether the local failure was attributable to the change. It is not. Single-file runs pass, cold cache included, so a local run can validate one file and says nothing about the suite. The preview pipeline's logs are the only authority. And one genuinely stale thing found while investigating, left as an actionable note: `lint.yml`'s unit job excludes browser tests on the grounds that they "carry the cold-optimizeDeps flake documented at vitest.config.mts:98-124" — but that section documents the FIX, and a cold-cache single-file run passes. The stated reason no longer holds. Doc-only. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * docs(handoff): add the followups handoff, with the component-tests root cause Second doc on this PR. The predecessor records the original session; this one records where the stream stands now and carries one fully-diagnosed open bug. The headline: `preview / component-tests` went success -> failure exactly at #3574's audit-fix round, and the root cause is mine. The 'Runs (range)' stat's tooltip begins 'Generations run through your app...', my test asserts getByText('Generations'), and getByText is substring + case-insensitive — so the locator resolves to 2 elements whenever that hover-only tooltip is mounted. civitai#3593 established that vitest browser mode shares ONE page across every .browser.test.tsx file, so a leftover pointer position from an earlier file can have it open at mount: fails in a full-suite run, never in a single-file run, which is exactly the observed pattern. Confirmed by computation that the PREVIOUS label ('Generation submits') did NOT substring-match that tooltip, so the vocabulary rename is what introduced it. Doc includes the verbatim fix to apply, the eliminations (cold-optimizeDeps flake is fixed; PR 3591 passes on a base containing the merge, so the suite is not universally broken; a local repro cannot settle it because the full-project run fails on this host with AND without the change), and the environment traps that produced false greens all session. Doc-only. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * docs(handoff): mark the component-tests investigation resolved (civitai#3606) Records the fix, the proof matrix, and the reproduction trap: the tooltip target is the 14px IconInfoCircle, not the label — hovering the label does NOT mount it, so a probe without its own positive control reads as 'hypothesis wrong' and would have shipped an unproven fix. Also flags what is still unconfirmed: preview/component-tests had not reported on #3606 at merge time, so the next PR's result on that check is the confirmation to read. Doc-only. * docs(handoff): sync the header with the fix, and record the pins-vs-published blocker Goal/state lines still described the component test as outstanding. Also records why cli#193 is BLOCKED: pins-vs-published is red on main too (npm published past the scaffold pins), it has nothing to do with these docs, and the one-command fix unblocks it while fixing a real defect. Doc-only. * docs(handoff): record the pins blocker as fixed by cli#194 The `pins-vs-published` blocker described here is resolved — cli#194 (189d5a4) bumped @civitai/app-sdk ^0.28.0 -> ^0.30.0 and @civitai/blocks-react ^0.37.0 -> ^0.38.0 via the repo's own bump-pins command. Three corrections to what this doc told the next session: - The bumper rewrites scaffold_test.go's assertions ITSELF, along with the package.json.tmpl and README.md.tmpl literals. The doc's "then update the matching assertions in scaffold_test.go" implied a hand-edit that would have been redundant. - The blocker is RECURRING, not a one-off: it fires whenever npm publishes a new @civitai/* minor, on any open PR regardless of content. Noted, with a pointer to check why bump-scaffold-pins.yml did not open the bump PR on its own. - Recorded the one thing the bumper gets slightly wrong (a README prose line that overstates the minimum SDK version post-bump), so it is not rediscovered. Also renumbers the ranked list and strikes the two completed items. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9 * docs(handoff): correct the bump-pins lead — cron latency, not a broken workflow My previous commit told the next session to "check why bump-scaffold-pins.yml did not open the bump PR itself". Measured it instead of leaving the hint: the workflow is fine and the framing was wrong. The scheduled run fired Mon 2026-08-03 10:51 UTC and correctly no-op'd. Both publishes landed LATER THE SAME DAY — @civitai/blocks-react 0.38.0 at 20:15 UTC, @civitai/app-sdk 0.30.0 at 22:13 UTC (npm registry `time` field). A successful no-op run and a broken detector look identical from the run list alone; the publish timestamps are the signal that separates them. The real exposure is the weekly cron (`17 7 * * 1`): a required check can sit red on main and every open PR for up to ~7 days after any @civitai/* minor. Records the two consequences — the fixed remedy (or `gh workflow run`), and that the lever is cron frequency, flagged as ask-first per AGENTS.md. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_017vvMdxcMKJP9ripQLEiPm9 --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a browser-based OAuth device-authorization grant to the CLI so
civitai login(no flags) authenticates via the browser and stores auto-refreshing tokens that work forapp submitandwhoami. Implements the server contract from civitai/civitai PR #2644.What changed
civitai login(no--token): device init (POST /api/auth/oauth/device) → print the verification URL + user code (best-effort browser-open ofverification_uri_complete;--no-browserto skip) → pollPOST /api/auth/oauth/device-tokeneveryintervals, respectingslow_down(interval bump) and stopping atexpires_in→ store tokens.expired_token/access_deniedsurface a clear terminal error.civitai login --token <key>stays unchanged (personal API key, no refresh).internal/config): storesaccess_token,refresh_token, absolutetoken_expiry,scope, and anauth_kindmarker (oauthvstoken), alongside the legacy single-tokenpersonal key. Backward compatible, written0600.CIVITAI_TOKENenv is always treated as a personal key.internal/auth.Source, anapi.TokenSource): before a request it refreshes an expired OAuth access token (POST /api/auth/oauth/token), persists the rotated refresh token if the server returns one, and retries once on a 401. A personal key is served as-is with no refresh.whoami+app submitboth use it, so they refresh transparently.app submituploads by default to the v1 token route/api/v1/blocks/submit-version(CIVITAI_SUBMIT_PATHstill overrides). Stale "not yet automated" copy + README updated.Client id
civitai-cliis a public (PKCE/device) client — no secret. Scope33554433=UserRead|AppBlocksSubmit, hardcoded as CLI constants.Tests (
go build ./... && go test ./...green)httptest: init → twoauthorization_pendingpolls → 200 success → tokens persisted (config contents +0600asserted).slow_downincreases the interval;expired_token/access_deniedterminal errors; overall timeout atexpires_in.app submitposts to/api/v1/blocks/submit-versionwith the storedBearer+bundleBase64body.--token) path still works and does not refresh.Self-audit
Tokens persisted
0600, never logged/printed (the test asserts thedevice_codesecret doesn't leak); refresh rotation handled (old refresh token retained when not rotated); the personal-key path is unchanged; submit defaults to the v1 token route.Not verified
Real end-to-end against prod — needs PR #2644 merged and the
civitai-cliOAuth client row provisioned server-side.🤖 Generated with Claude Code