From 14867e474ec84f431bde6380c26d8bb845561aad Mon Sep 17 00:00:00 2001 From: franconicola Date: Sat, 25 Jul 2026 18:51:24 +0200 Subject: [PATCH] fix(audit): surface tracking and evaluation failures --- CONTRIBUTING.md | 16 +++ .../attacks/evaluator/evaluation_step.py | 10 +- .../attacks/evaluator/inline_step_judge.py | 10 +- hackagent/attacks/evaluator/sync.py | 7 +- hackagent/attacks/orchestrator.py | 126 ++++++++++++++++-- hackagent/router/tracking/audit.py | 72 ++++++++++ hackagent/router/tracking/decorators.py | 37 +++-- hackagent/router/tracking/step.py | 37 ++++- hackagent/router/tracking/tracker.py | 22 ++- hackagent/server/storage/remote.py | 25 ++-- .../attacks/test_orchestrator_extended.py | 95 +++++++++++++ tests/unit/router/tracking/test_decorators.py | 6 + tests/unit/router/tracking/test_tracker.py | 59 +++++++- .../server/storage/test_remote_backend.py | 45 +++++++ 14 files changed, 520 insertions(+), 47 deletions(-) create mode 100644 hackagent/router/tracking/audit.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 342a5a13..e01fc281 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -146,6 +146,22 @@ We follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0 Commit messages should be structured as follows: +### Python Styleguide + +Audit-bearing code must never silently swallow an exception. This includes +run/result/trace persistence, router dispatch, orchestration, and evaluator +code that contributes to a security report. + +When handling an exception in these paths, use one of the following patterns: + +* Catch the specific exception type and recover without losing audit data. +* Log the exception with `exc_info=True` and re-raise it. +* Persist a structured failure containing at least + `{"step": ..., "status": "failed", "error": ...}` on the run or result. + +Do not add `except Exception: pass`, and do not turn a partially tracked or +partially evaluated run into a successful "no findings" result. + ## License By contributing to HackAgent, you agree that your contributions will be licensed under its [Apache License 2.0](LICENSE). diff --git a/hackagent/attacks/evaluator/evaluation_step.py b/hackagent/attacks/evaluator/evaluation_step.py index f361f679..fd06fdfd 100644 --- a/hackagent/attacks/evaluator/evaluation_step.py +++ b/hackagent/attacks/evaluator/evaluation_step.py @@ -325,6 +325,7 @@ def _sync_metrics_to_backend_structured(self, summary: Dict[str, Any]): self.logger.warning( "Failed to recompute summary from persisted results: %s", e, + exc_info=True, ) merged_run_config: Dict[str, Any] = {} @@ -333,6 +334,10 @@ def _sync_metrics_to_backend_structured(self, summary: Dict[str, Any]): if isinstance(existing_run.run_config, dict): merged_run_config = dict(existing_run.run_config) except Exception: + self.logger.warning( + "Failed to read existing run config before metrics sync", + exc_info=True, + ) merged_run_config = {} merged_run_config["evaluation_summary"] = summary_to_store @@ -346,7 +351,10 @@ def _sync_metrics_to_backend_structured(self, summary: Dict[str, Any]): self.logger.warning("No tracking client available; cannot sync metrics") except Exception as e: - self.logger.warning(f"Failed to sync structured metrics: {e}") + self.logger.warning( + f"Failed to sync structured metrics: {e}", + exc_info=True, + ) def resolve_agent_type(self, agent_type_value: Any) -> AgentTypeEnum: """Convert a string, enum, or ``None`` into an ``AgentTypeEnum``.""" diff --git a/hackagent/attacks/evaluator/inline_step_judge.py b/hackagent/attacks/evaluator/inline_step_judge.py index ed6a507f..2930e477 100644 --- a/hackagent/attacks/evaluator/inline_step_judge.py +++ b/hackagent/attacks/evaluator/inline_step_judge.py @@ -117,7 +117,10 @@ def __init__( ) self._judges.append((judge_type, judge_range, evaluator)) except Exception as exc: - logger.warning(f"Could not initialise judge '{judge_type}': {exc}") + logger.warning( + f"Could not initialise judge '{judge_type}': {exc}", + exc_info=True, + ) if not self._judges: logger.warning("No valid judges initialised for inline evaluation") @@ -188,7 +191,10 @@ def is_jailbreak( except (TypeError, ValueError): pass except Exception as exc: - self.logger.warning(f"Judge '{judge_type}' failed on candidate: {exc}") + self.logger.warning( + f"Judge '{judge_type}' failed on candidate: {exc}", + exc_info=True, + ) if not success_votes: return False, best_score, judge_cols diff --git a/hackagent/attacks/evaluator/sync.py b/hackagent/attacks/evaluator/sync.py index 767e5a20..c72c74a7 100644 --- a/hackagent/attacks/evaluator/sync.py +++ b/hackagent/attacks/evaluator/sync.py @@ -139,6 +139,11 @@ def update_single_result( ) merged_metadata = {**base, **metadata_updates} except Exception: + log.warning( + "Could not read existing result metadata for %s", + result_id, + exc_info=True, + ) merged_metadata = dict(metadata_updates) backend.update_result( @@ -151,7 +156,7 @@ def update_single_result( return True except Exception as e: - log.error(f"Exception updating result {result_id}: {e}") + log.error(f"Exception updating result {result_id}: {e}", exc_info=True) return False diff --git a/hackagent/attacks/orchestrator.py b/hackagent/attacks/orchestrator.py index 6cdd2a59..088855b2 100644 --- a/hackagent/attacks/orchestrator.py +++ b/hackagent/attacks/orchestrator.py @@ -42,6 +42,7 @@ import httpx from hackagent.errors import HackAgentError +from hackagent.router.tracking.audit import record_run_audit_failure from hackagent.attacks.techniques.config import ( DEFAULT_CATEGORY_CLASSIFIER_AGENT_TYPE, DEFAULT_CATEGORY_CLASSIFIER_ENDPOINT, @@ -252,6 +253,7 @@ def _backend_api_key_for_role_defaults(self) -> Optional[str]: try: api_key = getter() except Exception: + logger.debug("Configured API-key getter failed", exc_info=True) return None if isinstance(api_key, str) and api_key.strip(): @@ -456,7 +458,7 @@ def _create_server_run_record( def safe_uuid(val: str) -> UUID: try: return UUID(val) - except Exception: + except (AttributeError, TypeError, ValueError): # Log warning and fallback to a new UUID logger.warning(f"Invalid UUID '{val}', generating fallback UUID") return uuid4() @@ -680,6 +682,7 @@ def _autopull_missing_ollama_targets(self, targets: List[Dict[str, Any]]) -> Non try: installed = self._get_installed_ollama_models() except Exception: + logger.debug("Unable to inspect installed Ollama models", exc_info=True) return seen: set[str] = set() for model in candidates: @@ -728,6 +731,10 @@ def _validate_default_category_classifier_requirements( try: installed_models = self._get_installed_ollama_models() except Exception: + logger.warning( + "Unable to verify the pulled Ollama model", + exc_info=True, + ) installed_models = set() pulled = self._is_ollama_model_present(required_model, installed_models) if not pulled: @@ -998,6 +1005,10 @@ def _register_target( try: agent_instance = router_obj.get_agent_instance(registration_key) except Exception: + logger.debug( + "Unable to resolve registered agent for preflight", + exc_info=True, + ) agent_instance = None model_name = ( @@ -1116,6 +1127,10 @@ def _probe_router_registration( try: agent = router.get_agent_instance(registration_key) except Exception: + logger.debug( + "Unable to resolve registered agent during health check", + exc_info=True, + ) agent = None probe_ready = getattr(agent, "probe_ready", None) if callable(probe_ready): @@ -1504,7 +1519,9 @@ def _execute_local_attack( ) except Exception as e: logger.warning( - "Failed to apply max_tokens override to target adapter: %s", e + "Failed to apply max_tokens override to target adapter: %s", + e, + exc_info=True, ) # One monotonic start timestamp shared by all sub-runs/workers so @@ -1801,7 +1818,12 @@ def execute( status=StatusEnum.RUNNING.value, ) except Exception as e: - logger.warning(f"Failed to update run status to RUNNING: {e}") + logger.error( + f"Failed to update run status to RUNNING: {e}", + exc_info=True, + ) + if fail_on_run_error: + raise HackAgentError(f"Failed to start audit run {run_id}: {e}") from e if goal_labels_by_index: attack_config = { @@ -1853,6 +1875,7 @@ def execute( # ========================= # RUN EVALUATION PIPELINE # ========================= + evaluation_error: Optional[Exception] = None try: base_eval_config = { **attack_config, @@ -1910,7 +1933,15 @@ def execute( logger.info("Evaluation pipeline completed") except Exception as e: - logger.warning(f"Evaluation failed: {e}", exc_info=True) + evaluation_error = e + logger.error(f"Evaluation failed: {e}", exc_info=True) + record_run_audit_failure( + backend=self.hackagent_agent.backend, + run_id=run_id, + step="Evaluation Pipeline", + error=e, + logger=logger, + ) final_results = results # fallback if _tui_event_bus is not None: _tui_event_bus.emit( @@ -1930,23 +1961,83 @@ def execute( # ⏱ timing AFTER evaluation _total_elapsed = round(time.perf_counter() - _total_t0, 3) logger.info(f"Total run time: {_total_elapsed:.1f}s") + + # A tracking failure may already have marked the run FAILED while + # the attack logic continued. Reading the run also flushes queued + # remote audit writes, so COMPLETED is only possible after all + # audit artifacts have been persisted successfully. + final_status = ( + StatusEnum.FAILED + if evaluation_error is not None + else StatusEnum.COMPLETED + ) + if final_status is StatusEnum.COMPLETED: + try: + run_uuid = UUID(run_id) + except (AttributeError, TypeError, ValueError): + # Some custom/test backends use opaque run identifiers. + # Their update_run implementation remains authoritative. + logger.debug( + "Skipping final audit-status read for non-UUID run id %r", + run_id, + ) + run_uuid = None + try: + if run_uuid is not None: + persisted_run = self.hackagent_agent.backend.get_run(run_uuid) + persisted_status = str( + getattr(persisted_run, "status", "") or "" + ).upper() + if persisted_status == StatusEnum.FAILED.value: + final_status = StatusEnum.FAILED + except Exception as status_error: + logger.error( + "Failed to verify final audit status for run %s: %s", + run_id, + status_error, + exc_info=True, + ) + record_run_audit_failure( + backend=self.hackagent_agent.backend, + run_id=run_id, + step="Verify final audit status", + error=status_error, + logger=logger, + ) + final_status = StatusEnum.FAILED + if _tui_event_bus is not None: _tui_event_bus.emit( "step_ended", step_name="Attack Execution", - success=True, + success=final_status is StatusEnum.COMPLETED, elapsed_s=_total_elapsed, + error=( + str(evaluation_error) if evaluation_error is not None else None + ), ) - # ✅ Update run status to COMPLETED + # Only trustworthy, fully evaluated runs may be marked completed. try: - logger.info(f"Updating run {run_id} status to COMPLETED") + logger.info( + "Updating run %s status to %s", + run_id, + final_status.value, + ) self.hackagent_agent.backend.update_run( UUID(run_id), - status=StatusEnum.COMPLETED.value, + status=final_status.value, ) except Exception as e: - logger.warning(f"Failed to update run status to COMPLETED: {e}") + logger.error( + "Failed to update run %s status to %s: %s", + run_id, + final_status.value, + e, + exc_info=True, + ) + if fail_on_run_error: + raise return final_results @@ -1960,7 +2051,10 @@ def execute( run_notes=f"Execution failed: {str(e)}", ) except Exception as update_error: - logger.warning(f"Failed to update run status to FAILED: {update_error}") + logger.critical( + f"Failed to update run status to FAILED: {update_error}", + exc_info=True, + ) if _tui_event_bus is not None: _tui_event_bus.emit( "step_ended", @@ -1977,7 +2071,17 @@ def execute( try: flush() except Exception as flush_error: # noqa: BLE001 - logger.warning(f"Failed to flush backend writes: {flush_error}") + logger.error( + f"Failed to flush backend writes: {flush_error}", + exc_info=True, + ) + record_run_audit_failure( + backend=self.hackagent_agent.backend, + run_id=run_id, + step="Flush audit writes", + error=flush_error, + logger=logger, + ) # ======================================================================== # HTTP Response Helpers diff --git a/hackagent/router/tracking/audit.py b/hackagent/router/tracking/audit.py new file mode 100644 index 00000000..1f9ce794 --- /dev/null +++ b/hackagent/router/tracking/audit.py @@ -0,0 +1,72 @@ +# Copyright 2026 - AI4I. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Helpers for making audit-pipeline failures visible in run records.""" + +import json +import logging +from typing import Any, Dict +from uuid import UUID + +from hackagent.server.storage.enums import StatusEnum + + +class AuditPersistenceError(RuntimeError): + """Raised when an audit failure cannot itself be persisted.""" + + +def record_run_audit_failure( + backend: Any, + run_id: str, + step: str, + error: BaseException, + logger: logging.Logger, +) -> Dict[str, str]: + """Persist a structured audit failure and mark the run as failed. + + Audit-bearing code may continue after a recoverable tracking failure, but + it must never make that failure invisible. If the run record cannot be + updated, this helper raises so callers cannot report a trustworthy result. + """ + entry = { + "step": step, + "status": "failed", + "error": f"{type(error).__name__}: {error}"[:1000], + } + + try: + run_uuid = UUID(str(run_id)) + except (AttributeError, TypeError, ValueError) as exc: + logger.error( + "Cannot persist audit failure for %s: invalid run id %r", + step, + run_id, + exc_info=True, + ) + raise AuditPersistenceError( + f"Cannot persist audit failure for '{step}': invalid run id" + ) from exc + + try: + backend.update_run( + run_uuid, + status=StatusEnum.FAILED.value, + run_notes=json.dumps({"audit_failure": entry}, sort_keys=True), + ) + except Exception as exc: + logger.critical( + "Cannot persist audit failure for %s: %s", + step, + exc, + exc_info=True, + ) + raise AuditPersistenceError( + f"Cannot persist audit failure for '{step}'" + ) from exc + + logger.error( + "Recorded audit failure for %s: %s", + step, + entry["error"], + ) + return entry diff --git a/hackagent/router/tracking/decorators.py b/hackagent/router/tracking/decorators.py index 30eaf231..f2799600 100644 --- a/hackagent/router/tracking/decorators.py +++ b/hackagent/router/tracking/decorators.py @@ -81,11 +81,20 @@ def wrapper(*args, **kwargs): input_data = extract_input(args, kwargs) except Exception as e: tracker.logger.warning( - f"Failed to extract input data for '{step_name}': {e}" + f"Failed to extract input data for '{step_name}': {e}", + exc_info=True, ) + tracker.record_failure(f"{step_name}: extract input", e) else: # Default extraction: look for common parameter names - input_data = _default_extract_input(args, kwargs) + try: + input_data = _default_extract_input(args, kwargs) + except Exception as e: + tracker.logger.warning( + f"Failed to extract input data for '{step_name}': {e}", + exc_info=True, + ) + tracker.record_failure(f"{step_name}: extract input", e) # Extract config if extractor provided config = None @@ -94,8 +103,10 @@ def wrapper(*args, **kwargs): config = extract_config(args, kwargs) except Exception as e: tracker.logger.warning( - f"Failed to extract config for '{step_name}': {e}" + f"Failed to extract config for '{step_name}': {e}", + exc_info=True, ) + tracker.record_failure(f"{step_name}: extract config", e) else: # Default extraction: look for 'config' parameter config = kwargs.get("config") @@ -136,10 +147,7 @@ def _default_extract_input(args: tuple, kwargs: dict) -> Optional[Dict[str, Any] value = kwargs[key] if hasattr(value, "head"): # It's a DataFrame-like object - try: - return {"input_sample": value.head().to_dict()} - except Exception: - pass + return {"input_sample": value.head().to_dict()} # Try to get list inputs for key in ["goals", "targets", "inputs"]: @@ -152,10 +160,7 @@ def _default_extract_input(args: tuple, kwargs: dict) -> Optional[Dict[str, Any] # Try first positional argument if it's a DataFrame if args and hasattr(args[0], "head"): - try: - return {"input_sample": args[0].head().to_dict()} - except Exception: - pass + return {"input_sample": args[0].head().to_dict()} return None @@ -272,7 +277,15 @@ def wrapper(self, *args, **kwargs): return func(self, *args, **kwargs) # Extract input data - input_data = _default_extract_input(args, kwargs) + try: + input_data = _default_extract_input(args, kwargs) + except Exception as e: + tracker.logger.warning( + f"Failed to extract input data for '{step_name}': {e}", + exc_info=True, + ) + tracker.record_failure(f"{step_name}: extract input", e) + input_data = None # Extract config (might be in kwargs or self.config) config = kwargs.get("config") or getattr(self, "config", None) diff --git a/hackagent/router/tracking/step.py b/hackagent/router/tracking/step.py index d6daf8bd..b44d600b 100644 --- a/hackagent/router/tracking/step.py +++ b/hackagent/router/tracking/step.py @@ -24,6 +24,7 @@ ) from .context import TrackingContext +from .audit import AuditPersistenceError, record_run_audit_failure from .utils import deep_clean, sanitize_for_json @@ -68,6 +69,30 @@ def __init__(self, context: TrackingContext): self.context = context self.logger = context.logger + def record_failure(self, step_name: str, error: BaseException) -> Dict[str, str]: + """Record a structured tracking failure on the parent run.""" + if not self.context.is_enabled: + self.logger.error( + "Tracking failure in '%s' could not be persisted because tracking " + "is disabled: %s", + step_name, + error, + exc_info=True, + ) + return { + "step": step_name, + "status": "failed", + "error": f"{type(error).__name__}: {error}"[:1000], + } + + return record_run_audit_failure( + backend=self.context.backend, + run_id=str(self.context.run_id), + step=step_name, + error=error, + logger=self.logger, + ) + @contextmanager def track_step( self, @@ -182,9 +207,15 @@ def track_step( except Exception: self.logger.debug("StepTracker event emit failed", exc_info=True) + except AuditPersistenceError: + # The original tracking failure was already logged, and its + # fallback run-record write also failed. Do not recursively try to + # audit the audit-persistence error. + raise except Exception as e: # Handle step failure self.logger.error(f"Step '{step_name}' failed: {e}", exc_info=True) + self.record_failure(step_name, e) self._handle_step_error(step_name, str(e)) self._create_summary_trace( step_name=step_name, @@ -275,6 +306,7 @@ def _create_trace( self.logger.error( f"Exception creating trace for '{step_name}': {e}", exc_info=True ) + self.record_failure(f"{step_name}: create trace", e) return None @@ -356,6 +388,7 @@ def _create_summary_trace( f"Exception creating summary trace for '{step_name}': {e}", exc_info=True, ) + self.record_failure(f"{step_name}: create summary trace", e) return None @@ -384,6 +417,7 @@ def _handle_step_error(self, step_name: str, error_message: str) -> None: except Exception as e: self.logger.error(f"Failed to update error status: {e}", exc_info=True) + self.record_failure(f"{step_name}: update error status", e) def update_run_status(self, status: StatusEnum) -> bool: """ @@ -412,7 +446,7 @@ def update_run_status(self, status: StatusEnum) -> bool: return True except Exception as e: self.logger.error(f"Exception updating run status: {e}", exc_info=True) - return False + raise def update_result_status( self, @@ -454,6 +488,7 @@ def update_result_status( return True except Exception as e: self.logger.error(f"Exception updating result status: {e}", exc_info=True) + self.record_failure("Update result status", e) return False def add_step_metadata(self, key: str, value: Any) -> None: diff --git a/hackagent/router/tracking/tracker.py b/hackagent/router/tracking/tracker.py index 65c72d43..d3f82ad1 100644 --- a/hackagent/router/tracking/tracker.py +++ b/hackagent/router/tracking/tracker.py @@ -35,6 +35,7 @@ UNKNOWN_CATEGORY, UNKNOWN_SUBCATEGORY, ) +from .audit import record_run_audit_failure from .utils import deep_clean, sanitize_for_json @@ -166,6 +167,16 @@ def __init__( ) self._goal_contexts: Dict[int, Context] = {} + def _record_failure(self, step: str, error: BaseException) -> Dict[str, str]: + """Make a goal-tracking failure visible on the run record.""" + return record_run_audit_failure( + backend=self.backend, + run_id=self.run_id, + step=step, + error=error, + logger=self.logger, + ) + def _emit(self, event_type: str, **payload: Any) -> None: """Emit on ``event_bus`` if present; swallow any error.""" bus = self.event_bus @@ -280,6 +291,7 @@ def create_goal_result( self.logger.error( f"Exception creating result for goal {goal_index}: {e}", exc_info=True ) + self._record_failure(f"Goal {goal_index}: create result", e) self._goal_contexts[goal_index] = ctx self._emit( @@ -323,7 +335,9 @@ def _classify_goal_labels(self, goal: str, goal_index: int) -> Dict[str, str]: "Goal classification failed for goal %s: %s", goal[:80], e, + exc_info=True, ) + self._record_failure(f"Goal {goal_index}: classify", e) return fallback @@ -498,6 +512,7 @@ def _add_trace( f"Exception creating trace for goal {ctx.goal_index}: {e}", exc_info=True, ) + self._record_failure(f"Goal {ctx.goal_index}: create trace", e) return None @@ -548,7 +563,11 @@ def finalize_goal( except Exception as e: self.logger.debug( "Could not check existing evaluation status for goal " - f"{ctx.goal_index}: {e}" + f"{ctx.goal_index}: {e}", + exc_info=True, + ) + self._record_failure( + f"Goal {ctx.goal_index}: read evaluation status", e ) ctx.is_finalized = True @@ -630,6 +649,7 @@ def finalize_goal( self.logger.error( f"Exception finalizing goal {ctx.goal_index}: {e}", exc_info=True ) + self._record_failure(f"Goal {ctx.goal_index}: finalize", e) return False def get_goal_context(self, goal_index: int) -> Optional[Context]: diff --git a/hackagent/server/storage/remote.py b/hackagent/server/storage/remote.py index 0c687706..cacc9f97 100644 --- a/hackagent/server/storage/remote.py +++ b/hackagent/server/storage/remote.py @@ -196,9 +196,10 @@ def _enqueue(self, fn: Callable[..., Any], *args: Any, **kwargs: Any) -> None: fn(*args, **kwargs) except Exception: # noqa: BLE001 self._writer_failures += 1 - logger.warning( + logger.error( "RemoteBackend: synchronous fallback write failed", exc_info=True ) + raise return self._write_queue.put((fn, args, kwargs)) @@ -210,6 +211,11 @@ def flush(self) -> None: """ if not self._closed: self._write_queue.join() + if self._writer_failures: + raise RuntimeError( + "RemoteBackend: " + f"{self._writer_failures} background audit write(s) failed" + ) def close(self) -> None: """Drain the queue, stop the worker, and report any write failures.""" @@ -510,19 +516,9 @@ def update_run( created_at=_now(), updated_at=_now(), ) - logger.warning( + raise RuntimeError( f"RemoteBackend: update_run {run_id} returned {resp.status_code}" ) - return RunRecord( - id=run_id, - attack_id=UUID("00000000-0000-0000-0000-000000000000"), - agent_id=UUID("00000000-0000-0000-0000-000000000000"), - run_config=run_config or {}, - status=status or "", - run_notes=run_notes, - created_at=_now(), - updated_at=_now(), - ) def list_runs( self, @@ -606,6 +602,7 @@ def list_runs( return PaginatedResult(items=items, total=total or len(items)) def get_run(self, run_id: UUID) -> RunRecord: + self.flush() resp = run_retrieve.sync_detailed(id=run_id, client=self._client) if resp.status_code == 200 and resp.parsed: r = resp.parsed @@ -712,7 +709,7 @@ def _update_result_remote( id=result_id, client=self._client, body=body ) if resp.status_code >= 300: - logger.warning( + raise RuntimeError( f"RemoteBackend: update_result {result_id} returned {resp.status_code}" ) @@ -912,7 +909,7 @@ def _create_trace_remote( client=self._client, id=result_id, body=body ) if resp.status_code != 201: - logger.warning( + raise RuntimeError( f"RemoteBackend: create_trace for result {result_id} " f"returned {resp.status_code}" ) diff --git a/tests/unit/attacks/test_orchestrator_extended.py b/tests/unit/attacks/test_orchestrator_extended.py index 3e787eb6..9849f9e7 100644 --- a/tests/unit/attacks/test_orchestrator_extended.py +++ b/tests/unit/attacks/test_orchestrator_extended.py @@ -125,6 +125,44 @@ def test_execute_updates_run_status_to_completed( # Should have called update_run at least twice (RUNNING and COMPLETED) self.assertGreaterEqual(hack_agent.backend.update_run.call_count, 2) + @patch.object( + AttackOrchestrator, "_create_server_run_record", return_value=_VALID_RUN_ID + ) + @patch.object( + AttackOrchestrator, "_create_server_attack_record", return_value=_VALID_ATK_ID + ) + @patch.object( + AttackOrchestrator, + "_validate_required_models_availability", + return_value=None, + ) + @patch.object(AttackOrchestrator, "_execute_local_attack", return_value=["result"]) + def test_execute_preserves_preexisting_tracking_failure( + self, mock_exec, mock_validate_models, mock_create_atk, mock_create_run + ): + """A tracking failure must not be overwritten by final completion.""" + orch, hack_agent, _ = _make_orchestrator() + hack_agent.backend.get_run.return_value.status = "FAILED" + attack_config = { + "goals": ["test"], + "category_classifier": { + "identifier": "gpt-4o-mini", + "agent_type": "OPENAI", + "endpoint": "https://api.openai.com/v1", + }, + } + + orch.execute( + attack_config=attack_config, + run_config_override=None, + fail_on_run_error=False, + ) + + self.assertEqual( + hack_agent.backend.update_run.call_args.kwargs["status"], + "FAILED", + ) + @patch.object( AttackOrchestrator, "_create_server_run_record", return_value=_VALID_RUN_ID ) @@ -198,6 +236,63 @@ def test_execute_continues_when_status_update_fails( ) self.assertIsNotNone(results) + @patch.object( + AttackOrchestrator, "_create_server_run_record", return_value=_VALID_RUN_ID + ) + @patch.object( + AttackOrchestrator, "_create_server_attack_record", return_value=_VALID_ATK_ID + ) + @patch.object( + AttackOrchestrator, + "_validate_required_models_availability", + return_value=None, + ) + @patch.object(AttackOrchestrator, "_execute_local_attack", return_value=["result"]) + @patch( + "hackagent.attacks.evaluator.evaluation_step.BaseEvaluationStep.run_full_evaluation", + side_effect=RuntimeError("judge unavailable"), + ) + def test_evaluation_failure_is_recorded_and_run_stays_failed( + self, + mock_evaluate, + mock_exec, + mock_validate_models, + mock_create_atk, + mock_create_run, + ): + """A fallback result must not hide a failed evaluation pipeline.""" + orch, hack_agent, _ = _make_orchestrator() + attack_config = { + "goals": ["test"], + "category_classifier": { + "identifier": "gpt-4o-mini", + "agent_type": "OPENAI", + "endpoint": "https://api.openai.com/v1", + }, + } + + results = orch.execute( + attack_config=attack_config, + run_config_override=None, + fail_on_run_error=False, + ) + + self.assertEqual(results, ["result"]) + audit_calls = [ + call + for call in hack_agent.backend.update_run.call_args_list + if call.kwargs.get("run_notes") + ] + self.assertEqual(len(audit_calls), 1) + audit_failure = json.loads(audit_calls[0].kwargs["run_notes"])["audit_failure"] + self.assertEqual(audit_failure["step"], "Evaluation Pipeline") + self.assertEqual(audit_failure["status"], "failed") + self.assertIn("judge unavailable", audit_failure["error"]) + self.assertEqual( + hack_agent.backend.update_run.call_args.kwargs["status"], + "FAILED", + ) + @patch.object( AttackOrchestrator, "_create_server_run_record", return_value=_VALID_RUN_ID ) diff --git a/tests/unit/router/tracking/test_decorators.py b/tests/unit/router/tracking/test_decorators.py index cacb8465..1b3313da 100644 --- a/tests/unit/router/tracking/test_decorators.py +++ b/tests/unit/router/tracking/test_decorators.py @@ -3,6 +3,7 @@ """Tests for tracking decorators.""" +import json import unittest from unittest.mock import MagicMock, patch @@ -117,6 +118,11 @@ def my_function(data, tracker=None): # Should not raise, just log warning result = my_function(5, tracker=tracker) self.assertEqual(result, 10) + audit_call = mock_backend.update_run.call_args + audit_failure = json.loads(audit_call.kwargs["run_notes"])["audit_failure"] + self.assertEqual(audit_failure["step"], "Test Step: extract input") + self.assertEqual(audit_failure["status"], "failed") + self.assertIn("Extractor error", audit_failure["error"]) class TestDefaultExtractInput(unittest.TestCase): diff --git a/tests/unit/router/tracking/test_tracker.py b/tests/unit/router/tracking/test_tracker.py index 27a9523f..2e70d83e 100644 --- a/tests/unit/router/tracking/test_tracker.py +++ b/tests/unit/router/tracking/test_tracker.py @@ -9,6 +9,7 @@ from unittest.mock import MagicMock from hackagent.server.api.models import EvaluationStatusEnum, StatusEnum +from hackagent.router.tracking.audit import AuditPersistenceError from hackagent.router.tracking.context import TrackingContext from hackagent.router.tracking.step import StepTracker from hackagent.router.tracking.utils import sanitize_for_json @@ -80,8 +81,59 @@ def test_track_step_handles_exception(self): # Should have attempted to update error status mock_backend.update_result.assert_called() + audit_call = mock_backend.update_run.call_args + audit_failure = json.loads(audit_call.kwargs["run_notes"])["audit_failure"] + self.assertEqual(audit_call.kwargs["status"], StatusEnum.FAILED.value) + self.assertEqual(audit_failure["step"], "Test Step") + self.assertEqual(audit_failure["status"], "failed") + self.assertIn("Test error", audit_failure["error"]) self.assertGreaterEqual(mock_backend.create_trace.call_count, 1) + def test_tracking_side_exception_is_visible_in_run_record(self): + """A trace persistence failure must not produce a trustworthy run.""" + mock_backend = MagicMock() + context = TrackingContext( + backend=mock_backend, + run_id="12345678-1234-1234-1234-123456789abc", + parent_result_id="87654321-4321-4321-4321-cba987654321", + ) + tracker = StepTracker(context) + mock_backend.create_trace.side_effect = [ + RuntimeError("trace store unavailable"), + MagicMock(id="summary-trace-id"), + ] + + with tracker.track_step("Audit Step", "AUDIT_STEP"): + pass + + audit_call = mock_backend.update_run.call_args + audit_failure = json.loads(audit_call.kwargs["run_notes"])["audit_failure"] + self.assertEqual(audit_call.kwargs["status"], StatusEnum.FAILED.value) + self.assertEqual( + audit_failure, + { + "step": "Audit Step: create trace", + "status": "failed", + "error": "RuntimeError: trace store unavailable", + }, + ) + + def test_tracking_failure_is_raised_if_run_record_cannot_be_updated(self): + """Losing both a trace and its failure record must stop the run.""" + mock_backend = MagicMock() + context = TrackingContext( + backend=mock_backend, + run_id="12345678-1234-1234-1234-123456789abc", + parent_result_id="87654321-4321-4321-4321-cba987654321", + ) + tracker = StepTracker(context) + mock_backend.create_trace.side_effect = RuntimeError("trace write failed") + mock_backend.update_run.side_effect = RuntimeError("run write failed") + + with self.assertRaises(AuditPersistenceError): + with tracker.track_step("Audit Step", "AUDIT_STEP"): + pass + def test_track_step_records_metadata_and_progress(self): """Test that step metadata/progress logs are recorded in summary trace.""" mock_backend = MagicMock() @@ -210,11 +262,10 @@ def test_update_run_status_api_failure(self): ) tracker = StepTracker(context) - mock_backend.update_run.side_effect = Exception("Server error") + mock_backend.update_run.side_effect = RuntimeError("Server error") - result = tracker.update_run_status(StatusEnum.COMPLETED) - - self.assertFalse(result) + with self.assertRaises(RuntimeError): + tracker.update_run_status(StatusEnum.COMPLETED) class TestStepTrackerUpdateResultStatus(unittest.TestCase): diff --git a/tests/unit/server/storage/test_remote_backend.py b/tests/unit/server/storage/test_remote_backend.py index 8835f629..0da61986 100644 --- a/tests/unit/server/storage/test_remote_backend.py +++ b/tests/unit/server/storage/test_remote_backend.py @@ -322,6 +322,14 @@ def test_update_run(self): self.assertEqual(rec.status, "RUNNING") mock_patch.sync_detailed.assert_called_once() + def test_update_run_raises_on_error(self): + run_id = _uid() + + with patch("hackagent.server.storage.remote.run_partial_update") as mock_patch: + mock_patch.sync_detailed.return_value = _mock_response(500) + with self.assertRaisesRegex(RuntimeError, "update_run"): + self.backend.update_run(run_id, status="FAILED") + def test_list_runs_success(self): run_m = MagicMock() run_m.id = _uid() @@ -564,6 +572,25 @@ def test_update_result_success(self): self.assertIsInstance(rec, ResultRecord) + def test_update_result_failure_surfaces_at_flush(self): + result_id = _uid() + + with ( + patch( + "hackagent.server.storage.remote.result_partial_update" + ) as mock_patch, + patch("hackagent.server.storage.remote.logger"), + ): + mock_patch.sync_detailed.return_value = _mock_response(500) + self.backend.update_result( + result_id, + evaluation_status="ERROR_TEST_FRAMEWORK", + evaluation_notes="audit failure", + ) + with self.assertRaisesRegex(RuntimeError, "audit write"): + self.backend.flush() + self.backend.close() + def test_create_trace_success(self): result_id = _uid() trace_id = _uid() @@ -590,6 +617,24 @@ def test_create_trace_success(self): self.assertEqual(rec.result_id, result_id) self.assertEqual(rec.sequence, 1) + def test_create_trace_failure_surfaces_at_flush(self): + result_id = _uid() + + with ( + patch("hackagent.server.storage.remote.result_trace_create") as mock_create, + patch("hackagent.server.storage.remote.logger"), + ): + mock_create.sync_detailed.return_value = _mock_response(500) + self.backend.create_trace( + result_id=result_id, + sequence=1, + step_type="OTHER", + content={"msg": "lost audit trace"}, + ) + with self.assertRaisesRegex(RuntimeError, "audit write"): + self.backend.flush() + self.backend.close() + def test_create_trace_success_with_integer_id(self): result_id = _uid() parsed_m = MagicMock()