cleveragents/cleveragents-core

Fork 3

bug(plan): plan_phase_migration raw SQL test passes for wrong reason — NOT NULL violation masks CHECK constraint test #9411

New issue

Closed

opened 2026-04-14 16:54:39 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-14 16:54:39 +00:00

Owner

Metadata

Commit Message: fix(test): add root_plan_id to raw SQL in plan_phase_migration constraint tests
Branch: fix/plan-phase-migration-raw-sql-root-plan-id

Background and Context

The features/plan_phase_migration.feature BDD feature tests the database CHECK constraint that rejects 'applied' as a phase value in the v3_plans table. The migration a5_005_rebaseline_plan_phases.py correctly defines:

CHECK (phase IN ('action', 'strategize', 'execute', 'apply'))

The test step step_try_insert_plan_with_phase_and_state in features/steps/plan_phase_migration_steps.py uses raw SQL to insert a plan with an invalid phase value. However, the raw SQL omits the root_plan_id column, which is defined as NOT NULL in the LifecyclePlanModel (and in the schema created by Base.metadata.create_all).

Current Behavior

The raw SQL insert:

INSERT INTO v3_plans 
(plan_id, action_name, namespaced_name, namespace, 
phase, processing_state, description, tags_json, 
created_at, updated_at) 
VALUES (:pid, :aname, :nname, :ns, :phase, :state, 
:desc, :tags, :cat, :uat)

fails with a NOT NULL constraint failed: v3_plans.root_plan_id error — not a CHECK constraint violation on the phase column. The test assertion only checks that the insert failed (context.phase_rebaseline_result == "error"), so it passes, but for the wrong reason.

The CHECK constraint for phase values (ck_v3_plans_phase) is never actually exercised by these test scenarios.

Expected Behavior

The raw SQL insert should include root_plan_id (set to the same value as plan_id for a root plan) so that the insert fails specifically due to the CHECK constraint violation on the phase column, not due to a missing NOT NULL field.

The test should verify that the CHECK constraint is the cause of the failure, not a NOT NULL constraint on an unrelated column.

Acceptance Criteria

step_try_insert_plan_with_phase_and_state includes root_plan_id in the raw SQL INSERT
The scenarios "Phase constraint rejects 'applied' as a phase" and "Phase constraint rejects legacy 'applied' phase value" fail specifically due to CHECK constraint violations, not NOT NULL violations
All plan_phase_migration.feature scenarios pass
Test coverage >= 97%

Supporting Information

Affected file: features/steps/plan_phase_migration_steps.py, function step_try_insert_plan_with_phase_and_state

Affected scenarios:

"Phase constraint rejects 'applied' as a phase"
"Phase constraint rejects legacy 'applied' phase value"

Root cause: The raw SQL in step_try_insert_plan_with_phase_and_state omits root_plan_id which is NOT NULL in the schema. SQLite evaluates NOT NULL constraints before CHECK constraints, so the insert fails for the wrong reason.

Fix: Add root_plan_id to the raw SQL INSERT statement:

session.execute(
    text(
        "INSERT INTO v3_plans "
        "(plan_id, root_plan_id, action_name, namespaced_name, namespace, "
        "phase, processing_state, description, tags_json, "
        "created_at, updated_at) "
        "VALUES (:pid, :pid, :aname, :nname, :ns, :phase, :state, "
        ":desc, :tags, :cat, :uat)"
    ),
    ...
)

Subtasks

Add root_plan_id to the raw SQL INSERT in step_try_insert_plan_with_phase_and_state
Verify the scenarios now fail due to CHECK constraint violation (not NOT NULL)
Run python -m behave features/plan_phase_migration.feature and confirm all scenarios pass
Verify coverage >= 97%

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
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.

Automated by CleverAgents Bot
Supervisor: UAT Test Pool | Agent: uat-test-pool-supervisor

## Metadata - **Commit Message**: `fix(test): add root_plan_id to raw SQL in plan_phase_migration constraint tests` - **Branch**: `fix/plan-phase-migration-raw-sql-root-plan-id` ## Background and Context The `features/plan_phase_migration.feature` BDD feature tests the database CHECK constraint that rejects `'applied'` as a phase value in the `v3_plans` table. The migration `a5_005_rebaseline_plan_phases.py` correctly defines: ```sql CHECK (phase IN ('action', 'strategize', 'execute', 'apply')) ``` The test step `step_try_insert_plan_with_phase_and_state` in `features/steps/plan_phase_migration_steps.py` uses raw SQL to insert a plan with an invalid phase value. However, the raw SQL omits the `root_plan_id` column, which is defined as `NOT NULL` in the `LifecyclePlanModel` (and in the schema created by `Base.metadata.create_all`). ## Current Behavior The raw SQL insert: ```sql INSERT INTO v3_plans (plan_id, action_name, namespaced_name, namespace, phase, processing_state, description, tags_json, created_at, updated_at) VALUES (:pid, :aname, :nname, :ns, :phase, :state, :desc, :tags, :cat, :uat) ``` fails with a `NOT NULL constraint failed: v3_plans.root_plan_id` error — **not** a CHECK constraint violation on the `phase` column. The test assertion only checks that the insert failed (`context.phase_rebaseline_result == "error"`), so it passes, but for the wrong reason. The CHECK constraint for phase values (`ck_v3_plans_phase`) is never actually exercised by these test scenarios. ## Expected Behavior The raw SQL insert should include `root_plan_id` (set to the same value as `plan_id` for a root plan) so that the insert fails specifically due to the CHECK constraint violation on the `phase` column, not due to a missing NOT NULL field. The test should verify that the CHECK constraint is the cause of the failure, not a NOT NULL constraint on an unrelated column. ## Acceptance Criteria - [ ] `step_try_insert_plan_with_phase_and_state` includes `root_plan_id` in the raw SQL INSERT - [ ] The scenarios "Phase constraint rejects 'applied' as a phase" and "Phase constraint rejects legacy 'applied' phase value" fail specifically due to CHECK constraint violations, not NOT NULL violations - [ ] All `plan_phase_migration.feature` scenarios pass - [ ] Test coverage >= 97% ## Supporting Information **Affected file**: `features/steps/plan_phase_migration_steps.py`, function `step_try_insert_plan_with_phase_and_state` **Affected scenarios**: - "Phase constraint rejects 'applied' as a phase" - "Phase constraint rejects legacy 'applied' phase value" **Root cause**: The raw SQL in `step_try_insert_plan_with_phase_and_state` omits `root_plan_id` which is `NOT NULL` in the schema. SQLite evaluates NOT NULL constraints before CHECK constraints, so the insert fails for the wrong reason. **Fix**: Add `root_plan_id` to the raw SQL INSERT statement: ```python session.execute( text( "INSERT INTO v3_plans " "(plan_id, root_plan_id, action_name, namespaced_name, namespace, " "phase, processing_state, description, tags_json, " "created_at, updated_at) " "VALUES (:pid, :pid, :aname, :nname, :ns, :phase, :state, " ":desc, :tags, :cat, :uat)" ), ... ) ``` ## Subtasks - [ ] Add `root_plan_id` to the raw SQL INSERT in `step_try_insert_plan_with_phase_and_state` - [ ] Verify the scenarios now fail due to CHECK constraint violation (not NOT NULL) - [ ] Run `python -m behave features/plan_phase_migration.feature` and confirm all scenarios pass - [ ] Verify coverage >= 97% ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - 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. --- **Automated by CleverAgents Bot** Supervisor: UAT Test Pool | Agent: uat-test-pool-supervisor

HAL9000 referenced this issue

2026-04-14 17:31:28 +00:00

[AUTO-PROJ-OWN] Status: Project Owner Pool Supervisor (Cycle 8) #9423

HAL9000 added the

labels

2026-04-14 17:38:59 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-14 17:39:00 +00:00

HAL9000 commented

2026-04-14 17:39:00 +00:00

Author

Owner

✅ Triage Decision [AUTO-OWNR-1]: Verified as a valid test quality bug. The plan_phase_migration raw SQL test passes for the wrong reason — a NOT NULL violation masks the intended CHECK constraint test. This is a Should Have fix to ensure test correctness and prevent false confidence in constraint enforcement.

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

✅ **Triage Decision [AUTO-OWNR-1]**: Verified as a valid test quality bug. The `plan_phase_migration` raw SQL test passes for the wrong reason — a NOT NULL violation masks the intended CHECK constraint test. This is a `Should Have` fix to ensure test correctness and prevent false confidence in constraint enforcement. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue from a commit

2026-04-15 16:38:54 +00:00

fix(test): add root_plan_id to raw SQL in plan_phase_migration constraint tests

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

2026-04-15 16:41:04 +00:00

fix(test): add root_plan_id to raw SQL in plan_phase_migration constraint tests #9825

HAL9000 added

and removed