From a757ca81e32dd401f0016a38f79429fc93ccd6b2 Mon Sep 17 00:00:00 2001 From: Virgil Date: Mon, 30 Mar 2026 15:33:01 +0000 Subject: [PATCH] fix(ax): preserve transport causes and remove MustCompile Co-Authored-By: Virgil --- .core/reference/docs/RFC.md | 3 ++- docs/RFC.md | 1 + pkg/agentic/review_queue.go | 16 ++++++++++++++-- pkg/agentic/transport.go | 26 ++++++++++++++------------ pkg/agentic/transport_test.go | 9 +++++++++ pkg/brain/direct.go | 12 ++++++++++-- 6 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.core/reference/docs/RFC.md b/.core/reference/docs/RFC.md index 90ef3b8..4fe1d73 100644 --- a/.core/reference/docs/RFC.md +++ b/.core/reference/docs/RFC.md @@ -435,6 +435,7 @@ func (s *PrepSubsystem) gitCmd(ctx context.Context, dir string, args ...string) ## Changelog - 2026-03-29: cmd/core-agent no longer rewrites `os.Args` before startup. The binary-owned commands now use named handlers, keeping the entrypoint on Core CLI primitives instead of repo-local argument mutation. -- 2026-03-26: WIP — net/http consolidated to transport.go (ONE file). net/url + io/fs eliminated. RFC-025 updated with 3 new quality gates (net/http, net/url, io/fs). 1:1 test + example test coverage. Array[T].Deduplicate replaces custom helpers. Remaining: remove dead `client` field from test literals, brain/provider.go Gin handler. +- 2026-03-30: transport helpers preserve request and read causes, brain direct API calls surface upstream bodies, and review queue retry parsing no longer uses `MustCompile`. +- 2026-03-26: net/http consolidated to transport.go (ONE file). net/url + io/fs eliminated. RFC-025 updated with 3 new quality gates (net/http, net/url, io/fs). 1:1 test + example test coverage. Array[T].Deduplicate replaces custom helpers. - 2026-03-25: Quality gates pass. Zero disallowed imports (all 10). encoding/json→Core JSON. path/filepath→Core Path. os→Core Env/Fs. io→Core ReadAll/WriteAll. go-process fully Result-native. ServiceRuntime on all subsystems. 22 named Actions + Task pipeline. ChannelNotifier→IPC. Reference docs synced. - 2026-03-25: Initial spec — written with full core/go v0.8.0 domain context. diff --git a/docs/RFC.md b/docs/RFC.md index 9a3ee97..29c5ffb 100644 --- a/docs/RFC.md +++ b/docs/RFC.md @@ -434,6 +434,7 @@ func (s *PrepSubsystem) gitCmd(ctx context.Context, dir string, args ...string) ## Changelog +- 2026-03-30: transport helpers preserve request and read causes, brain direct API calls surface upstream bodies, and review queue retry parsing no longer uses `MustCompile`. - 2026-03-30: main now logs startup failures with structured context, and the workspace contract reference restored usage-example comments for the Action lifecycle messages. - 2026-03-30: plan IDs now come from core.ID(), workspace prep validates org/repo names with core.ValidateName, and plan paths use core.SanitisePath. - 2026-03-29: cmd/core-agent no longer rewrites `os.Args` before startup. The binary-owned commands now use named handlers, keeping the entrypoint on Core CLI primitives instead of repo-local argument mutation. diff --git a/pkg/agentic/review_queue.go b/pkg/agentic/review_queue.go index e6b4fbe..5067732 100644 --- a/pkg/agentic/review_queue.go +++ b/pkg/agentic/review_queue.go @@ -53,6 +53,16 @@ type RateLimitInfo struct { Message string `json:"message,omitempty"` } +var retryAfterPattern = compileRetryAfterPattern() + +func compileRetryAfterPattern() *regexp.Regexp { + pattern, err := regexp.Compile(`(\d+)\s*minutes?\s*(?:and\s*)?(\d+)?\s*seconds?`) + if err != nil { + return nil + } + return pattern +} + func (s *PrepSubsystem) registerReviewQueueTool(server *mcp.Server) { mcp.AddTool(server, &mcp.Tool{ Name: "agentic_review_queue", @@ -293,8 +303,10 @@ func countFindings(output string) int { // parseRetryAfter extracts the retry duration from a rate limit message. // Example: "please try after 4 minutes and 56 seconds" func parseRetryAfter(message string) time.Duration { - re := regexp.MustCompile(`(\d+)\s*minutes?\s*(?:and\s*)?(\d+)?\s*seconds?`) - matches := re.FindStringSubmatch(message) + if retryAfterPattern == nil { + return 5 * time.Minute + } + matches := retryAfterPattern.FindStringSubmatch(message) if len(matches) >= 2 { mins := parseInt(matches[1]) secs := 0 diff --git a/pkg/agentic/transport.go b/pkg/agentic/transport.go index a15a829..25174fc 100644 --- a/pkg/agentic/transport.go +++ b/pkg/agentic/transport.go @@ -48,7 +48,8 @@ func (s *httpStream) Send(data []byte) error { r := core.ReadAll(resp.Body) if !r.OK { - return core.E("httpStream.Send", "failed to read response", nil) + err, _ := r.Value.(error) + return core.E("httpStream.Send", "failed to read response", err) } s.response = []byte(r.Value.(string)) return nil @@ -183,7 +184,7 @@ func httpDo(ctx context.Context, method, url, body, token, authScheme string) co req, err = http.NewRequestWithContext(ctx, method, url, nil) } if err != nil { - return core.Result{OK: false} + return core.Result{Value: core.E("httpDo", "create request", err), OK: false} } req.Header.Set("Content-Type", "application/json") @@ -197,12 +198,13 @@ func httpDo(ctx context.Context, method, url, body, token, authScheme string) co resp, err := defaultClient.Do(req) if err != nil { - return core.Result{OK: false} + return core.Result{Value: core.E("httpDo", "request failed", err), OK: false} } r := core.ReadAll(resp.Body) if !r.OK { - return core.Result{OK: false} + err, _ := r.Value.(error) + return core.Result{Value: core.E("httpDo", "failed to read response", err), OK: false} } return core.Result{Value: r.Value.(string), OK: resp.StatusCode < 400} @@ -246,13 +248,13 @@ func mcpInitializeResult(ctx context.Context, url, token string) core.Result { body := core.JSONMarshalString(initReq) req, err := http.NewRequestWithContext(ctx, "POST", url, core.NewReader(body)) if err != nil { - return core.Result{Value: core.E("mcpInitialize", "create request", nil), OK: false} + return core.Result{Value: core.E("mcpInitialize", "create request", err), OK: false} } mcpHeaders(req, token, "") resp, err := defaultClient.Do(req) if err != nil { - return core.Result{Value: core.E("mcpInitialize", "request failed", nil), OK: false} + return core.Result{Value: core.E("mcpInitialize", "request failed", err), OK: false} } defer resp.Body.Close() @@ -270,7 +272,10 @@ func mcpInitializeResult(ctx context.Context, url, token string) core.Result { "jsonrpc": "2.0", "method": "notifications/initialized", }) - notifReq, _ := http.NewRequestWithContext(ctx, "POST", url, core.NewReader(notif)) + notifReq, err := http.NewRequestWithContext(ctx, "POST", url, core.NewReader(notif)) + if err != nil { + return core.Result{Value: core.E("mcpInitialize", "create notification request", err), OK: false} + } mcpHeaders(notifReq, token, sessionID) notifResp, err := defaultClient.Do(notifReq) if err == nil { @@ -300,13 +305,13 @@ func mcpCall(ctx context.Context, url, token, sessionID string, body []byte) ([] func mcpCallResult(ctx context.Context, url, token, sessionID string, body []byte) core.Result { req, err := http.NewRequestWithContext(ctx, "POST", url, core.NewReader(string(body))) if err != nil { - return core.Result{Value: core.E("mcpCall", "create request", nil), OK: false} + return core.Result{Value: core.E("mcpCall", "create request", err), OK: false} } mcpHeaders(req, token, sessionID) resp, err := defaultClient.Do(req) if err != nil { - return core.Result{Value: core.E("mcpCall", "request failed", nil), OK: false} + return core.Result{Value: core.E("mcpCall", "request failed", err), OK: false} } defer resp.Body.Close() @@ -339,9 +344,6 @@ func readSSEDataResult(resp *http.Response) core.Result { r := core.ReadAll(resp.Body) if !r.OK { err, _ := r.Value.(error) - if err == nil { - return core.Result{Value: core.E("readSSEData", "failed to read response", nil), OK: false} - } return core.Result{Value: core.E("readSSEData", "failed to read response", err), OK: false} } for _, line := range core.Split(r.Value.(string), "\n") { diff --git a/pkg/agentic/transport_test.go b/pkg/agentic/transport_test.go index 24204a2..f8dc098 100644 --- a/pkg/agentic/transport_test.go +++ b/pkg/agentic/transport_test.go @@ -36,6 +36,15 @@ func TestTransport_HTTPGet_Good(t *testing.T) { assert.Equal(t, `{"status":"ok"}`, result.Value.(string)) } +func TestTransport_HTTPGet_Bad_InvalidURL(t *testing.T) { + result := HTTPGet(context.Background(), "://bad", "", "") + + assert.False(t, result.OK) + err, ok := result.Value.(error) + require.True(t, ok) + assert.Contains(t, err.Error(), "create request") +} + func TestTransport_DriveGet_Good(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/repos/core/go-io", r.URL.Path) diff --git a/pkg/brain/direct.go b/pkg/brain/direct.go index b96fc35..512d678 100644 --- a/pkg/brain/direct.go +++ b/pkg/brain/direct.go @@ -108,6 +108,7 @@ func (s *DirectSubsystem) apiCall(ctx context.Context, method, path string, body if s.apiKey == "" { return core.Result{ Value: core.E("brain.apiCall", "no API key (set CORE_BRAIN_KEY or create ~/.claude/brain.key)", nil), + OK: false, } } @@ -119,13 +120,20 @@ func (s *DirectSubsystem) apiCall(ctx context.Context, method, path string, body r := agentic.HTTPDo(ctx, method, requestURL, bodyStr, s.apiKey, "Bearer") if !r.OK { core.Error("brain API call failed", "method", method, "path", path) - return core.Result{Value: core.E("brain.apiCall", "API call failed", nil)} + if err, ok := r.Value.(error); ok { + return core.Result{Value: core.E("brain.apiCall", "API call failed", err), OK: false} + } + if responseBody, ok := r.Value.(string); ok && responseBody != "" { + return core.Result{Value: core.E("brain.apiCall", core.Concat("API call failed: ", core.Trim(responseBody)), nil), OK: false} + } + return core.Result{Value: core.E("brain.apiCall", "API call failed", nil), OK: false} } var result map[string]any if ur := core.JSONUnmarshalString(r.Value.(string), &result); !ur.OK { core.Error("brain API response parse failed", "method", method, "path", path) - return core.Result{Value: core.E("brain.apiCall", "parse response", nil)} + err, _ := ur.Value.(error) + return core.Result{Value: core.E("brain.apiCall", "parse response", err), OK: false} } return core.Result{Value: result, OK: true}