diff --git a/backend/internal/httpd/api.go b/backend/internal/httpd/api.go index b17419727..f04cf20cb 100644 --- a/backend/internal/httpd/api.go +++ b/backend/internal/httpd/api.go @@ -10,16 +10,15 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/controllers" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" + prsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/pr" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" ) -// APIDeps bundles every Manager the API layer's controllers depend on. -// Controllers see only resource-level interfaces; they do not reach through to -// lifecycle reducers, adapters, or storage. A nil dependency keeps its routes -// registered but returns the OpenAPI-backed 501 response. +// APIDeps bundles every service the API layer's controllers depend on. type APIDeps struct { Projects projectsvc.Manager Sessions controllers.SessionService + PRs prsvc.ActionManager } // API owns one controller per resource and is the single Register call the @@ -28,6 +27,7 @@ type API struct { cfg config.Config projects *controllers.ProjectsController sessions *controllers.SessionsController + prs *controllers.PRsController } // NewAPI constructs the API surface from its dependencies. cfg carries the @@ -42,6 +42,7 @@ func NewAPI(cfg config.Config, deps APIDeps) *API { sessions: &controllers.SessionsController{ Svc: deps.Sessions, }, + prs: &controllers.PRsController{Svc: deps.PRs}, } } @@ -61,6 +62,7 @@ func (a *API) Register(root chi.Router) { r.Use(middleware.Timeout(timeout)) a.projects.Register(r) a.sessions.Register(r) + a.prs.Register(r) // Sibling REST controllers plug in here. }) // Surfaces that intentionally bypass the REST timeout register at this level. diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index c121ffe49..0439d89f9 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -177,6 +177,90 @@ paths: summary: Fetch one project; discriminates ok vs degraded tags: - projects + /api/v1/prs/{id}/merge: + post: + operationId: mergePR + parameters: + - description: PR number. + in: path + name: id + required: true + schema: + description: PR number. + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/MergePRResponse' + description: OK + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Found + "409": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Conflict + "422": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Unprocessable Entity + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Squash-merge a pull request + tags: + - prs + /api/v1/prs/{id}/resolve-comments: + post: + operationId: resolveComments + parameters: + - description: PR number. + in: path + name: id + required: true + schema: + description: PR number. + type: string + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/ResolveCommentsResponse' + description: OK + "404": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Found + "422": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Unprocessable Entity + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: Resolve review threads on a pull request + tags: + - prs /api/v1/sessions: get: operationId: listSessions @@ -563,6 +647,19 @@ components: required: - sessions type: object + MergePRResponse: + properties: + method: + type: string + ok: + type: boolean + prNumber: + type: integer + required: + - ok + - prNumber + - method + type: object OrchestratorResponse: properties: id: @@ -658,6 +755,16 @@ components: required: - displayName type: object + ResolveCommentsResponse: + properties: + ok: + type: boolean + resolved: + type: integer + required: + - ok + - resolved + type: object RestoreSessionResponse: properties: ok: @@ -822,3 +929,5 @@ tags: name: projects - description: Agent session lifecycle and messaging name: sessions +- description: Pull-request actions (SCM lane) + name: prs diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index f3cc8f435..406d41b81 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -59,6 +59,8 @@ func Build() ([]byte, error) { "Project registry, configuration, and lifecycle administration"), *(&openapi31.Tag{Name: "sessions"}).WithDescription( "Agent session lifecycle and messaging"), + *(&openapi31.Tag{Name: "prs"}).WithDescription( + "Pull-request actions (SCM lane)"), } for _, op := range operations() { @@ -119,8 +121,6 @@ var schemaNames = map[string]string{ "ControllersProjectResponse": "ProjectResponse", "ControllersGetProjectResponse": "ProjectGetResponse", "ControllersProjectOrDegraded": "ProjectOrDegraded", - "ControllersProjectIDParam": "ProjectIDParam", - "ControllersSessionIDParam": "SessionIDParam", "ControllersListSessionsQuery": "ListSessionsQuery", "ControllersListSessionsResponse": "ListSessionsResponse", "ControllersSpawnSessionRequest": "SpawnSessionRequest", @@ -133,6 +133,10 @@ var schemaNames = map[string]string{ "ControllersSpawnOrchestratorRequest": "SpawnOrchestratorRequest", "ControllersSpawnOrchestratorResponse": "SpawnOrchestratorResponse", "ControllersOrchestratorResponse": "OrchestratorResponse", + // httpd/controllers — PR wire envelopes + "ControllersMergePRResponse": "MergePRResponse", + "ControllersResolveCommentsRequest": "ResolveCommentsRequest", + "ControllersResolveCommentsResponse": "ResolveCommentsResponse", // service/project entities + DTOs "ProjectProject": "Project", "ProjectSummary": "ProjectSummary", @@ -216,6 +220,7 @@ type operation struct { func operations() []operation { ops := append([]operation{}, projectOperations()...) ops = append(ops, sessionOperations()...) + ops = append(ops, prOperations()...) return ops } @@ -360,3 +365,35 @@ func sessionOperations() []operation { }, } } + +// prOperations declares the PR action operations. These live in the SCM lane: +// the handler delegates to a PRService backed by the SCM provider. A nil +// PRService (SCM not configured) returns 501 for both routes. +func prOperations() []operation { + return []operation{ + { + method: http.MethodPost, path: "/api/v1/prs/{id}/merge", id: "mergePR", tag: "prs", + summary: "Squash-merge a pull request", + pathParams: []any{controllers.PRIDParam{}}, + resps: []respUnit{ + {http.StatusOK, controllers.MergePRResponse{}}, + {http.StatusNotFound, envelope.APIError{}}, + {http.StatusConflict, envelope.APIError{}}, + {http.StatusUnprocessableEntity, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, + { + method: http.MethodPost, path: "/api/v1/prs/{id}/resolve-comments", id: "resolveComments", tag: "prs", + summary: "Resolve review threads on a pull request", + pathParams: []any{controllers.PRIDParam{}}, + reqBody: nil, // body is optional: omitting it resolves all unresolved threads + resps: []respUnit{ + {http.StatusOK, controllers.ResolveCommentsResponse{}}, + {http.StatusNotFound, envelope.APIError{}}, + {http.StatusUnprocessableEntity, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, + } +} diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 22d8f42e7..8efd581cd 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -174,3 +174,26 @@ type OrchestratorResponse struct { ProjectID domain.ProjectID `json:"projectId"` ProjectName string `json:"projectName,omitempty"` } + +// PRIDParam is the {id} path parameter shared by the /prs/{id} routes. +type PRIDParam struct { + ID string `path:"id" description:"PR number."` +} + +// MergePRResponse is the body of POST /api/v1/prs/{id}/merge (200). +type MergePRResponse struct { + OK bool `json:"ok"` + PRNumber int `json:"prNumber"` + Method string `json:"method"` +} + +// ResolveCommentsRequest is the optional body of POST /api/v1/prs/{id}/resolve-comments. +type ResolveCommentsRequest struct { + CommentIDs []string `json:"commentIds,omitempty"` +} + +// ResolveCommentsResponse is the body of POST /api/v1/prs/{id}/resolve-comments (200). +type ResolveCommentsResponse struct { + OK bool `json:"ok"` + Resolved int `json:"resolved"` +} diff --git a/backend/internal/httpd/controllers/prs.go b/backend/internal/httpd/controllers/prs.go new file mode 100644 index 000000000..94a9f9c32 --- /dev/null +++ b/backend/internal/httpd/controllers/prs.go @@ -0,0 +1,83 @@ +package controllers + +import ( + "errors" + "io" + "net/http" + + "github.com/go-chi/chi/v5" + + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" + prsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/pr" +) + +// PRsController owns the /prs action routes. +type PRsController struct { + Svc prsvc.ActionManager +} + +// Register mounts the PR action routes on the supplied router. +func (c *PRsController) Register(r chi.Router) { + r.Post("/prs/{id}/merge", c.merge) + r.Post("/prs/{id}/resolve-comments", c.resolveComments) +} + +func (c *PRsController) merge(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "POST", "/api/v1/prs/{id}/merge") + return + } + prID := chi.URLParam(r, "id") + res, err := c.Svc.Merge(r.Context(), prID) + if err != nil { + writePRError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, MergePRResponse{OK: true, PRNumber: res.PRNumber, Method: res.Method}) +} + +func (c *PRsController) resolveComments(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "POST", "/api/v1/prs/{id}/resolve-comments") + return + } + prID := chi.URLParam(r, "id") + + // Body is optional: omitting it resolves all unresolved threads. + var in ResolveCommentsRequest + if err := decodeJSON(r, &in); err != nil && !isEmptyBody(err) { + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_JSON", "Invalid JSON body", nil) + return + } + + res, err := c.Svc.ResolveComments(r.Context(), prID, in.CommentIDs) + if err != nil { + writePRError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, ResolveCommentsResponse{OK: true, Resolved: res.Resolved}) +} + +// writePRError maps PR sentinel errors to their locked HTTP envelopes, +// falling back to 500 for unexpected failures. +func writePRError(w http.ResponseWriter, r *http.Request, err error) { + switch { + case errors.Is(err, prsvc.ErrPRNotFound): + envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "PR_NOT_FOUND", "Unknown PR", nil) + case errors.Is(err, prsvc.ErrPRNotMergeable): + envelope.WriteAPIError(w, r, http.StatusConflict, "conflict", "PR_NOT_MERGEABLE", "PR is not mergeable", nil) + case errors.Is(err, prsvc.ErrPRPreconditions): + envelope.WriteAPIError(w, r, http.StatusUnprocessableEntity, "unprocessable", "PR_PRECONDITIONS_UNMET", "PR merge preconditions are not met", nil) + case errors.Is(err, prsvc.ErrNothingToResolve): + envelope.WriteAPIError(w, r, http.StatusUnprocessableEntity, "unprocessable", "NOTHING_TO_RESOLVE", "No unresolved review threads to resolve", nil) + default: + envelope.WriteAPIError(w, r, http.StatusInternalServerError, "internal", "PR_OPERATION_FAILED", "PR operation failed", nil) + } +} + +// isEmptyBody reports whether err signals an absent or empty request body. +// io.ErrUnexpectedEOF means a truncated/malformed body — bad request, not absent. +func isEmptyBody(err error) bool { + return errors.Is(err, io.EOF) +} diff --git a/backend/internal/httpd/controllers/prs_test.go b/backend/internal/httpd/controllers/prs_test.go new file mode 100644 index 000000000..7d255b981 --- /dev/null +++ b/backend/internal/httpd/controllers/prs_test.go @@ -0,0 +1,159 @@ +package controllers_test + +import ( + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/aoagents/agent-orchestrator/backend/internal/config" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd" + prsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/pr" +) + +type fakePRService struct { + mergeResult prsvc.MergeResult + mergeErr error + resolveResult prsvc.ResolveResult + resolveErr error +} + +func (f *fakePRService) Merge(_ context.Context, _ string) (prsvc.MergeResult, error) { + return f.mergeResult, f.mergeErr +} + +func (f *fakePRService) ResolveComments(_ context.Context, _ string, _ []string) (prsvc.ResolveResult, error) { + return f.resolveResult, f.resolveErr +} + +func newPRTestServer(t *testing.T, svc prsvc.ActionManager) *httptest.Server { + t.Helper() + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + srv := httptest.NewServer(httpd.NewRouterWithAPI(config.Config{}, log, nil, httpd.APIDeps{PRs: svc})) + t.Cleanup(srv.Close) + return srv +} + +// ---- Nil service → 503 SCM_NOT_CONFIGURED ---- + +func TestPRsRoutes_NilService_MergeReturns501(t *testing.T) { + srv := newPRTestServer(t, nil) + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/1/merge", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusNotImplemented, "NOT_IMPLEMENTED") +} + +func TestPRsRoutes_NilService_ResolveCommentsReturns501(t *testing.T) { + srv := newPRTestServer(t, nil) + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/1/resolve-comments", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusNotImplemented, "NOT_IMPLEMENTED") +} + +// ---- Merge: 200 ---- + +func TestPRsRoutes_Merge_200(t *testing.T) { + svc := &fakePRService{mergeResult: prsvc.MergeResult{PRNumber: 42, Method: "squash"}} + srv := newPRTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/prs/42/merge", "") + if status != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", status, body) + } + var resp struct { + OK bool `json:"ok"` + PRNumber int `json:"prNumber"` + Method string `json:"method"` + } + mustJSON(t, body, &resp) + if !resp.OK || resp.PRNumber != 42 || resp.Method != "squash" { + t.Errorf("resp = %+v, want {ok:true prNumber:42 method:squash}", resp) + } +} + +// ---- Merge: 404 ---- + +func TestPRsRoutes_Merge_404(t *testing.T) { + svc := &fakePRService{mergeErr: prsvc.ErrPRNotFound} + srv := newPRTestServer(t, svc) + + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/99/merge", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusNotFound, "PR_NOT_FOUND") +} + +// ---- Merge: 409 ---- + +func TestPRsRoutes_Merge_409(t *testing.T) { + svc := &fakePRService{mergeErr: prsvc.ErrPRNotMergeable} + srv := newPRTestServer(t, svc) + + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/1/merge", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusConflict, "PR_NOT_MERGEABLE") +} + +// ---- Merge: 422 ---- + +func TestPRsRoutes_Merge_422(t *testing.T) { + svc := &fakePRService{mergeErr: prsvc.ErrPRPreconditions} + srv := newPRTestServer(t, svc) + + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/1/merge", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusUnprocessableEntity, "PR_PRECONDITIONS_UNMET") +} + +// ---- ResolveComments: 200 ---- + +func TestPRsRoutes_ResolveComments_200(t *testing.T) { + svc := &fakePRService{resolveResult: prsvc.ResolveResult{Resolved: 3}} + srv := newPRTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/prs/42/resolve-comments", `{"commentIds":["T_1","T_2","T_3"]}`) + if status != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", status, body) + } + var resp struct { + OK bool `json:"ok"` + Resolved int `json:"resolved"` + } + mustJSON(t, body, &resp) + if !resp.OK || resp.Resolved != 3 { + t.Errorf("resp = %+v, want {ok:true resolved:3}", resp) + } +} + +func TestPRsRoutes_ResolveComments_200_NoBody(t *testing.T) { + svc := &fakePRService{resolveResult: prsvc.ResolveResult{Resolved: 2}} + srv := newPRTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/prs/42/resolve-comments", "") + if status != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", status, body) + } +} + +// ---- ResolveComments: 404 ---- + +func TestPRsRoutes_ResolveComments_404(t *testing.T) { + svc := &fakePRService{resolveErr: prsvc.ErrPRNotFound} + srv := newPRTestServer(t, svc) + + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/99/resolve-comments", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusNotFound, "PR_NOT_FOUND") +} + +// ---- ResolveComments: 422 ---- + +func TestPRsRoutes_ResolveComments_422(t *testing.T) { + svc := &fakePRService{resolveErr: prsvc.ErrNothingToResolve} + srv := newPRTestServer(t, svc) + + body, status, headers := doRequest(t, srv, "POST", "/api/v1/prs/1/resolve-comments", "") + assertJSON(t, headers) + assertErrorCode(t, body, status, http.StatusUnprocessableEntity, "NOTHING_TO_RESOLVE") +} diff --git a/backend/internal/service/pr/action_service.go b/backend/internal/service/pr/action_service.go new file mode 100644 index 000000000..c79ebe954 --- /dev/null +++ b/backend/internal/service/pr/action_service.go @@ -0,0 +1,46 @@ +package pr + +import ( + "context" + "strconv" +) + +// ActionManager is the controller-facing contract for /prs/{id} action routes. +type ActionManager interface { + Merge(ctx context.Context, prID string) (MergeResult, error) + ResolveComments(ctx context.Context, prID string, commentIDs []string) (ResolveResult, error) +} + +// MergeResult is the successful outcome of a PR merge. +type MergeResult struct { + PRNumber int + Method string // always "squash" +} + +// ResolveResult is the successful outcome of a resolve-comments operation. +type ResolveResult struct { + Resolved int +} + +// ActionService implements ActionManager. Business logic is not yet implemented. +type ActionService struct{} + +var _ ActionManager = (*ActionService)(nil) + +// NewActionService returns a stub ActionService. +func NewActionService() *ActionService { + return &ActionService{} +} + +// Merge squash-merges the PR identified by prID. +// TODO: implement — squash-merge the PR via the SCM provider. +func (s *ActionService) Merge(_ context.Context, prID string) (MergeResult, error) { + n, _ := strconv.Atoi(prID) + return MergeResult{PRNumber: n, Method: "squash"}, nil +} + +// ResolveComments resolves review threads on the PR identified by prID. +// TODO: implement — resolve review threads via the SCM provider. +func (s *ActionService) ResolveComments(_ context.Context, _ string, _ []string) (ResolveResult, error) { + return ResolveResult{Resolved: 0}, nil +} diff --git a/backend/internal/service/pr/action_service_test.go b/backend/internal/service/pr/action_service_test.go new file mode 100644 index 000000000..1eb3d5f70 --- /dev/null +++ b/backend/internal/service/pr/action_service_test.go @@ -0,0 +1,28 @@ +package pr + +import ( + "context" + "testing" +) + +func TestMerge_ReturnsSquash(t *testing.T) { + svc := NewActionService() + res, err := svc.Merge(context.Background(), "42") + if err != nil { + t.Fatal(err) + } + if res.Method != "squash" { + t.Errorf("method = %q, want squash", res.Method) + } + if res.PRNumber != 42 { + t.Errorf("PRNumber = %d, want 42", res.PRNumber) + } +} + +func TestResolveComments_ReturnsOK(t *testing.T) { + svc := NewActionService() + _, err := svc.ResolveComments(context.Background(), "1", nil) + if err != nil { + t.Fatal(err) + } +} diff --git a/backend/internal/service/pr/errors.go b/backend/internal/service/pr/errors.go new file mode 100644 index 000000000..ed54504f8 --- /dev/null +++ b/backend/internal/service/pr/errors.go @@ -0,0 +1,11 @@ +package pr + +import "errors" + +// Sentinel errors returned by the PR action service. +var ( + ErrPRNotFound = errors.New("pr: not found") + ErrPRNotMergeable = errors.New("pr: not mergeable") + ErrPRPreconditions = errors.New("pr: merge preconditions unmet") + ErrNothingToResolve = errors.New("pr: nothing to resolve") +)