Files
placeholder/features/steps/plans_table_schema_alignment_steps.py
CoreRasurae a4a6b061a6 fix(db): align v3_plans schema with specification DDL
Aligned v3_plans table with specification DDL:

1. Added effective_profile_snapshot column (TEXT NOT NULL) for
   storing frozen JSON snapshot of automation profile at plan
   creation time.  Added Pydantic field_validator ensuring the
   value is well-formed JSON.  Validator catches RecursionError
   for deeply nested JSON, consistent with automation_profile
   deserialization hardening.  Validator error message uses
   length-only to avoid potential information disclosure.
   Documented that the default "{}" exists for backward
   compatibility; new plans should explicitly set the snapshot.

2. Made root_plan_id NOT NULL — root plans self-reference their
   own plan_id, child plans reference the root ancestor.  Added
   explicit ondelete="RESTRICT" FK policy for consistency with
   other FKs in the model.  Documented known FK policy drift
   between ORM model (RESTRICT) and migrated databases (retained
   SET NULL) in the migration; data integrity is preserved by
   the NOT NULL constraint regardless.  Moved root_plan_id
   self-reference resolution into a PlanIdentity model_validator
   so the domain model is consistent with the DB NOT NULL
   constraint before and after persistence (previously the
   resolution only happened in from_domain(), creating an
   asymmetry where root_plan_id was None in-memory but non-null
   after round-tripping through the database).

3. Made automation_profile NOT NULL with default "balanced".

4. Documented intentional deviation: phase default is "action"
   (code) vs "strategize" (spec) because the Action phase was
   added as a pre-Strategize setup step.

5. Created Alembic migration with backfill logic for existing
   rows.  Root-ancestor backfill uses level-by-level propagation
   with a parent-readiness guard to correctly resolve plans at
   arbitrary hierarchy depth (3+ levels).  Added safety bound
   (max 100 iterations) with logged error on exhaustion to guard
   against cycles in parent_plan_id.  Merged batch_alter_table
   operations to avoid redundant full-table copies in SQLite
   batch mode.  Migration backfill also handles empty-string
   automation_profile values.  Documented downgrade limitation
   (backfill is not reversible).  Orphan-row fallback now logs
   affected row count at WARNING level.  Migration cycle-detection
   now logs affected plan_id values before the orphan fallback
   overwrites them.  All migration SQL uses sa.text() for
   consistency with SQLAlchemy best practices.

6. Hardened automation_profile deserialization in to_domain() to
   catch ValueError (invalid StrEnum provenance), Pydantic
   ValidationError, and RecursionError (deeply nested JSON) in
   addition to JSONDecodeError and KeyError, preventing
   unreadable plans from corrupted DB rows.  Applied the same
   defensive deserialization pattern to effective_profile_snapshot
   in to_domain(): corrupted JSON falls back to '{}' with a
   WARNING log instead of crashing the read path.  Added TypeError
   to the effective_profile_snapshot exception list in to_domain()
   for consistency with the Pydantic validator.  Logging of
   unparseable values uses length only to avoid potential
   information disclosure.

7. Used explicit None check (is not None) instead of truthiness
   for root_plan_id resolution in from_domain(), for
   effective_profile_snapshot in to_domain(), and in
   _serialize_automation_profile() for consistency.

8. Documented intentional column naming conventions vs spec DDL
   (e.g. automation_profile vs automation_profile_name, *_actor
   vs *_actor_name, processing_state vs state, v3_plans vs
   plans).  Documented the semantic difference: automation_profile
   stores either a bare name or structured JSON with provenance,
   whereas the spec automation_profile_name stores a plain name.

9. Fixed benchmark plan constructors
   (plan_phase_migration_bench.py) that were missing the now-
   required root_plan_id and effective_profile_snapshot fields.

10. Replaced defensive getattr() with direct attribute access for
    effective_profile_snapshot in from_domain() and update(),
    since the field is now defined on the Plan domain model.

11. Fixed Any type annotation in test helper _make_plan() to use
    AutomationProfileRef | None for proper type safety.

12. Added BDD scenarios for PlanIdentity self-reference
    resolution, NULL effective_profile_snapshot constraint
    enforcement, valid-JSON-missing-profile_name-key
    deserialization, invalid-JSON and empty-string snapshot
    rejection by Pydantic validator, and corrupted
    effective_profile_snapshot DB fallback in to_domain().

13. Extracted default automation profile name to a module-level
    constant (DEFAULT_AUTOMATION_PROFILE) to reduce sentinel
    duplication across models.py and repositories.py.

14. Centralised automation-profile serialisation into
    LifecyclePlanModel._serialize_automation_profile() to
    eliminate duplication between from_domain() and
    LifecyclePlanRepository.update().

15. Fixed to_domain() root_plan_id type cast from str | None
    to str, reflecting the NOT NULL column constraint.

16. Added PlanIdentity model_validator that resolves None
    root_plan_id to plan_id at domain construction time, ensuring
    the domain model honours the spec DDL NOT NULL constraint
    regardless of persistence state.  Simplified from_domain()
    root resolution accordingly.

ISSUES CLOSED: #921
2026-03-30 23:40:36 +01:00

540 lines
19 KiB
Python

"""Step definitions for v3_plans table schema alignment (issue #921).
Tests the effective_profile_snapshot column, root_plan_id self-referencing,
automation_profile NOT NULL default, bare-string deserialization, and
NULL root_plan_id constraint enforcement.
"""
from __future__ import annotations
from datetime import UTC, datetime
from behave import given, then, when
from behave.runner import Context
from sqlalchemy import create_engine, text
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm import sessionmaker
from cleveragents.domain.models.core.action import Action, ActionState
from cleveragents.domain.models.core.plan import (
AutomationProfileProvenance,
AutomationProfileRef,
NamespacedName,
Plan,
PlanIdentity,
PlanPhase,
PlanTimestamps,
ProcessingState,
)
from cleveragents.infrastructure.database.models import (
Base,
LifecyclePlanModel,
)
from cleveragents.infrastructure.database.repositories import (
ActionRepository,
LifecyclePlanRepository,
)
_NOW = datetime(2026, 3, 19, tzinfo=UTC)
def _make_plan(
plan_id: str,
*,
parent_plan_id: str | None = None,
root_plan_id: str | None = None,
effective_profile_snapshot: str = "{}",
automation_profile: AutomationProfileRef | None = None,
action_name: str = "local/schema-test",
) -> Plan:
"""Build a Plan domain object for schema alignment tests."""
return Plan(
identity=PlanIdentity(
plan_id=plan_id,
parent_plan_id=parent_plan_id,
root_plan_id=root_plan_id,
attempt=1,
),
namespaced_name=NamespacedName(namespace="local", name="schema-test"),
action_name=action_name,
description="Schema alignment test plan",
definition_of_done="All assertions pass",
phase=PlanPhase.ACTION,
processing_state=ProcessingState.QUEUED,
automation_profile=automation_profile,
effective_profile_snapshot=effective_profile_snapshot,
strategy_actor="local/s",
execution_actor="local/e",
timestamps=PlanTimestamps(created_at=_NOW, updated_at=_NOW),
created_by="test-schema",
tags=[],
reusable=True,
read_only=False,
)
def _setup_db(context: Context) -> None:
"""Create a fresh in-memory SQLite database and attach to context."""
engine = create_engine("sqlite:///:memory:", echo=False)
Base.metadata.create_all(engine)
sm = sessionmaker(bind=engine)
session = sm()
context._sa_engine = engine
context._sa_session = session
context._sa_session_factory = lambda: session
context._sa_plan_repo = LifecyclePlanRepository(
session_factory=context._sa_session_factory,
)
context._sa_action_repo = ActionRepository(
session_factory=context._sa_session_factory,
)
@given("a schema-aligned fresh in-memory database")
def step_setup_db(context: Context) -> None:
_setup_db(context)
@given('a schema-aligned prerequisite action "{action_name}" exists')
def step_create_action(context: Context, action_name: str) -> None:
ns = NamespacedName.parse(action_name)
action = Action(
namespaced_name=ns,
description="Schema alignment test action",
definition_of_done="Done",
strategy_actor="local/s",
execution_actor="local/e",
state=ActionState.AVAILABLE,
created_at=_NOW,
updated_at=_NOW,
)
context._sa_action_repo.create(action)
context._sa_session.commit()
@given('a schema-aligned plan with ID "{plan_id}"')
def step_create_plan(context: Context, plan_id: str) -> None:
context._sa_plan = _make_plan(plan_id)
@given("the plan effective_profile_snapshot is '{snapshot}'")
def step_set_snapshot(context: Context, snapshot: str) -> None:
context._sa_plan = context._sa_plan.model_copy(
update={"effective_profile_snapshot": snapshot}
)
@given('a schema-aligned root plan with ID "{plan_id}"')
def step_create_root_plan(context: Context, plan_id: str) -> None:
# root_plan_id is None here; from_domain will set it to plan_id
context._sa_plan = _make_plan(plan_id, root_plan_id=None)
@given(
'a schema-aligned child plan with ID "{child_id}" parent "{parent_id}" root "{root_id}"'
)
def step_create_child_plan(
context: Context, child_id: str, parent_id: str, root_id: str
) -> None:
context._sa_child_plan = _make_plan(
child_id, parent_plan_id=parent_id, root_plan_id=root_id
)
@given(
'a schema-aligned grandchild plan with ID "{gc_id}" parent "{parent_id}" root "{root_id}"'
)
def step_create_grandchild_plan(
context: Context, gc_id: str, parent_id: str, root_id: str
) -> None:
context._sa_grandchild_plan = _make_plan(
gc_id, parent_plan_id=parent_id, root_plan_id=root_id
)
@given('a schema-aligned plan with ID "{plan_id}" and no automation profile')
def step_create_plan_no_profile(context: Context, plan_id: str) -> None:
context._sa_plan = _make_plan(plan_id, automation_profile=None)
@when("I persist the schema-aligned plan")
def step_persist_plan(context: Context) -> None:
context._sa_plan_repo.create(context._sa_plan)
context._sa_session.commit()
plan_id = context._sa_plan.identity.plan_id
context._sa_retrieved = context._sa_plan_repo.get(plan_id)
@when("I persist both schema-aligned plans")
def step_persist_both(context: Context) -> None:
context._sa_plan_repo.create(context._sa_plan)
context._sa_session.commit()
context._sa_plan_repo.create(context._sa_child_plan)
context._sa_session.commit()
child_id = context._sa_child_plan.identity.plan_id
context._sa_retrieved_child = context._sa_plan_repo.get(child_id)
@when("I persist root, child, and grandchild schema-aligned plans")
def step_persist_three_levels(context: Context) -> None:
context._sa_plan_repo.create(context._sa_plan)
context._sa_session.commit()
context._sa_plan_repo.create(context._sa_child_plan)
context._sa_session.commit()
context._sa_plan_repo.create(context._sa_grandchild_plan)
context._sa_session.commit()
gc_id = context._sa_grandchild_plan.identity.plan_id
context._sa_retrieved_grandchild = context._sa_plan_repo.get(gc_id)
@when("I update the plan effective_profile_snapshot to '{new_snapshot}'")
def step_update_snapshot(context: Context, new_snapshot: str) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved before update"
updated = retrieved.model_copy(update={"effective_profile_snapshot": new_snapshot})
context._sa_plan_repo.update(updated)
context._sa_session.commit()
plan_id = updated.identity.plan_id
context._sa_retrieved = context._sa_plan_repo.get(plan_id)
@then("the retrieved plan effective_profile_snapshot should be '{expected}'")
def step_check_snapshot(context: Context, expected: str) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved"
assert retrieved.effective_profile_snapshot == expected, (
f"Expected snapshot '{expected}', got '{retrieved.effective_profile_snapshot}'"
)
@then('the plan root_plan_id should equal the plan_id "{plan_id}"')
def step_check_root_self_ref(context: Context, plan_id: str) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved"
assert retrieved.identity.root_plan_id == plan_id, (
f"Expected root_plan_id '{plan_id}', got '{retrieved.identity.root_plan_id}'"
)
@then('the child plan root_plan_id should be "{expected}"')
def step_check_child_root(context: Context, expected: str) -> None:
child = context._sa_retrieved_child
assert child is not None, "Child plan was not retrieved"
assert child.identity.root_plan_id == expected, (
f"Expected root_plan_id '{expected}', got '{child.identity.root_plan_id}'"
)
@then('the grandchild plan root_plan_id should be "{expected}"')
def step_check_grandchild_root(context: Context, expected: str) -> None:
gc = context._sa_retrieved_grandchild
assert gc is not None, "Grandchild plan was not retrieved"
assert gc.identity.root_plan_id == expected, (
f"Expected root_plan_id '{expected}', got '{gc.identity.root_plan_id}'"
)
@then('the grandchild plan parent_plan_id should be "{expected}"')
def step_check_grandchild_parent(context: Context, expected: str) -> None:
gc = context._sa_retrieved_grandchild
assert gc is not None, "Grandchild plan was not retrieved"
assert gc.identity.parent_plan_id == expected, (
f"Expected parent_plan_id '{expected}', got '{gc.identity.parent_plan_id}'"
)
@then('the child plan parent_plan_id should be "{expected}"')
def step_check_child_parent(context: Context, expected: str) -> None:
child = context._sa_retrieved_child
assert child is not None, "Child plan was not retrieved"
assert child.identity.parent_plan_id == expected, (
f"Expected parent_plan_id '{expected}', got '{child.identity.parent_plan_id}'"
)
@then('the persisted automation_profile column should be "{expected}"')
def step_check_automation_profile_col(context: Context, expected: str) -> None:
plan_id = context._sa_plan.identity.plan_id
row = context._sa_session.execute(
text("SELECT automation_profile FROM v3_plans WHERE plan_id = :pid"),
{"pid": plan_id},
).fetchone()
assert row is not None, "Row not found in v3_plans"
assert row[0] == expected, (
f"Expected automation_profile='{expected}', got '{row[0]}'"
)
# --- M3: Bare-string / invalid-provenance automation_profile handling ---
@given('the plan automation_profile column is set to bare string "{value}"')
def step_set_bare_string_profile(context: Context, value: str) -> None:
"""Persist the plan, then overwrite the column with a bare string."""
context._sa_plan_repo.create(context._sa_plan)
context._sa_session.commit()
plan_id = context._sa_plan.identity.plan_id
context._sa_session.execute(
text("UPDATE v3_plans SET automation_profile = :val WHERE plan_id = :pid"),
{"val": value, "pid": plan_id},
)
context._sa_session.commit()
context._sa_target_plan_id = plan_id
@given("the plan automation_profile column is set to '{value}'")
def step_set_invalid_json_profile(context: Context, value: str) -> None:
"""Persist the plan, then overwrite the column with arbitrary JSON."""
context._sa_plan_repo.create(context._sa_plan)
context._sa_session.commit()
plan_id = context._sa_plan.identity.plan_id
context._sa_session.execute(
text("UPDATE v3_plans SET automation_profile = :val WHERE plan_id = :pid"),
{"val": value, "pid": plan_id},
)
context._sa_session.commit()
context._sa_target_plan_id = plan_id
@when("I load the plan from the database")
def step_load_plan(context: Context) -> None:
plan_id = context._sa_target_plan_id
context._sa_retrieved = context._sa_plan_repo.get(plan_id)
@then("the plan automation_profile ref should be None")
def step_check_profile_is_none(context: Context) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved"
assert retrieved.automation_profile is None, (
f"Expected automation_profile=None, got {retrieved.automation_profile!r}"
)
# --- L3: NULL root_plan_id constraint enforcement ---
@given('a schema-aligned plan with ID "{plan_id}" forced NULL root_plan_id')
def step_create_plan_null_root(context: Context, plan_id: str) -> None:
context._sa_null_root_plan_id = plan_id
@when("I attempt to persist the plan with NULL root_plan_id")
def step_persist_null_root(context: Context) -> None:
plan_id = context._sa_null_root_plan_id
now_iso = _NOW.isoformat()
try:
context._sa_session.execute(
text(
"INSERT INTO v3_plans "
"(plan_id, root_plan_id, action_name, namespaced_name, "
"namespace, phase, processing_state, attempt, description, "
"tags_json, effective_profile_snapshot, automation_profile, "
"reusable, read_only, created_at, updated_at) "
"VALUES (:pid, NULL, :act, :ns, 'local', 'action', 'queued', "
"1, 'test', '[]', '{}', 'balanced', 1, 0, :now, :now)"
),
{
"pid": plan_id,
"act": "local/schema-test",
"ns": "local/schema-test",
"now": now_iso,
},
)
context._sa_session.commit()
context._sa_integrity_error = None
except IntegrityError as exc:
context._sa_session.rollback()
context._sa_integrity_error = exc
@then("the database should reject the insert with an integrity error")
def step_check_integrity_error(context: Context) -> None:
assert context._sa_integrity_error is not None, (
"Expected IntegrityError for NULL root_plan_id, but insert succeeded"
)
# --- T2: AutomationProfileRef round-trip ---
@given(
'a schema-aligned plan "{plan_id}" with profile "{profile}" and provenance "{provenance}"'
)
def step_create_plan_with_profile(
context: Context, plan_id: str, profile: str, provenance: str
) -> None:
ref = AutomationProfileRef(
profile_name=profile,
provenance=AutomationProfileProvenance(provenance),
)
context._sa_plan = _make_plan(plan_id, automation_profile=ref)
@then('the plan automation_profile name should be "{expected}"')
def step_check_profile_name(context: Context, expected: str) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved"
assert retrieved.automation_profile is not None, (
"Expected automation_profile to be set, got None"
)
assert retrieved.automation_profile.profile_name == expected, (
f"Expected profile_name='{expected}', "
f"got '{retrieved.automation_profile.profile_name}'"
)
@then('the plan automation_profile provenance should be "{expected}"')
def step_check_profile_provenance(context: Context, expected: str) -> None:
retrieved = context._sa_retrieved
assert retrieved is not None, "Plan was not retrieved"
assert retrieved.automation_profile is not None, (
"Expected automation_profile to be set, got None"
)
assert retrieved.automation_profile.provenance.value == expected, (
f"Expected provenance='{expected}', "
f"got '{retrieved.automation_profile.provenance.value}'"
)
# --- TC5: NULL effective_profile_snapshot constraint enforcement ---
@given(
'a schema-aligned plan with ID "{plan_id}" forced NULL effective_profile_snapshot'
)
def step_create_plan_null_snapshot(context: Context, plan_id: str) -> None:
context._sa_null_snapshot_plan_id = plan_id
@when("I attempt to persist the plan with NULL effective_profile_snapshot")
def step_persist_null_snapshot(context: Context) -> None:
plan_id = context._sa_null_snapshot_plan_id
now_iso = _NOW.isoformat()
try:
context._sa_session.execute(
text(
"INSERT INTO v3_plans "
"(plan_id, root_plan_id, action_name, namespaced_name, "
"namespace, phase, processing_state, attempt, description, "
"tags_json, effective_profile_snapshot, automation_profile, "
"reusable, read_only, created_at, updated_at) "
"VALUES (:pid, :pid, :act, :ns, 'local', 'action', 'queued', "
"1, 'test', '[]', NULL, 'balanced', 1, 0, :now, :now)"
),
{
"pid": plan_id,
"act": "local/schema-test",
"ns": "local/schema-test",
"now": now_iso,
},
)
context._sa_session.commit()
context._sa_snapshot_integrity_error = None
except IntegrityError as exc:
context._sa_session.rollback()
context._sa_snapshot_integrity_error = exc
@then("the database should reject the snapshot insert with an integrity error")
def step_check_snapshot_integrity_error(context: Context) -> None:
assert context._sa_snapshot_integrity_error is not None, (
"Expected IntegrityError for NULL effective_profile_snapshot, "
"but insert succeeded"
)
# --- TC4: PlanIdentity model_validator self-reference resolution ---
@given('a schema-aligned plan with ID "{plan_id}" and None root_plan_id')
def step_create_plan_none_root(context: Context, plan_id: str) -> None:
context._sa_plan = _make_plan(plan_id, root_plan_id=None)
@then('the domain plan root_plan_id should already equal "{expected}"')
def step_check_domain_root_resolved(context: Context, expected: str) -> None:
plan = context._sa_plan
assert plan is not None, "Plan was not created"
assert plan.identity.root_plan_id == expected, (
f"Expected PlanIdentity.root_plan_id='{expected}' after model_validator, "
f"got '{plan.identity.root_plan_id}'"
)
@when("I convert the plan via from_domain")
def step_convert_from_domain(context: Context) -> None:
context._sa_orm_model = LifecyclePlanModel.from_domain(context._sa_plan)
@then('the ORM model root_plan_id should equal "{expected}"')
def step_check_orm_root(context: Context, expected: str) -> None:
model = context._sa_orm_model
assert model is not None, "ORM model was not created"
assert model.root_plan_id == expected, (
f"Expected ORM root_plan_id='{expected}', got '{model.root_plan_id}'"
)
# --- TC6: Invalid JSON in effective_profile_snapshot ---
@when('the effective_profile_snapshot column is corrupted to "{bad_json}"')
def step_corrupt_snapshot_column(context: Context, bad_json: str) -> None:
"""Overwrite the effective_profile_snapshot column with invalid JSON."""
plan_id = context._sa_plan.identity.plan_id
context._sa_session.execute(
text(
"UPDATE v3_plans SET effective_profile_snapshot = :val WHERE plan_id = :pid"
),
{"val": bad_json, "pid": plan_id},
)
context._sa_session.commit()
@when("I reload the plan from the database")
def step_reload_plan(context: Context) -> None:
"""Expire cached ORM state and reload from DB to pick up raw SQL changes."""
context._sa_session.expire_all()
plan_id = context._sa_plan.identity.plan_id
context._sa_retrieved = context._sa_plan_repo.get(plan_id)
@when('I attempt to construct a Plan with effective_profile_snapshot "{bad_json}"')
def step_construct_plan_bad_snapshot(context: Context, bad_json: str) -> None:
from pydantic import ValidationError as PydanticValidationError
try:
_make_plan(
"01HV000000000000000000SA21",
effective_profile_snapshot=bad_json,
)
context._sa_validation_error = None
except PydanticValidationError as exc:
context._sa_validation_error = exc
@when("I attempt to construct a Plan with an empty effective_profile_snapshot")
def step_construct_plan_empty_snapshot(context: Context) -> None:
from pydantic import ValidationError as PydanticValidationError
try:
_make_plan(
"01HV000000000000000000SA22",
effective_profile_snapshot="",
)
context._sa_validation_error = None
except PydanticValidationError as exc:
context._sa_validation_error = exc
@then(
"the Plan construction should raise a validation error "
"for effective_profile_snapshot"
)
def step_check_validation_error(context: Context) -> None:
assert context._sa_validation_error is not None, (
"Expected a ValidationError for invalid JSON in "
"effective_profile_snapshot, but Plan was created successfully"
)