Skip to content

fix(copilot): display device code in UI and prevent running state reset during auth#191

Open
cong91 wants to merge 2 commits intoheyhuynhgiabuu:mainfrom
cong91:main
Open

fix(copilot): display device code in UI and prevent running state reset during auth#191
cong91 wants to merge 2 commits intoheyhuynhgiabuu:mainfrom
cong91:main

Conversation

@cong91
Copy link

@cong91 cong91 commented Mar 3, 2026

  • Fix extract_user_code() to scan all lines unconditionally for XXXX-XXXX pattern; previous logic missed copilot-api v0.7+ format 'Please enter the code' which contains 'the' and no colon
  • Fix check_copilot_health to preserve in-memory running flag instead of deriving it from HTTP; server does not listen during GitHub device flow so HTTP failure was incorrectly marking the process as stopped
  • Skip frontend health poll while device auth message is active to prevent running state from being overwritten during auth
  • Add CopilotAuthInfo struct (user_code, verification_uri, raw_message)
  • Update onCopilotAuthRequired event callback type from string to CopilotAuthInfo
  • Show device code prominently in UI with Copy button
image

mrc added 2 commits March 3, 2026 14:57
…et during auth

- Fix extract_user_code() to scan all lines unconditionally for XXXX-XXXX
  pattern; previous logic missed copilot-api v0.7+ format 'Please enter
  the code' which contains 'the' and no colon
- Fix check_copilot_health to preserve in-memory running flag instead of
  deriving it from HTTP; server does not listen during GitHub device flow
  so HTTP failure was incorrectly marking the process as stopped
- Skip frontend health poll while device auth message is active to prevent
  running state from being overwritten during auth
- Add CopilotAuthInfo struct (user_code, verification_uri, raw_message)
- Update onCopilotAuthRequired event callback type from string to CopilotAuthInfo
- Show device code prominently in UI with Copy button
…Hub Copilot

Adds an embedded axum HTTP server (port 4142 by default) that proxies
/v1/embeddings requests to copilot-api. CLIProxyAPI does not support
embeddings endpoints natively, so this sidecar fills the gap.

- New embeddings_proxy.rs: axum server with /v1/embeddings and /v1/models
- CopilotConfig: add embeddingsPort field (default 4142)
- CopilotStatus: expose embeddingsPort to frontend
- AppState: store shutdown handle (oneshot::Sender) for clean teardown
- copilot.rs: start proxy on copilot-api start, stop on stop
- CopilotCard.tsx: show embeddings endpoint when connected; add port setting
- Supported models: text-embedding-3-small, text-embedding-3-small-inference,
  text-embedding-ada-002 (all free quota, no policy restrictions)
Copy link
Owner

@heyhuynhgiabuu heyhuynhgiabuu left a comment

Choose a reason for hiding this comment

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

PR #191 Review

Scope: Copilot device code auth fix + embeddings proxy feature

Good Changes

Device code auth fix — well-motivated and correctly implemented:

  • extract_user_code() scans all lines for XXXX-XXXX pattern — fixes copilot-api v0.7+ format
  • check_copilot_health preserves in-memory running flag — during device auth, server is not listening yet, so HTTP failure was incorrectly marking process as stopped
  • Frontend skips health poll while authMessage() is active
  • CopilotAuthInfo struct replaces raw string — clean data model
  • Device code shown prominently with Copy button — great UX improvement
  • Multi-line output accumulation + auth_emitted AtomicBool prevents duplicates

Embeddings proxy — fills a real gap (CLIProxyAPI does not support /v1/embeddings):

  • Clean axum server with graceful shutdown via oneshot::Sender
  • Non-fatal failure — copilot-api still works if proxy cannot bind
  • Configurable port with serde defaults
  • Frontend shows endpoint info when connected

Issues to Address

1. Unbounded body read — security concern
embeddings_proxy.rs:88: axum::body::to_bytes(body, usize::MAX) — a malicious or buggy client could send a multi-GB request and exhaust memory. Should cap at a reasonable limit:

let bytes = axum::body::to_bytes(body, 10 * 1024 * 1024) // 10MB

2. reqwest::Client::new() per request — performance
embeddings_proxy.rs:77: Creates a new HTTP client for every embeddings request, skipping connection pooling. Should store the client in ProxyConfig (axum State):

struct ProxyConfig {
    upstream_base: String,
    client: reqwest::Client,
}

3. Accumulated output grows indefinitely
copilot.rs ~line 232: accumulated_output string is never cleared or bounded. If copilot-api is verbose, this grows without limit. Should clear after auth_emitted fires, or cap at ~10KB.

4. New dependency: axum 0.8.8
Brings in axum, axum-core, matchit, httpdate, and more of hyper/tower. Meaningful binary size increase for a simple HTTP proxy. Not a blocker, but worth noting.

5. Hardcoded English strings
CopilotCard.tsx: "Embeddings endpoint", "Embeddings proxy port", "Models: text-embedding-3-small...", "Copy", "Copied!" — should be in the i18n catalog.

6. No clipboard error handling
CopilotCard.tsx:handleCopyCode: navigator.clipboard.writeText can throw in certain contexts. Should wrap in try/catch.

7. Cargo.lock version mismatch
PR sets version to 0.4.8, main is now 0.4.9. Needs rebase.

Suggestion

These two changes should ideally be separate PRs — the device code fix is a bug fix, while the embeddings proxy is a new feature with a new dependency. Mixing them makes it harder to review and revert independently.

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.

2 participants