diff --git a/kb-server/app/api/routes/notes.py b/kb-server/app/api/routes/notes.py index ba1fdcd..8949551 100644 --- a/kb-server/app/api/routes/notes.py +++ b/kb-server/app/api/routes/notes.py @@ -111,7 +111,7 @@ def write_note( [path], f"human: update {path}", actor=git_service.USER_ACTOR ) if settings.git_push_enabled: - git_service.push(actor=git_service.USER_ACTOR) + git_service.push() else: batcher.enqueue(path) @@ -145,6 +145,6 @@ def delete_note( [path], f"human: delete {path}", actor=git_service.USER_ACTOR ) if settings.git_push_enabled: - git_service.push(actor=git_service.USER_ACTOR) + git_service.push() else: batcher.enqueue(path) diff --git a/kb-server/app/services/git_service.py b/kb-server/app/services/git_service.py index 1d35f02..0eed223 100644 --- a/kb-server/app/services/git_service.py +++ b/kb-server/app/services/git_service.py @@ -7,16 +7,27 @@ import stat import subprocess import tempfile +from enum import Enum from pathlib import Path -from typing import Literal from app.core.config import settings log = logging.getLogger(__name__) -GitActor = Literal["user", "agent"] -USER_ACTOR: GitActor = "user" -AGENT_ACTOR: GitActor = "agent" + +class GitActor(str, Enum): + """Actor identity for git operations. + + Use ``USER`` for human-origin writes on the configured base branch and + ``AGENT`` for API/batch workflows that operate on kb-api/* branches. + """ + + USER = "user" + AGENT = "agent" + + +USER_ACTOR = GitActor.USER +AGENT_ACTOR = GitActor.AGENT class GitError(Exception): @@ -76,13 +87,15 @@ def _actor_env(actor: GitActor | None) -> dict[str, str]: committer_email = settings.git_user_committer_email or author_email ssh_command = settings.git_user_ssh_command https_token = "" - else: + elif actor == AGENT_ACTOR: author_name = settings.git_agent_author_name author_email = settings.git_agent_author_email committer_name = settings.git_agent_committer_name or author_name committer_email = settings.git_agent_committer_email or author_email ssh_command = settings.git_agent_ssh_command https_token = settings.git_agent_https_token + else: + raise GitError(f"unsupported git actor: {actor}") if author_name: env["GIT_AUTHOR_NAME"] = author_name @@ -204,20 +217,21 @@ def commit_files( return sha -def push( - remote: str | None = None, - branch: str | None = None, - retries: int = 2, - actor: GitActor | None = None, -) -> None: - """Push to remote with simple retry on transient failures.""" - remote = remote or settings.git_remote - branch = branch or settings.git_branch +def push(retries: int = 2) -> None: + """Push the configured remote/base branch for human-origin writes. + + This helper is intentionally user-only; agent writes use ``push_branch()`` + on kb-api/* branches. + + See also: ``push_branch()``. + """ + remote = settings.git_remote + branch = settings.git_branch last_err: Exception | None = None for attempt in range(1, retries + 1): try: - _run("push", remote, branch, actor=actor) + _run("push", remote, branch, actor=USER_ACTOR) log.info("pushed %s/%s", remote, branch) return except GitError as exc: diff --git a/kb-server/app/services/github_service.py b/kb-server/app/services/github_service.py index 924cb89..6c6ece0 100644 --- a/kb-server/app/services/github_service.py +++ b/kb-server/app/services/github_service.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import threading from typing import Any import httpx @@ -13,6 +14,8 @@ log = logging.getLogger(__name__) GITHUB_API = "https://api.github.com" +_agent_token_fallback_logged = False +_agent_token_fallback_lock = threading.Lock() class GitHubError(Exception): @@ -20,9 +23,22 @@ class GitHubError(Exception): def _token_for_actor(actor: GitActor = AGENT_ACTOR) -> str: - if actor == AGENT_ACTOR and settings.github_agent_token: - return settings.github_agent_token - return settings.github_token + global _agent_token_fallback_logged + if actor == AGENT_ACTOR: + if settings.github_agent_token: + return settings.github_agent_token + if settings.github_token: + with _agent_token_fallback_lock: + if not _agent_token_fallback_logged: + log.warning( + "GITHUB_AGENT_TOKEN not configured; falling back to GITHUB_TOKEN for agent GitHub API calls" + ) + _agent_token_fallback_logged = True + return settings.github_token + return "" + if actor == GitActor.USER: + return settings.github_token + raise GitHubError(f"unsupported GitHub actor: {actor}") def _headers(actor: GitActor = AGENT_ACTOR) -> dict[str, str]: diff --git a/kb-server/app/workers/autosave.py b/kb-server/app/workers/autosave.py index d676ce0..d885523 100644 --- a/kb-server/app/workers/autosave.py +++ b/kb-server/app/workers/autosave.py @@ -154,7 +154,7 @@ def _do_autosave(self, files: set[str]) -> None: session.commit() if settings.git_push_enabled: - git_service.push(actor=git_service.USER_ACTOR) + git_service.push() session.add( VaultEvent( event_type="autosave_push", diff --git a/kb-server/tests/test_autosave.py b/kb-server/tests/test_autosave.py index 1149794..d2767af 100644 --- a/kb-server/tests/test_autosave.py +++ b/kb-server/tests/test_autosave.py @@ -90,4 +90,4 @@ def test_autosave_commits_and_pushes_as_user(self, watcher: AutosaveWatcher): gs.commit_files.assert_called_once() assert gs.commit_files.call_args.kwargs["actor"] == gs.USER_ACTOR - gs.push.assert_called_once_with(actor=gs.USER_ACTOR) + gs.push.assert_called_once_with() diff --git a/kb-server/tests/test_git_service.py b/kb-server/tests/test_git_service.py index 8f3773e..71dbace 100644 --- a/kb-server/tests/test_git_service.py +++ b/kb-server/tests/test_git_service.py @@ -110,7 +110,6 @@ def test_push_fails_without_remote(self, tmp_vault: Path): with pytest.raises(GitError, match="push failed"): push(retries=1) - class TestRunAuthBehavior: def test_sets_non_interactive_git_env(self): with patch("app.services.git_service.subprocess.run") as run_mock: diff --git a/kb-server/tests/test_github_service.py b/kb-server/tests/test_github_service.py index 0ab8054..1cd5194 100644 --- a/kb-server/tests/test_github_service.py +++ b/kb-server/tests/test_github_service.py @@ -1,6 +1,9 @@ from unittest.mock import MagicMock, patch +import pytest + from app.services import github_service +from app.services.git_service import GitActor class TestGitHubTokenSelection: @@ -43,3 +46,19 @@ def test_create_pr_falls_back_to_legacy_token(self): github_service.create_pr("kb-api/2026-03-11", "Title") assert client.post.call_args.kwargs["headers"]["Authorization"] == "Bearer fallback-token" + + def test_headers_rejects_unsupported_actor(self): + with patch("app.services.github_service.settings") as settings: + settings.github_agent_token = "agent-token" + settings.github_token = "fallback-token" + + with pytest.raises(github_service.GitHubError, match="unsupported GitHub actor"): + github_service._headers(actor="other") # type: ignore[arg-type] + + def test_user_actor_uses_legacy_token(self): + with patch("app.services.github_service.settings") as settings: + settings.github_agent_token = "agent-token" + settings.github_token = "fallback-token" + + headers = github_service._headers(actor=GitActor.USER) + assert headers["Authorization"] == "Bearer fallback-token" diff --git a/kb-server/tests/test_source_and_delete.py b/kb-server/tests/test_source_and_delete.py index 72e2c43..d12e341 100644 --- a/kb-server/tests/test_source_and_delete.py +++ b/kb-server/tests/test_source_and_delete.py @@ -182,7 +182,7 @@ def test_human_write_uses_user_actor(self): "human: update notes/test.md", actor=gs.USER_ACTOR, ) - gs.push.assert_called_once_with(actor=gs.USER_ACTOR) + gs.push.assert_called_once_with() def test_api_write_enqueues_without_direct_commit(self): session = MagicMock() @@ -220,4 +220,4 @@ def test_human_delete_uses_user_actor(self): "human: delete notes/test.md", actor=gs.USER_ACTOR, ) - gs.push.assert_called_once_with(actor=gs.USER_ACTOR) + gs.push.assert_called_once_with()