fix(workflow_run_trigger): 204 dispatch is success; resolve run via listing (#41)
Gitea's workflow_dispatch endpoint returns 204 No Content with no Location header. DispatchWorkflow required that header, so every successful dispatch errored with "missing Location header" and never yielded a run ID — making the tool unusable for CAD dispatch. - DispatchWorkflow now returns error-only; 204 = success, no Location needed. Body already carried ref+inputs; kept and covered by test. - Tool snapshots the newest existing workflow_dispatch run before dispatch, then polls ListWorkflowRuns after and returns the newest run with ID above that baseline (avoids returning a stale prior run). Falls back to an honest "dispatched, run not yet registered" result rather than failing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user