Skip to content
Merged
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
2 changes: 1 addition & 1 deletion .github/workflows/cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:

steps:
- name: Checkout code
uses: actions/checkout@v5
uses: actions/checkout@v6
with:
fetch-depth: 0

Expand Down
68 changes: 61 additions & 7 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,81 @@ on:
pull_request:
branches: [ master ]

permissions:
contents: read

jobs:
# Job id must stay build-test-coverage: it is a required status check on the repo.
build-test-coverage:
runs-on: ubuntu-latest

steps:
- name: Checkout code
uses: actions/checkout@v5
uses: actions/checkout@v6
Comment thread
coderabbitai[bot] marked this conversation as resolved.
with:
persist-credentials: false

- name: Set up Go
uses: actions/setup-go@v5
uses: actions/setup-go@v6
with:
go-version-file: go.mod

- name: Build application
run: make build

- name: Vet
run: make vet

- name: Run tests
run: make test

- name: Run tests with coverage
run: make test-coverage
build-image:
# Verify images build on PRs; push happens in the CD workflow on master.
name: Build Image (${{ matrix.name }})
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
include:
- name: cli-mcp-server
image: codeready-toolchain/cli-mcp-server
containerfile: Containerfile.server
- name: cli-mcp-sandbox
image: codeready-toolchain/cli-mcp-sandbox
containerfile: Containerfile.agent

steps:
- name: Checkout code
uses: actions/checkout@v6
with:
persist-credentials: false

- name: Build image
uses: redhat-actions/buildah-build@v2
with:
image: ${{ matrix.image }}
tags: latest
containerfiles: |
${{ matrix.containerfile }}
platforms: linux/amd64
build-args: |
GIT_COMMIT=${{ github.sha }}
BUILD_TIME=ci
Comment thread
coderabbitai[bot] marked this conversation as resolved.

lint:
name: Lint
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v6
with:
persist-credentials: false

- name: Set up Go
uses: actions/setup-go@v6
with:
go-version-file: go.mod

- name: Lint
uses: golangci/golangci-lint-action@v9
with:
# Match sibling MCP repos; bump deliberately when lint breaks.
version: latest
args: --config=./.golangci.yml --verbose
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

# Output of the go coverage tool, specifically when used with LiteIDE
*.out
coverage.html

# Dependency directories
# vendor/
Expand Down
67 changes: 67 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
version: "2"
linters:
enable:
- asciicheck
- bidichk
- bodyclose
- durationcheck
- errchkjson
- errorlint
- exhaustive
- gocheckcompilerdirectives
- gochecksumtype
- gocyclo
- gosec
- gosmopolitan
- loggercheck
- makezero
- misspell
- musttag
- nilerr
- nilnesserr
- noctx
- nolintlint
- protogetter
- reassign
- recvcheck
- revive
- rowserrcheck
- spancheck
- sqlclosecheck
- testifylint
- unparam
- zerologlint
disable:
- asasalint
- prealloc
settings:
exhaustive:
default-signifies-exhaustive: true
govet:
enable-all: true
disable:
- fieldalignment
nolintlint:
require-explanation: false
require-specific: true
allow-unused: false
exclusions:
generated: lax
presets:
- comments
- common-false-positives
- legacy
- std-error-handling
paths:
- third_party$
- builtin$
- examples$
formatters:
enable:
- gofmt
exclusions:
generated: lax
paths:
- third_party$
- builtin$
- examples$
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ vet:
# Test with coverage
test-coverage:
$(GOTEST) -v -coverprofile=coverage.out ./...
$(GOCMD) tool cover -html=coverage.out
$(GOCMD) tool cover -html=coverage.out -o coverage.html

# Clean build artifacts
clean:
Expand Down
8 changes: 6 additions & 2 deletions cmd/agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ func main() {
mux.HandleFunc("GET /health", handler.HandleHealth)

srv := &http.Server{
Addr: ":8090",
Handler: mux,
Addr: ":8090",
Handler: mux,
ReadHeaderTimeout: 10 * time.Second,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ReadTimeout: 10 * time.Second,
IdleTimeout: 60 * time.Second,
// WriteTimeout intentionally unset: /exec may run up to MaxTimeout (300s).
}

go func() {
Expand Down
8 changes: 5 additions & 3 deletions pkg/agent/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ const (
// Communication uses plain HTTP within the Kubernetes pod network trust boundary:
// NetworkPolicy restricts ingress to sandbox pods, and bearer tokens authenticate
// callers. TLS can be added later as a localized change if needed.
//
//nolint:revive // name matches design docs
type AgentClient struct {
httpClient *http.Client
port int
Expand Down Expand Up @@ -101,7 +103,7 @@ func (c *AgentClient) Execute(ctx context.Context, podIP, token string, req Exec
if err != nil {
return nil, classifyDoError(err, op, url)
}
defer resp.Body.Close() //nolint:errcheck // body close errors are not actionable
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return nil, &StatusError{
Expand Down Expand Up @@ -166,7 +168,7 @@ func (c *AgentClient) Assign(ctx context.Context, podIP string, req AssignReques
if err != nil {
return classifyDoError(err, op, url)
}
defer resp.Body.Close() //nolint:errcheck
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return &StatusError{
Expand Down Expand Up @@ -194,7 +196,7 @@ func (c *AgentClient) HealthCheck(ctx context.Context, podIP string) error {
if err != nil {
return classifyDoError(err, op, url)
}
defer resp.Body.Close() //nolint:errcheck
defer resp.Body.Close()

if resp.StatusCode != http.StatusOK {
return &StatusError{
Expand Down
34 changes: 21 additions & 13 deletions pkg/agent/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ func TestExecute(t *testing.T) {
gotPath = r.URL.Path
gotAuth = r.Header.Get("Authorization")
gotCT = r.Header.Get("Content-Type")
require.NoError(t, json.NewDecoder(r.Body).Decode(&gotReq))
if decodeErr := json.NewDecoder(r.Body).Decode(&gotReq); decodeErr != nil {
t.Errorf("decode request: %v", decodeErr)
return
}
w.Header().Set("Content-Type", "application/json")
require.NoError(t, json.NewEncoder(w).Encode(expected))
if encodeErr := json.NewEncoder(w).Encode(expected); encodeErr != nil {
t.Errorf("encode response: %v", encodeErr)
}
}))
t.Cleanup(ts.Close)
client, host := newTestClient(t, ts)
Expand Down Expand Up @@ -121,7 +126,7 @@ func TestExecute(t *testing.T) {
assert.Equal(t, "execute", decodeErr.Op)
assert.Equal(t, http.StatusOK, decodeErr.StatusCode)
assert.Equal(t, "not-json", decodeErr.Body)
assert.NotNil(t, decodeErr.Err)
assert.Error(t, decodeErr.Err)
})

t.Run("connection refused returns NetworkError", func(t *testing.T) {
Expand All @@ -137,13 +142,13 @@ func TestExecute(t *testing.T) {
require.ErrorAs(t, err, &netErr)
assert.Equal(t, "execute", netErr.Op)
assert.Contains(t, netErr.URL, "127.0.0.1:1/exec")
assert.NotNil(t, netErr.Err)
assert.Error(t, netErr.Err)
})

t.Run("context deadline returns NetworkError wrapping DeadlineExceeded", func(t *testing.T) {
// given
release := make(chan struct{})
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
ts := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
<-release
}))
t.Cleanup(func() {
Expand All @@ -162,14 +167,14 @@ func TestExecute(t *testing.T) {
var netErr *NetworkError
require.ErrorAs(t, err, &netErr)
assert.Equal(t, "execute", netErr.Op)
assert.True(t, errors.Is(err, context.DeadlineExceeded))
assert.ErrorIs(t, err, context.DeadlineExceeded)
})

t.Run("context cancellation returns NetworkError wrapping Canceled", func(t *testing.T) {
// given
started := make(chan struct{})
release := make(chan struct{})
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
ts := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
close(started)
<-release
}))
Expand All @@ -194,7 +199,7 @@ func TestExecute(t *testing.T) {
// then
var netErr *NetworkError
require.ErrorAs(t, err, &netErr)
assert.True(t, errors.Is(err, context.Canceled))
assert.ErrorIs(t, err, context.Canceled)
})

t.Run("response exceeding max size returns DecodeError", func(t *testing.T) {
Expand Down Expand Up @@ -228,7 +233,10 @@ func TestAssign(t *testing.T) {
gotPath = r.URL.Path
gotAuth = r.Header.Get("Authorization")
gotCT = r.Header.Get("Content-Type")
require.NoError(t, json.NewDecoder(r.Body).Decode(&gotReq))
if decodeErr := json.NewDecoder(r.Body).Decode(&gotReq); decodeErr != nil {
t.Errorf("decode request: %v", decodeErr)
return
}
w.WriteHeader(http.StatusOK)
}))
t.Cleanup(ts.Close)
Expand Down Expand Up @@ -343,7 +351,7 @@ func TestNewAgentClientOptions(t *testing.T) {
assert.Equal(t, 30*time.Second, c.httpClient.Timeout)
assert.Equal(t, 8090, c.port)
assert.Equal(t, DefaultMaxResponseSize, c.maxResponseSize)
assert.Equal(t, int64(10*1024*1024), DefaultMaxResponseSize)
assert.Equal(t, DefaultMaxResponseSize, int64(10*1024*1024))
})

t.Run("WithTimeout overrides timeout", func(t *testing.T) {
Expand Down Expand Up @@ -417,7 +425,7 @@ func TestErrorTypes(t *testing.T) {
err := &NetworkError{Op: "execute", URL: "http://x/exec", Err: context.DeadlineExceeded}

// then
assert.True(t, errors.Is(err, context.DeadlineExceeded))
require.ErrorIs(t, err, context.DeadlineExceeded)
assert.Equal(t, "execute http://x/exec: context deadline exceeded", err.Error())
})

Expand All @@ -427,7 +435,7 @@ func TestErrorTypes(t *testing.T) {
err := &DecodeError{Op: "execute", URL: "http://x/exec", StatusCode: 200, Err: inner, Body: "x"}

// then
assert.True(t, errors.Is(err, inner))
require.ErrorIs(t, err, inner)
assert.Equal(t, "execute http://x/exec: invalid character", err.Error())
})

Expand All @@ -436,7 +444,7 @@ func TestErrorTypes(t *testing.T) {
err := &StatusError{Op: "assign", URL: "http://x/assign", StatusCode: 409, Body: "conflict"}

// then
assert.Nil(t, errors.Unwrap(err))
require.NoError(t, errors.Unwrap(err))
assert.Equal(t, "assign http://x/assign: unexpected status 409", err.Error())
})

Expand Down
9 changes: 6 additions & 3 deletions pkg/sandbox/bash.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package sandbox

import (
"bufio"
"context"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -56,7 +57,7 @@ type BashSession struct {
}

// NewBashSession spawns a persistent bash process.
func NewBashSession(cfg BashConfig) (*BashSession, error) {
func NewBashSession(_ BashConfig) (*BashSession, error) {
bs := &BashSession{}
if err := bs.spawn(); err != nil {
return nil, fmt.Errorf("failed to start bash: %w", err)
Expand All @@ -65,7 +66,7 @@ func NewBashSession(cfg BashConfig) (*BashSession, error) {
}

func (bs *BashSession) spawn() error {
cmd := exec.Command("bash", "--norc", "--noprofile")
cmd := exec.CommandContext(context.Background(), "bash", "--norc", "--noprofile")
cmd.Env = append(os.Environ(), "PS1=", "PS2=")
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}

Expand Down Expand Up @@ -102,7 +103,7 @@ func (bs *BashSession) reapProcess() {
return
}
if bs.stdin != nil {
bs.stdin.Close() //nolint:errcheck
_ = bs.stdin.Close()
}
_ = syscall.Kill(-bs.cmd.Process.Pid, syscall.SIGKILL)
bs.cmd.Wait() //nolint:errcheck
Expand All @@ -128,6 +129,8 @@ func (bs *BashSession) Close() error {

// Execute runs a command in the persistent bash session with the given timeout.
// If the bash process has crashed, it respawns automatically and reports the reset in stderr.
//
//nolint:gocyclo // delimiter protocol state machine
func (bs *BashSession) Execute(command string, timeout time.Duration) (*ExecResult, error) {
bs.mu.Lock()
defer bs.mu.Unlock()
Expand Down
Loading
Loading