Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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.
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.
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.
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