Code Review

Overall: The fix is correct and necessary — sending raw GitHub OAuth tokens to the Copilot inference API is the root cause of 403s. The implementation follows the same exchange pattern as opencode/openclaw/VS Code. Good test coverage and well-documented PR.

Several items worth addressing before merge:

Issues

1. TOCTOU race on token cache file permissions (copilot_auth.py)

cache_path.write_text(json.dumps(payload), encoding="utf-8")
try:
    cache_path.chmod(0o600)
except OSError:
    pass

The file is created with default permissions (often 0o644) and then restricted to 0o600. Between write_text and chmod, another process could read the token. Use os.open with O_CREAT | O_WRONLY and mode 0o600, or set os.umask(0o077) before writing (and restore after).

2. base_url is computed but never returned to the caller

resolve_copilot_token() returns (session_token, source) but discards session.base_url. The caller has no way to use the enterprise-specific API base URL without re-parsing the token themselves. This undermines the enterprise support claim in the PR description. Either:

• Return a 3-tuple (token, base_url, source), or
• Return the CopilotSessionToken dataclass directly

3. Duplicate expires_at normalization logic

The if expires_at > 1e11: expires_at /= 1000.0 pattern appears in both _load_cached_session_token and exchange_github_token_for_copilot_session. Extract into a helper like _normalize_expiry(value) -> float to avoid drift.

4. Redundant import inside _derive_base_url_from_token

from urllib.parse import urlparse  # inside function body

urllib.parse is already imported at the top of the file. Use urllib.parse.urlparse(proxy_ep) directly.

5. Hardcoded Editor-Version header

"Editor-Version": "vscode/1.104.1",

Fragile. If GitHub validates or rate-limits by editor version, this breaks silently. Consider making it a module-level constant with a comment, or using the actual Hermes version string.

6. resolve_copilot_token swallows exchange errors silently

except RuntimeError as exc:
    logger.error("Copilot token exchange failed: %s", exc)
    return "", ""

Returning ("", "") on exchange failure is indistinguishable from "no token found at all". The caller can't tell if the user has a valid GitHub token but the exchange endpoint is down vs. no credentials exist. Consider raising or returning a more descriptive failure.

Minor

• Whitespace-only changes (blank lines between methods, dict reformatting) inflate the diff. Consider separating formatting into its own commit.
• _get_token_cache_path() does try: from hermes_constants import ... on every call. Cache at module level or use a sentinel to avoid repeated import attempts.
• Cache is checked before input validation in exchange_github_token_for_copilot_session — passing github_token="" still "succeeds" if cache is valid. Probably intentional for perf, worth a comment.

What's Good

• Correct fix for a real 403 bug
• Token caching with 5-min refresh margin avoids hammering the exchange endpoint
• File permissions restricted to 0o600
• Good test coverage: cache roundtrip, expiry, permissions, exchange success/failure, full resolution chain
• Compatible with openclaw's cache format
• _derive_base_url_from_token correctly handles enterprise, individual, and missing proxy-ep

All 6 issues and 3 minor items addressed in 8dc572c4:

Issue 1 — TOCTOU race: _save_session_token now uses os.open with O_CREAT | O_WRONLY and mode 0o600. The file is created with restricted permissions atomically.

Issue 2 — base_url not returned: resolve_copilot_token() now returns a 3-tuple (token, base_url, source). Caller in auth.py updated. Enterprise callers get the correct endpoint without re-parsing.

Issue 3 — Duplicate normalization: Extracted _normalize_expiry(value) -> float helper used by both _load_cached_session_token and exchange_github_token_for_copilot_session.

Issue 4 — Redundant import: Removed from urllib.parse import urlparse inside _derive_base_url_from_token. Uses urllib.parse.urlparse() from the module-level import directly.

Issue 5 — Hardcoded Editor-Version: Extracted to _EDITOR_VERSION module constant with a comment noting the fragility.

Issue 6 — Silent error swallowing: resolve_copilot_token now logs at warning level with the github_source context ("GitHub token found (gh auth token) but Copilot session exchange failed: ..."), making it distinguishable from no-token-found.

Minor items:

• _get_token_cache_path now caches the hermes_home result at module level via _cached_hermes_home sentinel
• Added comment explaining cache-before-validation ordering in exchange_github_token_for_copilot_session
• Whitespace changes left as-is (formatter-induced, mixed with the original PR)

Tests updated: 40 copilot_auth tests + 127 api_key_providers tests all pass.

Rebased onto latest main (1cec910b), resolved one formatting conflict in auth.py. All 6 issues + 3 minor items from the review remain addressed in the rebased commits.

Tests: 40 copilot_auth + 14 copilot provider tests pass.

Thanks for the contribution @emma-clawdbot. Closing as superseded — the same token-exchange feature (GitHub OAuth token → Copilot API JWT via /copilot_internal/v2/token) landed in #15114 using a cleaner, focused implementation from @difujia's PR #12876.

Your PR was submitted first and solved the same problem. Both contributors are credited in the salvage PR body. The 6500-line diff made a direct salvage impractical, but the feature you identified is now in main.

Merged: d7ad07d fix(copilot): exchange raw GitHub token for Copilot API JWT
#15114