diff --git a/CHANGELOG.md b/CHANGELOG.md index b34fecb0..757c2ac3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), - **The conventions standard now resolves for projects created before it existed.** It previously read the committed `.roboco/conventions.yml` and the repo scan from `project.workspace_path` — a field only a manual API call ever set — so an older project (or one whose workspace was cleared) showed an empty "missing" map no matter what was pushed. The service now ensures a dedicated, default-branch read clone on demand and reads from it, persisting the resolved path + HEAD (the backfill). The panel tab, the spawn-time ambient block, and the per-task constraints all resolve the committed standard with no manual setup. - **The conventions ambient prompt block no longer truncates mid-line.** It now lists only modules that actually constrain a kind, and when the list would exceed its budget it trims at a line boundary with a `+N more` pointer instead of cutting a module in half. +- **The conventions read clone now stays current on a private repo.** Its refresh reused the orchestrator's token-less best-effort fetch, but the clone's remote URL is credential-stripped — so on a private repo the refresh fetch failed silently and the clone stayed frozen at clone-time, never seeing commits merged afterwards (the panel showed "auto-derived defaults" even after the standard was merged to the default branch). The refresh now performs a token-authenticated fetch + hard-reset, mirroring the clone. - **The toolchain gate no longer passes silently on an unverifiable workspace.** A `broken` interpreter still blocks; an `unknown` status — the smoke could not confirm the suite is collectable — now emits a warning when the gate proceeds, instead of slipping through unseen. - **The crypto tests are hermetic.** The Fernet round-trip tests supply their own key instead of depending on `ROBOCO_ENCRYPTION_KEY` in the environment, so they pass in any gate container without the production secret being injected. - **`ollama-init` is best-effort and gates startup on the models being present**, so a slow or unreachable model registry can no longer down a fully-cached deployment. diff --git a/roboco/services/workspace.py b/roboco/services/workspace.py index 475dc4b5..1eb9aa1e 100644 --- a/roboco/services/workspace.py +++ b/roboco/services/workspace.py @@ -804,10 +804,14 @@ class WorkspaceService: now = _monotonic() last = _read_clone_synced.get(str(workspace), -math.inf) if (now - last) >= _READ_CLONE_FETCH_TTL_SECONDS: + token = await self._read_clone_token(project_service, project_slug) await asyncio.to_thread(self._prune_broken_refs, workspace) - await self._fetch_origin_best_effort(workspace, project_slug) await asyncio.to_thread( - self._reset_to_default, workspace, default_branch + self._sync_read_clone, + workspace, + git_url, + default_branch, + token, ) _read_clone_synced[str(workspace)] = _monotonic() return workspace @@ -823,8 +827,40 @@ class WorkspaceService: return workspace @staticmethod - def _reset_to_default(workspace: Path, default_branch: str) -> None: - """Hard-reset the read clone to ``origin/``. Best-effort.""" + async def _read_clone_token(project_service: Any, project_slug: str) -> str | None: + """The project's decrypted git token, or ``None`` — never raises. + + The read-clone refresh must authenticate against a private origin, but a + public repo legitimately has no token, so (unlike the clone path) we do + not hard-require one here. + """ + from roboco.utils.crypto import EncryptionError + + try: + token = await project_service.get_decrypted_token_by_slug(project_slug) + except EncryptionError: + return None + return cast("str | None", token) + + @staticmethod + def _sync_read_clone( + workspace: Path, + git_url: str, + default_branch: str, + git_token: str | None, + ) -> None: + """Token-authenticated fetch + hard-reset of the read clone to origin's + default branch. Best-effort: logs and returns on failure. + + ``_clone_repo`` scrubs the token from ``.git/config`` (the secret-exfil + mitigation), so a plain ``git fetch origin`` cannot authenticate against + a PRIVATE repo — the refresh silently fails and the clone stays frozen at + clone-time, never seeing commits merged afterwards. The read clone runs + orchestrator-side and is never mounted into an agent container, so the + token is injected transiently into the fetch argv (mirroring the clone) + to keep a private repo current. + """ + auth_url = _inject_token_into_url(git_url, git_token) def _git(*args: str) -> subprocess.CompletedProcess[str]: return subprocess.run( @@ -835,8 +871,16 @@ class WorkspaceService: check=False, ) + fetched = _git("fetch", "--no-tags", auth_url, default_branch) + if fetched.returncode != 0: + logger.warning( + "conventions read-clone fetch failed", + workspace=str(workspace), + error=(fetched.stderr or fetched.stdout).strip()[:200], + ) + return _git("checkout", default_branch) - _git("reset", "--hard", f"origin/{default_branch}") + _git("reset", "--hard", "FETCH_HEAD") async def _clone_repo( self, diff --git a/tests/unit/services/test_workspace_read_clone.py b/tests/unit/services/test_workspace_read_clone.py new file mode 100644 index 00000000..db3c3df3 --- /dev/null +++ b/tests/unit/services/test_workspace_read_clone.py @@ -0,0 +1,68 @@ +"""WorkspaceService._sync_read_clone — the conventions read-clone refresh. + +The read clone is hard-reset to the default branch on every refresh. The bug it +fixes: the old refresh used a token-less fetch, so a private repo's clone stayed +frozen at clone-time and never saw commits merged afterwards. This proves the +refresh actually advances the clone to a post-clone commit. +""" + +from __future__ import annotations + +import subprocess +from typing import TYPE_CHECKING + +from roboco.services.workspace import WorkspaceService + +if TYPE_CHECKING: + from pathlib import Path + + +def _git(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["git", *args], cwd=str(cwd), capture_output=True, text=True, check=False + ) + + +def _commit(repo: Path, message: str) -> str: + _git(repo, "add", "-A") + _git(repo, "-c", "user.email=t@t", "-c", "user.name=t", "commit", "-qm", message) + return _git(repo, "rev-parse", "HEAD").stdout.strip() + + +def test_sync_read_clone_advances_to_a_post_clone_commit(tmp_path: Path) -> None: + origin = tmp_path / "origin" + origin.mkdir() + _git(origin, "init", "-q", "-b", "master") + (origin / "README.md").write_text("v1\n") + _commit(origin, "first") + + clone = tmp_path / "clone" + _git(tmp_path, "clone", "-q", str(origin), str(clone)) + first = _git(clone, "rev-parse", "HEAD").stdout.strip() + + # A new commit lands on origin AFTER the clone — the frozen-clone scenario. + (origin / "NEW.txt").write_text("added later\n") + second = _commit(origin, "second") + assert first != second + + # token=None: a file:// fetch needs no auth (mirrors a public-repo refresh); + # for a private https repo the token would be injected into the fetch URL. + WorkspaceService._sync_read_clone(clone, f"file://{origin}", "master", None) + + assert _git(clone, "rev-parse", "HEAD").stdout.strip() == second + assert (clone / "NEW.txt").is_file() + + +def test_sync_read_clone_is_best_effort_on_unreachable_origin(tmp_path: Path) -> None: + origin = tmp_path / "origin" + origin.mkdir() + _git(origin, "init", "-q", "-b", "master") + (origin / "README.md").write_text("v1\n") + _commit(origin, "first") + clone = tmp_path / "clone" + _git(tmp_path, "clone", "-q", str(origin), str(clone)) + head = _git(clone, "rev-parse", "HEAD").stdout.strip() + + # A bogus origin must not raise — the refresh logs and leaves the clone as-is. + WorkspaceService._sync_read_clone(clone, "file:///nonexistent/repo", "master", None) + assert _git(clone, "rev-parse", "HEAD").stdout.strip() == head