From 1ebe8afc23cedc4edf03bb803626e873645816d8 Mon Sep 17 00:00:00 2001 From: Dev Agent Date: Tue, 2 Jun 2026 21:12:06 +0300 Subject: [PATCH] feat(worktree): git worktree per task to isolate shared /repos (ORCH-2 / S-4) - add src/git_worktree.py: ensure/remove/get_worktree_path - config: worktrees_dir=/repos/_wt - launcher: agent runs in per-branch worktree; task-file + commit/push in worktree; no shared checkout - qg/checks: read artifacts + run make test from worktree (branch arg, backward-compatible) - webhooks/plane: pass branch into QG dispatch; review fallback from worktree - webhooks/gitea: keep read-only branch --contains in main clone (documented) - tests: test_git_worktree.py (isolation) + update test_launcher write-task-file - docs: ARCHITECTURE worktree section + BUGFIXES_2026-06-02_ORCH2 Preserves B-1/B-2/S-1/S-5 fixes (paths now point at worktree). --- docs/ARCHITECTURE.md | 40 ++++++-- docs/BUGFIXES_2026-06-02_ORCH2.md | 81 ++++++++++++++++ src/agents/launcher.py | 72 +++++++------- src/config.py | 1 + src/git_worktree.py | 107 +++++++++++++++++++++ src/qg/checks.py | 57 ++++++----- src/webhooks/gitea.py | 4 +- src/webhooks/plane.py | 9 +- tests/test_git_worktree.py | 152 ++++++++++++++++++++++++++++++ tests/test_launcher.py | 40 +++++--- 10 files changed, 474 insertions(+), 89 deletions(-) create mode 100644 docs/BUGFIXES_2026-06-02_ORCH2.md create mode 100644 src/git_worktree.py create mode 100644 tests/test_git_worktree.py diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 696b659..7aec12f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -39,7 +39,7 @@ STAGE_TRANSITIONS = { |-------|---------------| | check_analysis_approved | Filesystem: 4 файла + :approved: comment в Plane | | check_architecture_done | Filesystem: ADR dir или infra-requirements.md | -| check_tests_local | Оркестратор сам гоняет `make test` в `/repos/` (judge по exit-code). Заменил check_ci_green: Gitea CI не сконфигурирован. | +| check_tests_local | Оркестратор сам гоняет `make test` в **worktree задачи** `/repos/_wt//` (judge по exit-code). Заменил check_ci_green: Gitea CI не сконфигурирован. Worktree-изоляция → безопасно при параллельных задачах (ORCH-2 / S-4). | | check_reviewer_verdict | Filesystem: читает `verdict: APPROVED\|REQUEST_CHANGES` из YAML-frontmatter `12-review.md` (только машиночитаемое поле, не подстроки в тексте) | | check_tests_passed | Filesystem: test-report.md содержит "PASS" | | check_ci_green | (legacy) Gitea API: GET /commits/{branch}/status — больше не используется как QG развития | @@ -188,7 +188,7 @@ services: Каждый агент — Claude CLI с: - **System prompt**: `.openclaw/agents/{role}.md` (в репозитории) -- **Task file**: `.task-{suffix}.md` — генерируется orchestrator **прямой записью в смонтированный volume `/repos//`** (B-1, без docker). В `.gitignore` репозитория проекта (рантайм-артефакт, не коммитится). +- **Task file**: `.task-{suffix}.md` — генерируется orchestrator **прямой записью в worktree задачи** `/repos/_wt///` (B-1, без docker; ORCH-2 — в изолированную рабочую копию, не в shared `/repos/`). В `.gitignore` репозитория проекта (рантайм-артефакт, не коммитится). - **Tools**: Read, Write, Edit, Bash - **Output**: `--print` mode (весь вывод в stdout после завершения) @@ -201,13 +201,39 @@ services: | tester | test-report.md, e2e results | 10-25 мин | | deployer | merge PR + SSH deploy-hook + smoke | 5-10 мин | +## Изоляция через git worktree (ORCH-2 / S-4) + +Каждая задача (= одна git-ветка) работает в **изолированной git worktree**, а не в общем +`/repos/`. Это убирает гонки `git checkout`, когда две задачи активны одновременно. + +``` +/repos/ ← основной clone (fetch / управление worktree, read-only запросы) +/repos/_wt// ← worktree конкретной задачи (рабочая копия агента) +``` + +Модуль `src/git_worktree.py`: +- `get_worktree_path(repo, branch)` — путь worktree (не создаёт). +- `ensure_worktree(repo, branch)` — создаёт (или переиспользует) worktree на нужной ветке; + для новой ветки создаёт её от `origin/main`. Возвращает путь. +- `remove_worktree(repo, branch)` — опциональная очистка при `done`. + +Где используется worktree: +- **launcher**: агент запускается с `cd ` (без `git checkout` в cmd); task-файл + пишется в worktree; commit/push в `_monitor_agent` идут в worktree. +- **qg/checks**: чтение артефактов агента (`check_analysis_complete`, `check_architecture_done`, + `check_tests_passed`, `check_reviewer_verdict`) и `check_tests_local` (`make test`) — из worktree. + Артефакт-функции принимают опциональный `branch`; без него падают на shared `/repos/` + (обратная совместимость). +- **webhooks/gitea**: `git branch -r --contains ` оставлен в основном clone — это + **read-only** запрос (нет checkout/мутации), гонок не создаёт. + +> Один branch может быть checked out только в одной worktree одновременно — +> это и есть нужное свойство: одна задача = одна ветка = одна worktree. + ## Известные ограничения -- **Shared `/repos` checkout (гонки при параллельных задачах).** Все агенты и - `check_tests_local` делают `git checkout` в одном `/repos/`. При двух - одновременно активных задачах checkout одной перетрёт рабочую копию другой. - Пока приемлемо (задачи идут последовательно). **Исправление — git worktree per task/branch - (запланировано отдельной задачей S-4).** +- ~~Shared `/repos` checkout (гонки при параллельных задачах).~~ **РЕШЕНО (ORCH-2 / S-4):** + git worktree per task/branch — см. раздел «Изоляция через git worktree» ниже. - **In-process daemon-потоки.** Агенты запускаются в daemon-потоках uvicorn. При рестарте uvicorn запущенные агенты осиротевают → ловит orphan-recovery (M-1). Целевая архитектура — очередь задач (F-2b, отдельно). diff --git a/docs/BUGFIXES_2026-06-02_ORCH2.md b/docs/BUGFIXES_2026-06-02_ORCH2.md new file mode 100644 index 0000000..6034d57 --- /dev/null +++ b/docs/BUGFIXES_2026-06-02_ORCH2.md @@ -0,0 +1,81 @@ +# ORCH-2 / S-4 — git worktree per task (изоляция shared /repos) + +**Дата:** 2026-06-02 +**Ветка:** `feature/ORCH-2-worktree` +**Источник:** `AUDIT_2026-06-02.md` (SERIOUS S-4), `DEV_TASK_ORCH2_WORKTREE.md` +**Исполнитель:** Dev (Opus 4.8 Tokenator) + +## Проблема (S-4) + +Все git-операции (`launcher.launch` cmd, `_monitor_agent` commit/push, `check_tests_local`) +делали `git checkout ` в одном общем `/repos/`. При двух активных задачах +checkout одной перетирал рабочую копию другой → гонки (на ET-009 это дало «два коллектора» +и путаницу веток). + +## Решение + +**git worktree per branch.** Каждая задача (ветка) работает в изолированной рабочей копии: + +``` +/repos/ ← основной clone (fetch / worktree mgmt / read-only) +/repos/_wt// ← worktree задачи (рабочая копия агента) +``` + +## Изменения + +| Файл | Что | +|------|-----| +| `src/config.py` | + `worktrees_dir: str = "/repos/_wt"` | +| `src/git_worktree.py` (новый) | `_safe`, `get_worktree_path`, `ensure_worktree`, `remove_worktree` | +| `src/agents/launcher.py` | `launch()`: ветка резолвится заранее → `ensure_worktree`; cmd = `cd ` без `git checkout`; `_write_task_file(repo, branch, ...)` пишет в worktree; `_monitor_agent` commit/push в worktree (checkout убран); чтение `01-questions.md`/`10-conflict.md` из worktree; QG-диспетчер прокидывает `branch` | +| `src/qg/checks.py` | `_repo_path(repo, branch)` helper (worktree если есть, иначе shared); артефакт-чеки получили опциональный `branch`; `check_tests_local` → `ensure_worktree` + `make test` в worktree (TODO про S-4 удалён) | +| `src/webhooks/plane.py` | QG-диспетчер прокидывает `branch`; review-файл fallback читается из worktree | +| `src/webhooks/gitea.py` | `git branch -r --contains ` — подтверждено read-only, оставлено в main clone (+ комментарий) | +| `tests/test_git_worktree.py` (новый) | покрытие `_safe`/`get_worktree_path`/`ensure_worktree`/`remove_worktree` + изоляция двух веток (реальные локальные git-репо в tmp, без сети) | +| `tests/test_launcher.py` | `TestWriteTaskFile` обновлён под новую сигнатуру (запись в worktree) | +| `docs/ARCHITECTURE.md` | раздел «Изоляция через git worktree»; убран пункт про shared-checkout гонки | + +## Совместимость с прежними фиксами + +- **B-1** (запись task-файла без docker, прямой `open()`): сохранена — теперь путь = worktree. +- **B-2** (Popen stdout → файл, monitor `proc.wait()` без зомби): не тронут. +- **S-5** (`check_reviewer_verdict` — только YAML-frontmatter): не тронут, добавлен лишь worktree-путь. +- **S-1** (`check_tests_local` — свой `make test` вместо Gitea CI): сохранён, тесты теперь в worktree. + +Обратная совместимость QG-диспетчеризации: артефакт-чеки принимают `branch` опционально +(default `None` → shared `/repos/`), поэтому существующие 2-арг вызовы/тесты не сломаны. + +## Проверка + +```bash +# Тесты (в контейнере через образ — хостовый .venv сломан): +IMG=$(docker inspect orchestrator --format '{{.Config.Image}}') +docker run --rm -v /home/slin/repos/orchestrator:/code -w /code --entrypoint python3 $IMG -m pytest tests/ -q +# → 37 passed, 9 failed (pre-existing test_webhooks 401/signature — НЕ относятся к ORCH-2, +# идентичны baseline на main). + +# test_git_worktree.py изолированно → 9 passed. +``` + +### Тест изоляции (в работающем контейнере) + +```bash +docker exec orchestrator python3 -c " +import sys; sys.path.insert(0,'/app') +from src.git_worktree import ensure_worktree +import subprocess +p1 = ensure_worktree('enduro-trails','feature/wt-test-A') +p2 = ensure_worktree('enduro-trails','feature/wt-test-B') +b1 = subprocess.run(['git','-C',p1,'branch','--show-current'],capture_output=True,text=True).stdout.strip() +b2 = subprocess.run(['git','-C',p2,'branch','--show-current'],capture_output=True,text=True).stdout.strip() +assert p1!=p2 and b1!=b2, 'NOT ISOLATED' +print('ISOLATION OK', p1, p2, b1, b2) +" +``` + +(Результат прогона на сервере — см. ниже / в отчёте Стрим.) + +## Ограничения / заметки + +- Очередь задач (ORCH-1 / F-2b) **не** входит в эту задачу. +- `remove_worktree` существует, но автоматический вызов при `done` не подключён (опционально, отдельным шагом). diff --git a/src/agents/launcher.py b/src/agents/launcher.py index 47d6574..b129412 100644 --- a/src/agents/launcher.py +++ b/src/agents/launcher.py @@ -6,6 +6,7 @@ import signal from ..config import settings from ..db import get_db, get_task_by_repo_branch, update_task_stage from ..stages import get_next_stage, get_qg_for_stage, get_agent_for_stage +from ..git_worktree import ensure_worktree, get_worktree_path from ..qg.checks import QG_CHECKS from ..notifications import notify_stage_change, notify_qg_failure, notify_agent_started, notify_agent_finished, notify_approve_requested from ..plane_sync import notify_stage_change as plane_notify_stage, add_comment as plane_add_comment @@ -71,15 +72,22 @@ class AgentLauncher: if not config: raise ValueError(f"Unknown agent: {agent}") - # Container-local path (repos mounted at /repos) + # Main clone lives at /repos/; the agent works in an isolated worktree + # (ORCH-2 / S-4) so concurrent tasks never fight over a shared checkout. local_repo_path = os.path.join(settings.repos_dir, repo) - if not os.path.isdir(local_repo_path): raise FileNotFoundError(f"Repo not found: {local_repo_path}") - # Write task file if content provided (B-1: direct write to mounted /repos, no docker) + # Determine branch (needed before we touch the worktree / task file). + _br_row = get_db().execute("SELECT branch FROM tasks WHERE id=?", (task_id,)).fetchone() if task_id else None + agent_branch = _br_row[0] if _br_row else "main" + + # Ensure the per-branch worktree exists and is on the right branch. + work_path = ensure_worktree(repo, agent_branch) + + # Write task file if content provided (B-1: direct write; now into the worktree). if task_content: - self._write_task_file(repo, config["task_file"], task_content) + self._write_task_file(repo, agent_branch, config["task_file"], task_content) # Record run in DB conn = get_db() @@ -99,15 +107,13 @@ class AgentLauncher: system_prompt = config["system_prompt"] allowed_tools = config["allowed_tools"] - # Determine branch for checkout - _br_row = get_db().execute("SELECT branch FROM tasks WHERE id=?", (task_id,)).fetchone() if task_id else None - agent_branch = _br_row[0] if _br_row else "main" - model = config.get("model", "") model_flag = f"--model {model} " if model else "" + # No git fetch/checkout here: ensure_worktree() already put the worktree on + # the right branch. The agent simply runs inside its isolated work_path. cmd = ( - f'cd {local_repo_path} && git fetch origin 2>/dev/null; git checkout {agent_branch} 2>/dev/null || git checkout -b {agent_branch} origin/{agent_branch} 2>/dev/null; ' + f'cd {work_path} && ' f'{self.CLAUDE_BIN} --print ' f'{model_flag}' f'"$(cat {task_file})" ' @@ -219,8 +225,10 @@ class AgentLauncher: notify_agent_finished(run_id, agent, exit_code, task_id=_task_id, duration_s=_duration_s) - # Commit and push any changes - repo_path = os.path.join(settings.repos_dir, repo) + # Commit and push any changes — in the per-branch worktree (ORCH-2 / S-4), + # NOT in the shared /repos/. The worktree is already on `branch` + # (ensure_worktree did the checkout), so no checkout is needed here. + repo_path = get_worktree_path(repo, branch) try: git_env = { **os.environ, @@ -230,20 +238,6 @@ class AgentLauncher: "GIT_COMMITTER_NAME": "claude-bot", "GIT_COMMITTER_EMAIL": "claude-bot@mva154.local", } - # Checkout feature branch before committing - subprocess.run( - ["git", "-C", repo_path, "fetch", "origin"], - capture_output=True, text=True, timeout=30, env=git_env - ) - checkout_result = subprocess.run( - ["git", "-C", repo_path, "checkout", branch], - capture_output=True, text=True, timeout=30, env=git_env - ) - if checkout_result.returncode != 0: - subprocess.run( - ["git", "-C", repo_path, "checkout", "-b", branch, f"origin/{branch}"], - capture_output=True, text=True, timeout=30, env=git_env - ) result = subprocess.run( ["git", "-C", repo_path, "status", "--porcelain"], capture_output=True, text=True, timeout=10, env=git_env @@ -351,7 +345,7 @@ class AgentLauncher: if agent == "analyst" and qg_name == "check_analysis_approved" and work_item_id: files_check = QG_CHECKS.get("check_analysis_complete") if files_check: - files_ok, _ = files_check(repo, work_item_id) + files_ok, _ = files_check(repo, work_item_id, branch) if files_ok: # Full artifacts ready -> In Review from ..plane_sync import set_issue_in_review @@ -364,10 +358,10 @@ class AgentLauncher: notify_approve_requested(task_id) logger.info(f"Task {task_id}: analyst finished, requested :approved: in Plane") else: - # Check if questions file exists + # Check if questions file exists (in the task worktree) import os as _os questions_path = _os.path.join( - settings.repos_dir, repo, + get_worktree_path(repo, branch), f"docs/work-items/{work_item_id}/01-questions.md" ) if _os.path.isfile(questions_path): @@ -392,11 +386,14 @@ class AgentLauncher: ) return elif qg_name in ("check_ci_green", "check_tests_local"): + # (repo, branch) signature — already worktree-aware. passed, reason = check_fn(repo, branch) elif qg_name == "check_tests_passed": - passed, reason = check_fn(repo, work_item_id or "") + # Artifact check — pass branch so it reads from the worktree. + passed, reason = check_fn(repo, work_item_id or "", branch) else: - passed, reason = check_fn(repo, work_item_id or "") + # Other artifact checks (check_architecture_done, etc.) — worktree-aware. + passed, reason = check_fn(repo, work_item_id or "", branch) if not passed: logger.info(f"Task {task_id}: QG '{qg_name}' not passed after {agent}: {reason}") @@ -461,7 +458,7 @@ class AgentLauncher: if agent == "architect" and qg_name == "check_architecture_done" and not passed: import os as _os conflict_path = _os.path.join( - settings.repos_dir, repo, + get_worktree_path(repo, branch), f"docs/work-items/{work_item_id}/10-conflict.md" ) if _os.path.isfile(conflict_path): @@ -578,15 +575,16 @@ class AgentLauncher: logger.error(f"Auto-merge failed for {branch}: {e}") return False - def _write_task_file(self, repo: str, task_file: str, content: str): - """Write task file directly to the mounted repo volume (/repos). + def _write_task_file(self, repo: str, branch: str, task_file: str, content: str): + """Write task file directly into the task's worktree. - B-1 fix: no docker. The repos directory is mounted RW at settings.repos_dir - (/repos inside the container), so write straight to /repos//. + B-1 fix: no docker (direct open()). ORCH-2/S-4: the target is the per-branch + worktree (/repos/_wt//), not the shared /repos/, so the + agent reads the task ZADANIE from its own isolated working copy. Raise on failure instead of silently swallowing errors. """ - container_repo_path = os.path.join(settings.repos_dir, repo) # /repos/ - full_path = os.path.join(container_repo_path, task_file) + work_path = get_worktree_path(repo, branch) # /repos/_wt// + full_path = os.path.join(work_path, task_file) try: with open(full_path, "w", encoding="utf-8") as f: f.write(content) diff --git a/src/config.py b/src/config.py index 1d00d2c..07c1fed 100644 --- a/src/config.py +++ b/src/config.py @@ -20,6 +20,7 @@ class Settings(BaseSettings): claude_bin: str = "/opt/claude-code/bin/claude.exe" repos_dir: str = "/repos" host_repos_dir: str = "/home/slin/repos" + worktrees_dir: str = "/repos/_wt" # ORCH-2 / S-4: isolated worktree per task/branch # DB db_path: str = "/app/data/orchestrator.db" diff --git a/src/git_worktree.py b/src/git_worktree.py new file mode 100644 index 0000000..1721907 --- /dev/null +++ b/src/git_worktree.py @@ -0,0 +1,107 @@ +"""Git worktree management — isolated working copy per task/branch (ORCH-2 / S-4). + +Background +---------- +Previously every git operation (checkout/commit/push/test) ran in the single shared +clone ``/repos/``. With two active tasks a ``git checkout`` of one branch would +overwrite the working copy of the other -> races (see AUDIT S-4 / ET-009 "two collectors"). + +Solution +-------- +Each task (branch) gets an isolated git worktree:: + + /repos/ <- main clone (fetch / worktree management) + /repos/_wt// <- worktree for one task/branch (agent works here) + +A branch can only be checked out in ONE worktree at a time, which is exactly the +property we want: one task = one branch = one worktree. +""" +import os +import re +import subprocess +import logging +from .config import settings + +logger = logging.getLogger("orchestrator.git_worktree") + + +def _safe(branch: str) -> str: + """Filesystem-safe branch name for use in a path component.""" + return re.sub(r"[^A-Za-z0-9._-]", "_", branch) + + +def get_worktree_path(repo: str, branch: str) -> str: + """Path of the worktree for (repo, branch). Does NOT create it.""" + return os.path.join(settings.worktrees_dir, repo, _safe(branch)) + + +def _main_repo(repo: str) -> str: + return os.path.join(settings.repos_dir, repo) + + +def ensure_worktree(repo: str, branch: str) -> str: + """Create (or reuse) an isolated worktree for ``branch``. Returns its path. + + Main clone stays at ``/repos/``. Worktree lives at + ``/repos/_wt//``. + + - If the worktree already exists, it is fetched + fast-aligned to the branch + (and to ``origin/`` when that remote branch exists). + - If the branch exists (locally or on origin) it is checked out into a fresh + worktree; otherwise a new branch is created from ``origin/main``. + """ + main_repo = _main_repo(repo) + wt = get_worktree_path(repo, branch) + + if not os.path.isdir(main_repo): + raise FileNotFoundError(f"Main repo not found: {main_repo}") + + # Always refresh refs in the main clone first. + subprocess.run(["git", "-C", main_repo, "fetch", "origin"], + capture_output=True, timeout=60) + + # Reuse existing worktree (.git may be a dir or a file pointer for worktrees). + if os.path.isdir(os.path.join(wt, ".git")) or os.path.isfile(os.path.join(wt, ".git")): + subprocess.run(["git", "-C", wt, "fetch", "origin"], capture_output=True, timeout=60) + subprocess.run(["git", "-C", wt, "checkout", branch], capture_output=True, timeout=30) + # Align to remote only if the remote branch exists (avoid wiping local-only work). + rb = subprocess.run( + ["git", "-C", wt, "rev-parse", "--verify", "--quiet", f"origin/{branch}"], + capture_output=True, + ) + if rb.returncode == 0: + subprocess.run(["git", "-C", wt, "reset", "--hard", f"origin/{branch}"], + capture_output=True, timeout=30) + logger.info(f"Worktree reused: {wt} (branch {branch})") + return wt + + os.makedirs(os.path.dirname(wt), exist_ok=True) + + # Try to attach an existing branch (local or remote-tracking) to the new worktree. + r = subprocess.run(["git", "-C", main_repo, "worktree", "add", wt, branch], + capture_output=True, text=True, timeout=60) + if r.returncode != 0: + # Branch doesn't exist yet — create it from origin/main. + r2 = subprocess.run( + ["git", "-C", main_repo, "worktree", "add", "-b", branch, wt, "origin/main"], + capture_output=True, text=True, timeout=60, + ) + if r2.returncode != 0: + raise RuntimeError( + f"git worktree add failed for {repo}:{branch}: " + f"{r.stderr.strip()} | {r2.stderr.strip()}" + ) + logger.info(f"Worktree ready: {wt} (branch {branch})") + return wt + + +def remove_worktree(repo: str, branch: str): + """Remove the worktree for (repo, branch) — optional cleanup when a task is done.""" + main_repo = _main_repo(repo) + wt = get_worktree_path(repo, branch) + subprocess.run(["git", "-C", main_repo, "worktree", "remove", "--force", wt], + capture_output=True, timeout=30) + # Prune dangling administrative entries. + subprocess.run(["git", "-C", main_repo, "worktree", "prune"], + capture_output=True, timeout=30) + logger.info(f"Worktree removed: {wt}") diff --git a/src/qg/checks.py b/src/qg/checks.py index fdca16e..7750b9e 100644 --- a/src/qg/checks.py +++ b/src/qg/checks.py @@ -7,12 +7,28 @@ from ..config import settings logger = logging.getLogger("orchestrator.qg") +from ..git_worktree import get_worktree_path, ensure_worktree + + +def _repo_path(repo: str, branch: str | None = None) -> str: + """Resolve the working path to read agent artifacts from. + + ORCH-2 / S-4: artifacts now live in the per-branch worktree. When a branch is + given and its worktree exists on disk, read from there; otherwise fall back to + the shared /repos/ clone (keeps backward-compat for 2-arg callers/tests). + """ + if branch: + wt = get_worktree_path(repo, branch) + if os.path.isdir(wt): + return wt + return os.path.join(settings.repos_dir, repo) + # Shared httpx client config GITEA_HEADERS = {"Authorization": f"token {settings.gitea_token}"} GITEA_BASE = f"{settings.gitea_url}/api/v1" -def check_analysis_complete(repo: str, work_item_id: str) -> tuple[bool, str]: +def check_analysis_complete(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ Check if analysis artifacts exist in the repo branch. Required files: @@ -28,7 +44,7 @@ def check_analysis_complete(repo: str, work_item_id: str) -> tuple[bool, str]: f"docs/work-items/{work_item_id}/04-test-plan.yaml", ] - repo_path = os.path.join(settings.repos_dir, repo) + repo_path = _repo_path(repo, branch) missing = [] for f in required_files: @@ -41,13 +57,13 @@ def check_analysis_complete(repo: str, work_item_id: str) -> tuple[bool, str]: return True, "All analysis artifacts present" -def check_architecture_done(repo: str, work_item_id: str) -> tuple[bool, str]: +def check_architecture_done(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ Check if architecture artifacts exist. Required: docs/work-items//06-adr/ (at least 1 file) OR: docs/work-items//07-infra-requirements.md """ - repo_path = os.path.join(settings.repos_dir, repo) + repo_path = _repo_path(repo, branch) adr_dir = os.path.join(repo_path, f"docs/work-items/{work_item_id}/06-adr") infra_file = os.path.join(repo_path, f"docs/work-items/{work_item_id}/07-infra-requirements.md") @@ -119,12 +135,12 @@ def check_review_approved(repo: str, pr_number: int) -> tuple[bool, str]: return False, f"API error: {e}" -def check_tests_passed(repo: str, work_item_id: str) -> tuple[bool, str]: +def check_tests_passed(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ Check if test report exists and contains PASS indicator. File: docs/work-items//13-test-report.md """ - repo_path = os.path.join(settings.repos_dir, repo) + repo_path = _repo_path(repo, branch) report_path = os.path.join(repo_path, f"docs/work-items/{work_item_id}/13-test-report.md") if not os.path.isfile(report_path): @@ -141,7 +157,7 @@ def check_tests_passed(repo: str, work_item_id: str) -> tuple[bool, str]: -def check_analysis_approved(repo: str, work_item_id: str) -> tuple[bool, str]: +def check_analysis_approved(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ Check if analysis is complete AND approved by stakeholder. Requirements: @@ -152,7 +168,7 @@ def check_analysis_approved(repo: str, work_item_id: str) -> tuple[bool, str]: so the approval check verifies file completeness as a safety gate. """ # First check files - files_ok, files_reason = check_analysis_complete(repo, work_item_id) + files_ok, files_reason = check_analysis_complete(repo, work_item_id, branch) if not files_ok: return False, files_reason @@ -187,7 +203,7 @@ def check_analysis_approved(repo: str, work_item_id: str) -> tuple[bool, str]: -def check_reviewer_verdict(repo: str, work_item_id: str) -> tuple[bool, str]: +def check_reviewer_verdict(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ Check reviewer agent verdict from 12-review.md (S-5 fix). @@ -198,7 +214,7 @@ def check_reviewer_verdict(repo: str, work_item_id: str) -> tuple[bool, str]: (False, ...) -> verdict: REQUEST_CHANGES, missing verdict, or no frontmatter """ import yaml - repo_path = os.path.join(settings.repos_dir, repo) + repo_path = _repo_path(repo, branch) review_path = os.path.join(repo_path, f"docs/work-items/{work_item_id}/12-review.md") if not os.path.isfile(review_path): @@ -229,26 +245,15 @@ def check_reviewer_verdict(repo: str, work_item_id: str) -> tuple[bool, str]: def check_tests_local(repo: str, branch: str) -> tuple[bool, str]: """ - S-1 fix: run the project test suite locally in /repos/ and judge by exit - code, instead of depending on Gitea CI (which is not configured -> always false). + S-1 fix: run the project test suite locally and judge by exit code, instead of + depending on Gitea CI (which is not configured -> always false). - Checks out `branch` in the shared /repos checkout and runs `make test`. - NOTE (known limitation): the shared /repos checkout means this is not safe for - concurrent active tasks. git-worktree-per-task is a separate task (S-4). + ORCH-2 / S-4: tests run inside the per-branch worktree (ensure_worktree), so this + is safe for concurrent active tasks — no shared /repos checkout race. """ import subprocess - repo_path = os.path.join(settings.repos_dir, repo) try: - subprocess.run( - ["git", "-C", repo_path, "fetch", "origin"], - capture_output=True, timeout=30, - ) - co = subprocess.run( - ["git", "-C", repo_path, "checkout", branch], - capture_output=True, text=True, timeout=30, - ) - if co.returncode != 0: - return False, f"Cannot checkout branch '{branch}': {co.stderr.strip()[-200:]}" + repo_path = ensure_worktree(repo, branch) r = subprocess.run( ["make", "test"], cwd=repo_path, capture_output=True, text=True, timeout=600, diff --git a/src/webhooks/gitea.py b/src/webhooks/gitea.py index d0d3ef3..78e19d1 100644 --- a/src/webhooks/gitea.py +++ b/src/webhooks/gitea.py @@ -146,7 +146,9 @@ async def handle_ci_status(payload: dict): if not branch: sha = payload.get("sha", "") repo_name = payload.get("repository", {}).get("name", settings.default_repo) - # Try to find task by checking git branch containing this SHA + # Try to find task by checking git branch containing this SHA. + # ORCH-2 / S-4: this is a READ-ONLY query of remote-tracking refs in the main + # clone (no checkout / no mutation), so it is safe to keep on /repos/. try: result = subprocess.run( ["git", "-C", os.path.join(settings.repos_dir, repo_name), diff --git a/src/webhooks/plane.py b/src/webhooks/plane.py index 639b750..07ee6c5 100644 --- a/src/webhooks/plane.py +++ b/src/webhooks/plane.py @@ -304,7 +304,8 @@ async def _try_advance_stage( # Determine args based on QG function if qg_name in ("check_analysis_approved", "check_analysis_complete", "check_architecture_done", "check_tests_passed", "check_reviewer_verdict"): - passed, reason = qg_func(repo, work_item_id) + # ORCH-2 / S-4: pass branch so artifacts are read from the task worktree. + passed, reason = qg_func(repo, work_item_id, branch) elif qg_name in ("check_ci_green", "check_tests_local"): passed, reason = qg_func(repo, branch) elif qg_name == "check_review_approved": @@ -327,8 +328,10 @@ async def _try_advance_stage( else: # No open PR but review file exists — check file-based import os - _review_path = os.path.join(_s.repos_dir, repo, f"docs/work-items/{work_item_id}/12-review.md") - _review_path2 = os.path.join(_s.repos_dir, repo, f"docs/work-items/{work_item_id}/09-review.md") + from ..git_worktree import get_worktree_path as _gwp + _wt = _gwp(repo, branch) if os.path.isdir(_gwp(repo, branch)) else os.path.join(_s.repos_dir, repo) + _review_path = os.path.join(_wt, f"docs/work-items/{work_item_id}/12-review.md") + _review_path2 = os.path.join(_wt, f"docs/work-items/{work_item_id}/09-review.md") if os.path.isfile(_review_path) or os.path.isfile(_review_path2): passed, reason = True, "Review file exists (file-based approval)" else: diff --git a/tests/test_git_worktree.py b/tests/test_git_worktree.py new file mode 100644 index 0000000..66d5d1e --- /dev/null +++ b/tests/test_git_worktree.py @@ -0,0 +1,152 @@ +"""Tests for src/git_worktree (ORCH-2 / S-4): isolated worktree per task/branch. + +Uses real local git repos in tmp (a bare 'origin' + a working main clone) so that +`git fetch origin`, `git worktree add`, branch creation from origin/main, reuse and +removal are all exercised without network access. +""" +import os +import subprocess +import tempfile + +import pytest + +# Env must be set before importing app modules (same convention as the other suites). +_test_db = os.path.join(tempfile.gettempdir(), "test_orchestrator_wt.db") +os.environ["ORCH_DB_PATH"] = _test_db +os.environ["ORCH_REPOS_DIR"] = tempfile.gettempdir() +os.environ["ORCH_GITEA_TOKEN"] = "test-token" +os.environ["ORCH_PLANE_API_TOKEN"] = "test-token" + +from src import git_worktree +from src.git_worktree import ( + _safe, + get_worktree_path, + ensure_worktree, + remove_worktree, +) + + +def _git(cwd, *args): + return subprocess.run(["git", "-C", cwd, *args], capture_output=True, text=True) + + +@pytest.fixture +def repos(tmp_path, monkeypatch): + """Build a bare 'origin' with main + a feature branch, plus a main clone at repos_dir/. + + Returns the repo name. settings.repos_dir / worktrees_dir are pointed at tmp. + """ + repo = "enduro-trails" + repos_dir = tmp_path / "repos" + wt_dir = tmp_path / "repos" / "_wt" + repos_dir.mkdir(parents=True) + + monkeypatch.setattr(git_worktree.settings, "repos_dir", str(repos_dir)) + monkeypatch.setattr(git_worktree.settings, "worktrees_dir", str(wt_dir)) + + # Bare origin + origin = tmp_path / "origin.git" + subprocess.run(["git", "init", "--bare", "-b", "main", str(origin)], capture_output=True) + + # Seed repo + seed = tmp_path / "seed" + seed.mkdir() + _git(str(seed), "init", "-b", "main") + _git(str(seed), "config", "user.email", "t@t") + _git(str(seed), "config", "user.name", "t") + (seed / "README.md").write_text("# seed\n") + _git(str(seed), "add", ".") + _git(str(seed), "commit", "-m", "init") + _git(str(seed), "remote", "add", "origin", str(origin)) + _git(str(seed), "push", "origin", "main") + # An existing feature branch on origin + _git(str(seed), "checkout", "-b", "feature/existing") + (seed / "f.txt").write_text("feature\n") + _git(str(seed), "add", ".") + _git(str(seed), "commit", "-m", "feat") + _git(str(seed), "push", "origin", "feature/existing") + + # Main clone at repos_dir/ + main_clone = repos_dir / repo + subprocess.run(["git", "clone", str(origin), str(main_clone)], capture_output=True) + _git(str(main_clone), "config", "user.email", "t@t") + _git(str(main_clone), "config", "user.name", "t") + return repo + + +# --------------------------------------------------------------------------- +# _safe / get_worktree_path +# --------------------------------------------------------------------------- +class TestSafeAndPath: + def test_safe_replaces_slashes_and_specials(self): + assert _safe("feature/ET-001-x") == "feature_ET-001-x" + assert _safe("a b/c:d") == "a_b_c_d" + assert _safe("keep.dots-and_underscores") == "keep.dots-and_underscores" + + def test_get_worktree_path(self, monkeypatch): + monkeypatch.setattr(git_worktree.settings, "worktrees_dir", "/repos/_wt") + assert get_worktree_path("repo", "feature/x") == "/repos/_wt/repo/feature_x" + + +# --------------------------------------------------------------------------- +# ensure_worktree +# --------------------------------------------------------------------------- +class TestEnsureWorktree: + def test_missing_main_repo_raises(self, tmp_path, monkeypatch): + monkeypatch.setattr(git_worktree.settings, "repos_dir", str(tmp_path / "nope")) + monkeypatch.setattr(git_worktree.settings, "worktrees_dir", str(tmp_path / "_wt")) + with pytest.raises(FileNotFoundError): + ensure_worktree("enduro-trails", "main") + + def test_creates_worktree_for_existing_branch(self, repos): + wt = ensure_worktree(repos, "feature/existing") + assert os.path.isdir(wt) + assert wt == get_worktree_path(repos, "feature/existing") + # On the right branch + cur = _git(wt, "branch", "--show-current").stdout.strip() + assert cur == "feature/existing" + # Feature file from that branch is present (proves correct checkout) + assert os.path.isfile(os.path.join(wt, "f.txt")) + + def test_creates_new_branch_from_origin_main(self, repos): + wt = ensure_worktree(repos, "feature/brand-new") + assert os.path.isdir(wt) + cur = _git(wt, "branch", "--show-current").stdout.strip() + assert cur == "feature/brand-new" + # Based on main -> README present, no feature file + assert os.path.isfile(os.path.join(wt, "README.md")) + assert not os.path.isfile(os.path.join(wt, "f.txt")) + + def test_reuse_returns_same_path(self, repos): + wt1 = ensure_worktree(repos, "feature/existing") + wt2 = ensure_worktree(repos, "feature/existing") + assert wt1 == wt2 + assert os.path.isdir(wt2) + + def test_two_branches_are_isolated(self, repos): + a = ensure_worktree(repos, "feature/wt-A") + b = ensure_worktree(repos, "feature/wt-B") + assert a != b + ba = _git(a, "branch", "--show-current").stdout.strip() + bb = _git(b, "branch", "--show-current").stdout.strip() + assert ba == "feature/wt-A" + assert bb == "feature/wt-B" + # Writing in A must not affect B + with open(os.path.join(a, "only-a.txt"), "w") as f: + f.write("a") + assert not os.path.isfile(os.path.join(b, "only-a.txt")) + + +# --------------------------------------------------------------------------- +# remove_worktree +# --------------------------------------------------------------------------- +class TestRemoveWorktree: + def test_remove_deletes_worktree_dir(self, repos): + wt = ensure_worktree(repos, "feature/to-remove") + assert os.path.isdir(wt) + remove_worktree(repos, "feature/to-remove") + assert not os.path.isdir(wt) + + def test_remove_nonexistent_is_noop(self, repos): + # Should not raise even if the worktree was never created. + remove_worktree(repos, "feature/never-made") diff --git a/tests/test_launcher.py b/tests/test_launcher.py index f9cd376..8f05dda 100644 --- a/tests/test_launcher.py +++ b/tests/test_launcher.py @@ -26,24 +26,34 @@ from src.qg.checks import check_reviewer_verdict # B-1: _write_task_file # --------------------------------------------------------------------------- class TestWriteTaskFile: - def test_writes_to_repos_volume_path(self, tmp_path, monkeypatch): - """Task file is written to //, content matches.""" - monkeypatch.setattr("src.agents.launcher.settings.repos_dir", str(tmp_path)) - repo_dir = tmp_path / "enduro-trails" - repo_dir.mkdir() + """B-1 fix preserved + ORCH-2/S-4: task file now lands in the per-branch worktree. + + _write_task_file(repo, branch, task_file, content) writes to + /// with a plain open() (no docker). + """ + + def _wt_dir(self, tmp_path, repo, branch): + from src.git_worktree import _safe + d = tmp_path / "_wt" / repo / _safe(branch) + d.mkdir(parents=True) + return d + + def test_writes_to_worktree_path(self, tmp_path, monkeypatch): + """Task file is written to the worktree path, content matches (B-1 + S-4).""" + monkeypatch.setattr("src.git_worktree.settings.worktrees_dir", str(tmp_path / "_wt")) + wt = self._wt_dir(tmp_path, "enduro-trails", "feature/ET-001-x") launcher = AgentLauncher() - launcher._write_task_file("enduro-trails", ".task-dev.md", "hello-content") + launcher._write_task_file("enduro-trails", "feature/ET-001-x", ".task-dev.md", "hello-content") - written = repo_dir / ".task-dev.md" + written = wt / ".task-dev.md" assert written.is_file() assert written.read_text() == "hello-content" def test_does_not_use_docker(self, tmp_path, monkeypatch): """No subprocess/docker call: if subprocess.run were used it would error here.""" - monkeypatch.setattr("src.agents.launcher.settings.repos_dir", str(tmp_path)) - repo_dir = tmp_path / "enduro-trails" - repo_dir.mkdir() + monkeypatch.setattr("src.git_worktree.settings.worktrees_dir", str(tmp_path / "_wt")) + self._wt_dir(tmp_path, "enduro-trails", "main") called = {"run": False} @@ -54,17 +64,17 @@ class TestWriteTaskFile: monkeypatch.setattr("src.agents.launcher.subprocess.run", _fail_run) launcher = AgentLauncher() - launcher._write_task_file("enduro-trails", ".task.md", "x") + launcher._write_task_file("enduro-trails", "main", ".task.md", "x") assert called["run"] is False def test_raises_on_write_failure(self, tmp_path, monkeypatch): - """If the target dir does not exist, raise RuntimeError (do not fail silently).""" - monkeypatch.setattr("src.agents.launcher.settings.repos_dir", str(tmp_path)) - # repo dir intentionally NOT created -> open() raises OSError + """If the target worktree dir does not exist, raise RuntimeError (no silent fail).""" + monkeypatch.setattr("src.git_worktree.settings.worktrees_dir", str(tmp_path / "_wt")) + # worktree dir intentionally NOT created -> open() raises OSError launcher = AgentLauncher() with pytest.raises(RuntimeError): - launcher._write_task_file("nonexistent-repo", ".task.md", "x") + launcher._write_task_file("nonexistent-repo", "main", ".task.md", "x") # ---------------------------------------------------------------------------