Skip to content

[AUTOMATION] fix: optimize internal/auth login scope resolution#242

Open
michiosw wants to merge 1 commit into
mainfrom
fix/optimize-login-scope-resolution-daily
Open

[AUTOMATION] fix: optimize internal/auth login scope resolution#242
michiosw wants to merge 1 commit into
mainfrom
fix/optimize-login-scope-resolution-daily

Conversation

@michiosw

@michiosw michiosw commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary
This optimizes login scope resolution by deduplicating requested scopes in one pass.

Before this, resolveLoginScopes in internal/auth/oidc.go rescanned the resolved slice for every requested scope, which made repeated or long custom scope lists more expensive than needed.

Now a small seen-set is the canonical path:

for _, scope := range scopes {
	if _, exists := seen[scope]; exists {
		continue
	}
	seen[scope] = struct{}{}
	resolved = append(resolved, scope)
}

Why
This gives kontext-cli a cheaper runtime path for browser login scope setup:

login input scopes
-> resolveLoginScopes
-> stable deduplicated OAuth scope list

This PR does not broaden behavior beyond the optimization scope.

What changed
Optimized internal/auth/oidc.go scope deduplication
Removed repeated linear scans during custom scope resolution
Preserved scope order and default-versus-custom base scope behavior
Updated tests for repeated custom scopes

Verification
go test ./internal/auth
go test ./internal/guard/judge -run TestStartLlamaServerHealthCheckAndStop -count=1
go test ./...
go vet ./...
git diff --check

michiosw commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@greptile-apps

greptile-apps Bot commented Jun 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR optimizes login scope resolution in the auth package. The main changes are:

  • Replaces repeated linear scope checks with a seen map.
  • Deduplicates repeated custom login scopes while preserving first-seen order.
  • Adds a test for repeated custom scope deduplication.

Confidence Score: 3/5

This should be fixed before merging.

  • Custom-scope logins can no longer request only the caller-provided scope.
  • The gateway reauth fallback can ask for gateway:access but send extra identity scopes to the authorization server.
  • The new unit test covers deduplication but not the actual authorization request produced by Login.

internal/auth/oidc.go

Important Files Changed

Filename Overview
internal/auth/oidc.go Changes the scope resolver used by browser login requests.
internal/auth/oidc_test.go Adds coverage for duplicate custom scope handling.

Comments Outside Diff (1)

  1. internal/auth/oidc.go, line 201-204 (link)

    P1 Preserve exact custom scopes

    When Login is called with a specific custom scope, this branch still prepends openid, email, and profile to the browser authorization request. The gateway fallback calls login(..., "gateway:access") to recover from a missing gateway grant, but the real request becomes openid email profile gateway:access instead of the exact gateway:access scope that flow asks for. An authorization server that constrains this reauth flow to the missing gateway scope can reject the browser login with invalid_scope, so the connect fallback still fails.

Reviews (1): Last reviewed commit: "fix: optimize login scope resolution" | Re-trigger Greptile

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