-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(ci): support STRIX_COPILOT_TOKEN/STRIX_COPILOT_ACCESS for non-interactive Copilot auth #224
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
base: main
Are you sure you want to change the base?
feat(ci): support STRIX_COPILOT_TOKEN/STRIX_COPILOT_ACCESS for non-interactive Copilot auth #224
Conversation
Greptile SummaryAdded CI-friendly GitHub Copilot authentication with environment variable support ( Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant LLM
participant Auth
participant GitHub
participant Copilot
alt Interactive Authentication
User->>CLI: strix auth login
CLI->>Auth: Start device flow
Auth->>GitHub: Request device code
GitHub-->>Auth: Returns flow data
Auth-->>User: Display verification URL
User->>GitHub: Complete browser auth
loop Poll authorization
Auth->>GitHub: Check status
GitHub-->>Auth: Status response
end
Auth->>Auth: Save credentials locally
Auth-->>CLI: Success
end
alt Environment Direct Access
User->>LLM: Request with copilot model
LLM->>Auth: Request credentials
Auth->>Auth: Read env variable
Auth-->>LLM: Return immediately
LLM->>Copilot: API request
Copilot-->>LLM: Response
end
alt Environment Refresh Exchange
User->>LLM: Request with copilot model
LLM->>Auth: Request credentials
Auth->>Auth: Use refresh from env
Auth->>GitHub: Exchange for access
GitHub-->>Auth: Returns access with expiry
Auth->>Auth: Save locally
Auth-->>LLM: Return result
LLM->>Copilot: API request
Copilot-->>LLM: Response
end
alt Expired Credential Refresh
User->>LLM: Request with copilot model
LLM->>Auth: Request credentials
Auth->>Auth: Check expiry
Auth->>Auth: Acquire lock
Auth->>GitHub: Exchange for new access
GitHub-->>Auth: New credentials
Auth->>Auth: Save locally
Auth-->>LLM: Return refreshed
LLM->>Copilot: API attempt 1
Copilot-->>LLM: 401 Error
LLM->>Auth: Force refresh
Auth->>GitHub: Exchange again
GitHub-->>Auth: Fresh credentials
Auth-->>LLM: Return result
LLM->>Copilot: API attempt 2
Copilot-->>LLM: Success
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (4)
-
strix/llm/copilot_auth.py, line 272-278 (link)style: when
STRIX_COPILOT_TOKENis provided via environment variable, tokens are still written to disk at line 317 even though they may be ephemeral CI credentials -
strix/llm/copilot_auth.py, line 297 (link)style: unnecessary parentheses around
effective_enterpriseNote: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
-
README.md, line 221 (link)style: missing code block syntax highlighting
bash
export STRIX_LLM="github-copilot/<model>"<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> -
README.md, line 239-243 (link)style: example should use proper code block formatting with bash syntax highlighting
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
7 files reviewed, 4 comments
|
Related to issue #225 |
|
Related to issue #226 |
|
This feature was added with the assistance of AI (LLM). |
…tecture - Add httpx dependency to pyproject.toml - Create strix/llm/copilot_auth.py with OAuth device-flow implementation - Token storage with secure permissions (0o600) - CI authentication via environment variables - Enterprise domain support - Thread-safe token refresh with async lock - Modify strix/llm/llm.py to integrate Copilot authentication - Add Copilot model detection (github-copilot/* prefix) - Integrate token fetching in _stream_request() method - Automatic token refresh on 401/403 authentication errors - Add Copilot-specific headers for VS Code compatibility - Preserve all upstream architectural patterns (streaming, retry logic) - Modify strix/interface/main.py to add authentication commands - Add handle_auth_login() and handle_auth_logout() async functions - Add auth subparsers (login/logout) in CLI - Add Copilot detection in warm_up_llm() for non-interactive mode - Make --target argument optional (required=False) - Remove deprecated HOST_GATEWAY_HOSTNAME import - Create strix/tests/test_copilot_auth.py with comprehensive test coverage - test_env_access_token_returns_immediately - test_env_refresh_token_triggers_exchange - test_non_interactive_without_token_raises - Update README.md with Copilot authentication documentation - Add GitHub Copilot models section with usage examples - Add interactive authentication examples (strix auth login/logout) - Add non-interactive/CI authentication examples - Update CI/CD workflow documentation All changes preserve upstream main architecture and improvements. No merge conflicts - clean integration. All tests passing (100%).
Fixed AttributeError when tool execution returns string error messages instead of dict results. The method now checks result type and displays string errors formatted as error messages. Fixes crashes when tool server fails or returns error strings.
c7d1b0c to
74ad166
Compare
Remove temporary development files and directories that should not be in the repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7 files reviewed, 1 comment
| base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise) | ||
|
|
||
| # If forcing refresh or token is expired, perform refresh with lock | ||
| if force_refresh or not (info.access and info.expires and info.expires > _now_ms()): | ||
| async with _refresh_lock: | ||
| if force_refresh: | ||
| logger.info("Forcing Copilot access token refresh due to API failure") | ||
| else: | ||
| logger.info("Refreshing expired Copilot access token") | ||
|
|
||
| info = _load_oauth_info_from_disk(token_path) if not env_refresh else info | ||
| base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise) | ||
|
|
||
| # If forcing refresh or token is expired, perform refresh with lock | ||
| if force_refresh or not (info.access and info.expires and info.expires > _now_ms()): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: duplicate code block - lines 310-324 are identical to lines 321-335 inside the lock
| base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise) | |
| # If forcing refresh or token is expired, perform refresh with lock | |
| if force_refresh or not (info.access and info.expires and info.expires > _now_ms()): | |
| async with _refresh_lock: | |
| if force_refresh: | |
| logger.info("Forcing Copilot access token refresh due to API failure") | |
| else: | |
| logger.info("Refreshing expired Copilot access token") | |
| info = _load_oauth_info_from_disk(token_path) if not env_refresh else info | |
| base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise) | |
| # If forcing refresh or token is expired, perform refresh with lock | |
| if force_refresh or not (info.access and info.expires and info.expires > _now_ms()): | |
| # Compute base_url/domain (recompute if enterprise_url argument used and env not set) | |
| effective_enterprise = enterprise_domain or ( | |
| _normalize_domain(enterprise_url) if enterprise_url else None | |
| ) | |
| base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise) | |
| # If forcing refresh or token is expired, perform refresh with lock | |
| if force_refresh or not (info.access and info.expires and info.expires > _now_ms()): | |
| async with _refresh_lock: | |
| if force_refresh: | |
| logger.info("Forcing Copilot access token refresh due to API failure") | |
| else: | |
| logger.info("Refreshing expired Copilot access token") | |
| # Reload token info to ensure we have latest state |
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/llm/copilot_auth.py
Line: 310:324
Comment:
**logic:** duplicate code block - lines 310-324 are identical to lines 321-335 inside the lock
```suggestion
# Compute base_url/domain (recompute if enterprise_url argument used and env not set)
effective_enterprise = enterprise_domain or (
_normalize_domain(enterprise_url) if enterprise_url else None
)
base_url = copilot_openai_base_url(enterprise_domain=effective_enterprise)
# If forcing refresh or token is expired, perform refresh with lock
if force_refresh or not (info.access and info.expires and info.expires > _now_ms()):
async with _refresh_lock:
if force_refresh:
logger.info("Forcing Copilot access token refresh due to API failure")
else:
logger.info("Refreshing expired Copilot access token")
# Reload token info to ensure we have latest state
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Adds GitHub Copilot authentication support with both interactive device flow and CI-friendly environment variables, along with CLI auth commands and a terminal renderer bug fix. Enables non-interactive usage of Copilot models in CI/CD pipelines.
Changes
New Features
strix/llm/copilot_auth.py(381 lines): Full OAuth device flow implementationstrix auth loginSTRIX_COPILOT_ACCESS- ready-to-use access tokenSTRIX_COPILOT_TOKEN- refresh token (automatically exchanged)STRIX_COPILOT_ENTERPRISE- optional enterprise domain0o600permissionsCLI Authentication Commands (
strix/interface/main.py)strix auth loginandstrix auth logoutIntegration Updates
strix/llm/llm.py: Copilot model integrationgithub-copilot/prefixBug Fixes
strix/interface/tool_components/terminal_renderer.py: FixedAttributeErrorwhen tool execution returns string error messages instead of dict resultsDependencies
httpx(async HTTP client) topyproject.tomlpoetry.lock(vertex dependencies adjusted, other dependency updates)Documentation
README.mdwith GitHub Copilot authentication sectionTests
strix/tests/test_copilot_auth.py(60 lines): Comprehensive test coveragetest_env_access_token_returns_immediately- validates CI env var usagetest_env_refresh_token_triggers_exchange- tests token exchange flowtest_non_interactive_without_token_raises- fail-fast behaviorVerification
strix auth login--non-interactiveflagSTRIX_COPILOT_TOKEN/STRIX_COPILOT_ACCESSNotes for Reviewers
--targetnow optional to support auth subcommandsHow to Test Locally
Related