diff --git a/src/ghdcbot/adapters/storage/sqlite.py b/src/ghdcbot/adapters/storage/sqlite.py index 22f4ff6..82ff602 100644 --- a/src/ghdcbot/adapters/storage/sqlite.py +++ b/src/ghdcbot/adapters/storage/sqlite.py @@ -4,10 +4,17 @@ import sqlite3 from datetime import datetime, timezone from pathlib import Path -from typing import Any, Iterable, Sequence +from typing import Any, Iterable, Literal, Sequence -from ghdcbot.config.models import IdentityMapping -from ghdcbot.core.models import ContributionEvent, ContributionSummary, Score +from ghdcbot.core.interfaces import ( + AuditEventDict, + IdentityLinkDict, + IdentityStatusDict, + IssueRequestDict, + NotificationRecordDict, + UnlinkResultDict, +) +from ghdcbot.core.models import ContributionEvent, ContributionSummary, IdentityMapping, Score class SqliteStorage: @@ -435,7 +442,7 @@ def create_identity_claim( ), ) - def get_identity_link(self, discord_user_id: str, github_user: str) -> dict | None: + def get_identity_link(self, discord_user_id: str, github_user: str) -> IdentityLinkDict | None: self.init_schema() gh_norm = github_user.strip().lower() with self._connect() as conn: @@ -468,7 +475,7 @@ def mark_identity_verified(self, discord_user_id: str, github_user: str) -> None def unlink_identity( self, discord_user_id: str, cooldown_hours: int - ) -> dict | None: + ) -> UnlinkResultDict | None: """Unlink the verified identity for this Discord user (set verified=0, unlinked_at=now). Rows are never deleted. Returns unlink info for audit, or None if no verified link. Raises ValueError if inside cooldown window. @@ -539,9 +546,9 @@ def list_verified_identity_mappings(self) -> list[IdentityMapping]: for row in rows ] - def get_identity_links_for_discord_user(self, discord_user_id: str) -> list[dict]: + def get_identity_links_for_discord_user(self, discord_user_id: str) -> list[IdentityLinkDict]: """Return all identity link rows for a Discord user (verified and pending). - Optional method; not part of the Storage protocol. Used for /verify and /status. + Used for /verify and /status. """ with self._connect() as conn: rows = conn.execute( @@ -556,7 +563,7 @@ def get_identity_links_for_discord_user(self, discord_user_id: str) -> list[dict ).fetchall() return [dict(row) for row in rows] - def get_identity_status(self, discord_user_id: str, max_age_days: int | None = None) -> dict: + def get_identity_status(self, discord_user_id: str, max_age_days: int | None = None) -> IdentityStatusDict: """Read-only: return current identity status for a Discord user. Returns dict with github_user, status ('verified'|'verified_stale'|'pending'|'not_linked'), verified_at (UTC ISO or None), is_stale (bool). @@ -626,7 +633,7 @@ def insert_issue_request( (request_id, discord_user_id, github_user, owner, repo, issue_number, issue_url, now), ) - def list_pending_issue_requests(self) -> list[dict]: + def list_pending_issue_requests(self) -> list[IssueRequestDict]: """Return all issue requests with status pending, ordered by created_at ascending.""" with self._connect() as conn: rows = conn.execute( @@ -640,7 +647,7 @@ def list_pending_issue_requests(self) -> list[dict]: ).fetchall() return [dict(row) for row in rows] - def get_issue_request(self, request_id: str) -> dict | None: + def get_issue_request(self, request_id: str) -> IssueRequestDict | None: """Return a single issue request by request_id, or None.""" with self._connect() as conn: row = conn.execute( @@ -649,28 +656,30 @@ def get_issue_request(self, request_id: str) -> dict | None: ).fetchone() return dict(row) if row else None - def update_issue_request_status(self, request_id: str, status: str) -> None: + def update_issue_request_status( + self, + request_id: str, + status: Literal["pending", "approved", "rejected", "cancelled"], + ) -> None: """Update request status to approved, rejected, or cancelled.""" if status not in ("pending", "approved", "rejected", "cancelled"): raise ValueError(f"Invalid status: {status}") with self._connect() as conn: conn.execute("UPDATE issue_requests SET status = ? WHERE request_id = ?", (status, request_id)) - def append_audit_event(self, event: dict) -> None: - """Append a single audit event (append-only) to data_dir/audit_events.jsonl. - Optional method; not part of the Storage protocol. - """ + def append_audit_event(self, event: AuditEventDict) -> None: + """Append a single audit event (append-only) to data_dir/audit_events.jsonl.""" path = self._db_path.parent / "audit_events.jsonl" payload = dict(event) if "timestamp" not in payload: payload["timestamp"] = datetime.now(timezone.utc).isoformat() line = json.dumps(payload, separators=(",", ":")) + "\n" - path.open("a", encoding="utf-8").write(line) + with path.open("a", encoding="utf-8") as f: + f.write(line) - def list_audit_events(self) -> list[dict]: + def list_audit_events(self) -> list[AuditEventDict]: """Read-only: return all audit events from audit_events.jsonl. Returns empty list if file doesn't exist. Does not modify data. - Optional method; not part of the Storage protocol. """ path = self._db_path.parent / "audit_events.jsonl" events = [] @@ -697,7 +706,7 @@ def was_notification_sent(self, dedupe_key: str) -> bool: def mark_notification_sent( self, dedupe_key: str, - event: Any, + event: ContributionEvent, discord_user_id: str, channel_id: str | None, target_github_user: str | None = None, @@ -726,10 +735,9 @@ def mark_notification_sent( ), ) - def list_recent_notifications(self, limit: int = 1000) -> list[dict]: + def list_recent_notifications(self, limit: int = 1000) -> list[NotificationRecordDict]: """List recent notifications (for snapshot export). Returns list of notification dicts, ordered by sent_at DESC. - Optional method; not part of the Storage protocol. """ with self._connect() as conn: rows = conn.execute( diff --git a/src/ghdcbot/core/interfaces.py b/src/ghdcbot/core/interfaces.py index 9d2e1a9..2a3d29b 100644 --- a/src/ghdcbot/core/interfaces.py +++ b/src/ghdcbot/core/interfaces.py @@ -1,17 +1,94 @@ from __future__ import annotations +from collections.abc import Iterable, Sequence from datetime import datetime -from typing import Iterable, Protocol, Sequence +from typing import Literal, Protocol, TypedDict from ghdcbot.core.models import ( AssignmentPlan, ContributionEvent, ContributionSummary, + IdentityMapping, ReviewPlan, Score, ) +class _IdentityLinkRequired(TypedDict): + discord_user_id: str + github_user: str + github_user_normalized: str + verified: int + created_at: str + + +class IdentityLinkDict(_IdentityLinkRequired, total=False): + verification_code: str | None + expires_at: str | None + verified_at: str | None + unlinked_at: str | None + + +class _UnlinkResultRequired(TypedDict): + discord_user_id: str + github_user: str + verified_at: str + unlinked_at: str + + +class UnlinkResultDict(_UnlinkResultRequired, total=False): + cooldown_until: str | None + cooldown_hours: int + + +class IdentityStatusDict(TypedDict): + github_user: str | None + status: Literal["verified", "verified_stale", "pending", "not_linked"] + verified_at: str | None + is_stale: bool + + +class IssueRequestDict(TypedDict): + request_id: str + discord_user_id: str + github_user: str + owner: str + repo: str + issue_number: int + issue_url: str + created_at: str + status: Literal["pending", "approved", "rejected", "cancelled"] + + +class AuditEventContext(TypedDict, total=False): + org: str + repo: str + snapshot_dir: str + run_id: str + files_written: int + timestamp: str + + +class _AuditEventRequired(TypedDict): + event_type: str + + +class AuditEventDict(_AuditEventRequired, total=False): + timestamp: str + context: AuditEventContext + + +class NotificationRecordDict(TypedDict): + dedupe_key: str + event_type: str + github_user: str + discord_user_id: str + repo: str + target: str | None + channel_id: str | None + sent_at: str + + class GitHubReader(Protocol): def list_contributions(self, since: datetime) -> Iterable[ContributionEvent]: """Yield contributions since the given timestamp.""" @@ -59,6 +136,7 @@ def list_contribution_summaries( period_start: datetime, period_end: datetime, weights: dict[str, int], + difficulty_weights: dict[str, int] | None = None, ) -> Sequence[ContributionSummary]: """Aggregate contribution counts and scores for the period.""" @@ -74,6 +152,98 @@ def get_cursor(self, source: str) -> datetime | None: def set_cursor(self, source: str, cursor: datetime) -> None: """Persist last sync cursor for a source.""" + # Identity linking + + def create_identity_claim( + self, + discord_user_id: str, + github_user: str, + verification_code: str, + expires_at: datetime, + *, + max_age_days: int | None = None, + ) -> None: + """Create or refresh a pending identity claim for (discord_user_id, github_user).""" + + def get_identity_link( + self, discord_user_id: str, github_user: str + ) -> IdentityLinkDict | None: + """Return identity link row for (discord_user_id, github_user), or None.""" + + def mark_identity_verified(self, discord_user_id: str, github_user: str) -> None: + """Mark an identity claim as verified.""" + + def unlink_identity( + self, discord_user_id: str, cooldown_hours: int + ) -> UnlinkResultDict | None: + """Unlink the verified identity for a Discord user. Returns unlink info or None.""" + + def list_verified_identity_mappings(self) -> list[IdentityMapping]: + """Return all verified identity mappings.""" + + def get_identity_links_for_discord_user( + self, discord_user_id: str + ) -> list[IdentityLinkDict]: + """Return all identity link rows for a Discord user (verified and pending).""" + + def get_identity_status( + self, discord_user_id: str, max_age_days: int | None = None + ) -> IdentityStatusDict: + """Return current identity status dict for a Discord user.""" + + # Issue requests + + def insert_issue_request( + self, + request_id: str, + discord_user_id: str, + github_user: str, + owner: str, + repo: str, + issue_number: int, + issue_url: str, + ) -> None: + """Store a new issue assignment request with status pending.""" + + def list_pending_issue_requests(self) -> list[IssueRequestDict]: + """Return all pending issue requests ordered by created_at ascending.""" + + def get_issue_request(self, request_id: str) -> IssueRequestDict | None: + """Return a single issue request by request_id, or None.""" + + def update_issue_request_status( + self, + request_id: str, + status: Literal["pending", "approved", "rejected", "cancelled"], + ) -> None: + """Update an issue request status (pending, approved, rejected, cancelled).""" + + # Audit log + + def append_audit_event(self, event: AuditEventDict) -> None: + """Append an audit event (append-only).""" + + def list_audit_events(self) -> list[AuditEventDict]: + """Return all audit events.""" + + # Notifications + + def was_notification_sent(self, dedupe_key: str) -> bool: + """Check if a notification was already sent (deduplication).""" + + def mark_notification_sent( + self, + dedupe_key: str, + event: ContributionEvent, + discord_user_id: str, + channel_id: str | None, + target_github_user: str | None = None, + ) -> None: + """Record that a notification was sent (deduplication tracking).""" + + def list_recent_notifications(self, limit: int = 1000) -> list[NotificationRecordDict]: + """Return recent sent notifications ordered by sent_at descending.""" + class ScoreStrategy(Protocol): def compute_scores( diff --git a/src/ghdcbot/core/models.py b/src/ghdcbot/core/models.py index d0de9a8..87cdcf0 100644 --- a/src/ghdcbot/core/models.py +++ b/src/ghdcbot/core/models.py @@ -5,6 +5,12 @@ from typing import Any +@dataclass(frozen=True) +class IdentityMapping: + github_user: str + discord_user_id: str + + @dataclass(frozen=True) class ContributionEvent: github_user: str diff --git a/src/ghdcbot/engine/snapshots.py b/src/ghdcbot/engine/snapshots.py index f785b8c..b58ae56 100644 --- a/src/ghdcbot/engine/snapshots.py +++ b/src/ghdcbot/engine/snapshots.py @@ -18,11 +18,20 @@ import uuid from datetime import datetime, timezone from pathlib import Path -from typing import Any +from typing import Any, Protocol from ghdcbot.config.models import BotConfig, IdentityMapping +from ghdcbot.core.interfaces import AuditEventDict, IssueRequestDict, NotificationRecordDict from ghdcbot.core.models import ContributionEvent, ContributionSummary, Score + +class SnapshotStorage(Protocol): + """Narrow protocol for the storage methods used by the snapshot writer.""" + + def append_audit_event(self, event: AuditEventDict) -> None: ... + def list_pending_issue_requests(self) -> list[IssueRequestDict]: ... + def list_recent_notifications(self, limit: int = 1000) -> list[NotificationRecordDict]: ... + logger = logging.getLogger("Snapshots") # Snapshot schema version (increment when breaking changes) @@ -30,7 +39,7 @@ def write_snapshots_to_github( - storage: Any, + storage: SnapshotStorage, config: BotConfig, github_writer: Any, identity_mappings: list[IdentityMapping], @@ -83,7 +92,7 @@ def write_snapshots_to_github( def _write_snapshots( - storage: Any, + storage: SnapshotStorage, config: BotConfig, github_writer: Any, snapshot_config: Any, @@ -138,23 +147,21 @@ def _write_snapshots( }, ) # Audit log - append_audit = getattr(storage, "append_audit_event", None) - if callable(append_audit): - append_audit({ - "event_type": "snapshot_written", - "context": { - "org": config.github.org, - "repo": f"{owner}/{repo}", - "snapshot_dir": snapshot_dir, - "run_id": run_id, - "files_written": files_written, - "timestamp": now.isoformat(), - }, - }) + storage.append_audit_event({ + "event_type": "snapshot_written", + "context": { + "org": config.github.org, + "repo": f"{owner}/{repo}", + "snapshot_dir": snapshot_dir, + "run_id": run_id, + "files_written": files_written, + "timestamp": now.isoformat(), + }, + }) def _collect_snapshot_data( - storage: Any, + storage: SnapshotStorage, config: BotConfig, identity_mappings: list[IdentityMapping], scores: list[Score], @@ -252,22 +259,20 @@ def _collect_snapshot_data( } # Issue requests snapshot - issue_requests_data = [] - list_pending = getattr(storage, "list_pending_issue_requests", None) - if callable(list_pending): - pending_requests = list_pending() - for req in pending_requests: - issue_requests_data.append({ - "request_id": req.get("request_id"), - "discord_user_id": req.get("discord_user_id"), - "github_user": req.get("github_user"), - "owner": req.get("owner"), - "repo": req.get("repo"), - "issue_number": req.get("issue_number"), - "issue_url": req.get("issue_url"), - "created_at": req.get("created_at"), - "status": req.get("status"), - }) + issue_requests_data = [ + { + "request_id": req["request_id"], + "discord_user_id": req["discord_user_id"], + "github_user": req["github_user"], + "owner": req["owner"], + "repo": req["repo"], + "issue_number": req["issue_number"], + "issue_url": req["issue_url"], + "created_at": req["created_at"], + "status": req["status"], + } + for req in storage.list_pending_issue_requests() + ] issue_requests = { "schema_version": SCHEMA_VERSION, @@ -278,21 +283,19 @@ def _collect_snapshot_data( } # Notifications snapshot (recent sent notifications) - notifications_data = [] - list_notifications = getattr(storage, "list_recent_notifications", None) - if callable(list_notifications): - recent_notifications = list_notifications(limit=1000) # Last 1000 notifications - for notif in recent_notifications: - notifications_data.append({ - "dedupe_key": notif.get("dedupe_key"), - "event_type": notif.get("event_type"), - "github_user": notif.get("github_user"), - "discord_user_id": notif.get("discord_user_id"), - "repo": notif.get("repo"), - "target": notif.get("target"), - "channel_id": notif.get("channel_id"), - "sent_at": notif.get("sent_at"), - }) + notifications_data = [ + { + "dedupe_key": notif["dedupe_key"], + "event_type": notif["event_type"], + "github_user": notif["github_user"], + "discord_user_id": notif["discord_user_id"], + "repo": notif["repo"], + "target": notif["target"], + "channel_id": notif["channel_id"], + "sent_at": notif["sent_at"], + } + for notif in storage.list_recent_notifications(limit=1000) + ] notifications = { "schema_version": SCHEMA_VERSION, diff --git a/tests/test_snapshots.py b/tests/test_snapshots.py index 03b128a..6fc30ca 100644 --- a/tests/test_snapshots.py +++ b/tests/test_snapshots.py @@ -19,6 +19,7 @@ ) from ghdcbot.core.modes import RunMode from ghdcbot.core.models import ContributionSummary, Score +from ghdcbot.core.interfaces import AuditEventDict, IssueRequestDict, NotificationRecordDict from ghdcbot.engine.snapshots import ( SCHEMA_VERSION, _collect_snapshot_data, @@ -29,16 +30,20 @@ class MockStorage: """Mock storage for testing.""" - + def __init__(self) -> None: - self.notifications = [] - - def list_recent_notifications(self, limit: int = 1000) -> list[dict]: + self.notifications: list[NotificationRecordDict] = [] + self.audit_events: list[AuditEventDict] = [] + + def list_recent_notifications(self, limit: int = 1000) -> list[NotificationRecordDict]: return self.notifications[:limit] - - def list_pending_issue_requests(self) -> list[dict]: + + def list_pending_issue_requests(self) -> list[IssueRequestDict]: return [] + def append_audit_event(self, event: AuditEventDict) -> None: + self.audit_events.append(event) + class MockGitHubWriter: """Mock GitHub writer for testing.""" @@ -298,6 +303,10 @@ def test_write_snapshots_enabled() -> None: assert "schema_version" in parsed assert parsed["org"] == "test-org" + # Audit event should be recorded after a successful snapshot write + assert len(storage.audit_events) == 1 + assert storage.audit_events[0]["event_type"] == "snapshot_written" + def test_write_snapshots_handles_errors() -> None: """Test that snapshot writing errors don't propagate.""" @@ -334,3 +343,6 @@ def test_write_snapshots_handles_errors() -> None: # Should not have written files due to error assert len(github_writer.files_written) == 0 + + # Should not have recorded an audit event on error path + assert len(storage.audit_events) == 0