From 5f327935508053fd83ad5dfd6165bed79dd1242a Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Wed, 17 Jun 2026 14:11:38 +0530 Subject: [PATCH 1/3] test: add handler unit tests for sandbox agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cover auth enforcement: missing header, wrong scheme, wrong token - Cover assign state machine: unassigned→assigned, 409 on repeat - Cover health endpoint: 200 alive, 503 dead - Integration test for warm-pool flow (assign then exec) SANDBOX-1809 Co-authored-by: Cursor Signed-off-by: Feny Mehta --- pkg/sandbox/handler_test.go | 353 ++++++++++++++++++++++++++++++++++++ 1 file changed, 353 insertions(+) create mode 100644 pkg/sandbox/handler_test.go diff --git a/pkg/sandbox/handler_test.go b/pkg/sandbox/handler_test.go new file mode 100644 index 0000000..1983e34 --- /dev/null +++ b/pkg/sandbox/handler_test.go @@ -0,0 +1,353 @@ +package sandbox + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/codeready-toolchain/cli-mcp-server/pkg/agent" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func newTestHandler(t *testing.T, token string) *Handler { + t.Helper() + bs := newTestSession(t) + state := NewAgentState(token) + return NewHandler(bs, state) +} + +func TestHandleHealth(t *testing.T) { + t.Run("returns 200 when bash is alive", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + req := httptest.NewRequest(http.MethodGet, "/health", nil) + rec := httptest.NewRecorder() + + // when + h.HandleHealth(rec, req) + + // then + assert.Equal(t, http.StatusOK, rec.Code) + var body map[string]string + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + assert.Equal(t, "ok", body["status"]) + }) + + t.Run("returns 503 when bash is dead", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + require.NoError(t, h.session.Close()) + req := httptest.NewRequest(http.MethodGet, "/health", nil) + rec := httptest.NewRecorder() + + // when + h.HandleHealth(rec, req) + + // then + assert.Equal(t, http.StatusServiceUnavailable, rec.Code) + var body map[string]string + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &body)) + assert.Equal(t, "unhealthy", body["status"]) + }) +} + +func TestHandleExec(t *testing.T) { + t.Run("rejects when agent not assigned", func(t *testing.T) { + // given + h := newTestHandler(t, "") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer anything") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusServiceUnavailable, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "agent not assigned", resp.Error) + }) + + t.Run("rejects missing authorization header", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusUnauthorized, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "missing or invalid authorization header", resp.Error) + }) + + t.Run("rejects malformed authorization header", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Basic dXNlcjpwYXNz") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusUnauthorized, rec.Code) + }) + + t.Run("rejects wrong token", func(t *testing.T) { + // given + h := newTestHandler(t, "correct-secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer wrong-secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusUnauthorized, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "unauthorized", resp.Error) + }) + + t.Run("rejects invalid request body", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader([]byte("not json"))) + req.Header.Set("Authorization", "Bearer secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusBadRequest, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "invalid request body", resp.Error) + }) + + t.Run("rejects empty command", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: ""}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusBadRequest, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "command is required", resp.Error) + }) + + t.Run("executes command with correct token", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo hello_handler"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusOK, rec.Code) + var resp agent.ExecResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "hello_handler", resp.Stdout) + assert.Equal(t, 0, resp.ExitCode) + assert.GreaterOrEqual(t, resp.DurationMs, int64(0)) + }) + + t.Run("returns non-zero exit code from failing command", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "(exit 42)"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusOK, rec.Code) + var resp agent.ExecResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, 42, resp.ExitCode) + }) + + t.Run("captures stderr output", func(t *testing.T) { + // given + h := newTestHandler(t, "secret") + body, _ := json.Marshal(agent.ExecRequest{Command: "echo oops >&2"}) + req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req.Header.Set("Authorization", "Bearer secret") + rec := httptest.NewRecorder() + + // when + h.HandleExec(rec, req) + + // then + assert.Equal(t, http.StatusOK, rec.Code) + var resp agent.ExecResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "oops", resp.Stderr) + }) +} + +func TestHandleAssign(t *testing.T) { + t.Run("assigns token to unassigned agent", func(t *testing.T) { + // given + h := newTestHandler(t, "") + body, _ := json.Marshal(agent.AssignRequest{Token: "new-token"}) + req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + // when + h.HandleAssign(rec, req) + + // then + assert.Equal(t, http.StatusOK, rec.Code) + assert.True(t, h.state.IsAssigned()) + assert.Equal(t, "new-token", h.state.GetToken()) + }) + + t.Run("returns 409 when already assigned", func(t *testing.T) { + // given + h := newTestHandler(t, "existing-token") + body, _ := json.Marshal(agent.AssignRequest{Token: "another-token"}) + req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + // when + h.HandleAssign(rec, req) + + // then + assert.Equal(t, http.StatusConflict, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "already assigned", resp.Error) + }) + + t.Run("returns 409 on second assign attempt", func(t *testing.T) { + // given + h := newTestHandler(t, "") + body1, _ := json.Marshal(agent.AssignRequest{Token: "first-token"}) + req1 := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body1)) + rec1 := httptest.NewRecorder() + h.HandleAssign(rec1, req1) + require.Equal(t, http.StatusOK, rec1.Code) + + body2, _ := json.Marshal(agent.AssignRequest{Token: "second-token"}) + req2 := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body2)) + rec2 := httptest.NewRecorder() + + // when + h.HandleAssign(rec2, req2) + + // then + assert.Equal(t, http.StatusConflict, rec2.Code) + assert.Equal(t, "first-token", h.state.GetToken()) + }) + + t.Run("rejects invalid request body", func(t *testing.T) { + // given + h := newTestHandler(t, "") + req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader([]byte("bad"))) + rec := httptest.NewRecorder() + + // when + h.HandleAssign(rec, req) + + // then + assert.Equal(t, http.StatusBadRequest, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "invalid request body", resp.Error) + }) + + t.Run("rejects empty token", func(t *testing.T) { + // given + h := newTestHandler(t, "") + body, _ := json.Marshal(agent.AssignRequest{Token: ""}) + req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + rec := httptest.NewRecorder() + + // when + h.HandleAssign(rec, req) + + // then + assert.Equal(t, http.StatusBadRequest, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "token is required", resp.Error) + }) +} + +func TestAssignThenExec(t *testing.T) { + t.Run("exec works after assign", func(t *testing.T) { + // given + h := newTestHandler(t, "") + assignBody, _ := json.Marshal(agent.AssignRequest{Token: "dynamic-token"}) + assignReq := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignRec := httptest.NewRecorder() + h.HandleAssign(assignRec, assignReq) + require.Equal(t, http.StatusOK, assignRec.Code) + + execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo from_warm_pool"}) + execReq := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq.Header.Set("Authorization", "Bearer dynamic-token") + execRec := httptest.NewRecorder() + + // when + h.HandleExec(execRec, execReq) + + // then + assert.Equal(t, http.StatusOK, execRec.Code) + var resp agent.ExecResponse + require.NoError(t, json.Unmarshal(execRec.Body.Bytes(), &resp)) + assert.Equal(t, "from_warm_pool", resp.Stdout) + assert.Equal(t, 0, resp.ExitCode) + }) + + t.Run("exec rejected with wrong token after assign", func(t *testing.T) { + // given + h := newTestHandler(t, "") + assignBody, _ := json.Marshal(agent.AssignRequest{Token: "correct-token"}) + assignReq := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignRec := httptest.NewRecorder() + h.HandleAssign(assignRec, assignReq) + require.Equal(t, http.StatusOK, assignRec.Code) + + execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo should_fail"}) + execReq := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq.Header.Set("Authorization", "Bearer wrong-token") + execRec := httptest.NewRecorder() + + // when + h.HandleExec(execRec, execReq) + + // then + assert.Equal(t, http.StatusUnauthorized, execRec.Code) + }) +} From 9e8c165497633f44d4ca7709e317625beabb67d7 Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Thu, 18 Jun 2026 15:41:00 +0530 Subject: [PATCH 2/3] fix: use httptest.NewRequestWithContext to satisfy noctx lint rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address CodeRabbit review comment — replace httptest.NewRequest with httptest.NewRequestWithContext(context.Background(), ...) across all test request constructions, consistent with the noctx linter enabled in other codeready-toolchain repos. Co-authored-by: Cursor --- pkg/sandbox/handler_test.go | 43 +++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/pkg/sandbox/handler_test.go b/pkg/sandbox/handler_test.go index 1983e34..799b394 100644 --- a/pkg/sandbox/handler_test.go +++ b/pkg/sandbox/handler_test.go @@ -2,6 +2,7 @@ package sandbox import ( "bytes" + "context" "encoding/json" "net/http" "net/http/httptest" @@ -23,7 +24,7 @@ func TestHandleHealth(t *testing.T) { t.Run("returns 200 when bash is alive", func(t *testing.T) { // given h := newTestHandler(t, "secret") - req := httptest.NewRequest(http.MethodGet, "/health", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/health", nil) rec := httptest.NewRecorder() // when @@ -40,7 +41,7 @@ func TestHandleHealth(t *testing.T) { // given h := newTestHandler(t, "secret") require.NoError(t, h.session.Close()) - req := httptest.NewRequest(http.MethodGet, "/health", nil) + req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/health", nil) rec := httptest.NewRecorder() // when @@ -59,7 +60,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer anything") rec := httptest.NewRecorder() @@ -77,7 +78,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -94,7 +95,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Basic dXNlcjpwYXNz") rec := httptest.NewRecorder() @@ -109,7 +110,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "correct-secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer wrong-secret") rec := httptest.NewRecorder() @@ -126,7 +127,7 @@ func TestHandleExec(t *testing.T) { t.Run("rejects invalid request body", func(t *testing.T) { // given h := newTestHandler(t, "secret") - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader([]byte("not json"))) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader([]byte("not json"))) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -144,7 +145,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: ""}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -162,7 +163,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hello_handler"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -182,7 +183,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "(exit 42)"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -200,7 +201,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo oops >&2"}) - req := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -220,7 +221,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.AssignRequest{Token: "new-token"}) - req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -236,7 +237,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "existing-token") body, _ := json.Marshal(agent.AssignRequest{Token: "another-token"}) - req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -253,13 +254,13 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body1, _ := json.Marshal(agent.AssignRequest{Token: "first-token"}) - req1 := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body1)) + req1 := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body1)) rec1 := httptest.NewRecorder() h.HandleAssign(rec1, req1) require.Equal(t, http.StatusOK, rec1.Code) body2, _ := json.Marshal(agent.AssignRequest{Token: "second-token"}) - req2 := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body2)) + req2 := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body2)) rec2 := httptest.NewRecorder() // when @@ -273,7 +274,7 @@ func TestHandleAssign(t *testing.T) { t.Run("rejects invalid request body", func(t *testing.T) { // given h := newTestHandler(t, "") - req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader([]byte("bad"))) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader([]byte("bad"))) rec := httptest.NewRecorder() // when @@ -290,7 +291,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.AssignRequest{Token: ""}) - req := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -309,13 +310,13 @@ func TestAssignThenExec(t *testing.T) { // given h := newTestHandler(t, "") assignBody, _ := json.Marshal(agent.AssignRequest{Token: "dynamic-token"}) - assignReq := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(assignBody)) assignRec := httptest.NewRecorder() h.HandleAssign(assignRec, assignReq) require.Equal(t, http.StatusOK, assignRec.Code) execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo from_warm_pool"}) - execReq := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(execBody)) execReq.Header.Set("Authorization", "Bearer dynamic-token") execRec := httptest.NewRecorder() @@ -334,13 +335,13 @@ func TestAssignThenExec(t *testing.T) { // given h := newTestHandler(t, "") assignBody, _ := json.Marshal(agent.AssignRequest{Token: "correct-token"}) - assignReq := httptest.NewRequest(http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(assignBody)) assignRec := httptest.NewRecorder() h.HandleAssign(assignRec, assignReq) require.Equal(t, http.StatusOK, assignRec.Code) execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo should_fail"}) - execReq := httptest.NewRequest(http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(execBody)) execReq.Header.Set("Authorization", "Bearer wrong-token") execRec := httptest.NewRecorder() From 6e955a2743bfe005b6586029fcd62a18726994ab Mon Sep 17 00:00:00 2001 From: Feny Mehta Date: Mon, 22 Jun 2026 12:19:31 +0530 Subject: [PATCH 3/3] fix: address review comments on handler tests - Add testContext(t) helper using context.WithTimeout(t.Context(), 5s) instead of context.Background() to prevent tests from hanging indefinitely (rajivnathan review) - Add error body assertion to "rejects malformed authorization header" test for consistency with other auth failure subtests (rajivnathan review) - Rename "executes command with correct token" to "executes command and returns stdout" for clarity Co-authored-by: Cursor --- pkg/sandbox/handler_test.go | 55 ++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/pkg/sandbox/handler_test.go b/pkg/sandbox/handler_test.go index 799b394..9163914 100644 --- a/pkg/sandbox/handler_test.go +++ b/pkg/sandbox/handler_test.go @@ -7,12 +7,20 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/codeready-toolchain/cli-mcp-server/pkg/agent" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func testContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + t.Cleanup(cancel) + return ctx +} + func newTestHandler(t *testing.T, token string) *Handler { t.Helper() bs := newTestSession(t) @@ -24,7 +32,7 @@ func TestHandleHealth(t *testing.T) { t.Run("returns 200 when bash is alive", func(t *testing.T) { // given h := newTestHandler(t, "secret") - req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/health", nil) + req := httptest.NewRequestWithContext(testContext(t), http.MethodGet, "/health", nil) rec := httptest.NewRecorder() // when @@ -41,7 +49,7 @@ func TestHandleHealth(t *testing.T) { // given h := newTestHandler(t, "secret") require.NoError(t, h.session.Close()) - req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/health", nil) + req := httptest.NewRequestWithContext(testContext(t), http.MethodGet, "/health", nil) rec := httptest.NewRecorder() // when @@ -60,7 +68,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer anything") rec := httptest.NewRecorder() @@ -78,7 +86,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -95,7 +103,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Basic dXNlcjpwYXNz") rec := httptest.NewRecorder() @@ -104,13 +112,16 @@ func TestHandleExec(t *testing.T) { // then assert.Equal(t, http.StatusUnauthorized, rec.Code) + var resp ErrorResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "missing or invalid authorization header", resp.Error) }) t.Run("rejects wrong token", func(t *testing.T) { // given h := newTestHandler(t, "correct-secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hi"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer wrong-secret") rec := httptest.NewRecorder() @@ -127,7 +138,7 @@ func TestHandleExec(t *testing.T) { t.Run("rejects invalid request body", func(t *testing.T) { // given h := newTestHandler(t, "secret") - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader([]byte("not json"))) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader([]byte("not json"))) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -145,7 +156,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: ""}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -159,11 +170,11 @@ func TestHandleExec(t *testing.T) { assert.Equal(t, "command is required", resp.Error) }) - t.Run("executes command with correct token", func(t *testing.T) { + t.Run("executes command and returns stdout", func(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo hello_handler"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -183,7 +194,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "(exit 42)"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -201,7 +212,7 @@ func TestHandleExec(t *testing.T) { // given h := newTestHandler(t, "secret") body, _ := json.Marshal(agent.ExecRequest{Command: "echo oops >&2"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(body)) req.Header.Set("Authorization", "Bearer secret") rec := httptest.NewRecorder() @@ -221,7 +232,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.AssignRequest{Token: "new-token"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -237,7 +248,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "existing-token") body, _ := json.Marshal(agent.AssignRequest{Token: "another-token"}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -254,13 +265,13 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body1, _ := json.Marshal(agent.AssignRequest{Token: "first-token"}) - req1 := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body1)) + req1 := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(body1)) rec1 := httptest.NewRecorder() h.HandleAssign(rec1, req1) require.Equal(t, http.StatusOK, rec1.Code) body2, _ := json.Marshal(agent.AssignRequest{Token: "second-token"}) - req2 := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body2)) + req2 := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(body2)) rec2 := httptest.NewRecorder() // when @@ -274,7 +285,7 @@ func TestHandleAssign(t *testing.T) { t.Run("rejects invalid request body", func(t *testing.T) { // given h := newTestHandler(t, "") - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader([]byte("bad"))) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader([]byte("bad"))) rec := httptest.NewRecorder() // when @@ -291,7 +302,7 @@ func TestHandleAssign(t *testing.T) { // given h := newTestHandler(t, "") body, _ := json.Marshal(agent.AssignRequest{Token: ""}) - req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(body)) + req := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(body)) rec := httptest.NewRecorder() // when @@ -310,13 +321,13 @@ func TestAssignThenExec(t *testing.T) { // given h := newTestHandler(t, "") assignBody, _ := json.Marshal(agent.AssignRequest{Token: "dynamic-token"}) - assignReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignReq := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(assignBody)) assignRec := httptest.NewRecorder() h.HandleAssign(assignRec, assignReq) require.Equal(t, http.StatusOK, assignRec.Code) execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo from_warm_pool"}) - execReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(execBody)) execReq.Header.Set("Authorization", "Bearer dynamic-token") execRec := httptest.NewRecorder() @@ -335,13 +346,13 @@ func TestAssignThenExec(t *testing.T) { // given h := newTestHandler(t, "") assignBody, _ := json.Marshal(agent.AssignRequest{Token: "correct-token"}) - assignReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/assign", bytes.NewReader(assignBody)) + assignReq := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/assign", bytes.NewReader(assignBody)) assignRec := httptest.NewRecorder() h.HandleAssign(assignRec, assignReq) require.Equal(t, http.StatusOK, assignRec.Code) execBody, _ := json.Marshal(agent.ExecRequest{Command: "echo should_fail"}) - execReq := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/exec", bytes.NewReader(execBody)) + execReq := httptest.NewRequestWithContext(testContext(t), http.MethodPost, "/exec", bytes.NewReader(execBody)) execReq.Header.Set("Authorization", "Bearer wrong-token") execRec := httptest.NewRecorder()