Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 674f4e7339 |
@@ -79,6 +79,8 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
removal. Mocked existing steps to allow remaining V2 features to be
|
||||
covered/tested.
|
||||
|
||||
- **Data integrity fix: remove silent argument swap in ValidationAttachmentRepository.attach** (PR #8177): Fixed a critical data integrity bug where `validation_name` and `resource_id` arguments were being silently swapped based on the fragile heuristic `( "/" in resource_id and "/" not in validation_name )`. This caused silent data corruption without any error or warning. The 2-line conditional swap block has been removed, ensuring arguments flow directly from caller to the persistence layer in their declared order. Added comprehensive BDD test suite (11 scenarios) verifying argument preservation across all boundary conditions including slash-containing IDs, namespacing, optional parameters, and duplicate rejection.
|
||||
|
||||
- **TUI Prompt Symbol Mode Awareness** (#6431): The prompt widget now displays a
|
||||
mode-dependent symbol (`❯` normal, `/` command, `$` shell, `☰` multi-line),
|
||||
implemented via `_PromptSymbolMixin` and `InputMode.MULTILINE`. The widget uses
|
||||
|
||||
@@ -36,6 +36,7 @@ Below are some of the specific details of various contributions.
|
||||
* HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration.
|
||||
* HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch.
|
||||
* HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files.
|
||||
* HAL 9000 has contributed the ValidationAttachmentRepository data-integrity fix (PR #8177 / issue #7492): removed the silent argument swap in `ValidationAttachmentRepository.attach()` where `validation_name` and `resource_id` were being swapped based on a fragile `/` heuristic, eliminating data corruption without any error. Added comprehensive BDD test suite (11 scenarios) verifying argument preservation across all boundary conditions including slash-containing IDs, namespacing, optional parameters, and duplicate rejection.
|
||||
* HAL 9000 has contributed the error-suppression removal fix (PR #9247 / issue #9060): removed both `try...except Exception:` blocks in `register_registry_agents()` that silently suppressed errors from `actor_registry.list_actors()` and the route bridge refresh, enabling exceptions to propagate per CONTRIBUTING.md fail-fast policy. Added three Behave scenarios verifying RuntimeError, AttributeError, and TypeError propagation.
|
||||
* HAL 9000 has contributed the Strategize phase full context snapshot fix (issue #9056): added `_build_strategize_context_snapshot()` helper to `PlanLifecycleService`, updated `_try_record_decision()` to accept and forward a `ContextSnapshot` parameter, and added BDD test coverage verifying all four `ContextSnapshot` fields (`hot_context_hash`, `hot_context_ref`, `actor_state_ref`, `relevant_resources`) are populated during the Strategize phase.
|
||||
* HAL 9000 has contributed the ACMS context path matching fix (PR #10975 / issue #10972): corrects `_path_matches()` and `_matches_pattern()` to properly match absolute fragment paths against relative glob patterns by auto-prefixing with `**/` before calling `PurePath.full_match()`, preventing silent inefficacy of include/exclude filters for absolute paths in fragment metadata.
|
||||
|
||||
@@ -0,0 +1,288 @@
|
||||
"""Step definitions for TDD issue #7492: ValidationAttachmentRepository attach argument swap fix (PR #8177).
|
||||
|
||||
Tests that validation_name and resource_id are preserved in their correct order
|
||||
without any silent heuristic-based swapping.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from cleveragents.infrastructure.database.models import Base
|
||||
from cleveragents.infrastructure.database.repositories import (
|
||||
DuplicateValidationAttachmentError,
|
||||
ValidationAttachmentRepository,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_in_memory_repo(context: Context) -> None:
|
||||
"""Set up an in-memory SQLite database with the validation attachment table."""
|
||||
engine = create_engine("sqlite:///:memory:", echo=False)
|
||||
Base.metadata.create_all(engine)
|
||||
factory = sessionmaker(bind=engine, expire_on_commit=False)
|
||||
context._va_engine = engine
|
||||
# Keep a persistent session so all operations share the same connection
|
||||
session = factory()
|
||||
context._va_session = session
|
||||
context.attachment_repo = ValidationAttachmentRepository(
|
||||
session_factory=lambda: context._va_session,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Given steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a database engine with validation_attachments table schema")
|
||||
def step_given_db_engine(context: Context) -> None:
|
||||
"""Create an in-memory SQLite DB with all CleverAgents tables."""
|
||||
_make_in_memory_repo(context)
|
||||
|
||||
|
||||
@given("a validation attachment repository with session factory")
|
||||
def step_given_repo(context: Context) -> None:
|
||||
"""Ensure the repo instance is available on context."""
|
||||
if not hasattr(context, "attachment_repo"):
|
||||
_make_in_memory_repo(context)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Attach helper — shared logic for scenarios
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _attach_validation(
|
||||
context: Context,
|
||||
validation_name: str,
|
||||
resource_id: str,
|
||||
mode: str = "required",
|
||||
project_name: str | None = None,
|
||||
plan_id: str | None = None,
|
||||
args: dict | None = None,
|
||||
) -> dict:
|
||||
"""Call attach() and store the result."""
|
||||
if hasattr(context, "_attach_result"):
|
||||
del context._attach_result
|
||||
try:
|
||||
result = context.attachment_repo.attach(
|
||||
validation_name=validation_name,
|
||||
resource_id=resource_id,
|
||||
mode=mode,
|
||||
project_name=project_name,
|
||||
plan_id=plan_id,
|
||||
args=args,
|
||||
)
|
||||
context._attach_result = result # type: ignore[attr-defined]
|
||||
except Exception as exc:
|
||||
context._attach_exception = exc # type: ignore[attr-defined]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# When steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when(
|
||||
'I attach the validation to the resource in mode "{mode}"',
|
||||
)
|
||||
def step_when_attach_basic(context: Context, mode: str) -> None:
|
||||
"""Attach with provided validation_name and resource_id."""
|
||||
vn = getattr(context, "_vn", "unknown")
|
||||
rid = getattr(context, "_rid", "")
|
||||
_attach_validation(context, vn, rid, mode=mode)
|
||||
|
||||
|
||||
@when(
|
||||
'I attach the validation to the resource in mode "{mode}" with optional params',
|
||||
)
|
||||
def step_when_attach_with_optional_params(context: Context, mode: str = "required") -> None:
|
||||
"""Attach with optional project_name and plan_id."""
|
||||
vn = getattr(context, "_vn", "unknown")
|
||||
rid = getattr(context, "_rid", "")
|
||||
pn = getattr(context, "_pn", None)
|
||||
pid = getattr(context, "_pid", None)
|
||||
_attach_validation(context, vn, rid, mode=mode, project_name=pn, plan_id=pid)
|
||||
|
||||
|
||||
@when("I attach the validation to the resource with optional args")
|
||||
def step_when_attach_with_args(context: Context) -> None:
|
||||
"""Attach including an args dict."""
|
||||
vn = getattr(context, "_vn", "unknown")
|
||||
rid = getattr(context, "_rid", "")
|
||||
pn = getattr(context, "_pn", None)
|
||||
pid = getattr(context, "_pid", None)
|
||||
args = getattr(context, "_args", {})
|
||||
_attach_validation(
|
||||
context, vn, rid, mode="required", project_name=pn, plan_id=pid, args=args
|
||||
)
|
||||
|
||||
|
||||
@when("I attach the validation with all optional parameters")
|
||||
def step_when_attach_all_optional(context: Context) -> None:
|
||||
"""Attach with mode, project_name, plan_id, and args."""
|
||||
vn = getattr(context, "_vn", "unknown")
|
||||
rid = getattr(context, "_rid", "")
|
||||
pn = getattr(context, "_pn", None)
|
||||
pid = getattr(context, "_pid", None)
|
||||
args = getattr(context, "_args", {})
|
||||
mode = getattr(context, "_mode", "required")
|
||||
_attach_validation(
|
||||
context, vn, rid, mode=mode, project_name=pn, plan_id=pid, args=args
|
||||
)
|
||||
|
||||
|
||||
@when("I attach the validation to the resource successfully once")
|
||||
def step_when_attach_first_time(context: Context) -> None:
|
||||
"""First attachment — expected to succeed."""
|
||||
_attach_validation(
|
||||
context,
|
||||
getattr(context, "_vn", ""),
|
||||
getattr(context, "_rid", ""),
|
||||
mode=getattr(context, "_mode", "required"),
|
||||
project_name=getattr(context, "_pn", None),
|
||||
plan_id=getattr(context, "_pid", None),
|
||||
)
|
||||
|
||||
|
||||
@when("I attempt a second attachment with the same parameters")
|
||||
def step_when_attach_duplicate(context: Context) -> None:
|
||||
"""Duplicate attachment — expected to raise DuplicateValidationAttachmentError."""
|
||||
try:
|
||||
_attach_validation(
|
||||
context,
|
||||
getattr(context, "_vn", ""),
|
||||
getattr(context, "_rid", ""),
|
||||
mode=getattr(context, "_mode", "required"),
|
||||
project_name=getattr(context, "_pn", None),
|
||||
plan_id=getattr(context, "_pid", None),
|
||||
)
|
||||
except DuplicateValidationAttachmentError as exc:
|
||||
context._attach_exception = exc # type: ignore[attr-defined]
|
||||
|
||||
|
||||
@when('I attach with project_name "{pn}" and plan_id "{pid}"')
|
||||
def step_when_attach_project_plan(context: Context, pn: str, pid: str) -> None:
|
||||
"""Attach with specific project_name and plan_id."""
|
||||
_attach_validation(
|
||||
context,
|
||||
getattr(context, "_vn", ""),
|
||||
getattr(context, "_rid", ""),
|
||||
mode=getattr(context, "_mode", "required"),
|
||||
project_name=pn,
|
||||
plan_id=pid,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Given steps for parameter setup
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given('a validation name "{vn}" and resource_id "{rid}"')
|
||||
def step_given_params(context: Context, vn: str, rid: str) -> None:
|
||||
"""Store the validation name and resource_id on context."""
|
||||
context._vn = vn # type: ignore[attr-defined]
|
||||
context._rid = rid # type: ignore[attr-defined]
|
||||
|
||||
|
||||
@given('optional scope parameters project_name "{pn}" and plan_id "{pid}"')
|
||||
def step_given_optional_scope(context: Context, pn: str, pid: str) -> None:
|
||||
"""Set optional project name and plan id."""
|
||||
context._pn = pn # type: ignore[attr-defined]
|
||||
context._pid = pid # type: ignore[attr-defined]
|
||||
|
||||
|
||||
@given(
|
||||
'an args dict with keys "{key1}" (value "{val1}") and "{key2}" (value {val2})',
|
||||
)
|
||||
def step_given_args_dict_string(context: Context, key1: str, val1: str, key2: str, val2: int | str) -> None:
|
||||
"""Create a simple args dict from two keys."""
|
||||
context._args = {key1: val1, key2: val2} # type: ignore[attr-defined]
|
||||
|
||||
|
||||
@given(
|
||||
'an args dict with key "{key}" (value {val}), project_name "{pn}", plan_id "{pid}", mode "{mode}"',
|
||||
)
|
||||
def step_given_all_optional_params(
|
||||
context: Context, key: str, val: float, pn: str, pid: str, mode: str
|
||||
) -> None:
|
||||
"""Set all optional parameters."""
|
||||
context._args = {key: val} # type: ignore[attr-defined]
|
||||
context._pn = pn # type: ignore[attr-defined]
|
||||
context._pid = pid # type: ignore[attr-defined]
|
||||
context._mode = mode # type: ignore[attr-defined]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Then steps
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then(
|
||||
'the attachment should have validation_name exactly "{exp_vn}" and resource_id exactly "{exp_rid}"',
|
||||
)
|
||||
def step_then_preserve_args(context: Context, exp_vn: str, exp_rid: str) -> None:
|
||||
"""Verify arguments are stored in correct order — not swapped."""
|
||||
assert (
|
||||
hasattr(context, "_attach_result") # type: ignore[arg-type]
|
||||
), "Expected attachment to succeed but no result found"
|
||||
result = context._attach_result # type: ignore[attr-defined]
|
||||
assert result["validation_name"] == exp_vn, (
|
||||
f"validation_name mismatch: expected {exp_vn!r}, got {result['validation_name']!r}"
|
||||
)
|
||||
assert result["resource_id"] == exp_rid, (
|
||||
f"resource_id mismatch: expected {exp_rid!r}, got {result['resource_id']!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the attachment should have validation_name exactly " "{exp_vn}")
|
||||
def step_then_preserve_validation_name(context: Context, exp_vn: str) -> None:
|
||||
"""Verify validation_name is correct."""
|
||||
assert (
|
||||
hasattr(context, "_attach_result") # type: ignore[arg-type]
|
||||
), "Expected attachment to succeed"
|
||||
result = context._attach_result # type: ignore[attr-defined]
|
||||
assert result["validation_name"] == exp_vn, (
|
||||
f"validation_name mismatch: expected {exp_vn!r}, got {result['validation_name']!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the attachment_args_json should contain serialized args json")
|
||||
def step_then_args_serialized(context: Context) -> None:
|
||||
"""Verify args were serialized correctly."""
|
||||
assert (
|
||||
hasattr(context, "_attach_result") # type: ignore[arg-type]
|
||||
), "Expected attachment to succeed"
|
||||
result = context._attach_result # type: ignore[attr-defined]
|
||||
stored_args = result.get("args_json", "") or "{}"
|
||||
assert "scope" in stored_args, "Expected 'scope' key in serialized args"
|
||||
|
||||
|
||||
@then("a DuplicateValidationAttachmentError should be raised")
|
||||
def step_then_duplicate_error(context: Context) -> None:
|
||||
"""Verify duplicate attachment was rejected."""
|
||||
expected_exc_type = hasattr(context, "_attach_exception") # type: ignore[arg-type]
|
||||
ctx_exc = getattr(context, "_attach_exception", None)
|
||||
assert ctx_exc is not None, "Expected DuplicateValidationAttachmentError but no exception raised"
|
||||
assert isinstance(
|
||||
ctx_exc, DuplicateValidationAttachmentError
|
||||
), f"Expected DuplicateValidationAttachmentError, got {type(ctx_exc).__name__}"
|
||||
|
||||
|
||||
@then("both attachments should succeed independently")
|
||||
def step_then_both_succeed(context: Context) -> None:
|
||||
"""Verify both attachments were successful."""
|
||||
has_result = hasattr(context, "_attach_result") # type: ignore[arg-type]
|
||||
assert has_result, "Expected second attachment to succeed but no result found"
|
||||
@@ -0,0 +1,82 @@
|
||||
@tdd_issue @tdd_issue_8177 @phase1 @domain @repository @validation
|
||||
Feature: ValidationAttachmentRepository argument order preservation (Issue #8177)
|
||||
|
||||
As a system operator attaching validations to resources
|
||||
I want validation_name and resource_id to be stored in the correct order
|
||||
So that data integrity is preserved without silent corruption
|
||||
|
||||
Background:
|
||||
Given a database engine with "validation_attachments" table schema
|
||||
And a validation attachment repository with session factory
|
||||
|
||||
@validation_attach_basic @preserve_args
|
||||
Scenario: Simple names with slash in resource_id - no swap occurs after fix
|
||||
Given a validation name "code-review" and resource_id "file:///project/main.py"
|
||||
When I attach the validation to the resource in mode "required"
|
||||
Then the attachment should have validation_name exactly "code-review" and resource_id exactly "file:///project/main.py"
|
||||
|
||||
@validation_attach_basic @preserve_args
|
||||
Scenario: Both contain slashes - arguments preserved unchanged
|
||||
Given a validation name "scope/lint-check" and resource_id "file:///project/src/module.py"
|
||||
When I attach the validation to the resource in mode "informational"
|
||||
Then the attachment should have validation_name exactly "scope/lint-check" and resource_id exactly "file:///project/src/module.py"
|
||||
|
||||
@validation_attach_basic @preserve_args
|
||||
Scenario: Neither contains slashes - arguments preserved unchanged
|
||||
Given a validation name "check-style" and resource_id "simple-resource-12345"
|
||||
When I attach the validation to the resource in mode "required"
|
||||
Then the attachment should have validation_name exactly "check-style" and resource_id exactly "simple-resource-12345"
|
||||
|
||||
@validation_attach_basic @preserve_args
|
||||
Scenario: Only validation_name contains slash - no swap occurs after fix
|
||||
Given a validation name "security/review" and resource_id "simple-resource-id"
|
||||
When I attach the validation to the resource in mode "required"
|
||||
Then the attachment should have validation_name exactly "security/review" and resource_id exactly "simple-resource-id"
|
||||
|
||||
@validation_attach_optional_params @preserve_args
|
||||
Scenario: Optional project_name and plan_id preserved with no swap after fix
|
||||
Given a validation name "code-review" and resource_id "file:///project/main.py"
|
||||
And optional scope parameters project_name "main-project" and plan_id "plan-abc-def-"
|
||||
When I attach the validation to the resource in mode "required" with optional params
|
||||
Then the attachment should have validation_name exactly "code-review" and resource_id exactly "file:///project/main.py"
|
||||
|
||||
@validation_attach_optional_params @preserve_args
|
||||
Scenario: Args dict serialized correctly without swap after fix
|
||||
Given a validation name "check-permissions" and resource_id "file:///secure/endpoint.api"
|
||||
And an args dict with keys "scope" (value "write") and "enforce_level" (value 3)
|
||||
When I attach the validation to the resource with optional args
|
||||
Then the attachment_args_json should contain serialized args json
|
||||
And the attachment should have validation_name exactly "check-permissions"
|
||||
|
||||
@validation_attach_optional_params @preserve_args
|
||||
Scenario: All optional parameters together preserve argument order after fix
|
||||
Given a validation name "scope/integrity" and resource_id "file:///data/pipeline.step"
|
||||
And an args dict with key "threshold" (value 0.95), project_name "etl", plan_id "plan-xyz-", mode "informational"
|
||||
When I attach the validation with all optional parameters
|
||||
Then the attachment should preserve validation_name exactly "scope/integrity" and resource_id exactly "file:///data/pipeline.step"
|
||||
|
||||
@validation_attach_duplicate
|
||||
Scenario: Duplicate attachments rejected without swap interference after fix
|
||||
Given a validation name "duplicate-check" and resource_id "file:///dup/resource.py"
|
||||
When I attach the validation to the resource successfully once
|
||||
And I attempt a second attachment with the same parameters
|
||||
Then a DuplicateValidationAttachmentError should be raised
|
||||
|
||||
@validation_attach_duplicate
|
||||
Scenario: Different plan scopes create separate non-duplicate attachments after fix
|
||||
Given a validation name "plan-scoped" and resource_id "file:///shared/resource.txt"
|
||||
When I attach with project_name "proj-a" and plan_id "plan-01"
|
||||
And I attach again with project_name "proj-a" and plan_id "plan-02"
|
||||
Then both attachments should succeed independently
|
||||
|
||||
@validation_attach_edge_cases @preserve_args
|
||||
Scenario: Slash-containing resource_id does not trigger swap after fix
|
||||
Given a validation name "final-check" and resource_id "repo/org/project/file.module:class"
|
||||
When I attach the validation in mode "required"
|
||||
Then the attachment should have validation_name exactly "final-check" and resource_id exactly "repo/org/project/file.module:class"
|
||||
|
||||
@validation_attach_edge_cases @preserve_args
|
||||
Scenario: Multiple slashes in both parameters preserve original values after fix
|
||||
Given a validation name "a/b/c/d" and resource_id "x/y/z/w/v"
|
||||
When I attach the validation in mode "informational"
|
||||
Then the attachment should have validation_name exactly "a/b/c/d" and resource_id exactly "x/y/z/w/v"
|
||||
@@ -3916,9 +3916,6 @@ class ValidationAttachmentRepository:
|
||||
|
||||
from ulid import ULID as _ULID
|
||||
|
||||
if "/" in resource_id and "/" not in validation_name:
|
||||
validation_name, resource_id = resource_id, validation_name
|
||||
|
||||
session = self._session()
|
||||
try:
|
||||
# Check for existing attachment with same validation+resource+scope
|
||||
|
||||
Reference in New Issue
Block a user