Skip to content

Commit 8fbdbc2

Browse files
anshulsaoclaude
andauthored
feat(login): reuse stored token, skip browser when valid (#15)
* feat(login): reuse stored token, skip browser when valid `praxis login` now reuses the active profile's stored credentials instead of opening the browser on every invocation. Re-running login (the documented way to refresh skills + MCP manifest) is now a no-browser operation when the profile already holds a valid token. Precedence in loginCmd.RunE: - --token X → unchanged (verify, save, post-auth setup) - --force → skip reuse, always open the browser - else → reuse: if the active profile has a token AND the resolved URL == the stored profile URL, verify via /ai-api/auth/me. Valid → persist + post-auth setup, no browser. Invalid/expired → graceful one-line fallback to the browser flow. Also: - resolveLoginURL now errors for a NEW *named* profile without --url (no URL to reuse, and guessing askpraxis.ai would be wrong). The `default` profile keeps the built-in fallback for zero-config first run. - Extract the persist + post-auth tail of saveAndVerifyToken into a shared persistAndSetup() helper used by both the verify-then-save path and the reuse path (reuse needs an error return for graceful fallback, not os.Exit). - browserLoginFn / postAuthSetup package seams so RunE path selection and persistence are unit-testable without a browser, network, or real skill installs. Switching profiles still resets org skills (wipe praxis-* + reinstall the new profile's catalog); reuse only skips the browser/key-creation step, the post-auth setup runs unchanged. TDD: 13 tests added covering URL resolution, the reuse decision, and RunE precedence. Full `go test -race ./...` green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(login): table-driven reuse tests + assert error contents Address CodeRabbit review on PR #15, aligning with the repo's testing conventions (CLAUDE.md): - Consolidate the four resolveLoginURL tests and the four tryReuseStoredToken tests into table-driven tests. - Assert error *contents* (substring "--url") rather than just err == nil for the new-named-profile-without-URL cases, in both resolveLoginURL and the RunE-level test. RunE precedence tests stay as separate functions — each exercises a distinct branch with its own seam stubbing and browser-called/error assertions, so a shared table would obscure more than it consolidates. No production code change; full go test -race ./... green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ff08616 commit 8fbdbc2

2 files changed

Lines changed: 435 additions & 18 deletions

File tree

cmd/login.go

Lines changed: 110 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,24 @@ var (
3232
loginProfile string
3333
loginURL string
3434
loginToken string
35+
loginForce bool
3536
loginJSON bool
3637
loginTimeout time.Duration
3738
)
3839

40+
// browserLoginFn and postAuthSetup are package-level seams so tests can
41+
// exercise login's path selection (reuse vs. browser) and persistence
42+
// without opening a browser, hitting the network, or installing skills.
43+
var (
44+
browserLoginFn = browserSessionPollLogin
45+
postAuthSetup = runPostAuthSetup
46+
)
47+
3948
func init() {
4049
loginCmd.Flags().StringVar(&loginProfile, "profile", "", "save under this profile name (default: \"default\")")
4150
loginCmd.Flags().StringVar(&loginURL, "url", "", "Praxis deployment URL (default: existing profile URL or "+credentials.DefaultURL+")")
4251
loginCmd.Flags().StringVar(&loginToken, "token", "", "skip browser flow; save and verify the given API key directly")
52+
loginCmd.Flags().BoolVar(&loginForce, "force", false, "skip reusing a stored token; always open the browser")
4353
loginCmd.Flags().BoolVar(&loginJSON, "json", false, "JSON output")
4454
loginCmd.Flags().DurationVar(&loginTimeout, "timeout", 90*time.Second, "max time to wait for browser callback")
4555
rootCmd.AddCommand(loginCmd)
@@ -53,7 +63,10 @@ var loginCmd = &cobra.Command{
5363
1. Install the praxis meta-skill into every detected AI host
5464
(~/.claude/skills/praxis, ~/.agents/skills/praxis,
5565
~/.gemini/skills/praxis) — idempotent.
56-
2. Open a browser to create a Praxis API key (or use --token to skip).
66+
2. Reuse the active profile's stored token when it's still valid for
67+
this URL (no browser); otherwise open a browser to create a Praxis
68+
API key. Use --token to supply a key directly, or --force to always
69+
re-authenticate via the browser.
5770
3. Save credentials and flip the active profile pointer.
5871
4. Wipe any praxis-* org skills from the previous profile.
5972
5. Fetch this profile's skill catalog from the server and install
@@ -78,26 +91,92 @@ separate refresh command in v0.7.`,
7891
if profileName == "" {
7992
profileName = credentials.DefaultProfileName
8093
}
81-
baseURL := resolveLoginURL(profileName, loginURL)
94+
baseURL, err := resolveLoginURL(profileName, loginURL)
95+
if err != nil {
96+
render.PrintError(out, asJSON, err.Error(),
97+
"pass --url <https://your-praxis-deployment> to create this profile",
98+
exitcode.Usage)
99+
return err
100+
}
82101

102+
// --token is the explicit non-browser path: verify the supplied
103+
// key and persist it, unchanged by the reuse logic.
83104
if loginToken != "" {
84105
return saveAndVerifyToken(out, asJSON, profileName, baseURL, loginToken)
85106
}
86-
return browserSessionPollLogin(out, asJSON, profileName, baseURL, loginTimeout)
107+
108+
// Smart default: if the active profile already has a token valid
109+
// for this URL, refresh skills + manifest without a browser hop.
110+
// --force opts out and always re-authenticates via the browser.
111+
if !loginForce {
112+
reused, rerr := tryReuseStoredToken(out, asJSON, profileName, baseURL)
113+
if reused {
114+
return rerr
115+
}
116+
}
117+
return browserLoginFn(out, asJSON, profileName, baseURL, loginTimeout)
87118
},
88119
}
89120

90121
// resolveLoginURL resolves the URL for a NEW or EXISTING profile during
91122
// login: explicit --url > existing profile's saved URL > built-in default.
92-
func resolveLoginURL(profileName, flagURL string) string {
123+
//
124+
// A NEW *named* profile (one not present in the store) without --url is
125+
// an error — there's no URL to reuse and guessing askpraxis.ai for a
126+
// named deployment would be wrong. The "default" profile keeps the
127+
// built-in fallback so a zero-config first run still works.
128+
func resolveLoginURL(profileName, flagURL string) (string, error) {
93129
if flagURL != "" {
94-
return flagURL
130+
return flagURL, nil
95131
}
96132
store, _ := credentials.Load()
97133
if p, ok := store[profileName]; ok && p.URL != "" {
98-
return p.URL
134+
return p.URL, nil
135+
}
136+
if profileName == credentials.DefaultProfileName {
137+
return credentials.DefaultURL, nil
138+
}
139+
return "", fmt.Errorf("profile %q does not exist yet; pass --url to create it", profileName)
140+
}
141+
142+
// tryReuseStoredToken attempts a no-browser login using the token already
143+
// stored for profileName.
144+
//
145+
// It returns reused=true only when it has TAKEN OWNERSHIP of the login —
146+
// the stored token verified and the persist+setup tail ran (the returned
147+
// error is that tail's result, nil on success). The caller must NOT fall
148+
// back to the browser in that case.
149+
//
150+
// reused=false means no reuse was possible — no stored token, the profile
151+
// is being re-targeted at a different URL, or the stored token failed
152+
// verification (expired/revoked). The error is always nil here; the caller
153+
// should proceed to the browser flow. A verification failure is reported
154+
// as a one-line notice on stderr, not an error, so the fallback is smooth.
155+
func tryReuseStoredToken(out io.Writer, asJSON bool, profileName, baseURL string) (bool, error) {
156+
store, err := credentials.Load()
157+
if err != nil {
158+
return false, nil // can't read the store — just use the browser
159+
}
160+
prof, ok := store[profileName]
161+
if !ok || prof.Token == "" {
162+
return false, nil // nothing stored to reuse
163+
}
164+
if prof.URL != baseURL {
165+
// Re-targeting this profile at a different URL: the stored token
166+
// belongs to the old deployment, so it can't be reused here.
167+
return false, nil
99168
}
100-
return credentials.DefaultURL
169+
170+
user, err := fetchAuthMe(baseURL, prof.Token)
171+
if err != nil {
172+
if !asJSON {
173+
fmt.Fprintf(os.Stderr,
174+
"Stored token for profile %q is no longer valid (%v); opening browser…\n",
175+
profileName, err)
176+
}
177+
return false, nil // graceful fallback to the browser
178+
}
179+
return true, persistAndSetup(out, asJSON, profileName, baseURL, prof.Token, user.Email)
101180
}
102181

103182
// browserSessionPollLogin opens the browser to the api-keys page with a
@@ -250,6 +329,10 @@ func suggestedKeyName() string {
250329
return "praxis-cli-" + hex.EncodeToString(b)[:5]
251330
}
252331

332+
// saveAndVerifyToken verifies a freshly-obtained token (from --token or
333+
// the browser flow) and persists it. A verification failure here is fatal
334+
// — the user explicitly supplied this key, so there's no graceful
335+
// fallback to attempt.
253336
func saveAndVerifyToken(out io.Writer, asJSON bool, profileName, baseURL, token string) error {
254337
user, err := fetchAuthMe(baseURL, token)
255338
if err != nil {
@@ -259,10 +342,19 @@ func saveAndVerifyToken(out io.Writer, asJSON bool, profileName, baseURL, token
259342
exitcode.Auth)
260343
os.Exit(exitcode.Auth)
261344
}
345+
return persistAndSetup(out, asJSON, profileName, baseURL, token, user.Email)
346+
}
262347

348+
// persistAndSetup saves the verified token under profileName, flips the
349+
// active-profile pointer, runs post-auth setup (meta-skill + catalog +
350+
// MCP manifest), and renders the result. It is the shared tail of both
351+
// the verify-then-save path (saveAndVerifyToken) and the reuse path
352+
// (tryReuseStoredToken). Returning an error rather than os.Exit lets the
353+
// reuse path stay non-fatal up to this point.
354+
func persistAndSetup(out io.Writer, asJSON bool, profileName, baseURL, token, email string) error {
263355
prof := credentials.Profile{
264356
URL: baseURL,
265-
Username: user.Email,
357+
Username: email,
266358
Token: token,
267359
}
268360
if err := credentials.Put(profileName, prof); err != nil {
@@ -274,24 +366,24 @@ func saveAndVerifyToken(out io.Writer, asJSON bool, profileName, baseURL, token
274366

275367
// Post-auth: install meta-skill, wipe previous org skills, install
276368
// this profile's catalog, refresh the MCP tools snapshot.
277-
postAuthState := runPostAuthSetup(out, asJSON, baseURL, token)
369+
state := postAuthSetup(out, asJSON, baseURL, token)
278370

279371
if asJSON {
280372
return render.JSON(out, map[string]any{
281373
"ok": true,
282374
"profile": profileName,
283-
"username": user.Email,
375+
"username": email,
284376
"url": baseURL,
285-
"meta_skill": postAuthState.metaSkill,
286-
"catalog_skills": postAuthState.catalogSkills,
287-
"removed_skills": postAuthState.removedSkills,
288-
"agents": postAuthState.agents,
289-
"removed_agents": postAuthState.removedAgents,
290-
"snapshot_path": postAuthState.snapshotPath,
291-
"snapshot_warning": postAuthState.snapshotWarning,
377+
"meta_skill": state.metaSkill,
378+
"catalog_skills": state.catalogSkills,
379+
"removed_skills": state.removedSkills,
380+
"agents": state.agents,
381+
"removed_agents": state.removedAgents,
382+
"snapshot_path": state.snapshotPath,
383+
"snapshot_warning": state.snapshotWarning,
292384
})
293385
}
294-
fmt.Fprintf(out, "\n✓ Logged in as %s (profile: %s, url: %s)\n", user.Email, profileName, baseURL)
386+
fmt.Fprintf(out, "\n✓ Logged in as %s (profile: %s, url: %s)\n", email, profileName, baseURL)
295387
return nil
296388
}
297389

0 commit comments

Comments
 (0)