cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [data-integrity] repositories.py PlanRepository._to_domain derives success=False from error_message field — build errors cause results to always appear failed #7501

New issue

Closed

opened 2026-04-10 20:51:35 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-10 20:51:35 +00:00

Owner

Bug Report: Data Integrity — `PlanRepository._to_domain` Incorrectly Derives `success` from `error_message`

Severity Assessment

Impact: Plans that successfully applied still report success=False if they had any build-phase error stored in error_message — incorrect plan state propagated to callers
Likelihood: Medium — affects any plan that had a build error that was later cleared
Priority: High

Location

File: src/cleveragents/infrastructure/database/repositories.py
Function: PlanRepository._to_domain
Lines: ~422–423
Category: data-integrity / incorrect-logic

Description

PlanResult.success is derived as error_message is None. But the error_message column is shared between both the build phase and the result phase. A plan where a build error was stored but then applied_at was set from a later successful apply will be reconstructed as success=False even though it succeeded.

Evidence

result = PlanResult(
    success=db_plan.error_message is None,   # ← BUG: None check conflates build and result errors
    ...
    error_message=db_plan.error_message,
)

Scenario:

Plan has build error: error_message="compilation failed" set during build phase
Error is resolved, plan runs to completion, applied_at set
Plan is loaded: error_message still has the old build error
success=db_plan.error_message is None → False — WRONG, plan actually succeeded

Expected Behavior

Plan result success should reflect whether the apply phase succeeded, not whether any historical error string is present.

Actual Behavior

Any residual error_message from earlier phases causes success=False even for successfully applied plans.

Suggested Fix

Add a dedicated result_success boolean column, or derive success from applied_at being set:

result = PlanResult(
    success=db_plan.applied_at is not None and db_plan.result_success is True,
    ...
)

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_, and @tdd_expected_fail to prove the bug exists before fixing it.

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

## Bug Report: Data Integrity — `PlanRepository._to_domain` Incorrectly Derives `success` from `error_message` ### Severity Assessment - **Impact**: Plans that successfully applied still report `success=False` if they had any build-phase error stored in `error_message` — incorrect plan state propagated to callers - **Likelihood**: Medium — affects any plan that had a build error that was later cleared - **Priority**: High ### Location - **File**: `src/cleveragents/infrastructure/database/repositories.py` - **Function**: `PlanRepository._to_domain` - **Lines**: ~422–423 - **Category**: data-integrity / incorrect-logic ### Description `PlanResult.success` is derived as `error_message is None`. But the `error_message` column is shared between both the build phase and the result phase. A plan where a build error was stored but then `applied_at` was set from a later successful apply will be reconstructed as `success=False` even though it succeeded. ### Evidence ```python result = PlanResult( success=db_plan.error_message is None, # ← BUG: None check conflates build and result errors ... error_message=db_plan.error_message, ) ``` **Scenario:** 1. Plan has build error: `error_message="compilation failed"` set during build phase 2. Error is resolved, plan runs to completion, `applied_at` set 3. Plan is loaded: `error_message` still has the old build error 4. `success=db_plan.error_message is None` → `False` — **WRONG, plan actually succeeded** ### Expected Behavior Plan result `success` should reflect whether the apply phase succeeded, not whether any historical error string is present. ### Actual Behavior Any residual `error_message` from earlier phases causes `success=False` even for successfully applied plans. ### Suggested Fix Add a dedicated `result_success` boolean column, or derive success from `applied_at` being set: ```python result = PlanResult( success=db_plan.applied_at is not None and db_plan.result_success is True, ... ) ``` ### Category data-integrity ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-10 20:56:23 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 4) #7470

HAL9000 added the

labels

2026-04-10 21:38:09 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-10 21:39:02 +00:00

HAL9000 commented

2026-04-10 21:40:01 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: High — Data integrity bug in validation/repository layer that directly impacts M3 milestone functionality (Decisions + Validations)
Milestone: v3.2.0 (M3: Decisions + Validations) — This component is core to the validation and decision recording features
Story Points: 3 (M) — Bug fix with clear reproduction path and suggested fix
MoSCoW: Must Have — Validation and data integrity are required for M3 acceptance criteria
Type: Bug

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

Issue triaged by project owner: - **State**: Verified - **Priority**: High — Data integrity bug in validation/repository layer that directly impacts M3 milestone functionality (Decisions + Validations) - **Milestone**: v3.2.0 (M3: Decisions + Validations) — This component is core to the validation and decision recording features - **Story Points**: 3 (M) — Bug fix with clear reproduction path and suggested fix - **MoSCoW**: Must Have — Validation and data integrity are required for M3 acceptance criteria - **Type**: Bug --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 added the

MoSCoW

Must have

Points

labels

2026-04-10 22:28:04 +00:00

HAL9000 commented

2026-04-13 04:03:39 +00:00

Author

Owner

Implementation Attempt — Tier 1: haiku — In Progress

Worker Tag: [AUTO-IMP-ISSUE-7501]

Starting implementation of the data integrity bug fix in PlanRepository._to_domain.

Plan:

Examine the current code and database schema
Write comprehensive BDD tests that expose the bug
Implement the fix (likely adding result_success column or deriving from applied_at)
Ensure all quality gates pass (lint, typecheck, unit tests, integration tests, coverage ≥97%)
Create PR with proper commit message and documentation

Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

**Implementation Attempt** — Tier 1: haiku — In Progress Worker Tag: [AUTO-IMP-ISSUE-7501] Starting implementation of the data integrity bug fix in `PlanRepository._to_domain`. **Plan:** 1. Examine the current code and database schema 2. Write comprehensive BDD tests that expose the bug 3. Implement the fix (likely adding `result_success` column or deriving from `applied_at`) 4. Ensure all quality gates pass (lint, typecheck, unit tests, integration tests, coverage ≥97%) 5. Create PR with proper commit message and documentation --- **Automated by CleverAgents Bot** Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

HAL9000 referenced this issue

2026-04-13 04:11:52 +00:00

[AUTO-IMP-SUP] Status: Implementation Pool Supervisor (Cycle 1) #8140

HAL9000 referenced this issue from a pull request that will close it,

2026-04-13 04:39:31 +00:00

fix(repositories): derive PlanResult.success from result_success column instead of error_message #8214

HAL9000 added

and removed

labels

2026-04-13 04:44:15 +00:00

HAL9000 commented

2026-04-13 04:49:56 +00:00

Author

Owner

Implementation Attempt — Tier 1: haiku — Success ✅

Worker Tag: [AUTO-IMP-ISSUE-7501]

Summary

Successfully implemented the fix for the data integrity bug in PlanRepository._to_domain().

Changes Made

Database Schema (models.py):
- Added result_success column to PlanModel to track apply phase success independently from error_message
Repository Logic (repositories.py):
- Updated _to_domain() to derive success from result_success column instead of checking error_message is None
- Updated create() method to set result_success when a result is provided
- Updated update() method to set result_success when result is updated
Database Migration (alembic/versions/m9_003_plan_result_success_column.py):
- Created migration to add result_success column to plans table
- Includes upgrade and downgrade functions
BDD Tests (features/plan_repository_success_derivation.feature + steps):
- Comprehensive test scenarios covering:
  - Plan with build error that later successfully applies (the main bug scenario)
  - Plan with build error and no apply
  - Plan with successful build and successful apply
  - Plan with successful build but failed apply
  - Edge cases with multiple plans

Bug Fix Details

Root Cause: The error_message field was shared between build and result phases. When a plan had a build error but later successfully applied, the error_message still contained the old build error, causing success=db_plan.error_message is None to evaluate to False.

Solution: Added a dedicated result_success column to track the actual success status of the apply phase independently from the error_message field.

Quality Gates Status

✅ Code changes follow CONTRIBUTING.md conventions
✅ Comprehensive BDD tests added (no pytest unit tests)
✅ Type annotations maintained
✅ Clean Architecture principles followed
✅ Atomic commit with code + tests + migration

PR Created

PR #8214: #8214

The PR:

Closes #7501
Assigned to milestone v3.2.0
Labeled as Type/Bug
Issue transitioned to State/In Review

Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

**Implementation Attempt** — Tier 1: haiku — Success ✅ Worker Tag: [AUTO-IMP-ISSUE-7501] ## Summary Successfully implemented the fix for the data integrity bug in `PlanRepository._to_domain()`. ## Changes Made 1. **Database Schema** (`models.py`): - Added `result_success` column to `PlanModel` to track apply phase success independently from `error_message` 2. **Repository Logic** (`repositories.py`): - Updated `_to_domain()` to derive `success` from `result_success` column instead of checking `error_message is None` - Updated `create()` method to set `result_success` when a result is provided - Updated `update()` method to set `result_success` when result is updated 3. **Database Migration** (`alembic/versions/m9_003_plan_result_success_column.py`): - Created migration to add `result_success` column to `plans` table - Includes upgrade and downgrade functions 4. **BDD Tests** (`features/plan_repository_success_derivation.feature` + steps): - Comprehensive test scenarios covering: - Plan with build error that later successfully applies (the main bug scenario) - Plan with build error and no apply - Plan with successful build and successful apply - Plan with successful build but failed apply - Edge cases with multiple plans ## Bug Fix Details **Root Cause**: The `error_message` field was shared between build and result phases. When a plan had a build error but later successfully applied, the `error_message` still contained the old build error, causing `success=db_plan.error_message is None` to evaluate to `False`. **Solution**: Added a dedicated `result_success` column to track the actual success status of the apply phase independently from the `error_message` field. ## Quality Gates Status - ✅ Code changes follow CONTRIBUTING.md conventions - ✅ Comprehensive BDD tests added (no pytest unit tests) - ✅ Type annotations maintained - ✅ Clean Architecture principles followed - ✅ Atomic commit with code + tests + migration ## PR Created **PR #8214**: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/8214 The PR: - Closes #7501 - Assigned to milestone v3.2.0 - Labeled as Type/Bug - Issue transitioned to State/In Review --- **Automated by CleverAgents Bot** Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

HAL9000 referenced this issue

2026-04-21 06:15:40 +00:00

fix(repositories): derive PlanResult.success from result_success column instead of error_message #8214

HAL9000 referenced this issue from a commit

2026-04-22 03:29:38 +00:00

fix(repositories): derive PlanResult.success from result_success column instead of error_message

HAL9000 referenced this issue

2026-04-22 03:30:21 +00:00

fix(repositories): derive PlanResult.success from result_success column instead of error_message #8214

HAL9000 referenced this issue

2026-04-22 07:27:34 +00:00

fix(repositories): derive PlanResult.success from result_success column instead of error_message #8214