Repository navigation
feat: Add GitHub Copilot CLI as a supported tool profile #90 - #128
Conversation
|
@xai I created this draft PR to share what I have done so far. Since yesterday the authentication somehow fails, but I can't figure out why. Maybe you have some hints where to look at. But I think it is better if you can see what I have done so you can give me some suggestions to improve it. |
|
Thanks a lot @fipro78! I'll take a look! |
|
I just tested this branch (on linux). For me it works when using device code authentication! After starting the tool the first time, I ran Next, I tested the browser oauth flow and it seems that copilot picks a random port for the callback (as opposed to e.g. codex' |
|
Regarding the browser oauth flow: I found no (documented or undocumented) way to make it use a fixed port, so we can't do the I was still able to authenticate by following the flow until i get the "Firefox can’t connect to the server at 127.0.0.1:" problem in my browser, copied the full url from its location bar and then just ran One annoying thing is that copilot still asks whether to trust a directory on each start, but I'm sure there's a way around that. |
|
So regarding your authentication problem: Have you tried deleting the credentials from If that fixes it, I wonder wether parallel sessions, especially long running ones could be the problem. We had a lot of trouble with this for Claude in the past. |
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Adds a copilot tool extension (draft, WIP). It installs @github/copilot through the shared npm helper and declares COPILOT_GITHUB_TOKEN as a gateway-injected credential for api.github.com and *.githubcopilot.com. It treats ~/.copilot/config.json as the shared auth file and adds a settings template, an allowlist and an enclave-help skill. Everything is spec-only except a no-op Go handler.
Main points for maintainers:
- Auth failures: Copilot CLI falls back to
GH_TOKEN/GITHUB_TOKEN. With the default github-cli feature, those env vars hold a placeholder that is never released to*.githubcopilot.com. That would also explain why device-code OAuth worked in the maintainer test. See the inline comment oncredentials. - Auth file scope:
config.jsonlikely holds more than auth. Because it is the shared auth file, any state Copilot writes there is shared across projects, and thefile_existssession check will pass after the first start even without a login. The planned folder-trust support depends on getting this right. Codex handles trust in itsentrypoint.d/setup.shbehindENCLAVE_YOLO. - Failing test:
go test ./internal/config/fails because the golden snapshots for copilot are missing. The README and thedocs/tools.mdentry are also missing. - Default image: maintainers should decide whether this ships in the default set or stays
defaultIncluded: falselike mistral-vibe until auth and the random OAuth callback port are settled.
enclave validate-extensions and make check-license-headers pass.
| providers: | ||
| - name: github | ||
| credentials: [copilot-github-token] | ||
| authFiles: [config.json] |
There was a problem hiding this comment.
Is config.json auth only? If Copilot also keeps state there (trusted folders, model, first-launch flags), file_exists passes after the first start without any login, and all of that gets shared across projects via the auth store. A json_pointer check on the token key would be accurate, and this also matters for the folder trust you want to add.
There was a problem hiding this comment.
@xai Not sure what to do here. any hints or suggestions?
There was a problem hiding this comment.
It's not auth only, this is the problem I mentioned in the above inline comment. In copilot, config.json holds both configuration data and login credentials. It is even worse, because it also stores project-specific stuff there, such as folder trust, so it is basically leaks trust information between projects.
If we decide to break /login as I suggested on line 15, we can remove the authFiles stuff here completely, i.e., the provider block becomes:
providers:
- name: github
credentials: [copilot-github-token]
| passthroughPaths: [ | ||
| agents/, | ||
| copilot-instructions.md, | ||
| extensions/, | ||
| hooks/, | ||
| instructions/, | ||
| lsp-config.json, | ||
| mcp-config.json, | ||
| providers.json, | ||
| settings.json, | ||
| skills/] |
There was a problem hiding this comment.
Nit: trailing whitespace on each of these lines, and most new files lack a final newline. A block list like pi/opencode use reads better here.
There was a problem hiding this comment.
Missing from the tool checklist: extensions/tools/copilot/README.md (required per AGENTS.md), a row in docs/tools.md, and the golden snapshots. go test ./internal/config/ currently fails with read golden tool-copilot ... no such file or directory, regenerate with -update-golden.
There was a problem hiding this comment.
This only embeds BaseHandler, which tools.Resolve already returns for unregistered tools (registry). Drop go/ like mistral-vibe and pi. As is, cmd/enclave/tool_imports.go wasn't regenerated, so the package isn't linked anyway.
There was a problem hiding this comment.
cmd/enclave/tool_imports.go was regenerated. it is part of this PR or what is missing here?
There was a problem hiding this comment.
That's ok. Nothing is missing, tool_imports.go was regenerated and looks good. The bot's point was that the handler currently does nothing, since the registry falls back to the base handler anyway. Please keep the file as it is. I can look into that as a follow-up issue after the PR is merged.
I will try tomorrow. But I used the approach with the environment variable and not oauth. Tested today a BYOK setup which worked fine. |
Using the env (with a restrictively scoped token) is probably the safer way. I wonder what the scope of the oauth token really is. I am going to poke around with it a bit to see if it is actually safe for enclave users to authenticate via oauth flow in copilot or if that would mean that someone who snatches the auth token can do write stuff on github. Hopefully that poking around won't get me banned ;) |
Oh ffs!!! Don't use oauth flows with copilot. That token is quite powerful: Especially the We should document how users should use copilot in enclave, i.e., what it actually needs as a minimum scope to function and the recommend the env approach you are using. We should still allow the oauth login method of course, because if users want to do dangerous things they should be able to, but we should make them aware of it. I am really a bit shocked right now that they would issue such a broad token for a simple Edit: github/copilot-cli#953. January. What in the world is wrong with people? In github/copilot-cli#953 (comment), they recommend using |
|
@xai In the network logs I see several entries like this: To verify if there is an issue with the token itself I started the container and connected to the shell like this: enclave --tool copilot shellThen I checked if the agent@e161878e6c43:/home/<user>/<project_folder>$ echo $COPILOT_GITHUB_TOKEN
ENCLAVE_SECRET_cf9b7e5b20bcc533596aeb389085357a46786066453e2b53If I change this to the real token value and not the enclave secret one, I can call The folder I also checked the entries in Is it possible that the resolving of the enclave secret is not working correctly in the copilot tool as I have it configured at the moment? |
|
@xai |
|
Thanks a lot for the update! I'll look into the secret release problem and get back to you. |
|
I can reproduce it.
With only This is a bit annoying of course. I guess we just have to hack our way around the check. I could imagine doing something like this (as an enclave core change): The placeholder would then be |
|
@xai |
Yes! |
|
#143 should do it. Once it's merged, we can rebase this one here, add the configuration bit for copilot and then hopefully everything works! |
|
@xai Done |
|
@xai |
The test expects each tool extension to provide two reference settings files in You can generate the files by running Is the secret injection problem resolved now? |
|
I am blocked by another topic right now. But I will test, confirm and update over the weekend. |
Looks ok now. It worked, but felt a bit slow. Not sure why. I tested the configuration with a I then also tested a BYOK configuration via this setting: {
"tool_overrides": {
"copilot": {
"pass_env": [
"COPILOT_PROVIDER_BASE_URL",
"COPILOT_PROVIDER_API_KEY",
"COPILOT_MODEL"
]
}
},
"allow_domains": [
"<my-llm-gateway-url>"
]
}Which also worked fine, even faster. Of course I can't tell if the performance difference is because of a networking issue or server utilization. Just something I noticed in direct comparison. I updated the PR to add the two reference settings files and I updated the install script to use |
Thanks for the update! I can reproduce the peformance issue, it is indeed very slow! When i run with |
|
FYI: I created #154, which will fix the performance problem we are seeing with copilot. So far, copilot cli has been a very interesting case, as it behaves quite differently from our existing tools and therefore has made some gaps visible. |
|
Update: #154 is merged. If you rebase against main, the performance issues should be resolved. |
|
@xai |
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Follow-up since my last review. Since then: the COPILOT_GITHUB_TOKEN placeholder-prefix workaround landed (uses the new credentials.sources.*.placeholder feature from #143, matches the documented shape), the folder-trust jq/JSONC fix from xai's suggestion was applied, the npm install pin moved to @github/copilot@latest, and the README, docs/tools.md entry, and golden snapshots were added.
Still open from my earlier review and unchanged: the config.json auth-session check is still a bare file_exists (no json_pointer on the token), go/handler.go still just wraps BaseHandler and isn't wired into tool_imports.go, and several files still lack a trailing newline.
Two new comments based on the PR discussion since my last pass: the new README doesn't document the OAuth token's overly broad scope (flagged by the maintainer during testing) or recommend the fine-grained PAT approach, and with that scope issue plus the unpinned OAuth callback port both still open, it's worth considering defaultIncluded: false for now like mistral-vibe.
These previous comments can be resolved as they are now handled:
I can't resolve them myself as I would need write permission on this repository.
| The provider declares `~/.copilot/config.json` as its auth file. Enclave uses | ||
| its presence to detect an existing auth session and includes it in auth | ||
| import/export. See [Authentication & Secrets](../../../docs/auth.md). |
There was a problem hiding this comment.
This doesn't mention the OAuth scope problem from the PR discussion: the device/browser login token comes back with repo (read+write) and write:plugin_gateway_connections, way more than Copilot needs (see the finding). Worth telling users here to prefer COPILOT_GITHUB_TOKEN with a fine-grained PAT and flagging the OAuth risk explicitly, since that's the actual guidance that came out of testing this PR.
| @@ -0,0 +1,59 @@ | |||
| # Copyright (C) 2026 Dirk Fauth and others. | |||
There was a problem hiding this comment.
Two things from the PR discussion are still unresolved: the OAuth token comes back overly broad in scope, and the browser OAuth flow can't be pinned to a fixed callback port (no oauthPorts entry here, unlike codex). Given that, should this ship with defaultIncluded: false for now, same as mistral-vibe (
enclave/extensions/tools/mistral-vibe/spec.yaml
Lines 8 to 13 in 671cca2
There was a problem hiding this comment.
@xai To be honest I am not sure what to do here. Any hints or suggestions?
There was a problem hiding this comment.
I don't think we need defaultIncluded: false. Keep this as it is.
|
Thanks for the great work @fipro78! I'm reviewing and manually testing now and will get back to you with feedback and, of course, an answer to your question above. Overall, it looks good, and I am quite confident we can make it in our 1.0.0 release, which will happen at the end of October, which I would love, because copilot is a really relevant tool for many users. |
xai
left a comment
xai
left a comment
There was a problem hiding this comment.
Thanks for the work!
I have some comments, please look into them and see what you can do.
Copilot has very interesting behavior in some areas and we need to make two decisions, more context in the inline comments:
- do we break
/loginand force users to use a PAT? if we don't there is a weird fallback in case aGH_TOKENis set as well. - copilot does store memory, but on github servers. do we want to disable that by default? this could easily re-enabled by users who want to opt into this.
| description: Copilot CLI | ||
|
|
||
| sandbox: | ||
| entrypoint: { run: [copilot, --disable-builtin-mcps] } |
There was a problem hiding this comment.
My most recent tests showed that copilot reads COPILOT_GITHUB_TOKEN, then GH_TOKEN, then GITHUB_TOKEN, and when its token is rejected it falls back to gh's token and even to the gh CLI's stored login if there is one.
Inside Enclave, the github-cli feature sets GH_TOKEN for a different purpose (and therefore probably a token with a very different access level than a token for copilot requests), and in my test sessions, I watched copilot send that token to api.github.com. We should replace this line with:
entrypoint: { run: [copilot, --disable-builtin-mcps, --auth-token-env, COPILOT_GITHUB_TOKEN, --no-auto-login, --disable-client-telemetry] }
With these flags Copilot uses only COPILOT_GITHUB_TOKEN (verified on 1.0.101 and 1.0.102). This basically disables /login in copilot in Enclave, which IMHO is really unsafe to use currently. If we keep /login by not adding these flags, this basically also means we accept the gh fallback. If you think that we should keep /login nevertheless, also ok for me, your choice!
Either way, I would add --disable-client-telemetry.
There was a problem hiding this comment.
While I totally agree that the OAuth usage sounds unsafe to use. But as you said, it might be a current issue that changes in the future. And also, an open source project should not be the ones restricting usage. So the clear recommendation is to use the token, and this is documented in the readme with a clear statement why. If users decide to take the risk, it's their choice. Therefore I would keep the /login.
There was a problem hiding this comment.
ACK, you convinced me. Users should be in control and we should not make the same mistake that other "opinionated" projects make. Let's keep it.
|
|
||
| sandbox: | ||
| entrypoint: { run: [copilot, --disable-builtin-mcps] } | ||
| configDir: .copilot |
There was a problem hiding this comment.
When a user has a config override or patch in their enclave config for copilot, Enclave currently rebuilds the config directory on every start and keeps only declared state. Without this line, Copilot's conversation history and session database are deleted, so --continue and --resume find nothing. Verified in real sessions, including sidebar-sessions-state/, which keeps the sidebar's list of recent sessions. Please add after the configDir line:
statePaths: [config.json, session-state/, sidebar-sessions-state/, command-history-state*, session-store.db*, permissions-config.json, mcp-oauth-config/, mcp-secrets/, installed-plugins/, plugin-data/]
| providers: | ||
| - name: github | ||
| credentials: [copilot-github-token] | ||
| authFiles: [config.json] |
There was a problem hiding this comment.
It's not auth only, this is the problem I mentioned in the above inline comment. In copilot, config.json holds both configuration data and login credentials. It is even worse, because it also stores project-specific stuff there, such as folder trust, so it is basically leaks trust information between projects.
If we decide to break /login as I suggested on line 15, we can remove the authFiles stuff here completely, i.e., the provider block becomes:
providers:
- name: github
credentials: [copilot-github-token]
| ## Memory and Skills | ||
|
|
||
| The extension does not declare a native memory directory or memory-disable | ||
| arguments, so Enclave does not provide Copilot-specific project-scoped memory |
There was a problem hiding this comment.
Copilot Memory is stored by GitHub on their servers. I verified that by having it store memories and inspecting what gets sent and received in each session. This is also a great way to leak data across projects, so we should think about disabling copilot's memory feature by default.
Enclave's per-project memory isolation (which is designed for local memories) can't help here :(
See also my other comment in the README for this problem
| { | ||
| "remoteExport": false, | ||
| "autoUpdate": false, | ||
| "logLevel": "default", | ||
| "includeCoAuthoredBy": false, | ||
| "storeTokenPlaintext": false, | ||
| "model": "auto" | ||
| } |
There was a problem hiding this comment.
Copilot Memory stores memories/facts on GitHub's servers and serves them to every later session of that account: other projects, other enclave state roots, copilot outside Enclave, anything. In my test sessions, a prompt asking copilot to remember a word made the agent store it with user scope under --yolo without any notice, and every later project's session fetched it at start. That is also a nice way for agents in different projects and sessions to collaborate with each other by using the memory feature as a message board. I think the huggingface incident has shown us what can happen with this. So I would argue to disable copilot's memory feature by default.
We could use
{
"remoteExport": false,
"autoUpdate": false,
"logLevel": "default",
"includeCoAuthoredBy": false,
"storeTokenPlaintext": false,
"model": "auto",
"memory": false
}
Here. Users who want the feature nevertheless can run /memory on in a session, and the setting then persists in that project's config store.
There was a problem hiding this comment.
I added that setting. As users are able to change this, it is a better default for enclave to disable memory. I extended the README to cover this aspect.
| @@ -0,0 +1,59 @@ | |||
| # Copyright (C) 2026 Dirk Fauth and others. | |||
There was a problem hiding this comment.
I don't think we need defaultIncluded: false. Keep this as it is.
xai
left a comment
xai
left a comment
There was a problem hiding this comment.
Thanks again for making this work!
It was a lot harder and trickier than I thought at the beginning, but this PR and copilot was a great use case that helped us close some gaps in core as well 👍
3 participants
What it does
This PR adds GitHub Copilot CLI as supported tool profile.
Note that this PR is work in progress. I actually need some support in finishing the contribution. I would like to add the folder trust configuration and need some additional testing from others. The setup worked for me two days ago but suddenly the authentication does not work anymore. My token is correct and freshly created, so not sure if the newest CLI version had some changes or the company guidelines changed.
How to test
Follow-ups
Breaking changes
Review checklist