Skip to content
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

Add support for GitHub Copilot #333

Closed
wants to merge 10 commits into from

Conversation

cloudbridgeuy
Copy link
Contributor

Adds support to interact with OpenAi GPT-4 Models using Copilot.

Copy link
Member

@caarlos0 caarlos0 left a comment

Choose a reason for hiding this comment

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

hey, thanks for the PR!

just a couple of comments

Comment on lines +362 to +368
fileContent := string(data)
re := regexp.MustCompile(`.*oauth_token...|\".*`)
key = re.ReplaceAllString(fileContent, "")
if key == "" {
return fmt.Errorf("No Copilot OAuth token found in %s", filePath)
}
key = strings.TrimSpace(key)
Copy link
Member

Choose a reason for hiding this comment

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

probably better to unmarshal the json here instead of string manipulating it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are probably right. I rewrote this code from bash and never even thought of doing that.

Comment on lines +355 to +356
var key string
if cfg.API == "copilot" {
Copy link
Member

Choose a reason for hiding this comment

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

these should probably be below, where there is a switch case on cfg.API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure there's a switch on cfg.API? I can't find it.

Copy link
Member

Choose a reason for hiding this comment

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

sorry, its on mod.API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I tried to make it work inside the mod.API switch statement but rolled back the changes because I wanted to keep as much of the OpenAI logic as possible, and only change the env and http endpoint values.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's a good idea, as the way you did it will also use the openai model from the configuration, which I don't think is correct.

Copy link
Contributor Author

@cloudbridgeuy cloudbridgeuy Sep 11, 2024

Choose a reason for hiding this comment

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

Yes, it will only work if you set it to gpt-4o or one its supported variants. But there's no need to be too specific. Using gpt-4o is enough.

I currently run this only with:

  copilot:
    base-url: https://api.githubcopilot.com

And,

default-model: gpt-4o

@caarlos0
Copy link
Member

something like this:

diff --git a/config_template.yml b/config_template.yml
index 432a580..d806436 100644
--- a/config_template.yml
+++ b/config_template.yml
@@ -92,6 +92,12 @@ apis:
         fallback:
   copilot:
     base-url: https://api.githubcopilot.com
+    models:
+      gpt-4o-2024-05-13:
+        max-input-chars: 392000
+        fallback: gpt-4
+      gpt-4:
+        max-input-chars: 392000
   anthropic:
     base-url: https://api.anthropic.com/v1
     api-key:
diff --git a/mods.go b/mods.go
index 0f2ea63..2017dc2 100644
--- a/mods.go
+++ b/mods.go
@@ -3,6 +3,7 @@ package main
 import (
 	"bufio"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -351,34 +352,25 @@ func (m *Mods) startCompletionCmd(content string) tea.Cmd {
 			if mod.API == "azure-ad" {
 				ccfg.APIType = openai.APITypeAzureAD
 			}
+		case "copilot":
+			filePath := os.Getenv("HOME") + "/.config/github-copilot/hosts.json"
+			bts, err := os.ReadFile(filePath)
+			if err != nil {
+				return err
+			}
+			hosts := map[string]map[string]string{}
+			if err := json.Unmarshal(bts, &hosts); err != nil {
+				return err
+			}
+			authToken := hosts["github.com"]["oauth_token"]
+			ccfg = openai.DefaultConfig(authToken)
+			ccfg.BaseURL = api.BaseURL
 		default:
 			var key string
-			if cfg.API == "copilot" {
-				filePath := os.Getenv("HOME") + "/.config/github-copilot/hosts.json"
-				data, err := os.ReadFile(filePath)
-				if err != nil {
-					return err
-				}
-				fileContent := string(data)
-				re := regexp.MustCompile(`.*oauth_token...|\".*`)
-				key = re.ReplaceAllString(fileContent, "")
-				if key == "" {
-					return fmt.Errorf("No Copilot OAuth token found in %s", filePath)
-				}
-				key = strings.TrimSpace(key)
-				ccfg = openai.DefaultConfig(key)
-				for _, a := range cfg.APIs {
-					if "copilot" == a.Name {
-						api = a
-						break
-					}
-				}
-			} else {
-				var err error
-				key, err = m.ensureKey(api, "OPENAI_API_KEY", "https://platform.openai.com/account/api-keys")
-				if err != nil {
-					return err
-				}
+			var err error
+			key, err = m.ensureKey(api, "OPENAI_API_KEY", "https://platform.openai.com/account/api-keys")
+			if err != nil {
+				return err
 			}
 			ccfg = openai.DefaultConfig(key)
 			if api.BaseURL != "" {
@@ -447,7 +439,7 @@ func (m *Mods) handleRequestError(err error, mod Model, content string) tea.Msg
 	if errors.As(err, &ae) {
 		return m.handleAPIError(ae, mod, content)
 	}
-	return modsError{ae, fmt.Sprintf(
+	return modsError{err, fmt.Sprintf(
 		"There was a problem with the %s API request.",
 		mod.API,
 	)}

although, testing this, it doesn't seem to work...

@cloudbridgeuy
Copy link
Contributor Author

Yes, that doesn't work because of the way we select the value of mod.API. At that point in the code, the value is openai, thus it never falls into that case statement.

That's why I added the check inside the default case.

@caarlos0
Copy link
Member

Yes, that doesn't work because of the way we select the value of mod.API. At that point in the code, the value is openai, thus it never falls into that case statement.

it does though

CleanShot 2024-09-11 at 11 15 17@2x

@caarlos0 caarlos0 mentioned this pull request Sep 11, 2024
@caarlos0
Copy link
Member

tried to get it to work in #345, but I always get error responses from copilot... I think we're missing something, not sure what though

@cloudbridgeuy
Copy link
Contributor Author

cloudbridgeuy commented Sep 13, 2024

Mmm... I have it working on my end, albeit with my crude implementation:

image

I'll check that PR and see if I can make it work.

EDIT: I double-checked this by adding some fmt.Println statements to log the URL and Key being used and confirmed they are the ones required to talk to the copilot. Also, you can't see it in the image, but my default mode is set to GPT-4o.

@caarlos0
Copy link
Member

can you run something like mods --api copilot "who are you?"? Just to make sure it is really copilot replying

@cloudbridgeuy
Copy link
Contributor Author

Since they are both using the same model, they tend to answer the same thing, so I added back the logs:

image

@caarlos0
Copy link
Member

yeah, for me both branches give the same error

@cloudbridgeuy
Copy link
Contributor Author

cloudbridgeuy commented Sep 13, 2024

Mmm... If you run this from your console, do you get an answer?

curl -n "https://api.githubcopilot.com/chat/completions" \
  -H "Content-Type: application/json" \
  -H "Authorization: Bearer $(cat ~/.config/github-copilot/hosts.json | sed -e 's/.*oauth_token...//;s/\".*//')" \
  -d '{
    "model": "gpt-4o",
    "messages": [
      {
        "role": "system",
        "content": "You are a helpful assistant."
      },
      {
        "role": "user",
        "content": "Who won the world series in 2020?"
      },
      {
        "role": "assistant",
        "content": "The Los Angeles Dodgers won the World Series in 2020."
      },
      {
        "role": "user",
        "content": "Where was it played?"
      }
    ]
  }'

This was the code I was trying to port to mods.

@caarlos0
Copy link
Member

I get an "unknown integration" error

@cloudbridgeuy
Copy link
Contributor Author

That command work for me, I wonder what's the difference.

@caarlos0
Copy link
Member

That command work for me, I wonder what's the difference.

same... no idea, it works inside neovim apparently... dunno what's the difference 🤔

@caarlos0 caarlos0 closed this in #345 Nov 4, 2024
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.

3 participants