diff --git a/backend/src/agent/_confirmation.py b/backend/src/agent/_confirmation.py index af69e0aac..83be12b36 100644 --- a/backend/src/agent/_confirmation.py +++ b/backend/src/agent/_confirmation.py @@ -12,6 +12,7 @@ from typing import Any from langchain_openai import ChatOpenAI +from src.agent._llm_params import chat_openai_kwargs from src.agent._tool_resolver import ( extract_tool_call_from_state, find_tool, @@ -284,13 +285,12 @@ async def _format_tool_output_via_llm( config = await _fetch_llm_config() if config and config.get("configured"): try: - llm = ChatOpenAI( + llm = ChatOpenAI(**chat_openai_kwargs( model=config.get("default_model", "gpt-4o-mini"), base_url=config.get("base_url", "https://api.openai.com/v1"), api_key=config["api_key"], - temperature=0, max_tokens=1024, - ) + )) prompt = ( f"Tool '{tool_name}' returned this data:\n\n{text}\n\n" "Summarize this data in a concise, human-readable format. " diff --git a/backend/src/agent/_llm_params.py b/backend/src/agent/_llm_params.py new file mode 100644 index 000000000..78f902719 --- /dev/null +++ b/backend/src/agent/_llm_params.py @@ -0,0 +1,70 @@ +# backend/src/agent/_llm_params.py +# #region AgentChat.LlmParams [C:3] [TYPE Module] [SEMANTICS agent-chat,llm,openai,compatibility] +# @defgroup AgentChat Shared LLM parameter compatibility helpers. +# @LAYER Service +# @BRIEF Build provider-safe ChatOpenAI kwargs and raw OpenAI payloads. +# @POST Unsupported sampling parameters are omitted for reasoning/codex models. +# @RATIONALE Some OpenAI-compatible gateways reject temperature for reasoning/codex +# models. Centralising the guard prevents resume, health-check, and title +# generation paths from diverging. + +from typing import Any + + +_TEMPERATURE_UNSUPPORTED_PREFIXES = ( + "codex/", + "omni/codex/", + "gpt-5", + "o1", + "o3", + "o4", +) + + +def _canonical_model_name(model: str | None) -> str: + """Return provider-stripped, lowercase model name for compatibility checks.""" + name = (model or "").strip().lower() + if name.startswith(("codex/", "omni/codex/")): + return name + if "/" in name: + return name.rsplit("/", 1)[-1] + return name + + +def supports_temperature(model: str | None) -> bool: + """Return False for model families whose APIs reject temperature.""" + name = _canonical_model_name(model) + return not any(name.startswith(prefix) for prefix in _TEMPERATURE_UNSUPPORTED_PREFIXES) + + +def chat_openai_kwargs( + *, + model: str, + base_url: str | None, + api_key: str, + max_tokens: int, + temperature: float = 0, +) -> dict[str, Any]: + """Build ChatOpenAI kwargs without unsupported temperature for reasoning models.""" + kwargs: dict[str, Any] = { + "model": model, + "base_url": base_url, + "api_key": api_key, + "max_tokens": max_tokens, + } + if supports_temperature(model): + kwargs["temperature"] = temperature + return kwargs + + +def add_temperature_if_supported( + payload: dict[str, Any], + *, + model: str | None, + temperature: float = 0, +) -> dict[str, Any]: + """Mutate and return payload with temperature only when the model supports it.""" + if supports_temperature(model): + payload["temperature"] = temperature + return payload +# #endregion AgentChat.LlmParams diff --git a/backend/src/agent/_persistence.py b/backend/src/agent/_persistence.py index c58ed0377..9cb82f312 100644 --- a/backend/src/agent/_persistence.py +++ b/backend/src/agent/_persistence.py @@ -16,6 +16,7 @@ import uuid import httpx from src.agent._config import AGENT_PREFETCH_DASHBOARD_LIMIT as _PREFETCH_LIMIT, FASTAPI_URL, SERVICE_JWT as _SERVICE_JWT +from src.agent._llm_params import add_temperature_if_supported from src.core.logger import logger SAVE_API_URL = FASTAPI_URL + "/api/agent/conversations/save" @@ -195,8 +196,8 @@ async def _call_llm_for_title(user_text: str) -> str | None: "model": model, "messages": [{"role": "user", "content": prompt}], "max_tokens": 15, - "temperature": 0, } + add_temperature_if_supported(payload, model=model) headers = { "Content-Type": "application/json", diff --git a/backend/src/agent/app.py b/backend/src/agent/app.py index e672c1a83..b70a092b8 100644 --- a/backend/src/agent/app.py +++ b/backend/src/agent/app.py @@ -34,6 +34,7 @@ from langchain_openai import ChatOpenAI from openai import APIConnectionError, APITimeoutError, AuthenticationError from src.agent._config import GRADIO_SERVER_NAME, GRADIO_SERVER_PORT, STORAGE_ROOT as _STORAGE_ROOT +from src.agent._llm_params import chat_openai_kwargs from src.agent._confirmation import ( _pending_confirmations, confirmation_payload, @@ -216,13 +217,12 @@ async def _check_llm_provider_health() -> str: if not config or not config.get("configured"): return _llm_status["status"] - llm = ChatOpenAI( + llm = ChatOpenAI(**chat_openai_kwargs( model=config.get("default_model", "gpt-4o-mini"), base_url=config.get("base_url", "https://api.openai.com/v1"), api_key=config.get("api_key", ""), - temperature=0, max_tokens=1, - ) + )) await llm.ainvoke([HumanMessage(content="ping")]) _llm_status["status"] = "ok" _llm_status["last_error"] = "" @@ -244,6 +244,11 @@ async def _check_llm_provider_health() -> str: _llm_status["last_check_ts"] = time.time() return "auth_error" except Exception as exc: + if "usage_metadata.total_tokens" in str(exc): + _llm_status["status"] = "ok" + _llm_status["last_error"] = "" + _llm_status["last_check_ts"] = time.time() + return "ok" logger.explore("LLM health check failed", error=str(exc), extra={"src": "AgentChat.GradioApp.LlmHealthCheck"}) @@ -535,11 +540,24 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio if kind == "on_chat_model_stream": chunk = event["data"]["chunk"] if hasattr(chunk, "content") and chunk.content: + content = chunk.content + if isinstance(content, str): + token_text = content + elif isinstance(content, list): + token_text = "".join( + str(item.get("text") or item.get("content") or "") + if isinstance(item, dict) else str(item) + for item in content + ) + else: + token_text = str(content) + if not token_text: + continue emitted_any = True - assistant_parts.append(chunk.content) + assistant_parts.append(token_text) yield json.dumps({ - "content": chunk.content, - "metadata": {"type": "stream_token", "token": chunk.content}, + "content": token_text, + "metadata": {"type": "stream_token", "token": token_text}, }) elif kind == "on_tool_start": tool_name = event["name"] @@ -677,14 +695,15 @@ async def agent_handler( # noqa: C901 — intentionally complex C4 orchestratio "content": f"❌ Ошибка: {exc}", "metadata": {"type": "error", "code": "PROCESSING_ERROR", "detail": str(exc)}, }) - await save_conversation(conv_id, visible_user_text, user_id, assistant_text="".join(assistant_parts)) + await save_conversation(conv_id, visible_user_text, user_id, assistant_text="".join(str(part) for part in assistant_parts)) return - await save_conversation(conv_id, visible_user_text, user_id, assistant_text="".join(assistant_parts)) + assistant_text = "".join(str(part) for part in assistant_parts) + await save_conversation(conv_id, visible_user_text, user_id, assistant_text=assistant_text) await _generate_title_best_effort(conv_id, visible_user_text) logger.reflect( "Agent handler completed", - payload={"conv_id": conv_id, "assistant_len": len("".join(assistant_parts))}, + payload={"conv_id": conv_id, "assistant_len": len(assistant_text)}, extra={"src": "AgentChat.GradioApp.Handler"}, ) diff --git a/backend/src/agent/langgraph_setup.py b/backend/src/agent/langgraph_setup.py index 16dd0d2d0..311c98c64 100644 --- a/backend/src/agent/langgraph_setup.py +++ b/backend/src/agent/langgraph_setup.py @@ -21,6 +21,7 @@ from langgraph.prebuilt import create_react_agent from psycopg.rows import dict_row from src.agent._config import FASTAPI_URL, AGENT_CONFIRM_TOOLS, AGENT_INTERRUPT_BEFORE as _INTERRUPT_BEFORE +from src.agent._llm_params import chat_openai_kwargs from src.core.logger import logger # ── Monkey-patch: OpenAI SDK for Pydantic BaseModel classes ── @@ -167,13 +168,12 @@ async def create_agent( extra={"src": "AgentChat.LangGraph.Setup"}, ) - llm = ChatOpenAI( + llm = ChatOpenAI(**chat_openai_kwargs( model=model, base_url=base_url, api_key=api_key, - temperature=0, max_tokens=2048, - ) + )) # System prompt — env_id injected deterministically, not in user message prompt = ( diff --git a/backend/src/api/routes/__tests__/test_git_status_route.py b/backend/src/api/routes/__tests__/test_git_status_route.py index 707191fd7..d8c44c941 100644 --- a/backend/src/api/routes/__tests__/test_git_status_route.py +++ b/backend/src/api/routes/__tests__/test_git_status_route.py @@ -9,6 +9,12 @@ from unittest.mock import MagicMock from fastapi import HTTPException from src.api.routes import git as git_routes +from src.api.routes.git import _helpers as git_helpers +from src.services.git._sync import GitServiceSyncMixin + + +async def _resolved_dashboard_id(*_args, **_kwargs): + return 12 # #region test_get_repository_status_returns_no_repo_payload_for_missing_repo [TYPE Function] @@ -18,9 +24,9 @@ from src.api.routes import git as git_routes # @POST Route returns a deterministic NO_REPO status payload. def test_get_repository_status_returns_no_repo_payload_for_missing_repo(monkeypatch): class MissingRepoGitService: - def _get_repo_path(self, dashboard_id: int) -> str: + async def _get_repo_path(self, dashboard_id: int) -> str: return f"/tmp/missing-repo-{dashboard_id}" - def get_status(self, dashboard_id: int) -> dict: + async def get_status(self, dashboard_id: int) -> dict: raise AssertionError("get_status must not be called when repository path is missing") monkeypatch.setattr(git_routes, "git_service", MissingRepoGitService()) response = asyncio.run(git_routes.get_repository_status(34)) @@ -36,9 +42,9 @@ def test_get_repository_status_returns_no_repo_payload_for_missing_repo(monkeypa # @POST Raised exception preserves original status and detail. def test_get_repository_status_propagates_non_404_http_exception(monkeypatch): class ConflictGitService: - def _get_repo_path(self, dashboard_id: int) -> str: + async def _get_repo_path(self, dashboard_id: int) -> str: return f"/tmp/existing-repo-{dashboard_id}" - def get_status(self, dashboard_id: int) -> dict: + async def get_status(self, dashboard_id: int) -> dict: raise HTTPException(status_code=409, detail="Conflict") monkeypatch.setattr(git_routes, "git_service", ConflictGitService()) monkeypatch.setattr(git_routes.os.path, "exists", lambda _path: True) @@ -54,7 +60,7 @@ def test_get_repository_status_propagates_non_404_http_exception(monkeypatch): # @POST Endpoint raises same HTTPException values. def test_get_repository_diff_propagates_http_exception(monkeypatch): class DiffGitService: - def get_diff(self, dashboard_id: int, file_path=None, staged: bool = False) -> str: + async def get_diff(self, dashboard_id: int, file_path=None, staged: bool = False) -> str: raise HTTPException(status_code=404, detail="Repository missing") monkeypatch.setattr(git_routes, "git_service", DiffGitService()) with pytest.raises(HTTPException) as exc_info: @@ -69,7 +75,7 @@ def test_get_repository_diff_propagates_http_exception(monkeypatch): # @POST Endpoint returns HTTPException with status 500 and route context. def test_get_history_wraps_unexpected_error_as_500(monkeypatch): class HistoryGitService: - def get_commit_history(self, dashboard_id: int, limit: int = 50): + async def get_commit_history(self, dashboard_id: int, limit: int = 50): raise ValueError("broken parser") monkeypatch.setattr(git_routes, "git_service", HistoryGitService()) with pytest.raises(HTTPException) as exc_info: @@ -84,7 +90,7 @@ def test_get_history_wraps_unexpected_error_as_500(monkeypatch): # @POST Endpoint raises HTTPException(500) with route context. def test_commit_changes_wraps_unexpected_error_as_500(monkeypatch): class CommitGitService: - def commit_changes(self, dashboard_id: int, message: str, files): + async def commit_changes(self, dashboard_id: int, message: str, files): raise RuntimeError("index lock") class CommitPayload: message = "test" @@ -102,9 +108,9 @@ def test_commit_changes_wraps_unexpected_error_as_500(monkeypatch): # @POST Returned map includes resolved status for each requested dashboard ID. def test_get_repository_status_batch_returns_mixed_statuses(monkeypatch): class BatchGitService: - def _get_repo_path(self, dashboard_id: int) -> str: + async def _get_repo_path(self, dashboard_id: int) -> str: return f"/tmp/repo-{dashboard_id}" - def get_status(self, dashboard_id: int) -> dict: + async def get_status(self, dashboard_id: int) -> dict: if dashboard_id == 2: return {"sync_state": "SYNCED", "sync_status": "OK"} raise HTTPException(status_code=404, detail="not found") @@ -123,9 +129,9 @@ def test_get_repository_status_batch_returns_mixed_statuses(monkeypatch): # @POST Failed dashboard status is marked as ERROR. def test_get_repository_status_batch_marks_item_as_error_on_service_failure(monkeypatch): class BatchErrorGitService: - def _get_repo_path(self, dashboard_id: int) -> str: + async def _get_repo_path(self, dashboard_id: int) -> str: return f"/tmp/repo-{dashboard_id}" - def get_status(self, dashboard_id: int) -> dict: + async def get_status(self, dashboard_id: int) -> dict: raise RuntimeError("boom") monkeypatch.setattr(git_routes, "git_service", BatchErrorGitService()) monkeypatch.setattr(git_routes.os.path, "exists", lambda _path: True) @@ -142,9 +148,9 @@ def test_get_repository_status_batch_marks_item_as_error_on_service_failure(monk # @POST Result contains unique IDs up to configured cap. def test_get_repository_status_batch_deduplicates_and_truncates_ids(monkeypatch): class SafeBatchGitService: - def _get_repo_path(self, dashboard_id: int) -> str: + async def _get_repo_path(self, dashboard_id: int) -> str: return f"/tmp/repo-{dashboard_id}" - def get_status(self, dashboard_id: int) -> dict: + async def get_status(self, dashboard_id: int) -> dict: return {"sync_state": "SYNCED", "sync_status": "OK"} monkeypatch.setattr(git_routes, "git_service", SafeBatchGitService()) monkeypatch.setattr(git_routes.os.path, "exists", lambda _path: True) @@ -164,9 +170,9 @@ def test_commit_changes_applies_profile_identity_before_commit(monkeypatch): def __init__(self): self.configured_identity = None self.commit_payload = None - def configure_identity(self, dashboard_id: int, git_username: str, git_email: str): + async def configure_identity(self, dashboard_id: int, git_username: str, git_email: str): self.configured_identity = (dashboard_id, git_username, git_email) - def commit_changes(self, dashboard_id: int, message: str, files): + async def commit_changes(self, dashboard_id: int, message: str, files): self.commit_payload = (dashboard_id, message, files) class PreferenceRow: git_username = "user_1" @@ -189,7 +195,7 @@ def test_commit_changes_applies_profile_identity_before_commit(monkeypatch): monkeypatch.setattr( git_routes, "_resolve_dashboard_id_from_ref", - lambda *_args, **_kwargs: 12, + _resolved_dashboard_id, ) asyncio.run( git_routes.commit_changes( @@ -213,10 +219,10 @@ def test_pull_changes_applies_profile_identity_before_pull(monkeypatch): def __init__(self): self.configured_identity = None self.pulled_dashboard_id = None - def configure_identity(self, dashboard_id: int, git_username: str, git_email: str): + async def configure_identity(self, dashboard_id: int, git_username: str, git_email: str): self.configured_identity = (dashboard_id, git_username, git_email) - def pull_changes(self, dashboard_id: int): - self.pulled_dashboard_id = dashboard_id + async def pull_changes(self, dashboard_id: int, pat: str | None = None): + self.pulled_payload = (dashboard_id, pat) class PreferenceRow: git_username = "user_1" git_email = "user1@mail.ru" @@ -235,7 +241,7 @@ def test_pull_changes_applies_profile_identity_before_pull(monkeypatch): monkeypatch.setattr( git_routes, "_resolve_dashboard_id_from_ref", - lambda *_args, **_kwargs: 12, + _resolved_dashboard_id, ) asyncio.run( git_routes.pull_changes( @@ -246,8 +252,127 @@ def test_pull_changes_applies_profile_identity_before_pull(monkeypatch): ) ) assert identity_service.configured_identity == (12, "user_1", "user1@mail.ru") - assert identity_service.pulled_dashboard_id == 12 + assert identity_service.pulled_payload == (12, None) # #endregion test_pull_changes_applies_profile_identity_before_pull +# #region test_push_changes_passes_decrypted_profile_pat [TYPE Function] +# @RELATION BINDS_TO -> TestGitStatusRoute +# @BRIEF Ensure push route decrypts current user's profile PAT and passes it to GitService. +# @PRE Profile preference contains encrypted git_personal_access_token. +# @POST git_service.push_changes receives the decrypted token via pat keyword. +def test_push_changes_passes_decrypted_profile_pat(monkeypatch): + class PushGitService: + def __init__(self): + self.pushed_payload = None + async def push_changes(self, dashboard_id: int, pat: str | None = None): + self.pushed_payload = (dashboard_id, pat) + class PreferenceRow: + git_personal_access_token_encrypted = "encrypted-token" + class PreferenceQuery: + def filter(self, *_args, **_kwargs): + return self + def first(self): + return PreferenceRow() + class DbStub: + def query(self, _model): + return PreferenceQuery() + class UserStub: + id = "u-1" + class EncryptionStub: + def decrypt(self, encrypted_data: str) -> str: + assert encrypted_data == "encrypted-token" + return "profile-pat" + push_service = PushGitService() + monkeypatch.setattr(git_routes, "git_service", push_service) + monkeypatch.setattr(git_helpers, "EncryptionManager", lambda: EncryptionStub()) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) + asyncio.run( + git_routes.push_changes( + "dashboard-12", + config_manager=MagicMock(), + db=DbStub(), + current_user=UserStub(), + ) + ) + assert push_service.pushed_payload == (12, "profile-pat") +# #endregion test_push_changes_passes_decrypted_profile_pat +# #region test_pull_changes_passes_decrypted_profile_pat [TYPE Function] +# @RELATION BINDS_TO -> TestGitStatusRoute +# @BRIEF Ensure pull route decrypts current user's profile PAT and passes it to GitService. +# @PRE Profile preference contains git identity and encrypted PAT. +# @POST git_service.pull_changes receives decrypted token after identity configuration. +def test_pull_changes_passes_decrypted_profile_pat(monkeypatch): + class PullGitService: + def __init__(self): + self.configured_identity = None + self.pulled_payload = None + async def configure_identity(self, dashboard_id: int, git_username: str, git_email: str): + self.configured_identity = (dashboard_id, git_username, git_email) + async def pull_changes(self, dashboard_id: int, pat: str | None = None): + self.pulled_payload = (dashboard_id, pat) + class PreferenceRow: + git_username = "user_1" + git_email = "user1@mail.ru" + git_personal_access_token_encrypted = "encrypted-token" + class PreferenceQuery: + def filter(self, *_args, **_kwargs): + return self + def first(self): + return PreferenceRow() + class DbStub: + def query(self, _model): + return PreferenceQuery() + class UserStub: + id = "u-1" + class EncryptionStub: + def decrypt(self, encrypted_data: str) -> str: + assert encrypted_data == "encrypted-token" + return "profile-pat" + pull_service = PullGitService() + monkeypatch.setattr(git_routes, "git_service", pull_service) + monkeypatch.setattr(git_helpers, "EncryptionManager", lambda: EncryptionStub()) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) + asyncio.run( + git_routes.pull_changes( + "dashboard-12", + config_manager=MagicMock(), + db=DbStub(), + current_user=UserStub(), + ) + ) + assert pull_service.configured_identity == (12, "user_1", "user1@mail.ru") + assert pull_service.pulled_payload == (12, "profile-pat") +# #endregion test_pull_changes_passes_decrypted_profile_pat +# #region test_git_sync_mixin_embeds_and_redacts_pat [TYPE Function] +# @RELATION BINDS_TO -> TestGitStatusRoute +# @BRIEF Ensure PAT embedding URL-encodes secrets and redaction removes them from messages. +# @PRE Origin remote uses an HTTPS URL and PAT contains URL-sensitive characters. +# @POST Origin URL receives encoded PAT; redaction hides raw and URL-shaped tokens. +def test_git_sync_mixin_embeds_and_redacts_pat(): + class OriginStub: + def __init__(self): + self.urls = ["https://github.com/org/repo.git"] + self.set_urls = [] + def set_url(self, url: str): + self.set_urls.append(url) + self.urls = [url] + class RepoStub: + def __init__(self): + self.origin = OriginStub() + def remote(self, name: str): + assert name == "origin" + return self.origin + repo = RepoStub() + original_url = GitServiceSyncMixin._embed_pat_in_origin_url(repo, "tok@:/secret") + assert original_url == "https://github.com/org/repo.git" + assert repo.origin.set_urls == ["https://git-user:tok%40%3A%2Fsecret@github.com/org/repo.git"] + redacted = GitServiceSyncMixin._redact_pat_from_message( + "fatal https://git-user:tok%40%3A%2Fsecret@github.com/org/repo.git tok@:/secret", + "tok@:/secret", + ) + assert "tok@:/secret" not in redacted + assert "tok%40%3A%2Fsecret" not in redacted + assert "***" in redacted +# #endregion test_git_sync_mixin_embeds_and_redacts_pat # #region test_get_merge_status_returns_service_payload [TYPE Function] # @RELATION BINDS_TO -> TestGitStatusRoute # @BRIEF Ensure merge status route returns service payload as-is. @@ -255,7 +380,7 @@ def test_pull_changes_applies_profile_identity_before_pull(monkeypatch): # @POST Route response contains has_unfinished_merge=True. def test_get_merge_status_returns_service_payload(monkeypatch): class MergeStatusGitService: - def get_merge_status(self, dashboard_id: int) -> dict: + async def get_merge_status(self, dashboard_id: int) -> dict: return { "has_unfinished_merge": True, "repository_path": "/tmp/repo-12", @@ -266,7 +391,7 @@ def test_get_merge_status_returns_service_payload(monkeypatch): "conflicts_count": 2, } monkeypatch.setattr(git_routes, "git_service", MergeStatusGitService()) - monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", lambda *_args, **_kwargs: 12) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) response = asyncio.run( git_routes.get_merge_status( "dashboard-12", @@ -284,7 +409,7 @@ def test_get_merge_status_returns_service_payload(monkeypatch): def test_resolve_merge_conflicts_passes_resolution_items_to_service(monkeypatch): captured = {} class MergeResolveGitService: - def resolve_merge_conflicts(self, dashboard_id: int, resolutions): + async def resolve_merge_conflicts(self, dashboard_id: int, resolutions): captured["dashboard_id"] = dashboard_id captured["resolutions"] = resolutions return ["dashboards/a.yaml"] @@ -294,7 +419,7 @@ def test_resolve_merge_conflicts_passes_resolution_items_to_service(monkeypatch) return {"file_path": "dashboards/a.yaml", "resolution": "mine", "content": None} resolutions = [_Resolution()] monkeypatch.setattr(git_routes, "git_service", MergeResolveGitService()) - monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", lambda *_args, **_kwargs: 12) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) response = asyncio.run( git_routes.resolve_merge_conflicts( "dashboard-12", @@ -313,11 +438,11 @@ def test_resolve_merge_conflicts_passes_resolution_items_to_service(monkeypatch) # @POST Route returns aborted status. def test_abort_merge_calls_service_and_returns_result(monkeypatch): class AbortGitService: - def abort_merge(self, dashboard_id: int): + async def abort_merge(self, dashboard_id: int): assert dashboard_id == 12 return {"status": "aborted"} monkeypatch.setattr(git_routes, "git_service", AbortGitService()) - monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", lambda *_args, **_kwargs: 12) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) response = asyncio.run( git_routes.abort_merge( "dashboard-12", @@ -333,14 +458,14 @@ def test_abort_merge_calls_service_and_returns_result(monkeypatch): # @POST Route returns committed status and hash. def test_continue_merge_passes_message_and_returns_commit(monkeypatch): class ContinueGitService: - def continue_merge(self, dashboard_id: int, message: str): + async def continue_merge(self, dashboard_id: int, message: str): assert dashboard_id == 12 assert message == "Resolve all conflicts" return {"status": "committed", "commit_hash": "abc123"} class ContinueData: message = "Resolve all conflicts" monkeypatch.setattr(git_routes, "git_service", ContinueGitService()) - monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", lambda *_args, **_kwargs: 12) + monkeypatch.setattr(git_routes, "_resolve_dashboard_id_from_ref", _resolved_dashboard_id) response = asyncio.run( git_routes.continue_merge( "dashboard-12", diff --git a/backend/src/api/routes/agent_superset_explore.py b/backend/src/api/routes/agent_superset_explore.py index de59c4931..8cb3e181d 100644 --- a/backend/src/api/routes/agent_superset_explore.py +++ b/backend/src/api/routes/agent_superset_explore.py @@ -17,11 +17,13 @@ router = APIRouter(prefix="/api/agent/superset", tags=["Agent Superset"]) async def _get_superset_client(environment_id: str) -> SupersetClient: """Resolve a SupersetClient for the given environment_id.""" - from ..api.routes.environments import _get_environment_by_id as _resolve_env - env_data = await _resolve_env(environment_id) - if not env_data: + from src.dependencies import get_config_manager + + env = get_config_manager().get_environment(environment_id) + if env is None: raise HTTPException(status_code=404, detail=f"Environment '{environment_id}' not found.") - env = Environment(**env_data) + if not isinstance(env, Environment): + env = Environment.model_validate(env) return SupersetClient(env) diff --git a/backend/src/api/routes/environments.py b/backend/src/api/routes/environments.py index 069eb9758..b0b4f7101 100644 --- a/backend/src/api/routes/environments.py +++ b/backend/src/api/routes/environments.py @@ -56,6 +56,25 @@ class DatabaseResponse(BaseModel): engine: str | None # #endregion DatabaseResponse + +# #region _get_environment_by_id [TYPE Function] +# @BRIEF Legacy resolver for agent Superset routes that still import environment by id. +# @PRE Environment id can be an id or configured environment name. +# @POST Returns full Environment-compatible dict or None when missing. +# @RATIONALE Keeps hot-reloaded/stale agent route code from failing while the direct +# ConfigManager resolver path is deployed in agent_superset_explore.py. +async def _get_environment_by_id(environment_id: str) -> dict | None: + env = get_config_manager().get_environment(environment_id) + if env is None: + return None + if hasattr(env, "model_dump"): + return env.model_dump() + if hasattr(env, "dict"): + return env.dict() + return dict(env) +# #endregion _get_environment_by_id + + # #region get_environments [TYPE Function] [SEMANTICS list, environments, config] # @ingroup Api # @BRIEF List all configured environments. diff --git a/backend/src/api/routes/git/_helpers.py b/backend/src/api/routes/git/_helpers.py index ed153c521..4ba6d0768 100644 --- a/backend/src/api/routes/git/_helpers.py +++ b/backend/src/api/routes/git/_helpers.py @@ -11,6 +11,7 @@ import os from fastapi import HTTPException from sqlalchemy.orm import Session +from src.core.encryption import EncryptionManager from src.core.logger import logger from src.core.superset_client import SupersetClient from src.models.auth import User @@ -313,6 +314,51 @@ def _resolve_current_user_git_identity( # #endregion _resolve_current_user_git_identity +# #region _resolve_current_user_git_token [C:2] [TYPE Function] +# @BRIEF Resolve and decrypt the Git personal access token from current user's profile preferences. +def _resolve_current_user_git_token( + db: Session, + current_user: User | None, +) -> str | None: + if db is None or not hasattr(db, "query"): + return None + + user_id = _sanitize_optional_identity_value(getattr(current_user, "id", None)) + if not user_id: + return None + + try: + preference = ( + db.query(UserDashboardPreference) + .filter(UserDashboardPreference.user_id == user_id) + .first() + ) + except Exception as resolve_error: + logger.explore( + "Failed to load profile preference for resolving git PAT", + extra={"src": "_resolve_current_user_git_token", "payload": {"user_id": user_id}, "error": str(resolve_error)}, + ) + return None + + if not preference: + return None + + encrypted_token = getattr(preference, "git_personal_access_token_encrypted", None) + if not encrypted_token: + return None + + try: + encryption = EncryptionManager() + return encryption.decrypt(encrypted_token) + except Exception as decrypt_error: + logger.explore( + "Failed to decrypt git PAT from profile", + extra={"src": "_resolve_current_user_git_token", "user_id": user_id, "error": str(decrypt_error)}, + ) + return None +# #endregion _resolve_current_user_git_token + + # #region _apply_git_identity_from_profile [C:2] [TYPE Function] # @BRIEF Apply user-scoped Git identity to repository-local config before write/pull operations. async def _apply_git_identity_from_profile( diff --git a/backend/src/api/routes/git/_repo_operations_routes.py b/backend/src/api/routes/git/_repo_operations_routes.py index 2000f9b49..13064f770 100644 --- a/backend/src/api/routes/git/_repo_operations_routes.py +++ b/backend/src/api/routes/git/_repo_operations_routes.py @@ -25,6 +25,7 @@ from ._helpers import ( _apply_git_identity_from_profile, _build_no_repo_status_payload, _handle_unexpected_git_route_error, + _resolve_current_user_git_token, _resolve_repository_status, ) from ._router import router @@ -105,6 +106,8 @@ async def push_changes( dashboard_ref: str, env_id: str | None = None, config_manager=Depends(get_config_manager), + db: Session = Depends(get_db), + current_user: User = Depends(get_current_user), _=Depends(has_permission("plugin:git", "EXECUTE")), ): _gs = get_git_service() @@ -115,7 +118,8 @@ async def push_changes( dashboard_id = await _resolve_dashboard_id_from_ref( dashboard_ref, config_manager, env_id ) - await _gs.push_changes(dashboard_id) + pat = _resolve_current_user_git_token(db, current_user) + await _gs.push_changes(dashboard_id, pat=pat) return {"status": "success"} except HTTPException: raise @@ -178,7 +182,8 @@ async def pull_changes( extra={"src": "pull_changes"}, ) await _apply_git_identity_from_profile(dashboard_id, db, current_user) - await _gs.pull_changes(dashboard_id) + pat = _resolve_current_user_git_token(db, current_user) + await _gs.pull_changes(dashboard_id, pat=pat) return {"status": "success"} except HTTPException: raise diff --git a/backend/src/services/git/_sync.py b/backend/src/services/git/_sync.py index 8012c801b..7baf9d38c 100644 --- a/backend/src/services/git/_sync.py +++ b/backend/src/services/git/_sync.py @@ -6,6 +6,7 @@ # @RELATION DEPENDS_ON -> [GitServiceUrlMixin] import os +import re from fastapi import HTTPException from git.exc import GitCommandError @@ -19,11 +20,49 @@ from src.models.git import GitRepository, GitServerConfig # @defgroup Services Module group. # @BRIEF Mixin providing push and pull operations with origin host alignment. class GitServiceSyncMixin: + @staticmethod + def _redact_pat_from_message(message: str, pat: str | None = None) -> str: + redacted = str(message or "") + if pat: + redacted = redacted.replace(pat, "***") + return re.sub(r"(https?://[^\s:/@]+:)[^\s@]+(@)", r"\1***\2", redacted) + # region push_changes [C:4] [TYPE Function] [SEMANTICS git,push,lock] # @PURPOSE: Push local commits to remote (concurrent-safe). # @PRE Repository exists and has an 'origin' remote. # @POST Local branch commits are pushed to origin. - async def push_changes(self, dashboard_id: int): + # region _embed_pat_in_origin_url [C:2] [TYPE Function] [SEMANTICS git,auth,pat,url] + # @BRIEF Temporarily embed a personal access token into the origin remote URL. + # @POST Origin URL is updated with embedded PAT; returns the original URL for restoration. + @staticmethod + def _embed_pat_in_origin_url(repo, pat: str) -> str | None: + from urllib.parse import urlparse, urlunparse, quote + + try: + origin = repo.remote(name="origin") + original_url = list(origin.urls)[0] + except Exception: + return None + parsed = urlparse(original_url) + if parsed.scheme not in ("https", "http"): + return None + username = parsed.username or "git-user" + encoded_pat = quote(pat, safe="") + auth_part = f"{quote(username, safe='')}:{encoded_pat}@" + host_part = parsed.hostname or "" + if parsed.port: + host_part = f"{host_part}:{parsed.port}" + new_url = urlunparse( + (parsed.scheme, f"{auth_part}{host_part}", parsed.path, parsed.params, parsed.query, parsed.fragment) + ) + try: + origin.set_url(new_url) + return original_url + except Exception: + return None + # endregion _embed_pat_in_origin_url + + async def push_changes(self, dashboard_id: int, pat: str | None = None): with self._locked(dashboard_id): with belief_scope("GitService.push_changes"): repo = await self.get_repo(dashboard_id) @@ -82,42 +121,56 @@ class GitServiceSyncMixin: f"Push diagnostics dashboard={dashboard_id} config_id={binding_config_id} config_url={binding_config_url} binding_remote_url={binding_remote_url} origin_urls={origin_urls} origin_realigned={bool(realigned_origin_url)}", extra={"src": "push_changes"}, ) + # ── Embed PAT credential if provided ────────────────────────────── + _original_push_url: str | None = None + if pat: + _original_push_url = self._embed_pat_in_origin_url(repo, pat) + # ────────────────────────────────────────────────────────────────── try: - current_branch = repo.active_branch - logger.reason(f"Pushing branch {current_branch.name} to origin", extra={"src": "push_changes"}) - tracking_branch = None try: - tracking_branch = current_branch.tracking_branch() - except Exception: + current_branch = repo.active_branch + logger.reason(f"Pushing branch {current_branch.name} to origin", extra={"src": "push_changes"}) tracking_branch = None - if tracking_branch is None: - repo.git.push("--set-upstream", "origin", f"{current_branch.name}:{current_branch.name}") - else: - push_info = origin.push(refspec=f'{current_branch.name}:{current_branch.name}') - for info in push_info: - if info.flags & info.ERROR: - logger.explore("Error pushing ref", extra={"src": "push_changes"}, payload={"ref": info.remote_ref_string}, error=str(info.summary)) - raise Exception(f"Git push error for {info.remote_ref_string}: {info.summary}") - except GitCommandError as e: - details = str(e) - lowered = details.lower() - if "non-fast-forward" in lowered or "rejected" in lowered: - raise HTTPException( - status_code=409, - detail="Push rejected: remote branch contains newer commits. Run Pull first, resolve conflicts if any, then push again.", - ) - logger.explore("Failed to push changes", extra={"src": "push_changes"}, error=str(e)) - raise HTTPException(status_code=500, detail=f"Git push failed: {details}") - except Exception as e: - logger.explore("Failed to push changes", extra={"src": "push_changes"}, error=str(e)) - raise HTTPException(status_code=500, detail=f"Git push failed: {e!s}") + try: + tracking_branch = current_branch.tracking_branch() + except Exception: + tracking_branch = None + if tracking_branch is None: + repo.git.push("--set-upstream", "origin", f"{current_branch.name}:{current_branch.name}") + else: + push_info = origin.push(refspec=f'{current_branch.name}:{current_branch.name}') + for info in push_info: + if info.flags & info.ERROR: + safe_summary = self._redact_pat_from_message(str(info.summary), pat) + logger.explore("Error pushing ref", extra={"src": "push_changes"}, payload={"ref": info.remote_ref_string}, error=safe_summary) + raise Exception(f"Git push error for {info.remote_ref_string}: {safe_summary}") + except GitCommandError as e: + details = self._redact_pat_from_message(str(e), pat) + lowered = details.lower() + if "non-fast-forward" in lowered or "rejected" in lowered: + raise HTTPException( + status_code=409, + detail="Push rejected: remote branch contains newer commits. Run Pull first, resolve conflicts if any, then push again.", + ) + logger.explore("Failed to push changes", extra={"src": "push_changes"}, error=details) + raise HTTPException(status_code=500, detail=f"Git push failed: {details}") + except Exception as e: + details = self._redact_pat_from_message(str(e), pat) + logger.explore("Failed to push changes", extra={"src": "push_changes"}, error=details) + raise HTTPException(status_code=500, detail=f"Git push failed: {details}") + finally: + if _original_push_url is not None: + try: + origin.set_url(_original_push_url) + except Exception: + pass # endregion push_changes # region pull_changes [C:4] [TYPE Function] [SEMANTICS git,pull,lock] # @PURPOSE: Pull changes from remote (concurrent-safe). # @PRE Repository exists and has an 'origin' remote. # @POST Changes from origin are pulled and merged into the active branch. - async def pull_changes(self, dashboard_id: int): + async def pull_changes(self, dashboard_id: int, pat: str | None = None): with self._locked(dashboard_id): with belief_scope("GitService.pull_changes"): repo = await self.get_repo(dashboard_id) @@ -129,49 +182,63 @@ class GitServiceSyncMixin: extra={"src": "pull_changes"}, ) raise HTTPException(status_code=409, detail=payload) + # ── Embed PAT credential if provided ────────────────────────────── + _original_pull_url: str | None = None + if pat: + _original_pull_url = self._embed_pat_in_origin_url(repo, pat) + # ────────────────────────────────────────────────────────────────── + origin = None try: - origin = repo.remote(name='origin') - current_branch = repo.active_branch.name try: - origin_urls = list(origin.urls) - except Exception: - origin_urls = [] - logger.reason( - f"Pull diagnostics dashboard={dashboard_id} repo_path={repo.working_tree_dir} branch={current_branch} origin_urls={origin_urls}", - extra={"src": "pull_changes"}, - ) - origin.fetch(prune=True) - remote_ref = f"origin/{current_branch}" - has_remote_branch = any(ref.name == remote_ref for ref in repo.refs) - logger.reason( - f"Pull remote branch check dashboard={dashboard_id} branch={current_branch} remote_ref={remote_ref} exists={has_remote_branch}", - extra={"src": "pull_changes"}, - ) - if not has_remote_branch: - raise HTTPException( - status_code=409, - detail=f"Remote branch '{current_branch}' does not exist yet. Push this branch first.", + origin = repo.remote(name='origin') + current_branch = repo.active_branch.name + try: + origin_urls = [self._redact_pat_from_message(url, pat) for url in origin.urls] + except Exception: + origin_urls = [] + logger.reason( + f"Pull diagnostics dashboard={dashboard_id} repo_path={repo.working_tree_dir} branch={current_branch} origin_urls={origin_urls}", + extra={"src": "pull_changes"}, ) - logger.reason(f"Pulling changes from origin/{current_branch}", extra={"src": "pull_changes"}) - repo.git.pull("--no-rebase", "origin", current_branch) - except ValueError: - logger.explore("Remote 'origin' not found", extra={"src": "pull_changes"}, payload={"dashboard_id": dashboard_id}) - raise HTTPException(status_code=400, detail="Remote 'origin' not configured") - except GitCommandError as e: - details = str(e) - lowered = details.lower() - if "conflict" in lowered or "not possible to fast-forward" in lowered: - raise HTTPException( - status_code=409, - detail="Pull requires conflict resolution. Resolve conflicts in repository and repeat operation.", + origin.fetch(prune=True) + remote_ref = f"origin/{current_branch}" + has_remote_branch = any(ref.name == remote_ref for ref in repo.refs) + logger.reason( + f"Pull remote branch check dashboard={dashboard_id} branch={current_branch} remote_ref={remote_ref} exists={has_remote_branch}", + extra={"src": "pull_changes"}, ) - logger.explore("Failed to pull changes", extra={"src": "pull_changes"}, error=str(e)) - raise HTTPException(status_code=500, detail=f"Git pull failed: {details}") - except HTTPException: - raise - except Exception as e: - logger.explore("Failed to pull changes", extra={"src": "pull_changes"}, error=str(e)) - raise HTTPException(status_code=500, detail=f"Git pull failed: {e!s}") + if not has_remote_branch: + raise HTTPException( + status_code=409, + detail=f"Remote branch '{current_branch}' does not exist yet. Push this branch first.", + ) + logger.reason(f"Pulling changes from origin/{current_branch}", extra={"src": "pull_changes"}) + repo.git.pull("--no-rebase", "origin", current_branch) + except ValueError: + logger.explore("Remote 'origin' not found", extra={"src": "pull_changes"}, payload={"dashboard_id": dashboard_id}) + raise HTTPException(status_code=400, detail="Remote 'origin' not configured") + except GitCommandError as e: + details = self._redact_pat_from_message(str(e), pat) + lowered = details.lower() + if "conflict" in lowered or "not possible to fast-forward" in lowered: + raise HTTPException( + status_code=409, + detail="Pull requires conflict resolution. Resolve conflicts in repository and repeat operation.", + ) + logger.explore("Failed to pull changes", extra={"src": "pull_changes"}, error=details) + raise HTTPException(status_code=500, detail=f"Git pull failed: {details}") + except HTTPException: + raise + except Exception as e: + details = self._redact_pat_from_message(str(e), pat) + logger.explore("Failed to pull changes", extra={"src": "pull_changes"}, error=details) + raise HTTPException(status_code=500, detail=f"Git pull failed: {details}") + finally: + if _original_pull_url is not None and origin is not None: + try: + origin.set_url(_original_pull_url) + except Exception: + pass # endregion pull_changes # #endregion GitServiceSyncMixin # #endregion GitServiceSyncMixin diff --git a/frontend/src/lib/components/layout/Navbar.svelte b/frontend/src/lib/components/layout/Navbar.svelte index 138ad443e..30344fc82 100644 --- a/frontend/src/lib/components/layout/Navbar.svelte +++ b/frontend/src/lib/components/layout/Navbar.svelte @@ -8,8 +8,6 @@ @LAYER UI @RELATION BINDS_TO -> [EXT:frontend:authStore] @RELATION BINDS_TO -> [EXT:frontend:i18n] -@RELATION DEPENDS_ON -> [LanguageSwitcher] - @UX_STATE: Idle -> Navigation links and settings menus are visible. @UX_STATE: Authenticated -> User identity and logout action are rendered. --> @@ -17,7 +15,6 @@ import { onMount } from 'svelte'; import { page } from '$app/state'; import { t } from '$lib/i18n/index.svelte.js'; - import { LanguageSwitcher } from '$lib/ui'; import { auth } from '$lib/auth/store.svelte.js'; import { goto } from '$app/navigation'; import { ROUTES } from '$lib/routes'; @@ -77,8 +74,6 @@ {/if} - - {#if _authState.isAuthenticated}
{_authState.user?.username} diff --git a/frontend/src/lib/i18n/locales/en/profile.json b/frontend/src/lib/i18n/locales/en/profile.json index 89857b6b1..b4972e8c1 100644 --- a/frontend/src/lib/i18n/locales/en/profile.json +++ b/frontend/src/lib/i18n/locales/en/profile.json @@ -4,6 +4,9 @@ "dashboard_preferences": "Dashboard Preferences", "security_access": "Security & Access", "read_only": "Read-only", + "preferences": "Preferences", + "saved": "Preferences saved", + "language": "Language", "security_read_only_note": "This section is read-only. Role changes are managed in Admin → Users.", "current_role": "Current Role", "role_source": "Role Source", @@ -43,6 +46,12 @@ "table_density_compact": "Compact", "table_density_comfortable": "Comfortable", "auto_open_task_drawer": "Automatically open task drawer for long-running tasks", + "notifications": "Notifications", + "notification_email": "Notification email", + "notification_email_placeholder": "Enter notification email", + "telegram_id": "Telegram ID", + "telegram_id_placeholder": "Enter Telegram ID", + "notify_on_fail": "Notify on validation failure", "filter_badge_active": "Profile filters active", "filter_badge_override": "Showing all dashboards temporarily", "filter_empty_state": "No dashboards found for active profile filters. Try adjusting your filter settings.", diff --git a/frontend/src/lib/i18n/locales/ru/profile.json b/frontend/src/lib/i18n/locales/ru/profile.json index dbd941ea3..dacc68eb1 100644 --- a/frontend/src/lib/i18n/locales/ru/profile.json +++ b/frontend/src/lib/i18n/locales/ru/profile.json @@ -4,6 +4,9 @@ "dashboard_preferences": "Настройки дашбордов", "security_access": "Безопасность и доступ", "read_only": "Только чтение", + "preferences": "Настройки", + "saved": "Настройки сохранены", + "language": "Язык", "security_read_only_note": "Этот раздел только для чтения. Изменение ролей выполняется в Админ → Users.", "current_role": "Текущая роль", "role_source": "Источник роли", @@ -43,6 +46,12 @@ "table_density_compact": "Компактная", "table_density_comfortable": "Комфортная", "auto_open_task_drawer": "Автоматически открывать Task Drawer для долгих задач", + "notifications": "Уведомления", + "notification_email": "Email для уведомлений", + "notification_email_placeholder": "Введите email для уведомлений", + "telegram_id": "Telegram ID", + "telegram_id_placeholder": "Введите Telegram ID", + "notify_on_fail": "Уведомлять при ошибках валидации", "filter_badge_active": "Активны фильтры профиля", "filter_badge_override": "Временно показаны все дашборды", "filter_empty_state": "По активным фильтрам профиля дашборды не найдены. Попробуйте изменить настройки фильтра.", diff --git a/frontend/src/routes/profile/+page.svelte b/frontend/src/routes/profile/+page.svelte index 65acd797a..567932a17 100644 --- a/frontend/src/routes/profile/+page.svelte +++ b/frontend/src/routes/profile/+page.svelte @@ -1,29 +1,171 @@ - + - + + - + -
- +
+ + {#if $t.profile?.description} +

{$t.profile.description}

+ {/if} {#if loading} -
-
-
-
+
+
+
+
{:else} - -
-
- - - - - - + options={startPageOptions} + id="profile-start-page" + name="start_page" + /> + + + {$t.profile?.auto_open_task_drawer || 'Automatically open task drawer for long-running tasks'} +
-
- + + + +
+ + +
+ + + +
+ + + +
+ + + + +
+ +
+ + {$t.profile?.git_token_hint || 'Token is never returned in plain text. Leave empty to keep current token.'} + + + {$t.profile?.git_token_masked_label || 'Current token'}: + {preferences.git_personal_access_token_masked || $t.profile?.git_token_not_set || 'Token is not set'} + +
+ +
+
+ + + +
+ + + + + +
+ + + +
+
+

{$t.profile?.current_role || 'Current Role'}

+

{security.current_role || security.roles?.[0] || '-'}

+
+
+

{$t.profile?.role_source || 'Role Source'}

+

{security.role_source || security.auth_source || '-'}

+
+
+

{$t.profile?.permissions || 'Permissions'}

+ {#if security.permissions?.length} +
+ {#each security.permissions as permission} + + {permission.key}: {permission.allowed ? 'allowed' : 'denied'} + + {/each} +
+ {:else} +

{$t.profile?.permission_none || 'No permissions available'}

+ {/if} +
+

+ {$t.profile?.security_read_only_note || 'This section is read-only. Role changes are managed in Admin -> Users.'} +

+
+ + +
+
- +
{/if}
diff --git a/frontend/src/routes/profile/__tests__/profile-preferences.integration.test.ts b/frontend/src/routes/profile/__tests__/profile-preferences.integration.test.ts index 4c6f17d82..5fbff4091 100644 --- a/frontend/src/routes/profile/__tests__/profile-preferences.integration.test.ts +++ b/frontend/src/routes/profile/__tests__/profile-preferences.integration.test.ts @@ -1,7 +1,7 @@ // #region ProfilePreferencesIntegrationTest [C:2] [TYPE Module] // @COMPLEXITY: 3 // @SEMANTICS: tests, profile, integration, load, save -// @PURPOSE: Verifies profile page loads preferences and saves them. +// @PURPOSE: Verifies profile page loads expanded preferences and saves them through PATCH contract. // @LAYER UI (Tests) // @RELATION DEPENDS_ON -> [ProfilePage] @@ -10,17 +10,49 @@ import { render, screen, fireEvent, waitFor } from "@testing-library/svelte"; import ProfilePage from "../+page.svelte"; import { api } from "$lib/api.js"; import { addToast } from "$lib/toasts.svelte.js"; +import { setTaskDrawerAutoOpenPreference } from "$lib/stores/taskDrawer.svelte.js"; const mockedApi = /** @type {any} */ (api); const mockedAddToast = /** @type {any} */ (addToast); +const mockedSetTaskDrawerAutoOpenPreference = /** @type {any} */ (setTaskDrawerAutoOpenPreference); + +const profileResponse = { + status: "success", + preference: { + user_id: "u-1", + superset_username: "superset_admin", + show_only_my_dashboards: true, + show_only_slug_dashboards: true, + git_username: "git-admin", + git_email: "git@example.test", + has_git_personal_access_token: true, + git_personal_access_token_masked: "ghp_***", + start_page: "dashboards", + auto_open_task_drawer: true, + dashboards_table_density: "comfortable", + telegram_id: "12345", + email_address: "notify@example.test", + notify_on_fail: true, + }, + security: { + read_only: true, + current_role: "Admin", + role_source: "local", + permissions: [{ key: "admin:settings", allowed: true }], + }, +}; vi.mock("$lib/api.js", () => ({ api: { getProfilePreferences: vi.fn(), - postApi: vi.fn(), + updateProfilePreferences: vi.fn(), }, })); +vi.mock("$lib/stores/taskDrawer.svelte.js", () => ({ + setTaskDrawerAutoOpenPreference: vi.fn(), +})); + vi.mock('$lib/toasts.svelte.js', () => ({ addToast: vi.fn(), })); @@ -30,15 +62,54 @@ vi.mock("$lib/auth/store.svelte.js", () => ({ })); vi.mock('$lib/i18n/index.svelte.js', () => ({ + locale: { + subscribe: (run) => { run('en'); return () => {}; }, + set: vi.fn(), + update: vi.fn(), + }, t: { subscribe: (run) => { run({ common: { save: "Save", cancel: "Cancel" }, nav: { profile: "Profile", dashboards: "Dashboards", datasets: "Datasets", reports: "Reports" }, profile: { + title: "Profile", + description: "Manage your profile preferences", saved: "Preferences saved", preferences: "Preferences", - start_page: "Start page", + user_preferences: "User Preferences", + dashboard_preferences: "Dashboard Preferences", + git_integration: "Git Integration", + notifications: "Notifications", + security_access: "Security & Access", + start_page: "Start Page", + start_page_dashboards: "Dashboards", + start_page_datasets: "Datasets", + start_page_reports: "Reports / Logs", + language: "Language", + table_density: "Table Density", + table_density_comfortable: "Comfortable", + table_density_compact: "Compact", + auto_open_task_drawer: "Automatically open task drawer for long-running tasks", + superset_account: "Your Apache Superset Account", + show_only_my_dashboards: "Show only my dashboards by default", + show_only_slug_dashboards: "Show only dashboards with slug by default", + git_username: "Git Username", + git_email: "Git Email", + git_token: "GitLab / GitHub Token", + git_token_clear: "Clear token", + git_token_masked_label: "Current token", + git_token_not_set: "Token is not set", + notification_email: "Notification email", + telegram_id: "Telegram ID", + notify_on_fail: "Notify on validation failure", + save_preferences: "Save Preferences", + save_success: "Preferences saved", + current_role: "Current Role", + role_source: "Role Source", + permissions: "Permissions", + permission_none: "No permissions available", + security_read_only_note: "This section is read-only.", }, }); return () => {}; @@ -50,16 +121,11 @@ vi.mock('$lib/i18n/index.svelte.js', () => ({ describe("profile-preferences.integration", () => { beforeEach(() => { vi.clearAllMocks(); - mockedApi.getProfilePreferences.mockResolvedValue({ - status: "success", - preference: { - user_id: "u-1", - start_page: "dashboards", - }, - }); + mockedApi.getProfilePreferences.mockResolvedValue(profileResponse); + mockedApi.updateProfilePreferences.mockResolvedValue(profileResponse); }); - it("loads preferences and renders the profile form", async () => { + it("loads preferences and renders expanded profile sections", async () => { render(ProfilePage); await waitFor(() => { @@ -67,30 +133,45 @@ describe("profile-preferences.integration", () => { }); expect(screen.getByText("Profile")).toBeDefined(); - expect(screen.getByText("Preferences")).toBeDefined(); - expect(screen.getByText("Start page")).toBeDefined(); - const select = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox")); - expect(select.value).toBe("dashboards"); + expect(screen.getByText("User Preferences")).toBeDefined(); + expect(screen.getByText("Dashboard Preferences")).toBeDefined(); + expect(screen.getByText("Git Integration")).toBeDefined(); + expect(screen.getByText("Notifications")).toBeDefined(); + expect(screen.getByText("Security & Access")).toBeDefined(); + + const startPage = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox", { name: "Start Page" })); + expect(startPage.value).toBe("dashboards"); + expect(screen.getByRole("combobox", { name: "Language" })).toBeDefined(); + expect(screen.getByDisplayValue("superset_admin")).toBeDefined(); + expect(screen.getByDisplayValue("git@example.test")).toBeDefined(); + expect(screen.getAllByText((_content, element) => Boolean(element?.textContent?.includes("ghp_***"))).length).toBeGreaterThan(0); + expect(screen.getByText("Admin")).toBeDefined(); + expect(mockedSetTaskDrawerAutoOpenPreference).toHaveBeenCalledWith(true); }); - it("saves preferences via API and shows success toast", async () => { - mockedApi.postApi.mockResolvedValue({ status: "success" }); - + it("saves preferences via PATCH API and shows success toast", async () => { render(ProfilePage); await waitFor(() => { expect(mockedApi.getProfilePreferences).toHaveBeenCalledTimes(1); }); - const select = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox")); - await fireEvent.change(select, { target: { value: "datasets" } }); + const startPage = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox", { name: "Start Page" })); + await fireEvent.change(startPage, { target: { value: "datasets" } }); - await fireEvent.click(screen.getByText("Save")); + const autoOpen = /** @type {HTMLInputElement} */ (screen.getByRole("checkbox", { name: "Automatically open task drawer for long-running tasks" })); + await fireEvent.click(autoOpen); + + await fireEvent.click(screen.getByRole("button", { name: "Save Preferences" })); await waitFor(() => { - expect(mockedApi.postApi).toHaveBeenCalledWith("/profile/preferences", { - preference: expect.objectContaining({ start_page: "datasets" }), - }); + expect(mockedApi.updateProfilePreferences).toHaveBeenCalledWith(expect.objectContaining({ + start_page: "datasets", + auto_open_task_drawer: false, + superset_username: "superset_admin", + git_email: "git@example.test", + notify_on_fail: true, + })); }); expect(mockedAddToast).toHaveBeenCalledWith("Preferences saved", "success"); diff --git a/frontend/src/routes/profile/__tests__/profile-settings-state.integration.test.ts b/frontend/src/routes/profile/__tests__/profile-settings-state.integration.test.ts index 7e2e7b0e0..f8f6a0007 100644 --- a/frontend/src/routes/profile/__tests__/profile-settings-state.integration.test.ts +++ b/frontend/src/routes/profile/__tests__/profile-settings-state.integration.test.ts @@ -1,7 +1,7 @@ // #region ProfileSettingsStateIntegrationTest [C:2] [TYPE Module] // @COMPLEXITY: 3 // @SEMANTICS: tests, profile, integration, load, change, save -// @PURPOSE: Verifies profile loads preferences, allows changes, and saves correctly. +// @PURPOSE: Verifies profile state changes for filters, notifications, and token clearing. // @LAYER UI (Tests) // @RELATION DEPENDS_ON -> [ProfilePage] @@ -14,13 +14,38 @@ import { addToast } from "$lib/toasts.svelte.js"; const mockedApi = /** @type {any} */ (api); const mockedAddToast = /** @type {any} */ (addToast); +const profileResponse = { + status: "success", + preference: { + user_id: "u-1", + superset_username: "", + show_only_my_dashboards: false, + show_only_slug_dashboards: true, + git_username: "", + git_email: "", + has_git_personal_access_token: true, + git_personal_access_token_masked: "***", + start_page: "dashboards", + auto_open_task_drawer: true, + dashboards_table_density: "comfortable", + telegram_id: "", + email_address: "", + notify_on_fail: true, + }, + security: { read_only: true, roles: [], permissions: [] }, +}; + vi.mock("$lib/api.js", () => ({ api: { getProfilePreferences: vi.fn(), - postApi: vi.fn(), + updateProfilePreferences: vi.fn(), }, })); +vi.mock("$lib/stores/taskDrawer.svelte.js", () => ({ + setTaskDrawerAutoOpenPreference: vi.fn(), +})); + vi.mock('$lib/toasts.svelte.js', () => ({ addToast: vi.fn(), })); @@ -30,15 +55,54 @@ vi.mock("$lib/auth/store.svelte.js", () => ({ })); vi.mock('$lib/i18n/index.svelte.js', () => ({ + locale: { + subscribe: (run) => { run('en'); return () => {}; }, + set: vi.fn(), + update: vi.fn(), + }, t: { subscribe: (run) => { run({ common: { save: "Save", cancel: "Cancel" }, nav: { profile: "Profile", dashboards: "Dashboards", datasets: "Datasets", reports: "Reports" }, profile: { + title: "Profile", + description: "Manage your profile preferences", saved: "Preferences saved", preferences: "Preferences", - start_page: "Start page", + user_preferences: "User Preferences", + dashboard_preferences: "Dashboard Preferences", + git_integration: "Git Integration", + notifications: "Notifications", + security_access: "Security & Access", + start_page: "Start Page", + start_page_dashboards: "Dashboards", + start_page_datasets: "Datasets", + start_page_reports: "Reports / Logs", + language: "Language", + table_density: "Table Density", + table_density_comfortable: "Comfortable", + table_density_compact: "Compact", + auto_open_task_drawer: "Automatically open task drawer for long-running tasks", + superset_account: "Your Apache Superset Account", + show_only_my_dashboards: "Show only my dashboards by default", + show_only_slug_dashboards: "Show only dashboards with slug by default", + git_username: "Git Username", + git_email: "Git Email", + git_token: "GitLab / GitHub Token", + git_token_clear: "Clear token", + git_token_masked_label: "Current token", + git_token_not_set: "Token is not set", + notification_email: "Notification email", + telegram_id: "Telegram ID", + notify_on_fail: "Notify on validation failure", + save_preferences: "Save Preferences", + save_success: "Preferences saved", + current_role: "Current Role", + role_source: "Role Source", + permissions: "Permissions", + permission_none: "No permissions available", + security_read_only_note: "This section is read-only.", }, }); return () => {}; @@ -50,45 +114,50 @@ vi.mock('$lib/i18n/index.svelte.js', () => ({ describe("profile-settings-state.integration", () => { beforeEach(() => { vi.clearAllMocks(); - mockedApi.getProfilePreferences.mockResolvedValue({ - status: "success", - preference: { - user_id: "u-1", - start_page: "dashboards", - }, - }); + mockedApi.getProfilePreferences.mockResolvedValue(profileResponse); + mockedApi.updateProfilePreferences.mockResolvedValue(profileResponse); }); - it("loads saved preferences and displays them", async () => { + it("loads saved preferences and displays them by label", async () => { render(ProfilePage); await waitFor(() => { expect(mockedApi.getProfilePreferences).toHaveBeenCalledTimes(1); }); - const select = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox")); - expect(select.value).toBe("dashboards"); - expect(screen.getByText("Profile")).toBeDefined(); + const startPage = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox", { name: "Start Page" })); + expect(startPage.value).toBe("dashboards"); + expect(screen.getByRole("combobox", { name: "Table Density" })).toBeDefined(); + expect(screen.getByText("No permissions available")).toBeDefined(); }); - it("saves changed preferences and shows success toast", async () => { - mockedApi.postApi.mockResolvedValue({ status: "success" }); - + it("saves changed filter, notification, and token-clear state", async () => { render(ProfilePage); await waitFor(() => { expect(mockedApi.getProfilePreferences).toHaveBeenCalledTimes(1); }); - const select = /** @type {HTMLSelectElement} */ (screen.getByRole("combobox")); - await fireEvent.change(select, { target: { value: "reports" } }); + await fireEvent.input(screen.getByRole("textbox", { name: "Your Apache Superset Account" }), { + target: { value: "owner_name" }, + }); + await fireEvent.click(screen.getByRole("checkbox", { name: "Show only my dashboards by default" })); + await fireEvent.input(screen.getByRole("textbox", { name: "Notification email" }), { + target: { value: "owner@example.test" }, + }); + await fireEvent.click(screen.getByRole("checkbox", { name: "Notify on validation failure" })); + await fireEvent.click(screen.getByRole("button", { name: "Clear token" })); - await fireEvent.click(screen.getByText("Save")); + await fireEvent.click(screen.getByRole("button", { name: "Save Preferences" })); await waitFor(() => { - expect(mockedApi.postApi).toHaveBeenCalledWith("/profile/preferences", { - preference: expect.objectContaining({ start_page: "reports" }), - }); + expect(mockedApi.updateProfilePreferences).toHaveBeenCalledWith(expect.objectContaining({ + superset_username: "owner_name", + show_only_my_dashboards: true, + email_address: "owner@example.test", + notify_on_fail: false, + git_personal_access_token: null, + })); }); expect(mockedAddToast).toHaveBeenCalledWith("Preferences saved", "success");