UAT: Multiple silent exception suppressions in _compute_actor_impact() hide database errors #3884

New issue

Open

opened 2026-04-06 07:08:32 +00:00 by freemo · 1 comment

freemo commented

2026-04-06 07:08:32 +00:00

Owner

Metadata

Branch: fix/actor-impact-exception-logging
Commit Message: fix(cli): add debug logging to silent exception blocks in _compute_actor_impact()
Milestone: (backlog — see note below)
Parent Epic: (to be linked — see orphan note below)

Backlog note: This issue was discovered during autonomous operation
on milestone v3.7.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Bug Report

Feature Area: Error Handling and Resilience
Severity: Medium
Found by: UAT automated testing

Background and Context

The _compute_actor_impact() function in src/cleveragents/cli/commands/actor.py (lines 212–276) was analyzed for compliance with the CONTRIBUTING.md exception propagation rules. This function is used to warn users before removing actors — it queries sessions, lifecycle plans, and actions to determine how many are referencing the actor being removed. Silent failures in this function directly undermine the safety check it is meant to provide.

Current Behavior (Violations Found)

The _compute_actor_impact() function contains three separate except Exception: pass blocks that silently swallow all exceptions:

# src/cleveragents/cli/commands/actor.py lines 231-276
    # --- Sessions referencing this actor ---
    session_count = 0
    try:
        session_service = container.session_service()
        sessions = session_service.list()
        session_count = sum(1 for s in sessions if s.actor_name == actor_name)
    except Exception:  # pragma: no cover - defensive; DB may be unavailable
        pass  # <-- VIOLATION #1

    # --- Active lifecycle plans referencing this actor ---
    active_plan_count = 0
    try:
        uow = container.unit_of_work()
        with uow.transaction() as ctx:
            plans = ctx.lifecycle_plans.list_plans(limit=10000)
        ...
    except Exception:  # pragma: no cover - defensive; DB may be unavailable
        pass  # <-- VIOLATION #2

    # --- Actions configured to use this actor ---
    action_count = 0
    try:
        lifecycle_service = container.plan_lifecycle_service()
        actions = lifecycle_service.list_actions()
        ...
    except Exception:  # pragma: no cover - defensive; DB may be unavailable
        pass  # <-- VIOLATION #3

Expected Behavior (from CONTRIBUTING.md)

Errors must never be suppressed. Exceptions should be allowed to propagate up to the top-level execution handler.
Catch-all exception handlers should not be used unless they re-raise the exception.
Exceptions should only be caught when there is a meaningful way to handle them.

Each except Exception: pass block must at minimum emit a logger.debug(...) call with exc_info=True so that failures are observable in debug output.

Why This Is a Problem

No logging: If any of these database queries fail, there is zero diagnostic information.
Violates CONTRIBUTING.md: Three separate bare pass blocks with no logging or re-raise.
Silent data corruption: If the database is unavailable, _compute_actor_impact() returns (0, 0, 0), which means the actor removal confirmation dialog shows "0 sessions, 0 plans, 0 actions affected" even when the database is broken. This could lead to users accidentally removing actors that ARE in use.
Security concern: The function is used to warn users before removing actors. Silent failures mean the safety check is silently bypassed.

Acceptance Criteria

Each of the three except Exception: pass blocks is replaced with a logger.debug(...) call that includes exc_info=True and the actor_name as a structured field.
No bare pass remains in any except block within _compute_actor_impact().
The function still returns (0, 0, 0) gracefully when the database is unavailable (degraded-mode behaviour is preserved), but the failure is now logged.
All existing Behave unit tests continue to pass.
New Behave scenarios cover the case where each service raises an exception and verifies that debug logging occurs.
Coverage remains ≥ 97%.

Recommended Fix

Replace each bare pass with a debug-level log:

    try:
        session_service = container.session_service()
        sessions = session_service.list()
        session_count = sum(1 for s in sessions if s.actor_name == actor_name)
    except Exception:
        logger.debug("Failed to query sessions for actor impact", actor_name=actor_name, exc_info=True)

    try:
        uow = container.unit_of_work()
        with uow.transaction() as ctx:
            plans = ctx.lifecycle_plans.list_plans(limit=10000)
        ...
    except Exception:
        logger.debug("Failed to query plans for actor impact", actor_name=actor_name, exc_info=True)

    try:
        lifecycle_service = container.plan_lifecycle_service()
        actions = lifecycle_service.list_actions()
        ...
    except Exception:
        logger.debug("Failed to query actions for actor impact", actor_name=actor_name, exc_info=True)

Code Location

File: src/cleveragents/cli/commands/actor.py
Lines: 212–276
Function: _compute_actor_impact()

Subtasks

Replace VIOLATION #1 (session_count block) bare pass with logger.debug(...) including exc_info=True
Replace VIOLATION #2 (active_plan_count block) bare pass with logger.debug(...) including exc_info=True
Replace VIOLATION #3 (action_count block) bare pass with logger.debug(...) including exc_info=True
Tests (Behave): Add scenarios for each exception path verifying debug log is emitted
Verify coverage ≥ 97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
No bare except Exception: pass blocks remain in _compute_actor_impact().
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/actor-impact-exception-logging` - **Commit Message**: `fix(cli): add debug logging to silent exception blocks in _compute_actor_impact()` - **Milestone**: *(backlog — see note below)* - **Parent Epic**: *(to be linked — see orphan note below)* > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.7.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- ## Bug Report **Feature Area:** Error Handling and Resilience **Severity:** Medium **Found by:** UAT automated testing ### Background and Context The `_compute_actor_impact()` function in `src/cleveragents/cli/commands/actor.py` (lines 212–276) was analyzed for compliance with the CONTRIBUTING.md exception propagation rules. This function is used to warn users before removing actors — it queries sessions, lifecycle plans, and actions to determine how many are referencing the actor being removed. Silent failures in this function directly undermine the safety check it is meant to provide. ### Current Behavior (Violations Found) The `_compute_actor_impact()` function contains **three** separate `except Exception: pass` blocks that silently swallow all exceptions: ```python # src/cleveragents/cli/commands/actor.py lines 231-276 # --- Sessions referencing this actor --- session_count = 0 try: session_service = container.session_service() sessions = session_service.list() session_count = sum(1 for s in sessions if s.actor_name == actor_name) except Exception: # pragma: no cover - defensive; DB may be unavailable pass # <-- VIOLATION #1 # --- Active lifecycle plans referencing this actor --- active_plan_count = 0 try: uow = container.unit_of_work() with uow.transaction() as ctx: plans = ctx.lifecycle_plans.list_plans(limit=10000) ... except Exception: # pragma: no cover - defensive; DB may be unavailable pass # <-- VIOLATION #2 # --- Actions configured to use this actor --- action_count = 0 try: lifecycle_service = container.plan_lifecycle_service() actions = lifecycle_service.list_actions() ... except Exception: # pragma: no cover - defensive; DB may be unavailable pass # <-- VIOLATION #3 ``` ### Expected Behavior (from CONTRIBUTING.md) > Errors must **never** be suppressed. Exceptions should be allowed to propagate up to the top-level execution handler. > Catch-all exception handlers should not be used unless they re-raise the exception. > Exceptions should only be caught when there is a meaningful way to handle them. Each `except Exception: pass` block must at minimum emit a `logger.debug(...)` call with `exc_info=True` so that failures are observable in debug output. ### Why This Is a Problem 1. **No logging**: If any of these database queries fail, there is zero diagnostic information. 2. **Violates CONTRIBUTING.md**: Three separate bare `pass` blocks with no logging or re-raise. 3. **Silent data corruption**: If the database is unavailable, `_compute_actor_impact()` returns `(0, 0, 0)`, which means the actor removal confirmation dialog shows "0 sessions, 0 plans, 0 actions affected" even when the database is broken. This could lead to users accidentally removing actors that ARE in use. 4. **Security concern**: The function is used to warn users before removing actors. Silent failures mean the safety check is silently bypassed. ### Acceptance Criteria - [ ] Each of the three `except Exception: pass` blocks is replaced with a `logger.debug(...)` call that includes `exc_info=True` and the `actor_name` as a structured field. - [ ] No bare `pass` remains in any `except` block within `_compute_actor_impact()`. - [ ] The function still returns `(0, 0, 0)` gracefully when the database is unavailable (degraded-mode behaviour is preserved), but the failure is now logged. - [ ] All existing Behave unit tests continue to pass. - [ ] New Behave scenarios cover the case where each service raises an exception and verifies that debug logging occurs. - [ ] Coverage remains ≥ 97%. ### Recommended Fix Replace each bare `pass` with a debug-level log: ```python try: session_service = container.session_service() sessions = session_service.list() session_count = sum(1 for s in sessions if s.actor_name == actor_name) except Exception: logger.debug("Failed to query sessions for actor impact", actor_name=actor_name, exc_info=True) try: uow = container.unit_of_work() with uow.transaction() as ctx: plans = ctx.lifecycle_plans.list_plans(limit=10000) ... except Exception: logger.debug("Failed to query plans for actor impact", actor_name=actor_name, exc_info=True) try: lifecycle_service = container.plan_lifecycle_service() actions = lifecycle_service.list_actions() ... except Exception: logger.debug("Failed to query actions for actor impact", actor_name=actor_name, exc_info=True) ``` ### Code Location - **File**: `src/cleveragents/cli/commands/actor.py` - **Lines**: 212–276 - **Function**: `_compute_actor_impact()` --- ## Subtasks - [ ] Replace VIOLATION #1 (`session_count` block) bare `pass` with `logger.debug(...)` including `exc_info=True` - [ ] Replace VIOLATION #2 (`active_plan_count` block) bare `pass` with `logger.debug(...)` including `exc_info=True` - [ ] Replace VIOLATION #3 (`action_count` block) bare `pass` with `logger.debug(...)` including `exc_info=True` - [ ] Tests (Behave): Add scenarios for each exception path verifying debug log is emitted - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - [ ] All subtasks above are completed and checked off. - [ ] No bare `except Exception: pass` blocks remain in `_compute_actor_impact()`. - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - [ ] The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo commented

2026-04-06 07:08:48 +00:00

Author

Owner

⚠️ Orphan Issue — Manual Linking Required

This issue was created without a known parent Epic. Per CONTRIBUTING.md, all non-Epic/non-Legendary issues must be linked to a parent Epic using Forgejo's dependency system (child blocks parent).

A maintainer should:

Identify the appropriate parent Epic (likely in the "Actors, Skills & Tools" workstream or an "Error Handling / Resilience" Epic).
Open this issue and add the parent Epic under "blocks", OR open the parent Epic and add this issue under "depends on".

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

⚠️ **Orphan Issue — Manual Linking Required** This issue was created without a known parent Epic. Per CONTRIBUTING.md, all non-Epic/non-Legendary issues must be linked to a parent Epic using Forgejo's dependency system (child **blocks** parent). A maintainer should: 1. Identify the appropriate parent Epic (likely in the "Actors, Skills & Tools" workstream or an "Error Handling / Resilience" Epic). 2. Open this issue and add the parent Epic under **"blocks"**, OR open the parent Epic and add this issue under **"depends on"**. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo referenced this issue

2026-04-06 07:13:06 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 07:21:26 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 10:35:53 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775