diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b944affe..eb1c2bf46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +- Added four CLI-based integration test cases to M5 E2E verification suite + for v3.4.0 milestone acceptance criteria validation. Tests exercise + `project create`, `resource add git-checkout`, `project link-resource`, and + `project show` via real subprocess calls to `python -m cleveragents` with + per-test workspace isolation. (#496) +- Fixed `ProjectResourceLinkRepository.create_link()` and `remove_link()` + only calling `session.flush()` without `session.commit()`, causing linked + resource data to be lost between sessions. Added `finally: session.close()` + to both methods to match the session-factory lifecycle pattern used by all + other mutating repository methods. (#496) - Fixed `agents session list`, `agents session create`, and other session subcommands raising `AttributeError: 'DynamicContainer' object has no attribute 'db'` after `agents init`. Root cause: `_get_session_service()` diff --git a/features/project_repository.feature b/features/project_repository.feature index 1ee570068..afe03f4ce 100644 --- a/features/project_repository.feature +++ b/features/project_repository.feature @@ -132,3 +132,17 @@ Feature: Namespaced project repository operations And a resource exists with id "00000000000000000000000009" When I create a read-only link for project "local/ro-proj" to resource "00000000000000000000000009" Then the link read_only flag should be True + + Scenario: Created link is visible in a new session (cross-session persistence) + Given a namespaced project "local/xsess-proj" exists in the repository + And a resource exists with id "00000000000000000000000010" + When I create a link for project "local/xsess-proj" to resource "00000000000000000000000010" without alias + Then the link should be visible in a new session from the same engine + + Scenario: Removed link is absent in a new session (cross-session persistence) + Given a namespaced project "local/xsess-rm" exists in the repository + And a resource exists with id "00000000000000000000000011" + And a link exists for project "local/xsess-rm" to resource "00000000000000000000000011" without alias + When I remove the link by its stored id + Then the remove result should be True + And the removed link should be absent in a new session from the same engine diff --git a/features/steps/project_repository_steps.py b/features/steps/project_repository_steps.py index b0d80c707..1122260c3 100644 --- a/features/steps/project_repository_steps.py +++ b/features/steps/project_repository_steps.py @@ -510,3 +510,58 @@ def step_pr_create_ro_link(context: Any, ns_name: str, resource_id: str) -> None @then("the link read_only flag should be True") def step_pr_link_readonly(context: Any) -> None: assert bool(context.pr_link.read_only) is True + + +# --------------------------------------------------------------------------- +# Cross-session persistence verification steps +# --------------------------------------------------------------------------- + + +@then("the link should be visible in a new session from the same engine") +def step_pr_link_visible_new_session(context: Any) -> None: + """Open a fresh session from the same engine and verify the link exists. + + This guards against the bug where ``create_link()`` only called + ``flush()`` without ``commit()``, making data visible within the + same session but lost when a different session queries the database. + """ + from cleveragents.infrastructure.database.models import ProjectResourceLinkModel + + new_session = context.pr_session_factory() + try: + row = ( + new_session.query(ProjectResourceLinkModel) + .filter_by(link_id=context.pr_stored_link_id) + .first() + ) + assert row is not None, ( + f"Link {context.pr_stored_link_id} not found in a new session — " + "commit() may be missing" + ) + finally: + new_session.close() + + +@then("the removed link should be absent in a new session from the same engine") +def step_pr_removed_link_absent_new_session(context: Any) -> None: + """Open a fresh session and confirm the removed link no longer exists. + + This guards against the bug where ``remove_link()`` only called + ``flush()`` without ``commit()``, making the deletion visible within + the same session but not persisted across sessions. + """ + from cleveragents.infrastructure.database.models import ProjectResourceLinkModel + + new_session = context.pr_session_factory() + try: + row = ( + new_session.query(ProjectResourceLinkModel) + .filter_by(link_id=context.pr_stored_link_id) + .first() + ) + assert row is None, ( + f"Link {context.pr_stored_link_id} still present in a new session — " + "commit() may be missing from remove_link()" + ) + finally: + new_session.close() diff --git a/robot/m5_e2e_verification.robot b/robot/m5_e2e_verification.robot index b90f332c8..d9dc5215c 100644 --- a/robot/m5_e2e_verification.robot +++ b/robot/m5_e2e_verification.robot @@ -5,6 +5,18 @@ Documentation End-to-end verification of M5 success criteria: ... indexing verification, context tier management ... (hot/warm/cold), ACMS v1 pipeline with phase ... view inheritance and scoped context output. +... +... CLI acceptance criteria from the v3.4.0 milestone +... description are tested via actual subprocess calls +... to ``agents`` (``python -m cleveragents``). +... Each CLI test creates an isolated workspace with its +... own ``CLEVERAGENTS_HOME`` to prevent shared state. +... +... Context tier management and ACMS scoped context output +... criteria are validated at the Python API level via +... ``helper_m5_e2e_verification.py`` because the CLI does +... not yet expose dedicated context/ACMS inspection +... commands. Resource ${CURDIR}/common.resource Suite Setup Setup Test Environment Suite Teardown Cleanup Test Environment @@ -13,6 +25,237 @@ Suite Teardown Cleanup Test Environment ${HELPER} ${CURDIR}/helper_m5_e2e_verification.py *** Test Cases *** +# ================================================================ +# CLI Success Criteria (milestone description) +# ================================================================ + +CLI Project Create Large Project + [Documentation] ``agents project create local/large-project`` creates + ... a project via the real CLI subprocess. + ${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='m5_cli_create_') + ${init}= Run Process ${PYTHON} -m cleveragents init m5-cli-test + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${init.stdout} + Log ${init.stderr} + Should Be Equal As Integers ${init.rc} 0 + ... msg=agents init failed (rc ${init.rc}): ${init.stderr} + ${create}= Run Process ${PYTHON} -m cleveragents + ... project create local/large-project + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${create.stdout} + Log ${create.stderr} + Should Be Equal As Integers ${create.rc} 0 + ... msg=project create failed (rc ${create.rc}): ${create.stderr} + # Verify the project is persisted via list + ${list}= Run Process ${PYTHON} -m cleveragents + ... project list --format plain + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${list.stdout} + Should Be Equal As Integers ${list.rc} 0 + Should Contain ${list.stdout} local/large-project + ... msg=project list should contain local/large-project + [Teardown] Remove Directory ${tmpdir} recursive=True + +CLI Resource Add Git Checkout + [Documentation] ``agents resource add git-checkout local/large-repo + ... --path /path/to/large/repo --branch main`` registers + ... a large repository resource via the real CLI subprocess. + ${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='m5_cli_res_') + # Create a fake repo directory to use as --path. + # The CLI does not validate that the path is a real git repository + # at registration time; it only records the location for later use. + ${repo_dir}= Set Variable ${tmpdir}${/}large-repo + Create Directory ${repo_dir} + # Initialize workspace + ${init}= Run Process ${PYTHON} -m cleveragents init m5-res-test + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${init.rc} 0 + ... msg=agents init failed: ${init.stderr} + # Register git-checkout resource + ${add}= Run Process ${PYTHON} -m cleveragents resource add + ... git-checkout local/large-repo + ... --path ${repo_dir} --branch main + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${add.stdout} + Log ${add.stderr} + Should Be Equal As Integers ${add.rc} 0 + ... msg=resource add failed (rc ${add.rc}): ${add.stderr} + # Verify resource is visible via show + ${show}= Run Process ${PYTHON} -m cleveragents resource show + ... --format plain local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${show.stdout} + Should Be Equal As Integers ${show.rc} 0 + ... msg=resource show failed: ${show.stderr} + Should Contain ${show.stdout} local/large-repo + ... msg=resource show should display resource name + Should Match Regexp ${show.stdout} branch.*main + ... msg=resource show should display the configured branch (main) + [Teardown] Remove Directory ${tmpdir} recursive=True + +CLI Project Link Resource + [Documentation] ``agents project link-resource local/large-project + ... local/large-repo`` links the resource to the project + ... via the real CLI subprocess. + ${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='m5_cli_link_') + # Create a fake repo directory (CLI records location without git validation) + ${repo_dir}= Set Variable ${tmpdir}${/}link-repo + Create Directory ${repo_dir} + # Initialize workspace + ${init}= Run Process ${PYTHON} -m cleveragents init m5-link-test + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${init.rc} 0 + # Create project + ${create}= Run Process ${PYTHON} -m cleveragents + ... project create local/large-project + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${create.rc} 0 + ... msg=project create failed: ${create.stderr} + # Register resource + ${add}= Run Process ${PYTHON} -m cleveragents resource add + ... git-checkout local/large-repo + ... --path ${repo_dir} --branch main + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${add.rc} 0 + ... msg=resource add failed: ${add.stderr} + # Link resource to project + ${link}= Run Process ${PYTHON} -m cleveragents + ... project link-resource local/large-project local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${link.stdout} + Log ${link.stderr} + Should Be Equal As Integers ${link.rc} 0 + ... msg=project link-resource failed (rc ${link.rc}): ${link.stderr} + # Verify the link command output confirms the linked resource + Should Contain ${link.stdout} local/large-repo + ... msg=link-resource output should confirm the linked resource + Should Contain ${link.stdout} local/large-project + ... msg=link-resource output should confirm the target project + # Capture the resource ULID so we can verify the exact ID in project show + ${res_show}= Run Process ${PYTHON} -m cleveragents resource show + ... --format plain local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${res_show.rc} 0 + ... msg=resource show failed: ${res_show.stderr} + ${ulid_match}= Get Regexp Matches ${res_show.stdout} + ... "resource_id":\\s*"([0-9A-Za-z]{26})" 1 + ${has_ulid}= Evaluate len($ulid_match) > 0 + # Verify the link was persisted by checking project show output + ${show}= Run Process ${PYTHON} -m cleveragents project show + ... --format plain local/large-project + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${show.stdout} + Should Be Equal As Integers ${show.rc} 0 + ... msg=project show after link failed: ${show.stderr} + Should Contain ${show.stdout} "resource_id": + ... msg=project show should include linked resource entries after linking + # If we captured the resource ULID, verify the exact ID appears + IF ${has_ulid} + Should Contain ${show.stdout} ${ulid_match}[0] + ... msg=project show should contain the exact linked resource ULID + END + [Teardown] Remove Directory ${tmpdir} recursive=True + +CLI Project Show Displays Linked Resource + [Documentation] ``agents project show local/large-project`` displays + ... project details with the specific linked resource ULID + ... via the real CLI subprocess. + ... + ... This test differs from *CLI Project Link Resource* by + ... capturing the resource's ULID from ``resource show`` + ... and asserting that exact ULID appears in + ... ``project show`` output. It also verifies + ... ``resource show`` independently after linking. + ... + ... Context tier and indexing fields are validated at the + ... Python API level via ``helper_m5_e2e_verification.py`` + ... because the CLI does not yet render those fields. + ${tmpdir}= Evaluate __import__('tempfile').mkdtemp(prefix='m5_cli_show_') + # Create a fake repo directory (CLI records location without git validation) + ${repo_dir}= Set Variable ${tmpdir}${/}show-repo + Create Directory ${repo_dir} + # Initialize, create, register, link (full flow) + ${init}= Run Process ${PYTHON} -m cleveragents init m5-show-test + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${init.rc} 0 + ${create}= Run Process ${PYTHON} -m cleveragents + ... project create local/large-project + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${create.rc} 0 + ${add}= Run Process ${PYTHON} -m cleveragents resource add + ... git-checkout local/large-repo + ... --path ${repo_dir} --branch main + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${add.rc} 0 + # Capture the resource ULID from resource show output so we can + # assert the specific ID appears in project show later (not just + # the generic "resource_id": key). + ${res_show}= Run Process ${PYTHON} -m cleveragents resource show + ... --format plain local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${res_show.stdout} + Should Be Equal As Integers ${res_show.rc} 0 + ... msg=resource show failed: ${res_show.stderr} + Should Contain ${res_show.stdout} local/large-repo + ... msg=resource show should display resource name + # Extract the resource_id ULID (26-char alphanumeric string) from output + ${match}= Get Regexp Matches ${res_show.stdout} + ... "resource_id":\\s*"([0-9A-Za-z]{26})" 1 + ${has_ulid}= Evaluate len($match) > 0 + # Link the resource + ${link}= Run Process ${PYTHON} -m cleveragents + ... project link-resource local/large-project local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${link.rc} 0 + # Verify resource show still works after linking (independent check) + ${res_show2}= Run Process ${PYTHON} -m cleveragents resource show + ... --format plain local/large-repo + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Should Be Equal As Integers ${res_show2.rc} 0 + ... msg=resource show after linking failed: ${res_show2.stderr} + # Show project and verify it displays project name, linked resource, + # and (when available) the specific resource ULID. + ${show}= Run Process ${PYTHON} -m cleveragents project show + ... --format plain local/large-project + ... timeout=60s on_timeout=kill cwd=${tmpdir} + ... env:CLEVERAGENTS_HOME=${tmpdir} + Log ${show.stdout} + Log ${show.stderr} + Should Be Equal As Integers ${show.rc} 0 + ... msg=project show failed (rc ${show.rc}): ${show.stderr} + Should Contain ${show.stdout} local/large-project + ... msg=project show should display project name + Should Contain ${show.stdout} "resource_id": + ... msg=project show should display linked resource entries + # If we captured the ULID, verify the exact resource_id appears + IF ${has_ulid} + Should Contain ${show.stdout} ${match}[0] + ... msg=project show should contain the exact resource ULID (${match}[0]) + END + [Teardown] Remove Directory ${tmpdir} recursive=True + +# ================================================================ +# Python API Technical Criteria (domain model + persistence) +# ================================================================ + Large Project Creation With Ten Thousand Files [Documentation] Create a project, register a git-checkout resource ... at a directory with 10,000+ files, link the resource, diff --git a/src/cleveragents/infrastructure/database/repositories.py b/src/cleveragents/infrastructure/database/repositories.py index 0fabb59f8..127cb0e02 100644 --- a/src/cleveragents/infrastructure/database/repositories.py +++ b/src/cleveragents/infrastructure/database/repositories.py @@ -3011,6 +3011,8 @@ class ProjectResourceLinkRepository: Uses a session-factory pattern: each public method obtains its own session from the factory, ensuring proper session lifecycle management. + All mutating methods (create_link, remove_link) flush and commit + within their own session, then close the session. """ def __init__(self, session_factory: Callable[[], Session]) -> None: @@ -3068,6 +3070,12 @@ class ProjectResourceLinkRepository: ) session.add(link) session.flush() + session.commit() + # Refresh attributes before closing the session so the + # returned ORM instance is fully loaded and can be used + # after the session is closed (detached state). + session.refresh(link) + session.expunge(link) return link except DuplicateLinkError: raise @@ -3077,6 +3085,8 @@ class ProjectResourceLinkRepository: except (OperationalError, SQLAlchemyDatabaseError) as exc: session.rollback() raise DatabaseError(f"Failed to create link: {exc}") from exc + finally: + session.close() @database_retry def list_links(self, project_name: str) -> list[Any]: @@ -3144,10 +3154,13 @@ class ProjectResourceLinkRepository: return False session.delete(row) session.flush() + session.commit() return True except (OperationalError, SQLAlchemyDatabaseError) as exc: session.rollback() raise DatabaseError(f"Failed to remove link '{link_id}': {exc}") from exc + finally: + session.close() # ---------------------------------------------------------------------------