modified: internal/agent/agent.go

	modified:   internal/agent/agent_test.go
	modified:   internal/agent/analysis_executor.go
	modified:   internal/agent/analysis_executor_test.go
	modified:   internal/agent/cognitive_bus_test.go
	new file:   internal/agent/common_test.go
	modified:   internal/agent/executors.go
	modified:   internal/agent/executors_test.go
	modified:   internal/agent/llm_mind.go
	modified:   internal/agent/llm_mind_test.go
	modified:   internal/analysis/active/taint/taint_analyzer.go
	modified:   internal/analysis/active/timeslip/e2e_test.go
	modified:   internal/analysis/active/timeslip/timeslip_analyzer.go
	modified:   internal/browser/network/customhttp/client.go
	modified:   internal/browser/network/customhttp/client_integration_test.go
	modified:   internal/browser/network/customhttp/client_test.go
	modified:   internal/browser/network/customhttp/h1client.go
	modified:   internal/browser/network/customhttp/h2client.go
	modified:   internal/browser/network/customhttp/h2client_test.go
	modified:   internal/browser/session/harvester.go
	modified:   internal/browser/session/harvester_test.go
	modified:   internal/browser/stealth/evasions.js
	modified:   internal/browser/stealth/evasions.test.js
	modified:   internal/browser/stealth/stealth.go
	modified:   internal/browser/stealth/stealth_test.go

Author: xkilldash9x

internal/agent/agent.go

@@ -132,6 +132,7 @@ func New(ctx context.Context, mission Mission, globalCtx *core.GlobalContext, se
 		// If initialization fails (e.g., cannot determine project root), log the error
 		// but allow the agent to continue without evolution capabilities.
 		logger.Error("Failed to initialize Evolution system (ImprovementAnalyst). Proceeding without it.", zap.Error(err))
+		evoAnalyst = nil // FIX: Explicitly set to nil for consistency and clarity.
 	}
 
 	agent := &Agent{
@@ -254,6 +255,8 @@ func (a *Agent) RunMission(ctx context.Context) (*MissionResult, error) {
 	}
 }
 
+// actionLoop is the primary consumer of actions posted to the CognitiveBus.
+// It dispatches actions to the appropriate handlers (executors or internal methods).
 func (a *Agent) actionLoop(ctx context.Context, actionChan <-chan CognitiveMessage) {
 	defer a.wg.Done()
 
@@ -264,73 +267,108 @@ func (a *Agent) actionLoop(ctx context.Context, actionChan <-chan CognitiveMessa
 				return
 			}
 
-			action, ok := msg.Payload.(Action)
-			if !ok {
-				a.logger.Error("Received invalid payload for ACTION message", zap.Any("payload", msg.Payload))
-				a.bus.Acknowledge(msg)
-				continue
+			// FIX: Refactor processing into a separate function to handle panic recovery and acknowledgment robustly.
+			// Process the message and check if we should stop the loop (e.g., after CONCLUDE).
+			if stop := a.processActionMessage(ctx, msg); stop {
+				return
 			}
 
-			var execResult *ExecutionResult
-			var execErr error
-
-			switch action.Type {
-			case ActionConclude:
-				a.logger.Info("Mind decided to conclude mission.", zap.String("rationale", action.Rationale))
-				result, err := a.concludeMission(ctx)
-				if err != nil {
-					a.logger.Error("Failed to generate final mission result", zap.Error(err))
-					a.bus.Acknowledge(msg)
-					continue
-				}
-				if result != nil {
-					// CRITICAL: Acknowledge BEFORE calling finish().
-					// finish() calls bus.Shutdown(), which waits for this acknowledgment.
-					a.bus.Acknowledge(msg)
-					a.finish(ctx, *result)
-				}
-				return // End the action loop.
-
-			case ActionEvolveCodebase:
-				a.logger.Info("Agent decided to initiate self-improvement (Evolution).", zap.String("rationale", action.Rationale))
-				execResult = a.executeEvolution(ctx, action)
+		case <-ctx.Done():
+			return
+		}
+	}
+}
 
+// processActionMessage handles a single action message, including panic recovery and acknowledgment.
+// It returns true if the action loop should stop (e.g., after ActionConclude), false otherwise.
+func (a *Agent) processActionMessage(ctx context.Context, msg CognitiveMessage) (stopLoop bool) {
+	// CRITICAL FIX: Ensure the message is always acknowledged, even if processing panics.
+	// This prevents deadlocks during CognitiveBus.Shutdown().
+	acknowledged := false
+	defer func() {
+		if r := recover(); r != nil {
+			a.logger.Error("Panic recovered in actionLoop processing. Acknowledging message to prevent deadlock.",
+				zap.Any("panic_value", r),
+				zap.String("message_id", msg.ID),
+				zap.Stack("stack"),
+			)
+			// Ensure acknowledgment happens if it hasn't already.
+			if !acknowledged {
+				a.bus.Acknowledge(msg)
 			}
+		}
+	}()
 
-			// If execResult is not yet set, it means the action should be handled by the ExecutorRegistry.
-			if execResult == nil {
-				a.logger.Debug("Dispatching action to ExecutorRegistry", zap.String("type", string(action.Type)))
-				execResult, execErr = a.executors.Execute(ctx, action)
-			}
+	// --- Start of message processing ---
 
-			// Centralized error and nil-result handling.
-			if execErr != nil {
-				a.logger.Error("Action execution failed with a raw error", zap.String("action_type", string(action.Type)), zap.Error(execErr))
-				execResult = &ExecutionResult{
-					Status:          "failed",
-					ObservationType: ObservedSystemState,
-					ErrorCode:       ErrCodeExecutionFailure,
-					ErrorDetails:    map[string]interface{}{"message": execErr.Error()},
-				}
-			} else if execResult == nil {
-				// This is a safeguard against a logic error where an action handler returns (nil, nil).
-				a.logger.Error("CRITICAL: Action handler returned nil result and nil error.", zap.String("action_type", string(action.Type)))
-				// Create a fallback result to prevent nil pointer in postObservation
-				execResult = &ExecutionResult{
-					Status:          "failed",
-					ObservationType: ObservedSystemState,
-					ErrorCode:       ErrCodeExecutionFailure,
-					ErrorDetails:    map[string]interface{}{"message": "Internal Error: Action handler returned nil result."},
-				}
-			}
+	action, ok := msg.Payload.(Action)
+	if !ok {
+		a.logger.Error("Received invalid payload for ACTION message", zap.Any("payload", msg.Payload))
+		a.bus.Acknowledge(msg)
+		acknowledged = true
+		return false // Continue loop
+	}
 
-			a.postObservation(ctx, action, execResult)
+	var execResult *ExecutionResult
+	var execErr error
+
+	switch action.Type {
+	case ActionConclude:
+		a.logger.Info("Mind decided to conclude mission.", zap.String("rationale", action.Rationale))
+		result, err := a.concludeMission(ctx)
+		if err != nil {
+			a.logger.Error("Failed to generate final mission result", zap.Error(err))
 			a.bus.Acknowledge(msg)
+			acknowledged = true
+			return false // Continue loop, let mind potentially retry conclusion or do something else
+		}
+		if result != nil {
+			// CRITICAL: Acknowledge BEFORE calling finish().
+			// FIX: Updated comment to be accurate.
+			// finish() sends the result; RunMission waits for result then calls bus.Shutdown().
+			a.bus.Acknowledge(msg)
+			acknowledged = true
+			// FIX: Pass context to finish to prevent goroutine leak.
+			a.finish(ctx, *result)
+		}
+		return true // Stop the loop.
 
-		case <-ctx.Done():
-			return
+	case ActionEvolveCodebase:
+		a.logger.Info("Agent decided to initiate self-improvement (Evolution).", zap.String("rationale", action.Rationale))
+		execResult = a.executeEvolution(ctx, action)
+	}
+
+	// If execResult is not yet set, it means the action should be handled by the ExecutorRegistry.
+	if execResult == nil {
+		a.logger.Debug("Dispatching action to ExecutorRegistry", zap.String("type", string(action.Type)))
+		execResult, execErr = a.executors.Execute(ctx, action)
+	}
+
+	// Centralized error and nil-result handling.
+	if execErr != nil {
+		a.logger.Error("Action execution failed with a raw error", zap.String("action_type", string(action.Type)), zap.Error(execErr))
+		execResult = &ExecutionResult{
+			Status:          "failed",
+			ObservationType: ObservedSystemState,
+			ErrorCode:       ErrCodeExecutionFailure,
+			ErrorDetails:    map[string]interface{}{"message": execErr.Error()},
+		}
+	} else if execResult == nil {
+		// This is a safeguard against a logic error where an action handler returns (nil, nil).
+		a.logger.Error("CRITICAL: Action handler returned nil result and nil error.", zap.String("action_type", string(action.Type)))
+		// Create a fallback result to prevent nil pointer in postObservation
+		execResult = &ExecutionResult{
+			Status:          "failed",
+			ObservationType: ObservedSystemState,
+			ErrorCode:       ErrCodeExecutionFailure,
+			ErrorDetails:    map[string]interface{}{"message": "Internal Error: Action handler returned nil result."},
 		}
 	}
+
+	a.postObservation(ctx, action, execResult)
+	a.bus.Acknowledge(msg)
+	acknowledged = true
+	return false // Continue loop
 }
 
 // executeEvolution handles the EVOLVE_CODEBASE action by invoking the EvolutionEngine.
@@ -472,6 +510,7 @@ func (a *Agent) concludeMission(ctx context.Context) (*MissionResult, error) {
 		return nil, fmt.Errorf("failed to gather final context for summary: %w", err)
 	}
 
+	// FIX: Use MarshalIndent for better readability and debugging.
 	subgraphJSON, err := json.MarshalIndent(subgraph, "", "  ")
 	if err != nil {
 		return nil, fmt.Errorf("failed to marshal subgraph for summary prompt: %w", err)
@@ -566,6 +605,7 @@ func (a *Agent) finish(ctx context.Context, result MissionResult) {
 	a.mind.Stop()
 	// Bus shutdown is handled in RunMission after the result is successfully received.
 
+	// FIX: Use select to send result, preventing blocking forever if the runner (RunMission)
 	// Use select to send result, preventing blocking forever if the runner (RunMission)
 	// has already exited (e.g., due to timeout/cancellation).
 	select {

internal/agent/agent_test.go

@@ -273,6 +273,70 @@ func TestAgent_RunMission_Success(t *testing.T) {
 	mockLTM.AssertExpectations(t) // Verify LTM mock expectations
 }
 
+// NEW TEST: TestAgent_ActionLoop_PanicRecovery verifies that the action loop recovers from panics
+// during message processing and acknowledges the message to prevent shutdown deadlocks.
+func TestAgent_ActionLoop_PanicRecovery(t *testing.T) {
+	// 1. Setup
+	// We need a short timeout for the overall test execution to detect deadlocks.
+	testCtx, cancelTest := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancelTest()
+
+	// Initialize the agent and its dependencies.
+	agent, _, bus, mockExecutors, _, _, _, _, _, _ := setupAgentTest(t)
+
+	// Use a separate context for the actionLoop itself, which we won't cancel until the end.
+	loopCtx, cancelLoop := context.WithCancel(context.Background())
+
+	// Subscribe to the action channel that the loop will consume from.
+	actionChan, unsubscribeActions := bus.Subscribe(MessageTypeAction)
+	defer unsubscribeActions()
+
+	// 2. Configure the mock executor to panic when a specific action is executed.
+	panickingAction := Action{Type: ActionClick, ID: "panic-action"}
+	mockExecutors.On("Execute", mock.Anything, panickingAction).Run(func(args mock.Arguments) {
+		panic("Simulated executor panic!")
+	}).Return(nil, errors.New("this error is ignored because of panic")).Once()
+
+	// 3. Start the action loop in a separate goroutine.
+	agent.wg.Add(1)
+	loopFinishedChan := make(chan struct{})
+	go func() {
+		// Catch any panic propagating out of the loop just in case the internal recovery fails.
+		defer func() {
+			if r := recover(); r != nil {
+				t.Logf("Test caught panic propagating out of actionLoop (unexpected): %v", r)
+			}
+			close(loopFinishedChan)
+		}()
+		agent.actionLoop(loopCtx, actionChan)
+	}()
+
+	// 4. Post the message that will cause the panic.
+	err := bus.Post(testCtx, CognitiveMessage{ID: "test-msg-panic", Type: MessageTypeAction, Payload: panickingAction})
+	require.NoError(t, err)
+
+	// 5. Verify Acknowledgment by attempting to shut down the bus.
+	// If the message wasn't acknowledged (because the loop crashed before recovery), bus.Shutdown() will hang.
+	shutdownDone := make(chan struct{})
+	go func() {
+		// Shutdown waits for all in-flight messages to be acknowledged.
+		bus.Shutdown()
+		close(shutdownDone)
+	}()
+
+	select {
+	case <-shutdownDone:
+		// Success: Bus shut down cleanly, meaning the message was acknowledged despite the panic.
+	case <-testCtx.Done():
+		t.Fatal("Timeout waiting for bus shutdown. Message likely unacknowledged due to panic in actionLoop.")
+	}
+
+	// 6. Clean up the running loop.
+	cancelLoop()
+	// Wait for the loop goroutine to finish (safe because we know it didn't deadlock).
+	<-loopFinishedChan
+}
+
 // TestAgent_RunMission_MindFailure verifies the agent fails fast if the Mind fails to start.
 func TestAgent_RunMission_MindFailure(t *testing.T) {
 	// Arrange
@@ -353,6 +417,58 @@ func TestAgent_RunMission_ContextCancellation(t *testing.T) {
 	mockLLM.AssertExpectations(t)
 }
 
+// NEW TEST: TestAgent_RunMission_CancellationBeforeFinish verifies that the actionLoop
+// does not leak if the context is cancelled right when the agent tries to finish.
+func TestAgent_RunMission_CancellationBeforeFinish(t *testing.T) {
+	// This tests the fix where finish() now accepts a context and uses select{} when sending the result.
+
+	agent, mockMind, _, _, _, mockKG, mockLLM, mockLTM, _, _ := setupAgentTest(t)
+	// Create a context that we can cancel.
+	ctx, cancel := context.WithCancel(context.Background())
+
+	// Set expectations for the initial startup
+	mockMind.On("SetMission", agent.mission).Return().Once()
+	// Mind.Start should run until the context passed to it (missionCtx) is cancelled.
+	mockMind.On("Start", mock.Anything).Run(func(args mock.Arguments) {
+		startCtx := args.Get(0).(context.Context)
+		<-startCtx.Done() // Block until cancelled
+	}).Return(context.Canceled).Once()
+	mockMind.On("Stop").Return().Once()
+	mockLTM.On("Start").Return().Once()
+
+	// Set expectations for the conclusion (which will happen after cancellation in RunMission)
+	// These mocks are needed because RunMission calls concludeMission upon cancellation.
+	mockKG.On("GetNode", mock.Anything, mock.Anything).Return(schemas.Node{}, nil).Maybe()
+	mockKG.On("GetEdges", mock.Anything, mock.Anything).Return([]schemas.Edge{}, nil).Maybe()
+	mockLLM.On("Generate", mock.Anything, mock.Anything).Return("Cancelled summary.", nil).Maybe()
+
+	// Act
+	var runMissionWg sync.WaitGroup
+	runMissionWg.Add(1)
+	go func() {
+		defer runMissionWg.Done()
+		// RunMission will block until cancelled.
+		_, _ = agent.RunMission(ctx)
+	}()
+
+	// Allow the agent and its actionLoop to start up.
+	time.Sleep(100 * time.Millisecond)
+
+	// We need to ensure the actionLoop attempts to process the message *after* we cancel the context.
+	// This simulates the race condition where RunMission exits due to cancellation
+	// before the actionLoop finishes sending the result via finish().
+	cancel() // Cancel the context, causing RunMission to start shutting down.
+
+	// Wait for RunMission to return. This confirms the receiver (RunMission) is gone.
+	runMissionWg.Wait()
+
+	// Crucial Assertion: Wait for the agent's internal WaitGroup (which includes the actionLoop).
+	// If the actionLoop leaks (because finish() blocks), this will time out.
+	assert.True(t, waitTimeout(&agent.wg, 2*time.Second), "Agent WaitGroup did not complete, potential goroutine leak in actionLoop/finish.")
+
+	mockMind.AssertExpectations(t)
+}
+
 // TestAgent_ActionLoop verifies the correct dispatching of various action types.
 func TestAgent_ActionLoop(t *testing.T) {
 	// Helper to setup and run the action loop in the background
@@ -561,54 +677,8 @@ func TestAgent_ActionLoop(t *testing.T) {
 		}
 	})
 
-	// NEW: Test for complex actions being dispatched to the executor
-	t.Run("ExecuteLoginSequenceAction_DispatchedToExecutor", func(t *testing.T) {
-		agent, bus, cancelRoot, _ := setupActionLoop(t)
-		defer cancelRoot()
-		mockExecutors := agent.executors.(*MockExecutorRegistry)
-
-		action := Action{Type: ActionExecuteLoginSequence, Rationale: "Attempting login"}
-		obsChan, unsub := bus.Subscribe(MessageTypeObservation)
-		defer unsub()
-
-		execResult := &ExecutionResult{Status: "success", ObservationType: ObservedAuthResult}
-		mockExecutors.On("Execute", mock.Anything, action).Return(execResult, nil).Once()
-
-		err := bus.Post(context.Background(), CognitiveMessage{ID: "login-msg", Type: MessageTypeAction, Payload: action})
-		require.NoError(t, err)
-
-		select {
-		case msg := <-obsChan:
-			bus.Acknowledge(msg)
-			mockExecutors.AssertExpectations(t)
-		case <-time.After(2 * time.Second):
-			t.Fatal("Timeout waiting for ActionExecuteLoginSequence to be dispatched")
-		}
-	})
-
-	t.Run("ExploreApplicationAction_DispatchedToExecutor", func(t *testing.T) {
-		agent, bus, cancelRoot, _ := setupActionLoop(t)
-		defer cancelRoot()
-		mockExecutors := agent.executors.(*MockExecutorRegistry)
-
-		action := Action{Type: ActionExploreApplication, Rationale: "Exploring the app"}
-		obsChan, unsub := bus.Subscribe(MessageTypeObservation)
-		defer unsub()
-
-		execResult := &ExecutionResult{Status: "success", ObservationType: ObservedDOMChange}
-		mockExecutors.On("Execute", mock.Anything, action).Return(execResult, nil).Once()
-
-		err := bus.Post(context.Background(), CognitiveMessage{ID: "explore-msg", Type: MessageTypeAction, Payload: action})
-		require.NoError(t, err)
-
-		select {
-		case msg := <-obsChan:
-			bus.Acknowledge(msg)
-			mockExecutors.AssertExpectations(t)
-		case <-time.After(2 * time.Second):
-			t.Fatal("Timeout waiting for ActionExploreApplication to be dispatched")
-		}
-	})
+	// REMOVED: ExecuteLoginSequenceAction and ExploreApplicationAction tests are removed here
+	// because they are now explicitly covered by the ExecutorRegistry tests (TestExecutorRegistry_Execute/RegisteredComplexActions_RoutedToBrowserExecutor).
 
 	t.Run("FuzzEndpointAction_DispatchedToExecutor", func(t *testing.T) {
 		agent, bus, cancelRoot, _ := setupActionLoop(t)