wrap_unexpected mutates CleverAgentsError.details in-place, contradicting its own docstring #8435

Open
opened 2026-04-13 18:53:11 +00:00 by HAL9000 · 1 comment
Owner

Metadata

  • Commit: Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.
  • Branch: main
  • SHA: 5a9aaa79ed

Background and Context

wrap_unexpected in src/cleveragents/core/error_handling.py is documented to handle CleverAgentsError instances by merging optional context into "a copy of its details (the original details dict object is not mutated)." However, the implementation replaces exc.details on the exception object itself:

exc.details = {**exc.details, **extra_context}

While a new dict object is created (so the original dict is not mutated), the exception object's .details attribute is replaced. This means:

  1. Any other reference to the exception object will see the modified .details.
  2. Calling wrap_unexpected twice on the same exception with different contexts will accumulate context from both calls on the same exception object.
  3. The docstring is misleading — it implies the exception is returned unchanged except for a copy, but the exception IS mutated.

Current Behavior

exc = CleverAgentsError("test", details={"key": "original"})
original_details = exc.details

wrap_unexpected(exc, context={"extra": "data"})
# exc.details is now {"key": "original", "extra": "data"}  ← MUTATED
# original_details is still {"key": "original"}  ← original dict unchanged

# Second call accumulates further
wrap_unexpected(exc, context={"more": "data"})
# exc.details is now {"key": "original", "extra": "data", "more": "data"}

Relevant code in error_handling.py:

if isinstance(exc, CleverAgentsError):
    if extra_context:
        # Create a new dict so the original details object is not mutated.
        exc.details = {**exc.details, **extra_context}  # ← mutates exc object!
    return exc

Expected Behavior

wrap_unexpected should not mutate the exception object. Instead, it should create and return a new CleverAgentsError with merged details, or at minimum the docstring should accurately describe the mutation:

Option A (preferred): Return a new exception with merged details:

if isinstance(exc, CleverAgentsError):
    merged_details = {**exc.details, **extra_context} if extra_context else exc.details
    return CleverAgentsError(exc.message, details=merged_details)

Option B: Update the docstring to accurately describe the mutation behaviour.

Acceptance Criteria

  • Calling wrap_unexpected(exc, context={...}) on a CleverAgentsError does not modify exc.details
  • The returned error contains the merged context
  • Calling wrap_unexpected twice on the same exception does not accumulate context on the original exception
  • BDD test scenario verifies the original exception is not mutated

Subtasks

  • Fix wrap_unexpected to not mutate the input exception object
  • Update docstring to accurately describe the return value
  • Add BDD test verifying no mutation of the original exception
  • Verify no existing tests break

Definition of Done

The issue is closed when wrap_unexpected does not mutate the input CleverAgentsError object, with a passing BDD test confirming immutability, merged to main.


Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata - **Commit**: Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR. - **Branch**: main - **SHA**: 5a9aaa79edaefb1a257114f054ea87facb8efe69 ## Background and Context `wrap_unexpected` in `src/cleveragents/core/error_handling.py` is documented to handle `CleverAgentsError` instances by merging optional context into "a **copy** of its details (the original `details` dict object is not mutated)." However, the implementation replaces `exc.details` on the exception object itself: ```python exc.details = {**exc.details, **extra_context} ``` While a new `dict` object is created (so the original dict is not mutated), the **exception object's `.details` attribute is replaced**. This means: 1. Any other reference to the exception object will see the modified `.details`. 2. Calling `wrap_unexpected` twice on the same exception with different contexts will accumulate context from both calls on the same exception object. 3. The docstring is misleading — it implies the exception is returned unchanged except for a copy, but the exception IS mutated. ## Current Behavior ```python exc = CleverAgentsError("test", details={"key": "original"}) original_details = exc.details wrap_unexpected(exc, context={"extra": "data"}) # exc.details is now {"key": "original", "extra": "data"} ← MUTATED # original_details is still {"key": "original"} ← original dict unchanged # Second call accumulates further wrap_unexpected(exc, context={"more": "data"}) # exc.details is now {"key": "original", "extra": "data", "more": "data"} ``` Relevant code in `error_handling.py`: ```python if isinstance(exc, CleverAgentsError): if extra_context: # Create a new dict so the original details object is not mutated. exc.details = {**exc.details, **extra_context} # ← mutates exc object! return exc ``` ## Expected Behavior `wrap_unexpected` should not mutate the exception object. Instead, it should create and return a new `CleverAgentsError` with merged details, or at minimum the docstring should accurately describe the mutation: **Option A (preferred)**: Return a new exception with merged details: ```python if isinstance(exc, CleverAgentsError): merged_details = {**exc.details, **extra_context} if extra_context else exc.details return CleverAgentsError(exc.message, details=merged_details) ``` **Option B**: Update the docstring to accurately describe the mutation behaviour. ## Acceptance Criteria - [ ] Calling `wrap_unexpected(exc, context={...})` on a `CleverAgentsError` does not modify `exc.details` - [ ] The returned error contains the merged context - [ ] Calling `wrap_unexpected` twice on the same exception does not accumulate context on the original exception - [ ] BDD test scenario verifies the original exception is not mutated ## Subtasks - [ ] Fix `wrap_unexpected` to not mutate the input exception object - [ ] Update docstring to accurately describe the return value - [ ] Add BDD test verifying no mutation of the original exception - [ ] Verify no existing tests break ## Definition of Done The issue is closed when `wrap_unexpected` does not mutate the input `CleverAgentsError` object, with a passing BDD test confirming immutability, merged to `main`. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor
HAL9000 added this to the v3.3.0 milestone 2026-04-13 19:00:44 +00:00
Author
Owner

[AUTO-OWNR-3] Triage Decision

Status: Verified

MoSCoW: Should Have
Priority: Medium

Rationale: wrap_unexpected mutates the CleverAgentsError exception object in-place, directly contradicting its own docstring which states the original details dict is not mutated. This is a correctness bug — repeated calls accumulate context on the same exception object, and any external reference to the exception will see unexpected mutations. Classified as Should Have — this is a code correctness issue that should be fixed for reliability, but it does not block v3.3.0 core features.

Next Steps: Implement Option A (preferred): return a new CleverAgentsError with merged details rather than mutating the input exception. Update the docstring to accurately describe the return value. Add a BDD test verifying immutability of the original exception object before closing.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

## [AUTO-OWNR-3] Triage Decision **Status**: ✅ Verified **MoSCoW**: Should Have **Priority**: Medium **Rationale**: `wrap_unexpected` mutates the `CleverAgentsError` exception object in-place, directly contradicting its own docstring which states the original details dict is not mutated. This is a correctness bug — repeated calls accumulate context on the same exception object, and any external reference to the exception will see unexpected mutations. Classified as Should Have — this is a code correctness issue that should be fixed for reliability, but it does not block v3.3.0 core features. **Next Steps**: Implement Option A (preferred): return a new `CleverAgentsError` with merged details rather than mutating the input exception. Update the docstring to accurately describe the return value. Add a BDD test verifying immutability of the original exception object before closing. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#8435
No description provided.