diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 64bad0d0a3..c51c723dbc 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -39,9 +39,6 @@ jobs: - name: Build run: go build -v ./cmd/kandev - - name: Run go vet - run: go vet ./... - - name: Run golangci-lint uses: golangci/golangci-lint-action@v9 with: @@ -51,14 +48,14 @@ jobs: - name: Run tests with coverage run: | - go test -v -race -coverprofile=coverage.out -covermode=atomic -json ./... > test-results.json 2>&1 || true - go test -v -race -coverprofile=coverage.out -covermode=atomic ./... 2>&1 | tee test-output.txt + set -o pipefail + go test -v -race -coverprofile=coverage.out -covermode=atomic -json ./... 2>&1 | tee test-results.json - name: Generate test report uses: robherley/go-test-action@v0 if: always() with: - moduleDirectory: apps/backend + fromJSONFile: apps/backend/test-results.json - name: Upload coverage to Codecov uses: codecov/codecov-action@v4 @@ -76,5 +73,5 @@ jobs: name: test-results path: | apps/backend/coverage.out - apps/backend/test-output.txt + apps/backend/test-results.json retention-days: 30 diff --git a/apps/backend/internal/agentctl/server/process/interactive_runner.go b/apps/backend/internal/agentctl/server/process/interactive_runner.go index 79d7654fa3..f51646271d 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_runner.go +++ b/apps/backend/internal/agentctl/server/process/interactive_runner.go @@ -102,6 +102,7 @@ type interactiveProcess struct { // Lifecycle stopOnce sync.Once stopSignal chan struct{} + waitDone chan struct{} // closed when wait() returns (cmd.Wait completed) mu sync.Mutex } @@ -254,6 +255,7 @@ func (r *InteractiveRunner) Start(ctx context.Context, req InteractiveStartReque idleTimeout: idleTimeout, lastState: StateUnknown, stopSignal: make(chan struct{}), + waitDone: make(chan struct{}), // Store start parameters for deferred initialization started: false, startCmd: req.Command, @@ -473,19 +475,14 @@ func (r *InteractiveRunner) Stop(ctx context.Context, processID string) error { if proc.cmd != nil && proc.cmd.Process != nil { _ = proc.cmd.Process.Signal(syscall.SIGTERM) - // Wait for graceful exit, then force kill - done := make(chan struct{}) - go func() { - _ = proc.cmd.Wait() - close(done) - }() - + // Wait for the wait() goroutine to finish (it calls cmd.Wait). + // If it doesn't exit in time, force-kill the process. select { case <-ctx.Done(): _ = proc.cmd.Process.Kill() case <-time.After(2 * time.Second): _ = proc.cmd.Process.Kill() - case <-done: + case <-proc.waitDone: // Process exited cleanly } } @@ -837,6 +834,7 @@ func (r *InteractiveRunner) handleStateChange(proc *interactiveProcess, state Ag // 2. Stuck processes should be terminated via Stop() which sends SIGTERM/SIGKILL // 3. Adding a timeout here would leave the process unreachable and create leaks func (r *InteractiveRunner) wait(proc *interactiveProcess) { + defer close(proc.waitDone) err := proc.cmd.Wait() exitCode := 0 status := types.ProcessStatusExited diff --git a/apps/backend/internal/orchestrator/executor/executor_test.go b/apps/backend/internal/orchestrator/executor/executor_test.go index 8aa336529a..4dbc0ee33c 100644 --- a/apps/backend/internal/orchestrator/executor/executor_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_test.go @@ -2,6 +2,7 @@ package executor import ( "context" + "sync/atomic" "testing" "time" @@ -748,11 +749,11 @@ func TestLaunchPreparedSession_ExistingWorkspace_StartAgent(t *testing.T) { } repo.sessions[session.ID] = session - startAgentCalled := false + var startAgentCalled atomic.Bool descriptionSet := "" agentManager := &mockAgentManager{ startAgentProcessFunc: func(ctx context.Context, id string) error { - startAgentCalled = true + startAgentCalled.Store(true) if id != "exec-existing" { t.Errorf("Expected execution ID exec-existing, got %s", id) } @@ -794,7 +795,7 @@ func TestLaunchPreparedSession_ExistingWorkspace_StartAgent(t *testing.T) { // Wait for async goroutine time.Sleep(100 * time.Millisecond) - if !startAgentCalled { + if !startAgentCalled.Load() { t.Error("Expected StartAgentProcess to be called") } } diff --git a/apps/backend/pkg/claudecode/client_test.go b/apps/backend/pkg/claudecode/client_test.go index d10ce5fbff..6f8436250e 100644 --- a/apps/backend/pkg/claudecode/client_test.go +++ b/apps/backend/pkg/claudecode/client_test.go @@ -13,6 +13,31 @@ import ( "github.com/kandev/kandev/internal/common/logger" ) +// syncBuffer is a thread-safe bytes.Buffer for tests where a goroutine writes +// and the test goroutine reads. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (sb *syncBuffer) Write(p []byte) (int, error) { + sb.mu.Lock() + defer sb.mu.Unlock() + return sb.buf.Write(p) +} + +func (sb *syncBuffer) Len() int { + sb.mu.Lock() + defer sb.mu.Unlock() + return sb.buf.Len() +} + +func (sb *syncBuffer) Bytes() []byte { + sb.mu.Lock() + defer sb.mu.Unlock() + return append([]byte(nil), sb.buf.Bytes()...) +} + func newTestLogger() *logger.Logger { log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json", OutputPath: "stdout"}) return log @@ -194,8 +219,8 @@ func TestClient_NoHandlerAutoReject(t *testing.T) { // Create a control request message input := `{"type":"control_request","request_id":"req123","request":{"subtype":"can_use_tool","tool_name":"Bash"}}` + "\n" - var buf bytes.Buffer - client := NewClient(&buf, strings.NewReader(input), newTestLogger()) + buf := &syncBuffer{} + client := NewClient(buf, strings.NewReader(input), newTestLogger()) // No request handler set - should auto-reject