From ff9e71a5c66537d9c5d7f20495de83f646d501d9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 21:32:14 +0000 Subject: [PATCH 1/2] Initial plan From c826db94819b761664dfbc870a2e359f94854375 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 3 Mar 2026 21:41:42 +0000 Subject: [PATCH 2/2] fix: write HTTP response to _http_response_writer when remote plugin step stops pipeline with response_status Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> --- plugin/external/remote_step.go | 46 ++++++++++- plugin/external/remote_step_test.go | 117 ++++++++++++++++++++++++++++ 2 files changed, 162 insertions(+), 1 deletion(-) diff --git a/plugin/external/remote_step.go b/plugin/external/remote_step.go index 6f977233..1a06742e 100644 --- a/plugin/external/remote_step.go +++ b/plugin/external/remote_step.go @@ -3,6 +3,8 @@ package external import ( "context" "fmt" + "io" + "net/http" "github.com/GoCodeAlone/workflow/module" pb "github.com/GoCodeAlone/workflow/plugin/external/proto" @@ -70,8 +72,50 @@ func (s *RemoteStep) Execute(ctx context.Context, pc *module.PipelineContext) (* return nil, fmt.Errorf("remote step execute: %s", resp.Error) } + output := structToMap(resp.Output) + + // When the plugin signals a pipeline stop with an HTTP response encoded in + // the output (response_status, response_body, response_headers), write that + // response to _http_response_writer so the caller sees the correct status + // code rather than a default 200/202. This mirrors the pattern used by + // step.auth_validate for 401 responses. + if resp.StopPipeline { + if w, ok := pc.Metadata["_http_response_writer"].(http.ResponseWriter); ok { + if statusRaw, hasStatus := output["response_status"]; hasStatus { + var statusCode int + switch v := statusRaw.(type) { + case float64: + statusCode = int(v) + case int: + statusCode = v + } + if statusCode > 0 { + if headersRaw, ok := output["response_headers"]; ok { + if headers, ok := headersRaw.(map[string]any); ok { + for k, v := range headers { + if vs, ok := v.(string); ok { + w.Header().Set(k, vs) + } + } + } + } + if w.Header().Get("Content-Type") == "" { + w.Header().Set("Content-Type", "application/json") + } + w.WriteHeader(statusCode) + if bodyRaw, ok := output["response_body"]; ok { + if bodyStr, ok := bodyRaw.(string); ok { + _, _ = io.WriteString(w, bodyStr) + } + } + pc.Metadata["_response_handled"] = true + } + } + } + } + return &module.StepResult{ - Output: structToMap(resp.Output), + Output: output, Stop: resp.StopPipeline, }, nil } diff --git a/plugin/external/remote_step_test.go b/plugin/external/remote_step_test.go index d3e17197..9514a3b3 100644 --- a/plugin/external/remote_step_test.go +++ b/plugin/external/remote_step_test.go @@ -2,6 +2,8 @@ package external import ( "context" + "net/http" + "net/http/httptest" "testing" "github.com/GoCodeAlone/workflow/module" @@ -158,3 +160,118 @@ func TestRemoteStep_Execute_StaticConfigPassthrough(t *testing.T) { t.Errorf("expected timeout=30, got %v", sent["timeout"]) } } + +// TestRemoteStep_Execute_WritesHTTPResponseOnStopPipeline verifies that when a +// remote plugin returns StopPipeline=true with response_status, response_body, +// and response_headers in the output, the RemoteStep writes those to the +// _http_response_writer and sets _response_handled=true in pipeline metadata. +func TestRemoteStep_Execute_WritesHTTPResponseOnStopPipeline(t *testing.T) { + output, _ := structpb.NewStruct(map[string]any{ + "response_status": float64(http.StatusForbidden), + "response_body": `{"error":"forbidden: insufficient permissions"}`, + "response_headers": map[string]any{ + "Content-Type": "application/json", + }, + }) + stub := &stubPluginServiceClient{ + response: &pb.ExecuteStepResponse{ + Output: output, + StopPipeline: true, + }, + } + step := NewRemoteStep("authz-step", "handle-authz", stub, nil) + + recorder := httptest.NewRecorder() + pc := module.NewPipelineContext(nil, nil) + pc.Metadata["_http_response_writer"] = http.ResponseWriter(recorder) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !result.Stop { + t.Error("expected Stop=true") + } + + if recorder.Code != http.StatusForbidden { + t.Errorf("expected HTTP 403, got %d", recorder.Code) + } + + body := recorder.Body.String() + if body != `{"error":"forbidden: insufficient permissions"}` { + t.Errorf("unexpected body: %q", body) + } + + if pc.Metadata["_response_handled"] != true { + t.Error("expected _response_handled=true in metadata") + } +} + +// TestRemoteStep_Execute_NoHTTPWriterOnStopPipeline verifies that when there is +// no _http_response_writer in the pipeline metadata, the step still returns the +// correct StepResult without panicking. +func TestRemoteStep_Execute_NoHTTPWriterOnStopPipeline(t *testing.T) { + output, _ := structpb.NewStruct(map[string]any{ + "response_status": float64(http.StatusForbidden), + "response_body": `{"error":"forbidden"}`, + }) + stub := &stubPluginServiceClient{ + response: &pb.ExecuteStepResponse{ + Output: output, + StopPipeline: true, + }, + } + step := NewRemoteStep("authz-step", "handle-authz2", stub, nil) + + pc := module.NewPipelineContext(nil, nil) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.Stop { + t.Error("expected Stop=true") + } + if pc.Metadata["_response_handled"] == true { + t.Error("expected _response_handled to remain unset when no writer present") + } +} + +// TestRemoteStep_Execute_StopPipelineWithoutResponseStatus verifies that a +// StopPipeline=true result without response_status does NOT write to the +// response writer and does NOT set _response_handled. +func TestRemoteStep_Execute_StopPipelineWithoutResponseStatus(t *testing.T) { + output, _ := structpb.NewStruct(map[string]any{ + "some_output": "value", + }) + stub := &stubPluginServiceClient{ + response: &pb.ExecuteStepResponse{ + Output: output, + StopPipeline: true, + }, + } + step := NewRemoteStep("authz-step", "handle-authz3", stub, nil) + + recorder := httptest.NewRecorder() + pc := module.NewPipelineContext(nil, nil) + pc.Metadata["_http_response_writer"] = http.ResponseWriter(recorder) + + result, err := step.Execute(context.Background(), pc) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !result.Stop { + t.Error("expected Stop=true") + } + // No response_status in output → should not write anything + if recorder.Code != http.StatusOK { // recorder default is 200 until WriteHeader is called + t.Errorf("expected recorder to be untouched (200), got %d", recorder.Code) + } + if recorder.Body.Len() != 0 { + t.Errorf("expected empty body, got %q", recorder.Body.String()) + } + if pc.Metadata["_response_handled"] == true { + t.Error("expected _response_handled to remain unset") + } +}