diff --git a/internal/gitea/workflows.go b/internal/gitea/workflows.go index 353b988..76a9d56 100644 --- a/internal/gitea/workflows.go +++ b/internal/gitea/workflows.go @@ -6,7 +6,6 @@ import ( "fmt" "net/url" "strconv" - "strings" ) // DispatchWorkflowArgs is the request body for a workflow_dispatch trigger. @@ -15,42 +14,26 @@ type DispatchWorkflowArgs struct { Inputs map[string]any `json:"inputs,omitempty"` } -// WorkflowRunTrigger holds the run ID extracted from the Location header. -type WorkflowRunTrigger struct { - RunID int64 -} - -// DispatchWorkflow triggers a workflow_dispatch event and returns the new run ID. -func (c *Client) DispatchWorkflow(ctx context.Context, owner, repo, workflow string, args DispatchWorkflowArgs) (*WorkflowRunTrigger, error) { +// DispatchWorkflow triggers a workflow_dispatch event. Gitea returns 204 No +// Content with NO Location header, so the response carries no run ID — callers +// resolve the new run separately via ListWorkflowRuns. Returns nil on success. +func (c *Client) DispatchWorkflow(ctx context.Context, owner, repo, workflow string, args DispatchWorkflowArgs) error { p := fmt.Sprintf("/api/v1/repos/%s/%s/actions/workflows/%s/dispatches", owner, repo, workflow) payload, err := json.Marshal(args) if err != nil { - return nil, err + return err } resp, err := c.doRaw(ctx, "POST", p, payload) if err != nil { - return nil, err + return err } if resp.Status != 204 { if mapErr := MapStatus(resp.Status, resp.Body); mapErr != nil { - return nil, mapErr + return mapErr } - return nil, fmt.Errorf("unexpected status %d", resp.Status) + return fmt.Errorf("unexpected status %d", resp.Status) } - location := resp.Headers.Get("Location") - if location == "" { - return nil, fmt.Errorf("missing Location header in dispatch response") - } - // Location is e.g. "/api/v1/repos/o/r/actions/runs/123" — take the last segment. - parts := strings.Split(strings.TrimRight(location, "/"), "/") - if len(parts) == 0 { - return nil, fmt.Errorf("malformed Location: %s", location) - } - runID, err := strconv.ParseInt(parts[len(parts)-1], 10, 64) - if err != nil { - return nil, fmt.Errorf("parse run id from %q: %w", location, err) - } - return &WorkflowRunTrigger{RunID: runID}, nil + return nil } // WorkflowRun represents a Gitea Actions run. diff --git a/internal/gitea/workflows_test.go b/internal/gitea/workflows_test.go index 5644f08..b76d871 100644 --- a/internal/gitea/workflows_test.go +++ b/internal/gitea/workflows_test.go @@ -14,6 +14,9 @@ import ( "github.com/stretchr/testify/require" ) +// Gitea's dispatch endpoint returns 204 No Content with NO Location header. +// Dispatch must succeed and forward ref+inputs in the body; the run ID is +// resolved separately by the tool via ListWorkflowRuns. func TestDispatchWorkflow(t *testing.T) { var gotBody []byte srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -22,18 +25,17 @@ func TestDispatchWorkflow(t *testing.T) { var err error gotBody, err = io.ReadAll(r.Body) assert.NoError(t, err) - w.Header().Set("Location", "/api/v1/repos/o/r/actions/runs/789") + // No Location header — matches real Gitea. w.WriteHeader(http.StatusNoContent) })) defer srv.Close() c := gitea.NewClient(srv.URL, "tok") - result, err := c.DispatchWorkflow(context.Background(), "o", "r", "ci.yml", gitea.DispatchWorkflowArgs{ + err := c.DispatchWorkflow(context.Background(), "o", "r", "ci.yml", gitea.DispatchWorkflowArgs{ Ref: "main", Inputs: map[string]any{"env": "prod"}, }) require.NoError(t, err) - assert.Equal(t, int64(789), result.RunID) var body map[string]any require.NoError(t, json.Unmarshal(gotBody, &body)) @@ -43,19 +45,6 @@ func TestDispatchWorkflow(t *testing.T) { assert.Equal(t, "prod", inputs["env"]) } -func TestDispatchWorkflowMissingLocation(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // 204 but no Location header - w.WriteHeader(http.StatusNoContent) - })) - defer srv.Close() - - c := gitea.NewClient(srv.URL, "tok") - _, err := c.DispatchWorkflow(context.Background(), "o", "r", "ci.yml", gitea.DispatchWorkflowArgs{Ref: "main"}) - require.Error(t, err) - assert.Contains(t, err.Error(), "Location") -} - func TestDispatchWorkflowError404(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) @@ -63,7 +52,7 @@ func TestDispatchWorkflowError404(t *testing.T) { defer srv.Close() c := gitea.NewClient(srv.URL, "tok") - _, err := c.DispatchWorkflow(context.Background(), "o", "r", "ci.yml", gitea.DispatchWorkflowArgs{Ref: "main"}) + err := c.DispatchWorkflow(context.Background(), "o", "r", "ci.yml", gitea.DispatchWorkflowArgs{Ref: "main"}) require.Error(t, err) assert.True(t, errors.Is(err, gitea.ErrNotFound)) } diff --git a/internal/tools/workflow_run_trigger.go b/internal/tools/workflow_run_trigger.go index 23d5d2d..8a8dcdb 100644 --- a/internal/tools/workflow_run_trigger.go +++ b/internal/tools/workflow_run_trigger.go @@ -4,12 +4,22 @@ import ( "context" "encoding/json" "fmt" + "time" "gitea.d-ma.be/mathias/gitea-mcp/internal/allowlist" "gitea.d-ma.be/mathias/gitea-mcp/internal/gitea" "gitea.d-ma.be/mathias/gitea-mcp/internal/registry" ) +// dispatchResolveBudget bounds how many times we poll for the dispatched run to +// appear, and dispatchResolvePoll is the gap between polls. Gitea returns 204 +// with no run ID, so we list workflow_dispatch runs and take the newest above +// the pre-dispatch baseline; it may register a beat after the 204. +const ( + dispatchResolveBudget = 5 + dispatchResolvePoll = time.Second +) + // WorkflowRunTrigger triggers a Gitea Actions workflow_dispatch run. type WorkflowRunTrigger struct { c *gitea.Client @@ -68,17 +78,82 @@ func (t *WorkflowRunTrigger) Call(ctx context.Context, raw json.RawMessage) (jso } } - result, err := t.c.DispatchWorkflow(ctx, args.Owner, args.Name, args.Workflow, gitea.DispatchWorkflowArgs{ + // Snapshot the newest existing workflow_dispatch run BEFORE dispatching, so we + // can tell our fresh run apart from a prior one (Gitea's 204 carries no run ID). + baseline := t.newestDispatchRunID(ctx, args.Owner, args.Name, args.Workflow, ref) + + if err := t.c.DispatchWorkflow(ctx, args.Owner, args.Name, args.Workflow, gitea.DispatchWorkflowArgs{ Ref: ref, Inputs: args.Inputs, - }) - if err != nil { + }); err != nil { return nil, err } - htmlURL := fmt.Sprintf("%s/%s/%s/actions/runs/%d", t.baseURL, args.Owner, args.Name, result.RunID) + // Resolve the new run by listing workflow_dispatch runs and taking the newest + // one whose ID exceeds the baseline. Poll briefly: the run can register a beat + // after the 204. + var run *gitea.WorkflowRun + for i := 0; i < dispatchResolveBudget; i++ { + if i > 0 { + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(dispatchResolvePoll): + } + } + if r := t.newestDispatchRun(ctx, args.Owner, args.Name, args.Workflow, ref); r != nil && r.ID > baseline { + run = r + break + } + } + + // Dispatch succeeded (204). If the run has not surfaced yet, say so honestly + // rather than failing — the workflow IS firing; the caller can list runs. + if run == nil { + return textOK(map[string]any{ + "dispatched": true, + "note": "dispatch accepted but the run did not register within the resolve window; " + + "list recent workflow_dispatch runs to find it", + }) + } + + htmlURL := run.HTMLURL + if htmlURL == "" { + htmlURL = fmt.Sprintf("%s/%s/%s/actions/runs/%d", t.baseURL, args.Owner, args.Name, run.ID) + } return textOK(map[string]any{ - "run_id": result.RunID, - "html_url": htmlURL, + "dispatched": true, + "run_id": run.ID, + "html_url": htmlURL, }) } + +// newestDispatchRun returns the most recent workflow_dispatch run for the given +// workflow and ref, or nil if none / on listing error (best-effort resolution). +func (t *WorkflowRunTrigger) newestDispatchRun(ctx context.Context, owner, name, workflow, ref string) *gitea.WorkflowRun { + resp, err := t.c.ListWorkflowRuns(ctx, owner, name, gitea.ListWorkflowRunsArgs{ + Event: "workflow_dispatch", + Workflow: workflow, + Branch: ref, + Limit: 20, + }) + if err != nil || resp == nil { + return nil + } + var newest *gitea.WorkflowRun + for i := range resp.WorkflowRuns { + r := &resp.WorkflowRuns[i] + if newest == nil || r.ID > newest.ID { + newest = r + } + } + return newest +} + +// newestDispatchRunID is newestDispatchRun's ID, or 0 if none. +func (t *WorkflowRunTrigger) newestDispatchRunID(ctx context.Context, owner, name, workflow, ref string) int64 { + if r := t.newestDispatchRun(ctx, owner, name, workflow, ref); r != nil { + return r.ID + } + return 0 +} diff --git a/internal/tools/workflow_run_trigger_test.go b/internal/tools/workflow_run_trigger_test.go index 86b3251..a85a3fc 100644 --- a/internal/tools/workflow_run_trigger_test.go +++ b/internal/tools/workflow_run_trigger_test.go @@ -3,8 +3,10 @@ package tools_test import ( "context" "encoding/json" + "io" "net/http" "net/http/httptest" + "strings" "testing" "gitea.d-ma.be/mathias/gitea-mcp/internal/allowlist" @@ -14,10 +16,67 @@ import ( "github.com/stretchr/testify/require" ) -func TestWorkflowRunTriggerSuccess(t *testing.T) { - // Fake server handles both the repo endpoint (default_branch) and the dispatch endpoint. +// runsListJSON is a workflow_runs listing body with a single run of the given id. +func runsListJSON(id int) string { + return `{"total_count":1,"workflow_runs":[{"id":` + + fmtInt(id) + + `,"status":"queued","event":"workflow_dispatch","html_url":"http://gitea.example/mathias/myrepo/actions/runs/` + + fmtInt(id) + `"}]}` +} + +func fmtInt(i int) string { b, _ := json.Marshal(i); return string(b) } + +// The dispatch endpoint returns 204 with NO Location header (real Gitea). The +// tool must treat that as success, forward inputs, and resolve the new run by +// listing workflow_dispatch runs and picking the newest one above the +// pre-dispatch baseline. +func TestWorkflowRunTriggerResolvesRunViaListing(t *testing.T) { + dispatched := false + var gotBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api/v1/repos/mathias/myrepo/actions/workflows/ci.yml/dispatches" && r.Method == http.MethodPost: + gotBody, _ = io.ReadAll(r.Body) + dispatched = true + w.WriteHeader(http.StatusNoContent) // no Location header + case strings.HasPrefix(r.URL.Path, "/api/v1/repos/mathias/myrepo/actions/runs"): + w.Header().Set("Content-Type", "application/json") + if !dispatched { + _, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":[]}`)) // baseline: none yet + return + } + _, _ = w.Write([]byte(runsListJSON(100))) + default: + http.NotFound(w, r) + } + })) + defer srv.Close() + + tool := tools.NewWorkflowRunTrigger(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"mathias"}), srv.URL) + out, err := tool.Call(context.Background(), json.RawMessage( + `{"owner":"mathias","name":"myrepo","workflow":"ci.yml","ref":"main","inputs":{"issue_number":"36","harness":"agentsquad"}}`)) + require.NoError(t, err) + assert.True(t, dispatched, "expected POST dispatch") + + // inputs forwarded to the dispatch body + var body map[string]any + require.NoError(t, json.Unmarshal(gotBody, &body)) + assert.Equal(t, "main", body["ref"]) + inputs, ok := body["inputs"].(map[string]any) + require.True(t, ok, "inputs must be present in dispatch body") + assert.Equal(t, "36", inputs["issue_number"]) + assert.Equal(t, "agentsquad", inputs["harness"]) + + // run id resolved via listing (not a Location header) + var result map[string]any + require.NoError(t, json.Unmarshal(out, &result)) + assert.Equal(t, float64(100), result["run_id"]) + assert.Contains(t, result["html_url"], "/actions/runs/100") +} + +func TestWorkflowRunTriggerDefaultBranch(t *testing.T) { repoHit := false - dispatchHit := false + dispatched := false srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch { case r.URL.Path == "/api/v1/repos/mathias/myrepo" && r.Method == http.MethodGet: @@ -25,9 +84,15 @@ func TestWorkflowRunTriggerSuccess(t *testing.T) { w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{"name":"myrepo","full_name":"mathias/myrepo","default_branch":"main"}`)) case r.URL.Path == "/api/v1/repos/mathias/myrepo/actions/workflows/ci.yml/dispatches" && r.Method == http.MethodPost: - dispatchHit = true - w.Header().Set("Location", "/api/v1/repos/mathias/myrepo/actions/runs/42") + dispatched = true w.WriteHeader(http.StatusNoContent) + case strings.HasPrefix(r.URL.Path, "/api/v1/repos/mathias/myrepo/actions/runs"): + w.Header().Set("Content-Type", "application/json") + if !dispatched { + _, _ = w.Write([]byte(`{"total_count":0,"workflow_runs":[]}`)) + return + } + _, _ = w.Write([]byte(runsListJSON(55))) default: http.NotFound(w, r) } @@ -38,37 +103,10 @@ func TestWorkflowRunTriggerSuccess(t *testing.T) { out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"mathias","name":"myrepo","workflow":"ci.yml"}`)) require.NoError(t, err) assert.True(t, repoHit, "expected GET /repo for default branch") - assert.True(t, dispatchHit, "expected POST dispatch") var result map[string]any require.NoError(t, json.Unmarshal(out, &result)) - assert.Equal(t, float64(42), result["run_id"]) - assert.Contains(t, result["html_url"], "/mathias/myrepo/actions/runs/42") -} - -func TestWorkflowRunTriggerExplicitRef(t *testing.T) { - repoHit := false - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/api/v1/repos/mathias/myrepo" { - repoHit = true - } - if r.URL.Path == "/api/v1/repos/mathias/myrepo/actions/workflows/ci.yml/dispatches" { - w.Header().Set("Location", "/api/v1/repos/mathias/myrepo/actions/runs/99") - w.WriteHeader(http.StatusNoContent) - return - } - http.NotFound(w, r) - })) - defer srv.Close() - - tool := tools.NewWorkflowRunTrigger(gitea.NewClient(srv.URL, "tok"), allowlist.New([]string{"mathias"}), srv.URL) - out, err := tool.Call(context.Background(), json.RawMessage(`{"owner":"mathias","name":"myrepo","workflow":"ci.yml","ref":"develop"}`)) - require.NoError(t, err) - assert.False(t, repoHit, "should not call GET /repo when ref is provided") - - var result map[string]any - require.NoError(t, json.Unmarshal(out, &result)) - assert.Equal(t, float64(99), result["run_id"]) + assert.Equal(t, float64(55), result["run_id"]) } func TestWorkflowRunTriggerAllowlistRejects(t *testing.T) {