Skip to content

Commit af9f0ab

Browse files
starbopsclaude
andcommitted
fix: comprehensive CI fixes - formatting, race conditions, and security issues
This commit addresses all CI failures identified in PR #66: **Formatting Fixes:** - Applied go fmt to 24 files across multiple packages - Fixed code formatting issues in internal/executor/, internal/monitoring/, internal/reporting/, internal/resilience/, and tests/chaos/ **Race Condition Fixes:** - Added mutex synchronization to MockNotificationHandler in reporting tests - Implemented thread-safe getter methods for notification access - Fixed concurrent read/write access to notifications slice - Added proper synchronization to prevent race conditions during test execution **Security Improvements:** - Fixed integer overflow warnings in metrics_collector.go - Implemented safe int64 to uint64 conversion with bounds checking - Eliminated all 4 security warnings from gosec scan **Test Infrastructure:** - Enhanced MockNotificationHandler with GetNotificationCount() and GetNotification() methods - Updated test assertions to use thread-safe access methods - Improved test reliability under race detection All local validation now passes: - make lint: 0 issues - go test -race ./...: no race conditions - make security: 0 security issues - All tests passing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent e581a81 commit af9f0ab

24 files changed

Lines changed: 1726 additions & 1696 deletions

internal/executor/errors.go

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,28 @@ type ErrorType string
1515
const (
1616
// ErrorTypeInfrastructure indicates infrastructure-related failures
1717
ErrorTypeInfrastructure ErrorType = "infrastructure"
18-
18+
1919
// ErrorTypeResource indicates resource exhaustion or limits
2020
ErrorTypeResource ErrorType = "resource"
21-
21+
2222
// ErrorTypeTimeout indicates execution timeout
2323
ErrorTypeTimeout ErrorType = "timeout"
24-
24+
2525
// ErrorTypeUserCode indicates user script execution errors
2626
ErrorTypeUserCode ErrorType = "user_code"
27-
27+
2828
// ErrorTypeValidation indicates input validation errors
2929
ErrorTypeValidation ErrorType = "validation"
30-
30+
3131
// ErrorTypeSecurity indicates security policy violations
3232
ErrorTypeSecurity ErrorType = "security"
33-
33+
3434
// ErrorTypeNetwork indicates network connectivity issues
3535
ErrorTypeNetwork ErrorType = "network"
36-
36+
3737
// ErrorTypeRateLimit indicates rate limiting enforcement
3838
ErrorTypeRateLimit ErrorType = "rate_limit"
39-
39+
4040
// ErrorTypeQuota indicates quota enforcement
4141
ErrorTypeQuota ErrorType = "quota"
4242
)
@@ -331,7 +331,7 @@ func ClassifyError(err error, taskID uuid.UUID, context string) *ExecutionError
331331

332332
errMsg := err.Error()
333333
errMsgLower := strings.ToLower(errMsg)
334-
334+
335335
// Default execution error
336336
execErr := NewExecutionError(ErrorTypeInfrastructure, "UNKNOWN_ERROR", "Unknown execution error", taskID).
337337
WithCause(err).
@@ -446,14 +446,14 @@ func ClassifyError(err error, taskID uuid.UUID, context string) *ExecutionError
446446
// ClassifyDockerError specifically classifies Docker-related errors
447447
func ClassifyDockerError(err error, taskID uuid.UUID, containerID string) *ExecutionError {
448448
execErr := ClassifyError(err, taskID, "docker_operation")
449-
449+
450450
if containerID != "" {
451451
_ = execErr.WithContainerID(containerID)
452452
}
453453

454454
// Add Docker-specific context
455455
_ = execErr.WithContext("error_source", "docker_client")
456-
456+
457457
return execErr
458458
}
459459

internal/executor/errors_test.go

Lines changed: 108 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func TestExecutionError(t *testing.T) {
240240

241241
t.Run("NewExecutionError creates error with required fields", func(t *testing.T) {
242242
err := NewExecutionError(ErrorTypeTimeout, "TIMEOUT_001", "Execution timed out", taskID)
243-
243+
244244
assert.Equal(t, ErrorTypeTimeout, err.Type)
245245
assert.Equal(t, "TIMEOUT_001", err.Code)
246246
assert.Equal(t, "Execution timed out", err.Message)
@@ -252,7 +252,7 @@ func TestExecutionError(t *testing.T) {
252252

253253
t.Run("ExecutionError builder methods work correctly", func(t *testing.T) {
254254
cause := errors.New("underlying error")
255-
255+
256256
err := NewExecutionError(ErrorTypeResource, "MEM_001", "Out of memory", taskID).
257257
WithCause(cause).
258258
WithDetails("Container exceeded 512MB limit").
@@ -285,7 +285,7 @@ func TestExecutionError(t *testing.T) {
285285
t.Run("ExecutionError Unwrap works correctly", func(t *testing.T) {
286286
cause := errors.New("underlying error")
287287
err := NewExecutionError(ErrorTypeTimeout, "TIMEOUT_001", "Execution timed out", taskID).WithCause(cause)
288-
288+
289289
assert.Equal(t, cause, err.Unwrap())
290290
assert.True(t, errors.Is(err, cause))
291291
})
@@ -295,139 +295,139 @@ func TestClassifyError(t *testing.T) {
295295
taskID := uuid.New()
296296

297297
tests := []struct {
298-
name string
299-
inputError error
300-
context string
301-
expectedType ErrorType
302-
expectedCode string
303-
expectedRetry bool
298+
name string
299+
inputError error
300+
context string
301+
expectedType ErrorType
302+
expectedCode string
303+
expectedRetry bool
304304
}{
305305
{
306-
name: "Docker daemon unavailable",
307-
inputError: errors.New("Cannot connect to the Docker daemon"),
308-
context: "container_create",
309-
expectedType: ErrorTypeInfrastructure,
310-
expectedCode: "DOCKER_DAEMON_UNAVAILABLE",
311-
expectedRetry: true,
306+
name: "Docker daemon unavailable",
307+
inputError: errors.New("Cannot connect to the Docker daemon"),
308+
context: "container_create",
309+
expectedType: ErrorTypeInfrastructure,
310+
expectedCode: "DOCKER_DAEMON_UNAVAILABLE",
311+
expectedRetry: true,
312312
},
313313
{
314-
name: "Container not found",
315-
inputError: errors.New("No such container: abc123"),
316-
context: "container_stop",
317-
expectedType: ErrorTypeInfrastructure,
318-
expectedCode: "CONTAINER_NOT_FOUND",
319-
expectedRetry: false,
314+
name: "Container not found",
315+
inputError: errors.New("No such container: abc123"),
316+
context: "container_stop",
317+
expectedType: ErrorTypeInfrastructure,
318+
expectedCode: "CONTAINER_NOT_FOUND",
319+
expectedRetry: false,
320320
},
321321
{
322-
name: "Image not found",
323-
inputError: errors.New("pull access denied for python:latest"),
324-
context: "image_pull",
325-
expectedType: ErrorTypeInfrastructure,
326-
expectedCode: "IMAGE_NOT_FOUND",
327-
expectedRetry: false,
322+
name: "Image not found",
323+
inputError: errors.New("pull access denied for python:latest"),
324+
context: "image_pull",
325+
expectedType: ErrorTypeInfrastructure,
326+
expectedCode: "IMAGE_NOT_FOUND",
327+
expectedRetry: false,
328328
},
329329
{
330-
name: "Execution timeout",
331-
inputError: errors.New("context deadline exceeded"),
332-
context: "task_execution",
333-
expectedType: ErrorTypeTimeout,
334-
expectedCode: "EXECUTION_TIMEOUT",
335-
expectedRetry: true,
330+
name: "Execution timeout",
331+
inputError: errors.New("context deadline exceeded"),
332+
context: "task_execution",
333+
expectedType: ErrorTypeTimeout,
334+
expectedCode: "EXECUTION_TIMEOUT",
335+
expectedRetry: true,
336336
},
337337
{
338-
name: "Out of memory",
339-
inputError: errors.New("container killed due to OOM"),
340-
context: "task_execution",
341-
expectedType: ErrorTypeResource,
342-
expectedCode: "OUT_OF_MEMORY",
343-
expectedRetry: false,
338+
name: "Out of memory",
339+
inputError: errors.New("container killed due to OOM"),
340+
context: "task_execution",
341+
expectedType: ErrorTypeResource,
342+
expectedCode: "OUT_OF_MEMORY",
343+
expectedRetry: false,
344344
},
345345
{
346-
name: "Disk space exhausted",
347-
inputError: errors.New("no space left on device"),
348-
context: "container_create",
349-
expectedType: ErrorTypeResource,
350-
expectedCode: "OUT_OF_DISK_SPACE",
351-
expectedRetry: true,
346+
name: "Disk space exhausted",
347+
inputError: errors.New("no space left on device"),
348+
context: "container_create",
349+
expectedType: ErrorTypeResource,
350+
expectedCode: "OUT_OF_DISK_SPACE",
351+
expectedRetry: true,
352352
},
353353
{
354-
name: "CPU quota exceeded",
355-
inputError: errors.New("CPU quota exceeded for container"),
356-
context: "task_execution",
357-
expectedType: ErrorTypeResource,
358-
expectedCode: "CPU_QUOTA_EXCEEDED",
359-
expectedRetry: true,
354+
name: "CPU quota exceeded",
355+
inputError: errors.New("CPU quota exceeded for container"),
356+
context: "task_execution",
357+
expectedType: ErrorTypeResource,
358+
expectedCode: "CPU_QUOTA_EXCEEDED",
359+
expectedRetry: true,
360360
},
361361
{
362-
name: "Network error",
363-
inputError: errors.New("network connection failed"),
364-
context: "container_start",
365-
expectedType: ErrorTypeNetwork,
366-
expectedCode: "NETWORK_ERROR",
367-
expectedRetry: true,
362+
name: "Network error",
363+
inputError: errors.New("network connection failed"),
364+
context: "container_start",
365+
expectedType: ErrorTypeNetwork,
366+
expectedCode: "NETWORK_ERROR",
367+
expectedRetry: true,
368368
},
369369
{
370-
name: "Permission denied",
371-
inputError: errors.New("permission denied: cannot access file"),
372-
context: "file_access",
373-
expectedType: ErrorTypeSecurity,
374-
expectedCode: "PERMISSION_DENIED",
375-
expectedRetry: false,
370+
name: "Permission denied",
371+
inputError: errors.New("permission denied: cannot access file"),
372+
context: "file_access",
373+
expectedType: ErrorTypeSecurity,
374+
expectedCode: "PERMISSION_DENIED",
375+
expectedRetry: false,
376376
},
377377
{
378-
name: "Rate limit exceeded",
379-
inputError: errors.New("rate limit exceeded for API calls"),
380-
context: "api_call",
381-
expectedType: ErrorTypeRateLimit,
382-
expectedCode: "RATE_LIMIT_EXCEEDED",
383-
expectedRetry: true,
378+
name: "Rate limit exceeded",
379+
inputError: errors.New("rate limit exceeded for API calls"),
380+
context: "api_call",
381+
expectedType: ErrorTypeRateLimit,
382+
expectedCode: "RATE_LIMIT_EXCEEDED",
383+
expectedRetry: true,
384384
},
385385
{
386-
name: "Quota exceeded",
387-
inputError: errors.New("quota limit exceeded for user"),
388-
context: "resource_allocation",
389-
expectedType: ErrorTypeQuota,
390-
expectedCode: "QUOTA_EXCEEDED",
391-
expectedRetry: false,
386+
name: "Quota exceeded",
387+
inputError: errors.New("quota limit exceeded for user"),
388+
context: "resource_allocation",
389+
expectedType: ErrorTypeQuota,
390+
expectedCode: "QUOTA_EXCEEDED",
391+
expectedRetry: false,
392392
},
393393
{
394-
name: "Validation error",
395-
inputError: errors.New("invalid input format provided"),
396-
context: "input_validation",
397-
expectedType: ErrorTypeValidation,
398-
expectedCode: "VALIDATION_ERROR",
399-
expectedRetry: false,
394+
name: "Validation error",
395+
inputError: errors.New("invalid input format provided"),
396+
context: "input_validation",
397+
expectedType: ErrorTypeValidation,
398+
expectedCode: "VALIDATION_ERROR",
399+
expectedRetry: false,
400400
},
401401
{
402-
name: "User code error",
403-
inputError: errors.New("command failed with exit status 1"),
404-
context: "script_execution",
405-
expectedType: ErrorTypeUserCode,
406-
expectedCode: "USER_CODE_ERROR",
407-
expectedRetry: false,
402+
name: "User code error",
403+
inputError: errors.New("command failed with exit status 1"),
404+
context: "script_execution",
405+
expectedType: ErrorTypeUserCode,
406+
expectedCode: "USER_CODE_ERROR",
407+
expectedRetry: false,
408408
},
409409
{
410-
name: "Cancellation error",
411-
inputError: errors.New("operation was cancelled by user"),
412-
context: "task_execution",
413-
expectedType: ErrorTypeTimeout,
414-
expectedCode: "EXECUTION_CANCELLED",
415-
expectedRetry: false,
410+
name: "Cancellation error",
411+
inputError: errors.New("operation was cancelled by user"),
412+
context: "task_execution",
413+
expectedType: ErrorTypeTimeout,
414+
expectedCode: "EXECUTION_CANCELLED",
415+
expectedRetry: false,
416416
},
417417
{
418-
name: "Unknown error",
419-
inputError: errors.New("something completely unexpected happened"),
420-
context: "unknown_operation",
421-
expectedType: ErrorTypeInfrastructure,
422-
expectedCode: "UNKNOWN_ERROR",
423-
expectedRetry: true,
418+
name: "Unknown error",
419+
inputError: errors.New("something completely unexpected happened"),
420+
context: "unknown_operation",
421+
expectedType: ErrorTypeInfrastructure,
422+
expectedCode: "UNKNOWN_ERROR",
423+
expectedRetry: true,
424424
},
425425
}
426426

427427
for _, tt := range tests {
428428
t.Run(tt.name, func(t *testing.T) {
429429
execErr := ClassifyError(tt.inputError, taskID, tt.context)
430-
430+
431431
require.NotNil(t, execErr)
432432
assert.Equal(t, tt.expectedType, execErr.Type)
433433
assert.Equal(t, tt.expectedCode, execErr.Code)
@@ -446,7 +446,7 @@ func TestClassifyError(t *testing.T) {
446446
t.Run("Already classified ExecutionError returns as-is", func(t *testing.T) {
447447
original := NewExecutionError(ErrorTypeValidation, "TEST_001", "Test error", taskID)
448448
result := ClassifyError(original, taskID, "test")
449-
449+
450450
assert.Equal(t, original, result)
451451
})
452452
}
@@ -458,7 +458,7 @@ func TestClassifyDockerError(t *testing.T) {
458458
t.Run("Classifies Docker error with container ID", func(t *testing.T) {
459459
err := errors.New("Docker daemon connection refused")
460460
execErr := ClassifyDockerError(err, taskID, containerID)
461-
461+
462462
require.NotNil(t, execErr)
463463
assert.Equal(t, ErrorTypeInfrastructure, execErr.Type)
464464
assert.Equal(t, "DOCKER_DAEMON_UNAVAILABLE", execErr.Code)
@@ -470,7 +470,7 @@ func TestClassifyDockerError(t *testing.T) {
470470
t.Run("Handles empty container ID", func(t *testing.T) {
471471
err := errors.New("No such container")
472472
execErr := ClassifyDockerError(err, taskID, "")
473-
473+
474474
require.NotNil(t, execErr)
475475
assert.Equal(t, ErrorTypeInfrastructure, execErr.Type)
476476
assert.Equal(t, "CONTAINER_NOT_FOUND", execErr.Code)
@@ -484,7 +484,7 @@ func TestIsRetryableError(t *testing.T) {
484484
t.Run("ExecutionError retryable flag is respected", func(t *testing.T) {
485485
retryableErr := NewExecutionError(ErrorTypeTimeout, "TIMEOUT_001", "Timeout", taskID).SetRetryable(true)
486486
nonRetryableErr := NewExecutionError(ErrorTypeUserCode, "USER_001", "User error", taskID).SetRetryable(false)
487-
487+
488488
assert.True(t, IsRetryableError(retryableErr))
489489
assert.False(t, IsRetryableError(nonRetryableErr))
490490
})
@@ -509,13 +509,13 @@ func TestGetErrorType(t *testing.T) {
509509
assert.Equal(t, ErrorTypeTimeout, GetErrorType(ErrExecutionTimeout))
510510
assert.Equal(t, ErrorTypeInfrastructure, GetErrorType(ErrDockerUnavailable))
511511
assert.Equal(t, ErrorTypeResource, GetErrorType(ErrResourceExhausted))
512-
512+
513513
secErr := NewSecurityError("test", "test", nil)
514514
assert.Equal(t, ErrorTypeSecurity, GetErrorType(secErr))
515-
515+
516516
confErr := ErrInvalidConfig("test")
517517
assert.Equal(t, ErrorTypeValidation, GetErrorType(confErr))
518-
518+
519519
unknownErr := errors.New("unknown")
520520
assert.Equal(t, ErrorTypeInfrastructure, GetErrorType(unknownErr))
521521
})
@@ -532,7 +532,7 @@ func TestIsExecutionError(t *testing.T) {
532532
t.Run("Rejects non-ExecutionError types", func(t *testing.T) {
533533
assert.False(t, IsExecutionError(ErrExecutionTimeout))
534534
assert.False(t, IsExecutionError(errors.New("standard error")))
535-
535+
536536
containerErr := NewContainerError("container123", "test", "test", nil)
537537
assert.False(t, IsExecutionError(containerErr))
538538
})

0 commit comments

Comments
 (0)