fix(management): make antigravity webui callback ports configurable#2502
fix(management): make antigravity webui callback ports configurable#2502Pygmalione wants to merge 1 commit intorouter-for-me:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces configurable callback ports for the Antigravity OAuth flow in the Web UI, allowing overrides for both standard and premium instances. The changes include new configuration fields, a resolution helper, and updated logic in the token request handler. Feedback points out a critical resource leak where a cleanup function still uses a hardcoded port, and suggests improvements regarding code readability, the removal of magic numbers, and increasing test coverage for edge cases.
| } | ||
| var errStart error | ||
| if forwarder, errStart = startCallbackForwarder(antigravity.CallbackPort, "antigravity", targetURL); errStart != nil { | ||
| if forwarder, errStart = startCallbackForwarder(callbackPort, "antigravity", targetURL); errStart != nil { |
There was a problem hiding this comment.
While you've correctly updated startCallbackForwarder to use the dynamic callbackPort, the corresponding stopCallbackForwarderInstance call inside the goroutine on line 2005 still uses the old hardcoded antigravity.CallbackPort. This will lead to a resource leak as the forwarder won't be stopped correctly.
Please update it to use callbackPort as well:
defer stopCallbackForwarderInstance(callbackPort, forwarder)
| func (h *Handler) resolveAntigravityWebUICallbackPort(apiPort int) int { | ||
| if h != nil && h.cfg != nil { | ||
| if apiPort == 8318 { | ||
| if port := h.cfg.RemoteManagement.AntigravityWebUICallbackPortPremium; port > 0 { | ||
| return port | ||
| } | ||
| return defaultAntigravityPremiumPort | ||
| } | ||
| if port := h.cfg.RemoteManagement.AntigravityWebUICallbackPort; port > 0 { | ||
| return port | ||
| } | ||
| } | ||
| if apiPort == 8318 { | ||
| return defaultAntigravityPremiumPort | ||
| } | ||
| return defaultAntigravityStandardPort | ||
| } |
There was a problem hiding this comment.
The function resolveAntigravityWebUICallbackPort can be improved for clarity and maintainability:
- The logic is a bit redundant with repeated checks for
apiPort == 8318. Refactoring would make it more concise and easier to understand. - The magic number
8318(and8317in tests) should be defined as a named constant (e.g.,premiumAPIPort) to improve readability.
func (h *Handler) resolveAntigravityWebUICallbackPort(apiPort int) int {
isPremium := apiPort == 8318
if h != nil && h.cfg != nil {
if isPremium {
if port := h.cfg.RemoteManagement.AntigravityWebUICallbackPortPremium; port > 0 {
return port
}
} else {
if port := h.cfg.RemoteManagement.AntigravityWebUICallbackPort; port > 0 {
return port
}
}
}
if isPremium {
return defaultAntigravityPremiumPort
}
return defaultAntigravityStandardPort
}| apiPort: 8317, | ||
| cfg: nil, | ||
| want: defaultAntigravityStandardPort, | ||
| }, |
There was a problem hiding this comment.
The test suite for TestResolveAntigravityWebUICallbackPort is missing a test case for when the configuration is nil and the instance is premium (apiPort: 8318). Adding this test case would ensure complete coverage of the fallback logic.
},
{
name: "fallback to premium default when config is nil",
apiPort: 8318,
cfg: nil,
want: defaultAntigravityPremiumPort,
},
xkonjin
left a comment
There was a problem hiding this comment.
Code Review: Configurable Antigravity WebUI Callback Ports
Summary
Clean, targeted fix that adds flexibility for users running multiple instances or facing port conflicts. Good test coverage for the port resolution logic.
Strengths
- Well-scoped change — adds two optional config fields without breaking existing behavior
- Backwards compatible — defaults preserve existing port assignments (55121/55122)
- Good test coverage — unit tests cover nil config, standard/premium instance detection, and custom port overrides
- Clear documentation — config.example.yaml comments explain the use case
- Instance-aware logic — correctly distinguishes standard (8317) vs premium (8318) instances
Suggestions
1. Port validation in config loading
Consider adding validation during config parsing to reject obviously invalid ports (0, well-known ports <1024, or >65535). Currently invalid ports would fail silently at runtime when the callback forwarder cannot bind.
2. Document conflict scenarios
The example config mentions "conflicts with another listener" but does not specify which listener. Consider clarifying: conflicts with other Antigravity instances, or conflicts with any local service?
3. Test edge case — port collision at runtime
The startCallbackForwarder will fail if the port is already in use. Consider whether this error should be surfaced to the user with guidance ("Port X is in use, consider setting antigravity-webui-callback-port in config").
Security
- No new security surface — localhost-only callback URLs, no external exposure
- Port binding remains restricted to loopback interface
Verdict
LGTM — minor suggestions only, ready to merge.
Human written summary:
The intent of this change is, as written by a human:
The rest of this PR was written by gpt-5.4-adaptive, running in the OpenClaw harness. Full environment + prompt history appear at the end.
Changes
remote-management.antigravity-webui-callback-portandremote-management.antigravity-webui-callback-port-premiumconfig fields,config.example.yaml.Tests
GOTMPDIR=<exec-enabled-temp-dir> GOCACHE=<local-go-cache> go test ./internal/api/handlers/management/... ./internal/config/...-> passed,activeand HTTP health checks returned200,Risks
remote-management,Follow-ups
Prompt History
Environment
Harness: OpenClaw
Model: gpt-5.4
Thinking level: adaptive
Terminal: bash
System: Linux x64
Prompts
Request to avoid a hardcoded fix and prepare the change properly for PR.Request to continue autonomously until the implementation was complete.Request to open a polished PR in English.Request to override the workflow rule and have the assistant write the summary.This PR fixes the Antigravity web UI OAuth callback port conflict in CLIProxyAPI. The web UI callback ports should be configurable so standard and premium instances can run safely side by side without colliding with the default provider callback port.