Skip to content

[AUTOMATION] fix: optimize auth login scope deduplication#269

Open
michiosw wants to merge 1 commit into
mainfrom
fix/oidc-login-scope-dedup
Open

[AUTOMATION] fix: optimize auth login scope deduplication#269
michiosw wants to merge 1 commit into
mainfrom
fix/oidc-login-scope-dedup

Conversation

@michiosw

@michiosw michiosw commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

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

Before this, resolveLoginScopes in internal/auth/oidc.go rescanned the growing output slice for every requested scope, which made duplicate-heavy login scope lists do more work than needed.

Now a seen set is the canonical path:

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

Why

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

login request
-> resolveLoginScopes
-> deduplicated ordered scopes

This PR does not broaden behavior beyond the optimization scope.

What changed

Optimized login scope deduplication in internal/auth/oidc.go
Removed repeated linear rescans while building the scope list
Preserved base-scope ordering and first-seen custom scope ordering
Updated tests for duplicate custom scopes

Verification

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

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

Before this, resolveLoginScopes in internal/auth/oidc.go rescanned the growing output slice for every requested scope, which made duplicate-heavy scope lists cost more work than needed.

Now a seen set is the canonical path:

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

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

login request
-> resolveLoginScopes
-> deduplicated ordered scopes

This change does not broaden behavior beyond the optimization scope.

What changed
Optimized login scope deduplication in internal/auth/oidc.go
Removed repeated linear rescans while building the scope list
Preserved base-scope ordering and first-seen custom scope ordering
Updated tests for duplicate custom scopes

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

michiosw commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

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

@michiosw michiosw changed the title fix(auth): deduplicate login scopes in one pass [AUTOMATION] fix: optimize auth login scope deduplication Jun 9, 2026
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates login scope resolution to remove duplicate custom scopes. The main changes are:

  • Seeds scope tracking from the default identity scopes.
  • Deduplicates custom scopes in a single pass while preserving first-use order.
  • Adds a regression test for repeated custom scopes.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.
  • The new map-based deduplication preserves the existing scope order.
  • The added test covers the new duplicate custom-scope case.

Important Files Changed

Filename Overview
internal/auth/oidc.go Replaces linear scope checks with a seeded map while preserving existing resolved scope output.
internal/auth/oidc_test.go Adds coverage for duplicate custom login scopes and default-scope deduplication.

Reviews (1): Last reviewed commit: "fix(auth): deduplicate login scopes in o..." | 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