Add interactive login by ramilamparo · Pull Request #64 · Hypersequent/qas-cli

ramilamparo · 2026-04-08T13:29:12Z

Summary

Adds interactive qasphere auth commands for login, logout, and status — replacing the need to manually manage API keys and environment variables.

For testing purposes, you can override the QAS_LOGIN_SERVICE_URL.

Login methods

Device Authorization Grant (RFC 8628) — default flow. Opens a browser to the QA Sphere verification page, displays a user code, and polls until the user approves. No secrets typed into the terminal.

Credential storage

Credentials are persisted in priority order:

System keyring (@napi-rs/keyring) — preferred, used when the OS keyring daemon is available
File fallback (~/.config/qasphere/credentials.json) — restricted permissions (0600), used when keyring is unavailable

Credential resolution

resolveCredentialSource() checks sources in priority order:

QAS_TOKEN + QAS_URL environment variables
.env file in cwd
System keyring
~/.config/qasphere/credentials.json
.qaspherecli file (searched up directory tree)

Auth subcommands

Command	Description
`qasphere auth login`	Device authorization flow (browser-based)
`qasphere auth status`	Shows current auth source and validates the API key
`qasphere auth logout`	Clears keyring or file credentials; warns if env-based credentials remain

claude · 2026-04-08T14:37:34Z

Code Review: Add interactive login

Read review instructions
Gather full PR diff and context
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings and post review with inline comments
Clean up outdated bot comments and threads

Running specialized review subagents...

View job run

claude · 2026-04-15T09:57:20Z

Claude finished @ramilamparo's task in 8m 37s —— View job

Code Review: Add interactive login

Gather PR context and full diff
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings and post review
Clean up outdated bot comments and threads

The latest push (9837ba6 — "Fixes") addresses three open items from prior reviews: replace('-', '') → replaceAll, sourceLabels now includes keyring/credentials.json, and resolveAuth error output uses console.error.

Ran four specialized subagents against the full PR diff. No new noteworthy findings beyond what prior reviews have already raised. Five open items remain unaddressed but none are blocking:

oauth.ts:134 — refreshAccessToken omits client_id
browser.ts:15 — Windows cmd /c start shell metacharacter handling
keyring.ts:22 — loadKeyringModule() cache opportunity
oauth.ts — OAuth responses lack Zod runtime validation
auth-e2e.spec.ts — test gaps (default OAuth error branch, polling network error, keyring-backed refresh persistence, clearCredentials failure in logout)

Resolved 3 now-addressed bot threads, minimized 14 outdated bot review summaries, and deleted 1 progress-tracking comment. Submitted as APPROVE.

hi-rai

PR looks okay. Few minor nits

hi-rai · 2026-04-21T10:35:06Z

+		console.log(`  Status: ${chalk.red('invalid or expired')}`)
+	}
+
+	if (result.authType === 'bearer') {


We should show this only if token is still valid. Also we should show the refresh token expiry instead of access token expiry ?

Also we should show the refresh token expiry instead of access token expiry ?

Yes I think that's a more useful. But currently the API expires_in only returns the expiry time in seconds for access token. Should we just hardcoded constant 90 day expiry, or should we get this from backend?

Okay only the ListAuthorizations API returns it - which we use to show the expiry on UI. The /oauth/token authorization endpoint currently doesn't return it

The spec doesn't require the refresh token expiry to be present. But we can add it as a non-standard extension. Many providers do it as well. I will open a PR for it

hi-rai · 2026-04-21T10:36:33Z

+	keyring: 'the system keyring',
+	'credentials.json': 'a credentials file (~/.config/qasphere/credentials.json)',


Should we expose the internal details of how the oauth credentials are stored to the user ? Or simply mention oauth ?

I don't see the harm. Showing "OAuth" could be a bit misleading. Since keyring also uses OAuth.

hi-rai · 2026-04-21T10:39:03Z

+	const url = verification_uri_complete || `${verification_uri}?code=${normalizedCode}`
+	console.log(`\nIf the browser didn't open, visit:\n  ${url}\n`)
+	openBrowser(url)
+	console.log(`Verify the code displayed in the browser: ${chalk.bold(normalizedCode)}\n`)


nit: I guess the non-normalized code would be better for readability ?

I was thinking because verification_url_complete returns non-hyphenated code in the url http://tenant1.localhost:5173/settings/oauth/device?code=M4NGNW8R

So frontend will show the non-hyphenated code as well. User might confuse that hyphenated code missing is invalid. So I normalized it to non-hyphenated code.

The input on the UI currently automatically adds the hyphens. I will do the same for the final confirmation (it currently doesn't show hyphens). I guess we can ignore the URL

claude · 2026-04-24T12:26:53Z

Claude finished @ramilamparo's task in 10m 18s —— View job

Code Review: Add interactive login

Gather PR context and full diff
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings, deduplicate against existing comments, and post review
Clean up outdated bot comments and threads

The latest commit (b103da8 — "Tweaks") addresses hi-rai's review feedback: REFRESH_TOKEN_GRANT_TYPE constant added, status message changed to "Credentials connected via", token expiry only shown when status is valid, more specific revocation instructions, and simplified "Remove them manually." text.

Ran four specialized subagents against the full PR diff. No new noteworthy findings beyond what prior reviews have already raised. Two previously-open test gaps are now addressed (clearCredentials failure path at line 1001, keyring-backed refresh persistence at line 945). Resolved 2 now-addressed bot threads and minimized 13 outdated bot review summaries. Submitted as APPROVE.

claude · 2026-04-24T13:36:02Z

Claude finished @ramilamparo's task in 10m 25s —— View job

Code Review: Add interactive login

Gather PR context and full diff
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings, deduplicate against existing comments, and post review
Clean up outdated bot comments and threads

The latest commit (ba1fe61 — "Improve command description") updates the auth status describe text to better reflect its behavior, addressing hi-rai's feedback.

Ran four specialized subagents against the full PR diff. Two new findings posted as inline comments:

types.ts:9 — tenantUrl schema uses .min(1) instead of .url(), missing URL format validation at the credential boundary
resolvers.ts:153 — resolveAuth() error message leads with "Missing required environment variables" when OAuth is now the primary auth path

Minimized 16 outdated bot review summaries. Three open items from prior reviews remain unaddressed but none are blocking. Submitted as APPROVE.

claude

The latest commit (ba1fe61 — "Improve command description") updates the auth status describe text to better reflect its behavior, addressing hi-rai's feedback.

Ran four specialized subagents (code quality, security, performance, test coverage) against the full PR diff. Two new minor findings posted inline — tenantUrl schema validation and the resolveAuth error message ordering. Both are low severity.

Three open items from prior reviews remain unaddressed but none are blocking:

oauth.ts:134 — refreshAccessToken omits client_id (consistency with other OAuth calls)
browser.ts:15 — Windows cmd /c start shell metacharacter handling (URLs are server-controlled, low practical risk)
keyring.ts:22 — loadKeyringModule() cache opportunity

LGTM.

claude · 2026-04-24T13:44:10Z

+	accessToken: z.string().min(1),
+	refreshToken: z.string().min(1),
+	accessTokenExpiresAt: z.string().datetime(), // ISO 8601
+	tenantUrl: z.string().min(1),


Data integrity (low): tenantUrl validated as non-empty string, not as a URL

z.string().min(1) allows any non-empty string. When credentials are loaded from the keyring or credentials.json, a corrupted value (e.g., "not-a-url") would pass schema validation and then be used as the API base URL for all subsequent requests, producing confusing connection errors downstream.

Given that accessTokenExpiresAt already uses the tighter .datetime() validator (fixed in a prior review), applying the same pattern here would catch corruption at load time with a clear error:

Suggested change

tenantUrl: z.string().min(1),

tenantUrl: z.string().url(),

claude · 2026-04-24T13:44:17Z

+		console.error(
+			chalk.red('Missing required environment variables: ') +
+				qasEnvs.filter((k) => !process.env[k]).join(', ')


UX (low): Error message leads with env vars when OAuth is now the primary auth method

When no credentials are found, the first thing the user sees is "Missing required environment variables: QAS_TOKEN, QAS_URL". Since this PR introduces auth login as the recommended path, the message could confuse new users into thinking env vars are required.

The auth login suggestion is already here (line 158), but it's buried after the env-var error. Consider leading with the OAuth path:

console.error(chalk.red('Not authenticated.')) console.error('\nLog in using:') console.error(chalk.green(' qasphere auth login')) console.error('\nOr set QAS_TOKEN and QAS_URL via environment variables or a .qaspherecli file.')

claude · 2026-04-24T14:46:19Z

Code Review: Add interactive login

Gather PR context and full diff
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings, deduplicate against existing comments, and post review
Clean up outdated bot comments and threads

Running specialized review subagents...

View job run

claude · 2026-04-24T15:09:18Z

Code Review: Add interactive login

Read review instructions
Gather full PR diff and context
Run specialized review subagents (code quality, security, performance, test coverage)
Aggregate findings, deduplicate against existing comments, and post review
Clean up outdated bot comments and threads

Gathering PR context...

View job run

Add interactive login

131273b

This comment was marked as outdated.

Sign in to view

ramilamparo added 2 commits April 8, 2026 18:21

Address PR comments

43c66ed

Rename fields

9601435

ramilamparo added 2 commits April 8, 2026 18:42

Fix mocks

dd34f0a

Address feedbacks

a48a8ca

This comment was marked as outdated.

Sign in to view

Few improvements

6b05689

ramilamparo requested a review from a team April 9, 2026 13:40

ramilamparo force-pushed the dev/ram/interactive-login branch from 8908deb to 6b05689 Compare April 9, 2026 14:10

This comment was marked as outdated.

Sign in to view

Clean up a bit

dda926e

This comment was marked as outdated.

Sign in to view

ramilamparo added 2 commits April 9, 2026 18:43

Import type

a22c113

Refactor code to use standard oauth implementation.

335c096

This comment was marked as outdated.

Sign in to view

Check suspended flag

f35cc5e

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into dev/ram/interactive-login

2029f33

This comment was marked as outdated.

Sign in to view

Fix race condition in mocked imports

a29c5ce

This comment was marked as outdated.

Sign in to view

Update agent skill

399de0f

This comment was marked as outdated.

Sign in to view

Update table of contents

96eb40b

This comment was marked as outdated.

Sign in to view

Fix issues

a26cbd1

This comment was marked as outdated.

Sign in to view

Merge branch 'main' into dev/ram/interactive-login

b2a6f28

This comment was marked as outdated.

Sign in to view

Fixes

9837ba6

This comment was marked as outdated.

Sign in to view

hi-rai approved these changes Apr 21, 2026

View reviewed changes

Tweaks

b103da8

This comment was marked as outdated.

Sign in to view

Improve command description

ba1fe61

claude Bot approved these changes Apr 24, 2026

View reviewed changes

Add refresh token expiration

90bac86

Nits

5ce6d76

		keyring: 'the system keyring',
		'credentials.json': 'a credentials file (~/.config/qasphere/credentials.json)',

Conversation

ramilamparo commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Login methods

Credential storage

Credential resolution

Auth subcommands

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

claude Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: Add interactive login

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

claude Bot commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review: Add interactive login

Uh oh!

This comment was marked as outdated.

Uh oh!

hi-rai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hi-rai Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

ramilamparo Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

hi-rai Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

hi-rai Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

ramilamparo Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hi-rai Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

ramilamparo Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

hi-rai Apr 24, 2026

Choose a reason for hiding this comment

ramilamparo commented Apr 8, 2026 •

edited

Loading

claude Bot commented Apr 8, 2026 •

edited

Loading

claude Bot commented Apr 15, 2026 •

edited

Loading

ramilamparo Apr 24, 2026 •

edited

Loading

claude Bot commented Apr 24, 2026 •

edited

Loading

claude Bot commented Apr 24, 2026 •

edited

Loading

claude Bot commented Apr 24, 2026 •

edited

Loading

claude Bot commented Apr 24, 2026 •

edited

Loading