From aed4fe6fe4c8beb4a54b9534fae74db2418c06a8 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 27 May 2026 20:26:17 +0530 Subject: [PATCH 01/23] feat(harvester): implement repository configuration loader --- application/tests/harvester_test/__init__.py | 3 + .../fixtures/invalid_chunk_size.yaml | 17 +++ .../fixtures/invalid_missing_id.yaml | 17 +++ .../harvester_test/fixtures/invalid_yaml.yaml | 4 + .../harvester_test/fixtures/valid_repos.yaml | 16 +++ .../harvester_test/test_config_loader.py | 45 ++++++++ application/utils/harvester/__init__.py | 22 ++++ application/utils/harvester/config_loader.py | 26 +++++ application/utils/harvester/repos.yaml | 43 ++++++++ application/utils/harvester/schemas.py | 102 ++++++++++++++++++ 10 files changed, 295 insertions(+) create mode 100644 application/tests/harvester_test/__init__.py create mode 100644 application/tests/harvester_test/fixtures/invalid_chunk_size.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_missing_id.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_yaml.yaml create mode 100644 application/tests/harvester_test/fixtures/valid_repos.yaml create mode 100644 application/tests/harvester_test/test_config_loader.py create mode 100644 application/utils/harvester/__init__.py create mode 100644 application/utils/harvester/config_loader.py create mode 100644 application/utils/harvester/repos.yaml create mode 100644 application/utils/harvester/schemas.py diff --git a/application/tests/harvester_test/__init__.py b/application/tests/harvester_test/__init__.py new file mode 100644 index 000000000..06b0f7440 --- /dev/null +++ b/application/tests/harvester_test/__init__.py @@ -0,0 +1,3 @@ +""" +tests for Module A configuration layer (empty for now) +""" diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml new file mode 100644 index 000000000..4c81538f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -0,0 +1,17 @@ +repositories: + - type: github + + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml new file mode 100644 index 000000000..4c81538f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml @@ -0,0 +1,17 @@ +repositories: + - type: github + + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_yaml.yaml b/application/tests/harvester_test/fixtures/invalid_yaml.yaml new file mode 100644 index 000000000..3f74ab21c --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_yaml.yaml @@ -0,0 +1,4 @@ +repositories: + - repository_id: owasp-top10 + source: + type github diff --git a/application/tests/harvester_test/fixtures/valid_repos.yaml b/application/tests/harvester_test/fixtures/valid_repos.yaml new file mode 100644 index 000000000..98ddf7775 --- /dev/null +++ b/application/tests/harvester_test/fixtures/valid_repos.yaml @@ -0,0 +1,16 @@ +repositories: + - id: owasp-asvs + type: github + owner: OWASP + repo: ASVS + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py new file mode 100644 index 000000000..0a000cf0a --- /dev/null +++ b/application/tests/harvester_test/test_config_loader.py @@ -0,0 +1,45 @@ +from pathlib import Path +import pytest +from application.utils.harvester.config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +def test_load_valid_config(): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + assert len(config.repositories) == 1 + + repo = config.repositories[0] + + assert repo.id == "owasp-asvs" + assert repo.owner == "OWASP" + assert repo.repo == "ASVS" + + +def test_missing_repository_id(): + config_path = FIXTURES_DIR / "invalid_missing_id.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_invalid_chunk_size(): + config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_invalid_yaml_syntax(): + config_path = FIXTURES_DIR / "invalid_yaml.yaml" + with pytest.raises(ConfigLoaderError): + load_repo_config(config_path) + + +def test_missing_config_file(): + with pytest.raises(FileNotFoundError): + load_repo_config("does_not_exist.yaml") diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py new file mode 100644 index 000000000..928af5c96 --- /dev/null +++ b/application/utils/harvester/__init__.py @@ -0,0 +1,22 @@ +from .config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +from .schemas import ( + ChunkingConfig, + PathRules, + PollingConfig, + RepositoryConfig, + ReposFile, +) + +__all__ = [ + "ChunkingConfig", + "ConfigLoaderError", + "PathRules", + "PollingConfig", + "RepositoryConfig", + "ReposFile", + "load_repo_config", +] diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py new file mode 100644 index 000000000..bba76026c --- /dev/null +++ b/application/utils/harvester/config_loader.py @@ -0,0 +1,26 @@ +from pathlib import Path +import yaml +from pydantic import ValidationError +from .schemas import ReposFile + + +class ConfigLoaderError(Exception): + # when repo config loading fails + pass + + +def load_repo_config(path: str | Path) -> ReposFile: + config_path = Path(path) + + if not config_path.exists(): + raise FileNotFoundError(f"Configuration file not found: {config_path}") + try: + with config_path.open("r", encoding="utf-8") as file: + raw_config = yaml.safe_load(file) + except yaml.YAMLError as exc: + raise ConfigLoaderError(f"Invalid YAML syntax in {config_path}") from exc + + try: + return ReposFile.model_validate(raw_config) + except ValidationError as exc: + raise ConfigLoaderError(f"schema validation failed for {config_path}") from exc diff --git a/application/utils/harvester/repos.yaml b/application/utils/harvester/repos.yaml new file mode 100644 index 000000000..f39d07257 --- /dev/null +++ b/application/utils/harvester/repos.yaml @@ -0,0 +1,43 @@ +repositories: + - id: owasp-asvs + type: github + enabled: true + owner: OWASP + repo: ASVS + branch: master + paths: + include: + - "4.0/en/**/*.md" + + exclude: + - "**/archive/**" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 + + - id: owasp-cheatsheets + type: github + enabled: true + + owner: OWASP + repo: CheatSheetSeries + branch: master + + paths: + include: + - "cheatsheets/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1000 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 120 diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py new file mode 100644 index 000000000..d1599e737 --- /dev/null +++ b/application/utils/harvester/schemas.py @@ -0,0 +1,102 @@ +# core routes: +# PathRules +# ChunkingConfig +# ollingConfig +# RepositoryConfig +# ReposFile + +from typing import Literal +from pydantic import BaseModel, Field, ConfigDict + + +# this will control which repo paths are included and excluded during ingestions +class PathRules(BaseModel): + model_config = ConfigDict(extra="forbid") + + include: list[str] = Field( + ..., + min_length=1, + description="Glob patterns to include during ingestions", + ) + exclude: list[str] = Field( + default_factory=list, description="Glob patterns to exclude during ingestions" + ) + + +# this will define how the harvested data should be chunked before downstream +class ChunkingConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + strategy: Literal["markdown", "plaintext"] = Field( + ..., + description="Chunking startergy used for text segmentation", + ) + + max_tokens: int = Field(..., gt=0, description="max toekn size per chunk") + + overlap_tokens: int = Field( # a bit concerned about this + ge=0, # this can also be = 0 i suppose + default=100, + description="token overlap between adjacent chunks", + ) + + +# this one defines repository synchronize behaviour +class PollingConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + mode: Literal["full", "incremental"] = Field( + ..., description="repository sync mode" + ) + + interval_minutes: int = Field(..., gt=0, description="polling interval in minutes") + + +# top level repository ingestion configuration +class RepositoryConfig(BaseModel): + model_config = ConfigDict(extra="forbid") + + id: str = Field( + ..., + min_length=1, + description="unique repository identifier.", + ) + + type: Literal["github"] = Field( + ..., + description="repository source type.", + ) + enabled: bool = Field( + default=True, + description="whether ingestion is enabled for this repository.", + ) + owner: str = Field( + ..., + min_length=1, + description="repository organization.", + ) + repo: str = Field( + ..., + min_length=1, + description="repository name.", + ) + branch: str = Field( + default="main", + min_length=1, + description="Repository branch to ingest.", + ) + + paths: PathRules + chunking: ChunkingConfig + polling: PollingConfig + + +# Root configuration object loaded from repos.yaml. +class ReposFile(BaseModel): + model_config = ConfigDict(extra="forbid") + + repositories: list[RepositoryConfig] = Field( + ..., + min_length=1, + description="List of repositories configured for ingestion.", + ) From f370f730c8058deb0fea54f5cad2ad4bae2c0eee Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 29 May 2026 11:49:22 +0530 Subject: [PATCH 02/23] fix(harvester): address CodeRabbit validation feedback --- .../harvester_test/fixtures/invalid_chunk_size.yaml | 5 +++-- .../tests/harvester_test/test_config_loader.py | 2 +- application/utils/harvester/config_loader.py | 6 ++++-- application/utils/harvester/schemas.py | 11 ++--------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml index 4c81538f4..b06fdc0ad 100644 --- a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -1,6 +1,7 @@ repositories: - - type: github + - id: owasp-asvs + type: github owner: OWASP repo: ASVS @@ -10,7 +11,7 @@ repositories: chunking: strategy: markdown - max_tokens: 1200 + max_tokens: 0 polling: mode: incremental diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py index 0a000cf0a..4a02ff2d0 100644 --- a/application/tests/harvester_test/test_config_loader.py +++ b/application/tests/harvester_test/test_config_loader.py @@ -30,7 +30,7 @@ def test_missing_repository_id(): def test_invalid_chunk_size(): config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" - with pytest.raises(ConfigLoaderError): + with pytest.raises(ConfigLoaderError, match="max_tokens"): load_repo_config(config_path) diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py index bba76026c..77dba72bb 100644 --- a/application/utils/harvester/config_loader.py +++ b/application/utils/harvester/config_loader.py @@ -12,7 +12,7 @@ class ConfigLoaderError(Exception): def load_repo_config(path: str | Path) -> ReposFile: config_path = Path(path) - if not config_path.exists(): + if not config_path.is_file(): raise FileNotFoundError(f"Configuration file not found: {config_path}") try: with config_path.open("r", encoding="utf-8") as file: @@ -23,4 +23,6 @@ def load_repo_config(path: str | Path) -> ReposFile: try: return ReposFile.model_validate(raw_config) except ValidationError as exc: - raise ConfigLoaderError(f"schema validation failed for {config_path}") from exc + raise ConfigLoaderError( + f"schema validation failed for {config_path} : {exc}" + ) from exc diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index d1599e737..9a94f3285 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -1,10 +1,3 @@ -# core routes: -# PathRules -# ChunkingConfig -# ollingConfig -# RepositoryConfig -# ReposFile - from typing import Literal from pydantic import BaseModel, Field, ConfigDict @@ -29,10 +22,10 @@ class ChunkingConfig(BaseModel): strategy: Literal["markdown", "plaintext"] = Field( ..., - description="Chunking startergy used for text segmentation", + description="Chunking strategy used for text segmentation", ) - max_tokens: int = Field(..., gt=0, description="max toekn size per chunk") + max_tokens: int = Field(..., gt=0, description="max token size per chunk") overlap_tokens: int = Field( # a bit concerned about this ge=0, # this can also be = 0 i suppose From 9478e54856ae737e79b5eefb95991dacb2d9c9f0 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 14:37:17 +0530 Subject: [PATCH 03/23] feat(harvester): add repository config loader and validation --- .../fixtures/duplicate_repo_ids.yaml | 34 +++++++++++++++++++ .../fixtures/duplicate_repositories.yaml | 34 +++++++++++++++++++ .../fixtures/empty_include_paths.yaml | 16 +++++++++ .../fixtures/invalid_polling_interval.yaml | 17 ++++++++++ .../harvester_test/test_repos_validator.py | 34 +++++++++++++++++++ .../utils/harvester/repos_validator.py | 27 +++++++++++++++ 6 files changed, 162 insertions(+) create mode 100644 application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml create mode 100644 application/tests/harvester_test/fixtures/duplicate_repositories.yaml create mode 100644 application/tests/harvester_test/fixtures/empty_include_paths.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_polling_interval.yaml create mode 100644 application/tests/harvester_test/test_repos_validator.py create mode 100644 application/utils/harvester/repos_validator.py diff --git a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml new file mode 100644 index 000000000..b9f76c2e5 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: asvs + type: github + owner: OWASP + repo: CheatSheetSeries + + paths: + include: + - "cheatsheets/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml new file mode 100644 index 000000000..62f019db8 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs-1 + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: asvs-2 + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/empty_include_paths.yaml b/application/tests/harvester_test/fixtures/empty_include_paths.yaml new file mode 100644 index 000000000..afe23361a --- /dev/null +++ b/application/tests/harvester_test/fixtures/empty_include_paths.yaml @@ -0,0 +1,16 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: [] + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml new file mode 100644 index 000000000..94923046c --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml @@ -0,0 +1,17 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 0 diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py new file mode 100644 index 000000000..35bc4b8e4 --- /dev/null +++ b/application/tests/harvester_test/test_repos_validator.py @@ -0,0 +1,34 @@ +from pathlib import Path +import pytest +from application.utils.harvester.config_loader import ( + load_repo_config, +) +from application.utils.harvester.repos_validator import ( + RepositoryValidationError, + validate_repositories, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +def test_duplicate_repository_ids(): + config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" + + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="Duplicate repository id", + ): + validate_repositories(config) + + +def test_duplicate_repositories(): + config_path = FIXTURES_DIR / "duplicate_repositories.yaml" + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="Duplicate repository detected", + ): + validate_repositories(config) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py new file mode 100644 index 000000000..60eccdf0c --- /dev/null +++ b/application/utils/harvester/repos_validator.py @@ -0,0 +1,27 @@ +# as the name suggests +from application.utils.harvester.schemas import ReposFile + + +class RepositoryValidationError(Exception): + """Raised when repository configuration fails semantic validation.""" + + +def validate_repositories(config: ReposFile) -> None: + seen_ids: set[str] = set() + seen_repositories: set[tuple[str, str]] = set() + + for repository in config.repositories: + if repository.id in seen_ids: + raise RepositoryValidationError( + f"Duplicate repository id found: {repository.id}" + ) + seen_ids.add(repository.id) + repository_key = ( + repository.owner, + repository.repo, + ) + if repository_key in seen_repositories: + raise RepositoryValidationError( + f"Duplicate repository detected: {repository.owner}/{repository.repo}" + ) + seen_repositories.add(repository_key) From 43fb6cb6e2740d67447d153c43e6cc5335bdcf32 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 14:55:00 +0530 Subject: [PATCH 04/23] addressing coderabbit --- application/utils/harvester/repos_validator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 60eccdf0c..74da14df3 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -17,8 +17,8 @@ def validate_repositories(config: ReposFile) -> None: ) seen_ids.add(repository.id) repository_key = ( - repository.owner, - repository.repo, + repository.owner.casefold(), + repository.repo.casefold(), ) if repository_key in seen_repositories: raise RepositoryValidationError( From b9622463fef447a5c82f32b2dde0f71b125c41b9 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 15:38:36 +0530 Subject: [PATCH 05/23] addressing coderabbit again --- application/utils/harvester/schemas.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index 9a94f3285..1c48a0c43 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -1,5 +1,5 @@ from typing import Literal -from pydantic import BaseModel, Field, ConfigDict +from pydantic import BaseModel, Field, ConfigDict, model_validator # this will control which repo paths are included and excluded during ingestions @@ -33,6 +33,15 @@ class ChunkingConfig(BaseModel): description="token overlap between adjacent chunks", ) + @model_validator(mode="after") + def overlap_must_be_less_than_max(self) -> "ChunkingConfig": + if self.overlap_tokens >= self.max_tokens: + raise ValueError( + f"overlap_tokens ({self.overlap_tokens}) must be less than " + f"max_tokens ({self.max_tokens})" + ) + return self + # this one defines repository synchronize behaviour class PollingConfig(BaseModel): From b556308ef5ebf1ca055c084378a061065d615041 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 17:44:34 +0530 Subject: [PATCH 06/23] Add repository validation rules and fixtures --- .../fixtures/duplicate_include_paths.yaml | 22 ++++++++++++++ .../harvester_test/fixtures/empty_owner.yaml | 21 +++++++++++++ .../fixtures/invalid_chunking_strategy.yaml | 21 +++++++++++++ .../fixtures/invalid_polling_mode.yaml | 21 +++++++++++++ .../harvester_test/test_config_loader.py | 30 +++++++++++++++++++ .../harvester_test/test_repos_validator.py | 11 +++++++ .../utils/harvester/exclude_patterns.txt | 10 +++++++ .../utils/harvester/repos_validator.py | 8 ++++- 8 files changed, 143 insertions(+), 1 deletion(-) create mode 100644 application/tests/harvester_test/fixtures/duplicate_include_paths.yaml create mode 100644 application/tests/harvester_test/fixtures/empty_owner.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml create mode 100644 application/tests/harvester_test/fixtures/invalid_polling_mode.yaml create mode 100644 application/utils/harvester/exclude_patterns.txt diff --git a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml new file mode 100644 index 000000000..7b8cfef04 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml @@ -0,0 +1,22 @@ +repositories: + - id: duplicate-includes + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/empty_owner.yaml b/application/tests/harvester_test/fixtures/empty_owner.yaml new file mode 100644 index 000000000..a32fe69da --- /dev/null +++ b/application/tests/harvester_test/fixtures/empty_owner.yaml @@ -0,0 +1,21 @@ +repositories: + - id: empty-owner + type: github + enabled: true + + owner: "" + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml b/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml new file mode 100644 index 000000000..c4cb10905 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_chunking_strategy.yaml @@ -0,0 +1,21 @@ +repositories: + - id: invalid-strategy + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: invalid_strategy + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml new file mode 100644 index 000000000..d21b643f4 --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml @@ -0,0 +1,21 @@ +repositories: + - id: invalid-polling + type: github + enabled: true + + owner: OWASP + repo: ASVS + branch: master + + paths: + include: + - "docs/**/*.md" + + chunking: + strategy: markdown + max_tokens: 1200 + overlap_tokens: 100 + + polling: + mode: realtime + interval_minutes: 60 diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py index 4a02ff2d0..b6ca0e51a 100644 --- a/application/tests/harvester_test/test_config_loader.py +++ b/application/tests/harvester_test/test_config_loader.py @@ -43,3 +43,33 @@ def test_invalid_yaml_syntax(): def test_missing_config_file(): with pytest.raises(FileNotFoundError): load_repo_config("does_not_exist.yaml") + + +def test_empty_owner(): + config_path = FIXTURES_DIR / "empty_owner.yaml" + + with pytest.raises( + ConfigLoaderError, + match="owner", + ): + load_repo_config(config_path) + + +def test_invalid_chunking_strategy(): + config_path = FIXTURES_DIR / "invalid_chunking_strategy.yaml" + + with pytest.raises( + ConfigLoaderError, + match="strategy", + ): + load_repo_config(config_path) + + +def test_invalid_polling_mode(): + config_path = FIXTURES_DIR / "invalid_polling_mode.yaml" + + with pytest.raises( + ConfigLoaderError, + match="mode", + ): + load_repo_config(config_path) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py index 35bc4b8e4..517a4bbc1 100644 --- a/application/tests/harvester_test/test_repos_validator.py +++ b/application/tests/harvester_test/test_repos_validator.py @@ -32,3 +32,14 @@ def test_duplicate_repositories(): match="Duplicate repository detected", ): validate_repositories(config) + + +def test_duplicate_include_paths(): + config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" + config = load_repo_config(config_path) + + with pytest.raises( + RepositoryValidationError, + match="duplicate include paths", + ): + validate_repositories(config) diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt new file mode 100644 index 000000000..cd7f642f8 --- /dev/null +++ b/application/utils/harvester/exclude_patterns.txt @@ -0,0 +1,10 @@ +**/.git/** +**/node_modules/** +**/__pycache__/** +**/*.png +**/*.jpg +**/*.jpeg +**/*.svg +**/*.gif +**/*.pdf +**/archive/** diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 74da14df3..2d1f21340 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -22,6 +22,12 @@ def validate_repositories(config: ReposFile) -> None: ) if repository_key in seen_repositories: raise RepositoryValidationError( - f"Duplicate repository detected: {repository.owner}/{repository.repo}" + f"Duplicate repository detected: " + f"{repository.owner}/{repository.repo}" ) seen_repositories.add(repository_key) + include_patterns = set(repository.paths.include) + if len(include_patterns) != len(repository.paths.include): + raise RepositoryValidationError( + f"Repository '{repository.id}' " f"has duplicate include paths" + ) From fc4b58323b22342da169f50796389dd787542b77 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 31 May 2026 18:17:43 +0530 Subject: [PATCH 07/23] normalize repository ids during validation --- application/utils/harvester/repos_validator.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 2d1f21340..7f4858771 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -11,23 +11,23 @@ def validate_repositories(config: ReposFile) -> None: seen_repositories: set[tuple[str, str]] = set() for repository in config.repositories: - if repository.id in seen_ids: + repo_id_key = repository.id.casefold() + if repo_id_key in seen_ids: raise RepositoryValidationError( f"Duplicate repository id found: {repository.id}" ) - seen_ids.add(repository.id) + seen_ids.add(repo_id_key) repository_key = ( repository.owner.casefold(), repository.repo.casefold(), ) if repository_key in seen_repositories: raise RepositoryValidationError( - f"Duplicate repository detected: " - f"{repository.owner}/{repository.repo}" + f"Duplicate repository detected: {repository.owner}/{repository.repo}" ) seen_repositories.add(repository_key) include_patterns = set(repository.paths.include) if len(include_patterns) != len(repository.paths.include): raise RepositoryValidationError( - f"Repository '{repository.id}' " f"has duplicate include paths" + f"Repository '{repository.id}' has duplicate include paths" ) From 713f99d75106b1a121c79c710aebfc4c2301db74 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Tue, 2 Jun 2026 09:51:15 +0530 Subject: [PATCH 08/23] Refine harvester validation exports and YAML fixtures --- .../tests/harvester_test/fixtures/invalid_yaml.yaml | 6 +++--- application/utils/harvester/__init__.py | 7 ++++++- application/utils/harvester/repos_validator.py | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/application/tests/harvester_test/fixtures/invalid_yaml.yaml b/application/tests/harvester_test/fixtures/invalid_yaml.yaml index 3f74ab21c..c60d3e23b 100644 --- a/application/tests/harvester_test/fixtures/invalid_yaml.yaml +++ b/application/tests/harvester_test/fixtures/invalid_yaml.yaml @@ -1,4 +1,4 @@ repositories: - - repository_id: owasp-top10 - source: - type github + - id: broken + paths: + include: [unclosed diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 928af5c96..0be31a8a4 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -2,7 +2,10 @@ ConfigLoaderError, load_repo_config, ) - +from .repos_validator import ( + RepositoryValidationError, + validate_repositories, +) from .schemas import ( ChunkingConfig, PathRules, @@ -19,4 +22,6 @@ "RepositoryConfig", "ReposFile", "load_repo_config", + "RepositoryValidationError", + "validate_repositories", ] diff --git a/application/utils/harvester/repos_validator.py b/application/utils/harvester/repos_validator.py index 7f4858771..81cdc8273 100644 --- a/application/utils/harvester/repos_validator.py +++ b/application/utils/harvester/repos_validator.py @@ -1,5 +1,5 @@ # as the name suggests -from application.utils.harvester.schemas import ReposFile +from .schemas import ReposFile class RepositoryValidationError(Exception): From c7ecbf08cff5e4c1c42049eeae7e244575b59d77 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 7 Jun 2026 12:14:07 +0530 Subject: [PATCH 09/23] removed stable dev comments --- application/utils/harvester/schemas.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index 1c48a0c43..fd9d7d720 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -27,8 +27,8 @@ class ChunkingConfig(BaseModel): max_tokens: int = Field(..., gt=0, description="max token size per chunk") - overlap_tokens: int = Field( # a bit concerned about this - ge=0, # this can also be = 0 i suppose + overlap_tokens: int = Field( + ge=0, default=100, description="token overlap between adjacent chunks", ) From 0be6df87c7642908c76dc915c48b54c30227acff Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 7 Jun 2026 12:44:40 +0530 Subject: [PATCH 10/23] Add validator success test and clean exports --- application/tests/harvester_test/test_repos_validator.py | 8 ++++++++ application/utils/harvester/__init__.py | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py index 517a4bbc1..8c03a9de8 100644 --- a/application/tests/harvester_test/test_repos_validator.py +++ b/application/tests/harvester_test/test_repos_validator.py @@ -43,3 +43,11 @@ def test_duplicate_include_paths(): match="duplicate include paths", ): validate_repositories(config) + + +def test_validate_valid_repositories(): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + validate_repositories(config) diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 0be31a8a4..9c74939c7 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -20,8 +20,8 @@ "PathRules", "PollingConfig", "RepositoryConfig", + "RepositoryValidationError", "ReposFile", "load_repo_config", - "RepositoryValidationError", "validate_repositories", ] From c728fed6204d49958af61f0863699684f4f05d7b Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Thu, 2 Jul 2026 22:00:20 +0530 Subject: [PATCH 11/23] test(harvester): align tests with project conventions --- .../harvester_test/config_loader_test.py | 62 +++++++++++++++ .../harvester_test/repos_validator_test.py | 58 ++++++++++++++ .../harvester_test/test_config_loader.py | 75 ------------------- .../harvester_test/test_repos_validator.py | 53 ------------- 4 files changed, 120 insertions(+), 128 deletions(-) create mode 100644 application/tests/harvester_test/config_loader_test.py create mode 100644 application/tests/harvester_test/repos_validator_test.py delete mode 100644 application/tests/harvester_test/test_config_loader.py delete mode 100644 application/tests/harvester_test/test_repos_validator.py diff --git a/application/tests/harvester_test/config_loader_test.py b/application/tests/harvester_test/config_loader_test.py new file mode 100644 index 000000000..01f22ce18 --- /dev/null +++ b/application/tests/harvester_test/config_loader_test.py @@ -0,0 +1,62 @@ +from pathlib import Path +import unittest + +from application.utils.harvester.config_loader import ( + ConfigLoaderError, + load_repo_config, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +class ConfigLoaderTests(unittest.TestCase): + def test_load_valid_config(self): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + self.assertEqual(len(config.repositories), 1) + + repo = config.repositories[0] + + self.assertEqual(repo.id, "owasp-asvs") + self.assertEqual(repo.owner, "OWASP") + self.assertEqual(repo.repo, "ASVS") + + def test_missing_repository_id(self): + config_path = FIXTURES_DIR / "invalid_missing_id.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_invalid_chunk_size(self): + config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" + + with self.assertRaisesRegex(ConfigLoaderError, "max_tokens"): + load_repo_config(config_path) + + def test_invalid_yaml_syntax(self): + config_path = FIXTURES_DIR / "invalid_yaml.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_missing_config_file(self): + with self.assertRaises(FileNotFoundError): + load_repo_config("does_not_exist.yaml") + + def test_invalid_polling_interval(self): + config_path = FIXTURES_DIR / "invalid_polling_interval.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_empty_include_paths(self): + config_path = FIXTURES_DIR / "empty_include_paths.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/repos_validator_test.py b/application/tests/harvester_test/repos_validator_test.py new file mode 100644 index 000000000..3e0347d41 --- /dev/null +++ b/application/tests/harvester_test/repos_validator_test.py @@ -0,0 +1,58 @@ +from pathlib import Path +import unittest + +from application.utils.harvester.config_loader import ( + load_repo_config, +) +from application.utils.harvester.repos_validator import ( + RepositoryValidationError, + validate_repositories, +) + +FIXTURES_DIR = Path(__file__).parent / "fixtures" + + +class ReposValidatorTests(unittest.TestCase): + def test_duplicate_repository_ids(self): + config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository id", + ): + validate_repositories(config) + + def test_duplicate_repositories(self): + config_path = FIXTURES_DIR / "duplicate_repositories.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository detected", + ): + validate_repositories(config) + + def test_duplicate_include_paths(self): + config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "duplicate include paths", + ): + validate_repositories(config) + + def test_validate_valid_repositories(self): + config_path = FIXTURES_DIR / "valid_repos.yaml" + + config = load_repo_config(config_path) + + validate_repositories(config) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/test_config_loader.py b/application/tests/harvester_test/test_config_loader.py deleted file mode 100644 index b6ca0e51a..000000000 --- a/application/tests/harvester_test/test_config_loader.py +++ /dev/null @@ -1,75 +0,0 @@ -from pathlib import Path -import pytest -from application.utils.harvester.config_loader import ( - ConfigLoaderError, - load_repo_config, -) - -FIXTURES_DIR = Path(__file__).parent / "fixtures" - - -def test_load_valid_config(): - config_path = FIXTURES_DIR / "valid_repos.yaml" - - config = load_repo_config(config_path) - - assert len(config.repositories) == 1 - - repo = config.repositories[0] - - assert repo.id == "owasp-asvs" - assert repo.owner == "OWASP" - assert repo.repo == "ASVS" - - -def test_missing_repository_id(): - config_path = FIXTURES_DIR / "invalid_missing_id.yaml" - with pytest.raises(ConfigLoaderError): - load_repo_config(config_path) - - -def test_invalid_chunk_size(): - config_path = FIXTURES_DIR / "invalid_chunk_size.yaml" - with pytest.raises(ConfigLoaderError, match="max_tokens"): - load_repo_config(config_path) - - -def test_invalid_yaml_syntax(): - config_path = FIXTURES_DIR / "invalid_yaml.yaml" - with pytest.raises(ConfigLoaderError): - load_repo_config(config_path) - - -def test_missing_config_file(): - with pytest.raises(FileNotFoundError): - load_repo_config("does_not_exist.yaml") - - -def test_empty_owner(): - config_path = FIXTURES_DIR / "empty_owner.yaml" - - with pytest.raises( - ConfigLoaderError, - match="owner", - ): - load_repo_config(config_path) - - -def test_invalid_chunking_strategy(): - config_path = FIXTURES_DIR / "invalid_chunking_strategy.yaml" - - with pytest.raises( - ConfigLoaderError, - match="strategy", - ): - load_repo_config(config_path) - - -def test_invalid_polling_mode(): - config_path = FIXTURES_DIR / "invalid_polling_mode.yaml" - - with pytest.raises( - ConfigLoaderError, - match="mode", - ): - load_repo_config(config_path) diff --git a/application/tests/harvester_test/test_repos_validator.py b/application/tests/harvester_test/test_repos_validator.py deleted file mode 100644 index 8c03a9de8..000000000 --- a/application/tests/harvester_test/test_repos_validator.py +++ /dev/null @@ -1,53 +0,0 @@ -from pathlib import Path -import pytest -from application.utils.harvester.config_loader import ( - load_repo_config, -) -from application.utils.harvester.repos_validator import ( - RepositoryValidationError, - validate_repositories, -) - -FIXTURES_DIR = Path(__file__).parent / "fixtures" - - -def test_duplicate_repository_ids(): - config_path = FIXTURES_DIR / "duplicate_repo_ids.yaml" - - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="Duplicate repository id", - ): - validate_repositories(config) - - -def test_duplicate_repositories(): - config_path = FIXTURES_DIR / "duplicate_repositories.yaml" - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="Duplicate repository detected", - ): - validate_repositories(config) - - -def test_duplicate_include_paths(): - config_path = FIXTURES_DIR / "duplicate_include_paths.yaml" - config = load_repo_config(config_path) - - with pytest.raises( - RepositoryValidationError, - match="duplicate include paths", - ): - validate_repositories(config) - - -def test_validate_valid_repositories(): - config_path = FIXTURES_DIR / "valid_repos.yaml" - - config = load_repo_config(config_path) - - validate_repositories(config) From d4e7731a0d7c8328c80bcd11a6020dcb40ff5e44 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 3 Jul 2026 09:38:16 +0530 Subject: [PATCH 12/23] chore: ignore Claude and Cursor project files --- application/utils/harvester/exclude_patterns.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt index cd7f642f8..a3f890866 100644 --- a/application/utils/harvester/exclude_patterns.txt +++ b/application/utils/harvester/exclude_patterns.txt @@ -1,6 +1,8 @@ **/.git/** **/node_modules/** **/__pycache__/** +**/.claude/** +**/.cursor/** **/*.png **/*.jpg **/*.jpeg From f11f869f716dbca9b0d5ec5fd86f2729cfc4aed4 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Fri, 3 Jul 2026 14:46:42 +0530 Subject: [PATCH 13/23] refactor(harvester): address review feedback --- .../tests/harvester_test/config_loader_test.py | 3 ++- .../fixtures/duplicate_include_paths.yaml | 2 +- .../harvester_test/fixtures/duplicate_repo_ids.yaml | 4 ++-- .../fixtures/duplicate_repositories.yaml | 4 ++-- .../harvester_test/fixtures/empty_include_paths.yaml | 2 +- .../tests/harvester_test/fixtures/empty_owner.yaml | 2 +- .../harvester_test/fixtures/invalid_chunk_size.yaml | 2 +- .../harvester_test/fixtures/invalid_missing_id.yaml | 2 +- .../fixtures/invalid_polling_interval.yaml | 2 +- .../fixtures/invalid_polling_mode.yaml | 2 +- .../tests/harvester_test/fixtures/valid_repos.yaml | 2 +- application/utils/harvester/config_loader.py | 12 ++++++++---- application/utils/harvester/exclude_patterns.txt | 8 +++++++- application/utils/harvester/schemas.py | 4 ++-- 14 files changed, 31 insertions(+), 20 deletions(-) diff --git a/application/tests/harvester_test/config_loader_test.py b/application/tests/harvester_test/config_loader_test.py index 01f22ce18..01c404f18 100644 --- a/application/tests/harvester_test/config_loader_test.py +++ b/application/tests/harvester_test/config_loader_test.py @@ -3,6 +3,7 @@ from application.utils.harvester.config_loader import ( ConfigLoaderError, + ConfigFileNotFoundError, load_repo_config, ) @@ -42,7 +43,7 @@ def test_invalid_yaml_syntax(self): load_repo_config(config_path) def test_missing_config_file(self): - with self.assertRaises(FileNotFoundError): + with self.assertRaises(ConfigFileNotFoundError): load_repo_config("does_not_exist.yaml") def test_invalid_polling_interval(self): diff --git a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml index 7b8cfef04..5db642199 100644 --- a/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_include_paths.yaml @@ -13,7 +13,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml index b9f76c2e5..0cc4c1c1e 100644 --- a/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_repo_ids.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: @@ -26,7 +26,7 @@ repositories: - "cheatsheets/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml index 62f019db8..41614d39c 100644 --- a/application/tests/harvester_test/fixtures/duplicate_repositories.yaml +++ b/application/tests/harvester_test/fixtures/duplicate_repositories.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: @@ -26,7 +26,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/empty_include_paths.yaml b/application/tests/harvester_test/fixtures/empty_include_paths.yaml index afe23361a..abe0bab41 100644 --- a/application/tests/harvester_test/fixtures/empty_include_paths.yaml +++ b/application/tests/harvester_test/fixtures/empty_include_paths.yaml @@ -8,7 +8,7 @@ repositories: include: [] chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/empty_owner.yaml b/application/tests/harvester_test/fixtures/empty_owner.yaml index a32fe69da..8063a3f1b 100644 --- a/application/tests/harvester_test/fixtures/empty_owner.yaml +++ b/application/tests/harvester_test/fixtures/empty_owner.yaml @@ -12,7 +12,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml index b06fdc0ad..79d4cc8f0 100644 --- a/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml +++ b/application/tests/harvester_test/fixtures/invalid_chunk_size.yaml @@ -10,7 +10,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 0 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml index 4c81538f4..102e675b9 100644 --- a/application/tests/harvester_test/fixtures/invalid_missing_id.yaml +++ b/application/tests/harvester_test/fixtures/invalid_missing_id.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml index 94923046c..3f34f8baa 100644 --- a/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml +++ b/application/tests/harvester_test/fixtures/invalid_polling_interval.yaml @@ -9,7 +9,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml index d21b643f4..0aead47ad 100644 --- a/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml +++ b/application/tests/harvester_test/fixtures/invalid_polling_mode.yaml @@ -12,7 +12,7 @@ repositories: - "docs/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 diff --git a/application/tests/harvester_test/fixtures/valid_repos.yaml b/application/tests/harvester_test/fixtures/valid_repos.yaml index 98ddf7775..fe52c8430 100644 --- a/application/tests/harvester_test/fixtures/valid_repos.yaml +++ b/application/tests/harvester_test/fixtures/valid_repos.yaml @@ -8,7 +8,7 @@ repositories: - "4.0/en/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 polling: diff --git a/application/utils/harvester/config_loader.py b/application/utils/harvester/config_loader.py index 77dba72bb..db0431e11 100644 --- a/application/utils/harvester/config_loader.py +++ b/application/utils/harvester/config_loader.py @@ -5,15 +5,19 @@ class ConfigLoaderError(Exception): - # when repo config loading fails - pass + """Base class for configuration loading errors.""" + + +class ConfigFileNotFoundError(ConfigLoaderError): + """Raised when the configuration file cannot be found.""" def load_repo_config(path: str | Path) -> ReposFile: config_path = Path(path) if not config_path.is_file(): - raise FileNotFoundError(f"Configuration file not found: {config_path}") + raise ConfigFileNotFoundError(f"Configuration file not found: {config_path}") + try: with config_path.open("r", encoding="utf-8") as file: raw_config = yaml.safe_load(file) @@ -24,5 +28,5 @@ def load_repo_config(path: str | Path) -> ReposFile: return ReposFile.model_validate(raw_config) except ValidationError as exc: raise ConfigLoaderError( - f"schema validation failed for {config_path} : {exc}" + f"Schema validation failed for {config_path}: {exc}" ) from exc diff --git a/application/utils/harvester/exclude_patterns.txt b/application/utils/harvester/exclude_patterns.txt index a3f890866..499850ae8 100644 --- a/application/utils/harvester/exclude_patterns.txt +++ b/application/utils/harvester/exclude_patterns.txt @@ -1,4 +1,10 @@ -**/.git/** +# Placeholder for repository-level exclude patterns. + +# This file will be consumed by the Week 4 noise-reduction pipeline + +# to filter non-documentation files during harvesting. + +**/.git/* **/node_modules/** **/__pycache__/** **/.claude/** diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index fd9d7d720..c65d62b60 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -20,7 +20,7 @@ class PathRules(BaseModel): class ChunkingConfig(BaseModel): model_config = ConfigDict(extra="forbid") - strategy: Literal["markdown", "plaintext"] = Field( + strategy: Literal["markdown_heading", "html_readability", "fixed_size"] = Field( ..., description="Chunking strategy used for text segmentation", ) @@ -29,7 +29,7 @@ class ChunkingConfig(BaseModel): overlap_tokens: int = Field( ge=0, - default=100, + default=20, description="token overlap between adjacent chunks", ) From fbc6e20e930c083ebed4491c04027170404301f1 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 12:23:33 +0530 Subject: [PATCH 14/23] Add repository synchronization foundation --- .../test_git_repository_client.py | 39 ++++++ .../harvester_test/test_repository_cache.py | 12 ++ .../utils/harvester/git_repository_client.py | 116 ++++++++++++++++++ .../utils/harvester/repository_cache.py | 7 ++ .../utils/harvester/repository_client.py | 24 ++++ 5 files changed, 198 insertions(+) create mode 100644 application/tests/harvester_test/test_git_repository_client.py create mode 100644 application/tests/harvester_test/test_repository_cache.py create mode 100644 application/utils/harvester/git_repository_client.py create mode 100644 application/utils/harvester/repository_cache.py create mode 100644 application/utils/harvester/repository_client.py diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py new file mode 100644 index 000000000..b0aa7678d --- /dev/null +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -0,0 +1,39 @@ +from application.utils.harvester.git_repository_client import ( + GitRepositoryClient, +) + + +def test_repository_url_generation(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.repository_url == "https://github.com/OWASP/ASVS.git" + + +def test_local_repository_path(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs" + + +def test_repository_exists_locally_false(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.exists_locally() is False + + +def test_verify_repository_integrity_false(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + assert client.verify_repository_integrity() is False diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py new file mode 100644 index 000000000..5cf4b745f --- /dev/null +++ b/application/tests/harvester_test/test_repository_cache.py @@ -0,0 +1,12 @@ +from application.utils.harvester.repository_cache import ( + build_repository_cache_path, +) + + +def test_build_repository_cache_path(): + path = build_repository_cache_path( + "OWASP", + "ASVS", + ) + + assert str(path) == ".harvester_cache/owasp/asvs" diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py new file mode 100644 index 000000000..de97836ed --- /dev/null +++ b/application/utils/harvester/git_repository_client.py @@ -0,0 +1,116 @@ +import subprocess +from pathlib import Path + +from .repository_cache import build_repository_cache_path +from .repository_client import RepositoryClient +import logging + +logger = logging.getLogger(__name__) + + +class GitRepositoryClient(RepositoryClient): + def __init__(self, owner: str, repository: str, branch: str = "main") -> None: + self.owner = owner + self.repository = repository + self.branch = branch + + self.local_path = build_repository_cache_path( + owner, + repository, + ) + + @property + def repository_url(self) -> str: + return f"https://github.com/{self.owner}/{self.repository}.git" + + def clone(self) -> None: + logger.info( + "Cloning repository %s/%s", + self.owner, + self.repository, + ) + self.local_path.parent.mkdir(parents=True, exist_ok=True) + + subprocess.run( + [ + "git", + "clone", + "--branch", + self.branch, + self.repository_url, + str(self.local_path), + ], + check=True, + ) + + def fetch(self) -> None: + logger.info( + "Fetching repository %s/%s", + self.owner, + self.repository, + ) + + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "fetch", + "--all", + ], + check=True, + ) + + def checkout(self, reference: str) -> None: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "checkout", + reference, + ], + check=True, + ) + + def get_local_path(self) -> Path: + return self.local_path + + def exists_locally(self) -> bool: + return self.local_path.exists() + + def sync(self) -> None: + logger.info( + "Synchronizing repository %s/%s", + self.owner, + self.repository, + ) + if self.exists_locally(): + self.fetch() + else: + self.clone() + + def get_current_commit_sha(self) -> str: + result = subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "rev-parse", + "HEAD", + ], + capture_output=True, + text=True, + check=True, + ) + + return result.stdout.strip() + + def verify_repository_integrity(self) -> bool: + git_directory = self.local_path / ".git" + + return ( + self.local_path.exists() + and self.local_path.is_dir() + and git_directory.exists() + ) diff --git a/application/utils/harvester/repository_cache.py b/application/utils/harvester/repository_cache.py new file mode 100644 index 000000000..f2ee3343e --- /dev/null +++ b/application/utils/harvester/repository_cache.py @@ -0,0 +1,7 @@ +from pathlib import Path + +CACHE_ROOT = Path(".harvester_cache") + + +def build_repository_cache_path(owner: str, repository: str) -> Path: + return CACHE_ROOT / owner.casefold() / repository.casefold() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py new file mode 100644 index 000000000..6e6df5439 --- /dev/null +++ b/application/utils/harvester/repository_client.py @@ -0,0 +1,24 @@ +from abc import ABC, abstractmethod +from pathlib import Path + + +class RepositoryClient(ABC): + @abstractmethod + def clone(self) -> None: + """Clone repository locally.""" + + @abstractmethod + def fetch(self) -> None: + """Fetch latest remote changes.""" + + @abstractmethod + def checkout(self, reference: str) -> None: + """Checkout repository reference.""" + + @abstractmethod + def get_local_path(self) -> Path: + """Return local repository path.""" + + @abstractmethod + def exists_locally(self) -> bool: + """Check if repository already exists locally.""" From 29fec6aa652f43540f758d7a705f4756f61c4a7d Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 16:03:50 +0530 Subject: [PATCH 15/23] Addressing coderabbit and adding couple of extra gaurdrails --- .../test_git_repository_client.py | 81 ++++++++++++++++++- .../harvester_test/test_repository_cache.py | 18 ++++- application/utils/harvester/__init__.py | 7 ++ .../utils/harvester/git_repository_client.py | 37 ++++++++- .../utils/harvester/repository_cache.py | 11 ++- .../utils/harvester/repository_client.py | 19 ++++- 6 files changed, 162 insertions(+), 11 deletions(-) diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py index b0aa7678d..1f622fb39 100644 --- a/application/tests/harvester_test/test_git_repository_client.py +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -2,6 +2,8 @@ GitRepositoryClient, ) +from unittest.mock import patch + def test_repository_url_generation(): client = GitRepositoryClient( @@ -18,7 +20,7 @@ def test_local_repository_path(): repository="ASVS", ) - assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs" + assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs/main" def test_repository_exists_locally_false(): @@ -37,3 +39,80 @@ def test_verify_repository_integrity_false(): ) assert client.verify_repository_integrity() is False + + +def test_sync_clones_when_repository_missing(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=False, + ), + patch.object(client, "clone") as mock_clone, + ): + client.sync() + + mock_clone.assert_called_once() + + +def test_sync_fetches_when_repository_exists(): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=True, + ), + patch.object(client, "fetch") as mock_fetch, + ): + client.sync() + + mock_fetch.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_fetch_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.fetch() + + mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_checkout_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.checkout("main") + + mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_get_current_commit_sha_runs_git_command(mock_run): + mock_run.return_value.stdout = "abc123\n" + + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + sha = client.get_current_commit_sha() + + assert sha == "abc123" + mock_run.assert_called_once() diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py index 5cf4b745f..4a33601d9 100644 --- a/application/tests/harvester_test/test_repository_cache.py +++ b/application/tests/harvester_test/test_repository_cache.py @@ -9,4 +9,20 @@ def test_build_repository_cache_path(): "ASVS", ) - assert str(path) == ".harvester_cache/owasp/asvs" + assert str(path) == ".harvester_cache/owasp/asvs/main" + + +def test_different_branches_have_different_cache_paths(): + main_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="main", + ) + + dev_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="dev", + ) + + assert main_path != dev_path diff --git a/application/utils/harvester/__init__.py b/application/utils/harvester/__init__.py index 9c74939c7..2ac608b3e 100644 --- a/application/utils/harvester/__init__.py +++ b/application/utils/harvester/__init__.py @@ -14,11 +14,18 @@ ReposFile, ) +from .git_repository_client import GitRepositoryClient +from .repository_client import RepositoryClient +from .repository_cache import build_repository_cache_path + __all__ = [ + "build_repository_cache_path", "ChunkingConfig", "ConfigLoaderError", + "GitRepositoryClient", "PathRules", "PollingConfig", + "RepositoryClient", "RepositoryConfig", "RepositoryValidationError", "ReposFile", diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index de97836ed..9e6584efc 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -17,6 +17,7 @@ def __init__(self, owner: str, repository: str, branch: str = "main") -> None: self.local_path = build_repository_cache_path( owner, repository, + branch, ) @property @@ -24,12 +25,24 @@ def repository_url(self) -> str: return f"https://github.com/{self.owner}/{self.repository}.git" def clone(self) -> None: + if self.exists_locally(): + logger.warning( + "Repository %s/%s already exists locally", + self.owner, + self.repository, + ) + return + logger.info( "Cloning repository %s/%s", self.owner, self.repository, ) - self.local_path.parent.mkdir(parents=True, exist_ok=True) + + self.local_path.parent.mkdir( + parents=True, + exist_ok=True, + ) subprocess.run( [ @@ -41,6 +54,9 @@ def clone(self) -> None: str(self.local_path), ], check=True, + capture_output=True, + text=True, + timeout=300, ) def fetch(self) -> None: @@ -59,9 +75,19 @@ def fetch(self) -> None: "--all", ], check=True, + capture_output=True, + text=True, + timeout=300, ) def checkout(self, reference: str) -> None: + logger.info( + "Checking out %s in %s/%s", + reference, + self.owner, + self.repository, + ) + subprocess.run( [ "git", @@ -71,6 +97,9 @@ def checkout(self, reference: str) -> None: reference, ], check=True, + capture_output=True, + text=True, + timeout=300, ) def get_local_path(self) -> Path: @@ -85,7 +114,8 @@ def sync(self) -> None: self.owner, self.repository, ) - if self.exists_locally(): + + if self.verify_repository_integrity(): self.fetch() else: self.clone() @@ -99,9 +129,10 @@ def get_current_commit_sha(self) -> str: "rev-parse", "HEAD", ], + check=True, capture_output=True, text=True, - check=True, + timeout=300, ) return result.stdout.strip() diff --git a/application/utils/harvester/repository_cache.py b/application/utils/harvester/repository_cache.py index f2ee3343e..418425681 100644 --- a/application/utils/harvester/repository_cache.py +++ b/application/utils/harvester/repository_cache.py @@ -1,7 +1,12 @@ +import os from pathlib import Path -CACHE_ROOT = Path(".harvester_cache") +CACHE_ROOT = Path(os.getenv("HARVESTER_CACHE_DIR", ".harvester_cache")) -def build_repository_cache_path(owner: str, repository: str) -> Path: - return CACHE_ROOT / owner.casefold() / repository.casefold() +def build_repository_cache_path( + owner: str, + repository: str, + branch: str = "main", +) -> Path: + return CACHE_ROOT / owner.casefold() / repository.casefold() / branch.casefold() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py index 6e6df5439..22ee67700 100644 --- a/application/utils/harvester/repository_client.py +++ b/application/utils/harvester/repository_client.py @@ -9,11 +9,11 @@ def clone(self) -> None: @abstractmethod def fetch(self) -> None: - """Fetch latest remote changes.""" + """Fetch latest repository changes.""" @abstractmethod def checkout(self, reference: str) -> None: - """Checkout repository reference.""" + """Checkout a branch, tag, or commit.""" @abstractmethod def get_local_path(self) -> Path: @@ -21,4 +21,17 @@ def get_local_path(self) -> Path: @abstractmethod def exists_locally(self) -> bool: - """Check if repository already exists locally.""" + """Return whether repository exists locally.""" + + @abstractmethod + def sync(self) -> None: + """Clone if missing, otherwise fetch latest changes.""" + + @abstractmethod + def get_current_commit_sha(self) -> str: + """Return HEAD commit SHA.""" + + @abstractmethod + def verify_repository_integrity(self) -> bool: + """Verify local repository integrity.""" + From 278302315ba8bc228884e644be1c347bfe8fc702 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Wed, 10 Jun 2026 16:24:25 +0530 Subject: [PATCH 16/23] Addressing coderabbit final --- .../test_git_repository_client.py | 17 +++ .../utils/harvester/git_repository_client.py | 143 +++++++++++------- .../utils/harvester/repository_client.py | 1 - 3 files changed, 107 insertions(+), 54 deletions(-) diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py index 1f622fb39..151272345 100644 --- a/application/tests/harvester_test/test_git_repository_client.py +++ b/application/tests/harvester_test/test_git_repository_client.py @@ -116,3 +116,20 @@ def test_get_current_commit_sha_runs_git_command(mock_run): assert sha == "abc123" mock_run.assert_called_once() + + +@patch("application.utils.harvester.git_repository_client.subprocess.run") +def test_clone_runs_git_command(mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with patch.object( + client, + "verify_repository_integrity", + return_value=False, + ): + client.clone() + + mock_run.assert_called_once() diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 9e6584efc..793fb5535 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -25,7 +25,7 @@ def repository_url(self) -> str: return f"https://github.com/{self.owner}/{self.repository}.git" def clone(self) -> None: - if self.exists_locally(): + if self.verify_repository_integrity(): logger.warning( "Repository %s/%s already exists locally", self.owner, @@ -44,20 +44,29 @@ def clone(self) -> None: exist_ok=True, ) - subprocess.run( - [ - "git", - "clone", - "--branch", - self.branch, - self.repository_url, - str(self.local_path), - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + subprocess.run( + [ + "git", + "clone", + "--branch", + self.branch, + self.repository_url, + str(self.local_path), + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to clone repository %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise def fetch(self) -> None: logger.info( @@ -66,19 +75,28 @@ def fetch(self) -> None: self.repository, ) - subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "fetch", - "--all", - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "fetch", + "--all", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to fetch repository %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise def checkout(self, reference: str) -> None: logger.info( @@ -88,19 +106,29 @@ def checkout(self, reference: str) -> None: self.repository, ) - subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "checkout", + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "checkout", + reference, + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to checkout %s in %s/%s: %s", reference, - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + self.owner, + self.repository, + exc.stderr, + ) + raise def get_local_path(self) -> Path: return self.local_path @@ -121,19 +149,28 @@ def sync(self) -> None: self.clone() def get_current_commit_sha(self) -> str: - result = subprocess.run( - [ - "git", - "-C", - str(self.local_path), - "rev-parse", - "HEAD", - ], - check=True, - capture_output=True, - text=True, - timeout=300, - ) + try: + result = subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "rev-parse", + "HEAD", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + except subprocess.CalledProcessError as exc: + logger.error( + "Failed to retrieve commit SHA for %s/%s: %s", + self.owner, + self.repository, + exc.stderr, + ) + raise return result.stdout.strip() diff --git a/application/utils/harvester/repository_client.py b/application/utils/harvester/repository_client.py index 22ee67700..c545cce42 100644 --- a/application/utils/harvester/repository_client.py +++ b/application/utils/harvester/repository_client.py @@ -34,4 +34,3 @@ def get_current_commit_sha(self) -> str: @abstractmethod def verify_repository_integrity(self) -> bool: """Verify local repository integrity.""" - From 721afa8b194ff2b4512cf02b4011887e09845d89 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Mon, 6 Jul 2026 17:17:21 +0530 Subject: [PATCH 17/23] test(harvester): align week 2 tests with project conventions --- .../git_repository_client_test.py | 138 ++++++++++++++++++ .../harvester_test/repository_cache_test.py | 37 +++++ .../test_git_repository_client.py | 135 ----------------- .../harvester_test/test_repository_cache.py | 28 ---- 4 files changed, 175 insertions(+), 163 deletions(-) create mode 100644 application/tests/harvester_test/git_repository_client_test.py create mode 100644 application/tests/harvester_test/repository_cache_test.py delete mode 100644 application/tests/harvester_test/test_git_repository_client.py delete mode 100644 application/tests/harvester_test/test_repository_cache.py diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py new file mode 100644 index 000000000..4f548a8be --- /dev/null +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -0,0 +1,138 @@ +import unittest +from unittest.mock import patch + +from application.utils.harvester.git_repository_client import ( + GitRepositoryClient, +) + + +class GitRepositoryClientTests(unittest.TestCase): + def test_repository_url_generation(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertEqual( + client.repository_url, + "https://github.com/OWASP/ASVS.git", + ) + + def test_local_repository_path(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertEqual( + str(client.get_local_path()), + ".harvester_cache/owasp/asvs/main", + ) + + def test_repository_exists_locally_false(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertFalse(client.exists_locally()) + + def test_verify_repository_integrity_false(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + self.assertFalse(client.verify_repository_integrity()) + + def test_sync_clones_when_repository_missing(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=False, + ), + patch.object(client, "clone") as mock_clone, + ): + client.sync() + + mock_clone.assert_called_once() + + def test_sync_fetches_when_repository_exists(self): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with ( + patch.object( + client, + "verify_repository_integrity", + return_value=True, + ), + patch.object(client, "fetch") as mock_fetch, + ): + client.sync() + + mock_fetch.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_fetch_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.fetch() + + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_checkout_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + client.checkout("main") + + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_get_current_commit_sha_runs_git_command(self, mock_run): + mock_run.return_value.stdout = "abc123\n" + + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + sha = client.get_current_commit_sha() + + self.assertEqual(sha, "abc123") + mock_run.assert_called_once() + + @patch("application.utils.harvester.git_repository_client.subprocess.run") + def test_clone_runs_git_command(self, mock_run): + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + ) + + with patch.object( + client, + "verify_repository_integrity", + return_value=False, + ): + client.clone() + + mock_run.assert_called_once() + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/repository_cache_test.py b/application/tests/harvester_test/repository_cache_test.py new file mode 100644 index 000000000..547a4ca23 --- /dev/null +++ b/application/tests/harvester_test/repository_cache_test.py @@ -0,0 +1,37 @@ +import unittest + +from application.utils.harvester.repository_cache import ( + build_repository_cache_path, +) + + +class RepositoryCacheTests(unittest.TestCase): + def test_build_repository_cache_path(self): + path = build_repository_cache_path( + "OWASP", + "ASVS", + ) + + self.assertEqual( + str(path), + ".harvester_cache/owasp/asvs/main", + ) + + def test_different_branches_have_different_cache_paths(self): + main_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="main", + ) + + dev_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="dev", + ) + + self.assertNotEqual(main_path, dev_path) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/harvester_test/test_git_repository_client.py b/application/tests/harvester_test/test_git_repository_client.py deleted file mode 100644 index 151272345..000000000 --- a/application/tests/harvester_test/test_git_repository_client.py +++ /dev/null @@ -1,135 +0,0 @@ -from application.utils.harvester.git_repository_client import ( - GitRepositoryClient, -) - -from unittest.mock import patch - - -def test_repository_url_generation(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.repository_url == "https://github.com/OWASP/ASVS.git" - - -def test_local_repository_path(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert str(client.get_local_path()) == ".harvester_cache/owasp/asvs/main" - - -def test_repository_exists_locally_false(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.exists_locally() is False - - -def test_verify_repository_integrity_false(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - assert client.verify_repository_integrity() is False - - -def test_sync_clones_when_repository_missing(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with ( - patch.object( - client, - "verify_repository_integrity", - return_value=False, - ), - patch.object(client, "clone") as mock_clone, - ): - client.sync() - - mock_clone.assert_called_once() - - -def test_sync_fetches_when_repository_exists(): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with ( - patch.object( - client, - "verify_repository_integrity", - return_value=True, - ), - patch.object(client, "fetch") as mock_fetch, - ): - client.sync() - - mock_fetch.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_fetch_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - client.fetch() - - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_checkout_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - client.checkout("main") - - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_get_current_commit_sha_runs_git_command(mock_run): - mock_run.return_value.stdout = "abc123\n" - - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - sha = client.get_current_commit_sha() - - assert sha == "abc123" - mock_run.assert_called_once() - - -@patch("application.utils.harvester.git_repository_client.subprocess.run") -def test_clone_runs_git_command(mock_run): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) - - with patch.object( - client, - "verify_repository_integrity", - return_value=False, - ): - client.clone() - - mock_run.assert_called_once() diff --git a/application/tests/harvester_test/test_repository_cache.py b/application/tests/harvester_test/test_repository_cache.py deleted file mode 100644 index 4a33601d9..000000000 --- a/application/tests/harvester_test/test_repository_cache.py +++ /dev/null @@ -1,28 +0,0 @@ -from application.utils.harvester.repository_cache import ( - build_repository_cache_path, -) - - -def test_build_repository_cache_path(): - path = build_repository_cache_path( - "OWASP", - "ASVS", - ) - - assert str(path) == ".harvester_cache/owasp/asvs/main" - - -def test_different_branches_have_different_cache_paths(): - main_path = build_repository_cache_path( - owner="OWASP", - repository="ASVS", - branch="main", - ) - - dev_path = build_repository_cache_path( - owner="OWASP", - repository="ASVS", - branch="dev", - ) - - assert main_path != dev_path From 1738cb429141169f6605f256864ecd41386fd1c8 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sat, 18 Jul 2026 18:01:08 +0530 Subject: [PATCH 18/23] fix(harvester): address repository client review feedback --- application/tests/harvester_test/repository_cache_test.py | 5 +++-- application/utils/harvester/git_repository_client.py | 1 + application/utils/harvester/repos.yaml | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/application/tests/harvester_test/repository_cache_test.py b/application/tests/harvester_test/repository_cache_test.py index 547a4ca23..a308239f1 100644 --- a/application/tests/harvester_test/repository_cache_test.py +++ b/application/tests/harvester_test/repository_cache_test.py @@ -1,4 +1,5 @@ import unittest +from pathlib import Path from application.utils.harvester.repository_cache import ( build_repository_cache_path, @@ -13,8 +14,8 @@ def test_build_repository_cache_path(self): ) self.assertEqual( - str(path), - ".harvester_cache/owasp/asvs/main", + path, + Path(".harvester_cache/owasp/asvs/main"), ) def test_different_branches_have_different_cache_paths(self): diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 793fb5535..eff7c17d6 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -113,6 +113,7 @@ def checkout(self, reference: str) -> None: "-C", str(self.local_path), "checkout", + "--", reference, ], check=True, diff --git a/application/utils/harvester/repos.yaml b/application/utils/harvester/repos.yaml index f39d07257..c448e5111 100644 --- a/application/utils/harvester/repos.yaml +++ b/application/utils/harvester/repos.yaml @@ -13,7 +13,7 @@ repositories: - "**/archive/**" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1200 overlap_tokens: 100 @@ -34,7 +34,7 @@ repositories: - "cheatsheets/**/*.md" chunking: - strategy: markdown + strategy: markdown_heading max_tokens: 1000 overlap_tokens: 100 From 1638e0baae961d01212c9b80659b6e09b4ae528f Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 19 Jul 2026 23:37:56 +0530 Subject: [PATCH 19/23] test(harvester): strengthen configuration validation coverage --- .../harvester_test/config_loader_test.py | 59 +++++++++++++++++++ .../fixtures/invalid_overlap_tokens.yaml | 16 +++++ .../fixtures/whitespace_branch.yaml | 16 +++++ .../fixtures/whitespace_id.yaml | 16 +++++ .../fixtures/whitespace_owner.yaml | 16 +++++ .../fixtures/whitespace_repo.yaml | 16 +++++ application/utils/harvester/schemas.py | 2 +- 7 files changed, 140 insertions(+), 1 deletion(-) create mode 100644 application/tests/harvester_test/fixtures/invalid_overlap_tokens.yaml create mode 100644 application/tests/harvester_test/fixtures/whitespace_branch.yaml create mode 100644 application/tests/harvester_test/fixtures/whitespace_id.yaml create mode 100644 application/tests/harvester_test/fixtures/whitespace_owner.yaml create mode 100644 application/tests/harvester_test/fixtures/whitespace_repo.yaml diff --git a/application/tests/harvester_test/config_loader_test.py b/application/tests/harvester_test/config_loader_test.py index 01c404f18..730f4d708 100644 --- a/application/tests/harvester_test/config_loader_test.py +++ b/application/tests/harvester_test/config_loader_test.py @@ -58,6 +58,65 @@ def test_empty_include_paths(self): with self.assertRaises(ConfigLoaderError): load_repo_config(config_path) + def test_whitespace_only_repository_id(self): + config_path = FIXTURES_DIR / "whitespace_id.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_whitespace_only_owner(self): + config_path = FIXTURES_DIR / "whitespace_owner.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_whitespace_only_repo(self): + config_path = FIXTURES_DIR / "whitespace_repo.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_whitespace_only_branch(self): + config_path = FIXTURES_DIR / "whitespace_branch.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_overlap_tokens_must_be_less_than_max_tokens(self): + config_path = FIXTURES_DIR / "invalid_overlap_tokens.yaml" + + with self.assertRaisesRegex(ConfigLoaderError, "overlap_tokens"): + load_repo_config(config_path) + + def test_load_packaged_repos_yaml(self): + config_path = ( + Path(__file__).resolve().parents[2] / "utils" / "harvester" / "repos.yaml" + ) + + config = load_repo_config(config_path) + + self.assertEqual(len(config.repositories), 2) + self.assertEqual(config.repositories[0].id, "owasp-asvs") + self.assertEqual(config.repositories[1].id, "owasp-cheatsheets") + + def test_empty_owner(self): + config_path = FIXTURES_DIR / "empty_owner.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_invalid_chunking_strategy(self): + config_path = FIXTURES_DIR / "invalid_chunking_strategy.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + + def test_invalid_polling_mode(self): + config_path = FIXTURES_DIR / "invalid_polling_mode.yaml" + + with self.assertRaises(ConfigLoaderError): + load_repo_config(config_path) + if __name__ == "__main__": unittest.main() diff --git a/application/tests/harvester_test/fixtures/invalid_overlap_tokens.yaml b/application/tests/harvester_test/fixtures/invalid_overlap_tokens.yaml new file mode 100644 index 000000000..36d64b11c --- /dev/null +++ b/application/tests/harvester_test/fixtures/invalid_overlap_tokens.yaml @@ -0,0 +1,16 @@ +repositories: + - id: invalid-overlap + type: github + owner: OWASP + repo: ASVS + branch: master + paths: + include: + - "docs/**/*.md" + chunking: + strategy: markdown_heading + max_tokens: 100 + overlap_tokens: 100 + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/whitespace_branch.yaml b/application/tests/harvester_test/fixtures/whitespace_branch.yaml new file mode 100644 index 000000000..35dbdf578 --- /dev/null +++ b/application/tests/harvester_test/fixtures/whitespace_branch.yaml @@ -0,0 +1,16 @@ +repositories: + - id: whitespace-branch + type: github + owner: OWASP + repo: ASVS + branch: " " + paths: + include: + - "docs/**/*.md" + chunking: + strategy: markdown_heading + max_tokens: 1200 + overlap_tokens: 100 + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/whitespace_id.yaml b/application/tests/harvester_test/fixtures/whitespace_id.yaml new file mode 100644 index 000000000..0a5d00175 --- /dev/null +++ b/application/tests/harvester_test/fixtures/whitespace_id.yaml @@ -0,0 +1,16 @@ +repositories: + - id: " " + type: github + owner: OWASP + repo: ASVS + branch: master + paths: + include: + - "docs/**/*.md" + chunking: + strategy: markdown_heading + max_tokens: 1200 + overlap_tokens: 100 + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/whitespace_owner.yaml b/application/tests/harvester_test/fixtures/whitespace_owner.yaml new file mode 100644 index 000000000..112ef72e8 --- /dev/null +++ b/application/tests/harvester_test/fixtures/whitespace_owner.yaml @@ -0,0 +1,16 @@ +repositories: + - id: whitespace-owner + type: github + owner: " " + repo: ASVS + branch: master + paths: + include: + - "docs/**/*.md" + chunking: + strategy: markdown_heading + max_tokens: 1200 + overlap_tokens: 100 + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/whitespace_repo.yaml b/application/tests/harvester_test/fixtures/whitespace_repo.yaml new file mode 100644 index 000000000..772da966c --- /dev/null +++ b/application/tests/harvester_test/fixtures/whitespace_repo.yaml @@ -0,0 +1,16 @@ +repositories: + - id: whitespace-repo + type: github + owner: OWASP + repo: " " + branch: master + paths: + include: + - "docs/**/*.md" + chunking: + strategy: markdown_heading + max_tokens: 1200 + overlap_tokens: 100 + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/utils/harvester/schemas.py b/application/utils/harvester/schemas.py index c65d62b60..4589f3d69 100644 --- a/application/utils/harvester/schemas.py +++ b/application/utils/harvester/schemas.py @@ -56,7 +56,7 @@ class PollingConfig(BaseModel): # top level repository ingestion configuration class RepositoryConfig(BaseModel): - model_config = ConfigDict(extra="forbid") + model_config = ConfigDict(extra="forbid", str_strip_whitespace=True) id: str = Field( ..., From 19b6cccdfb4e3d74c70500134842cb85b60fd78b Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Sun, 19 Jul 2026 23:56:49 +0530 Subject: [PATCH 20/23] test(harvester): add case-insensitive repository validation tests --- .../fixtures/duplicate_repo_ids_case.yaml | 34 +++++++++++++++++++ .../fixtures/duplicate_repositories_case.yaml | 34 +++++++++++++++++++ .../harvester_test/repos_validator_test.py | 22 ++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 application/tests/harvester_test/fixtures/duplicate_repo_ids_case.yaml create mode 100644 application/tests/harvester_test/fixtures/duplicate_repositories_case.yaml diff --git a/application/tests/harvester_test/fixtures/duplicate_repo_ids_case.yaml b/application/tests/harvester_test/fixtures/duplicate_repo_ids_case.yaml new file mode 100644 index 000000000..10df90943 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repo_ids_case.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown_heading + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: ASVS + type: github + owner: OWASP + repo: CheatSheetSeries + + paths: + include: + - "cheatsheets/**/*.md" + + chunking: + strategy: markdown_heading + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/fixtures/duplicate_repositories_case.yaml b/application/tests/harvester_test/fixtures/duplicate_repositories_case.yaml new file mode 100644 index 000000000..ed9dd64d3 --- /dev/null +++ b/application/tests/harvester_test/fixtures/duplicate_repositories_case.yaml @@ -0,0 +1,34 @@ +repositories: + - id: asvs-1 + type: github + owner: OWASP + repo: ASVS + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown_heading + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 + + - id: asvs-2 + type: github + owner: owasp + repo: asvs + + paths: + include: + - "4.0/en/**/*.md" + + chunking: + strategy: markdown_heading + max_tokens: 1200 + + polling: + mode: incremental + interval_minutes: 60 diff --git a/application/tests/harvester_test/repos_validator_test.py b/application/tests/harvester_test/repos_validator_test.py index 3e0347d41..278018e26 100644 --- a/application/tests/harvester_test/repos_validator_test.py +++ b/application/tests/harvester_test/repos_validator_test.py @@ -53,6 +53,28 @@ def test_validate_valid_repositories(self): validate_repositories(config) + def test_duplicate_repository_ids_case_insensitive(self): + config_path = FIXTURES_DIR / "duplicate_repo_ids_case.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository id", + ): + validate_repositories(config) + + def test_duplicate_repositories_case_insensitive(self): + config_path = FIXTURES_DIR / "duplicate_repositories_case.yaml" + + config = load_repo_config(config_path) + + with self.assertRaisesRegex( + RepositoryValidationError, + "Duplicate repository detected", + ): + validate_repositories(config) + if __name__ == "__main__": unittest.main() From bb4a08b047c27cf663ceb68a6981a95d3b841ca4 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Tue, 21 Jul 2026 18:05:12 +0530 Subject: [PATCH 21/23] Validate repository cache paths and preserve branch identity --- .../git_repository_client_test.py | 16 +++- .../harvester_test/repository_cache_test.py | 46 ++++++++++ .../utils/harvester/git_repository_client.py | 83 +++++++++++++++++-- .../utils/harvester/repository_cache.py | 22 ++++- 4 files changed, 153 insertions(+), 14 deletions(-) diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py index 4f548a8be..4929f7810 100644 --- a/application/tests/harvester_test/git_repository_client_test.py +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -90,7 +90,7 @@ def test_fetch_runs_git_command(self, mock_run): client.fetch() - mock_run.assert_called_once() + self.assertEqual(mock_run.call_count, 2) @patch("application.utils.harvester.git_repository_client.subprocess.run") def test_checkout_runs_git_command(self, mock_run): @@ -101,7 +101,19 @@ def test_checkout_runs_git_command(self, mock_run): client.checkout("main") - mock_run.assert_called_once() + mock_run.assert_called_once_with( + [ + "git", + "-C", + str(client.get_local_path()), + "checkout", + "main", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) @patch("application.utils.harvester.git_repository_client.subprocess.run") def test_get_current_commit_sha_runs_git_command(self, mock_run): diff --git a/application/tests/harvester_test/repository_cache_test.py b/application/tests/harvester_test/repository_cache_test.py index a308239f1..d3193d454 100644 --- a/application/tests/harvester_test/repository_cache_test.py +++ b/application/tests/harvester_test/repository_cache_test.py @@ -33,6 +33,52 @@ def test_different_branches_have_different_cache_paths(self): self.assertNotEqual(main_path, dev_path) + def test_case_sensitive_branches_have_different_cache_paths(self): + release_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="Release", + ) + + release_lower_path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="release", + ) + + self.assertNotEqual(release_path, release_lower_path) + + def test_path_traversal_owner_rejected(self): + with self.assertRaises(ValueError): + build_repository_cache_path( + owner="../../tmp", + repository="ASVS", + ) + + def test_absolute_owner_rejected(self): + with self.assertRaises(ValueError): + build_repository_cache_path( + owner="/tmp", + repository="ASVS", + ) + + def test_invalid_repository_name_rejected(self): + with self.assertRaises(ValueError): + build_repository_cache_path( + owner="OWASP", + repository="../ASVS", + ) + + def test_branch_path_is_encoded(self): + path = build_repository_cache_path( + owner="OWASP", + repository="ASVS", + branch="feature/test", + ) + + self.assertNotIn("feature/test", str(path)) + self.assertIn("feature%2Ftest", str(path)) + if __name__ == "__main__": unittest.main() diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index eff7c17d6..afc18348f 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -82,13 +82,30 @@ def fetch(self) -> None: "-C", str(self.local_path), "fetch", - "--all", + "origin", + self.branch, + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "reset", + "--hard", + f"origin/{self.branch}", ], check=True, capture_output=True, text=True, timeout=300, ) + except subprocess.CalledProcessError as exc: logger.error( "Failed to fetch repository %s/%s: %s", @@ -99,6 +116,9 @@ def fetch(self) -> None: raise def checkout(self, reference: str) -> None: + if reference.startswith("-"): + raise ValueError("Invalid git reference") + logger.info( "Checking out %s in %s/%s", reference, @@ -113,7 +133,6 @@ def checkout(self, reference: str) -> None: "-C", str(self.local_path), "checkout", - "--", reference, ], check=True, @@ -176,10 +195,58 @@ def get_current_commit_sha(self) -> str: return result.stdout.strip() def verify_repository_integrity(self) -> bool: - git_directory = self.local_path / ".git" + if not (self.local_path.exists() and self.local_path.is_dir()): + return False - return ( - self.local_path.exists() - and self.local_path.is_dir() - and git_directory.exists() - ) + try: + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "rev-parse", + "--is-inside-work-tree", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + + remote = subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "config", + "--get", + "remote.origin.url", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ).stdout.strip() + + if remote.rstrip("/") != self.repository_url.rstrip("/"): + return False + + subprocess.run( + [ + "git", + "-C", + str(self.local_path), + "show-ref", + "--verify", + f"refs/remotes/origin/{self.branch}", + ], + check=True, + capture_output=True, + text=True, + timeout=300, + ) + + return True + + except subprocess.CalledProcessError: + return False diff --git a/application/utils/harvester/repository_cache.py b/application/utils/harvester/repository_cache.py index 418425681..94b721a60 100644 --- a/application/utils/harvester/repository_cache.py +++ b/application/utils/harvester/repository_cache.py @@ -1,12 +1,26 @@ import os +import re + from pathlib import Path +from urllib.parse import quote CACHE_ROOT = Path(os.getenv("HARVESTER_CACHE_DIR", ".harvester_cache")) +_VALID_COMPONENT = re.compile(r"^[A-Za-z0-9_.-]+$") + def build_repository_cache_path( - owner: str, - repository: str, - branch: str = "main", + owner: str, repository: str, branch: str = "main" ) -> Path: - return CACHE_ROOT / owner.casefold() / repository.casefold() / branch.casefold() + if not _VALID_COMPONENT.fullmatch(owner): + raise ValueError(f"Invalid repository owner: {owner}") + + if not _VALID_COMPONENT.fullmatch(repository): + raise ValueError(f"Invalid repository name: {repository}") + + encoded_branch = quote(branch, safe="") + candidate = CACHE_ROOT / owner.casefold() / repository.casefold() / encoded_branch + + candidate.resolve().relative_to(CACHE_ROOT.resolve()) + + return candidate From dd1148b4d90730ad3515b92784137491f0bda47f Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Tue, 21 Jul 2026 19:20:35 +0530 Subject: [PATCH 22/23] Add integration tests for Git repository client --- .../git_repository_client_integration_test.py | 170 ++++++++++++++++++ .../utils/harvester/git_repository_client.py | 16 +- 2 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 application/tests/harvester_test/git_repository_client_integration_test.py diff --git a/application/tests/harvester_test/git_repository_client_integration_test.py b/application/tests/harvester_test/git_repository_client_integration_test.py new file mode 100644 index 000000000..c11401bc1 --- /dev/null +++ b/application/tests/harvester_test/git_repository_client_integration_test.py @@ -0,0 +1,170 @@ +import subprocess +import tempfile +import unittest +from pathlib import Path + +from application.utils.harvester.git_repository_client import ( + GitRepositoryClient, +) + + +class IntegrationGitRepositoryClient(GitRepositoryClient): + def __init__(self, *args, repository_url: str, **kwargs): + super().__init__(*args, **kwargs) + self._repository_url = repository_url + + @property + def repository_url(self) -> str: + return self._repository_url + + +def git(*args, cwd=None): + subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ) + + +def git_output(*args, cwd=None): + return subprocess.run( + ["git", *args], + cwd=cwd, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +class GitRepositoryClientIntegrationTests(unittest.TestCase): + def setUp(self): + self.tempdir = tempfile.TemporaryDirectory() + self.root = Path(self.tempdir.name) + + self.remote = self.root / "remote.git" + self.work = self.root / "work" + self.cache = self.root / "cache" + + git("init", "--bare", self.remote) + + git("clone", self.remote, self.work) + + git("config", "user.name", "Test User", cwd=self.work) + git("config", "user.email", "test@example.com", cwd=self.work) + git("checkout", "-b", "main", cwd=self.work) + + (self.work / "test.txt").write_text("v1") + + git("add", ".", cwd=self.work) + git("commit", "-m", "initial", cwd=self.work) + git("push", "origin", "main", cwd=self.work) + + def tearDown(self): + self.tempdir.cleanup() + + def create_client(self): + return IntegrationGitRepositoryClient( + owner="OWASP", + repository="ASVS", + local_path=self.cache, + repository_url=str(self.remote), + ) + + def test_fetch_updates_worktree_and_commit(self): + client = self.create_client() + + client.clone() + + sha1 = client.get_current_commit_sha() + + self.assertEqual( + (client.get_local_path() / "test.txt").read_text(), + "v1", + ) + + (self.work / "test.txt").write_text("v2") + + git("add", ".", cwd=self.work) + git("commit", "-m", "update", cwd=self.work) + git("push", "origin", "main", cwd=self.work) + + expected_sha = git_output( + "rev-parse", + "HEAD", + cwd=self.work, + ) + + client.fetch() + + self.assertEqual( + client.get_current_commit_sha(), + expected_sha, + ) + + self.assertNotEqual( + sha1, + expected_sha, + ) + + self.assertEqual( + (client.get_local_path() / "test.txt").read_text(), + "v2", + ) + + def test_verify_repository_integrity_rejects_fake_git_directory(self): + fake = self.root / "fake" + + fake.mkdir() + (fake / ".git").mkdir() + + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + local_path=fake, + ) + + self.assertFalse( + client.verify_repository_integrity(), + ) + + def test_verify_repository_integrity_rejects_wrong_origin(self): + other_remote = self.root / "other.git" + + git("init", "--bare", other_remote) + + client = self.create_client() + + client.clone() + + git( + "remote", + "set-url", + "origin", + other_remote, + cwd=client.get_local_path(), + ) + + self.assertFalse( + client.verify_repository_integrity(), + ) + + def test_verify_repository_integrity_rejects_missing_branch(self): + client = IntegrationGitRepositoryClient( + owner="OWASP", + repository="ASVS", + branch="dev", + local_path=self.cache, + repository_url=str(self.remote), + ) + + git("clone", self.remote, self.cache) + + self.assertFalse( + client.verify_repository_integrity(), + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index afc18348f..05af12987 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -9,15 +9,21 @@ class GitRepositoryClient(RepositoryClient): - def __init__(self, owner: str, repository: str, branch: str = "main") -> None: + def __init__( + self, + owner: str, + repository: str, + branch: str = "main", + local_path: Path | None = None, + ) -> None: self.owner = owner self.repository = repository self.branch = branch - self.local_path = build_repository_cache_path( - owner, - repository, - branch, + self.local_path = ( + local_path + if local_path is not None + else build_repository_cache_path(owner, repository, branch) ) @property From a1c590f0a48140967ba9003985673938641e2625 Mon Sep 17 00:00:00 2001 From: ParthAggarwal16 Date: Tue, 21 Jul 2026 22:05:43 +0530 Subject: [PATCH 23/23] Implement atomic repository cloning and locking --- .../git_repository_client_integration_test.py | 36 +++++++++++ .../git_repository_client_test.py | 36 ++++++----- .../utils/harvester/git_repository_client.py | 59 +++++++++++++------ .../utils/harvester/repository_lock.py | 32 ++++++++++ 4 files changed, 131 insertions(+), 32 deletions(-) create mode 100644 application/utils/harvester/repository_lock.py diff --git a/application/tests/harvester_test/git_repository_client_integration_test.py b/application/tests/harvester_test/git_repository_client_integration_test.py index c11401bc1..4803e1d67 100644 --- a/application/tests/harvester_test/git_repository_client_integration_test.py +++ b/application/tests/harvester_test/git_repository_client_integration_test.py @@ -2,6 +2,7 @@ import tempfile import unittest from pathlib import Path +import threading from application.utils.harvester.git_repository_client import ( GitRepositoryClient, @@ -165,6 +166,41 @@ def test_verify_repository_integrity_rejects_missing_branch(self): client.verify_repository_integrity(), ) + def test_sync_serializes_clone_operations(self): + client1 = self.create_client() + client2 = self.create_client() + + exceptions = [] + + def run_sync(client): + try: + client.sync() + except Exception as exc: + exceptions.append(exc) + + t1 = threading.Thread(target=run_sync, args=(client1,)) + t2 = threading.Thread(target=run_sync, args=(client2,)) + + t1.start() + t2.start() + + t1.join() + t2.join() + + self.assertFalse(exceptions, f"Unexpected exceptions: {exceptions}") + + self.assertTrue(client1.verify_repository_integrity()) + self.assertTrue(client2.verify_repository_integrity()) + + self.assertTrue((self.cache / ".git").exists()) + + self.assertEqual( + client1.get_current_commit_sha(), + client2.get_current_commit_sha(), + ) + + self.assertEqual((self.cache / "test.txt").read_text(), "v1") + if __name__ == "__main__": unittest.main() diff --git a/application/tests/harvester_test/git_repository_client_test.py b/application/tests/harvester_test/git_repository_client_test.py index 4929f7810..774715617 100644 --- a/application/tests/harvester_test/git_repository_client_test.py +++ b/application/tests/harvester_test/git_repository_client_test.py @@ -1,6 +1,9 @@ import unittest from unittest.mock import patch +import tempfile +from pathlib import Path + from application.utils.harvester.git_repository_client import ( GitRepositoryClient, ) @@ -30,20 +33,24 @@ def test_local_repository_path(self): ) def test_repository_exists_locally_false(self): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) + with tempfile.TemporaryDirectory() as tmpdir: + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + local_path=Path(tmpdir) / "repo", + ) - self.assertFalse(client.exists_locally()) + self.assertFalse(client.exists_locally()) def test_verify_repository_integrity_false(self): - client = GitRepositoryClient( - owner="OWASP", - repository="ASVS", - ) + with tempfile.TemporaryDirectory() as tmpdir: + client = GitRepositoryClient( + owner="OWASP", + repository="ASVS", + local_path=Path(tmpdir) / "repo", + ) - self.assertFalse(client.verify_repository_integrity()) + self.assertFalse(client.verify_repository_integrity()) def test_sync_clones_when_repository_missing(self): client = GitRepositoryClient( @@ -136,14 +143,13 @@ def test_clone_runs_git_command(self, mock_run): repository="ASVS", ) - with patch.object( - client, - "verify_repository_integrity", - return_value=False, + with ( + patch.object(client, "verify_repository_integrity", return_value=False), + patch.object(client, "is_valid_repository", return_value=True), ): client.clone() - mock_run.assert_called_once() + mock_run.assert_called() if __name__ == "__main__": diff --git a/application/utils/harvester/git_repository_client.py b/application/utils/harvester/git_repository_client.py index 05af12987..1390e6606 100644 --- a/application/utils/harvester/git_repository_client.py +++ b/application/utils/harvester/git_repository_client.py @@ -4,6 +4,10 @@ from .repository_cache import build_repository_cache_path from .repository_client import RepositoryClient import logging +from .repository_lock import repository_lock +import os +import shutil +import tempfile logger = logging.getLogger(__name__) @@ -31,13 +35,6 @@ def repository_url(self) -> str: return f"https://github.com/{self.owner}/{self.repository}.git" def clone(self) -> None: - if self.verify_repository_integrity(): - logger.warning( - "Repository %s/%s already exists locally", - self.owner, - self.repository, - ) - return logger.info( "Cloning repository %s/%s", @@ -50,6 +47,16 @@ def clone(self) -> None: exist_ok=True, ) + self._clone_atomically() + + def _clone_atomically(self) -> None: + temp_path = Path( + tempfile.mkdtemp( + prefix=f"{self.repository}-", + dir=self.local_path.parent, + ) + ) + try: subprocess.run( [ @@ -58,13 +65,23 @@ def clone(self) -> None: "--branch", self.branch, self.repository_url, - str(self.local_path), + str(temp_path), ], check=True, capture_output=True, text=True, timeout=300, ) + + if not self.is_valid_repository(temp_path): + raise RuntimeError("Temporary clone failed integrity verification") + + if self.local_path.exists(): + shutil.rmtree(temp_path) + return + + os.replace(temp_path, self.local_path) + except subprocess.CalledProcessError as exc: logger.error( "Failed to clone repository %s/%s: %s", @@ -74,6 +91,10 @@ def clone(self) -> None: ) raise + finally: + if temp_path.exists(): + shutil.rmtree(temp_path, ignore_errors=True) + def fetch(self) -> None: logger.info( "Fetching repository %s/%s", @@ -169,10 +190,11 @@ def sync(self) -> None: self.repository, ) - if self.verify_repository_integrity(): - self.fetch() - else: - self.clone() + with repository_lock(self.local_path): + if self.verify_repository_integrity(): + self.fetch() + else: + self.clone() def get_current_commit_sha(self) -> str: try: @@ -200,8 +222,8 @@ def get_current_commit_sha(self) -> str: return result.stdout.strip() - def verify_repository_integrity(self) -> bool: - if not (self.local_path.exists() and self.local_path.is_dir()): + def is_valid_repository(self, repository_path: Path) -> bool: + if not (repository_path.exists() and repository_path.is_dir()): return False try: @@ -209,7 +231,7 @@ def verify_repository_integrity(self) -> bool: [ "git", "-C", - str(self.local_path), + str(repository_path), "rev-parse", "--is-inside-work-tree", ], @@ -223,7 +245,7 @@ def verify_repository_integrity(self) -> bool: [ "git", "-C", - str(self.local_path), + str(repository_path), "config", "--get", "remote.origin.url", @@ -241,7 +263,7 @@ def verify_repository_integrity(self) -> bool: [ "git", "-C", - str(self.local_path), + str(repository_path), "show-ref", "--verify", f"refs/remotes/origin/{self.branch}", @@ -256,3 +278,6 @@ def verify_repository_integrity(self) -> bool: except subprocess.CalledProcessError: return False + + def verify_repository_integrity(self) -> bool: + return self.is_valid_repository(self.local_path) diff --git a/application/utils/harvester/repository_lock.py b/application/utils/harvester/repository_lock.py new file mode 100644 index 000000000..9890e6e2e --- /dev/null +++ b/application/utils/harvester/repository_lock.py @@ -0,0 +1,32 @@ +import contextlib +import os +from pathlib import Path + +if os.name == "nt": + import msvcrt +else: + import fcntl + + +@contextlib.contextmanager +def repository_lock(repository_path: Path): + """ + Acquire an exclusive inter-process lock for a repository cache path. + """ + + lock_path = repository_path.with_suffix(".lock") + lock_path.parent.mkdir(parents=True, exist_ok=True) + + with lock_path.open("w") as lock_file: + if os.name == "nt": + msvcrt.locking(lock_file.fileno(), msvcrt.LK_LOCK, 1) + else: + fcntl.flock(lock_file.fileno(), fcntl.LOCK_EX) + + try: + yield + finally: + if os.name == "nt": + msvcrt.locking(lock_file.fileno(), msvcrt.LK_UNLCK, 1) + else: + fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN)