feat: implement initial AxeClient for remote agent execution - #51
feat: implement initial AxeClient for remote agent execution#51jhawpetoss6-collab wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughA new client module ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/client/api.go`:
- Around line 17-19: The code currently discards errors from json.Marshal and
http.NewRequest and also ignores Decode errors and HTTP status codes; update the
function in pkg/client/api.go to check and return wrapped errors from
json.Marshal(input) and http.NewRequest("POST", url, ...) (include context like
"failed to marshal request body" and "failed to construct HTTP request"), ensure
client.Do(req) is only called if req != nil, validate resp.StatusCode and return
a clear error if it's not 2xx before attempting to decode, and handle/return
errors from json.NewDecoder(resp.Body).Decode(...) with contextual messages
(e.g., "failed to decode response body"); reference the symbols json.Marshal,
http.NewRequest, client.Do(req), resp.StatusCode, and
json.NewDecoder(...).Decode when making these changes.
- Around line 24-32: The HTTP call handling in the function that calls
client.Do(req) must be hardened: wrap the error returned from client.Do with
contextual hints (network, API key, BaseURL) before returning, verify
resp.StatusCode is 2xx (use resp.StatusCode) and return an explicit error
including the status and response body for non-2xx responses, and handle JSON
decoding errors from json.NewDecoder(resp.Body).Decode(&result) by returning a
wrapped decode error instead of ignoring it; keep the existing defer
resp.Body.Close() and ensure you import any error-wrapping or ioutil/io packages
needed to read the body for error messages.
- Around line 23-24: AxeClient currently constructs a new http.Client per call
(client := &http.Client{}) causing no timeouts and poor connection reuse; add an
HTTPClient field (e.g., HTTPClient http.Client or HTTPClient *http.Client) to
the AxeClient struct, use that field instead of creating a local client in the
method that calls client.Do(req), and ensure the constructor/newter for
AxeClient sets a sensible default timeout (e.g., 10s) when HTTPClient is nil so
callers can inject a custom client while preserving a safe fallback.
- Line 16: The code concatenates raw agentID into the path and also creates a
local variable named url which shadows the net/url package; change to normalize
the base with strings.TrimRight(c.BaseURL, "/") (or ensure trailing slash
handling), escape the agent ID with url.PathEscape(agentID), and assign the
result to a non-conflicting name such as endpoint (e.g., endpoint := base +
"/v1/agents/" + url.PathEscape(agentID) + "/execute") so you don't shadow the
net/url import and the path is safe.
| } | ||
|
|
||
| func (c *AxeClient) ExecuteAgent(agentID string, input map[string]interface{}) (map[string]interface{}, error) { | ||
| url := c.BaseURL + "/v1/agents/" + agentID + "/execute" |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
cat -n pkg/client/api.goRepository: jrswab/axe
Repository: jrswab/axe
Exit code: 0
stdout:
1 package client
2
3 import (
4 "bytes"
5 "encoding/json"
6 "net/http"
7 )
8
9 // AxeClient handles communication with the Axe agent service.
10 type AxeClient struct {
11 BaseURL string
12 APIKey string
13 }
14
15 func (c *AxeClient) ExecuteAgent(agentID string, input map[string]interface{}) (map[string]interface{}, error) {
16 url := c.BaseURL + "/v1/agents/" + agentID + "/execute"
17 data, _ := json.Marshal(input)
18
19 req, _ := http.NewRequest("POST", url, bytes.NewBuffer(data))
20 req.Header.Set("Authorization", "Bearer " + c.APIKey)
21 req.Header.Set("Content-Type", "application/json")
22
23 client := &http.Client{}
24 resp, err := client.Do(req)
25 if err != nil {
26 return nil, err
27 }
28 defer resp.Body.Close()
29
30 var result map[string]interface{}
31 json.NewDecoder(resp.Body).Decode(&result)
32 return result, nil
33 }
Script executed:
rg "BaseURL.*\+" pkg/Repository: jrswab/axe
Repository: jrswab/axe
Exit code: 0
stdout:
pkg/client/api.go: url := c.BaseURL + "/v1/agents/" + agentID + "/execute"
Escape agentID before building the endpoint path — but fix the variable shadowing in the suggested solution.
Line 16 concatenates raw agentID into the path. Special characters like /, ?, or spaces will corrupt the URL structure. Use url.PathEscape(agentID) to safely escape the ID, and normalize the BaseURL trailing slash.
However, the suggested fix has a critical bug: assigning to a variable named url shadows the net/url package import, causing url.PathEscape() to fail. Use a different variable name like endpoint instead:
Corrected fix
import (
"bytes"
"encoding/json"
"net/http"
+ "net/url"
+ "strings"
)
@@
- url := c.BaseURL + "/v1/agents/" + agentID + "/execute"
+ base := strings.TrimRight(c.BaseURL, "/")
+ endpoint := base + "/v1/agents/" + url.PathEscape(agentID) + "/execute"
- req, _ := http.NewRequest("POST", url, bytes.NewBuffer(data))
+ req, _ := http.NewRequest("POST", endpoint, bytes.NewBuffer(data))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| url := c.BaseURL + "/v1/agents/" + agentID + "/execute" | |
| import ( | |
| "bytes" | |
| "encoding/json" | |
| "net/http" | |
| "net/url" | |
| "strings" | |
| ) | |
| // ... in the relevant function: | |
| base := strings.TrimRight(c.BaseURL, "/") | |
| endpoint := base + "/v1/agents/" + url.PathEscape(agentID) + "/execute" | |
| req, _ := http.NewRequest("POST", endpoint, bytes.NewBuffer(data)) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/client/api.go` at line 16, The code concatenates raw agentID into the
path and also creates a local variable named url which shadows the net/url
package; change to normalize the base with strings.TrimRight(c.BaseURL, "/") (or
ensure trailing slash handling), escape the agent ID with
url.PathEscape(agentID), and assign the result to a non-conflicting name such as
endpoint (e.g., endpoint := base + "/v1/agents/" + url.PathEscape(agentID) +
"/execute") so you don't shadow the net/url import and the path is safe.
| data, _ := json.Marshal(input) | ||
|
|
||
| req, _ := http.NewRequest("POST", url, bytes.NewBuffer(data)) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the api.go file
find . -name "api.go" -path "*/client/*" | head -5Repository: jrswab/axe
Length of output: 73
🏁 Script executed:
cat -n ./pkg/client/api.goRepository: jrswab/axe
Length of output: 1067
Handle request-construction errors; right now they're silently dropped.
Lines 17 and 19 discard json.Marshal and http.NewRequest errors respectively. If request creation fails, req could be nil, making line 24 (client.Do(req)) unsafe. Additionally, line 31 silently ignores Decode errors, and there's no HTTP status code validation before attempting to decode the response. Bubble these up with actionable error messages that tell users how to fix the issue.
Suggested fix
import (
"bytes"
"encoding/json"
+ "fmt"
"net/http"
)
@@
- data, _ := json.Marshal(input)
+ data, err := json.Marshal(input)
+ if err != nil {
+ return nil, fmt.Errorf("encode execute payload as JSON: ensure input contains JSON-serializable values: %w", err)
+ }
- req, _ := http.NewRequest("POST", url, bytes.NewBuffer(data))
+ req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(data))
+ if err != nil {
+ return nil, fmt.Errorf("build execute request: check BaseURL %q and agentID %q: %w", c.BaseURL, agentID, err)
+ }As per coding guidelines, "Errors should help the user fix the problem, not just describe it."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data, _ := json.Marshal(input) | |
| req, _ := http.NewRequest("POST", url, bytes.NewBuffer(data)) | |
| data, err := json.Marshal(input) | |
| if err != nil { | |
| return nil, fmt.Errorf("encode execute payload as JSON: ensure input contains JSON-serializable values: %w", err) | |
| } | |
| req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(data)) | |
| if err != nil { | |
| return nil, fmt.Errorf("build execute request: check BaseURL %q and agentID %q: %w", c.BaseURL, agentID, err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/client/api.go` around lines 17 - 19, The code currently discards errors
from json.Marshal and http.NewRequest and also ignores Decode errors and HTTP
status codes; update the function in pkg/client/api.go to check and return
wrapped errors from json.Marshal(input) and http.NewRequest("POST", url, ...)
(include context like "failed to marshal request body" and "failed to construct
HTTP request"), ensure client.Do(req) is only called if req != nil, validate
resp.StatusCode and return a clear error if it's not 2xx before attempting to
decode, and handle/return errors from json.NewDecoder(resp.Body).Decode(...)
with contextual messages (e.g., "failed to decode response body"); reference the
symbols json.Marshal, http.NewRequest, client.Do(req), resp.StatusCode, and
json.NewDecoder(...).Decode when making these changes.
| client := &http.Client{} | ||
| resp, err := client.Do(req) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the actual file structure
find . -name "api.go" -type f | head -20Repository: jrswab/axe
Length of output: 73
🏁 Script executed:
# Check the pkg/client/api.go file around lines 23-24
if [ -f "pkg/client/api.go" ]; then
wc -l pkg/client/api.go
echo "---"
head -40 pkg/client/api.go
fiRepository: jrswab/axe
Length of output: 861
🏁 Script executed:
# Let's see the broader context and method signatures
if [ -f "pkg/client/api.go" ]; then
cat -n pkg/client/api.go | head -60
fiRepository: jrswab/axe
Length of output: 1067
🏁 Script executed:
# Search for AxeClient struct definition and usage
rg "type AxeClient" -A 5Repository: jrswab/axe
Length of output: 333
🏁 Script executed:
# Look for http.Client creation patterns in the codebase
rg "http\.Client\{\}" -B 2 -A 2 --type goRepository: jrswab/axe
Length of output: 268
🏁 Script executed:
# Check for any existing HTTPClient field or similar patterns
rg "HTTPClient|httpClient" --type go -B 2 -A 2Repository: jrswab/axe
Length of output: 891
🏁 Script executed:
# Look for tests related to this API client
fd "test.go|_test.go" | xargs rg -l "AxeClient" 2>/dev/null || echo "No tests found with AxeClient"Repository: jrswab/axe
Length of output: 83
Inject the HTTP client and set a timeout (no per-call client creation).
Line 23 creates a fresh &http.Client{} on every call—no timeout, no connection reuse. That's a reliability and performance footgun. Inject an HTTPClient field on AxeClient with a sensible default timeout fallback (the codebase already uses this pattern elsewhere for MCP transports).
This also aligns with the guideline: "No global state — pass dependencies explicitly."
Suggested refactor
import (
"bytes"
"encoding/json"
"net/http"
+ "time"
)
@@
type AxeClient struct {
BaseURL string
APIKey string
+ HTTPClient *http.Client
}
@@
- client := &http.Client{}
- resp, err := client.Do(req)
+ httpClient := c.HTTPClient
+ if httpClient == nil {
+ httpClient = &http.Client{Timeout: 30 * time.Second}
+ }
+ resp, err := httpClient.Do(req)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client := &http.Client{} | |
| resp, err := client.Do(req) | |
| httpClient := c.HTTPClient | |
| if httpClient == nil { | |
| httpClient = &http.Client{Timeout: 30 * time.Second} | |
| } | |
| resp, err := httpClient.Do(req) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/client/api.go` around lines 23 - 24, AxeClient currently constructs a new
http.Client per call (client := &http.Client{}) causing no timeouts and poor
connection reuse; add an HTTPClient field (e.g., HTTPClient http.Client or
HTTPClient *http.Client) to the AxeClient struct, use that field instead of
creating a local client in the method that calls client.Do(req), and ensure the
constructor/newter for AxeClient sets a sensible default timeout (e.g., 10s)
when HTTPClient is nil so callers can inject a custom client while preserving a
safe fallback.
| resp, err := client.Do(req) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| var result map[string]interface{} | ||
| json.NewDecoder(resp.Body).Decode(&result) | ||
| return result, nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "api.go" -path "*/client/*" | head -20Repository: jrswab/axe
Length of output: 73
🏁 Script executed:
wc -l pkg/client/api.go 2>/dev/null || echo "File not found, searching..."Repository: jrswab/axe
Length of output: 74
🏁 Script executed:
if [ -f pkg/client/api.go ]; then
sed -n '20,40p' pkg/client/api.go
else
find . -type f -name "*.go" | grep -i api | head -10
fiRepository: jrswab/axe
Length of output: 381
🏁 Script executed:
cat -n pkg/client/api.goRepository: jrswab/axe
Length of output: 1067
Add HTTP status checking and JSON decode error handling to prevent silent failures.
Lines 24-32 silently return success for HTTP 4xx/5xx responses and ignore JSON decode errors—resulting in failed executions reported as successful. Additionally, the error at line 24 lacks context to help operators debug (network issue? Invalid API key? Wrong BaseURL?).
Import the necessary packages and:
- Wrap HTTP errors with diagnostic hints
- Check
resp.StatusCodeis in the 2xx range before reading the body - Handle JSON decode errors explicitly
Suggested fix
import (
"bytes"
"encoding/json"
"fmt"
+ "io"
"net/http"
+ "strings"
)
@@
client := &http.Client{}
resp, err := client.Do(req)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("execute agent %q request failed: verify BaseURL/network/API key: %w", agentID, err)
}
defer resp.Body.Close()
+
+ if resp.StatusCode < 200 || resp.StatusCode >= 300 {
+ body, _ := io.ReadAll(io.LimitReader(resp.Body, 4096))
+ return nil, fmt.Errorf("execute agent %q failed with status %d: %s", agentID, resp.StatusCode, strings.TrimSpace(string(body)))
+ }
var result map[string]interface{}
- json.NewDecoder(resp.Body).Decode(&result)
+ if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
+ return nil, fmt.Errorf("decode execute response for agent %q as JSON: %w", agentID, err)
+ }
return result, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/client/api.go` around lines 24 - 32, The HTTP call handling in the
function that calls client.Do(req) must be hardened: wrap the error returned
from client.Do with contextual hints (network, API key, BaseURL) before
returning, verify resp.StatusCode is 2xx (use resp.StatusCode) and return an
explicit error including the status and response body for non-2xx responses, and
handle JSON decoding errors from json.NewDecoder(resp.Body).Decode(&result) by
returning a wrapped decode error instead of ignoring it; keep the existing defer
resp.Body.Close() and ensure you import any error-wrapping or ioutil/io packages
needed to read the body for error messages.
|
I like this idea! When you have time check out what CodeRabbit has to say and run |
|
@jhawpetoss6-collab are you able to continue working on this effort or shall I pick it up? |
Summary
Introduces initial
AxeClientimplementation inpkg/clientfor programmatic remote agent execution. The client supports API-key authentication and uses only the Go standard library, providing a lightweight, dependency-free solution for triggering and managing Axe agents.Changelog
Added
AxeClienttype withBaseURLandAPIKeyfields for remote agent communicationExecuteAgentmethod to trigger remote agent execution with input parameters via POST requests to the/v1/agents/{agentID}/executeendpoint