diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index 3bcc708..1b04cb3 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -229,6 +229,62 @@ Legacy environment-only setups keep working unchanged until migrated. Each runbook names the **profile role** it runs under, the steps, and a safe prompt. Confirm the active profile first (`gitea_get_profile` / `gitea_whoami`). +## Work Selection Rule for LLMs + +Before starting any issue or PR work, acquire or verify a work lease. Do not +begin coding, reviewing, fixing, branching, committing, pushing, commenting, +or creating a PR until you prove the target is not already being worked. + +Required checks: + +1. List open PRs. +2. Search for PRs linked to the target issue. +3. Search local and remote branches for the issue number. +4. Search registered worktrees for the issue branch. +5. Check dirty worktrees. +6. Check active leases or recent handoffs. +7. Check whether the issue was already completed by a merged PR. + +If another active LLM/session owns the lease, stop. Allowed responses: +continue as the lease owner; review the existing PR if reviewer capability +allows; produce a handoff; request takeover after lease expiry; stop with +"work already claimed." + +Never create a parallel branch or PR for the same issue unless the old branch +is proven abandoned and the takeover is recorded. + +Gitea-Tools lease gates: `gitea_lock_issue` (fail-closed before author +mutations), `status:in-progress`, and claim comments. Full portable wording: +[`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md). + +## Global LLM Worktree Rule + +The main project checkout is a stable control checkout. It must stay on the +configured stable branch: `master`, `main`, or `dev`. + +All LLM task work must happen inside the project's `branches/` directory. + +Before any mutation, prove: + +1. current project root +2. current working directory +3. current branch +4. stable branch for the main checkout +5. session-owned worktree path under `branches/` + +If `cwd` is not inside `branches/`, stop. Do not edit, create, delete, format, +test-write, commit, merge, rebase, checkout task branches, resolve conflicts, +or run cleanup. + +There are no exceptions for small fixes, docs, tests, cleanup, PR review fixes, +conflict resolution, or emergencies. + +The main checkout may only be used for read-only inspection, fetching, +stable-branch update after merged PRs, creating `branches/` worktrees, or +explicit control-checkout repair. + +Portable wording: [`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md). + ## Branch worktree isolation All LLM implementation and review work happens in an isolated branch worktree diff --git a/docs/wiki/Operator-Guide.md b/docs/wiki/Operator-Guide.md index 35095ff..9052c47 100644 --- a/docs/wiki/Operator-Guide.md +++ b/docs/wiki/Operator-Guide.md @@ -13,6 +13,7 @@ Handbook for LLM operators and human developers using the Gitea-Tools MCP server 3. **One unit of work per session** — Implement one claimed issue *or* review/merge one PR; do not mix author and reviewer mutations in one session. 4. **No self-review / no self-merge** — The authenticated Gitea user must not approve or merge a PR they authored. 5. **Follow the gates** — Prompts express intent; MCP tools enforce safety. Never bypass gates via prompt instructions. +6. **Global LLM Worktree Rule** — Main checkout stays on `master`/`main`/`dev`; all mutations happen under `branches/`. Prove project root, `cwd`, branch, stable main-checkout branch, and session worktree path before editing. No exceptions. ## Supported Gitea instances diff --git a/docs/wiki/Workflow.md b/docs/wiki/Workflow.md index 7722426..2514dda 100644 --- a/docs/wiki/Workflow.md +++ b/docs/wiki/Workflow.md @@ -14,6 +14,12 @@ ## Step 2: Implement issues (author profile) +0. Work Selection Rule — verify a work lease before any mutations (open PRs, + issue-linked PRs, branches, worktrees, dirty worktrees, active leases/ + handoffs, merged-PR completion). Stop if another session owns the lease. +0b. Global LLM Worktree Rule — main checkout on `master`/`main`/`dev` only; + mutate only from a `branches/` worktree after proving root, cwd, branch, + stable main-checkout branch, and session worktree path (no exceptions). 1. `gitea_resolve_task_capability` for the author task. 2. `gitea_lock_issue` before implementation mutations. 3. Claim with `gitea_mark_issue` / `status:in-progress` label. diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 1c5c186..7a2c781 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -3460,11 +3460,40 @@ _GUIDE_RULES = { "Conflicting or stale PR state (e.g. prior said merged but live open, or head/updated mismatch) is a hard blocker: report it explicitly and stop until live state is unambiguous.", "After merge, re-list PRs and re-view the PR (plus verify master contains the merge) before claiming completion.", ], + "work_selection": ( + "Before any issue or PR work, acquire or verify a work lease. Do not " + "code, review, branch, commit, push, comment, or open a PR until you " + "prove the target is not already being worked. Required checks: list " + "open PRs; search PRs linked to the issue; search local/remote " + "branches for the issue number; search worktrees for the issue " + "branch; check dirty worktrees; check active leases or recent " + "handoffs; check whether a merged PR already completed the issue. If " + "another session owns the lease, stop (continue as owner, review the " + "existing PR, hand off, request takeover after expiry, or report " + "'work already claimed'). Never create a parallel branch/PR unless " + "the old branch is proven abandoned and takeover is recorded. " + "gitea_lock_issue is the fail-closed author lease gate."), + "global_worktree": ( + "Main checkout is a stable control checkout on master/main/dev only. " + "All LLM task work happens under branches/. Before any mutation, prove " + "project root, cwd, branch, main-checkout stable branch, and " + "session-owned branches/ worktree path. If cwd is not under " + "branches/, stop — no edit/create/delete/format/test-write/commit/" + "merge/rebase/checkout/cleanup, with no exceptions (docs, tests, " + "small fixes, review fixes, conflicts, emergencies). Main checkout: " + "read-only inspect, fetch, create worktrees, post-merge stable " + "update, explicit repair only."), } _COMMON_WORKFLOWS = [ "task routing: resolve task capability via gitea_resolve_task_capability " "to check required permission/role kind and identify the safe next action.", + "work selection: acquire or verify a work lease before any issue/PR work " + "(open PRs, linked PRs, branches, worktrees, dirty worktrees, leases, " + "merged completion); stop if another session owns the lease.", + "global worktree: main checkout on master/main/dev only; mutate only " + "from a branches/ worktree after proving root, cwd, branch, stable " + "main-checkout branch, and session worktree path.", "issue authoring: verify identity, create/claim the issue, keep scope " "explicit (remote/org/repo).", "implementation: claim issue, branch from fresh master, implement only " diff --git a/mcp_server.py b/mcp_server.py index 4288ef1..dbe5f1b 100644 --- a/mcp_server.py +++ b/mcp_server.py @@ -6,26 +6,24 @@ Runs over stdio. All tools authenticate via macOS keychain (git credential fill) import os import sys +from role_session_router import ( + python_bytes_have_conflict_markers, + skip_python_scan_walk_root, +) + + # Startup health check: scan all python files in the Gitea-Tools directory for unresolved conflict markers. def check_conflict_markers(): dir_path = os.path.dirname(os.path.abspath(__file__)) - # Construct conflict patterns dynamically so the loader does not match itself - conflict_patterns = [ - b"<" * 7 + b" ", - b"=" * 7 + b"\n", - b"=" * 7 + b"\r\n", - b">" * 7 + b" " - ] for root, dirs, files in os.walk(dir_path): - if any(p in root for p in ("venv", ".git", ".pytest_cache", "branches")): + if skip_python_scan_walk_root(dir_path, root): continue for file in files: if file.endswith(".py"): file_path = os.path.join(root, file) try: with open(file_path, "rb") as f: - content = f.read() - if any(pattern in content for pattern in conflict_patterns): + if python_bytes_have_conflict_markers(f.read()): rel_path = os.path.relpath(file_path, dir_path) print( f"infra_stop: Unresolved merge conflict detected in {rel_path}. " diff --git a/role_session_router.py b/role_session_router.py index d0422a0..742a961 100644 --- a/role_session_router.py +++ b/role_session_router.py @@ -14,6 +14,45 @@ ROUTE_TO_REVIEWER = "route_to_reviewer_session" ROUTE_AMBIGUOUS = "ambiguous_task_stop" ROUTE_INFRA_STOP = "infra_stop" +_CONFLICT_HEAD = b"<" * 7 + b" " +_CONFLICT_TAIL = b">" * 7 + b" " +_CONFLICT_SEPARATOR = b"=" * 7 + + +def python_bytes_have_conflict_markers(content: bytes) -> bool: + """Return True when *content* contains git merge-conflict marker lines.""" + for line in content.splitlines(): + stripped = line.rstrip(b"\r\n") + if stripped.startswith(_CONFLICT_HEAD): + return True + if stripped.startswith(_CONFLICT_TAIL): + return True + if stripped == _CONFLICT_SEPARATOR: + return True + return False + + +def skip_python_scan_walk_root(project_root: str, walk_root: str) -> bool: + """Skip venv/git/cache and sibling worktrees under orchestration checkout. + + When *project_root* is itself a worktree inside ``branches/``, still scan + that tree — do not treat the ``branches`` path segment as a skip signal. + """ + rel = os.path.relpath(walk_root, project_root) + if rel == ".": + return False + head = rel.split(os.sep, 1)[0] + if head in ("venv", ".git", ".pytest_cache"): + return True + if head == "branches": + nested = os.path.join(project_root, "branches") + if os.path.isdir(nested) and ( + walk_root == nested or walk_root.startswith(nested + os.sep) + ): + return True + return False + + REVIEWER_TASKS = frozenset({ "review_pr", "merge_pr", @@ -259,6 +298,25 @@ def check_author_mutation_after_reviewer_stop(mutation_task: str) -> tuple[bool, return True, [] +def first_conflict_marker_path(project_root: str | None = None) -> str | None: + """Return the first .py path containing a git conflict marker, or None.""" + root_dir = project_root or os.path.dirname(os.path.abspath(__file__)) + for root, dirs, files in os.walk(root_dir): + if skip_python_scan_walk_root(root_dir, root): + continue + for file in files: + if not file.endswith(".py"): + continue + file_path = os.path.join(root, file) + try: + with open(file_path, "rb") as f: + if python_bytes_have_conflict_markers(f.read()): + return file_path + except OSError: + pass + return None + + def check_mid_merge() -> bool: """Return True if the repository is mid-merge, mid-rebase, or has conflict markers.""" project_root = os.path.dirname(os.path.abspath(__file__)) @@ -269,24 +327,4 @@ def check_mid_merge() -> bool: or os.path.exists(os.path.join(git_dir, "rebase-apply"))): return True - # Scan python files for conflict markers - conflict_patterns = [ - b"<" * 7 + b" ", - b"=" * 7 + b"\n", - b"=" * 7 + b"\r\n", - b">" * 7 + b" " - ] - for root, dirs, files in os.walk(project_root): - if any(p in root for p in ("venv", ".git", ".pytest_cache", "branches")): - continue - for file in files: - if file.endswith(".py"): - file_path = os.path.join(root, file) - try: - with open(file_path, "rb") as f: - content = f.read() - if any(pattern in content for pattern in conflict_patterns): - return True - except Exception: - pass - return False \ No newline at end of file + return first_conflict_marker_path(project_root) is not None \ No newline at end of file diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index d8ad9f6..fcefe8f 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -53,10 +53,73 @@ Additional issue-first rules: owner decision.** Do not create a new repository or a new tracker unless explicitly approved by the owner. +## Work Selection Rule for LLMs + +Before starting any issue or PR work, acquire or verify a work lease. + +Do not begin coding, reviewing, fixing, branching, committing, pushing, +commenting, or creating a PR until you prove the target is not already being +worked. + +Required checks: + +1. List open PRs. +2. Search for PRs linked to the target issue. +3. Search local and remote branches for the issue number. +4. Search registered worktrees for the issue branch. +5. Check dirty worktrees. +6. Check active leases or recent handoffs. +7. Check whether the issue was already completed by a merged PR. + +If another active LLM/session owns the lease, stop. + +Allowed responses: + +- continue as the lease owner, +- review the existing PR if reviewer capability allows, +- produce a handoff, +- request takeover after lease expiry, +- stop with "work already claimed." + +Never create a parallel branch or PR for the same issue unless the old branch +is proven abandoned and the takeover is recorded. + +For Gitea-Tools: `gitea_lock_issue` is the fail-closed lease gate before author +mutations; `status:in-progress` and claim comments are supporting lease signals. +Use `review_proofs.classify_issue_for_selection` when reporting fresh issue +selection (#188). + +## Global LLM Worktree Rule + +The main project checkout is a stable control checkout. It must stay on the +configured stable branch: `master`, `main`, or `dev`. + +All LLM task work must happen inside the project's `branches/` directory. + +Before any mutation, prove: + +1. current project root +2. current working directory +3. current branch +4. stable branch for the main checkout +5. session-owned worktree path under `branches/` + +If `cwd` is not inside `branches/`, stop. Do not edit, create, delete, format, +test-write, commit, merge, rebase, checkout task branches, resolve conflicts, +or run cleanup. + +There are no exceptions for small fixes, docs, tests, cleanup, PR review fixes, +conflict resolution, or emergencies. + +The main checkout may only be used for read-only inspection, fetching, +stable-branch update after merged PRs, creating `branches/` worktrees, or +explicit control-checkout repair. + ## B. Isolated worktree rule -**Never implement or review in the main checkout.** The main checkout is for -orchestration and status only (issue creation, `git status`, creating worktrees). +**Never implement or review in the main checkout** (Global LLM Worktree Rule). +The main checkout is for orchestration and status only (issue creation, +`git status`, creating worktrees) and must remain on the stable branch. - Each issue gets its own branch worktree under an ignored `branches/` directory. - Review work uses a **separate** review worktree, never the author's folder. @@ -142,7 +205,12 @@ Worktree folder = branch with `/` replaced by `-` ## E. Start-work workflow -1. Verify the orchestration checkout (right repo, clean tree). +0. Acquire or verify a work lease (Work Selection Rule) — complete all seven + checks before any claim, branch, or PR work. +0b. Global LLM Worktree Rule — prove project root, `cwd`, branch, main-checkout + stable branch, and session-owned `branches/` worktree path. If `cwd` is not + under `branches/`, stop before any mutation (no exceptions). +1. Verify the orchestration checkout (right repo, clean tree, on stable branch). 2. Fetch/prune: `git fetch --prune`. 3. Confirm local `master` equals remote `master` (`git rev-list --left-right --count /master...master` → `0 0`). 4. Create/claim the issue (§A). diff --git a/skills/llm-project-workflow/templates/start-issue.md b/skills/llm-project-workflow/templates/start-issue.md index 54933fc..c4b348c 100644 --- a/skills/llm-project-workflow/templates/start-issue.md +++ b/skills/llm-project-workflow/templates/start-issue.md @@ -13,6 +13,21 @@ Rules (llm-project-workflow): - Do not self-review or self-merge. Steps: +0. Work Selection Rule — before any claim, branch, or file edits, acquire or + verify a work lease. Required checks: list open PRs; search PRs linked to + the target issue; search local/remote branches for the issue number; search + registered worktrees for the issue branch; check dirty worktrees; check + active leases or recent handoffs; check whether a merged PR already + completed the issue. If another session owns the lease, stop (continue only + as lease owner, review the existing PR, hand off, request takeover after + expiry, or report "work already claimed"). Never open a parallel branch/PR + unless the old branch is proven abandoned and takeover is recorded. +0b. Global LLM Worktree Rule — before any mutation, prove and state: project + root; cwd; current branch; stable branch for the main checkout (master/main/dev); + session-owned worktree path under branches/. If cwd is not inside branches/, + STOP (no exceptions — not for docs, tests, small fixes, review fixes, conflicts, + or cleanup). Main checkout is control-only: read-only inspect, fetch, create + worktrees, stable-branch update after merge, explicit repair. 1. Identity Checklist: Before claiming work, verify and state: - Required identity/profile for this task: author (allowed to push branches / create PRs) - Current authenticated identity (from whoami): diff --git a/tests/test_health.py b/tests/test_health.py index 0473e72..c177ddd 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -5,6 +5,7 @@ import unittest from unittest.mock import patch import role_session_router +from role_session_router import python_bytes_have_conflict_markers from mcp_server import gitea_route_task_session, gitea_resolve_task_capability _HEALTH_SUBPROCESS_TIMEOUT_SEC = 30 @@ -80,6 +81,20 @@ class TestMCPHealth(unittest.TestCase): with self.assertRaises(unittest.SkipTest): _health_test_python() + def test_conflict_marker_helper_ignores_decorative_equals_border(self): + sample = b'banner = """\n===========================================\n"""\n' + self.assertFalse(python_bytes_have_conflict_markers(sample)) + + def test_conflict_marker_helper_detects_real_markers(self): + sample = ( + b"<" * 7 + b" HEAD\n" + b"print('hello')\n" + b"=" * 7 + b"\n" + b"print('world')\n" + b">" * 7 + b" main\n" + ) + self.assertTrue(python_bytes_have_conflict_markers(sample)) + def test_startup_conflict_detection(self): # Create a Python file with conflict markers constructed dynamically with open(self.temp_file, "w") as f: diff --git a/tests/test_operator_guide.py b/tests/test_operator_guide.py index 80c07f4..602160a 100644 --- a/tests/test_operator_guide.py +++ b/tests/test_operator_guide.py @@ -132,7 +132,8 @@ class TestControlPlaneGuide(GuideTestBase): rules = g["rules"] for key in ("hard_stops", "fail_closed", "head_sha_pinning", "merge_confirmation", "redaction", "separation", - "profile_switching", "identity_verification"): + "profile_switching", "identity_verification", + "work_selection", "global_worktree"): self.assertIn(key, rules) self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"])) self.assertTrue(rules["hard_stops"]) diff --git a/tests/test_review_proofs.py b/tests/test_review_proofs.py index 3965880..92bdfc0 100644 --- a/tests/test_review_proofs.py +++ b/tests/test_review_proofs.py @@ -1051,6 +1051,10 @@ class TestControllerHandoff(unittest.TestCase): self.assertIn("## Controller Handoff", skill) self.assertIn("assess_controller_handoff", skill) self.assertIn("issue #182", skill) + self.assertIn("## Work Selection Rule for LLMs", skill) + self.assertIn("work already claimed", skill) + self.assertIn("## Global LLM Worktree Rule", skill) + self.assertIn("branches/", skill) def test_handoff_rejects_none_workspace_mutations_when_local_edits_exist(self): # 1. Workspace mutations: none is rejected when local_edits is True diff --git a/tests/test_role_session_router.py b/tests/test_role_session_router.py index a2cdffb..25b21c1 100644 --- a/tests/test_role_session_router.py +++ b/tests/test_role_session_router.py @@ -204,5 +204,49 @@ class TestRoleSessionRouter(unittest.TestCase): self.assertTrue(complete["complete"]) +class TestCheckMidMerge(unittest.TestCase): + def test_skip_scan_walk_root_skips_sibling_worktrees_only(self): + worktree_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + main_root = os.path.dirname(os.path.dirname(worktree_root)) + self.assertFalse( + role_session_router.skip_python_scan_walk_root( + main_root, main_root + ) + ) + self.assertTrue( + role_session_router.skip_python_scan_walk_root( + main_root, os.path.join(main_root, "branches", "fix-issue-1") + ) + ) + self.assertFalse( + role_session_router.skip_python_scan_walk_root( + worktree_root, worktree_root + ) + ) + self.assertFalse( + role_session_router.skip_python_scan_walk_root( + worktree_root, os.path.join(worktree_root, "tests") + ) + ) + + def test_decorative_equals_banner_is_not_mid_merge(self): + self.assertFalse(role_session_router.check_mid_merge()) + + def test_python_bytes_have_conflict_markers_rejects_decorative_equals(self): + banner = b"===========================================\n" + self.assertFalse(role_session_router.python_bytes_have_conflict_markers(banner)) + + def test_python_bytes_have_conflict_markers_detects_real_markers(self): + self.assertTrue( + role_session_router.python_bytes_have_conflict_markers(b"<<<<<<< HEAD\n") + ) + self.assertTrue( + role_session_router.python_bytes_have_conflict_markers(b"=======\n") + ) + self.assertTrue( + role_session_router.python_bytes_have_conflict_markers(b">>>>>>> topic\n") + ) + + if __name__ == "__main__": unittest.main() \ No newline at end of file