Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 47 additions & 4 deletions internal/api/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,52 @@ func New(cfg *config.Config) *Client {
}
}

// analyzeEndpoint is the API path for the supermodel graph analysis.
const analyzeEndpoint = "/v1/graphs/supermodel"

// Analyze uploads a repository ZIP and runs the full analysis pipeline,
// returning the DisplayGraphResponse.
// polling until the async job completes and returning the Graph.
func (c *Client) Analyze(ctx context.Context, zipPath, idempotencyKey string) (*Graph, error) {
job, err := c.postZip(ctx, zipPath, idempotencyKey)
if err != nil {
return nil, err
}

// Poll until the job completes.
for job.Status == "pending" || job.Status == "processing" {
wait := time.Duration(job.RetryAfter) * time.Second
if wait <= 0 {
wait = 5 * time.Second
}
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-time.After(wait):
}

job, err = c.postZip(ctx, zipPath, idempotencyKey)
if err != nil {
return nil, err
}
}
Comment on lines +47 to +63
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add a hard stop for polling to avoid indefinite hangs.

Right now, Line 48 can loop forever if the API never reaches a terminal state. At call sites (internal/analyze/handler.go Line 57, internal/find/handler.go Line 161), ctx is passed through without a timeout wrapper, so this can block indefinitely.

Suggested guardrail
 func (c *Client) Analyze(ctx context.Context, zipPath, idempotencyKey string) (*Graph, error) {
+	if _, hasDeadline := ctx.Deadline(); !hasDeadline {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, defaultTimeout)
+		defer cancel()
+	}
+
 	job, err := c.postZip(ctx, zipPath, idempotencyKey)
 	if err != nil {
 		return nil, err
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/api/client.go` around lines 47 - 63, The polling loop in
internal/api/client.go that checks job.Status and calls c.postZip can hang
indefinitely; add a hard stop by introducing a max poll guard (e.g.,
maxPollDuration or maxPollAttempts) measured from a start time (time.Now()) or a
counter, and after the limit is exceeded return a clear error (or
context.DeadlineExceeded) instead of looping forever; keep existing handling of
job.RetryAfter and the ctx.Done() case, but before each sleep/iteration check
the elapsed time or attempts and bail out with an error referencing the
job/idempotency context so callers can handle it.


if job.Error != nil {
return nil, fmt.Errorf("analysis failed: %s", *job.Error)
}
if job.Status != "completed" {
return nil, fmt.Errorf("unexpected job status: %s", job.Status)
}

var result jobResult
if err := json.Unmarshal(job.Result, &result); err != nil {
return nil, fmt.Errorf("decode graph result: %w", err)
}
return &result.Graph, nil
Comment on lines +72 to +76
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard against completed responses with null/empty result payload.

A completed job with result: null would currently decode to an empty graph and look like success. Fail fast here so bad server payloads don’t silently pass.

Suggested validation
 	var result jobResult
+	if trimmed := bytes.TrimSpace(job.Result); len(trimmed) == 0 || bytes.Equal(trimmed, []byte("null")) {
+		return nil, fmt.Errorf("analysis completed without graph result")
+	}
 	if err := json.Unmarshal(job.Result, &result); err != nil {
 		return nil, fmt.Errorf("decode graph result: %w", err)
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/api/client.go` around lines 72 - 76, Check for null/empty payloads
before attempting to unmarshal the job result: in the code that uses
json.Unmarshal into jobResult (the jobResult type and the result variable),
first verify job.Result is not nil and not an empty/whitespace/"null" payload
(e.g., len==0 or trimmed equals "null") and return a descriptive error if it is;
after unmarshalling, also validate that result.Graph is present/non-empty and
return an error if the graph is missing so a completed job with result:null
cannot silently succeed.

}

// postZip sends the repository ZIP to the analyze endpoint and returns the
// raw job response (which may be pending, processing, or completed).
func (c *Client) postZip(ctx context.Context, zipPath, idempotencyKey string) (*JobResponse, error) {
f, err := os.Open(zipPath)
if err != nil {
return nil, err
Expand All @@ -53,11 +96,11 @@ func (c *Client) Analyze(ctx context.Context, zipPath, idempotencyKey string) (*
}
mw.Close()

var g Graph
if err := c.request(ctx, http.MethodPost, "/v1/graphs/supermodel", mw.FormDataContentType(), &buf, idempotencyKey, &g); err != nil {
var job JobResponse
if err := c.request(ctx, http.MethodPost, analyzeEndpoint, mw.FormDataContentType(), &buf, idempotencyKey, &job); err != nil {
return nil, err
}
return &g, nil
return &job, nil
}

// DisplayGraph fetches the composed display graph for an already-analyzed repo.
Expand Down
19 changes: 18 additions & 1 deletion internal/api/types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package api

import "fmt"
import (
"encoding/json"
"fmt"
)

// Node represents a graph node returned by the Supermodel API.
type Node struct {
Expand Down Expand Up @@ -90,6 +93,20 @@ func (g *Graph) NodeByID(id string) (Node, bool) {
return Node{}, false
}

// JobResponse is the async envelope returned by the API for long-running jobs.
type JobResponse struct {
Status string `json:"status"`
JobID string `json:"jobId"`
RetryAfter int `json:"retryAfter"`
Error *string `json:"error"`
Result json.RawMessage `json:"result"`
}

// jobResult is the inner result object containing the graph.
type jobResult struct {
Graph Graph `json:"graph"`
}

// Error represents a non-2xx response from the API.
type Error struct {
StatusCode int `json:"-"`
Expand Down
Loading