feat(desktop): sign in to GitHub Copilot with a device grant - #3390
feat(desktop): sign in to GitHub Copilot with a device grant#3390hgaol wants to merge 5 commits into
Conversation
c7d8924 to
4f133f1
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for filling the sign-in gap described in #3388. The problem is valid, and the provider-specific RFC 8628 parsing is carefully bounded.
The remaining implementation concern is ownership: the device flow currently runs beside the existing Host OAuth authority, duplicating the attempt lifecycle and producing the race described inline. The simpler final state appears to be extending the existing Host OAuth provider slice for GitHub Copilot and keeping Desktop responsible only for importing credentials that genuinely depend on the local gh installation.
I also left two independent inline notes about token lifetime and the OAuth app identity.
AI-assisted review disclosure: Codex performed exact-head analysis, with independent reviewer passes focused on lifecycle, concurrency, security, compatibility, and OAuth protocol behavior. The findings were revalidated against head 4f133f11b4442ed572379da7040965e9a7699ca2.
|
PR need a rebase before reviewing and approve~ |
GitHub Copilot is the only registered oauth_token provider without a login flow. Desktop's connectExistingLogin only scavenges COPILOT_GITHUB_TOKEN, GH_TOKEN, GITHUB_TOKEN, or `gh auth token`, rejects classic PATs, and needs a credential carrying Copilot Requests — a permission plain `gh auth login` does not grant. Users who own a subscription still have no way to sign in. Add the RFC 8628 device authorization the provider was missing, alongside the existing Codex and xAI enrollments. The grant yields the same gho_/ghu_ account token createGitHubCopilotAccountTokens already accepts, so only credential acquisition changes. GitHub reports authorization_pending and slow_down as HTTP 200 with an error body rather than a 4xx, so the error code is read before the success shape. The verification URL is pinned to github.com because the presentation layer opens it in the user's browser, and only read:user is requested, so the grant cannot reach a user's code. The provider already sends the vscode Copilot Chat editor headers (GITHUB_COPILOT_COMPAT_HEADERS), so the client id presented here does not change who Maka claims to be. Fixes apache#3388 Generated-by: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Wire the device enrollment into Settings so a user with a Copilot subscription can sign in, instead of first minting a fine-grained PAT with Copilot Requests or having a `gh` login that happens to carry it. `beginDeviceLogin` asks GitHub for a code and `completeDeviceLogin` waits for the user to authorize it; both ride the existing direct-account login flow, so the pending guard, snapshot refresh, and unmount safety stay in one place. The pending grant lives in the main process, so closing Settings cannot strand a poll holding a credential, and the secret still reaches only the Host vault. Importing an existing `gh` credential remains available as a secondary action for machines that are already authenticated, and both paths now adopt an account through one place that refuses to store a credential before the account proves it returns usable models. The verification code is shown as text rather than opening a browser: the renderer has no general openExternal bridge by design, and the device URL is pinned to github.com by the enrollment. Fixes apache#3388 Generated-by: GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Route the GitHub Copilot device grant through `HostOAuthCoordinator` instead of a second login-session authority in Desktop. Assigning `deviceLogin` only after the begin request resolved let a reopened Settings pane or a second IPC client overlap attempts, so an older completion could replace the credential a newer login had just committed. `github-copilot` joins `OAUTH_LOGIN_PROVIDERS` and the interactive login providers, and the coordinator grows a provider slice beside xAI and Codex. It therefore inherits one serialized start, supersede and cancellation boundaries, Host residency across polling, the configured network transport, and an atomic credential commit. Desktop keeps only the local `gh` credential import; account state, refresh, and sign-out ride the shared channels the coordinator's adapter already registers. Older peers cannot start a login for the new provider, so the compatibility epoch moves to 32. Preserve the lifetime GitHub returns. Every account token was recorded with `expires_at = Number.MAX_SAFE_INTEGER` while the refresh path returned it unchanged, so an expiring token would stop working at expiry and require another interactive login. The enrollment now carries `expires_in` and `refresh_token` through, refresh performs the refresh grant for an expiring record, and an expiring response with no refresh token is rejected rather than stored. A token with no declared lifetime keeps the sentinel and still refreshes without provider I/O. Record the OAuth app identity. The client id is GitHub's Copilot editor app: GitHub's consent screen names that application while Maka receives and stores the credential, and no published authorization for third-party reuse exists. Provenance, consent identity, and the open authorization question are written down in `docs/github-copilot-oauth-identity.md`, and the device flow is now opt-in behind `MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=1` so it is not a public sign-in path before that basis is established. Importing a credential the user already holds stays the shipped route. Generated-by: Claude Code
The device flow was gated opt-in, which left Settings with no sign-in button at all — only the local credential import. Recording the app identity is what the review asked for; hiding the feature was not. Enrollment now ships on with a kill switch, the same shape Codex already uses: `MAKA_GITHUB_COPILOT_DEVICE_LOGIN_EXPERIMENTAL=0` refuses the login at the Host. The panel stops probing that flag and always renders the button, because the Host owns the decision and answers a disabled start with `experimental_disabled`; a second copy of the gate in the renderer could only disagree with it. The consent identity question in `docs/github-copilot-oauth-identity.md` is unchanged and still open. Generated-by: Claude Code
a3dcc93 to
16f6520
Compare
Rebased |
…-device-flow-login # Conflicts: # packages/runtime-host/src/__tests__/protocol.test.ts # packages/runtime-host/src/protocol/index.ts
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for moving GitHub Copilot enrollment onto HostOAuthCoordinator. That resolves the original lifecycle and stale-completion problem, and the token lifetime/refresh work is now on the right authority.
I do not think another round of isolated patches would be fair or productive here. Could we converge on one final state instead?
[P1] Shipping boundary: the current provenance document explicitly says the authorization basis for reusing the Copilot editor OAuth identity is not established, while the feature is enabled by default. Until GitHub provides a public compatibility basis or a Maka-authorized app identity, this path should be explicit opt-in/default-off. The kill switch can remain for operations, but it cannot serve as the authorization basis.
[P2] Account adoption: device authorization should not commit a credential until the GitHub provider slice has verified that the account exposes at least one usable Copilot model. That validation belongs before the coordinator’s atomic commit, using the same Host transport; the Desktop post-login synchronization should not be responsible for deciding whether authentication succeeded.
[P2] Refresh completion: GitHub Copilot should join the existing single-401 refresh/replay seam. An access token can be rejected before its local expires_at; in that case the Host should force-refresh once, CAS-persist the new generation, replace the Authorization header, and replay the request once.
I would also like this revision to remove the Desktop lifecycle that the Host path has replaced, rather than adding the fixes on top:
- fold
GitHubCopilotSubscriptionServiceinto a stateless “import existing local credential” adapter; - remove the unused
directmode fromuseOAuthLoginFlow; - route connection-detail re-login through the shared Host OAuth service;
- keep local
ghimport only as a secondary provider-catalog action; - remove the unused
isExperimentalEnabledpreload/IPC query surface.
The provider-specific device parsing and its security/error tests should stay. The goal is not to make OAuth artificially tiny; it is to leave one lifecycle authority and remove the roughly 280–330 lines whose responsibilities have moved to the Host.
A clean final shape would be: Runtime owns the GitHub device codec, Host owns attempt/entitlement/credential/refresh/401 recovery, and Desktop owns only generic presentation plus local credential discovery. Once that state is in one exact head with epoch 35 and focused entitlement/401 regressions, we can review the final result as a whole instead of asking for another sequence of patches.
AI-assisted review disclosure: OpenAI Codex coordinated independent exact-head reviews of the OAuth lifecycle, consent/provenance boundary, refresh behavior, and simplification opportunities. I reviewed and approved this final-state recommendation before posting.
2 participants
Summary
GitHub Copilot was the only registered
oauth_tokenprovider without a real sign-in path. Desktop only scavenged an existingCOPILOT_GITHUB_TOKEN/GH_TOKEN/GITHUB_TOKEN/gh auth token, rejected classic PATs, and left users minting a fine-grained PAT with Copilot Requests.This PR adds the missing RFC 8628 device-flow enrollment in
@maka/runtimeand wires it into Desktop Settings:beginDeviceLoginrequests a user code;completeDeviceLoginpolls until authorizedghimport adopt an account only after it proves usable modelsopenExternalbridge); enrollment pins the URL to github.com and requests onlyread:userImporting an existing
ghcredential remains available as a secondary action.Fixes #3388
Verification
lint/typecheck/ workspace suites not re-run in this PR-opening step; rely on CItestcheckScreenshot
Sign in
Successful GitHub Copilot subscription account import after sign-in:
AI use
Select exactly one:
Tool(s) and scope: GitHub Copilot CLI assisted implementation of the device-flow enrollment, Desktop wiring, tests, commit message cleanup, and PR creation. The human contributor remains responsible for the change.
Checklist
Does this PR entail a change in behavior?