fix(api-proxy): address review comments on OpenCode port 10004 routing#1984
fix(api-proxy): address review comments on OpenCode port 10004 routing#1984
Conversation
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
- types.ts: correct credential priority comment to reference user-configurable vars COPILOT_GITHUB_TOKEN/COPILOT_API_KEY instead of internal COPILOT_AUTH_TOKEN - smoke-opencode.md: add note that lock file compilation is pending until opencode engine support lands in gh-aw - server.js: add rate limiting to OpenCode (port 10004) HTTP handler using content-length-aware checkRateLimit() call - server.js: extract resolveOpenCodeRoute() helper for testability and refactor handler to use it - server.test.js: add 8 unit tests covering all OpenCode routing priority scenarios and header injection Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a0621fda-d1a3-449b-a3da-a9d0331c4c76
- Add needsAnthropicVersion flag to resolveOpenCodeRoute return value to centralize anthropic-version header logic - Use resolveOpenCodeRoute() for startup guard instead of repeating the credential availability check - Update tests to assert needsAnthropicVersion for all scenarios Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/a0621fda-d1a3-449b-a3da-a9d0331c4c76
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR updates the API proxy’s OpenCode (port 10004) routing to address prior review feedback, aligning credential selection, enforcing rate limiting, improving failure behavior, and adding unit coverage.
Changes:
- Updates OpenCode port documentation to reflect the intended credential priority and default routing behavior.
- Adds rate limiting to the OpenCode HTTP handler and centralizes routing/credential header selection via
resolveOpenCodeRoute()for both HTTP and WebSocket paths. - Adds unit tests for
resolveOpenCodeRoute()and introduces an (explicitly inactive) smoke workflow definition for OpenCode.
Show a summary per file
| File | Description |
|---|---|
src/types.ts |
Updates JSDoc to reflect correct OpenCode routing defaults and credential priority. |
containers/api-proxy/server.js |
Adds OpenCode rate limiting, centralizes routing logic, and returns explicit 503 errors when credentials are unavailable. |
containers/api-proxy/server.test.js |
Adds unit tests covering OpenCode route resolution and header exclusivity behavior. |
.github/workflows/smoke-opencode.md |
Adds an OpenCode smoke workflow definition with a note that it’s not compiled/active yet. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 2
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Smoke Test Results ✅ GitHub MCP: #1978 "fix: add size-based filtering to --env-all to prevent E2BIG", #1977 "fix: recover toolchain env vars from $GITHUB_ENV file" Overall: PASS
|
🔥 Smoke Test Results
Overall: PASS PR: fix(api-proxy): address review comments on OpenCode port 10004 routing
|
|
Smoke test matrix:
|
Smoke Test: GitHub Actions Services Connectivity
All checks passed. (
|
Chroot Version Comparison Results
Result: Not all tests passed — Python and Node.js versions differ between host and chroot. The
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Addresses all review comments from the PR #1979 review thread: missing rate limiting on port 10004, inaccurate credential docs referencing an internal token, no
.lock.ymlfor the new smoke workflow, and no test coverage for OpenCode routing.Changes
src/types.ts: Corrected JSDoc credential priority to reference user-configurable varsCOPILOT_GITHUB_TOKEN/COPILOT_API_KEYinstead of the internal derivedCOPILOT_AUTH_TOKEN.github/workflows/smoke-opencode.md: Added explicit note that this workflow has no compiled.lock.ymland is inactive in GitHub Actions; includes instructions for compiling once theopencodeengine is supportedcontainers/api-proxy/server.js:checkRateLimit()to OpenCode HTTP handler (was the only proxy port missing it)resolveOpenCodeRoute()helper to centralize credential priority and return aneedsAnthropicVersionflag, eliminating duplicated header injection logic across HTTP and WebSocket handlersresolveOpenCodeRoute()for startup guard — removes the duplicatedOPENAI_API_KEY || ANTHROPIC_API_KEY || COPILOT_AUTH_TOKENcheckcontainers/api-proxy/server.test.js: Added 8 unit tests forresolveOpenCodeRoute()covering all three credential priority branches, mutual exclusivity of auth headers,needsAnthropicVersionflag, and null return when no credentials are set