Repository: openlawlibrary/taf (language: python)

Issue:
Revert to last valid state if invalid commits found
Currently, if target repositories are out of sync with the authentication repositories, we start the validation from the beginning, and not the last validated commit. If the validation fails, repositories will be updated up to the last valid commit. That should automatically remove all invalid commits from the local repositories.

Create the following tests (some might already exist, we should check):

  1. Client's authentication repository and target repositories are not in sync. Authentication repository contains some commits following the last validated commit. Target repositories are in sync with the last validated commit - simulate the case when someone used `git pull` to update the authentication repository. In fact, that is probably how we want to set this up. Run the updater to clone all repositories (an auth repo and its targets, we should not make this more complicated by testing the update of a repo with dependencies). Make valid changes to both the auth repo and the target repos (only origin repos). Sync client's auth repo with remote without running the updater. Run the updater. Expect and check that all repositories were successfully updated to the last remote commit
2. Client's authentication repository is on last validated commits, but target repositories contain commits following the last validated ones. Simulate the case when someone used `git pull` to update the one or more target repositories.  Run the updater to clone all repositories. Make valid changes to both the auth repo and the target repos (only origin repos). Sync one or more target repos with remote without running the updater. Run the updater. Expect and check that all repositories were successfully updated to the last remote commit.
3. Clients auth repo has been manually updated, just like in 1. The update should result in a partial update. Clients auth repo should be reverted to the last valid commit, last validated commits set to that value, target repositories updated up to commits listed in auth repo's last validated commits
4. Same as 2, but target repositories contain invalid commits following commits which were manually pulled using `git pull`. Expect all repositories to be updated up to the last validated commits and invalid commits to be removed
 5. Client's auth repo is not on last validated commit, but target commits are all over the place. Some are after last validated, some are before. At the moment, i believe that the validation will start from the beginning in such a case and will be successful if there are no invalid commits.

There is some room to improve the performance here:
- if top commits of target repositories are not in sync with the auth repo, but the last validated commits are in git histories of these repositories, start the validation from last validated commit, not from the beginning. 
- if run with `--force`, remove all additional commits from target repositories. That is, revert to last validated commits. This decreases the likelihood that something will go wrong during the update phase 


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36c9ebb5..c6cbd07f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning][semver].
 ### Added
 
 - Allow for the displaying of varied levels of log and debug information based on the verbosity level ([493])
+- Added new tests to test out of sync repositories and manual updates [488]
 - Added lazy loading to CLI [481]
 - Testing repositories with dependencies ([479], [487])
 - Hid plaintext when users are prompted to insert YubiKey and press ENTER ([473])
@@ -38,6 +39,7 @@ and this project adheres to [Semantic Versioning][semver].
 
 [493]: https://github.com/openlawlibrary/taf/pull/493
 [489]: https://github.com/openlawlibrary/taf/pull/489
+[488]: https://github.com/openlawlibrary/taf/pull/488
 [487]: https://github.com/openlawlibrary/taf/pull/487
 [487]: https://github.com/openlawlibrary/taf/pull/485
 [481]: https://github.com/openlawlibrary/taf/pull/481


Test patch:
diff --git a/taf/tests/test_updater/conftest.py b/taf/tests/test_updater/conftest.py
index 7be189b2..cb1d5ebb 100644
--- a/taf/tests/test_updater/conftest.py
+++ b/taf/tests/test_updater/conftest.py
@@ -566,6 +566,19 @@ def update_role_metadata_without_signing(
     )
 
 
+def update_existing_file(repo: GitRepository, filename: str, commit_message: str):
+    text_to_add = _generate_random_text()
+    file_path = repo.path / filename
+    if file_path.exists():
+        with file_path.open("a") as file:
+            file.write(f"\n{text_to_add}")
+        repo.commit(commit_message)
+    else:
+        raise FileNotFoundError(
+            f"The file {filename} does not exist in the repository {repo.path}"
+        )
+
+
 def update_role_metadata_invalid_signature(
     auth_repo: AuthenticationRepository, role: str
 ):
@@ -665,3 +678,19 @@ def set_head_commit(auth_repo: AuthenticationRepository):
         auth_repo.set_last_validated_commit(last_valid_commit)
     else:
         raise ValueError("Failed to retrieve the last valid commit SHA.")
+
+
+def pull_specific_target_repo(client_dir: Path, repo_name: str):
+    client_target_repo = GitRepository(client_dir, repo_name)
+    client_target_repo.pull()
+
+
+def pull_all_target_repos(auth_repo: AuthenticationRepository, client_dir: Path):
+    client_target_repos = load_target_repositories(auth_repo, library_dir=client_dir)
+    for _, client_repo in client_target_repos.items():
+        client_repo.pull()
+
+
+def pull_client_auth_repo(auth_repo: AuthenticationRepository, client_dir: Path):
+    client_auth_repo = AuthenticationRepository(client_dir, auth_repo.name)
+    client_auth_repo.pull()
diff --git a/taf/tests/test_updater/test_update/test_validation_and_sync.py b/taf/tests/test_updater/test_update/test_validation_and_sync.py
new file mode 100644
index 00000000..12434b4e
--- /dev/null
+++ b/taf/tests/test_updater/test_update/test_validation_and_sync.py
@@ -0,0 +1,314 @@
+import pytest
+from taf.updater.types.update import OperationType
+from taf.tests.test_updater.update_utils import (
+    clone_repositories,
+    load_target_repositories,
+    update_and_check_commit_shas,
+    verify_client_repos_state,
+    verify_partial_update,
+)
+from taf.tests.test_updater.conftest import (
+    SetupManager,
+    add_valid_target_commits,
+    add_valid_unauthenticated_commits,
+    pull_all_target_repos,
+    pull_client_auth_repo,
+    pull_specific_target_repo,
+    remove_commits,
+    update_existing_file,
+    update_expiration_dates,
+    update_role_metadata_without_signing,
+)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ],
+        },
+    ],
+    indirect=True,
+)
+def test_auth_repo_not_in_sync(origin_auth_repo, client_dir):
+    """
+    Sync client's auth repo with remote without running the updater.
+    Run the updater. Expect and check that all repositories were successfully updated to the last remote commit.
+    """
+    clone_repositories(origin_auth_repo, client_dir)
+    original_commit = origin_auth_repo.head_commit_sha()
+
+    setup_manager = SetupManager(origin_auth_repo)
+    setup_manager.add_task(update_expiration_dates)
+    setup_manager.add_task(add_valid_target_commits)
+    setup_manager.execute_tasks()
+
+    new_origin_commit = origin_auth_repo.head_commit_sha()
+    assert original_commit != new_origin_commit
+
+    setup_manager.add_task(pull_client_auth_repo, kwargs={"client_dir": client_dir})
+    setup_manager.execute_tasks()
+
+    update_and_check_commit_shas(OperationType.UPDATE, origin_auth_repo, client_dir)
+    verify_client_repos_state(client_dir, origin_auth_repo)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ],
+        },
+    ],
+    indirect=True,
+)
+def test_target_repo_not_in_sync(origin_auth_repo, client_dir):
+    """
+    The client's authentication repository is at the last validated commits,
+    but the target repositories have additional commits that follow the last validated ones.
+    This simulates a scenario where someone used `git pull` to update one or more target repositories.
+    """
+    clone_repositories(
+        origin_auth_repo,
+        client_dir,
+    )
+    original_commit = origin_auth_repo.head_commit_sha()
+
+    setup_manager = SetupManager(origin_auth_repo)
+    setup_manager.add_task(add_valid_target_commits)
+    setup_manager.add_task(update_expiration_dates)
+    setup_manager.execute_tasks()
+
+    new_origin_commit = origin_auth_repo.head_commit_sha()
+    assert original_commit != new_origin_commit
+
+    setup_manager.add_task(pull_all_target_repos, kwargs={"client_dir": client_dir})
+    setup_manager.execute_tasks()
+
+    # Run the updater to update repositories
+    update_and_check_commit_shas(
+        OperationType.UPDATE,
+        origin_auth_repo,
+        client_dir,
+    )
+    verify_client_repos_state(client_dir, origin_auth_repo)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ],
+        },
+    ],
+    indirect=True,
+)
+def test_auth_repo_not_in_sync_partial(origin_auth_repo, client_dir):
+    """
+    Clients' auth repo has been manually updated.
+    The update should result in a partial update where there are invalid commits among the ones that the client pulled.
+    Clients' auth repo should be reverted to the last valid commit,
+    last validated commits set to that value, and target repositories
+    updated up to commits listed in the auth repo's last validated commits.
+    """
+    clone_repositories(origin_auth_repo, client_dir)
+
+    # Load the target repositories
+    target_repos = list(load_target_repositories(origin_auth_repo).values())
+    original_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+    original_commit = origin_auth_repo.head_commit_sha()
+
+    setup_manager = SetupManager(origin_auth_repo)
+    setup_manager.add_task(add_valid_unauthenticated_commits)
+    setup_manager.add_task(add_valid_target_commits)
+    setup_manager.add_task(
+        update_role_metadata_without_signing, kwargs={"role": "root"}
+    )
+    setup_manager.execute_tasks()
+
+    new_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+
+    for repo_name in original_commits:
+        assert original_commits[repo_name] != new_commits[repo_name]
+
+    new_origin_commit = origin_auth_repo.head_commit_sha()
+    assert original_commit != new_origin_commit
+
+    origin_auth_repo.commit("Committing uncommitted changes before pull")
+
+    setup_manager.add_task(pull_client_auth_repo, kwargs={"client_dir": client_dir})
+    setup_manager.execute_tasks()
+
+    update_and_check_commit_shas(
+        OperationType.UPDATE, origin_auth_repo, client_dir, force=True
+    )
+
+    verify_partial_update(client_dir, origin_auth_repo, original_commits)
+    verify_client_repos_state(client_dir, origin_auth_repo)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ],
+        },
+    ],
+    indirect=True,
+)
+def test_target_repo_not_in_sync_partial(origin_auth_repo, client_dir):
+    """
+    Same as 2, but target repositories contain invalid commits following commits which were manually pulled using git pull.
+    Expect all repositories to be updated up to the last validated commits and invalid commits to be removed
+    """
+    clone_repositories(
+        origin_auth_repo,
+        client_dir,
+    )
+    target_repos = list(load_target_repositories(origin_auth_repo).values())
+    original_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+
+    setup_manager = SetupManager(origin_auth_repo)
+    setup_manager.add_task(add_valid_unauthenticated_commits)
+    setup_manager.add_task(add_valid_target_commits)
+    setup_manager.execute_tasks()
+
+    new_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+    for repo_name in original_commits:
+        assert original_commits[repo_name] != new_commits[repo_name]
+
+    setup_manager.add_task(pull_all_target_repos, kwargs={"client_dir": client_dir})
+    setup_manager.execute_tasks()
+
+    update_and_check_commit_shas(OperationType.UPDATE, origin_auth_repo, client_dir)
+
+    verify_partial_update(client_dir, origin_auth_repo, original_commits)
+    verify_client_repos_state(client_dir, origin_auth_repo)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ]
+        },
+    ],
+    indirect=True,
+)
+def test_mixed_target_repo_states(origin_auth_repo, client_dir):
+    """
+    Client's auth repo is on last validated commit, but target commits are all over the place.
+    Some are after last validated, some are before.
+    """
+    clone_repositories(
+        origin_auth_repo,
+        client_dir,
+    )
+
+    target_repos = list(load_target_repositories(origin_auth_repo).values())
+    original_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+    reverted_repo = target_repos[0]  # target1
+    updated_repo = target_repos[1]  # target2
+
+    # Add valid commits first to
+    setup_manager = SetupManager(origin_auth_repo)
+
+    setup_manager.add_task(
+        remove_commits, kwargs={"repo_path": reverted_repo.path, "num_commits": 1}
+    )
+    setup_manager.add_task(
+        update_existing_file,
+        kwargs={
+            "repo": updated_repo,
+            "filename": "test1.txt",
+            "commit_message": "Manually update test1.txt",
+        },
+    )
+    setup_manager.execute_tasks()
+
+    # Check that the reverted repo has the same commit as the original
+    reverted_commit = reverted_repo.head_commit_sha()
+    assert original_commits[reverted_repo.name] == reverted_commit
+
+    # Check that the manually updated repo has a new commit
+    updated_commit = updated_repo.head_commit_sha()
+    assert original_commits[updated_repo.name] != updated_commit
+
+    setup_manager.add_task(pull_client_auth_repo, kwargs={"client_dir": client_dir})
+    setup_manager.execute_tasks()
+
+    update_and_check_commit_shas(
+        OperationType.UPDATE, origin_auth_repo, client_dir, force=True
+    )
+
+    verify_client_repos_state(client_dir, origin_auth_repo)
+
+
+@pytest.mark.parametrize(
+    "origin_auth_repo",
+    [
+        {
+            "targets_config": [
+                {"name": "target1", "allow_unauthenticated_commits": True},
+                {"name": "target2", "allow_unauthenticated_commits": True},
+            ],
+        },
+    ],
+    indirect=True,
+)
+def test_target_repo_mixed_manual_updates(origin_auth_repo, client_dir):
+    """
+    Some target repositories are updated manually, and some are not.
+    Expect that manually updated repositories have new commits, while others do not.
+    """
+    clone_repositories(
+        origin_auth_repo,
+        client_dir,
+    )
+
+    target_repos = list(load_target_repositories(origin_auth_repo).values())
+    original_commits = {repo.name: repo.head_commit_sha() for repo in target_repos}
+    updated_repo = target_repos[0]  # target1
+    reverted_repo = target_repos[1]  # target2
+
+    # Add commits to revert later
+    setup_manager = SetupManager(origin_auth_repo)
+
+    setup_manager.add_task(
+        add_valid_target_commits, kwargs={"target_repos": [updated_repo]}
+    )
+    setup_manager.add_task(
+        remove_commits, kwargs={"repo_path": reverted_repo.path, "num_commits": 1}
+    )
+    setup_manager.execute_tasks()
+    # Check that the reverted repo has the same commit as the original
+    reverted_commit = reverted_repo.head_commit_sha()
+    assert original_commits[reverted_repo.name] == reverted_commit
+
+    # Check that the manually updated repo has a new commit
+    updated_commit = updated_repo.head_commit_sha()
+    assert original_commits[updated_repo.name] != updated_commit
+
+    pull_specific_target_repo(client_dir, reverted_repo.name)
+    pull_specific_target_repo(client_dir, updated_repo.name)
+
+    setup_manager.execute_tasks()
+
+    update_and_check_commit_shas(OperationType.UPDATE, origin_auth_repo, client_dir)
+
+    verify_client_repos_state(client_dir, origin_auth_repo)
diff --git a/taf/tests/test_updater/update_utils.py b/taf/tests/test_updater/update_utils.py
index 324c671a..31765022 100644
--- a/taf/tests/test_updater/update_utils.py
+++ b/taf/tests/test_updater/update_utils.py
@@ -1,5 +1,6 @@
 import os
 import pytest
+from pathlib import Path
 from freezegun import freeze_time
 from collections import defaultdict
 from datetime import datetime
@@ -305,6 +306,93 @@ def _update_expect_error():
                 assert not client_repository.path.exists()
 
 
+def verify_client_repos_state(
+    client_dir: Path, origin_auth_repo: AuthenticationRepository
+):
+    """
+    Verify that the client's repositories are in the correct state.
+    This means that the target repositories in the client repo should be in sync with the origin repo,
+    and the client's auth repo should be updated to the last validated commit.
+    """
+    client_auth_repo = AuthenticationRepository(path=client_dir / origin_auth_repo.name)
+    client_target_repos = load_target_repositories(
+        origin_auth_repo, library_dir=client_dir
+    )
+
+    check_last_validated_commit(client_auth_repo.path)
+
+    successful_update = True
+    for repo_name, client_repo in client_target_repos.items():
+        client_commit = client_repo.head_commit_sha()
+        origin_commit = origin_auth_repo.head_commit_sha()
+
+        if client_commit != origin_commit:
+            successful_update = False
+            break
+
+    if successful_update:
+        check_if_commits_match(client_target_repos, origin_auth_repo.path.parent.parent)
+    else:
+        for repo_name, client_repo in client_target_repos.items():
+            client_commit = client_repo.head_commit_sha()
+
+            # Extract commit SHA from the target file in the client repo
+            target_commit_info = client_auth_repo.get_target(repo_name)
+            target_commit_sha = (
+                target_commit_info.get("commit") if target_commit_info else None
+            )
+
+            # Assert that the top commits of target repositories are the same as the commit SHA specified in the corresponding target files
+            assert (
+                client_commit == target_commit_sha
+            ), f"Target repo {repo_name} should have the same top commit as specified in the corresponding target file"
+            assert (
+                client_commit != origin_commit
+            ), f"Target repo {repo_name} should not have the same commit as specified in the client's auth repo"
+
+
+def verify_partial_update(
+    client_dir: Path, origin_auth_repo: AuthenticationRepository, original_commits: dict
+):
+    """
+    Verify that the client's repositories are in the correct state following a partial update.
+    This means that the top commits of the client's local target repositories are different
+    from the top commits of the origin repositories, and they match the most recent valid
+    commit as specified in the client's auth repo.
+    """
+    client_auth_repo = AuthenticationRepository(path=client_dir / origin_auth_repo.name)
+    client_target_repos = load_target_repositories(
+        origin_auth_repo, library_dir=client_dir
+    )
+
+    # Ensure the last validated commit exists in the client's auth repo
+    check_last_validated_commit(client_auth_repo.path)
+
+    for repo_name, client_repo in client_target_repos.items():
+        client_commit = client_repo.head_commit_sha()
+        origin_commit = origin_auth_repo.head_commit_sha()
+
+        # Ensure the client repository commit is different from the origin repo commit
+        assert (
+            client_commit != origin_commit
+        ), f"Target repo {repo_name} should not have the same top commit as the origin repo after a partial update"
+
+        # Verify that the client's repo commit matches the expected commit SHA in the auth repo
+        target = client_auth_repo.get_target(repo_name)
+
+        # Use the get method to safely access the "commit" key
+        expected_commit_sha = target.get("commit") if target else None
+
+        # Ensure expected_commit_sha is not None before proceeding
+        assert (
+            expected_commit_sha is not None
+        ), f"Commit SHA for {repo_name} is missing in the auth repo"
+
+        assert (
+            client_commit == expected_commit_sha
+        ), f"Target repo {repo_name} should have the same top commit as specified in the client's auth repo"
+
+
 def update_and_validate_repositories(
     library_with_dependencies,
     origin_dir,