Refactor API proxy request path into focused guard and transport modules#3159
Conversation
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.
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 refactors the API proxy request path by extracting guard logic, body transformations, rate limiting, and WebSocket transport into focused modules while keeping proxy-request.js as the composition/root export surface.
Changes:
- Extracts effective-token, max-runs, and timeout-steering guards.
- Moves body mutation and rate-limit response handling into reusable modules.
- Moves WebSocket tunneling logic into a dedicated transport module.
Show a summary per file
| File | Description |
|---|---|
containers/api-proxy/proxy-request.js |
Composes the extracted modules and preserves shared exports. |
containers/api-proxy/body-transform.js |
Contains request-body sanitization and steering-message injection helpers. |
containers/api-proxy/rate-limit.js |
Provides a factory for rate-limit checks and 429 responses. |
containers/api-proxy/websocket-proxy.js |
Contains WebSocket upgrade validation and tunneling behavior. |
containers/api-proxy/guards/effective-token-guard.js |
Manages effective-token accounting, limits, and token steering state. |
containers/api-proxy/guards/max-runs-guard.js |
Manages max-runs accounting and limit responses. |
containers/api-proxy/guards/timeout-steering.js |
Manages timeout-based steering warnings. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 7/7 changed files
- Comments generated: 1
| const { sanitizeNullToolCallTypes, injectSteeringMessage } = require('./body-transform'); | ||
| const { createRateLimitChecker } = require('./rate-limit'); | ||
| const { createProxyWebSocket } = require('./websocket-proxy'); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@copilot address review feedback and fix this failing ci check https://github.com/github/gh-aw-firewall/actions/runs/25870995854/job/76028886971?pr=3159 |
Fixed in commit I updated |
Claude Engine Validation - Smoke Test Results
Summary: 2/3 PASSED - One test failed due to GitHub API authentication issue in the proxy environment.
|
Smoke Test Results
Overall: FAIL The pre-step template variables (
|
Smoke Test: Copilot BYOK (Offline) Mode
Running in BYOK offline mode ( Overall: PASS
|
Codex Smoke TestPRs: fix: remove dead re-exports of WrapperConfig component types; fix(entrypoint): preserve setup-* PATH ordering in fallback hostedtoolcache scan Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
Gemini Smoke Test Results
Overall Status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
Chroot Smoke Test Results
Overall: ❌ Not all versions match — Python and Node.js differ between host and chroot environments.
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL —
|
✨ Enhancement
containers/api-proxy/proxy-request.jshad grown into a 1,196-line mixed-responsibility module (guards, steering, body mutation, rate limiting, HTTP, WebSocket) in a security-critical credential injection path. This change decomposes those concerns into focused modules while preserving the existingproxy-requestpublic API for callers.What does this improve?
Why is this valuable?
Implementation approach:
containers/api-proxy/guards/max-runs-guard.jscontainers/api-proxy/guards/effective-token-guard.jscontainers/api-proxy/guards/timeout-steering.jscontainers/api-proxy/body-transform.jsfor body sanitization and steering-message injection.containers/api-proxy/rate-limit.jsfor rate-limit response handling via an injected checker factory.containers/api-proxy/websocket-proxy.jsfor WebSocket tunneling logic.containers/api-proxy/proxy-request.jsnow composes the extracted modules and retains existing exports used byserver.jsand tests.