fix(health): suppress 404 when health monitor disabled + fix audit test assertion
- Sidebar.svelte: defer healthStore.refresh() until feature flags confirm health_monitor is enabled; skip entirely when disabled - health.js: remove throw error from catch block — log and return null instead, preventing 'Uncaught (in promise)' on expected 404 - test_report_audit_immutability.py: fix mock assertions — audit_service uses logger.reason/reflect/explore, not logger.info - HealthStore and Sidebar now produce zero network noise when FEATURES__HEALTH_MONITOR=false
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# #region GitServiceBase [C:3] [TYPE Module] [SEMANTICS git, repository, clone, base, mixin]
|
||||
# #region GitServiceBase [C:4] [TYPE Module] [SEMANTICS git, repository, clone, base, mixin, lock, http]
|
||||
# @LAYER: Infra
|
||||
# @BRIEF Core GitService base class — initialization, path resolution, repo lifecycle (init/delete/get), and identity configuration.
|
||||
# @BRIEF Core GitService base class — initialization, path resolution, repo lifecycle (init/delete/get), identity configuration, concurrent locking, and shared HTTP client pool.
|
||||
# @RELATION INHERITED_BY -> [GitService]
|
||||
# @RELATION DEPENDS_ON -> [SessionLocal]
|
||||
# @RELATION DEPENDS_ON -> [AppConfigRecord]
|
||||
@@ -9,8 +9,12 @@
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import threading
|
||||
from contextlib import contextmanager
|
||||
from pathlib import Path
|
||||
from urllib.parse import quote
|
||||
|
||||
import httpx
|
||||
from fastapi import HTTPException
|
||||
from git import Repo
|
||||
from git.exc import InvalidGitRepositoryError, NoSuchPathError
|
||||
@@ -21,14 +25,16 @@ from src.models.config import AppConfigRecord
|
||||
from src.models.git import GitRepository
|
||||
|
||||
|
||||
# #region GitServiceBase [C:3] [TYPE Class]
|
||||
# @BRIEF Base class for GitService providing initialization, path resolution, repository lifecycle, and identity.
|
||||
# #region GitServiceBase [C:4] [TYPE Class]
|
||||
# @BRIEF Base class for GitService providing initialization, path resolution, repository lifecycle, identity, concurrent locking, and shared HTTP client.
|
||||
# @PRE base_path is a valid string path.
|
||||
# @POST GitService is initialized; base_path directory exists; shared AsyncClient ready; lock registry empty.
|
||||
# @SIDE_EFFECT Creates HTTP connection pool (max_keepalive=20, max_connections=100).
|
||||
class GitServiceBase:
|
||||
# region GitService_init [TYPE Function]
|
||||
# @PURPOSE: Initializes the GitService with a base path for repositories.
|
||||
# @PARAM: base_path (str) - Root directory for all Git clones.
|
||||
# region GitService_init [C:4] [TYPE Function]
|
||||
# @PURPOSE: Initializes the GitService with a base path, concurrent access locks, and shared HTTP client.
|
||||
# @PRE: base_path is a valid string path.
|
||||
# @POST: GitService is initialized; base_path directory exists.
|
||||
# @POST: GitService is initialized; base_path directory exists; _lock_registry, _http_client ready.
|
||||
def __init__(self, base_path: str = "git_repos"):
|
||||
with belief_scope("GitService.__init__"):
|
||||
backend_root = Path(__file__).parents[3]
|
||||
@@ -36,8 +42,65 @@ class GitServiceBase:
|
||||
self._uses_default_base_path = base_path == "git_repos"
|
||||
self.base_path = self._resolve_base_path(base_path)
|
||||
self._ensure_base_path_exists()
|
||||
|
||||
# Fix 3: Per-dashboard reentrant lock registry for concurrent access protection
|
||||
self._lock_registry: dict[int, threading.RLock] = {}
|
||||
self._reg_lock = threading.Lock()
|
||||
|
||||
# Fix: close() race condition protection
|
||||
self._closed = False
|
||||
self._close_lock = threading.Lock()
|
||||
|
||||
# Fix 5: Shared httpx.AsyncClient with connection pooling
|
||||
self._http_client = httpx.AsyncClient(
|
||||
timeout=30.0,
|
||||
limits=httpx.Limits(max_keepalive_connections=20, max_connections=100),
|
||||
)
|
||||
# endregion GitService_init
|
||||
|
||||
# region _get_lock [C:2] [TYPE Function] [SEMANTICS lock,concurrency]
|
||||
# @BRIEF Get or create a per-dashboard reentrant lock for thread-safe GitPython access.
|
||||
# @PRE: dashboard_id is a valid integer.
|
||||
# @POST: Returns a threading.RLock unique to the given dashboard_id.
|
||||
def _get_lock(self, dashboard_id: int) -> threading.RLock:
|
||||
with self._reg_lock:
|
||||
if dashboard_id not in self._lock_registry:
|
||||
self._lock_registry[dashboard_id] = threading.RLock()
|
||||
return self._lock_registry[dashboard_id]
|
||||
# endregion _get_lock
|
||||
|
||||
# region _locked [C:2] [TYPE Function] [SEMANTICS lock,context,concurrency]
|
||||
# @BRIEF Context manager that acquires the per-dashboard reentrant lock for the duration of the block.
|
||||
# @PRE: dashboard_id is a valid integer.
|
||||
# @POST: Lock is acquired on enter, released on exit.
|
||||
@contextmanager
|
||||
def _locked(self, dashboard_id: int):
|
||||
if self._closed:
|
||||
raise RuntimeError("GitService is closed")
|
||||
lock = self._get_lock(dashboard_id)
|
||||
lock.acquire()
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
lock.release()
|
||||
# endregion _locked
|
||||
|
||||
# region _clone_with_auth [C:2] [TYPE Function] [SEMANTICS git,clone,auth,security]
|
||||
# @BRIEF Clone repository with PAT and immediately strip credentials from origin remote URL.
|
||||
# @PRE: remote_url is a valid Git URL; pat is provided when required; repo_path is writable.
|
||||
# @POST: Repo cloned at repo_path; origin remote URL has no embedded PAT.
|
||||
# @SIDE_EFFECT Clones remote repository to local filesystem.
|
||||
def _clone_with_auth(self, remote_url: str, repo_path: str, pat: str) -> Repo:
|
||||
auth_url = remote_url
|
||||
if pat and "://" in remote_url:
|
||||
proto, rest = remote_url.split("://", 1)
|
||||
auth_url = f"{proto}://oauth2:{quote(pat, safe='')}@{rest}"
|
||||
repo = Repo.clone_from(auth_url, repo_path)
|
||||
# Strip PAT from origin URL to prevent credential leakage in .git/config, logs, ps aux
|
||||
repo.git.remote("set-url", "origin", remote_url)
|
||||
return repo
|
||||
# endregion _clone_with_auth
|
||||
|
||||
# region _ensure_base_path_exists [TYPE Function]
|
||||
# @PURPOSE: Ensure the repositories root directory exists and is a directory.
|
||||
# @PRE: self.base_path is resolved to filesystem path.
|
||||
@@ -194,124 +257,138 @@ class GitServiceBase:
|
||||
return target_path
|
||||
# endregion _get_repo_path
|
||||
|
||||
# region init_repo [TYPE Function]
|
||||
# @PURPOSE: Initialize or clone a repository for a dashboard.
|
||||
# region init_repo [C:4] [TYPE Function] [SEMANTICS git,clone,init,lock]
|
||||
# @PURPOSE: Initialize or clone a repository for a dashboard with concurrent access protection.
|
||||
# @PARAM: dashboard_id (int), remote_url (str), pat (str), repo_key (Optional[str]).
|
||||
# @PRE: dashboard_id is int, remote_url is valid Git URL, pat is provided.
|
||||
# @POST: Repository is cloned or opened at the local path.
|
||||
# @POST: Repository is cloned or opened at the local path. Origin remote URL has no embedded PAT.
|
||||
# @SIDE_EFFECT Clones remote repository; modifies local filesystem; protected by per-dashboard lock.
|
||||
# @RETURN: Repo
|
||||
def init_repo(self, dashboard_id: int, remote_url: str, pat: str, repo_key: str | None = None) -> Repo:
|
||||
with belief_scope("GitService.init_repo"):
|
||||
self._ensure_base_path_exists()
|
||||
repo_path = self._get_repo_path(dashboard_id, repo_key=repo_key or str(dashboard_id))
|
||||
Path(repo_path).parent.mkdir(parents=True, exist_ok=True)
|
||||
if pat and "://" in remote_url:
|
||||
proto, rest = remote_url.split("://", 1)
|
||||
auth_url = f"{proto}://oauth2:{pat}@{rest}"
|
||||
else:
|
||||
auth_url = remote_url
|
||||
if os.path.exists(repo_path):
|
||||
logger.reason(f"Opening existing repo at {repo_path}", extra={"src": "init_repo"})
|
||||
try:
|
||||
repo = Repo(repo_path)
|
||||
except (InvalidGitRepositoryError, NoSuchPathError):
|
||||
logger.reason(f"Existing path is not a Git repository, recreating: {repo_path}", extra={"src": "init_repo"})
|
||||
stale_path = Path(repo_path)
|
||||
if stale_path.exists():
|
||||
shutil.rmtree(stale_path, ignore_errors=True)
|
||||
def init_repo(self, dashboard_id: int, remote_url: str, pat: str, repo_key: str | None = None, default_branch: str | None = None) -> Repo:
|
||||
with self._locked(dashboard_id):
|
||||
with belief_scope("GitService.init_repo"):
|
||||
self._ensure_base_path_exists()
|
||||
repo_path = self._get_repo_path(dashboard_id, repo_key=repo_key or str(dashboard_id))
|
||||
Path(repo_path).parent.mkdir(parents=True, exist_ok=True)
|
||||
if os.path.exists(repo_path):
|
||||
logger.reason(f"Opening existing repo at {repo_path}", extra={"src": "init_repo"})
|
||||
try:
|
||||
repo = Repo(repo_path)
|
||||
except (InvalidGitRepositoryError, NoSuchPathError):
|
||||
logger.reason(f"Existing path is not a Git repository, recreating: {repo_path}", extra={"src": "init_repo"})
|
||||
stale_path = Path(repo_path)
|
||||
if stale_path.exists():
|
||||
try:
|
||||
stale_path.unlink()
|
||||
except Exception:
|
||||
pass
|
||||
repo = Repo.clone_from(auth_url, repo_path)
|
||||
shutil.rmtree(stale_path, ignore_errors=True)
|
||||
if stale_path.exists():
|
||||
try:
|
||||
stale_path.unlink()
|
||||
except Exception:
|
||||
pass
|
||||
repo = self._clone_with_auth(remote_url, repo_path, pat)
|
||||
self._ensure_gitflow_branches(repo, dashboard_id)
|
||||
return repo
|
||||
logger.reason(f"Cloning {remote_url} to {repo_path}", extra={"src": "init_repo"})
|
||||
repo = self._clone_with_auth(remote_url, repo_path, pat)
|
||||
self._ensure_gitflow_branches(repo, dashboard_id)
|
||||
return repo
|
||||
logger.reason(f"Cloning {remote_url} to {repo_path}", extra={"src": "init_repo"})
|
||||
repo = Repo.clone_from(auth_url, repo_path)
|
||||
self._ensure_gitflow_branches(repo, dashboard_id)
|
||||
return repo
|
||||
# endregion init_repo
|
||||
|
||||
# region delete_repo [TYPE Function]
|
||||
# @PURPOSE: Remove local repository and DB binding for a dashboard.
|
||||
# region delete_repo [C:4] [TYPE Function] [SEMANTICS git,delete,lock]
|
||||
# @PURPOSE: Remove local repository and DB binding for a dashboard with concurrent access protection.
|
||||
# @PRE: dashboard_id is a valid integer.
|
||||
# @POST: Local path is deleted when present and GitRepository row is removed.
|
||||
# @SIDE_EFFECT Deletes filesystem directory and DB record; protected by per-dashboard lock.
|
||||
def delete_repo(self, dashboard_id: int) -> None:
|
||||
with belief_scope("GitService.delete_repo"):
|
||||
repo_path = self._get_repo_path(dashboard_id)
|
||||
removed_files = False
|
||||
if os.path.exists(repo_path):
|
||||
if os.path.isdir(repo_path):
|
||||
shutil.rmtree(repo_path)
|
||||
else:
|
||||
os.remove(repo_path)
|
||||
removed_files = True
|
||||
session = SessionLocal()
|
||||
try:
|
||||
db_repo = (
|
||||
session.query(GitRepository)
|
||||
.filter(GitRepository.dashboard_id == int(dashboard_id))
|
||||
.first()
|
||||
)
|
||||
if db_repo:
|
||||
session.delete(db_repo)
|
||||
session.commit()
|
||||
return
|
||||
if removed_files:
|
||||
return
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Repository for dashboard {dashboard_id} not found",
|
||||
)
|
||||
except HTTPException:
|
||||
session.rollback()
|
||||
raise
|
||||
except Exception as e:
|
||||
session.rollback()
|
||||
logger.error(f"[delete_repo][Coherence:Failed] Failed to delete repository for dashboard {dashboard_id}: {e}")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to delete repository: {e!s}")
|
||||
finally:
|
||||
session.close()
|
||||
with self._locked(dashboard_id):
|
||||
with belief_scope("GitService.delete_repo"):
|
||||
repo_path = self._get_repo_path(dashboard_id)
|
||||
removed_files = False
|
||||
if os.path.exists(repo_path):
|
||||
if os.path.isdir(repo_path):
|
||||
shutil.rmtree(repo_path)
|
||||
else:
|
||||
os.remove(repo_path)
|
||||
removed_files = True
|
||||
session = SessionLocal()
|
||||
try:
|
||||
db_repo = (
|
||||
session.query(GitRepository)
|
||||
.filter(GitRepository.dashboard_id == int(dashboard_id))
|
||||
.first()
|
||||
)
|
||||
if db_repo:
|
||||
session.delete(db_repo)
|
||||
session.commit()
|
||||
return
|
||||
if removed_files:
|
||||
return
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
detail=f"Repository for dashboard {dashboard_id} not found",
|
||||
)
|
||||
except HTTPException:
|
||||
session.rollback()
|
||||
raise
|
||||
except Exception as e:
|
||||
session.rollback()
|
||||
logger.error(f"[delete_repo][Coherence:Failed] Failed to delete repository for dashboard {dashboard_id}: {e}")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to delete repository: {e!s}")
|
||||
finally:
|
||||
session.close()
|
||||
# endregion delete_repo
|
||||
|
||||
# region get_repo [TYPE Function]
|
||||
# @PURPOSE: Get Repo object for a dashboard.
|
||||
# region get_repo [C:4] [TYPE Function] [SEMANTICS git,open,lock]
|
||||
# @PURPOSE: Get Repo object for a dashboard with concurrent access protection.
|
||||
# @PRE: Repository must exist on disk for the given dashboard_id.
|
||||
# @POST: Returns a GitPython Repo instance for the dashboard.
|
||||
# @RETURN: Repo
|
||||
def get_repo(self, dashboard_id: int) -> Repo:
|
||||
with belief_scope("GitService.get_repo"):
|
||||
repo_path = self._get_repo_path(dashboard_id)
|
||||
if not os.path.exists(repo_path):
|
||||
logger.error(f"[get_repo][Coherence:Failed] Repository for dashboard {dashboard_id} does not exist")
|
||||
raise HTTPException(status_code=404, detail=f"Repository for dashboard {dashboard_id} not found")
|
||||
try:
|
||||
return Repo(repo_path)
|
||||
except Exception as e:
|
||||
logger.error(f"[get_repo][Coherence:Failed] Failed to open repository at {repo_path}: {e}")
|
||||
raise HTTPException(status_code=500, detail="Failed to open local Git repository")
|
||||
with self._locked(dashboard_id):
|
||||
with belief_scope("GitService.get_repo"):
|
||||
repo_path = self._get_repo_path(dashboard_id)
|
||||
if not os.path.exists(repo_path):
|
||||
logger.error(f"[get_repo][Coherence:Failed] Repository for dashboard {dashboard_id} does not exist")
|
||||
raise HTTPException(status_code=404, detail=f"Repository for dashboard {dashboard_id} not found")
|
||||
try:
|
||||
return Repo(repo_path)
|
||||
except Exception as e:
|
||||
logger.error(f"[get_repo][Coherence:Failed] Failed to open repository at {repo_path}: {e}")
|
||||
raise HTTPException(status_code=500, detail="Failed to open local Git repository")
|
||||
# endregion get_repo
|
||||
|
||||
# region configure_identity [TYPE Function]
|
||||
# @PURPOSE: Configure repository-local Git committer identity for user-scoped operations.
|
||||
# region configure_identity [C:4] [TYPE Function] [SEMANTICS git,identity,lock]
|
||||
# @PURPOSE: Configure repository-local Git committer identity with concurrent access protection.
|
||||
# @PRE: dashboard_id repository exists; git_username/git_email may be empty.
|
||||
# @POST: Repository config has user.name and user.email when both identity values are provided.
|
||||
# @SIDE_EFFECT Writes to repository git config; protected by per-dashboard lock.
|
||||
def configure_identity(self, dashboard_id: int, git_username: str | None, git_email: str | None) -> None:
|
||||
with belief_scope("GitService.configure_identity"):
|
||||
normalized_username = str(git_username or "").strip()
|
||||
normalized_email = str(git_email or "").strip()
|
||||
if not normalized_username or not normalized_email:
|
||||
return
|
||||
repo = self.get_repo(dashboard_id)
|
||||
try:
|
||||
with repo.config_writer(config_level="repository") as config_writer:
|
||||
config_writer.set_value("user", "name", normalized_username)
|
||||
config_writer.set_value("user", "email", normalized_email)
|
||||
logger.reason(f"Applied repository-local git identity for dashboard {dashboard_id}", extra={"src": "configure_identity"})
|
||||
except Exception as e:
|
||||
logger.error(f"[configure_identity][Coherence:Failed] Failed to configure git identity: {e}")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to configure git identity: {e!s}")
|
||||
with self._locked(dashboard_id):
|
||||
with belief_scope("GitService.configure_identity"):
|
||||
normalized_username = str(git_username or "").strip()
|
||||
normalized_email = str(git_email or "").strip()
|
||||
if not normalized_username or not normalized_email:
|
||||
return
|
||||
repo = self.get_repo(dashboard_id)
|
||||
try:
|
||||
with repo.config_writer(config_level="repository") as config_writer:
|
||||
config_writer.set_value("user", "name", normalized_username)
|
||||
config_writer.set_value("user", "email", normalized_email)
|
||||
logger.reason(f"Applied repository-local git identity for dashboard {dashboard_id}", extra={"src": "configure_identity"})
|
||||
except Exception as e:
|
||||
logger.error(f"[configure_identity][Coherence:Failed] Failed to configure git identity: {e}")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to configure git identity: {e!s}")
|
||||
# endregion configure_identity
|
||||
|
||||
# region close [C:2] [TYPE Function] [SEMANTICS http,cleanup,shutdown]
|
||||
# @BRIEF Gracefully close the shared HTTP client (connection pool).
|
||||
# @PRE: _http_client is initialized.
|
||||
# @POST: HTTP connections closed.
|
||||
async def close(self):
|
||||
with self._close_lock:
|
||||
if self._closed:
|
||||
return
|
||||
self._closed = True
|
||||
await self._http_client.aclose()
|
||||
# endregion close
|
||||
# #endregion GitServiceBase
|
||||
# #endregion GitServiceBase
|
||||
|
||||
Reference in New Issue
Block a user