fix: address code review feedback

- Fix GetRaw() error handling to check status before reading body
- Add context.Context support to all HTTP requests
- Use consistent capitalized error messages
- Update copyright year from 2024 to 2025
- Add unit tests for modules/api/client.go
- Add tests for runs, jobs, logs commands

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

cmd/actions/jobs.go

@@ -1,4 +1,4 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package actions
@@ -33,12 +33,12 @@ func RunActionJobs(ctx stdctx.Context, cmd *cli.Command) error {
 	c.Ensure(context.CtxRequirement{RemoteRepo: true})
 
 	if cmd.Args().Len() < 1 {
-		return fmt.Errorf("run ID is required")
+		return fmt.Errorf("Run ID is required")
 	}
 
 	runID, err := utils.ArgToIndex(cmd.Args().First())
 	if err != nil {
-		return fmt.Errorf("invalid run ID: %w", err)
+		return fmt.Errorf("Invalid run ID: %w", err)
 	}
 
 	client := api.NewClient(c.Login)
@@ -47,7 +47,7 @@ func RunActionJobs(ctx stdctx.Context, cmd *cli.Command) error {
 		c.Owner, c.Repo, runID)
 
 	var jobs api.ActionJobList
-	if _, err := client.Get(path, &jobs); err != nil {
+	if _, err := client.Get(ctx, path, &jobs); err != nil {
 		return err
 	}
 

cmd/actions/jobs_test.go

@@ -0,0 +1,62 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package actions
+
+import (
+	"testing"
+)
+
+func TestJobsCommandFlags(t *testing.T) {
+	cmd := CmdActionsJobs
+
+	// Test that required flags exist
+	expectedFlags := []string{"output", "remote", "login", "repo"}
+
+	for _, flagName := range expectedFlags {
+		found := false
+		for _, flag := range cmd.Flags {
+			for _, name := range flag.Names() {
+				if name == flagName {
+					found = true
+					break
+				}
+			}
+			if found {
+				break
+			}
+		}
+
+		if !found {
+			t.Errorf("Expected flag %s not found in CmdActionsJobs", flagName)
+		}
+	}
+}
+
+func TestJobsCommandProperties(t *testing.T) {
+	cmd := CmdActionsJobs
+
+	if cmd.Name != "jobs" {
+		t.Errorf("Expected command name 'jobs', got %s", cmd.Name)
+	}
+
+	if len(cmd.Aliases) == 0 || cmd.Aliases[0] != "job" {
+		t.Errorf("Expected alias 'job' for jobs command")
+	}
+
+	if cmd.Usage == "" {
+		t.Error("Jobs command should have usage text")
+	}
+
+	if cmd.Description == "" {
+		t.Error("Jobs command should have description")
+	}
+
+	if cmd.ArgsUsage != "<run-id>" {
+		t.Errorf("Expected ArgsUsage '<run-id>', got %s", cmd.ArgsUsage)
+	}
+
+	if cmd.Action == nil {
+		t.Error("Jobs command should have an action")
+	}
+}

cmd/actions/logs.go

@@ -1,4 +1,4 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package actions
@@ -32,12 +32,12 @@ func RunActionLogs(ctx stdctx.Context, cmd *cli.Command) error {
 	c.Ensure(context.CtxRequirement{RemoteRepo: true})
 
 	if cmd.Args().Len() < 1 {
-		return fmt.Errorf("job ID is required")
+		return fmt.Errorf("Job ID is required")
 	}
 
 	jobID, err := utils.ArgToIndex(cmd.Args().First())
 	if err != nil {
-		return fmt.Errorf("invalid job ID: %w", err)
+		return fmt.Errorf("Invalid job ID: %w", err)
 	}
 
 	client := api.NewClient(c.Login)
@@ -45,7 +45,7 @@ func RunActionLogs(ctx stdctx.Context, cmd *cli.Command) error {
 	path := fmt.Sprintf("/repos/%s/%s/actions/jobs/%d/logs",
 		c.Owner, c.Repo, jobID)
 
-	logs, err := client.GetRaw(path)
+	logs, err := client.GetRaw(ctx, path)
 	if err != nil {
 		return err
 	}

cmd/actions/logs_test.go

@@ -0,0 +1,62 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package actions
+
+import (
+	"testing"
+)
+
+func TestLogsCommandFlags(t *testing.T) {
+	cmd := CmdActionsLogs
+
+	// Test that required flags exist
+	expectedFlags := []string{"output", "remote", "login", "repo"}
+
+	for _, flagName := range expectedFlags {
+		found := false
+		for _, flag := range cmd.Flags {
+			for _, name := range flag.Names() {
+				if name == flagName {
+					found = true
+					break
+				}
+			}
+			if found {
+				break
+			}
+		}
+
+		if !found {
+			t.Errorf("Expected flag %s not found in CmdActionsLogs", flagName)
+		}
+	}
+}
+
+func TestLogsCommandProperties(t *testing.T) {
+	cmd := CmdActionsLogs
+
+	if cmd.Name != "logs" {
+		t.Errorf("Expected command name 'logs', got %s", cmd.Name)
+	}
+
+	if len(cmd.Aliases) == 0 || cmd.Aliases[0] != "log" {
+		t.Errorf("Expected alias 'log' for logs command")
+	}
+
+	if cmd.Usage == "" {
+		t.Error("Logs command should have usage text")
+	}
+
+	if cmd.Description == "" {
+		t.Error("Logs command should have description")
+	}
+
+	if cmd.ArgsUsage != "<job-id>" {
+		t.Errorf("Expected ArgsUsage '<job-id>', got %s", cmd.ArgsUsage)
+	}
+
+	if cmd.Action == nil {
+		t.Error("Logs command should have an action")
+	}
+}

cmd/actions/runs.go

@@ -1,4 +1,4 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package actions
@@ -41,7 +41,7 @@ func RunActionRuns(ctx stdctx.Context, cmd *cli.Command) error {
 		flags.GetListOptions().PageSize)
 
 	var runs api.ActionRunList
-	if _, err := client.Get(path, &runs); err != nil {
+	if _, err := client.Get(ctx, path, &runs); err != nil {
 		return err
 	}
 

cmd/actions/runs_test.go

@@ -0,0 +1,58 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package actions
+
+import (
+	"testing"
+)
+
+func TestRunsCommandFlags(t *testing.T) {
+	cmd := CmdActionsRuns
+
+	// Test that required flags exist
+	expectedFlags := []string{"page", "limit", "output", "remote", "login", "repo"}
+
+	for _, flagName := range expectedFlags {
+		found := false
+		for _, flag := range cmd.Flags {
+			for _, name := range flag.Names() {
+				if name == flagName {
+					found = true
+					break
+				}
+			}
+			if found {
+				break
+			}
+		}
+
+		if !found {
+			t.Errorf("Expected flag %s not found in CmdActionsRuns", flagName)
+		}
+	}
+}
+
+func TestRunsCommandProperties(t *testing.T) {
+	cmd := CmdActionsRuns
+
+	if cmd.Name != "runs" {
+		t.Errorf("Expected command name 'runs', got %s", cmd.Name)
+	}
+
+	if len(cmd.Aliases) == 0 || cmd.Aliases[0] != "run" {
+		t.Errorf("Expected alias 'run' for runs command")
+	}
+
+	if cmd.Usage == "" {
+		t.Error("Runs command should have usage text")
+	}
+
+	if cmd.Description == "" {
+		t.Error("Runs command should have description")
+	}
+
+	if cmd.Action == nil {
+		t.Error("Runs command should have an action")
+	}
+}

modules/api/client.go

@@ -1,9 +1,10 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package api
 
 import (
+	"context"
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
@@ -35,13 +36,13 @@ func NewClient(login *config.Login) *Client {
 	}
 }
 
-// Get makes an authenticated GET request to the API
-func (c *Client) Get(path string, result interface{}) (*http.Response, error) {
+// Get makes an authenticated GET request to the API and decodes the JSON response
+func (c *Client) Get(ctx context.Context, path string, result interface{}) (*http.Response, error) {
 	url := fmt.Sprintf("%s/api/v1%s", c.login.URL, path)
 
-	req, err := http.NewRequest("GET", url, nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
 
 	req.Header.Set("Authorization", "token "+c.login.Token)
@@ -49,7 +50,7 @@ func (c *Client) Get(path string, result interface{}) (*http.Response, error) {
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to execute request: %w", err)
 	}
 	defer resp.Body.Close()
 
@@ -68,29 +69,30 @@ func (c *Client) Get(path string, result interface{}) (*http.Response, error) {
 }
 
 // GetRaw makes an authenticated GET request and returns the raw response body
-func (c *Client) GetRaw(path string) ([]byte, error) {
+func (c *Client) GetRaw(ctx context.Context, path string) ([]byte, error) {
 	url := fmt.Sprintf("%s/api/v1%s", c.login.URL, path)
 
-	req, err := http.NewRequest("GET", url, nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to create request: %w", err)
 	}
 
 	req.Header.Set("Authorization", "token "+c.login.Token)
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to execute request: %w", err)
 	}
 	defer resp.Body.Close()
 
-	body, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
+	if resp.StatusCode >= 400 {
+		body, _ := io.ReadAll(resp.Body)
+		return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body))
 	}
 
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("API request failed with status %d: %s", resp.StatusCode, string(body))
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read response: %w", err)
 	}
 
 	return body, nil

modules/api/client_test.go

@@ -0,0 +1,203 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"code.gitea.io/tea/modules/config"
+)
+
+func TestNewClient(t *testing.T) {
+	login := &config.Login{
+		URL:      "https://gitea.example.com",
+		Token:    "test-token",
+		Insecure: false,
+	}
+
+	client := NewClient(login)
+	if client == nil {
+		t.Fatal("NewClient returned nil")
+	}
+
+	if client.login != login {
+		t.Error("Client login not set correctly")
+	}
+
+	if client.httpClient == nil {
+		t.Error("Client httpClient not set")
+	}
+}
+
+func TestNewClientInsecure(t *testing.T) {
+	login := &config.Login{
+		URL:      "https://gitea.example.com",
+		Token:    "test-token",
+		Insecure: true,
+	}
+
+	client := NewClient(login)
+	if client == nil {
+		t.Fatal("NewClient returned nil")
+	}
+
+	// Verify that insecure transport is configured
+	if client.httpClient.Transport == nil {
+		t.Error("Expected custom transport for insecure client")
+	}
+}
+
+func TestGet(t *testing.T) {
+	// Create a test server
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Verify request
+		if r.Method != "GET" {
+			t.Errorf("Expected GET request, got %s", r.Method)
+		}
+
+		if r.Header.Get("Authorization") != "token test-token" {
+			t.Errorf("Expected authorization header, got %s", r.Header.Get("Authorization"))
+		}
+
+		if r.Header.Get("Accept") != "application/json" {
+			t.Errorf("Expected accept header, got %s", r.Header.Get("Accept"))
+		}
+
+		// Return test response
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(map[string]string{"status": "ok"})
+	}))
+	defer server.Close()
+
+	login := &config.Login{
+		URL:   server.URL,
+		Token: "test-token",
+	}
+
+	client := NewClient(login)
+
+	var result map[string]string
+	_, err := client.Get(context.Background(), "/test", &result)
+	if err != nil {
+		t.Fatalf("Get returned error: %v", err)
+	}
+
+	if result["status"] != "ok" {
+		t.Errorf("Expected status 'ok', got %s", result["status"])
+	}
+}
+
+func TestGetError(t *testing.T) {
+	// Create a test server that returns an error
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+		w.Write([]byte("not found"))
+	}))
+	defer server.Close()
+
+	login := &config.Login{
+		URL:   server.URL,
+		Token: "test-token",
+	}
+
+	client := NewClient(login)
+
+	var result map[string]string
+	_, err := client.Get(context.Background(), "/test", &result)
+	if err == nil {
+		t.Fatal("Expected error for 404 response")
+	}
+
+	expectedError := "API request failed with status 404"
+	if err.Error()[:len(expectedError)] != expectedError {
+		t.Errorf("Expected error starting with '%s', got '%s'", expectedError, err.Error())
+	}
+}
+
+func TestGetRaw(t *testing.T) {
+	expectedBody := "raw log content here"
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != "GET" {
+			t.Errorf("Expected GET request, got %s", r.Method)
+		}
+
+		if r.Header.Get("Authorization") != "token test-token" {
+			t.Errorf("Expected authorization header, got %s", r.Header.Get("Authorization"))
+		}
+
+		w.Write([]byte(expectedBody))
+	}))
+	defer server.Close()
+
+	login := &config.Login{
+		URL:   server.URL,
+		Token: "test-token",
+	}
+
+	client := NewClient(login)
+
+	body, err := client.GetRaw(context.Background(), "/logs")
+	if err != nil {
+		t.Fatalf("GetRaw returned error: %v", err)
+	}
+
+	if string(body) != expectedBody {
+		t.Errorf("Expected body '%s', got '%s'", expectedBody, string(body))
+	}
+}
+
+func TestGetRawError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write([]byte("internal error"))
+	}))
+	defer server.Close()
+
+	login := &config.Login{
+		URL:   server.URL,
+		Token: "test-token",
+	}
+
+	client := NewClient(login)
+
+	_, err := client.GetRaw(context.Background(), "/logs")
+	if err == nil {
+		t.Fatal("Expected error for 500 response")
+	}
+
+	expectedError := "API request failed with status 500"
+	if err.Error()[:len(expectedError)] != expectedError {
+		t.Errorf("Expected error starting with '%s', got '%s'", expectedError, err.Error())
+	}
+}
+
+func TestGetWithContextCancellation(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// This should not be reached if context is cancelled
+		w.Write([]byte("ok"))
+	}))
+	defer server.Close()
+
+	login := &config.Login{
+		URL:   server.URL,
+		Token: "test-token",
+	}
+
+	client := NewClient(login)
+
+	// Create a cancelled context
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	var result map[string]string
+	_, err := client.Get(ctx, "/test", &result)
+	if err == nil {
+		t.Fatal("Expected error for cancelled context")
+	}
+}

modules/api/types.go

@@ -1,4 +1,4 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2025 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package api