Skip to content

Allow all SKUs to use BYOK #242

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions src/extension/byok/common/byokProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,41 @@ export function resolveModelInfo(modelId: string, providerName: string, knownMod
};
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for comments if they were not there before. I know some devs prefer code with 0 comments.

* Determines if Bring Your Own Key (BYOK) functionality is enabled for the current user.
*
* BYOK availability rules:
* - Internal users: Always enabled
* - Individual users: Always enabled (backward compatibility)
* - Business/Enterprise users: Requires "Editor Preview Features" to be enabled by admin
* - GitHub Enterprise Server: Not available
*
* @param copilotToken The user's Copilot token (without the actual token value)
* @param capiClientService Service to check if running on GitHub Enterprise
* @returns true if BYOK should be enabled for this user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @returns true if BYOK should be enabled for this user
* @returns whether BYOK should be enabled for this user

*/
export function isBYOKEnabled(copilotToken: Omit<CopilotToken, "token">, capiClientService: ICAPIClientService): boolean {
const isGHE = capiClientService.dotcomAPIURL !== 'https://api.github.com';
const byokAllowed = (copilotToken.isInternal || copilotToken.isIndividual) && !isGHE;
return byokAllowed;

// Not available on GitHub Enterprise
if (isGHE) {
return false;
}

// Always allow for internal users
if (copilotToken.isInternal) {
return true;
}

// Allow for individual users without preview features gating (backward compatibility)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Allow for individual users without preview features gating (backward compatibility)
// Allow for individual users without preview feature-gating (backward compatibility)

if (copilotToken.isIndividual) {
return true;
}

// For business and enterprise users, require Editor Preview Features to be enabled
if (copilotToken.copilotPlan === 'business' || copilotToken.copilotPlan === 'enterprise') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to hard code 'business', 'enterprise'. There is probably some enum somewhere you can use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't think there is an enum - this value is currently defined as a string literal union:

get copilotPlan(): 'free' | 'individual' | 'individual_pro' | 'business' | 'enterprise' {

return copilotToken.isEditorPreviewFeaturesEnabled();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 enterprises have preview features enabled.
So I would not go down this route. For now I would just enable for insiders, and if GH wants a policy to disable once they deliver we can return that boolean. And I hope that is ENABLED by default.

}

return false;
}