-
-
Notifications
You must be signed in to change notification settings - Fork 566
Feat/enterprise and reasoning #154
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
Open
jkorsvik
wants to merge
12
commits into
ericc-ch:master
Choose a base branch
from
jkorsvik:feat/enterprise-and-reasoning
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8f6c468
feat: add GitHub Enterprise Server/Cloud support
jkorsvik 32e948d
fix: use copilot-api subdomain for enterprise Copilot endpoints
jkorsvik 6f07c56
chore: clean up docs and bump version to 0.8.0
jkorsvik ee4670a
refactor: clean up enterprise URL handling and simplify code patterns
jkorsvik 151646e
feat: add AGENTS.md for build, lint, test commands and code style gui…
jkorsvik 29668ce
feat: support copilot reasoning_opaque and reasoning_text
caozhiyuan a2467d3
feat: add signature field to AnthropicThinkingBlock
caozhiyuan 58f7a45
feat: add idleTimeout configuration for bun server
caozhiyuan 3fa5519
feat: enhance reasoning handling in tool calls and change the thinkin…
caozhiyuan dfb40d2
feat: conditionally handle reasoningOpaque in handleFinish based on t…
caozhiyuan 7657d87
fix: handleReasoningOpaqueInToolCalls add isToolBlockOpen judge
caozhiyuan 968ff12
merge: integrate chat-completions-reasoning support
jkorsvik File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix: use copilot-api subdomain for enterprise Copilot endpoints
- Update copilotBaseUrl to use copilot-api.{enterprise} for GHE
- Add 6 new tests for copilotBaseUrl with enterprise configuration
- Update CLAUDE.md with correct enterprise endpoint documentation
- Fixes 'Failed to get models' error for GitHub Enterprise users
🤖 Generated with [Claude Code](https://claude.com/claude-code)- Loading branch information
commit 32e948d5f64132a8284b3208d6f3e61c8d41c77c
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,227 @@ | ||
| # Enterprise Support Testing Plan | ||
|
|
||
| ## Overview | ||
| Comprehensive test plan to validate GitHub Enterprise Server/Cloud support in copilot-api. | ||
|
|
||
| ## Test Categories | ||
|
|
||
| ### 1. Unit Tests (Automated) | ||
| - [x] URL normalization helpers | ||
| - [x] githubBaseUrl with/without enterprise | ||
| - [x] githubApiBaseUrl with/without enterprise | ||
| - [x] looksLikeHost validation | ||
|
|
||
| ### 2. Integration Tests (Manual + Automated) | ||
|
|
||
| #### 2.1 File System Persistence | ||
| **Test**: Enterprise URL persistence | ||
| - [ ] Write enterprise URL to file | ||
| - [ ] Read enterprise URL from file | ||
| - [ ] Verify file permissions (0600) | ||
| - [ ] Handle empty/missing file gracefully | ||
|
|
||
| **Test**: Token and enterprise URL coordination | ||
| - [ ] Token persisted alongside enterprise URL | ||
| - [ ] Both files created in APP_DIR | ||
| - [ ] Files survive process restart | ||
|
|
||
| #### 2.2 OAuth Flow with Enterprise | ||
| **Test**: Device code endpoint | ||
| - [ ] Mock fetch to verify URL: `https://{enterprise}/login/device/code` | ||
| - [ ] Verify client_id and scope in request body | ||
| - [ ] Handle response correctly | ||
|
|
||
| **Test**: Access token endpoint | ||
| - [ ] Mock fetch to verify URL: `https://{enterprise}/login/oauth/access_token` | ||
| - [ ] Verify device_code and grant_type in request body | ||
| - [ ] Poll correctly with enterprise URL | ||
|
|
||
| **Test**: Without enterprise (backwards compatibility) | ||
| - [ ] Device code uses `https://github.com/login/device/code` | ||
| - [ ] Access token uses `https://github.com/login/oauth/access_token` | ||
|
|
||
| #### 2.3 Copilot API with Enterprise | ||
| **Test**: Copilot token fetch | ||
| - [ ] Mock fetch to verify URL: `https://api.{enterprise}/copilot_internal/v2/token` | ||
| - [ ] Verify authorization header with GitHub token | ||
| - [ ] Handle token response | ||
|
|
||
| **Test**: Copilot usage fetch | ||
| - [ ] Mock fetch to verify URL: `https://api.{enterprise}/copilot_internal/user` | ||
| - [ ] Verify headers and response handling | ||
|
|
||
| **Test**: Get GitHub user | ||
| - [ ] Mock fetch to verify URL: `https://api.{enterprise}/user` | ||
| - [ ] Verify authorization header | ||
|
|
||
| **Test**: Token refresh | ||
| - [ ] Verify refresh interval uses enterprise URL | ||
| - [ ] State.enterpriseUrl persists across refresh | ||
|
|
||
| **Test**: Without enterprise (backwards compatibility) | ||
| - [ ] Copilot token uses `https://api.github.com/copilot_internal/v2/token` | ||
| - [ ] Usage uses `https://api.github.com/copilot_internal/user` | ||
|
|
||
| #### 2.4 CLI Argument Parsing | ||
| **Test**: auth command with --enterprise-url | ||
| - [ ] Parse flag correctly | ||
| - [ ] Pass to setupGitHubToken | ||
| - [ ] Persist to file | ||
|
|
||
| **Test**: auth command without flag (interactive) | ||
| - [ ] Prompt: "Are you using GitHub Enterprise?" | ||
| - [ ] If yes, prompt for host | ||
| - [ ] Normalize and persist host | ||
|
|
||
| **Test**: start command with --enterprise-url | ||
| - [ ] Parse flag correctly | ||
| - [ ] Override persisted value if present | ||
| - [ ] Set state.enterpriseUrl | ||
|
|
||
| **Test**: start command without flag | ||
| - [ ] Load persisted enterprise URL if present | ||
| - [ ] Use github.com if not present | ||
|
|
||
| #### 2.5 End-to-End Scenarios | ||
|
|
||
| **Scenario 1**: Fresh auth with enterprise (interactive) | ||
| ```bash | ||
| bun run dev auth | ||
| # User prompted: "Are you using GitHub Enterprise?" -> y | ||
| # User prompted: "Enter host:" -> ghe.example.com | ||
| # Expected: Writes ghe.example.com to enterprise_url file | ||
| # Expected: Device code flow uses https://ghe.example.com/... | ||
| ``` | ||
|
|
||
| **Scenario 2**: Fresh auth with enterprise (CLI flag) | ||
| ```bash | ||
| bun run dev auth --enterprise-url https://ghe.company.com/ | ||
| # Expected: Normalizes to ghe.company.com | ||
| # Expected: Writes to enterprise_url file | ||
| # Expected: Device code flow uses https://ghe.company.com/... | ||
| ``` | ||
|
|
||
| **Scenario 3**: Start server with persisted enterprise | ||
| ```bash | ||
| # Assume enterprise_url file contains "ghe.example.com" | ||
| bun run dev start | ||
| # Expected: Loads ghe.example.com from file | ||
| # Expected: All API calls use enterprise endpoints | ||
| ``` | ||
|
|
||
| **Scenario 4**: Start server with override | ||
| ```bash | ||
| # Assume enterprise_url file contains "old.ghe.com" | ||
| bun run dev start --enterprise-url new.ghe.com | ||
| # Expected: Uses new.ghe.com for this run | ||
| # Expected: Does NOT overwrite persisted file (only auth writes) | ||
| ``` | ||
|
|
||
| **Scenario 5**: Backwards compatibility (no enterprise) | ||
| ```bash | ||
| # No enterprise_url file present | ||
| bun run dev start | ||
| # Expected: All endpoints use github.com/api.github.com | ||
| ``` | ||
|
|
||
| **Scenario 6**: URL normalization variations | ||
| - Input: `https://ghe.example.com/` -> `ghe.example.com` | ||
| - Input: `http://ghe.example.com` -> `ghe.example.com` | ||
| - Input: `ghe.example.com` -> `ghe.example.com` | ||
| - Input: `ghe.example.com/` -> `ghe.example.com` | ||
|
|
||
| ### 3. Mock-Based Integration Tests (to add) | ||
|
|
||
| Create `tests/enterprise-integration.test.ts`: | ||
|
|
||
| ```typescript | ||
| import { describe, it, expect, mock, beforeEach, afterEach } from "bun:test" | ||
| import { setupGitHubToken } from "../src/lib/token" | ||
| import { state } from "../src/lib/state" | ||
|
|
||
| describe("Enterprise OAuth Flow", () => { | ||
| beforeEach(() => { | ||
| // Reset state | ||
| state.enterpriseUrl = undefined | ||
| }) | ||
|
|
||
| it("should use enterprise URL for device code", async () => { | ||
| const fetchMock = mock((url: string) => { | ||
| expect(url).toBe("https://ghe.example.com/login/device/code") | ||
| return Promise.resolve({ | ||
| ok: true, | ||
| json: () => Promise.resolve({ | ||
| device_code: "test", | ||
| user_code: "ABCD-1234", | ||
| verification_uri: "https://ghe.example.com/login/device", | ||
| expires_in: 900, | ||
| interval: 5 | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| global.fetch = fetchMock as any | ||
|
|
||
| state.enterpriseUrl = "ghe.example.com" | ||
| // Test device code fetch | ||
| // ... | ||
| }) | ||
| }) | ||
| ``` | ||
|
|
||
| ## Test Execution Plan | ||
|
|
||
| ### Phase 1: Automated Tests (Current) | ||
| - [x] Run existing unit tests: `bun test` | ||
| - [x] Verify all 38 tests pass | ||
|
|
||
| ### Phase 2: Add Mock Integration Tests | ||
| - [ ] Create `tests/enterprise-integration.test.ts` | ||
| - [ ] Mock fetch for OAuth flows | ||
| - [ ] Mock fetch for Copilot API calls | ||
| - [ ] Verify correct URLs called | ||
| - [ ] Run: `bun test` | ||
|
|
||
| ### Phase 3: Manual CLI Testing | ||
| - [ ] Test auth interactive flow | ||
| - [ ] Test auth with --enterprise-url flag | ||
| - [ ] Test start with persisted enterprise | ||
| - [ ] Test start with override | ||
| - [ ] Test backwards compatibility | ||
|
|
||
| ### Phase 4: Real Enterprise Testing (if available) | ||
| - [ ] Test with actual GitHub Enterprise Server instance | ||
| - [ ] Complete full auth flow | ||
| - [ ] Verify Copilot token fetch | ||
| - [ ] Verify Copilot usage fetch | ||
| - [ ] Test server startup and API calls | ||
|
|
||
| ## Test Results Log | ||
|
|
||
| ### Automated Tests (Phase 1) | ||
| ``` | ||
| ✅ bun test - 38 tests pass | ||
| ✅ URL normalization tests pass | ||
| ✅ Build succeeds | ||
| ✅ Typecheck passes | ||
| ``` | ||
|
|
||
| ### Integration Tests (Phase 2) | ||
| - Status: Pending | ||
|
|
||
| ### Manual Tests (Phase 3) | ||
| - Status: Pending | ||
|
|
||
| ### Real Enterprise Tests (Phase 4) | ||
| - Status: Pending (requires GHE instance) | ||
|
|
||
| ## Known Limitations | ||
| - Cannot test against real GHE without access to instance | ||
| - Mock tests verify URL construction but not actual API compatibility | ||
| - Interactive prompt testing requires manual verification | ||
|
|
||
| ## Next Steps | ||
| 1. Create comprehensive mock-based integration tests | ||
| 2. Add test for file system persistence | ||
| 3. Manual CLI testing with mocked server responses | ||
| 4. Document findings and any issues |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Fix malformed export command example.
Lines 365-368 appear to be improperly formatted. The export command is split across lines without proper markdown code block formatting, making it difficult to read and copy.
Apply this diff to properly format the export command:
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
368-368: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents