-
Notifications
You must be signed in to change notification settings - Fork 0
Remove global setToken usage and refactor token handling to installation-scoped injection #1409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Remove global setToken usage and refactor token handling to installation-scoped injection #1409
Conversation
…precated shared/src/auth.ts and removed all usages\n- Refactored shared GitHub utilities to use repo-scoped installation auth:\n - usecases/workflows/autoResolveIssue now derives installation token and\n passes it through; no global state\n - lib/utils/utils-server now builds clone URL via installation token\n - lib/github/refs uses installation GraphQL client\n - lib/github/issues and lib/github/pullRequests use installation-scoped\n Octokit/GraphQL per repo\n - Stubbed deprecated shared/lib/github getOctokit/getGraphQLClient\n to enforce token injection pattern\n - Marked shared GraphQL listUserRepositories as unavailable in shared\n- Updated worker orchestrator to stop setting a global access token\n\nThis removes concurrency-unsafe global token state and injects tokens in the\nproper context (GitHub App installation per repository).
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
|
I’ll first inspect [3 tools called] I’ve confirmed how [7 tools called] Clarify how
|
shared/src/lib/utils/utils-server.ts
Outdated
| owner, | ||
| repo, | ||
| }) | ||
| cloneUrl = getCloneUrlWithAccessToken(repoFullName, installationToken) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this workflow of setupLocalRepository could be done with different types of tokens (user session token, installation token), we should abstract this getCloneUrlWithAccessToken a bit. The subscriber to this function should determine which token to use via injection. So this function can just focus on getting the cloneUrl, without worrying about the actual implementation.
See if you can help separate the concerns here, and make sure you update any subscribers to this function to make sure they follow the new approach.
| } | ||
| // Use installation-scoped Octokit for this repository (no global token) | ||
| const installation = await getInstallationFromRepo({ owner, repo }) | ||
| const installationOctokit = await getInstallationOctokit(installation.data.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Authentication here can also be injected, depending on the consumer's context. They may use user session tokens or installation tokens. Elsewhere in this PR we talked about a GitHub auth provider, let's see if we can reuse that here.
Merged latest changes from main while preserving the feature branch's approach of removing global token state in favor of installation-scoped GitHub App authentication. Key conflict resolutions: - Removed all usage of setAccessToken/getAccessToken global functions - Kept installation-scoped authentication via getInstallationTokenFromRepo - Updated import paths from 'shared/' to '@/shared/' per main's changes - Preserved functionality while eliminating concurrency-unsafe global state This maintains the architectural goal of using proper installation-scoped tokens per repository rather than global token state.
Summary
Key Changes
shared/src/usecases/workflows/autoResolveIssue.ts
shared/src/lib/utils/utils-server.ts
shared/src/lib/github/refs.ts
shared/src/lib/github/issues.ts & shared/src/lib/github/pullRequests.ts
shared/src/lib/github/index.ts
shared/src/lib/github/graphql/queries/listUserRepositories.ts
apps/workers/workflow-workers/src/orchestrators/autoResolveIssue.ts
Why
Notes
Follow-ups
Closes #1408