Skip to content
Closed
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
10 changes: 10 additions & 0 deletions org-templates/molecule-dev/org.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ defaults:
performance: [Backend Engineer]
docs: [Documentation Specialist]
mixed: [Dev Lead]
# Evolution-cron categories (#93): these four are fired by hourly
# self-review schedules (Research Lead, Technical Researcher, Dev Lead,
# DevOps Engineer). Routing them to the same role that generated them
# is a safe default — it converts the summary into a delegation back
# to the author so they act on their own findings. Override per-org
# if you want a different fan-out.
research: [Research Lead]
plugins: [Technical Researcher]
template: [Dev Lead]
channels: [DevOps Engineer]

# workspace_dir: not set by default — each agent gets an isolated Docker volume
# Set per-workspace to bind-mount a host directory as /workspace
Expand Down
67 changes: 67 additions & 0 deletions platform/internal/handlers/webhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,29 @@ type githubPRReviewCommentEvent struct {
Comment githubComment `json:"comment"`
}

// githubWorkflowRun captures the subset of GitHub's `workflow_run` event we
// route to workspaces (#101). Full schema is ~50 fields; we only need the
// handful that tell DevOps "which CI job failed, where, and how to get there."
type githubWorkflowRun struct {
ID int64 `json:"id"`
Name string `json:"name"` // workflow name, e.g. "CI"
Event string `json:"event"` // push / pull_request / etc.
Status string `json:"status"` // queued / in_progress / completed
Conclusion string `json:"conclusion"` // success / failure / cancelled / timed_out
HeadBranch string `json:"head_branch"`
HeadSHA string `json:"head_sha"`
HTMLURL string `json:"html_url"`
RunNumber int `json:"run_number"`
}

type githubWorkflowRunEvent struct {
WorkspaceID string `json:"workspace_id"`
Action string `json:"action"` // requested / in_progress / completed
Repository githubRepository `json:"repository"`
Sender githubSender `json:"sender"`
WorkflowRun githubWorkflowRun `json:"workflow_run"`
}

func buildGitHubA2APayload(eventType, deliveryID string, rawBody []byte) (string, map[string]interface{}, error) {
switch eventType {
case "issue_comment":
Expand Down Expand Up @@ -209,6 +232,50 @@ func buildGitHubA2APayload(eventType, deliveryID string, rawBody []byte) (string
"pull_request_num": payload.PullRequest.Number,
"comment_url": payload.Comment.HTMLURL,
}), nil
case "workflow_run":
// #101 — CI-break notifications for DevOps Engineer. Only surface
// *completed* runs with a non-success conclusion; queued / in_progress
// are noise. A success completion is dropped too (explicit filter
// rather than `errIgnoredGitHubAction` so the behaviour is visible
// in the switch).
var payload githubWorkflowRunEvent
if err := json.Unmarshal(rawBody, &payload); err != nil {
return "", nil, fmt.Errorf("invalid workflow_run payload: %w", err)
}
if payload.Action != "completed" {
return payload.WorkspaceID, nil, errIgnoredGitHubAction
}
if payload.WorkflowRun.Conclusion == "success" || payload.WorkflowRun.Conclusion == "skipped" || payload.WorkflowRun.Conclusion == "neutral" {
return payload.WorkspaceID, nil, errIgnoredGitHubAction
}
text := fmt.Sprintf(
"GitHub CI break — workflow '%s' run #%d %s on %s@%s\nTriggered by: %s (%s)\nRepo: %s\nRun URL: %s",
payload.WorkflowRun.Name,
payload.WorkflowRun.RunNumber,
payload.WorkflowRun.Conclusion,
payload.WorkflowRun.HeadBranch,
payload.WorkflowRun.HeadSHA[:min(7, len(payload.WorkflowRun.HeadSHA))],
payload.Sender.Login,
payload.WorkflowRun.Event,
payload.Repository.FullName,
payload.WorkflowRun.HTMLURL,
)
return payload.WorkspaceID, newGitHubMessagePayload(text, map[string]interface{}{
"source": "github",
"event": eventType,
"action": payload.Action,
"delivery_id": deliveryID,
"repository": payload.Repository.FullName,
"sender": payload.Sender.Login,
"workflow_name": payload.WorkflowRun.Name,
"run_id": payload.WorkflowRun.ID,
"run_number": payload.WorkflowRun.RunNumber,
"conclusion": payload.WorkflowRun.Conclusion,
"head_branch": payload.WorkflowRun.HeadBranch,
"head_sha": payload.WorkflowRun.HeadSHA,
"run_url": payload.WorkflowRun.HTMLURL,
"trigger_event": payload.WorkflowRun.Event,
}), nil
default:
return "", nil, errUnsupportedGitHubEvent
}
Expand Down
89 changes: 89 additions & 0 deletions platform/internal/handlers/webhooks_workflow_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package handlers

import (
"encoding/json"
"strings"
"testing"
)

// Tests the workflow_run → DevOps A2A routing added for #101.

func TestBuildGitHubA2APayload_WorkflowRunFailure(t *testing.T) {
raw := []byte(`{
"workspace_id": "ws-devops",
"action": "completed",
"repository": {"full_name": "Molecule-AI/molecule-monorepo"},
"sender": {"login": "hongming"},
"workflow_run": {
"id": 123456,
"name": "CI",
"event": "pull_request",
"status": "completed",
"conclusion": "failure",
"head_branch": "fix/thing",
"head_sha": "deadbeef1234567",
"html_url": "https://github.com/Molecule-AI/molecule-monorepo/actions/runs/123456",
"run_number": 42
}
}`)

wsID, payload, err := buildGitHubA2APayload("workflow_run", "delivery-abc", raw)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if wsID != "ws-devops" {
t.Errorf("workspace id: got %q want ws-devops", wsID)
}

body, _ := json.Marshal(payload)
text := string(body)
for _, needle := range []string{"failure", "CI", "run #42", "fix/thing", "deadbee", "Molecule-AI/molecule-monorepo"} {
if !strings.Contains(text, needle) {
t.Errorf("missing %q in payload: %s", needle, text)
}
}
}

func TestBuildGitHubA2APayload_WorkflowRunSuccessIgnored(t *testing.T) {
raw := []byte(`{
"workspace_id": "ws-devops",
"action": "completed",
"repository": {"full_name": "x/y"},
"sender": {"login": "u"},
"workflow_run": {"name": "CI", "status": "completed", "conclusion": "success", "head_sha": "abcdef1"}
}`)
_, _, err := buildGitHubA2APayload("workflow_run", "d1", raw)
if err != errIgnoredGitHubAction {
t.Errorf("success run should be ignored; got err=%v", err)
}
}

func TestBuildGitHubA2APayload_WorkflowRunNonCompletedIgnored(t *testing.T) {
raw := []byte(`{
"workspace_id": "ws-devops",
"action": "requested",
"repository": {"full_name": "x/y"},
"sender": {"login": "u"},
"workflow_run": {"name": "CI", "status": "in_progress", "conclusion": "", "head_sha": "abc"}
}`)
_, _, err := buildGitHubA2APayload("workflow_run", "d2", raw)
if err != errIgnoredGitHubAction {
t.Errorf("non-completed action should be ignored; got err=%v", err)
}
}

// Short-SHA truncation used to crash when head_sha was < 7 chars — the
// `min(7, len)` guard covers that edge case.
func TestBuildGitHubA2APayload_WorkflowRunShortSHA(t *testing.T) {
raw := []byte(`{
"workspace_id": "ws-devops",
"action": "completed",
"repository": {"full_name": "x/y"},
"sender": {"login": "u"},
"workflow_run": {"name": "CI", "status": "completed", "conclusion": "failure", "head_sha": "abc", "run_number": 1}
}`)
_, _, err := buildGitHubA2APayload("workflow_run", "d3", raw)
if err != nil {
t.Errorf("short-sha path: %v", err)
}
}
25 changes: 24 additions & 1 deletion platform/internal/middleware/ratelimit.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package middleware
import (
"context"
"net/http"
"strconv"
"sync"
"time"

Expand Down Expand Up @@ -71,11 +72,33 @@ func (rl *RateLimiter) Middleware() gin.HandlerFunc {
b.lastReset = time.Now()
}

// Issue #105 — advertise the current bucket state so clients and
// monitoring tools can back off proactively. Headers are set on every
// response (both allowed and throttled) so they're observable against
// any endpoint — /health, /metrics, and every /workspaces/* route.
//
// The `reset` value is seconds until the current bucket refills,
// matching the RFC 6585 Retry-After spec for 429 responses and the
// de-facto X-RateLimit-Reset convention (GitHub, Stripe, etc.).
remaining := b.tokens - 1
if remaining < 0 {
remaining = 0
}
resetSeconds := int(time.Until(b.lastReset.Add(rl.interval)).Seconds())
if resetSeconds < 0 {
resetSeconds = 0
}
c.Header("X-RateLimit-Limit", strconv.Itoa(rl.rate))
c.Header("X-RateLimit-Remaining", strconv.Itoa(remaining))
c.Header("X-RateLimit-Reset", strconv.Itoa(resetSeconds))

if b.tokens <= 0 {
rl.mu.Unlock()
// Retry-After is the canonical 429 signal per RFC 6585.
c.Header("Retry-After", strconv.Itoa(resetSeconds))
c.JSON(http.StatusTooManyRequests, gin.H{
"error": "rate limit exceeded",
"retry_after": rl.interval.Seconds(),
"retry_after": resetSeconds,
})
c.Abort()
return
Expand Down
72 changes: 72 additions & 0 deletions platform/internal/middleware/ratelimit_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package middleware

import (
"context"
"net/http"
"net/http/httptest"
"strconv"
"testing"
"time"

"github.com/gin-gonic/gin"
)

// newTestLimiter spins up a tiny limiter with a 2-token/5s budget so tests can
// exhaust + recover without real-time delays.
func newTestLimiter(t *testing.T) (*RateLimiter, *gin.Engine) {
t.Helper()
gin.SetMode(gin.TestMode)
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
rl := NewRateLimiter(2, 5*time.Second, ctx)
r := gin.New()
r.Use(rl.Middleware())
r.GET("/x", func(c *gin.Context) { c.String(http.StatusOK, "ok") })
return rl, r
}

// TestRateLimit_HeadersPresentOnAllowedRequest covers issue #105 — every
// response (not just 429s) must carry the X-RateLimit-* triplet so clients
// can back off proactively.
func TestRateLimit_HeadersPresentOnAllowedRequest(t *testing.T) {
_, r := newTestLimiter(t)
w := httptest.NewRecorder()
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))

if got := w.Header().Get("X-RateLimit-Limit"); got != "2" {
t.Errorf("X-RateLimit-Limit = %q, want 2", got)
}
if got := w.Header().Get("X-RateLimit-Remaining"); got != "1" {
t.Errorf("X-RateLimit-Remaining = %q, want 1", got)
}
reset, err := strconv.Atoi(w.Header().Get("X-RateLimit-Reset"))
if err != nil || reset < 0 || reset > 5 {
t.Errorf("X-RateLimit-Reset = %q, want 0-5", w.Header().Get("X-RateLimit-Reset"))
}
}

// TestRateLimit_RetryAfterOn429 — throttled responses must carry Retry-After
// per RFC 6585, so curl/fetch clients back off the exact required window.
func TestRateLimit_RetryAfterOn429(t *testing.T) {
_, r := newTestLimiter(t)
// Burn through both tokens.
for i := 0; i < 2; i++ {
w := httptest.NewRecorder()
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
if w.Code != http.StatusOK {
t.Fatalf("request %d: want 200, got %d", i+1, w.Code)
}
}
// Third should 429.
w := httptest.NewRecorder()
r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
if w.Code != http.StatusTooManyRequests {
t.Fatalf("3rd request: want 429, got %d", w.Code)
}
if got := w.Header().Get("Retry-After"); got == "" {
t.Error("missing Retry-After header on 429")
}
if got := w.Header().Get("X-RateLimit-Remaining"); got != "0" {
t.Errorf("X-RateLimit-Remaining = %q on 429, want 0", got)
}
}
Loading