diff --git a/.gitignore b/.gitignore index e6fe104..9fd03b9 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,7 @@ gitea-mcp*.json .vscode/ graphify-out/ branches/ +# Throwaway agent commit-encoding helpers (#261) — never commit. +/_encode_*.py +/_emit_*.py +/_inline_*.py diff --git a/agent_temp_artifacts.py b/agent_temp_artifacts.py new file mode 100644 index 0000000..5020521 --- /dev/null +++ b/agent_temp_artifacts.py @@ -0,0 +1,26 @@ +"""Detect throwaway agent helper scripts left in the repo root (#261).""" + +from __future__ import annotations + +import fnmatch + +AGENT_TEMP_BASENAME_PATTERNS = ( + "_encode_*.py", + "_emit_*.py", + "_inline_*.py", +) + + +def find_agent_temp_artifacts_from_porcelain(porcelain: str) -> list[str]: + """Return untracked repo-root helper paths matching agent temp patterns.""" + found: list[str] = [] + for line in (porcelain or "").splitlines(): + if not line.startswith("??"): + continue + path = line[3:].strip() + if not path or "/" in path or "\\" in path: + continue + basename = path.split("/")[-1] + if any(fnmatch.fnmatch(basename, pat) for pat in AGENT_TEMP_BASENAME_PATTERNS): + found.append(path) + return sorted(found) \ No newline at end of file diff --git a/docs/llm-workflow-runbooks.md b/docs/llm-workflow-runbooks.md index 1b04cb3..5c427f1 100644 --- a/docs/llm-workflow-runbooks.md +++ b/docs/llm-workflow-runbooks.md @@ -285,6 +285,33 @@ explicit control-checkout repair. Portable wording: [`skills/llm-project-workflow/SKILL.md`](../skills/llm-project-workflow/SKILL.md). +## Shell Spawn Hard-Stop Rule + +Symptom: a shell tool call returns `exit_code: -1` with empty stdout/stderr. +That is an executor spawn failure, not a command failure — the command never +ran, and retrying the identical call cannot succeed. + +Required behavior (fail closed, issue #258): + +1. **Probe once.** On the first spawn failure, run one trivial probe + (`echo ok` or `pwd`). If the probe also returns `exit_code: -1`, mark + shell unavailable for the session. +2. **Hard-stop at two.** After two consecutive spawn failures, stop all + further shell tool use for the session; never retry the same failing + spawn. A hundred retries produce a hundred identical failures (session + `019f382e`: 100+ tool calls stalled on a trivial encode-and-commit task). +3. **Emit a recovery report.** The report must direct the operator to: + - restart the session, + - kill hung background terminals (a hung test runner holding the + executor is a known contributor), + - prefer MCP-native paths for remaining mutations (for example + `gitea_commit_files` under `gitea.repo.commit`) instead of shell. +4. **No improvised fallbacks.** Shell unavailability never authorizes + WebFetch/browser/manual-encoding workarounds (see #260). No shell means + stop-and-report. + +Doc-contract tests: `tests/test_shell_spawn_hard_stop_docs.py`. + ## Branch worktree isolation All LLM implementation and review work happens in an isolated branch worktree @@ -316,6 +343,30 @@ may edit another issue's branch folder unless explicitly assigned to that issue. No LLM may clean another issue's branch folder unless the PR is merged or closed and cleanup is explicitly part of the task. +## Agent temp artifact cleanup (#261) + +Failed or aborted MCP commit attempts sometimes leave throwaway helper scripts in +the **repository root**. These are not part of any issue scope and pollute +`git status`, which can break `gitea_lock_issue` and preflight checks. + +**Patterns (repo root only, untracked):** + +- `_encode_*.py` — base64 payload encoders +- `_emit_*.py` — commit payload emitters +- `_inline_*.py` — inline encoding helpers + +**Required cleanup (after MCP commit completes or aborts):** + +1. Delete any matching files at the repo root (`rm ./_encode_*.py` etc.). +2. Confirm `git status` is clean on the orchestration checkout before + `gitea_lock_issue`. +3. Prefer native `gitea_commit_files` / gated commit paths — do not leave shell + encoding fallbacks behind. + +Root-level matches are listed in `.gitignore` so they never get committed. +`gitea_get_runtime_context` and `gitea_lock_issue` surface **warnings** (not +hard blocks) when these artifacts are still present. + Implementation work and review work must use separate branch folders. For example, an implementation branch might live under `branches/fix-issue-123-example`, while a review branch for the resulting PR @@ -370,6 +421,36 @@ git branch -d fix/issue-123-example All three helpers accept `--dry-run` to print the exact commands/paths without touching anything. +### MCP-native commit path (#260) + +When the active author profile allows **`gitea.repo.commit`** and +**`gitea_commit_files`** is visible in the client, that is the **only** approved +path for committing files to the tracked repository. Do not improvise alternate +encoding or transport when MCP commit is available. + +**Required before commit:** + +1. Call `gitea_resolve_task_capability` for `commit_files` or + `gitea_commit_files` and confirm `allowed_in_current_session` is true. +2. Use `gitea_commit_files` with file payloads prepared in the author worktree. +3. Stage only issue-scoped paths; never commit throwaway `_encode_*` / + `_emit_*` / `_inline_*` helpers. + +**Explicitly forbidden workarounds** when MCP commit is reachable: + +- `WebFetch` / HTTP calls to external decode sites (for example httpbin base64 + endpoints) +- Playwright or other browser automation to bypass MCP +- Manual LLM-generated base64 pasted into ad-hoc scripts +- Delegating commit authority to a subagent while the main session has + `gitea.repo.commit` on an author profile + +**If shell encoding is unavailable** (spawn failure, hung terminal) **and** MCP +commit cannot run: **stop** with a recovery report. Mention restarting the +session, clearing hung background terminals, switching to MCP-native commit, and +the agent temp artifact cleanup checklist. Do **not** retry shell encoding in a +loop and do **not** substitute WebFetch/Playwright/manual base64. + ### Create an issue / child issues - **Profile:** issue-manager or author (any profile allowed to create issues). diff --git a/docs/safety-model.md b/docs/safety-model.md index 1f80b51..31c740a 100644 --- a/docs/safety-model.md +++ b/docs/safety-model.md @@ -28,3 +28,21 @@ Any mutating action (e.g., Gitea issue creation from GlitchTip, or Jenkins build No default profile carries trigger capability (#152 / mcp-control-plane #56). - **GlitchTip to Gitea issue filing** is a library-only orchestrator in mcp-control-plane (not on `glitchtip-mcp`). See #153 / mcp-control-plane #57. + +## 6. Agent Commit Path (no improvised fallbacks) + +When an author execution profile allows **`gitea.repo.commit`** and the +**`gitea_commit_files`** tool is visible, agents must use that MCP path for +repository commits. Fail closed instead of improvising alternate transports. + +Forbidden when MCP commit is available: + +- WebFetch or other HTTP calls to external base64/decode services +- Playwright or browser automation used to work around MCP commit +- Manual LLM-generated base64 embedded in throwaway scripts as the primary + commit transport + +If shell helpers are unavailable and MCP commit cannot run, stop with a recovery +report (restart session, clear hung terminals, use MCP-native commit). See +[`llm-workflow-runbooks.md`](llm-workflow-runbooks.md) § MCP-native commit path +(#260) and agent temp artifact cleanup (#261). diff --git a/gitea_mcp_server.py b/gitea_mcp_server.py index 021b609..aecd212 100644 --- a/gitea_mcp_server.py +++ b/gitea_mcp_server.py @@ -171,6 +171,9 @@ _preflight_whoami_violation_files: list[str] = [] _preflight_capability_violation_files: list[str] = [] _preflight_reviewer_violation_files: list[str] = [] +ACTIVE_WORKTREE_ENV = "GITEA_ACTIVE_WORKTREE" +AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE" + def _preflight_in_test_mode() -> bool: return "pytest" in sys.modules or "unittest" in sys.modules @@ -184,19 +187,47 @@ def _ensure_process_start_porcelain() -> str: return _process_start_porcelain -def _get_workspace_porcelain() -> str: +def _resolve_preflight_workspace_path(worktree_path: str | None = None) -> str: + """Resolve the workspace root inspected by pre-flight guards.""" + path = (worktree_path or "").strip() + if not path: + path = (os.environ.get(ACTIVE_WORKTREE_ENV) or "").strip() + if not path: + path = (os.environ.get(AUTHOR_WORKTREE_ENV) or "").strip() + if not path: + path = PROJECT_ROOT + return os.path.realpath(os.path.abspath(path)) + + +def _get_git_root(path: str) -> str | None: + try: + res = subprocess.run( + ["git", "-C", path, "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + except Exception: + return None + if res.returncode != 0: + return None + return (res.stdout or "").strip() or None + + +def _get_workspace_porcelain(worktree_path: str | None = None) -> str: """Return tracked-workspace porcelain for pre-flight attribution.""" if os.environ.get("GITEA_TEST_FORCE_DIRTY"): return " M __gitea_test_force_dirty__.py\n" override = os.environ.get("GITEA_TEST_PORCELAIN") if override is not None: return override + workspace = _resolve_preflight_workspace_path(worktree_path) try: res = subprocess.run( ["git", "status", "--porcelain"], capture_output=True, text=True, - cwd=PROJECT_ROOT, + cwd=workspace, ) return res.stdout or "" except Exception: @@ -234,7 +265,35 @@ def _format_preflight_files(files: list[str]) -> str: return ", ".join(files) -def assess_preflight_status() -> dict: +def _preflight_workspace_details(worktree_path: str | None, dirty_files: list[str]) -> dict: + workspace = _resolve_preflight_workspace_path(worktree_path) + inspected_root = _get_git_root(workspace) + control_root = os.path.realpath(PROJECT_ROOT) + active_root = os.path.realpath(inspected_root or workspace) + if active_root == control_root: + dirty_scope = "control checkout" + else: + dirty_scope = "active task workspace" + return { + "mcp_server_process_root": control_root, + "active_task_workspace_root": active_root, + "inspected_git_root": inspected_root, + "dirty_files": list(dirty_files), + "dirty_scope": dirty_scope, + } + + +def _format_preflight_workspace_details(details: dict) -> str: + return ( + f"MCP server process root: {details.get('mcp_server_process_root')}; " + f"active task workspace root: {details.get('active_task_workspace_root')}; " + f"inspected git root: {details.get('inspected_git_root')}; " + f"dirty files: {_format_preflight_files(details.get('dirty_files') or [])}; " + f"dirty scope: {details.get('dirty_scope')}" + ) + + +def assess_preflight_status(worktree_path: str | None = None) -> dict: """Non-throwing pre-flight readiness for runtime-context alignment (#252).""" reasons: list[str] = [] if not _preflight_whoami_called: @@ -245,6 +304,25 @@ def assess_preflight_status() -> dict: reasons.append( "Task capability (gitea_resolve_task_capability) has not been resolved" ) + workspace_details = None + if worktree_path: + dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(worktree_path))) + workspace_details = _preflight_workspace_details(worktree_path, dirty_files) + if dirty_files: + reasons.append( + "Active task workspace has tracked file edits before mutation " + f"({_format_preflight_workspace_details(workspace_details)})" + ) + return { + "preflight_ready": not reasons, + "preflight_block_reasons": reasons, + "preflight_whoami_verified": _preflight_whoami_called, + "preflight_capability_resolved": _preflight_capability_called, + "preflight_whoami_violation_files": [], + "preflight_capability_violation_files": [], + "preflight_reviewer_violation_files": [], + "preflight_workspace": workspace_details, + } if _preflight_whoami_violation: reasons.append( "Workspace file edits occurred before gitea_whoami verification " @@ -260,14 +338,25 @@ def assess_preflight_status() -> dict: "Reviewer profile modified tracked workspace files after capability resolution " f"(offending files: {_format_preflight_files(_preflight_reviewer_violation_files)})" ) + warnings: list[str] = [] + agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain( + _get_workspace_porcelain() + ) + if agent_artifacts: + warnings.append( + "Agent temp artifacts detected at repo root (delete before mutations): " + f"{_format_preflight_files(agent_artifacts)}" + ) return { "preflight_ready": not reasons, "preflight_block_reasons": reasons, + "preflight_warnings": warnings, "preflight_whoami_verified": _preflight_whoami_called, "preflight_capability_resolved": _preflight_capability_called, "preflight_whoami_violation_files": list(_preflight_whoami_violation_files), "preflight_capability_violation_files": list(_preflight_capability_violation_files), "preflight_reviewer_violation_files": list(_preflight_reviewer_violation_files), + "preflight_workspace": _preflight_workspace_details(None, []), } @@ -308,7 +397,7 @@ def record_preflight_check(type_name: str, resolved_role: str | None = None): _preflight_resolved_role = resolved_role -def verify_preflight_purity(remote: str | None = None): +def verify_preflight_purity(remote: str | None = None, worktree_path: str | None = None): """Verify that identity and capability were verified prior to session edits.""" global _preflight_reviewer_violation_files @@ -328,6 +417,17 @@ def verify_preflight_purity(remote: str | None = None): "Pre-flight order violation: Task capability (gitea_resolve_task_capability) has not been resolved (fail closed)" ) + if worktree_path: + dirty_files = sorted(_parse_porcelain_entries(_get_workspace_porcelain(worktree_path))) + if dirty_files: + details = _preflight_workspace_details(worktree_path, dirty_files) + raise RuntimeError( + "Pre-flight order violation: Active task workspace has tracked " + "file edits before mutation (fail closed). " + f"{_format_preflight_workspace_details(details)}" + ) + return + if _preflight_whoami_violation: raise RuntimeError( "Pre-flight order violation: Workspace file edits occurred before " @@ -373,6 +473,7 @@ import role_session_router # noqa: E402 import role_namespace_gate # noqa: E402 import task_capability_map # noqa: E402 import review_proofs # noqa: E402 +import agent_temp_artifacts import issue_lock_worktree # noqa: E402 import merged_cleanup_reconcile # noqa: E402 @@ -729,6 +830,8 @@ def gitea_create_issue( Returns: dict with 'number' of the created issue ('url' only with the reveal opt-in). """ + h, o, r = _resolve(remote, host, org, repo) + auth = _auth(h) ok, block_reasons = role_session_router.check_author_mutation_after_reviewer_stop( "create_issue" ) @@ -750,8 +853,6 @@ def gitea_create_issue( if blocked: return blocked verify_preflight_purity(remote) - h, o, r = _resolve(remote, host, org, repo) - auth = _auth(h) base = repo_api_url(h, o, r) open_issues = api_get_all(f"{base}/issues?state=open&type=issues", auth) closed_issues = api_get_all( @@ -819,14 +920,23 @@ def gitea_lock_issue( f"Branch name '{branch_name}' must contain locked issue pattern '{expected_pattern}' (fail closed)" ) + blocked = _profile_permission_block( + task_capability_map.required_permission("lock_issue")) + if blocked: + return blocked + resolved_worktree = issue_lock_worktree.resolve_author_worktree_path( worktree_path, PROJECT_ROOT ) git_state = issue_lock_worktree.read_worktree_git_state(resolved_worktree) + verify_preflight_purity(remote, worktree_path=resolved_worktree) lock_assessment = issue_lock_worktree.assess_issue_lock_worktree( worktree_path=resolved_worktree, current_branch=git_state.get("current_branch"), porcelain_status=git_state.get("porcelain_status") or "", + base_equivalent=git_state.get("base_equivalent"), + inspected_git_root=git_state.get("inspected_git_root"), + base_branch=git_state.get("base_branch"), ) if lock_assessment["block"]: raise RuntimeError( @@ -878,7 +988,10 @@ def gitea_lock_issue( except Exception as e: raise RuntimeError(f"Could not write issue lock file: {e}") - return { + agent_artifacts = agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain( + git_state.get("porcelain_status") or "" + ) + result = { "success": True, "message": ( f"Successfully locked issue #{issue_number} to branch '{branch_name}' " @@ -888,6 +1001,12 @@ def gitea_lock_issue( "branch_name": branch_name, "worktree_path": resolved_worktree, } + if agent_artifacts: + result["warnings"] = [ + "Agent temp artifacts at repo root (delete before implementation): " + + ", ".join(agent_artifacts) + ] + return result @mcp.tool() @@ -2276,6 +2395,106 @@ def gitea_get_file( } +def _prepare_commit_payload_files(files: list[dict]) -> tuple[list[dict], list[dict]]: + import base64 + import json + import os + + processed_files = [] + source_proofs = [] + + lock_data = {} + if os.path.exists(ISSUE_LOCK_FILE): + try: + with open(ISSUE_LOCK_FILE, "r", encoding="utf-8") as f: + lock_data = json.load(f) + except Exception: + pass + + locked_worktree = lock_data.get("worktree_path") + if locked_worktree: + locked_worktree = os.path.realpath(locked_worktree) + + for f in files: + f_copy = dict(f) + path = f_copy.get("path", "") + + content_keys = [k for k in ["content", "content_plain", "workspace_path", "local_path"] if k in f_copy] + if len(content_keys) > 1: + raise ValueError( + f"Multiple content sources specified for file '{path}'; provide exactly one of " + f"'content', 'content_plain', 'workspace_path', or 'local_path' (fail closed)" + ) + + source = "none" + if "content_plain" in f_copy: + content_plain = f_copy.pop("content_plain") + if content_plain is not None: + content_bytes = content_plain.encode("utf-8") + f_copy["content"] = base64.b64encode(content_bytes).decode("utf-8") + source = "inline_plain" + elif "workspace_path" in f_copy: + workspace_path = f_copy.pop("workspace_path") + if not locked_worktree: + raise RuntimeError( + f"Issue lock is missing or does not define a worktree path. " + f"Cannot resolve workspace_path '{workspace_path}' (fail closed)." + ) + target_path = os.path.realpath(os.path.abspath(os.path.join(locked_worktree, workspace_path))) + + is_inside = target_path == locked_worktree or target_path.startswith(os.path.join(locked_worktree, "")) + is_tmp = target_path.startswith(os.path.realpath("/tmp") + os.sep) or target_path.startswith("/tmp" + os.sep) + + if not (is_inside or is_tmp): + raise ValueError( + f"workspace_path '{workspace_path}' resolves to '{target_path}' which falls outside " + f"of locked worktree '{locked_worktree}' (fail closed)" + ) + + if not os.path.exists(target_path): + raise FileNotFoundError(f"File not found: {target_path} (fail closed)") + + with open(target_path, "rb") as fh: + file_bytes = fh.read() + f_copy["content"] = base64.b64encode(file_bytes).decode("utf-8") + source = "workspace_path" + elif "local_path" in f_copy: + local_path = f_copy.pop("local_path") + if not locked_worktree: + raise RuntimeError( + f"Issue lock is missing or does not define a worktree path. " + f"Cannot resolve local_path '{local_path}' (fail closed)." + ) + target_path = os.path.realpath(os.path.abspath(local_path)) + + is_inside = target_path == locked_worktree or target_path.startswith(os.path.join(locked_worktree, "")) + is_tmp = target_path.startswith(os.path.realpath("/tmp") + os.sep) or target_path.startswith("/tmp" + os.sep) + + if not (is_inside or is_tmp): + raise ValueError( + f"local_path '{local_path}' resolves to '{target_path}' which falls outside " + f"of locked worktree '{locked_worktree}' (fail closed)" + ) + + if not os.path.exists(target_path): + raise FileNotFoundError(f"File not found: {target_path} (fail closed)") + + with open(target_path, "rb") as fh: + file_bytes = fh.read() + f_copy["content"] = base64.b64encode(file_bytes).decode("utf-8") + source = "local_path" + elif "content" in f_copy: + source = "inline_base64" + + processed_files.append(f_copy) + source_proofs.append({ + "path": path, + "source": source + }) + + return processed_files, source_proofs + + @mcp.tool() def gitea_commit_files( files: list[dict], @@ -2290,7 +2509,7 @@ def gitea_commit_files( """Commit changes to multiple files in a Gitea repository in a single atomic commit. Args: - files: List of file operations. Each file dict must contain 'operation' ('create', 'update', 'delete', 'rename'), 'path', and 'content' (base64 encoded for create/update), and optionally 'sha' (required for update/delete) or 'from_path' (for rename). + files: List of file operations. Each file dict must contain 'operation' ('create', 'update', 'delete', 'rename'), 'path', and one of content payload sources: 'content' (base64), 'content_plain', 'workspace_path', or 'local_path'. message: The commit message. branch: Optional existing branch to start/commit from. new_branch: Optional new branch name to create for this commit. @@ -2302,13 +2521,38 @@ def gitea_commit_files( Returns: dict with success status and commit/branch information. """ + ok, block_reasons = role_session_router.check_author_mutation_after_reviewer_stop( + "commit_files" + ) + if not ok: + return { + "success": False, + "performed": False, + "commit": "", + "branch": "", + "reasons": block_reasons, + } + blocked = _namespace_mutation_block( + "commit_files", commit="", branch="", remote=remote + ) + if blocked: + return blocked + blocked = _profile_permission_block( + task_capability_map.required_permission("commit_files"), + commit="", branch="", + ) + if blocked: + return blocked + verify_preflight_purity(remote) + processed_files, source_proofs = _prepare_commit_payload_files(files) + h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) url = f"{repo_api_url(h, o, r)}/contents" payload = { - "files": files, + "files": processed_files, "message": message, } if branch is not None: @@ -2319,13 +2563,14 @@ def gitea_commit_files( with _audited("commit_files", host=h, remote=remote, org=o, repo=r, target_branch=(new_branch or branch), request_metadata={"message": message, - "paths": [f.get("path") for f in files], - "operations": [f.get("operation") for f in files]}): + "paths": [f.get("path") for f in processed_files], + "operations": [f.get("operation") for f in processed_files]}): data = api_request("POST", url, auth, payload) return { "success": True, "commit": data.get("commit", {}).get("sha", ""), "branch": data.get("branch", {}).get("name", ""), + "content_source_proof": source_proofs, } @@ -3619,6 +3864,29 @@ _GUIDE_RULES = { "small fixes, review fixes, conflicts, emergencies). Main checkout: " "read-only inspect, fetch, create worktrees, post-merge stable " "update, explicit repair only."), + "shell_spawn_hard_stop": ( + "A shell result of exit_code: -1 with empty stdout/stderr is an " + "executor spawn failure, not a command failure. Probe once (echo/pwd); " + "if the probe fails, mark shell unavailable for the session. After " + "two consecutive spawn failures, hard-stop all shell use — never " + "retry the same failing spawn — and emit a recovery report: restart " + "the session, kill hung background terminals, prefer MCP-native " + "paths (e.g. gitea_commit_files) for remaining mutations. Shell " + "unavailability never authorizes WebFetch/browser/manual-encoding " + "fallbacks (#258)."), + "subagent_delegation": ( + "Deterministic write workflows (issue claim, branch creation, code " + "edits, commits, PR creation, review, merge, cleanup) run inline in " + "the parent session — subagents are blocked for them unless " + "explicitly allowed with a recorded justification. An authorized " + "write subagent must inherit the full gate context: issue lock, " + "branch/worktree path, identity/profile, allowed tool class, " + "command deny list, validation ledger requirement, and final report " + "schema. Subagent output is accepted only with the same proof " + "fields as the parent workflow; read-only delegation (search, " + "inventory, summarize) needs no explicit authorization. Enforced " + "fail closed via subagent_gate.assess_subagent_delegation and " + "validate_subagent_report (#266)."), } _COMMON_WORKFLOWS = [ @@ -4327,6 +4595,7 @@ def _build_runtime_task_capabilities( def gitea_get_runtime_context( remote: str = "dadeschools", host: str | None = None, + worktree_path: str | None = None, ) -> dict: """Read-only: explicit visibility into active profile, configuration model, and eligibility. @@ -4414,7 +4683,7 @@ def gitea_get_runtime_context( allowed, forbidden, config ) - preflight = assess_preflight_status() + preflight = assess_preflight_status(worktree_path) if not preflight["preflight_ready"]: safe_next_action = ( "Complete pre-flight verification before mutating: call gitea_whoami, then " @@ -4438,6 +4707,7 @@ def gitea_get_runtime_context( "safe_next_action": safe_next_action, "preflight_ready": preflight["preflight_ready"], "preflight_block_reasons": preflight["preflight_block_reasons"], + "preflight_workspace": preflight.get("preflight_workspace"), "session_capabilities": session_capabilities, } @@ -4689,6 +4959,7 @@ def gitea_mark_issue( host: str | None = None, org: str | None = None, repo: str | None = None, + worktree_path: str | None = None, ) -> dict: """Claim or release an issue via the status:in-progress label. @@ -4701,6 +4972,7 @@ def gitea_mark_issue( host: Override the Gitea host. org: Override the owner/organization. repo: Override the repository name. + worktree_path: Active task worktree to inspect for pre-flight purity. Returns: dict with 'success' boolean and 'message'. @@ -4712,7 +4984,7 @@ def gitea_mark_issue( task_capability_map.required_permission("mark_issue")) if blocked: return blocked - verify_preflight_purity(remote) + verify_preflight_purity(remote, worktree_path=worktree_path) h, o, r = _resolve(remote, host, org, repo) auth = _auth(h) base = repo_api_url(h, o, r) diff --git a/issue_lock_worktree.py b/issue_lock_worktree.py index 219c6b9..5d9dc4a 100644 --- a/issue_lock_worktree.py +++ b/issue_lock_worktree.py @@ -14,7 +14,7 @@ import subprocess from reviewer_worktree import parse_dirty_tracked_files AUTHOR_WORKTREE_ENV = "GITEA_AUTHOR_WORKTREE" -BASE_BRANCHES = frozenset({"master", "main"}) +BASE_BRANCHES = frozenset({"master", "main", "dev"}) def resolve_author_worktree_path( @@ -50,9 +50,28 @@ def read_worktree_git_state(worktree_path: str) -> dict: text=True, check=False, ) + root_res = subprocess.run( + ["git", "-C", path, "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + head_res = subprocess.run( + ["git", "-C", path, "rev-parse", "HEAD"], + capture_output=True, + text=True, + check=False, + ) + head_sha = (head_res.stdout or "").strip() if head_res.returncode == 0 else None + base_branch, base_sha = _find_matching_base_ref(path, head_sha) return { "current_branch": current_branch, "porcelain_status": status_res.stdout or "", + "inspected_git_root": (root_res.stdout or "").strip() if root_res.returncode == 0 else None, + "head_sha": head_sha, + "base_branch": base_branch, + "base_sha": base_sha, + "base_equivalent": bool(head_sha and base_sha and head_sha == base_sha), } @@ -61,6 +80,9 @@ def assess_issue_lock_worktree( worktree_path: str, current_branch: str | None, porcelain_status: str, + base_equivalent: bool | None = None, + inspected_git_root: str | None = None, + base_branch: str | None = None, base_branches: frozenset[str] | None = None, ) -> dict: """Fail closed when lock preconditions are not met on the declared worktree.""" @@ -77,21 +99,39 @@ def assess_issue_lock_worktree( if dirty_files: reasons.append( "tracked file edits exist before issue lock; " - "lock must precede implementation work" - ) - if not branch: - reasons.append( - "current branch unknown (detached HEAD?); issue lock must be taken " - f"from base branch ({_base_list(bases)})" - ) - elif branch not in bases: - reasons.append( - f"issue lock must be taken from base branch ({_base_list(bases)}), " - f"not '{branch}'" + f"lock must precede implementation work in '{path}' " + f"(dirty files: {', '.join(dirty_files)})" ) + if base_equivalent is False: + reasons.append( + "issue lock worktree must be base-equivalent to one of " + f"{_base_list(bases)} before implementation work; inspected " + f"branch '{branch or '(detached)'}' at '{path}'" + ) + elif base_equivalent is None: + if not branch: + reasons.append( + "current branch unknown (detached HEAD?); issue lock base-equivalence " + f"to {_base_list(bases)} could not be proven" + ) + elif branch not in bases: + reasons.append( + "issue lock worktree base-equivalence could not be proven; " + f"branch '{branch}' is not {_base_list(bases)}" + ) + proven = not reasons - return _assessment(proven, reasons, path, branch or None, dirty_files) + return _assessment( + proven, + reasons, + path, + branch or None, + dirty_files, + inspected_git_root=inspected_git_root, + base_branch=base_branch, + base_equivalent=base_equivalent, + ) def format_issue_lock_worktree_error(assessment: dict) -> str: @@ -145,12 +185,39 @@ def _assessment( worktree_path: str, current_branch: str | None, dirty_files: list[str], + *, + inspected_git_root: str | None = None, + base_branch: str | None = None, + base_equivalent: bool | None = None, ) -> dict: return { "proven": proven, "block": not proven, "reasons": reasons, "worktree_path": worktree_path or None, + "inspected_git_root": inspected_git_root, "current_branch": current_branch, "dirty_files": dirty_files, - } \ No newline at end of file + "base_branch": base_branch, + "base_equivalent": base_equivalent, + } + + +def _find_matching_base_ref(path: str, head_sha: str | None) -> tuple[str | None, str | None]: + """Return the stable branch ref whose commit matches HEAD, if any.""" + if not head_sha: + return None, None + candidates: list[str] = [] + for branch in sorted(BASE_BRANCHES): + candidates.extend((f"origin/{branch}", branch)) + for ref in candidates: + res = subprocess.run( + ["git", "-C", path, "rev-parse", "--verify", ref], + capture_output=True, + text=True, + check=False, + ) + sha = (res.stdout or "").strip() + if res.returncode == 0 and sha == head_sha: + return ref, sha + return None, None diff --git a/review_proofs.py b/review_proofs.py index c539a0b..be71582 100644 --- a/review_proofs.py +++ b/review_proofs.py @@ -704,6 +704,270 @@ def assess_live_state_recheck(recheck): return {"proven": proven, "block": not proven, "reasons": reasons} +_REQUEST_CHANGES_OVERRIDE_REASONS = frozenset({ + "incorrect_blocker", + "wrong_validation_environment", + "resolved_externally", +}) + + +def _blocking_request_changes_reviews(feedback: dict) -> list[dict]: + """Return undismissed REQUEST_CHANGES reviews (latest verdict per reviewer).""" + latest_by_reviewer: dict[str, dict] = {} + for entry in feedback.get("reviews") or []: + verdict = (entry.get("verdict") or "").upper() + reviewer = (entry.get("reviewer") or "").strip() + if verdict not in ("APPROVED", "REQUEST_CHANGES") or not reviewer: + continue + latest_by_reviewer[reviewer] = entry + return [ + entry for entry in latest_by_reviewer.values() + if entry.get("verdict") == "REQUEST_CHANGES" and not entry.get("dismissed") + ] + + +def _primary_blocking_review(blockers: list[dict]) -> dict | None: + if not blockers: + return None + return sorted( + blockers, + key=lambda entry: (entry.get("submitted_at") or "", entry.get("reviewer") or ""), + )[-1] + + +def assess_request_changes_approval_proof( + feedback: dict | None, + *, + override_reason: str | None = None, + override_explanation: str | None = None, + report_text: str = "", +) -> dict: + """#326: prove approval is safe when prior REQUEST_CHANGES exists on same head. + + Before approving, workflows must fetch ``gitea_get_pr_review_feedback`` and + pass the result here. When a prior undismissed REQUEST_CHANGES targets the + current head, approval requires explicit override proof; otherwise fail closed. + """ + if not feedback or feedback.get("success") is not True: + return { + "approve_allowed": False, + "block": True, + "reasons": ["PR review feedback missing or unreadable; fail closed"], + "blocking_review": None, + "head_changed_since_blocker": None, + "override_required": None, + "override_proof": None, + } + + blockers = _blocking_request_changes_reviews(feedback) + primary = _primary_blocking_review(blockers) + current_head = (feedback.get("current_head_sha") or "").strip().lower() + + if not blockers: + return { + "approve_allowed": True, + "block": False, + "reasons": [], + "blocking_review": None, + "head_changed_since_blocker": False, + "override_required": False, + "override_proof": None, + } + + blocking_head = (primary.get("reviewed_head_sha") or "").strip().lower() + head_changed = bool( + feedback.get("author_pushed_after_request_changes") + or (blocking_head and current_head and blocking_head != current_head) + ) + + blocker_report = { + "blocking_reviewer": primary.get("reviewer"), + "blocking_review_timestamp": primary.get("submitted_at"), + "blocking_head_sha": primary.get("reviewed_head_sha"), + "current_head_sha": feedback.get("current_head_sha"), + "blocker_text": (primary.get("body") or "").strip(), + "head_changed_since_blocker": head_changed, + } + + if head_changed: + return { + "approve_allowed": True, + "block": False, + "reasons": [], + "blocking_review": blocker_report, + "head_changed_since_blocker": True, + "override_required": False, + "override_proof": None, + } + + reason = (override_reason or "").strip().lower() + explanation = (override_explanation or "").strip() + report_lower = (report_text or "").lower() + blocker_text = blocker_report["blocker_text"] + reasons: list[str] = [] + + if reason not in _REQUEST_CHANGES_OVERRIDE_REASONS: + reasons.append( + "unchanged head after REQUEST_CHANGES requires override_reason " + f"in {sorted(_REQUEST_CHANGES_OVERRIDE_REASONS)}" + ) + if not explanation: + reasons.append( + "unchanged head after REQUEST_CHANGES requires override_explanation" + ) + if blocker_text and blocker_text.lower() not in report_lower: + reasons.append( + "report must include the blocking REQUEST_CHANGES body text" + ) + if reason and reason.replace("_", " ") not in report_lower and reason not in report_lower: + reasons.append( + "report must state the override reason for unchanged-head approval" + ) + + override_proof = { + "override_reason": reason or None, + "override_explanation": explanation or None, + "blocker_text_in_report": bool( + blocker_text and blocker_text.lower() in report_lower + ), + } + approve_allowed = not reasons + return { + "approve_allowed": approve_allowed, + "block": not approve_allowed, + "reasons": reasons, + "blocking_review": blocker_report, + "head_changed_since_blocker": False, + "override_required": True, + "override_proof": override_proof, + } + + +_PERFORMED_FILE_ACTIONS = frozenset({"edited", "created", "wrote", "generated"}) +_FILE_EDITS_FIELD_RE = re.compile( + r"^\s*file edits by reviewer\s*:\s*(.+?)\s*$", + re.I | re.M, +) +_WALKTHROUGH_ARTIFACT_RE = re.compile(r"walkthrough\.md", re.I) + + +def _performed_file_mutations(action_log: list[dict] | None) -> list[dict]: + """Return performed local file mutations, excluding gated rejections.""" + performed: list[dict] = [] + for entry in action_log or []: + if entry.get("gated_rejected") or entry.get("performed") is False: + continue + action = (entry.get("action") or "").strip().lower() + if action not in _PERFORMED_FILE_ACTIONS: + continue + path = (entry.get("path") or "").strip() + if not path: + continue + performed.append({**entry, "action": action, "path": path}) + return performed + + +def assess_mutation_ledger_report( + report_text: str, + *, + action_log: list[dict] | None = None, + final_git_status_reported: bool | None = None, + walkthrough_explicitly_requested: bool = False, +) -> dict: + """#331: verify reviewer final reports match observed file mutations. + + Compares an action log of local file writes/edits against the report's + mutation ledger and ``File edits by reviewer`` field. Gated rejections + (``performed: false`` or ``gated_rejected: true``) are excluded from the + performed mutation set but should still appear in a separate rejected-calls + section when reported. + """ + text = report_text or "" + lower = text.lower() + reasons: list[str] = [] + performed = _performed_file_mutations(action_log) + + field_match = _FILE_EDITS_FIELD_RE.search(text) + file_edits_value = (field_match.group(1).strip() if field_match else None) + claimed_none = bool( + file_edits_value and file_edits_value.lower() == "none" + ) + + if performed and claimed_none: + reasons.append( + "report claims 'File edits by reviewer: none' but action log " + "records performed file mutations" + ) + + unreported: list[str] = [] + for entry in performed: + path = entry["path"] + path_lower = path.lower() + if path_lower not in lower: + unreported.append(path) + reasons.append( + f"performed mutation path '{path}' missing from report " + "mutation ledger" + ) + continue + + if entry.get("outside_repo"): + if "outside repo" not in lower or path_lower not in lower: + reasons.append( + f"outside-repo mutation '{path}' must be reported with " + "'outside repo' label" + ) + elif entry.get("in_repo", True): + tracked = entry.get("tracked") + if tracked is True and "tracked" not in lower: + reasons.append( + f"in-repo tracked mutation '{path}' must state tracked/" + "untracked status in the ledger" + ) + elif tracked is False and "untracked" not in lower: + reasons.append( + f"in-repo untracked mutation '{path}' must state tracked/" + "untracked status in the ledger" + ) + + if ( + _WALKTHROUGH_ARTIFACT_RE.search(path) + and not walkthrough_explicitly_requested + ): + reasons.append( + "walkthrough artifact created without explicit workflow or " + "operator request" + ) + + if entry.get("after_git_status") and final_git_status_reported is not True: + reasons.append( + f"mutation '{path}' occurred after an earlier git status; " + "report must include a final git status" + ) + + rejected = [ + e for e in (action_log or []) + if e.get("gated_rejected") or e.get("performed") is False + ] + rejected_reported = "rejected" in lower or "gated" in lower or "no-op" in lower + if rejected and performed and not rejected_reported: + reasons.append( + "rejected/no-op gated tool calls must be reported separately from " + "performed mutations" + ) + + proven = not reasons + return { + "proven": proven, + "block": not proven, + "reasons": reasons, + "performed_mutations": performed, + "unreported_paths": unreported, + "file_edits_claimed_none": claimed_none, + "rejected_calls": rejected, + } + + def assess_role_boundary(proof=None, *, task_role=None, namespaces_used=None, justification=None): """Assess reviewer/author role separation for blind queue workflows. @@ -2471,3 +2735,84 @@ def build_review_mutation_proof(run_log: list[dict]) -> dict: "missing_fields": [], "reasons": [], } + + +# --------------------------------------------------------------------------- +# Identity disclosure (#305) +# --------------------------------------------------------------------------- + +EMAIL_ADDRESS_RE = re.compile( + r"[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}") + +EMAIL_JUSTIFICATION_MARKERS = ( + "email required", + "email is required", + "email necessary", + "necessary to disambiguate", + "disambiguate identity", + "tool requires the email", + "user explicitly asked", +) + + +def format_identity_summary(username, profile, role=None, remote=None): + """Return the no-email identity line for workflow reports (#305). + + Standard reports identify actors as `` / `` (plus + optional role/remote). Personal email is never part of the summary; if + an email lands in the username slot, only its local part is kept. + """ + name = (username or "").strip() + if "@" in name: + name = name.split("@", 1)[0] + summary = f"{name} / {(profile or '').strip()}" + extras = [str(part).strip() for part in (role, remote) + if part and str(part).strip()] + if extras: + summary += f" ({', '.join(extras)})" + return summary + + +def assess_email_disclosure( + report_text, + *, + justification_markers=EMAIL_JUSTIFICATION_MARKERS, +): + """Flag unnecessary personal-email disclosure in a report (#305). + + Username/profile identity is sufficient for normal workflow reports. + An email address is tolerated only when the report itself explains why + it is necessary (tool proof, explicit request, or disambiguation). + """ + text = report_text or "" + emails = sorted(set(EMAIL_ADDRESS_RE.findall(text))) + if not emails: + return { + "proven": True, + "flagged": False, + "justified": False, + "emails": [], + "reasons": [], + } + lower = text.lower() + justified = any(marker in lower for marker in justification_markers) + if justified: + return { + "proven": True, + "flagged": False, + "justified": True, + "emails": emails, + "reasons": [ + "email disclosure present but justified in the report", + ], + } + return { + "proven": False, + "flagged": True, + "justified": False, + "emails": emails, + "reasons": [ + f"unnecessary personal email disclosure: {email}" + for email in emails + ], + } diff --git a/skills/llm-project-workflow/SKILL.md b/skills/llm-project-workflow/SKILL.md index cfb723a..a47cb6b 100644 --- a/skills/llm-project-workflow/SKILL.md +++ b/skills/llm-project-workflow/SKILL.md @@ -1,65 +1,74 @@ --- name: llm-project-workflow description: >- - Portable, safe operating workflow for LLMs working on any Git/forge project: - issue-first, isolated branch worktrees, no self-review/self-merge, distinct - author/reviewer profiles, cleanup after merge, and fail-closed behavior. - Use at the start of any implementation, review, or merge task on a repo. + Router skill for safe LLM project work: identify task mode, load the matching + canonical workflow file, enforce mode isolation, and emit the correct final + report schema. Use at the start of any implementation, review, merge, + reconciliation, or issue-filing task. --- -# LLM Project Workflow +# LLM Project Workflow Skill -A reusable workflow any LLM can follow to work on any repository safely. Copy -this `skills/llm-project-workflow/` directory into another project unchanged; -adapt only the forge-specific names in [Adapting to a project](#adapting-to-a-project). +This skill is a **router**. Do not perform project work from this file alone. -The core promise: **an LLM never does unsafe or untracked work.** Every change -is tracked by an issue, isolated in its own worktree, reviewed by a different -identity, and cleaned up only after a real merge. +Before any project mutation, identify the task mode and load the matching +workflow file. + +## Workflow modes + +| Task mode | Workflow | Final report schema | +|-----------|----------|---------------------| +| PR review / approval / merge | [`workflows/review-merge-pr.md`](workflows/review-merge-pr.md) | [`schemas/review-merge-final-report.md`](schemas/review-merge-final-report.md) | +| Reconcile already-landed open PRs | [`workflows/reconcile-landed-pr.md`](workflows/reconcile-landed-pr.md) | [`schemas/reconcile-landed-final-report.md`](schemas/reconcile-landed-final-report.md) | +| Create or update Gitea issues | [`workflows/create-issue.md`](workflows/create-issue.md) | [`schemas/create-issue-final-report.md`](schemas/create-issue-final-report.md) | +| Work on an assigned issue / author code | [`workflows/work-issue.md`](workflows/work-issue.md) | [`schemas/work-issue-final-report.md`](schemas/work-issue-final-report.md) | + +## Universal rules + +- Prove identity, active profile, runtime context, and **exact** capability before + mutation. +- A nearby capability does not count. +- Do not self-review or self-merge. +- Do not mix modes in one run. +- If the required workflow cannot be loaded, stop and produce a recovery handoff + only. +- Final report must use the schema for the loaded workflow. +- If a task requires a different mode, stop and produce a handoff for the + correct workflow. + +## Mode isolation + +A run that starts in `review-merge-pr` mode may not create process issues, +implement fixes, or edit source files. + +A run that starts in `reconcile-landed-pr` mode may not approve, request +changes, merge, implement fixes, or create normal issues. + +A run that starts in `create-issue` mode may not review, approve, request +changes, merge, implement fixes, create branches, commit, push, or create PRs. + +A run that starts in `work-issue` mode may not review, approve, request changes, +merge, close PRs, or act as reviewer. + +If the task requires a different mode, stop and produce a handoff for the +correct workflow. --- - ## Definitions - **Merged**: Gitea PR metadata says `merged=true`. -- **Landed**: Equivalent content is present on remote `master`, but PR metadata may not say merged. +- **Landed**: Equivalent content is present on remote `master`, but PR metadata + may not say merged. - **Closed-not-merged**: PR state is closed and `merged=false`. -- **Reconciled**: A human/LLM verified whether closed-not-merged content landed, partially landed, or was lost, and repaired issue/label/tracker state. - -## A. Issue-first rule - -**No repository change without a tracking issue.** This includes creating, -editing, deleting, or `chmod`-ing files; docs; scripts; commits; pushes; and PRs. - -1. Before any change, confirm a tracking issue exists. -2. If none exists, create one first (title + problem + scope + acceptance). -3. Claim it (assign yourself or apply the `status:in-progress` label) and comment - that work is starting, including the planned branch name. -4. **If the issue cannot be created or claimed, stop.** Do not touch files. - -Reading the repo, running read-only status/`git log`, and creating/claiming the -issue itself are allowed from the orchestration checkout without a prior issue. - -Additional issue-first rules: - -- Do not implement code without an issue unless explicitly authorized. -- **Design-only work uses a discussion/RFC issue** — create one or comment on - the existing one. Design debates belong on the issue, where other LLMs - comment directly. Discussion-only tasks must **not** create branches or PRs; - their comments should include recommendations, risks, open questions, and a - Controller Handoff (§K; compact format unless high-risk). -- **If the repo/tracker home for the work is unclear, stop and ask for an - owner decision.** Do not create a new repository or a new tracker unless - explicitly approved by the owner. +- **Reconciled**: Verified whether closed-not-merged or already-landed content + is present on the target branch; issue/label/tracker state repaired. ## 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. +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: @@ -71,612 +80,75 @@ Required checks: 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. +If another active session owns the lease, stop with "work already claimed" or +produce a handoff. 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`. +The main project checkout is a stable control checkout on `master`, `main`, or +`dev`. All LLM task work must happen inside the project's `branches/` directory. -All LLM task work must happen inside the project's `branches/` directory. +If `cwd` is not inside `branches/`, stop before any file edit, test write, +commit, merge, rebase, or cleanup. The main checkout is orchestration-only. -Before any mutation, prove: +## Shell Spawn Hard-Stop Rule -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/` +`exit_code: -1` with empty stdout/stderr means the shell failed to spawn — not a +command failure. After two consecutive spawn failures, hard-stop shell use for +the session and emit a recovery report (#258). -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. +## Isolated worktree naming -There are no exceptions for small fixes, docs, tests, cleanup, PR review fixes, -conflict resolution, or emergencies. +Implementation: `(fix|feat|docs|chore)/issue--` -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. +Review: `review/pr--` -## B. Isolated worktree rule +Worktree folder: branch with `/` replaced by `-` under `branches/`. -**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. +Helpers: `scripts/worktree-start`, `scripts/worktree-review`, +`scripts/worktree-clean`. -- Each issue gets its own branch worktree under an ignored `branches/` directory. -- Review work uses a **separate** review worktree, never the author's folder. -- Dirty work in one branch folder must not block starting another issue. -- No LLM may edit another issue's worktree unless explicitly assigned to it. -- Branch folders are removed only after the PR is merged/closed **and** cleanup - is explicitly part of the task. +## Identity and profile safety -Every implementation branch **must include its issue number** so it is -traceable end to end: **issue → branch → worktree folder → PR → cleanup.** +- Author and reviewer identities must be distinct. +- Never place raw tokens in LLM/MCP config. +- Use `gitea_whoami` and `gitea_resolve_task_capability` before mutating. -Allowed implementation patterns: - -- `fix/issue-123-short-description` -- `feat/issue-123-short-description` -- `docs/issue-123-short-description` -- `chore/issue-123-short-description` - -Review-only branches: - -- `review/pr-456-short-description` - -Use a filesystem-safe folder under `branches/` by replacing slashes with -hyphens, for example `branches/fix-issue-123-short-description`. - -`scripts/worktree-start` **enforces** this: it rejects an implementation branch -that does not match `(fix|feat|docs|chore)/issue--…` (or a -`review/pr--…` branch), unless `--allow-unlinked` is passed. Traceability -is maintained by: - -- the branch name (contains the issue number), -- a claim comment on the issue, e.g. - `Claimed. Branch: fix/issue-123-short-description. Worktree: branches/fix-issue-123-short-description.`, -- the PR body — `Closes #123` or `Fixes #123` when the PR should close the issue - (do NOT use `Implements #123` or `Refs #123` to close, as Gitea will not auto-close), -- cleanup after merge — remove the remote branch, local branch, and the issue - worktree folder, and drop `status:in-progress`. - -For projects using `Gitea-Tools` helpers: - -```bash -scripts/worktree-start fix/issue-123-example # → branches/fix-issue-123-example -scripts/worktree-review fix/issue-123-example # → branches/review-fix-issue-123-example (detached) -scripts/worktree-clean --delete-branch fix/issue-123-example -``` - -Manual equivalent: - -```bash -git fetch --prune -git worktree add -b fix/issue-123-example branches/fix-issue-123-example /master -cd branches/fix-issue-123-example -``` - -`venv/` and similar are not copied into new worktrees — run checks with a known -interpreter path, or create a venv inside the branch folder. - -## C. Identity and profile safety - -- Use canonical execution profiles where available; the profile is the role, not the LLM. A task selects a profile; a profile is not permanently assigned. -- **Author and reviewer identities must be distinct.** -- Never place raw tokens/passwords in an LLM/MCP client config. Reference secrets by keychain id or environment variable name only. Prefer a single canonical config file selected by two env vars, e.g.: - - `GITEA_MCP_CONFIG` — path to the canonical profiles file - - `GITEA_MCP_PROFILE` — the profile to activate -- **Dual-Profile MCP Launcher Pattern (Recommended):** To avoid relaunch bottlenecks and PR-author deadlocks, register multiple instances of the same MCP server in the client's configuration simultaneously (e.g., `gitea-author` and `gitea-reviewer`), each pointing to its respective `GITEA_MCP_PROFILE`. - - Tool calls become namespace-scoped: `mcp__gitea-author__*` and `mcp__gitea-reviewer__*`. - - **Trust Model:** Separate tokens remain separate. Profile gates enforce allowed operations, `whoami` is still checked, and self-review/self-merge prevention remains mandatory. This pattern is for convenience and does not bypass security gates. - - **Deadlock Warning:** Reviewer/merge identities must not be used to create PRs, as this makes the reviewer the PR author in Gitea and blocks independent review. PRs should normally be created by the author/work identity, keeping the reviewer identity available for reviews. - - **Fallback:** If a dual-server launcher is not available in the client, relaunch or restart the client with the correct profile environment variable before claiming work. -- **If the authenticated user equals the PR author, stop** — no self-review, no self-merge. - -## D. Branch naming - -```text -fix/issue-123-short-description -feat/issue-123-short-description -docs/issue-123-short-description -review/pr-456-scope-check -``` - -Worktree folder = branch with `/` replaced by `-` -(`branches/fix-issue-123-short-description`). - -## E. Start-work workflow - -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). -4b. **Issue lock from your scratch clone (#249):** when using - `gitea_lock_issue`, pass `worktree_path` pointing at your own clean - scratch clone (or set `GITEA_AUTHOR_WORKTREE`). The lock gate validates - *that* path — clean tree on `master`/`main`, no tracked edits yet — - not the shared MCP/orchestration checkout. Another session's dirty - feature branch in the shared dev worktree must not block your lock. - Never stash, reset, or checkout files in the shared worktree to satisfy - the gate. Pass the same `worktree_path` to `gitea_create_pr` so the PR - gate matches the lock record. -5. Create the isolated worktree (§B) from latest remote `master`. -6. Implement the narrow scope only — no unrelated refactors or formatting churn. -7. Add/update focused tests when behavior changes. -8. Run the checks (tests, compile/lint, `git diff --check`, secret scan). - Record the branch name and `HEAD` SHA at validation time — the drift - check in step 9 compares against exactly this state. -9. **Branch proof before commit (#177):** prove and state, immediately - before staging/committing (`author_proofs.verify_branch_for_commit`, - `author_proofs.detect_branch_drift`): - - current branch (`git branch --show-current`) equals the intended - feature branch from the issue claim - - current branch is not `master`, `main`, `develop`, `development`, or - `dev` - - branch and `HEAD` have not changed since validation (step 8) — in a - shared checkout another session may switch branches mid-session; - treat that as expected and **stop before committing** when detected - If any check fails, stop and reconcile; do not commit. -10. Commit with an issue-linked message. -11. **Branch proof before push (#177):** prove that the local branch, the - push target branch, and the intended issue branch all match, and that - none of them is a protected branch - (`author_proofs.verify_push_target`). If a commit accidentally landed - on a protected branch, do **not** push: report the accident and the - exact repair steps (`author_proofs.assess_protected_branch_commit`) — - never silently continue after a repair. -12. Push the branch. -13. Open a PR to `master`. The final report must include the branch proofs - from steps 9 and 11 (`author_proofs.build_commit_push_report`). -14. **If you are the author, stop before review/merge.** -15. **Normal issue work must not directly push to `master`.** PR content should be merged through the forge PR merge mechanism. -16. Direct push to `master` is allowed only as a documented recovery exception. If used, the final report must include: - - why the PR merge path could not be used - - exact commits pushed - - PR metadata state - - issue labels/state repaired - - whether the PR is closed-not-merged - - -## F. Review workflow - -1. Use a separate review worktree (`scripts/worktree-review `), detached. -2. Verify your authenticated identity. -3. Verify the PR author — **you must not be the author.** Self-review - contamination must be *evidence-backed* (#173): state the authenticated - reviewer identity, the PR author identity, whether this session - authored/touched the PR branch, and the evidence source for any - "same-session author" claim. If the evidence is missing, report the - status as **unknown** — never declare contamination by assumption — and - choose another PR or stop (`review_proofs.assess_self_review_contamination`). -4. Verify the worktree is clean. -5. **Checkout proof (#173):** before reviewing or validating, prove and - state: the selected PR head SHA from Gitea (pinned), the local checkout - SHA (`git rev-parse HEAD`), that `HEAD ==` the pinned PR head SHA, and - that the diff base is the PR base branch. If `HEAD` does not match the - pinned head, **stop before review/merge** - (`review_proofs.verify_pinned_head_checkout`). -6. **Inventory proof (#173 + repo disambiguation hardening):** a blind queue - review must prove listing completeness before claiming "only PRs found". - Use repo-name disambiguation: - - "Gitea-Tools" / "gitea tool" / "MCP Gitea tool" / "gitea MCP tool" / - "gitea-tools repo" resolve **only** to `Scaled-Tech-Consulting/Gitea-Tools`. - - "mcp-control-plane" resolves only to `Scaled-Tech-Consulting/mcp-control-plane`. - - Ambiguous ("open PRs", no explicit repo, "MCP Gitea tooling") → inventory - **both** configured repos. - Report must state exactly which repo(s) were checked. If only one checked: - "Only was checked. Other configured repos were not checked. This is - not a complete queue inventory." Never let a single-repo zero hide PRs in - the other. - Both configured repos must be reported with state filter, pagination proof, - and open-PR count (`review_proofs.assess_inventory_completeness` and - `resolve_repos_from_user_reference`). Before inventory, reconcile the - operator-supplied PR backlog against the target repo - (`review_proofs.reconcile_queue_target`); never report `trusted_empty` - for one repo while ignoring contradictory supplied PR numbers in another. -7. **Role-boundary proof (#175):** a reviewer queue task must not silently - become author implementation. If no eligible PR exists, stop with the - queue report. Do not claim issues, create branches, commit, push, or open - PRs unless the operator explicitly retasks the run as author work. Mixed - reviewer+author namespace use must be reported with a justification, and - scratch-only notes are not durable evidence unless posted or committed - intentionally (`review_proofs.assess_role_boundary`). -8. Inspect the full diff; confirm scope matches the linked issue; flag unrelated files. -9. Run the tests. Validation reporting must include the exact command and - exact results: pass/fail, counts of tests passed/skipped/failed, any - ignored paths and why they are safe to ignore, and whether the command - differs from the repository's canonical validation command. Only claim a - validation result after the command has completed and its output has - been read (`review_proofs.assess_validation_report`). - During validation, review work is **read-only**: use - `gitea_dry_run_pr_review` to prove submission mechanics — never post live - APPROVE, REQUEST_CHANGES, or review comments to probe tool paths. After - validation completes, call `gitea_mark_final_review_decision`, then submit - exactly one live review via - `gitea_submit_pr_review(..., final_review_decision_ready=True)`. - After submitting, re-read `gitea_get_pr_review_feedback` and confirm the - verdict is visible (`approval_visible` true for APPROVE; PENDING drafts do - not count — #244). Do not merge until a visible APPROVED review exists. - Final reports must list exactly one review mutation - (`review_proofs.assess_review_mutation_final_report`) unless an - operator-approved correction flow was invoked and explained. -10. **Do not merge if checks fail. Do not merge if the reviewer is the author.** -11. **#179 A-bar proofs** (all fail closed when missing — - `review_proofs.assess_capability_evidence`, `assess_sweep_evidence`, - `assess_live_state_recheck`, `assess_role_boundary`): - - Capability claims must cite exact `gitea_resolve_task_capability` - output (or runtime context); a bare "capability checks passed" is - downgraded. - - The secret/provenance sweep must state the exact command/script/ - pattern/named method and the scope scanned. - - Immediately before submitting a review verdict (and again before any - merge), re-read live PR state and prove: still open, live head == - pinned head, base unchanged, no unresolved blocking review state. - - Reviewer runs stay in the reviewer namespace; any author-namespace - call requires an explicit justification in the report. -12. The final report must distinguish (`review_proofs.build_final_report`): - identity eligible; PR author different from reviewer; session - contamination absent (with evidence); validation performed on the pinned - head; capability evidence; sweep verdict; live-state recheck; role - boundary; merge performed; issue status verified. If any proof is - missing, stop or downgrade the result instead of merging confidently. - -## G. Merge / cleanup workflow - -Only an eligible (non-author) reviewer merges. Before merging: always verify -the authenticated identity **and** the PR author; cite exact capability -evidence for merge_pr (#179); respect runtime profile gates; run independent -validation (do not trust the author's reported results); perform the **final -live-state recheck** (#179 — PR still open, live head == pinned head, base -unchanged, no unresolved blocking review state) immediately before the merge -mutation; and merge with a **pinned head SHA** and, where supported, the -**expected changed-file set**, so a moved head or widened diff refuses the -merge. After a real merge: - -1. Confirm remote `master` actually contains the merge commit or expected squashed changes via post-merge file-presence verification (A PR is not done just because `master` moved or is marked "closed". Verify that expected files added/modified in the PR are actually present on `master` using `git pull`, `git log --oneline -- `, or `git merge-base --is-ancestor`; linked issues are closed; `status:in-progress` is removed). -2. Close/release the issue. -3. Whenever an issue is closed, check for `status:in-progress`: remove it, or report why it could not be removed. -4. Do not delete the remote source branch until: PR `merged=true`, or reconciliation confirms content is safely landed, or the issue owner explicitly abandons the work. -5. Remove the local branch. -6. Remove the branch worktree folder (`scripts/worktree-clean --delete-branch `). Branches/worktrees are cleaned only after the above is verified. -7. Fetch/prune. -8. Confirm the main checkout is clean and current (`0 0` vs remote). -9. Final merge/reconciliation reports must include: PR metadata (state, merged flag, merge commit/hash), Git content (remote master hash, expected content present or not), and the exact post-merge verification method used & results. - -Never run cleanup before the merge is confirmed on remote `master`. - -## H. Fail-closed cases - -**Stop and report — take no mutating action — if:** - -- No issue exists and one cannot be created. -- Worktree state is unclear or unexpected. -- Branch/PR state conflicts with the prompt (e.g. prompt says "merged" but it is not). -- A PR is closed but not merged (closed with `merged=false`). In this case: - - stop normal review/merge - - do not delete branches/worktrees - - do not start dependent work - - run reconciliation -- Local `master` is ahead of remote unexpectedly. -- The authenticated user is the PR author (for review/merge). -- Secrets/tokens appear in the diff. -- Tests fail. -- A cleanup step would delete unmerged work. - -When in doubt, stop and surface the discrepancy; do not guess or work around a gate. - -## I. Recovery patterns - -- **Dirty worktree from another issue:** do not touch it. Start your issue in its - own new worktree; unrelated dirty work must not block you. For Gitea-Tools - author flows, lock the issue from your scratch clone (`worktree_path` on - `gitea_lock_issue`) — do not manipulate the shared dev checkout. -- **Local `master` ahead of remote unexpectedly:** do not push `master`. Confirm - the commits are preserved on a feature branch (local + remote) first, then - `git reset --hard /master` to realign. Never discard commits that are - not safely pushed elsewhere. -- **PR closed but not merged (`merged=false`):** do not merge. Run reconciliation: compare PR content to remote `master` and decide: - - **fully landed:** comment that content is present on `master`, remove `status:in-progress`, keep/close issue as appropriate, clean up only after content equivalence is confirmed. - - **partially landed:** do not clean up, reopen issue if needed, create corrective issue/PR for missing pieces. - - **not landed:** reopen issue if needed, reopen PR or create replacement PR, do not clean up source branch/worktree. -- **Branch deleted before merge:** if the commits still exist locally (a branch or - reflog), re-push them and reopen the PR; otherwise recover via - `git fsck --lost-found`. Preserve first, then proceed. -- **Unauthorized/untracked file created:** do not commit it. Leave pre-existing - untracked artifacts (e.g. editor/agent dirs, reports) alone; stage only the - files your issue names (`git add `, never blind `git add -A`). -- **Preserve commits before a reset:** confirm the target commits are reachable - from a branch that is pushed to the remote, then reset. Verify with - `git branch --contains ` and `git log /`. - -## J. Prompt snippets - -Ready-to-copy templates live in [`templates/`](templates/): - -- [`start-issue.md`](templates/start-issue.md) — start a new issue. -- [`review-pr.md`](templates/review-pr.md) — review a PR. -- [`merge-pr.md`](templates/merge-pr.md) — merge a PR (eligible reviewer only). -- [`recover-bad-state.md`](templates/recover-bad-state.md) — recover from bad state. -- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md) — reconcile a closed-not-merged PR. -- [`worktree-cleanup.md`](templates/worktree-cleanup.md) — clean up after merge. -- [`release-tag.md`](templates/release-tag.md) — create a release tag. - -## K. Controller Handoff (required, every task) - -Every LLM task **must end with a `Controller Handoff`** (exact title) — whether the -task was implementation, review, merge, issue triage, documentation, -discussion-only, or blocked planning. It lets a controller LLM understand the -current state immediately, without rereading the conversation. - -The section title must be exactly "Controller Handoff" (or "Controller Handoff Summary" for long form). Reports without it are downgraded (see review_proofs.assess_controller_handoff). - -**The compact format is the default.** It is written for controller-LLM -readability, not as a full human status report. PR bodies still carry the -full review detail — the handoff never replaces PR documentation. - -Compact format (default, canonical field set per issue #182): - -```md ## Controller Handoff -- Task: -- Repo: -- Role: -- Identity: -- Issue/PR: -- Branch/SHA: -- Files changed: -- Validation: -- Mutations: -- Current status: -- Blockers: -- Next: -- Safety: -``` +Every task must end with a section titled exactly `Controller Handoff`. Compact +format canonical field set per issue #182; mode-specific schemas in +`schemas/*-final-report.md` define required fields. Use the final report schema +for the loaded workflow mode — not the legacy compact block alone. +`review_proofs.assess_controller_handoff()` validates presence. -Role-specific fields (append to the compact block): +## Prompt templates -- review/merge tasks: `Selected PR:`, `Reviewer eligibility:`, - `Pinned reviewed head:`, `Review decision:`, `Merge result:`, - `Linked issue status:`, `Cleanup status:` -- issue-filing tasks (#191): `Issue created or updated:`, `Related issues:`; - body must cite exact issue number/title, duplicate-search summary (issues - searched, closest matches, why update rejected / new issue justified), full - 40-char SHAs when citing commits, exact mutation capability per change, and - `Only mutation(s):` when a single mutation was performed - (`review_proofs.assess_issue_filing_final_report`). -- author tasks: `Selected issue:`, `Claim/comment status:`, - `PR number opened:`, `No review/merge:` (explicit confirmation) -- continuation tasks (#188): `Continuation mode:`, `Existing PR:`, - `PR author:`, `Branch:`, `Old PR head:`, `New PR head:`, - `Session authored PR:`, `Why continuation allowed:` — issues with open - PRs are excluded from fresh selection unless operator explicitly requests - continuation (`review_proofs.classify_issue_for_selection`, - `assess_issue_selection_final_report`) -- queue/inventory tasks: `Repositories checked:`, `Open PR counts:`, - `Selected PR or reason none selected:`, `Inventory completeness:` +Ready-to-copy task prompts live in [`templates/`](templates/): -The section title must be exactly `Controller Handoff`. -`review_proofs.assess_controller_handoff()` validates this section; reports -missing it (or missing required fields) are downgraded. The handoff never -replaces the full report — it is the compact continuation summary at the end, -and the full report must still carry exact validation results and mutation -confirmation. - -The `Safety:` line is never omitted; it is usually: - -```text -no self-review; no self-merge; no tags; no secrets; no prod -``` - -Rules (both formats): - -- Never omit the handoff, and never omit the safety confirmations. -- Never bury blockers in earlier text only — they must appear here. -- If you opened a PR, state clearly that review is needed. -- If you reviewed but could not merge, name the exact gate that blocked it. -- If you only commented on a discussion issue, say no code review is needed - but owner/design feedback may be needed. -- If release state was touched, state exactly which tag/commit changed and why. -- If blocked (permissions, missing repo, missing second reviewer identity, - stale dependency, unclear tracker home): stop and report clearly; **never - bypass classifiers, profile gates, missing permissions, or live-consent - requirements**; give the owner concrete options. - -**Use the long format below instead of the compact one only when the task was -high-risk or complex** — i.e. when any of these happened: - -- a merge, tag, or release -- failed validation -- permissions/profile gates blocked work -- secrets or production access were involved -- a complicated owner decision -- multiple repos or cross-issue state -- the owner explicitly asks for the full format - -Long format (high-risk/complex tasks only): - -```md -## Controller Handoff Summary - -### Work performed - -Briefly state what was done. - -### Current state - -Include: -- current repo -- current branch or master commit -- issue number(s) -- PR number(s), if any -- whether work is complete, blocked, ready for review, or discussion-only - -### Files changed - -List files changed, or say `None`. - -### Validation - -List commands run and results, or say `Not applicable — discussion only`. - -### Issues encountered - -List errors, confusing state, permission/profile problems, stale branches, -failing tests, missing labels, or blocked decisions. - -### Review needed? - -Say one of: -- `No review needed — discussion/comment only` -- `Review needed — PR is open` -- `Independent non-author review needed` -- `Owner decision needed` -- `Blocked` - -### Next recommended action - -State exactly what should happen next. - -### Safety confirmations - -Confirm: -- no self-review -- no self-merge -- no release/tag changes unless explicitly requested -- no secrets committed -- no production access used unless explicitly authorized -``` - -### Example blocked handoff - -```md -## Example blocked handoff - -### Work performed - -Audited phase-2 MCP Control Plane planning. Found target repo -`mcp-control-plane` does not exist. Prepared issue pack but did not file it. - -### Current state - -- Repo: `Scaled-Tech-Consulting/Gitea-Tools`, unmodified -- Target repo: `mcp-control-plane`, missing -- Issues: none open in Gitea-Tools -- PRs: none open -- Status: blocked pending owner decision - -### Files changed - -None. - -### Validation - -Tracker/repo audit only. No code validation required. - -### Issues encountered - -Repo creation was denied by permission/classifier because it would be scope -escalation without live consent. - -### Review needed? - -Owner decision needed. - -### Next recommended action - -Owner must choose: -1. create `Scaled-Tech-Consulting/mcp-control-plane` -2. authorize repo creation while present -3. file phase-2 issues in Gitea-Tools instead - -### Safety confirmations - -- no self-review -- no self-merge -- no release/tag changes -- no secrets committed -- no production access used -``` +- [`start-issue.md`](templates/start-issue.md) — author work (loads `work-issue.md`) +- [`review-pr.md`](templates/review-pr.md) — review (loads `review-merge-pr.md`) +- [`merge-pr.md`](templates/merge-pr.md) — merge (loads `review-merge-pr.md`) +- [`recover-bad-state.md`](templates/recover-bad-state.md) +- [`reconcile-closed-not-merged-pr.md`](templates/reconcile-closed-not-merged-pr.md) +- [`worktree-cleanup.md`](templates/worktree-cleanup.md) +- [`release-tag.md`](templates/release-tag.md) ## Adapting to a project -Replace these project-specific names when copying the skill elsewhere: - -| Placeholder | Meaning | Example here | -|-------------|---------|--------------| -| `` | Git remote for the forge | `prgs` | -| default branch | Integration branch | `master` | -| profile env vars | Canonical config + profile selectors | `GITEA_MCP_CONFIG`, `GITEA_MCP_PROFILE` | -| `branches/` | Ignored worktree directory | `branches/` | -| helper scripts | Worktree helpers | `scripts/worktree-start` / `-review` / `-clean` | - -The rules in §A–§K are project-agnostic and should not change. +| Placeholder | Example here | +|-------------|--------------| +| `` | `prgs` | +| default branch | `master` | +| profile env vars | `GITEA_MCP_CONFIG`, `GITEA_MCP_PROFILE` | +| `branches/` | `branches/` | +| helpers | `scripts/worktree-start` / `-review` / `-clean` | ## Versioning And Tagging -Releases follow SemVer: **`vMAJOR.MINOR.PATCH`** (use **`v0.x.y`** while -unstable). Choose the bump by the largest change since the last tag: - -- **PATCH** — bug fixes, docs, tests, wrappers, non-breaking workflow polish. -- **MINOR** — new tools/helpers/config features; backward-compatible behavior. -- **MAJOR** — breaking config/schema/API behavior or a changed MCP contract. - -Tags must: - -- be created **only from `master`** (the exact commit on remote `master`), -- be created **only after the full test suite passes**, -- be **annotated** tags (`git tag -a`), never lightweight, -- include release notes / a changelog summary referencing the merged PRs/issues. - -**Never tag** feature branches, dirty worktrees, unreviewed or self-authored -work, or commits not present on remote `master`. - -Additional tag rules: - -- Do **not** create, move, delete, or push tags unless explicitly instructed. -- Tag only **after** the intended PR is merged, and tag only the **verified - final master merge commit** (never the PR branch head unless the merge - commit is exactly that commit). -- Always **report the tag target commit** in the final report / handoff. - -Release process (see [`templates/release-tag.md`](templates/release-tag.md)): - -1. `git fetch --prune`. -2. Verify local `master` equals remote `master` (`0 0`) and the tree is clean. -3. Run the full test suite; stop on any failure. -4. Inspect merged issues/PRs since the last tag - (`git log --oneline ../master`). -5. Choose the version bump. -6. Create the annotated tag on remote `master` with release notes. -7. Push the tag. -8. Create/update release notes if the forge supports it. - -Where present, `scripts/release-tag` automates this with all gates built in -(SemVer, fetch/prune, on-master, clean tree, local==remote master, HEAD on -remote master, no duplicate tag, tests, annotated-only). Safe by default: no -push without `--push`; `--dry-run` changes nothing; `--skip-tests` must be -explicit and warns. +Releases follow SemVer from remote `master` only, after full test suite passes. +See [`templates/release-tag.md`](templates/release-tag.md) and +`scripts/release-tag`. \ No newline at end of file diff --git a/skills/llm-project-workflow/schemas/create-issue-final-report.md b/skills/llm-project-workflow/schemas/create-issue-final-report.md new file mode 100644 index 0000000..f35c233 --- /dev/null +++ b/skills/llm-project-workflow/schemas/create-issue-final-report.md @@ -0,0 +1,46 @@ +# Create-issue controller handoff schema + +**Task mode:** `create-issue` + +End every create-issue run with a section titled exactly `Controller Handoff`. +Use this canonical field set. Do not omit fields — use `none` or +`not verified in this session` where appropriate. + +Do not use legacy fields: `Workspace mutations`, `Mutations: None` (when +mutations occurred). + +```md +## Controller Handoff + +- Task: +- Repo: +- Role: +- Identity: +- Active profile: +- Runtime context: +- Requested issue task: +- Workflow source: +- Capability proof: +- Duplicate search terms: +- Duplicate search pagination proof: +- Duplicates found: +- Issues created: +- Issues commented: +- Issues edited: +- Issues skipped as duplicates: +- Labels/assignees/milestones changed: +- File edits by issue creator: +- Worktree/index mutations: +- Git ref mutations: +- MCP/Gitea mutations: +- Issue mutations: +- Label/assignment/milestone mutations: +- External-state mutations: +- Read-only diagnostics: +- Blockers: +- Current status: +- Safe next action: +- Safety statement: +``` + +Identity format: `username / profile` (not personal email unless required — #305). diff --git a/skills/llm-project-workflow/schemas/reconcile-landed-final-report.md b/skills/llm-project-workflow/schemas/reconcile-landed-final-report.md new file mode 100644 index 0000000..d7bf77b --- /dev/null +++ b/skills/llm-project-workflow/schemas/reconcile-landed-final-report.md @@ -0,0 +1,53 @@ +# Reconcile-landed controller handoff schema + +**Task mode:** `reconcile-landed-pr` + +End every reconciliation run with a section titled exactly `Controller Handoff`. +Use this canonical field set. Do not omit fields — use `none` or +`not verified in this session` where appropriate. + +Reject stale author/reviewer fields: `PR number opened`, `Pinned reviewed head`, +`Scratch worktree used`, `Workspace mutations`, `Mutations: None` (when mutations +occurred). + +```md +## Controller Handoff + +- Task: +- Repo: +- Role: +- Identity: +- Active profile: +- Runtime context: +- Selected PR: +- PR live state: +- Candidate head SHA: +- Target branch: +- Target branch SHA: +- Ancestor proof: +- Linked issue: +- Linked issue live status: +- Eligibility class: +- Capabilities proven: +- Missing capabilities: +- PR comments posted: +- Issue comments posted: +- PRs closed: +- Issues closed: +- File edits by reconciler: +- Worktree/index mutations: +- Git ref mutations: +- MCP/Gitea mutations: +- Reconciliation mutations: +- External-state mutations: +- Read-only diagnostics: +- Blockers: +- Current status: +- Safe next action: +- Safety statement: +- No review/merge confirmation: +``` + +Identity format: `username / profile` (not personal email unless required — #305). + +`git fetch` belongs under `Git ref mutations`, not read-only diagnostics (#297). \ No newline at end of file diff --git a/skills/llm-project-workflow/schemas/review-merge-final-report.md b/skills/llm-project-workflow/schemas/review-merge-final-report.md new file mode 100644 index 0000000..237d2f2 --- /dev/null +++ b/skills/llm-project-workflow/schemas/review-merge-final-report.md @@ -0,0 +1,82 @@ +# Review-merge controller handoff schema + +**Task mode:** `review-merge-pr` + +End every review/merge run with a section titled exactly `Controller Handoff`. +Use this canonical field set. Do not omit fields — use `none` or +`not verified in this session` where appropriate. + +Do not use legacy fields: `Pinned reviewed head`, `Scratch worktree used`, +`Workspace mutations`, `Mutations: None` (when mutations occurred). + +```md +## Controller Handoff + +- Task: +- Repo: +- Role: +- Identity: +- Active profile: +- Runtime context: +- Selected PR: +- Linked issue: +- Eligibility class: +- Queue ordering policy: +- Inventory pagination proof: +- Earlier PRs skipped: +- Candidate head SHA: +- Reviewed head SHA: +- Target branch: +- Target branch SHA: +- Already-landed gate: +- Author-safety result: +- Prior request-changes state: +- Review worktree used: +- Review worktree path: +- Review worktree inside branches: +- Review worktree HEAD state: +- Review worktree dirty before validation: +- Review worktree dirty after validation: +- Baseline worktree used: +- Baseline worktree path: +- Files reviewed: +- Validation: +- Official validation integrity status: +- Terminal review mutation: +- Review decision: +- Merge preflight: +- Merge result: +- Linked issue status: +- Main checkout branch: +- Main checkout dirty state: +- Main checkout updated: +- File edits by reviewer: +- Worktree/index mutations: +- Git ref mutations: +- MCP/Gitea mutations: +- Review mutations: +- Merge mutations: +- Cleanup mutations: +- External-state mutations: +- Read-only diagnostics: +- Blockers: +- Current status: +- Safe next action: +- Safety statement: +``` + +### Already-landed handoff overrides + +When eligibility class is `ALREADY_LANDED_RECONCILE_REQUIRED`: + +- Reviewed head SHA: `none` +- Review worktree used: `false` +- Review worktree path: `none` +- Review decision: `none` +- Merge result: `none` + +Identity format: `username / profile` (not personal email unless required — #305). + +Narrative final report and controller handoff must agree on eligibility class, +candidate/reviewed head SHA, mutation state, worktree usage, review decision, +terminal review mutation, merge result, and linked issue status. \ No newline at end of file diff --git a/skills/llm-project-workflow/schemas/work-issue-final-report.md b/skills/llm-project-workflow/schemas/work-issue-final-report.md new file mode 100644 index 0000000..d55f8a9 --- /dev/null +++ b/skills/llm-project-workflow/schemas/work-issue-final-report.md @@ -0,0 +1,73 @@ +# Work-issue controller handoff schema + +**Task mode:** `work-issue` + +End every work-issue run with a section titled exactly `Controller Handoff`. +Use this canonical field set. Do not omit fields — use `none` or +`not verified in this session` where appropriate. + +Do not use legacy fields: `Workspace mutations`, `Mutations: None` (when +mutations occurred). + +```md +## Controller Handoff + +- Task: +- Repo: +- Role: +- Identity: +- Active profile: +- Runtime context: +- Selected issue: +- Eligibility class: +- Issue ordering policy: +- Issue inventory pagination proof: +- Earlier issues skipped: +- Duplicate active work proof: +- Claim/lock state: +- Stable branch: +- Stable branch SHA: +- Branch name: +- Worktree path: +- Worktree inside branches: +- Worktree branch/HEAD state: +- Worktree dirty before implementation: +- Files changed: +- Validation: +- Baseline comparison: +- Commit SHA: +- Push result: +- PR number: +- PR URL: +- PR verification: +- Main checkout branch: +- Main checkout dirty state: +- Main checkout used for task work: +- File edits by author: +- Worktree/index mutations: +- Git ref mutations: +- MCP/Gitea mutations: +- Issue mutations: +- Branch mutations: +- Commit mutations: +- Push mutations: +- PR mutations: +- Cleanup mutations: +- External-state mutations: +- Read-only diagnostics: +- Blockers: +- Current status: +- Safe next action: +- Safety statement: +``` + +Identity format: `username / profile` (not personal email unless required — #305). + +Narrative final report and controller handoff must agree on eligibility class, +selected issue, and mutation ledger categories (#319, #320). + +`git fetch` and ref-updating commands belong under `Git ref mutations`, not +`Read-only diagnostics` (#297). + +Forbidden claims without proof (#330): `next eligible issue`, `issue claimed`, +`validation passed`, `PR created`, `worktree clean`, `all gates passed`, etc. \ No newline at end of file diff --git a/skills/llm-project-workflow/templates/merge-pr.md b/skills/llm-project-workflow/templates/merge-pr.md index bee3b75..ae99544 100644 --- a/skills/llm-project-workflow/templates/merge-pr.md +++ b/skills/llm-project-workflow/templates/merge-pr.md @@ -5,6 +5,10 @@ Copy, fill the `<...>` fields, and paste as the task prompt. ```text Task: merge PR # for issue # if it is eligible and checks pass. +Load the canonical workflow first: +`skills/llm-project-workflow/workflows/review-merge-pr.md` (task mode: review-merge-pr). +Final report schema: `schemas/review-merge-final-report.md`. + Rules (llm-project-workflow): - Only an eligible, NON-author reviewer merges. If authenticated user == PR author → STOP. diff --git a/skills/llm-project-workflow/templates/review-pr.md b/skills/llm-project-workflow/templates/review-pr.md index 088243b..391f962 100644 --- a/skills/llm-project-workflow/templates/review-pr.md +++ b/skills/llm-project-workflow/templates/review-pr.md @@ -30,6 +30,10 @@ Repo name disambiguation (Gitea-Tools blind review hardening): commit is not valid corroboration. Author-bound sessions must not present reviewer queue inventory as a reviewer decision. +Load the canonical workflow first: +`skills/llm-project-workflow/workflows/review-merge-pr.md` (task mode: review-merge-pr). +Final report schema: `schemas/review-merge-final-report.md`. + Rules (llm-project-workflow): - Review in a SEPARATE detached review worktree, never the author's folder. - Worktree safety (#233): before checkout, diff, validation, review, or merge, diff --git a/skills/llm-project-workflow/templates/start-issue.md b/skills/llm-project-workflow/templates/start-issue.md index c4b348c..4b8c159 100644 --- a/skills/llm-project-workflow/templates/start-issue.md +++ b/skills/llm-project-workflow/templates/start-issue.md @@ -5,6 +5,10 @@ Copy, fill the `<...>` fields, and paste as the task prompt. ```text Task: implement . +Load canonical workflow: skills/llm-project-workflow/workflows/work-issue.md +Final report schema: skills/llm-project-workflow/schemas/work-issue-final-report.md +Router: skills/llm-project-workflow/SKILL.md (task mode: work-issue) + Rules (llm-project-workflow): - No repo changes without a tracking issue. If none exists, create one first; if it can't be created, stop. diff --git a/skills/llm-project-workflow/workflows/create-issue.md b/skills/llm-project-workflow/workflows/create-issue.md new file mode 100644 index 0000000..00f34f8 --- /dev/null +++ b/skills/llm-project-workflow/workflows/create-issue.md @@ -0,0 +1,666 @@ +--- +task_mode: create-issue +canonical: true +final_report_schema: ../schemas/create-issue-final-report.md +--- + +# Create issue workflow (canonical) + +**Task mode:** `create-issue` + +This file is the canonical issue-creation workflow for Gitea-Tools. Load it +before any issue mutation. Final report schema: +[`schemas/create-issue-final-report.md`](../schemas/create-issue-final-report.md). + +**Default task prompt:** + +> Create or update Gitea issues in this project only if every identity, +> capability, duplicate-search, issue-scope, final-report, mutation-ledger, +> and proof-wording gate passes. + +Do not improvise around the gates. Follow project skills, MCP gates, and +workflow rules exactly. + +This is an issue-creation workflow. It is not a reviewer workflow and not an +implementation workflow. + +--- + +## 0. Load the canonical workflow first + +Before starting issue creation or issue update work, check whether the project provides a canonical create-issue workflow through a project skill, runbook, or MCP helper. + +If available, load it first and report: + +* workflow source +* workflow version, commit, or hash +* whether this prompt conflicts with the loaded workflow + +If the canonical workflow cannot be loaded and the project requires it, stop and produce a recovery handoff only. + +## 1. Mode isolation + +This run is `create-issue` mode only. + +Do not: + +* review PRs +* approve PRs +* request changes +* merge PRs +* close PRs +* close issues unless the user explicitly asks and exact close capability is proven +* implement code +* edit repo files +* create branches +* create commits +* push branches +* create PRs +* run tests unless the canonical workflow explicitly requires validation for issue creation +* perform reviewer-only actions +* perform author/coder-only actions +* perform MCP repair + +If the task requires review, merge, issue implementation, or MCP repair mode, stop and produce a handoff for the correct workflow. + +Do not mix modes in one run. + +## 2. Start with live identity, profile, runtime, and capability checks + +Prove: + +* authenticated identity +* active profile +* repo/project +* runtime context +* exact capability for reading/searching issues +* exact capability for creating issues, if creating issues +* exact capability for commenting on issues, if commenting on existing issues +* exact capability for editing issues, if editing existing issues +* exact capability for applying labels, if applying labels +* exact capability for assigning issues, if assigning issues +* exact capability for closing issues, only if explicitly requested + +A nearby capability does not count. + +Examples: + +* `create_issue` does not authorize `issue_comment` +* `issue_comment` does not authorize `create_issue` +* `create_pr` does not authorize `create_issue` +* `review_pr` does not authorize `create_issue` +* `merge_pr` does not authorize `issue_comment` +* `gitea.read` does not authorize creating, commenting, editing, labeling, assigning, or closing issues + +If exact capability cannot be proven, stop and produce a recovery handoff only. + +## 3. Stop immediately on blocked infrastructure + +If any of the following appears, stop immediately: + +* `infra_stop` +* MCP reconnect failure +* stale capability state +* missing capability +* workspace mismatch +* dirty control checkout, if the canonical workflow treats that as blocking +* broken canonical workflow loading +* failed required preflight +* capability resolver warning that says the current state may be unsafe +* stale or inconsistent runtime context + +Do not continue duplicate search, issue creation, issue commenting, issue editing, labeling, assignment, or cleanup. + +Produce an executable recovery handoff only. + +Blocked recovery handoffs must not include direct issue-create or issue-comment replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +## 4. Main checkout rule + +This workflow should not mutate repo files. + +Do not edit files in the main checkout. + +Do not create branches. + +Do not create commits. + +Do not push. + +Do not run implementation work. + +Do not run reviewer validation. + +Reading repository files is allowed only when needed to understand the requested issue and only if the canonical workflow permits it. + +If the main checkout is dirty and the project treats dirty control checkout as blocking, stop and produce a recovery handoff. + +## 5. No raw MCP repair during issue creation + +Do not run `pkill`, kill MCP processes, edit MCP config, restart servers, or perform control-checkout repair during issue creation. + +If MCP repair is required, stop issue creation and produce a separate `CONTROL-CHECKOUT REPAIR MODE` handoff. + +Do not mix MCP repair mode with create-issue mode. + +After repair, rerun the full workflow from the beginning. + +## 6. No background task tools + +Do not use `schedule`, `manage_task`, background jobs, async waits, delayed task tools, or monitoring tasks during issue creation. + +Use direct commands and MCP tools only. + +If a required action cannot complete synchronously, stop and produce a recovery handoff. + +Do not say “I will check later,” “I will monitor,” or “I will continue in the background.” + +## 7. No local Gitea fallback during normal issue creation + +During normal issue-creation workflows, do not read Gitea profile secret files. + +Do not inspect or open files such as: + +* `profiles.json` +* local token stores +* credential files +* local Gitea auth/profile config files +* `.env` files containing Gitea credentials +* keychain dumps +* token helper outputs + +Do not run local Gitea helper scripts when MCP tools are available. + +Use MCP tools for Gitea operations. + +Local fallback is allowed only in explicit recovery mode when MCP is unavailable and identity/profile/capability can be independently proven. + +If local fallback is used, report: + +* why MCP was unavailable +* exact identity proof +* exact profile proof +* exact repo proof +* exact capability proof +* exact local command used + +Do not use local fallback to bypass MCP gates. + +## 8. Understand the requested issue work + +Before searching or creating issues, restate the requested issue-creation task in operational terms. + +Identify: + +* target repo/project +* issue topic +* issue type, if known +* whether this is a new issue, duplicate check, issue update, or issue-comment task +* whether the user provided exact title/body text +* whether acceptance criteria were provided +* whether multiple issues are requested +* whether labels, assignees, milestones, or links are requested +* whether any requested action requires capability beyond issue creation + +Do not invent missing requirements. + +If the request is ambiguous but safe to proceed, make a reasonable best-effort issue with clear assumptions. + +If ambiguity would cause unsafe or wrong mutation, stop and ask for clarification or produce a recovery handoff according to project policy. + +## 9. Duplicate search before mutation + +Before creating any issue, search open and closed issues for duplicates. + +Duplicate search must happen before each issue creation unless a batch search clearly covers all proposed issues. + +Search terms must include: + +* exact proposed issue title +* key noun phrase from the problem +* key workflow/tool name +* key failure phrase or error phrase +* likely alternate wording +* linked PR number, issue number, or file name, if relevant + +If the user supplied search terms, use those too. + +For each proposed issue, report: + +* search terms used +* matching issue numbers/titles +* whether each match is open or closed +* whether any match fully covers the requested issue +* whether any match partially covers the requested issue +* whether a new issue is still needed + +Do not create a duplicate issue if an existing issue fully covers the problem. + +If a duplicate exists and fully covers the problem, stop issue creation for that topic and report the duplicate. + +If a duplicate exists but is missing important acceptance criteria, comment on the existing issue only if exact `issue_comment` capability is proven and the user/task authorizes commenting. + +If commenting is not authorized, report the existing issue and the missing criteria in the final handoff. + +## 10. Issue inventory pagination rule + +If listing/searching issues returns paginated results, follow pagination until the tool proves there are no more pages. + +Do not assume search results or issue inventory are complete. + +Pagination proof must not rely on assumed default API page size. + +Search/inventory is complete only if one of the following is proven: + +* the MCP response explicitly says there is no next page / `has_more=false` / final page +* the workflow traversed pages until an empty page or explicit final page was returned +* the tool response includes total-count or pagination metadata proving all relevant issues were returned +* the request explicitly set `page` / `limit` / `per_page`, and the response explicitly proves the server honored that page size and did not truncate results + +Do not say “duplicate search complete” merely because the result count is less than an assumed default page size. + +If pagination metadata is absent and the tool cannot page, report `ISSUE_SEARCH_PAGINATION_UNPROVEN`. + +If duplicate search cannot be trusted, do not create the issue unless the canonical workflow explicitly permits best-effort issue creation with that limitation disclosed. + +## 11. Issue creation scope rule + +Create only issues within the requested scope. + +Do not create extra issues just because related problems are noticed. + +Do not create process-hardening issues unless the user explicitly requested process-hardening or the current task is explicitly about workflow/tooling gaps. + +Do not create implementation issues during reviewer mode. + +Do not create reviewer issues during work-on-issue mode. + +Do not create issues in a different repository unless the user explicitly asked and exact capability is proven. + +If multiple issues are requested, create only the requested issues and only after duplicate search for each one. + +If a proposed issue is too broad, split it only if the user requested splitting or the canonical workflow requires issue granularity. + +## 12. Issue content quality rule + +Every created issue must be actionable. + +Include, when applicable: + +* title +* problem statement +* observed evidence +* expected behavior +* required behavior +* acceptance criteria +* affected workflow/tool/files +* safety or security considerations +* duplicate search summary +* related issues or PRs +* non-goals, if useful + +Acceptance criteria must be concrete and testable. + +Avoid vague issues like: + +* “make workflow better” +* “fix LLM behavior” +* “improve process” +* “handle this better” + +Instead, describe the exact wall, gate, verifier, test, schema, helper, or prompt change required. + +## 13. Issue title rule + +Use concise, specific titles. + +Good title patterns: + +* `Enforce ` +* `Add verifier for ` +* `Split into ` +* `Block during ` +* `Require before ` + +Avoid titles that are too broad or emotional. + +The title should be unique enough that duplicate search can find it later. + +## 14. Issue body rule + +Issue body must include the full acceptance criteria. + +Do not create placeholder issues. + +Do not create issues with only a title unless the user explicitly requested title-only creation. + +If the user supplied exact issue body text, preserve it unless it contains unsafe instructions, stale facts, or contradictions. + +If edits are needed, make the smallest correction necessary and report the correction. + +Do not silently change requested meaning. + +## 15. Labels, assignees, and metadata + +Apply labels, assignees, milestones, or project fields only if: + +* the user requested them, or +* the canonical workflow requires them, and +* exact capability is proven. + +Do not guess labels if project label policy is unknown. + +If labels are useful but capability or policy is unclear, mention recommended labels in the final report instead of applying them. + +Do not assign issues to people unless explicitly requested or required by project workflow. + +## 16. Comment-on-existing issue rule + +Comment on an existing issue only if: + +* an existing issue partially covers the requested work, or +* the user asked to add information to an existing issue, or +* the canonical workflow requires duplicate consolidation comments, and +* exact `issue_comment` capability is proven. + +Comment must be specific and useful. + +Include: + +* why the existing issue is relevant +* what acceptance criteria or evidence should be added +* whether this avoids creating a duplicate + +Do not comment just to say “duplicate found” unless the project workflow requires it. + +Do not close duplicate issues unless explicitly requested and exact close capability is proven. + +## 17. No hidden mutations + +Do not perform unreported mutations. + +Every issue creation, issue comment, issue edit, label change, assignment, milestone change, close/reopen action, or external-state change must be reported. + +If a tool call is dry-run-only, confirmation-gated, rejected, or no-op, report it separately from performed mutations. + +A dry run is not a mutation. + +A rejected call is not a performed mutation. + +A successful issue creation is an issue mutation. + +A successful issue comment is an issue mutation. + +A successful label/assignment/milestone update is an issue mutation or external-state mutation. + +## 18. Issue creation gate + +Before creating each issue, verify: + +* identity is still valid +* active profile is still valid +* runtime context is still safe +* exact `create_issue` capability is still valid +* duplicate search was completed or limitation was explicitly allowed +* proposed title is not a duplicate +* proposed body includes actionable acceptance criteria +* target repo is correct +* no mode switch has occurred + +If any gate fails, do not create the issue. + +Produce a recovery handoff or duplicate report. + +## 19. Issue commenting gate + +Before commenting on an existing issue, verify: + +* identity is still valid +* active profile is still valid +* runtime context is still safe +* exact `issue_comment` capability is still valid +* target issue number is correct +* comment body is specific and useful +* comment will not duplicate an existing comment +* no mode switch has occurred + +If any gate fails, do not comment. + +Produce a recovery handoff or report the intended comment as a recommendation only. + +## 20. Issue edit/update gate + +Before editing an existing issue, verify: + +* identity is still valid +* active profile is still valid +* runtime context is still safe +* exact edit capability is still valid +* target issue number is correct +* update is explicitly requested or required by canonical workflow +* update does not erase useful existing content +* no mode switch has occurred + +If any gate fails, do not edit. + +Prefer commenting over editing unless the user explicitly requested an edit or the canonical workflow requires issue body updates. + +## 21. Final report must be precise + +Include: + +* canonical workflow source/version/hash, if available +* authenticated identity/profile +* repo/project +* runtime context summary +* exact capability proof summary +* requested issue-creation task +* duplicate search terms used +* duplicate search result +* pagination/final-page proof for issue search, if applicable +* issues created, with issue numbers and URLs +* existing issues commented, with issue numbers and URLs +* existing issues edited, with issue numbers and URLs +* issues skipped as duplicates, with issue numbers and titles +* labels/assignees/milestones applied, if any +* blockers, if stopped +* confirmation that no PR review, approval, request-changes, merge, branch, checkout, commit, push, or repo-file mutation was performed + +If the report and actual tool/command log disagree, fix the report before final output. + +## 22. Final report must distinguish mutation types + +Do not use the legacy field `Workspace mutations`. + +Use only precise categories: + +* File edits by issue creator: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Issue mutations: +* Label/assignment/milestone mutations: +* External-state mutations: +* Read-only diagnostics: + +Use precise wording: + +* `File edits by issue creator: none` +* `Worktree/index mutations: none` +* `Git ref mutations: none` +* `Issue mutations: ...` + +If no repo files were edited, say: + +`File edits by issue creator: none` + +If no branches/worktrees were touched, say: + +`Worktree/index mutations: none` + +If no git refs were updated, say: + +`Git ref mutations: none` + +Do not hide issue mutations inside vague `MCP/Gitea mutations`. + +## 23. Local artifact and report consistency rule + +Do not create local walkthrough, notes, markdown, JSON, or report artifacts during issue-creation runs unless the canonical workflow or operator explicitly requires it. + +If any file is edited, created, generated, or written, report it under `File edits by issue creator`. + +For each file write, report: + +* exact path +* whether it was inside the repo +* whether it was tracked or untracked +* why it was created +* whether final status was checked after the write + +Do not say `File edits by issue creator: none` if any file write occurred. + +Do not write files after the final clean-status check unless you rerun and report a new final clean-status check. + +Default behavior: do not create local artifacts during issue creation. + +## 24. Forbidden final-report claims unless proven + +Do not claim: + +* `duplicate search complete` +* `no duplicate found` +* `issue created` +* `issue commented` +* `issue updated` +* `label applied` +* `capability proven` +* `runtime safe` +* `all gates passed` +* `no file edits` +* `no unsafe mutation` +* `no PR mutation` +* `no repo mutation` +* `pagination complete` +* `final page` +* `no next page` + +unless the corresponding proof is included. + +If anything blocks safe issue creation or issue update, stop immediately and produce an executable recovery handoff. + +Do not improvise around the gates. + +## 25. Proof wording enforcement + +The following phrases are forbidden unless directly supported by current-session evidence: + +* duplicate search complete +* no duplicate found +* issue created +* issue commented +* issue updated +* labels applied +* capability proven +* runtime safe +* all gates passed +* no file edits +* no unsafe mutation +* no PR mutation +* no repo mutation +* pagination complete +* final page +* no next page + +If the proof comes from prior state rather than a command/tool run in the current session, label it as prior proof, not live proof. + +If a tool call was rejected, confirmation-gated, dry-run-only, or no-op, report it separately from performed mutations. + +## 26. Final self-check before output + +Before final output, check the report for contradictions. + +Verify: + +* if any file was edited, `File edits by issue creator` is not `none` +* if any worktree was added/removed, `Worktree/index mutations` lists it +* if any fetch happened, `Git ref mutations` lists it +* if any issue was created, `Issue mutations` lists it +* if any issue was commented, `Issue mutations` lists it +* if any issue was edited, `Issue mutations` lists it +* if any labels/assignees/milestones were changed, the correct mutation category lists it +* if duplicate search is claimed complete, pagination/final-page proof is present or limitation is disclosed +* if no duplicate is claimed, search terms and results are present +* if issue created is claimed, issue number and URL are present +* if no PR mutation is claimed, no PR tool/action was used +* if no repo mutation is claimed, no branch/worktree/file/commit/push action occurred +* if all gates passed is claimed, every required gate has proof + +If any contradiction exists, fix the final report before output. + +## 27. Controller handoff schema + +End every run with a controller handoff using this schema. + +Do not omit fields. Use `none` or `not verified in this session` where appropriate. + +Controller Handoff: + +* Task: +* Repo: +* Role: +* Identity: +* Active profile: +* Runtime context: +* Requested issue task: +* Workflow source: +* Capability proof: +* Duplicate search terms: +* Duplicate search pagination proof: +* Duplicates found: +* Issues created: +* Issues commented: +* Issues edited: +* Issues skipped as duplicates: +* Labels/assignees/milestones changed: +* File edits by issue creator: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Issue mutations: +* Label/assignment/milestone mutations: +* External-state mutations: +* Read-only diagnostics: +* Blockers: +* Current status: +* Safe next action: +* Safety statement: + +## 28. Stop conditions summary + +Stop immediately and produce a recovery handoff if: + +* canonical workflow is required but cannot be loaded +* identity/profile/capability cannot be proven +* runtime context is blocked +* infra stop appears +* MCP reconnect fails +* capability state is stale +* duplicate search cannot be performed and best-effort creation is not allowed +* issue search pagination cannot be proven and best-effort creation is not allowed +* duplicate fully covers the requested issue +* requested issue body is unsafe or not actionable +* target repo cannot be proven +* create_issue capability is missing +* issue_comment capability is missing for a required comment +* issue edit capability is missing for a required edit +* mode switch would be required +* any report contradiction cannot be resolved + +Blocked handoffs must not include direct issue-create or issue-comment replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +Do not improvise around the gates. diff --git a/skills/llm-project-workflow/workflows/reconcile-landed-pr.md b/skills/llm-project-workflow/workflows/reconcile-landed-pr.md new file mode 100644 index 0000000..80a7c4b --- /dev/null +++ b/skills/llm-project-workflow/workflows/reconcile-landed-pr.md @@ -0,0 +1,387 @@ +--- +task_mode: reconcile-landed-pr +canonical: true +final_report_schema: ../schemas/reconcile-landed-final-report.md +--- + +# Reconcile already-landed open PR workflow (canonical) + +**Task mode:** `reconcile-landed-pr` + +This file is the canonical reconciliation workflow for open PRs whose head SHA +is already an ancestor of the target branch. Load it before any reconciliation +mutation. Final report schema: +[`schemas/reconcile-landed-final-report.md`](../schemas/reconcile-landed-final-report.md). + +**Default task prompt:** + +> Reconcile already-landed open PRs in this project. Do not review or merge +> normal PRs. Close or comment only when exact capability is proven. + +Do not improvise around the gates. Follow project skills, MCP gates, and +workflow rules exactly. + +This is a reconciliation workflow. It is not a normal PR review/merge workflow. + +--- + +## 0. Load the canonical workflow first + +Before starting reconciliation work, check whether the project provides a +canonical reconcile-landed-PR workflow through a project skill, runbook, or MCP +helper. + +If available, load it first and report: + +* workflow source +* workflow version, commit, or hash +* whether this prompt conflicts with the loaded workflow + +If the canonical workflow cannot be loaded and the project requires it, stop and +produce a recovery handoff only. + +## 1. Mode isolation + +This run is `reconcile-landed-pr` mode only. + +**Do not review or merge normal PRs.** + +Do not: + +* approve PRs +* request changes on PRs +* merge PRs +* implement code +* edit repo files +* create branches +* create commits +* push branches +* create PRs +* run normal PR validation as review approval input +* perform author/coder implementation work +* perform raw MCP repair + +If the task requires review, merge, issue implementation, or MCP repair mode, +stop and produce a handoff for the correct workflow. + +Do not mix modes in one run. + +## 2. Start with live identity, profile, runtime, and capability checks + +Prove: + +* authenticated identity +* active profile (reconciler or author with close capabilities, as required) +* repo/project +* runtime context +* exact capability for reading/listing PRs and issues +* exact capability for PR inspect (`gitea.read` / view PR) +* exact capability for issue inspect +* exact capability for PR comment, if commenting +* exact capability for issue comment, if commenting +* exact capability for PR close, if closing PRs +* exact capability for issue close, if closing issues + +A nearby capability does not count. + +Examples: + +* `review_pr` does not authorize PR close +* `merge_pr` does not authorize PR close or issue close +* `create_issue` does not authorize `issue_comment` +* `issue_comment` does not authorize PR close +* `gitea.read` does not authorize close or comment mutations + +If exact capability cannot be proven, stop and produce a recovery handoff only. + +## 3. Stop immediately on blocked infrastructure + +If any of the following appears, stop immediately: + +* `infra_stop` +* MCP reconnect failure +* stale capability state +* missing capability +* workspace mismatch +* broken canonical workflow loading +* failed required preflight +* capability resolver warning that says the current state may be unsafe +* stale or inconsistent runtime context + +Do not continue inventory, ancestry proof, commenting, closing, or cleanup. + +Produce an executable recovery handoff only. + +Blocked recovery handoffs must not include direct close or comment replay +commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +## 4. Main checkout rule + +This workflow should not mutate repo files. + +Do not edit files in the main checkout. + +Do not create branches, commits, or pushes. + +Do not run implementation or reviewer validation worktrees for code edits. + +Reading repository files is allowed only when needed to understand +reconciliation scope and only if this workflow permits it. + +## 5. No raw MCP repair during reconciliation + +Do not run `pkill`, kill MCP processes, edit MCP config, restart servers, or +perform control-checkout repair during reconciliation. + +If MCP repair is required, stop and produce a separate `CONTROL-CHECKOUT REPAIR +MODE` handoff. + +After repair, rerun the full workflow from the beginning. + +## 6. No background task tools + +Do not use `schedule`, `manage_task`, background jobs, async waits, delayed task +tools, or monitoring tasks during reconciliation. + +Use direct commands and MCP tools only. + +If a required action cannot complete synchronously, stop and produce a recovery +handoff. + +## 7. No local Gitea fallback during normal reconciliation + +During normal reconciliation workflows, do not read Gitea profile secret files. + +Do not inspect `profiles.json`, local token stores, credential files, `.env` +Gitea credentials, keychain dumps, or token helper outputs. + +Do not run local Gitea helper scripts when MCP tools are available. + +Use MCP tools for Gitea operations. + +Local fallback is allowed only in explicit recovery mode when MCP is unavailable +and identity/profile/capability can be independently proven. + +## 8. Build a complete live open PR inventory + +List open PRs for the target repo according to project policy. + +Follow pagination until the tool proves there are no more pages. + +Do not assume inventory is complete. + +Pagination proof must not rely on assumed default API page size. + +Inventory is complete only if one of the following is proven: + +* the MCP response explicitly says there is no next page / `has_more=false` / + final page +* the workflow traversed pages until an empty page or explicit final page was + returned +* the tool response includes total-count or pagination metadata proving all + relevant PRs were returned +* the request explicitly set `page` / `limit` / `per_page`, and the response + explicitly proves the server honored that page size and did not truncate results + +If pagination cannot be proven, report `INVENTORY_PAGINATION_UNPROVEN` and stop +unless project policy allows best-effort reconciliation with that limitation +disclosed. + +## 9. Already-landed proof + +For each candidate PR, prove whether the PR head SHA is already landed on the +target branch. + +**Already-landed proof** must include: + +* PR number and title +* candidate head SHA (full 40-hex) +* target branch name +* target branch SHA (full 40-hex) after fetch +* ancestor proof method (`git merge-base --is-ancestor`, equivalent forge API, or + documented project helper) +* ancestor proof result (true/false) +* live PR state (open/closed, merged flag) + +Do not classify a PR as already-landed without live ancestor proof. + +If ancestry cannot be proven, classify as `ANCESTRY_UNPROVEN` and skip close +mutations. + +## 10. Linked issue live verification + +If the PR claims to close or link an issue, fetch the linked issue live before +reporting its status. + +If the linked issue was not fetched live in the current session, report: + +`Linked issue status: not verified in this session` + +Do not claim `issue open`, `issue closed`, or `issue resolved` without live +proof. + +## 11. Reconciliation selection rules + +Select PRs eligible for reconciliation: + +* open PR state +* `merged=false` unless project policy says otherwise +* head SHA is ancestor of target branch (already-landed proof passed) +* not selected for normal review/merge in this run + +Eligibility class for selected PRs: `ALREADY_LANDED_RECONCILE_REQUIRED` + +Do not select PRs that fail already-landed proof for normal review/merge +treatment in this mode. + +## 12. Reconciliation comment policy + +Post a reconciliation comment only if: + +* exact PR-comment or issue-comment capability is proven +* the comment adds durable evidence (ancestor proof summary, recommended close + action, linked issue status) +* the comment will not duplicate an equivalent recent reconciliation comment + +If comment capability is missing, record the intended comment in the final +handoff only. + +## 13. PR close rules + +Close a PR only if: + +* already-landed proof passed in this session +* exact PR-close capability is proven +* PR is still open at mutation time (live re-fetch) +* head SHA still matches the proved candidate head SHA + +If PR-close capability is missing, produce a recovery handoff with exact PR, +proof, and required capability. Do not loop forever re-blocking the reviewer +queue. + +## 14. Issue close rules + +Close a linked issue only if: + +* exact issue-close capability is proven +* linked issue was fetched live +* issue resolution is justified by landed content and project policy +* issue is still open at mutation time + +If issue-close capability is missing, report the gap in the handoff. + +## 15. Missing capability behavior + +If any required mutation capability is missing: + +* do not improvise with review/merge tools +* do not ask the operator to bypass capability gates +* produce a recovery handoff listing exact missing capabilities +* include safe next action (profile switch, human close, or dedicated reconciler + profile) + +## 16. Mutation classification + +Use precise mutation categories in the final report: + +* File edits by reconciler: (expect `none`) +* Worktree/index mutations: +* Git ref mutations: (`git fetch` belongs here, not read-only diagnostics) +* MCP/Gitea mutations: +* Reconciliation mutations: (PR comment, issue comment, PR close, issue close) +* External-state mutations: +* Read-only diagnostics: + +Do not use legacy `Workspace mutations`. + +## 17. Identity privacy rule + +Report identity as `username / profile` (#305). + +Do not disclose personal email in final reports unless explicitly required. + +## 18. Precise final report + +Include: + +* canonical workflow source/version/hash +* authenticated identity/profile +* repo/project +* capability proof summary (separate lines for inspect, comment, PR close, + issue close) +* inventory pagination proof +* selected PR(s) with already-landed proof +* linked issue live status +* mutations performed or blocked +* missing capabilities +* confirmation that no normal review, approval, request-changes, or merge was + performed + +## 19. Local artifact and report consistency rule + +Do not create local walkthrough, notes, markdown, JSON, or report artifacts +during reconciliation unless explicitly required. + +If any file is edited, report under `File edits by reconciler`. + +Default: no repo file edits. + +## 20. Forbidden unsupported claims unless proven + +Do not claim: + +* `already-landed` +* `PR closed` +* `issue closed` +* `pagination complete` +* `inventory complete` +* `all gates passed` +* `no unsafe mutation` + +unless the corresponding proof is included. + +## 21. Proof wording enforcement + +Forbidden unless supported by current-session evidence: + +* pagination complete +* final page +* no next page +* PR closed +* issue closed +* all gates passed + +If proof comes from prior state, label as prior proof, not live proof. + +## 22. Final self-check before output + +Verify: + +* no normal review/merge mutations occurred +* `git fetch` is under Git ref mutations if it occurred +* already-landed proof is present for each selected PR +* handoff uses reconciliation schema, not author/reviewer merge schema +* no contradiction between narrative report and controller handoff + +## 23. Controller handoff schema + +End every run with `Controller Handoff` per +[`schemas/reconcile-landed-final-report.md`](../schemas/reconcile-landed-final-report.md). + +## 24. Stop conditions summary + +Stop immediately and produce a recovery handoff if: + +* canonical workflow cannot be loaded +* identity/profile/capability cannot be proven +* runtime context is blocked +* `infra_stop` appears +* inventory pagination cannot be proven and best-effort is not allowed +* already-landed proof cannot be completed +* required close capability is missing and mutation was attempted +* live PR/issue state contradicts proof +* any report contradiction cannot be resolved + +Do not improvise around the gates. \ No newline at end of file diff --git a/skills/llm-project-workflow/workflows/review-merge-pr.md b/skills/llm-project-workflow/workflows/review-merge-pr.md new file mode 100644 index 0000000..86eb359 --- /dev/null +++ b/skills/llm-project-workflow/workflows/review-merge-pr.md @@ -0,0 +1,1153 @@ +--- +task_mode: review-merge-pr +canonical: true +final_report_schema: ../schemas/review-merge-final-report.md +--- + +# Review and merge PR workflow (canonical) + +**Task mode:** `review-merge-pr` + +This file is the canonical PR review/merge workflow for Gitea-Tools. Load it +before any PR mutation. Final report schema: +[`schemas/review-merge-final-report.md`](../schemas/review-merge-final-report.md). + +**Default task prompt:** + +> Review the next eligible open PR in this project. Merge it only if every +> identity, capability, author-safety, inventory, queue-ordering, worktree, +> validation, already-landed, prior-review, final-report, mutation-ledger, +> proof-wording, and merge gate passes. + +Do not improvise around the gates. Follow project skills, MCP gates, and +workflow rules exactly. + +--- + +## 0. Load the canonical workflow first + +Before starting PR work, check whether the project provides a canonical PR review/merge workflow through a project skill, runbook, or MCP helper. + +If available, load it first and report: + +* workflow source +* workflow version, commit, or hash +* whether this prompt conflicts with the loaded workflow + +If the canonical workflow cannot be loaded and the project requires it, stop and produce a recovery handoff only. + +## 1. Start with live identity, profile, runtime, and capability checks + +Prove: + +* authenticated identity +* active reviewer profile +* repo/project +* runtime context +* exact capabilities needed for inventory, review, request-changes, approve, comment, and merge + +A nearby capability does not count. + +Examples: + +* `create_issue` does not authorize `issue_comment` +* `review_pr` does not authorize `merge_pr` +* `issue_comment` does not authorize `create_issue` +* `review_pr` does not authorize `request_changes` unless the capability explicitly covers that operation +* `review_pr` does not authorize `approve` unless the capability explicitly covers that operation + +If capability cannot be proven, stop and produce a recovery handoff only. + +## 2. Stop immediately on blocked infrastructure + +If any of the following appears, stop immediately: + +* `infra_stop` +* MCP reconnect failure +* stale capability state +* dirty control checkout +* dirty review worktree +* missing capability +* workspace mismatch +* stale target branch state +* broken canonical workflow loading +* failed required preflight +* capability resolver warning that says the current state may be unsafe +* stale or inconsistent runtime context + +Do not continue PR selection, validation, review, approval, merge, cleanup, or reconciliation. + +Produce an executable recovery handoff only. + +Blocked recovery handoffs must not include direct approve or merge replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +Do not preserve stale `ready to merge`, `approved`, or `all gates passed` state across MCP failures, reconnects, or capability re-resolution. + +## 3. Author safety + +If you are the PR author, stop immediately. + +Do not approve, request changes, comment as reviewer, merge, close, or mutate the PR. + +Produce a recovery handoff only. + +If author identity cannot be proven, stop and produce a recovery handoff only. + +If authenticated identity changes during the run, restart identity/profile/capability checks before any further mutation. + +## 4. Main checkout rule + +The main project checkout must stay on `master`, `main`, or `dev`. + +Do not review, test, fix, merge, resolve conflicts, or cleanup from the main checkout. + +Do not run tests in the main checkout. + +Do not run baseline validation in the main checkout. + +All PR validation and baseline comparison work must happen under the project’s `branches/` directory. + +No exceptions for small fixes, docs, tests, cleanup, conflict resolution, emergencies, or “just one file.” + +Updating the main checkout is allowed only after a successful merge and only if the project workflow explicitly allows stable-branch update. + +Do not update the main checkout if merge failed, was blocked, or produced reconciliation-only status. + +If the main checkout is dirty before selection, stop and produce a recovery handoff. + +If the main checkout becomes dirty during the run, stop and produce a recovery handoff unless the only change is the explicitly allowed post-merge stable-branch fast-forward. + +## 5. No raw MCP repair during normal review + +Do not run `pkill`, kill MCP processes, edit MCP config, restart servers, or perform control-checkout repair during normal PR review. + +If MCP repair is required, stop PR review and produce a separate `CONTROL-CHECKOUT REPAIR MODE` handoff. + +Do not mix MCP repair mode with PR review/merge mode. + +Do not use successful repair as permission to resume the same review flow. After repair, rerun the full workflow from the beginning. + +## 6. No background task tools + +Do not use `schedule`, `manage_task`, background jobs, async waits, or delayed task tools during PR review/merge. + +Use direct commands and MCP tools only. + +If a required action cannot complete synchronously, stop and produce a recovery handoff. + +Long synchronous commands, such as a test suite, are allowed only if they are run directly and reported with exact command, working directory, and result. + +Do not say “I will check later,” “I will monitor,” or “I will continue in the background.” + +## 7. No local Gitea fallback during normal review + +During normal reviewer workflows, do not read Gitea profile secret files. + +Do not inspect or open files such as: + +* `profiles.json` +* local token stores +* credential files +* local Gitea auth/profile config files +* `.env` files containing Gitea credentials +* keychain dumps +* token helper outputs + +Do not run local Gitea helper scripts when MCP tools are available. + +Use MCP tools for Gitea operations. + +Local fallback is allowed only in explicit recovery mode when MCP is unavailable and identity/profile/capability can be independently proven. + +If local fallback is used, report: + +* why MCP was unavailable +* exact identity proof +* exact profile proof +* exact repo proof +* exact capability proof +* exact local command used + +Do not use local fallback to bypass MCP gates. + +## 8. Build a complete live PR inventory + +List all open PRs. + +Follow pagination until the tool proves there are no more pages. + +Do not assume inventory is complete. + +Do not claim `oldest eligible PR`, `next eligible PR`, or complete queue inventory unless pagination is proven. + +Pagination proof must not rely on assumed default API page size. + +Inventory is complete only if one of the following is proven: + +* the MCP response explicitly says there is no next page / `has_more=false` / final page +* the workflow traversed pages until an empty page or explicit final page was returned +* the tool response includes total-count or pagination metadata proving all open PRs were returned +* the request explicitly set `page` / `limit` / `per_page`, and the response explicitly proves the server honored that page size and did not truncate results + +Do not say “inventory complete” merely because the result count is less than an assumed default page size. + +If the tool returns exactly the requested page size, assume there may be more pages unless the tool proves otherwise. + +If pagination metadata is absent and the tool cannot page, report `INVENTORY_PAGINATION_UNPROVEN` and stop unless the canonical workflow explicitly permits continuing with an incomplete inventory. + +For each open PR, identify: + +* PR number +* title +* author +* base branch +* head branch +* draft/WIP state +* mergeability +* linked issue +* current head SHA +* whether the PR appears already landed + +Final report must include pagination/final-page proof. + +## 9. Queue ordering proof rule + +State the queue ordering policy before selecting a PR. + +If the project uses oldest-first review, explicitly sort or reason by PR number or created date. + +Do not rely on the API response order unless the tool proves that order matches the project policy. + +If newer PRs appear before the selected PR in the API response, explain that they are newer and not earlier in the queue. + +Final report must include: + +* queue ordering policy used +* whether API order matched that policy +* selected PR’s position under that policy +* earlier PRs considered +* earlier PRs skipped and why + +If queue ordering cannot be proven, stop and produce a recovery handoff. + +## 10. Select the next actionable PR using project rules + +Do not review your own PR. + +Do not review stale, draft, blocked, duplicate, already-owned, dependency-blocked, already-landed, already-requested-changes work unless the rules explicitly allow it. + +Do not skip an earlier actionable PR without documenting why. + +Use these terms precisely: + +* `Oldest open PR requiring action` +* `Eligibility class` +* `Next review/merge eligible PR` + +Do not call an already-landed PR `eligible for review` or `eligible for merge`. + +If the oldest open PR is already landed, classify it as: + +`ALREADY_LANDED_RECONCILE_REQUIRED` + +If the oldest open PR is `ALREADY_LANDED_RECONCILE_REQUIRED`, stop normal review immediately unless project policy explicitly allows skipping reconciliation blockers. + +Report: + +`Queue blocked by already-landed PR requiring reconciliation` + +Do not keep rerunning normal review on the same already-landed PR. + +If the selected PR is not the oldest open PR requiring action, report why every earlier PR was not actionable. + +## 11. Skip-prior-PR rule + +Do not skip an earlier open PR without live proof or explicitly labeled prior proof from current-session review feedback. + +If skipping because of prior `REQUEST_CHANGES`, fetch current review feedback and verify the head SHA has not changed since the blocking review. + +If skipping because of non-mergeable status, fetch live PR state and provide conflict proof. + +Report for every skipped earlier PR: + +* skipped PR number +* current head SHA +* blocking category +* blocking review decision, if applicable +* blocking review head SHA, if applicable +* mergeability result, if applicable +* conflict proof command/tool, if applicable +* conflicting files, if available +* whether the head changed after the blocker +* reason it remains blocked + +If blocker proof cannot be fetched, classify the PR as: + +`BLOCKER_STATUS_UNVERIFIED` + +Then stop or produce a recovery handoff according to project policy. + +Do not skip an earlier PR based only on memory from a previous session. + +## 11A. Skipped PRs are read-only + +Skipping an earlier PR must be a read-only classification step. + +Do not submit `REQUEST_CHANGES`, approve, comment, merge, close, or otherwise mutate a skipped PR. + +If an earlier PR already has unresolved `REQUEST_CHANGES` at the same head SHA, do not duplicate the review. Fetch the review feedback, verify the head SHA is unchanged, classify the PR as blocked, and move on only if project policy allows skipping blocked PRs. + +If the head SHA changed after the blocking review, the PR is not safely skippable based on the old blocker. Re-evaluate it according to queue policy. + +The final report must distinguish: + +* prior blocker reused +* blocker revalidated live in this session +* blocker status unverified + +Do not claim live proof unless the proof command or MCP tool ran in the current session. + +If you mutate a skipped PR, stop immediately and report a workflow violation. + +## 12. Non-mergeable skip proof rule + +Do not skip a non-mergeable earlier PR without live proof. + +For every skipped non-mergeable PR, report: + +* PR number +* current head SHA +* mergeability result +* command/tool used to prove conflict +* conflicting files, if available +* whether head changed since any prior blocker + +If conflict proof cannot be retrieved, classify the PR as: + +`MERGEABILITY_UNVERIFIED` + +Then stop or produce a recovery handoff according to project policy. + +Conflict proof must be obtained in a session-owned diagnostic worktree under `branches/`, unless the MCP tool itself provides exact conflict files and proof. + +Do not perform conflict proof in the main checkout. + +## 13. Prior request-changes override rule + +Before approving a PR, fetch current PR review feedback. + +If the PR has a prior `REQUEST_CHANGES` review, report: + +* blocking reviewer +* blocking review timestamp +* blocking review head SHA +* current head SHA +* blocker text +* whether the head changed since the blocker + +If the head changed after the request-changes review, verify the new head addresses the blocker before approving. + +If the head did not change after the request-changes review, do not approve unless you prove one of: + +* the earlier blocker was incorrect +* the validation environment was wrong +* the issue was resolved externally in a way that does not require PR head changes +* project policy explicitly permits overriding that blocker with proof + +If this proof is missing, do not approve. + +If the selected PR still has unresolved `REQUEST_CHANGES` at the same head SHA and no override proof exists, submit `REQUEST_CHANGES` only if no equivalent unresolved request-changes review already exists at that same head SHA. Otherwise stop with a recovery handoff or classify it blocked according to project policy. + +Final report must include the prior blocker, current head, whether head changed, and why approval is safe. + +## 14. Pin the PR head SHA before validation + +Re-fetch the selected PR. + +Record the current head SHA. + +Call this the `candidate head SHA`. + +Do not call it the `reviewed head SHA` until validation and diff review have actually passed. + +If the head SHA changes before review submission or merge, restart review or stop. + +If the head SHA changes after approval but before merge, do not merge. Restart review or stop. + +Final report must include: + +* candidate head SHA +* reviewed head SHA, only if validation and diff review passed +* whether head changed during the run + +## 15. Refresh target branch before ancestry checks + +Fetch the PR target branch from the remote before checking whether the PR head is already landed. + +Do not rely on stale local `master`, `main`, or `dev`. + +Record the fetched target branch SHA used for ancestry checks. + +If the target branch cannot be fetched or verified, stop and produce a recovery handoff. + +`git fetch`, `git remote update`, and any command that updates refs must be reported under `Git ref mutations`, not read-only diagnostics. + +## 16. Already-landed PR gate + +After pinning the PR head SHA and refreshing the target branch, check whether the PR head SHA is already an ancestor of the target branch. + +If the PR head SHA is already landed on the target branch, stop normal review/merge flow. + +Do not approve the PR. + +Do not request changes unless there is a real code/review blocker. + +Do not call the merge API. + +Produce an `ALREADY_LANDED_RECONCILE_REQUIRED` handoff instead. + +The handoff must include: + +* PR number/title +* eligibility class +* candidate head SHA +* reviewed head SHA: `none` +* target branch +* target branch SHA +* ancestor proof +* linked issue status, if verified +* recommended reconciliation action +* required capabilities for reconciliation + +Any close/comment/issue mutation requires separate exact capability proof. + +Do not tell the user or another agent to manually close a PR or issue unless the handoff clearly says capability must be proven first or an authorized human must do it. + +## 17. Linked issue verification + +If the PR claims to close or link an issue, fetch the linked issue live before reporting its status. + +If the linked issue was not fetched live in the current session, report: + +`Linked issue status: not verified in this session` + +Do not claim `issue open`, `issue closed`, or `issue resolved` without live proof. + +Any PR close, issue close, or comment recommendation must say exact capability must be proven first, unless an authorized human performs it. + +The linked issue number in diagnostics, final report, and controller handoff must match the selected PR. + +Do not include stale issue numbers from prior PRs. + +If linked issue status changes after merge, fetch the issue again before reporting post-merge status. + +## 18. Review worktree ownership rule + +Prefer a fresh session-owned review worktree under `branches/`, named like: + +`branches/review-pr-` + +Do not reuse a branch-named or existing worktree unless safe-reuse proof passes. + +Safe-reuse proof must include: + +* exact worktree path +* worktree is inside `branches/` +* worktree is not the main checkout +* worktree is not owned by another active task/session +* clean tracked state +* clean untracked state +* current branch/head before reset +* reset target SHA +* explicit project policy allowing reuse/reset + +Do not run `git reset --hard`, `git clean`, checkout, or other destructive worktree commands unless the worktree is session-owned or safe-reuse proof passes. + +If safe-reuse proof cannot be produced, create a fresh session-owned review worktree. + +## 19. Reviewer worktree no-edit rule + +The PR validation worktree is for validating the submitted PR head only. + +Do not edit files in the PR validation worktree. + +Do not apply local fixes in the PR validation worktree. + +Do not run official validation after modifying the PR validation worktree. + +If you need to test a possible fix, create a separate diagnostic scratch worktree and label it clearly: + +`Diagnostic local experiment — not PR-head validation` + +Any diagnostic edits must be reported under `File edits by reviewer`. + +Diagnostic test results must not be reported as official PR validation results. + +Do not say `File edits by reviewer: none` if any file was edited in any review, diagnostic, baseline, or artifact worktree. + +## 20. Validate in a session-owned worktree under `branches/` + +Prove: + +* exact review worktree path +* worktree is inside `branches/` +* worktree is clean before validation +* PR head is checked out in that worktree +* review worktree HEAD matches candidate head SHA + +Run required validation: + +* tests +* compile checks +* lint checks +* diff checks +* secret/provenance checks +* dangerous artifact checks +* project-specific validation + +If validation fails, request changes with exact blockers, unless an equivalent unresolved request-changes review already exists at the same head SHA. + +If an equivalent unresolved request-changes review already exists at the same head SHA, do not duplicate it. Stop with a final report or recovery handoff according to project policy. + +If validation cannot run, do not approve or merge. + +## 21. Official validation integrity rule + +Record the PR head SHA before validation. + +Prove the validation worktree is clean before validation. + +Run validation against the unmodified PR head. + +After validation, prove the validation worktree is still clean. + +If validation fails, submit `REQUEST_CHANGES` using the unmodified PR-head failure, unless an equivalent unresolved request-changes review already exists at the same head SHA. + +If the worktree was edited after validation for diagnosis, report it separately and do not use post-edit test results as official validation. + +If official validation was contaminated by a local edit, stop and produce a recovery handoff. + +Official validation integrity status must be one of: + +* `passed` +* `failed` +* `not run` +* `contaminated — recovery required` + +Do not invent a softer status. + +## 22. Baseline validation rule + +Do not run tests in the main checkout. + +If the PR full suite fails and you need to compare failures against `master`, create a clean baseline worktree under `branches/`, such as: + +`branches/baseline-master-pr` + +Baseline comparison must include: + +* baseline worktree path +* baseline target SHA +* PR head SHA +* exact command run on both worktrees +* baseline failures +* PR failures +* proof the failure signatures match +* proof the baseline worktree was clean before and after validation +* proof the PR worktree was clean before and after validation + +Do not approve a PR with full-suite failures unless: + +* project policy allows baseline comparison +* baseline proof is complete +* failure signatures match +* new/changed tests pass +* failures are proven unrelated to the PR + +If full-suite failures differ or proof is incomplete, submit `REQUEST_CHANGES`, unless an equivalent unresolved request-changes review already exists at the same head SHA. + +Do not claim “same as master” unless the clean baseline worktree proof is included. + +Do not claim “full-suite failures are pre-existing” unless baseline proof is complete and the failure signatures match. + +## 23. Validation command proof rule + +Report the exact validation command as executed. + +Report the working directory where validation ran. + +If using bare `pytest`, also report: + +* `which pytest` +* `pytest --version` +* whether it resolves to the project venv + +Prefer the project venv executable when available, for example: + +`/Users/jasonwalker/Development/Gitea-Tools/venv/bin/pytest tests/` + +Do not summarize validation as just `pytest passed` unless the exact command, working directory, and result are included. + +For every validation command, report: + +* command +* working directory +* exit code or pass/fail result +* summary count if available +* whether it was official PR-head validation, baseline validation, or diagnostic-only validation + +## 24. Local merge simulation rule + +A local merge simulation is not read-only. + +Commands such as the following must be reported as worktree/index mutations: + +* `git merge --no-commit` +* `git merge --abort` +* `git reset` +* `git checkout` +* `git clean` +* `git worktree add` +* `git worktree remove` + +If merge simulation is needed, prefer a separate session-owned diagnostic worktree. + +If merge simulation is run in the review worktree, prove: + +* worktree clean before simulation +* exact merge command +* result/conflict status +* abort/reset command +* worktree clean after abort + +Do not list merge simulation under read-only diagnostics. + +Do not call the worktree clean after simulation unless clean status was actually checked after abort/reset. + +If conflicting files are reported, they must come from current-session proof. + +## 25. Review the actual diff + +Check: + +* correctness +* tests +* scope +* security boundaries +* workflow rule compliance +* whether the PR really closes or satisfies the linked issue +* unrelated changes +* dangerous generated artifacts +* secrets +* provenance markers +* temporary agent files +* whether the PR is redundant because the diff is already present on the target branch +* whether the PR modifies reviewer workflow, MCP gates, profiles, routing, or authorization boundaries +* whether documentation and tests match behavior + +Do not approve based only on test pass/fail status. Review the diff. + +Report files reviewed. + +## 26. Review decision + +If blockers exist, submit `REQUEST_CHANGES`, unless an equivalent unresolved request-changes review already exists at the same head SHA. + +If non-blocking notes exist, comment clearly only if exact comment capability is proven. + +If everything passes, approve only if exact approve/review capability is proven. + +Do not approve and merge unless both actions are separately authorized. + +Submit review only after dry-run review passes if the tool supports dry run. + +Do not approve already-landed, redundant, stale, or reconciliation-only PRs. + +Do not approve a PR with unresolved prior `REQUEST_CHANGES` unless the prior-request-changes override rule is satisfied. + +Do not submit review mutations on skipped PRs. + +## 26A. Terminal review mutation hard-stop + +A live review mutation is terminal for the current reviewer run except for same-PR merge continuation after approval. + +Terminal review mutations include: + +* `REQUEST_CHANGES` +* `APPROVED` +* review comment submission +* PR comment submission used as review feedback +* any tool that records final review decision state +* any tool that changes PR review state +* any tool that consumes a session review-mutation budget + +After `REQUEST_CHANGES` or blocking feedback, stop immediately and produce the final report. + +After `APPROVED`, continue only to merge preflight and merge for the same selected PR, if merge is authorized and all merge gates pass. + +Do not mutate one PR and then continue reviewing, approving, requesting changes, commenting on, or merging another PR in the same run. + +Do not consume a live review mutation on an earlier skipped PR. + +If a live mutation happens on a non-selected or skipped PR, stop immediately and report the workflow violation. + +If a tool blocks further review mutation because a live mutation already occurred, stop immediately. Do not continue validation of another PR. + +The final report must identify: + +* the terminal mutation +* PR number affected by the terminal mutation +* whether same-PR merge continuation was allowed +* whether the run stopped as required + +## 27. Merge rules + +Before merge, rerun fresh live checks: + +* whoami +* active profile/runtime +* exact merge capability +* author safety +* PR re-fetch +* reviewed head SHA unchanged +* target branch freshly fetched +* PR still open +* PR still mergeable +* PR head is not already landed +* required checks still pass +* linked issue handling is correct +* review worktree is clean + +Merge only after all gates pass. + +Do not merge if: + +* MCP reconnect failed +* capability state is stale +* worktree is dirty +* PR head changed +* validation failed +* inventory was incomplete +* PR is already landed +* target branch is stale +* you are the PR author +* prior request-changes override proof is missing +* terminal review mutation occurred on a different PR earlier in the run +* final decision was not marked for the selected PR, if the workflow requires final-decision marking + +If the merge tool requires an explicit confirmation string, report: + +* first gated/no-op call, if any +* confirmation string used +* which call actually performed the merge + +Rejected, dry-run, or confirmation-gated merge calls must not be reported as performed merge mutations. + +## 28. After merge + +Confirm: + +* merge result +* target branch updated +* merged commit/SHA, if available +* selected PR head SHA is now ancestor of target branch +* linked issue status if the PR claims to close an issue + +Clean only the session-owned `branches/` review worktree if the project workflow explicitly allows cleanup. + +Do not delete or mutate unrelated branches/worktrees. + +Do not touch the main checkout except to update the stable branch after merge if explicitly allowed by the workflow. + +Any stable-branch update after merge is a git mutation and must be reported. + +Do not update the main checkout if merge failed, was blocked, or produced reconciliation-only status. + +If any local artifact is created after final cleanup, run and report a new final status check. + +## 29. Recovery handoff rules + +If blocked, produce a recovery handoff with: + +* exact blocker +* failed tool/function, if any +* repo/project +* selected PR, if one was safely selected +* eligibility class +* candidate head SHA, if known +* reviewed head SHA, only if validation and diff review passed +* target branch and target branch SHA, if known +* linked issue status, or `not verified in this session` +* worktree path, if created +* exact state reached before stopping +* safe next action +* statement that no unsafe mutation was attempted + +Blocked handoffs must not include direct approve or merge replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +If the blocker is a terminal review mutation already consumed in the current run, the handoff must say that the next run must start from the beginning and must not reuse stale ready/approved state. + +## 30. Final report must be precise + +Include: + +* canonical workflow source/version/hash, if available +* authenticated identity/profile +* repo/project +* complete PR inventory proof, including pagination/final-page proof +* queue ordering policy used +* selected PR number/title/author +* eligibility class +* author-safety result +* skipped earlier PRs and live blocker proof or prior blocker proof, if any +* prior request-changes proof, if any +* candidate head SHA +* target branch and target branch SHA used for ancestry checks +* reviewed head SHA, only if validation and diff review passed +* already-landed gate result +* linked issue proof/status, if applicable +* main checkout branch +* main checkout dirty state +* review worktree used: true/false +* review worktree path +* review worktree inside `branches/`: true/false +* review worktree HEAD state: detached/branch/none +* review worktree dirty before validation: true/false/not applicable +* review worktree dirty after validation: true/false/not applicable +* baseline validation worktree, if used +* baseline validation result, if used +* files reviewed +* validation commands and results +* official validation integrity status +* diagnostic scratch experiments, if any +* local merge simulation result, if any +* review decision +* terminal review mutation, if any +* merge preflight results, if merge was attempted +* merge result, if merged +* cleanup result +* blockers, if stopped +* confirmation that the main checkout was not used for task work +* final proof that no unsupported proof wording is used + +If the report and actual tool/command log disagree, fix the report before final output. + +## 31. Final report must distinguish mutation types + +Do not use the legacy field `Workspace mutations`. + +Use only precise categories: + +* File edits by reviewer: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Review mutations: +* Merge mutations: +* Cleanup mutations: +* External-state mutations: +* Read-only diagnostics: + +`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics. + +If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`. + +Use precise wording: + +* `File edits by reviewer: none` +* `Worktree/index mutations: ...` +* `Git ref mutations: ...` +* `MCP/Gitea mutations: ...` + +Do not collapse review, merge, cleanup, or external-state mutations into vague wording. + +## 31A. Local artifact and report consistency rule + +Do not create local walkthrough, notes, markdown, JSON, or report artifacts during reviewer runs unless the canonical workflow or operator explicitly requires it. + +If any file is edited, created, generated, or written, report it under `File edits by reviewer`. + +For each file write, report: + +* exact path +* whether it was inside the repo +* whether it was tracked or untracked +* why it was created +* whether final `git status` was run after the write + +Do not say `File edits by reviewer: none` if any file write occurred. + +Do not write files after the final clean-status check unless you rerun and report a new final clean-status check. + +If the action log says `Edited`, `Created`, `Wrote`, or equivalent, the mutation ledger must include that file. + +If the file was created outside the repo, still report it under `File edits by reviewer` and label it `outside repo`. + +If the file was created inside the repo, report whether it is tracked or untracked. + +If an artifact was created only for operator handoff, state that explicitly. + +Default behavior: do not create `walkthrough.md` or similar artifacts unless the workflow explicitly requires it. + +## 32. Controller handoff consistency rule + +The controller handoff must use the same canonical schema and facts as the final report. + +If the already-landed gate fires, the controller handoff must include: + +* Eligibility class: `ALREADY_LANDED_RECONCILE_REQUIRED` +* Candidate head SHA: +* Reviewed head SHA: `none` +* Review worktree used: `false` +* Review worktree path: `none` +* Git ref mutations: +* Review decision: `none` +* Merge result: `none` + +Do not use these legacy fields: + +* `Pinned reviewed head` +* `Scratch worktree used` +* `Workspace mutations` +* `Mutations: None` when any git ref, MCP, Gitea, review, merge, cleanup, or external-state mutation occurred + +The narrative report and controller handoff must agree on: + +* eligibility class +* candidate head SHA +* reviewed head SHA +* mutation state +* worktree usage +* review decision +* terminal review mutation +* merge result +* linked issue status + +If they disagree, fix the report before final output. + +## 33. Forbidden final-report claims unless proven + +Do not claim: + +* `oldest eligible PR` +* `next eligible PR` +* `reviewed head SHA` +* `ready to merge` +* `workspace mutations none` +* `scratch worktree false` +* `all gates passed` +* `inventory complete` +* `already landed` +* `merged` +* `issue closed` +* `manual close required` +* `target branch up to date` +* `read-only merge simulation` +* `no file edits by reviewer` +* `same as master` +* `full-suite failures are pre-existing` +* `prior request-changes resolved` +* `local fallback was safe` +* `live proof` +* `live blocker proof` +* `skipped PR` +* `terminal mutation none` +* `no unsafe mutation` + +unless the corresponding proof is included. + +If anything blocks safe review or merge, stop immediately and produce an executable recovery handoff. + +Do not improvise around the gates. + +Do not preserve stale `ready to merge` state across MCP failures or reconnects. + +Do not include approve or merge replay commands in a blocked handoff. Say to restart the full workflow after the blocker clears. + +## 34. Proof wording enforcement + +The following phrases are forbidden unless directly supported by current-session evidence: + +* live proof +* live blocker proof +* inventory complete +* same as master +* no file edits +* full-suite failures are pre-existing +* issue closed +* merged +* target branch up to date +* all gates passed +* next eligible PR +* oldest eligible PR +* no unsafe mutation +* worktree clean +* linked issue verified +* PR still mergeable +* head unchanged +* final page +* no next page + +If the proof comes from prior review feedback rather than a command/tool run in the current session, label it as prior proof, not live proof. + +If relying on prior review feedback at unchanged head SHA, say: + +`Prior blocker reused; head SHA unchanged since blocking review.` + +Do not say: + +`Live blocker proof` + +unless the blocker was actually revalidated in the current session. + +If a tool call was rejected, confirmation-gated, dry-run-only, or no-op, report it separately from performed mutations. + +A dry run is not a mutation. + +A confirmation-gated rejected call is not a performed mutation. + +A successful merge call is a merge mutation. + +A successful review submission is a review mutation. + +A final-decision marking call is a review mutation if it records durable review state or consumes review-mutation budget. + +## 35. Duplicate request-changes prevention + +Before submitting `REQUEST_CHANGES`, fetch current review feedback. + +If an unresolved `REQUEST_CHANGES` review already exists at the same head SHA with the same blocker, do not submit another `REQUEST_CHANGES`. + +Report: + +* prior blocking reviewer +* prior blocking timestamp +* prior blocking head SHA +* current head SHA +* blocker text +* why the blocker still applies +* that no duplicate review mutation was submitted + +If the head SHA changed or the old blocker is not equivalent, evaluate normally. + +Do not consume the session’s review mutation budget just to duplicate an existing blocker. + +## 36. Final self-check before output + +Before final output, check the report for contradictions. + +Verify: + +* if any file was edited, `File edits by reviewer` is not `none` +* if any worktree was added/removed, `Worktree/index mutations` lists it +* if any fetch happened, `Git ref mutations` lists it +* if any review was submitted, `Review mutations` lists it +* if any merge was performed, `Merge mutations` lists it +* if any cleanup happened, `Cleanup mutations` lists it +* if any issue/PR external state changed, `External-state mutations` lists it +* if pagination is claimed complete, final-page proof is present +* if live proof is claimed, the proof ran in the current session +* if same-as-master is claimed, baseline proof is complete +* if issue closed is claimed, linked issue was fetched after merge +* if merged is claimed, merge result and target ancestry proof are present +* if selected PR is claimed next eligible, every earlier PR has proof-backed skip reasoning +* if a terminal mutation happened, no other PR was reviewed or mutated afterward + +If any contradiction exists, fix the final report before output. + +## 37. Controller handoff schema + +End every run with a controller handoff using this schema. + +Do not omit fields. Use `none` or `not verified in this session` where appropriate. + +Controller Handoff: + +* Task: +* Repo: +* Role: +* Identity: +* Active profile: +* Runtime context: +* Selected PR: +* Linked issue: +* Eligibility class: +* Queue ordering policy: +* Inventory pagination proof: +* Earlier PRs skipped: +* Candidate head SHA: +* Reviewed head SHA: +* Target branch: +* Target branch SHA: +* Already-landed gate: +* Author-safety result: +* Prior request-changes state: +* Review worktree used: +* Review worktree path: +* Review worktree inside branches: +* Review worktree HEAD state: +* Review worktree dirty before validation: +* Review worktree dirty after validation: +* Baseline worktree used: +* Baseline worktree path: +* Files reviewed: +* Validation: +* Official validation integrity status: +* Terminal review mutation: +* Review decision: +* Merge preflight: +* Merge result: +* Linked issue status: +* Main checkout branch: +* Main checkout dirty state: +* Main checkout updated: +* File edits by reviewer: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Review mutations: +* Merge mutations: +* Cleanup mutations: +* External-state mutations: +* Read-only diagnostics: +* Blockers: +* Current status: +* Safe next action: +* Safety statement: + +## 38. Stop conditions summary + +Stop immediately and produce a recovery handoff if: + +* canonical workflow is required but cannot be loaded +* identity/profile/capability cannot be proven +* authenticated identity is the PR author +* runtime context is blocked +* infra stop appears +* MCP reconnect fails +* capability state is stale +* inventory pagination cannot be proven +* queue ordering cannot be proven +* earlier PR blocker status cannot be verified +* selected PR is already landed +* target branch cannot be fetched +* linked issue cannot be verified when required for decision +* review worktree is dirty +* validation cannot run +* validation fails and request-changes cannot be safely submitted +* baseline comparison is required but incomplete +* prior request-changes override proof is missing +* PR head changes before review or merge +* merge preflight fails +* terminal review mutation occurs on a different PR +* any report contradiction cannot be resolved + +Blocked handoffs must not include direct approve or merge replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +Do not improvise around the gates. diff --git a/skills/llm-project-workflow/workflows/work-issue.md b/skills/llm-project-workflow/workflows/work-issue.md new file mode 100644 index 0000000..303b4cb --- /dev/null +++ b/skills/llm-project-workflow/workflows/work-issue.md @@ -0,0 +1,888 @@ +--- +task_mode: work-issue +canonical: true +final_report_schema: ../schemas/work-issue-final-report.md +--- + +# Work issue workflow (canonical) + +**Task mode:** `work-issue` + +This file is the canonical author/coder workflow for Gitea-Tools. Load it +before any issue implementation mutation. Final report schema: +[`schemas/work-issue-final-report.md`](../schemas/work-issue-final-report.md). + +**Default task prompt:** + +> Find the next eligible issue in this project, work on it only if all gates +> pass, and create a PR when complete. + +Do not improvise around the gates. Follow project skills, MCP gates, and +workflow rules exactly. + +This is an author/coder workflow. It is not a reviewer workflow. + +--- + +## 0. Load the canonical workflow first + +Before starting issue work, check whether the project provides a canonical work-on-issue workflow through a project skill, runbook, or MCP helper. + +If available, load it first and report: + +* workflow source +* workflow version, commit, or hash +* whether this prompt conflicts with the loaded workflow + +If the canonical workflow cannot be loaded and the project requires it, stop and produce a recovery handoff only. + +## 1. Mode isolation + +This run is `work-issue` mode only. + +Do not: + +* review PRs +* approve PRs +* request changes +* merge PRs +* close PRs unless the PR creation workflow explicitly does so through Gitea automation +* close unrelated issues +* mutate reviewer state +* perform reviewer-only actions +* create process-hardening issues unless explicitly authorized and the workflow switches to issue-creation mode + +If the task requires review, merge, issue creation, or MCP repair mode, stop and produce a handoff for the correct workflow. + +Do not mix modes in one run. + +## 2. Start with live identity, profile, runtime, and capability checks + +Prove: + +* authenticated identity +* active author/coder profile +* repo/project +* runtime context +* exact capability for reading issues +* exact capability for claiming/locking issues, if available +* exact capability for branch creation, if handled through MCP +* exact capability for pushing branches, if applicable +* exact capability for creating PRs +* exact capability for commenting on issues or PRs, if needed + +A nearby capability does not count. + +Examples: + +* `create_issue` does not authorize `issue_comment` +* `review_pr` does not authorize `merge_pr` +* `create_pr` does not authorize `merge_pr` +* `issue_comment` does not authorize `create_issue` +* `gitea.read` does not authorize issue claim, PR creation, or branch mutation + +If capability cannot be proven, stop and produce a recovery handoff only. + +## 3. Stop immediately on blocked infrastructure + +If any of the following appears, stop immediately: + +* `infra_stop` +* MCP reconnect failure +* stale capability state +* dirty control checkout +* dirty task worktree +* missing capability +* workspace mismatch +* stale target branch state +* broken canonical workflow loading +* failed required preflight +* capability resolver warning that says the current state may be unsafe +* stale or inconsistent runtime context + +Do not continue issue selection, claiming, implementation, validation, commit, push, PR creation, cleanup, or handoff mutation. + +Produce an executable recovery handoff only. + +Blocked recovery handoffs must not include direct commit, push, or PR replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +## 4. Main checkout rule + +The main project checkout must stay on `master`, `main`, or `dev`. + +Do not do task work in the main checkout. + +Do not edit files in the main checkout. + +Do not run tests in the main checkout. + +Do not commit from the main checkout. + +Do not create PRs from the main checkout. + +All task work must happen under the project’s `branches/` directory. + +No exceptions for small fixes, docs, tests, cleanup, conflict resolution, emergencies, or “just one file.” + +If the main checkout is dirty before selection, stop and produce a recovery handoff. + +If the main checkout becomes dirty during the run, stop and produce a recovery handoff unless the change is explicitly allowed by the canonical workflow. + +## 5. No raw MCP repair during normal issue work + +Do not run `pkill`, kill MCP processes, edit MCP config, restart servers, or perform control-checkout repair during normal issue work. + +If MCP repair is required, stop issue work and produce a separate `CONTROL-CHECKOUT REPAIR MODE` handoff. + +Do not mix MCP repair mode with work-on-issue mode. + +Do not use successful repair as permission to resume the same issue workflow. After repair, rerun the full workflow from the beginning. + +## 6. No background task tools + +Do not use `schedule`, `manage_task`, background jobs, async waits, delayed task tools, or monitoring tasks during issue work. + +Use direct commands and MCP tools only. + +If a required action cannot complete synchronously, stop and produce a recovery handoff. + +Long synchronous commands, such as a test suite, are allowed only if they are run directly and reported with exact command, working directory, and result. + +Do not say “I will check later,” “I will monitor,” or “I will continue in the background.” + +## 7. No local Gitea fallback during normal issue work + +During normal author/coder workflows, do not read Gitea profile secret files. + +Do not inspect or open files such as: + +* `profiles.json` +* local token stores +* credential files +* local Gitea auth/profile config files +* `.env` files containing Gitea credentials +* keychain dumps +* token helper outputs + +Do not run local Gitea helper scripts when MCP tools are available. + +Use MCP tools for Gitea operations. + +Local fallback is allowed only in explicit recovery mode when MCP is unavailable and identity/profile/capability can be independently proven. + +If local fallback is used, report: + +* why MCP was unavailable +* exact identity proof +* exact profile proof +* exact repo proof +* exact capability proof +* exact local command used + +Do not use local fallback to bypass MCP gates. + +## 8. Build a complete live issue inventory + +List open issues according to the project’s issue selection policy. + +Follow pagination until the tool proves there are no more pages. + +Do not assume inventory is complete. + +Do not claim `next eligible issue`, `oldest eligible issue`, or complete issue inventory unless pagination is proven. + +Pagination proof must not rely on assumed default API page size. + +Inventory is complete only if one of the following is proven: + +* the MCP response explicitly says there is no next page / `has_more=false` / final page +* the workflow traversed pages until an empty page or explicit final page was returned +* the tool response includes total-count or pagination metadata proving all relevant issues were returned +* the request explicitly set `page` / `limit` / `per_page`, and the response explicitly proves the server honored that page size and did not truncate results + +Do not say “inventory complete” merely because the result count is less than an assumed default page size. + +For each candidate issue, identify: + +* issue number +* title +* labels +* status +* author/requester, if relevant +* assignee/owner, if any +* linked PRs, if any +* dependency/blocker labels, if any +* whether it appears already claimed +* whether it appears already implemented or superseded +* whether it is eligible under project rules + +Final report must include pagination/final-page proof. + +## 9. Issue selection rules + +State the issue ordering policy before selecting an issue. + +If the project uses oldest-first, explicitly sort or reason by issue number or created date. + +Do not rely on API response order unless the tool proves that order matches the project policy. + +Do not pick: + +* already-claimed issues +* issues assigned to another active worker +* issues with an open PR already covering the work +* duplicate issues +* blocked issues +* dependency-blocked issues +* already implemented issues +* issues outside the current requested scope +* process-hardening issues unless this run was explicitly started for process-hardening work +* reviewer-only issues if this is author/coder mode + +For every earlier issue skipped, report: + +* issue number +* current status +* blocking category +* proof used +* whether there is an open PR +* whether there is an active claim +* reason it is not eligible + +If eligibility cannot be proven, classify it as: + +`ISSUE_ELIGIBILITY_UNVERIFIED` + +Then stop or produce a recovery handoff according to project policy. + +Do not select an issue based only on memory from a previous session. + +## 10. Linked PR / duplicate active work proof + +Before claiming or working on an issue, check whether there is already an open PR, branch, or active claim for that issue. + +If an open PR already exists for the issue, do not implement duplicate work. + +Classify the issue as: + +`OPEN_PR_EXISTS` + +and skip it only if project policy allows skipping. + +If branch naming or PR title convention links issues to branches, search for matching branches or PRs. + +Report: + +* issue number +* linked/open PRs found +* matching branches found, if checked +* active claims found +* duplicate work status + +Do not create another branch/PR for the same issue unless the project explicitly allows taking over or updating existing work and exact capability is proven. + +## 11. Claim or lock the issue before implementation + +Claim/lock the issue before implementation if the project provides a claim/lock mechanism. + +If claim/lock requires a Gitea mutation, prove exact capability first. + +If claim/lock fails, stop. + +Do not implement unclaimed work. + +If the claim/lock gates are broken, produce a recovery handoff. + +Create a tooling issue only if this run is explicitly authorized to switch to issue-creation mode and exact `create_issue` capability is proven. + +Report: + +* claim mechanism used +* claim result +* claim timestamp, if available +* issue owner/assignee after claim, if available + +## 12. Refresh stable branch before branch/worktree creation + +Fetch the stable target branch from the remote before creating a task branch or worktree. + +Do not rely on stale local `master`, `main`, or `dev`. + +Record the fetched stable branch SHA. + +If the stable branch cannot be fetched or verified, stop and produce a recovery handoff. + +`git fetch`, `git remote update`, and any command that updates refs must be reported under `Git ref mutations`, not read-only diagnostics. + +## 13. Branch and worktree ownership rule + +Create a fresh session-owned worktree under `branches/`. + +Prefer a branch name that includes the issue number, for example: + +`feat/issue--short-description` + +or: + +`fix/issue--short-description` + +Prefer a worktree path like: + +`branches/issue--short-description` + +Before any file edits, prove: + +* project root +* current working directory +* main checkout branch +* stable branch +* stable branch SHA +* task branch name +* session-owned worktree path +* worktree path is inside `branches/` +* worktree is not the main checkout +* clean tracked state +* clean untracked state +* worktree HEAD/branch state + +Do not reuse an existing worktree unless safe-reuse proof passes. + +Safe-reuse proof must include: + +* exact worktree path +* worktree is inside `branches/` +* worktree is not the main checkout +* worktree is not owned by another active task/session +* clean tracked state +* clean untracked state +* current branch/head before reset +* reset target SHA +* explicit project policy allowing reuse/reset + +Do not run `git reset --hard`, `git clean`, checkout, or other destructive commands unless the worktree is session-owned or safe-reuse proof passes. + +If safe-reuse proof cannot be produced, create a fresh session-owned worktree. + +## 14. Implementation scope rule + +Implement only what is required for the selected issue. + +Do not perform opportunistic refactors. + +Do not fix unrelated tests unless they are required for the selected issue and clearly documented. + +Do not modify reviewer workflow files unless the selected issue explicitly requires workflow changes. + +Do not modify Gitea profiles, MCP authorization, tokens, secrets, deployment config, production config, or credentials unless the selected issue explicitly requires it and exact capability/proof gates pass. + +Do not introduce provenance markers, agent signatures, temporary files, debug dumps, or generated artifacts unless required. + +If implementation uncovers a separate issue, note it in the final report or create a follow-up issue only if exact capability is proven and project policy allows it. + +## 15. File edit rule + +All edits must happen only inside the session-owned issue worktree. + +Do not edit files in the main checkout. + +Do not edit files in reviewer worktrees. + +Do not edit unrelated worktrees. + +Track every edited, created, deleted, or generated file. + +If any file is edited, created, generated, or written, report it under `File edits by author`. + +For each file write, report: + +* exact path +* whether it was inside the repo +* whether it was tracked or untracked +* why it was created +* whether final `git status` was run after the write + +Do not say `File edits by author: none` if any file write occurred. + +Do not write files after the final clean-status check unless you rerun and report a new final clean-status check. + +## 16. Validation rule + +Run appropriate validation for the selected issue. + +Validation may include: + +* targeted tests +* full test suite +* compile checks +* lint checks +* type checks +* diff checks +* secret/provenance checks +* dangerous artifact checks +* project-specific validation + +If validation cannot run, explain why and include the exact failure. + +Do not hide failures. + +Do not claim success if tests failed. + +Do not skip required validation silently. + +Do not bypass MCP gates. + +Report every validation command with: + +* exact command +* working directory +* exit code or pass/fail result +* summary count if available +* whether it was targeted, full-suite, compile, lint, diff, secret/provenance, or diagnostic validation + +If using bare `pytest`, also report: + +* `which pytest` +* `pytest --version` +* whether it resolves to the project venv + +Prefer the project venv executable when available. + +## 17. Baseline comparison rule + +Do not run tests in the main checkout. + +If the full suite fails and you need to prove failures are pre-existing, create a clean baseline worktree under `branches/`, such as: + +`branches/baseline-master-issue-` + +Baseline comparison must include: + +* baseline worktree path +* baseline target SHA +* task branch SHA +* exact command run on both worktrees +* baseline failures +* task branch failures +* proof the failure signatures match +* proof the baseline worktree was clean before and after validation +* proof the issue worktree was clean before and after validation + +Do not claim “same as master” unless the clean baseline worktree proof is included. + +Do not claim “full-suite failures are pre-existing” unless baseline proof is complete and the failure signatures match. + +If full-suite failures differ or proof is incomplete, do not create a PR unless project policy explicitly allows PR creation with documented validation failures. + +## 18. Pre-commit review + +Before committing, review the actual diff. + +Check: + +* correctness +* tests +* scope +* security boundaries +* workflow rule compliance +* whether the implementation really satisfies the selected issue +* unrelated changes +* dangerous generated artifacts +* secrets +* provenance markers +* temporary agent files +* debug output +* formatting-only churn +* docs/tests consistency + +Run: + +* `git status` +* `git diff --stat` +* `git diff` +* project-required diff checks + +Do not commit if unrelated or unsafe changes are present. + +## 19. Commit rules + +Commit only from the session-owned issue worktree. + +Do not commit from the main checkout. + +Commit only after implementation and required validation pass, unless project policy explicitly allows draft PRs with failing validation. + +Commit message must reference the issue number. + +Preferred format: + +`fix: short summary (Closes #)` + +or: + +`feat: short summary (Closes #)` + +Before commit, prove: + +* worktree path +* branch name +* selected issue number +* staged files +* diff summary +* validation status + +After commit, record: + +* commit SHA +* commit message +* changed files + +Do not amend, reset, rebase, squash, or force-push unless the project workflow explicitly allows it and the worktree is session-owned. + +## 20. Push rules + +Push only the session-owned task branch. + +Do not push `master`, `main`, `dev`, tags, or unrelated branches. + +Before push, prove: + +* current branch +* upstream/remote target +* commit SHA being pushed +* selected issue number +* branch name matches the issue + +After push, report: + +* remote +* branch +* pushed commit SHA +* push result + +If push fails, stop and produce a recovery handoff. + +## 21. PR creation rules + +Create a PR only if implementation and validation pass, unless project policy explicitly allows draft PRs with documented validation failures. + +Do not create a PR if: + +* issue was not claimed/locked +* issue eligibility was unproven +* duplicate open PR exists +* task branch does not reference the issue +* implementation is incomplete +* validation failed without allowed exception +* worktree is dirty +* secrets/provenance/dangerous artifacts are present +* capability for PR creation is missing +* runtime context is blocked +* authenticated identity/profile changed unexpectedly + +PR must reference or close the issue. + +PR body must include: + +* summary +* linked issue +* files changed +* validation commands and results +* risk +* exact worktree path +* branch name +* commit SHA +* known limitations, if any + +Do not merge your own PR. + +Do not approve your own PR. + +Do not request changes on your own PR. + +After PR creation, fetch or view the PR to verify: + +* PR number +* PR URL +* PR title +* base branch +* head branch +* linked issue +* head SHA +* open status + +## 22. Cleanup rules + +Clean only session-owned temporary/baseline worktrees if the project workflow explicitly allows cleanup. + +Do not delete unrelated branches/worktrees. + +Do not delete the task worktree if the project expects it to remain for handoff unless policy says cleanup is allowed after PR creation. + +Do not update the main checkout unless the canonical workflow explicitly allows it. + +Any cleanup is a mutation and must be reported. + +## 23. Recovery handoff rules + +If blocked, produce a recovery handoff with: + +* exact blocker +* failed tool/function, if any +* repo/project +* selected issue, if one was safely selected +* eligibility class +* claim/lock state +* branch name, if created +* worktree path, if created +* stable branch and stable branch SHA, if known +* files changed, if any +* validation state +* commit SHA, if committed +* PR number/URL, if created +* exact state reached before stopping +* safe next action +* statement that no unsafe mutation was attempted + +Blocked handoffs must not include direct commit, push, or PR replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +## 24. Final report must be precise + +Include: + +* canonical workflow source/version/hash, if available +* authenticated identity/profile +* repo/project +* capability proof summary +* issue inventory proof, including pagination/final-page proof +* issue ordering policy used +* selected issue number/title +* eligibility class +* skipped earlier issues and proof, if any +* duplicate active work proof +* claim/lock result +* stable branch and stable branch SHA +* branch name +* worktree path +* worktree inside `branches/`: true/false +* worktree branch/HEAD state +* worktree dirty before implementation: true/false +* files changed +* validation commands and results +* baseline comparison result, if used +* pre-commit diff review result +* commit SHA and commit message, if committed +* push result, if pushed +* PR number and URL, if created +* PR verification result, if created +* cleanup result +* blockers, if stopped +* confirmation that the main checkout was not used for task work + +If the report and actual tool/command log disagree, fix the report before final output. + +## 25. Final report must distinguish mutation types + +Do not use the legacy field `Workspace mutations`. + +Use only precise categories: + +* File edits by author: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Issue mutations: +* Branch mutations: +* Commit mutations: +* Push mutations: +* PR mutations: +* Cleanup mutations: +* External-state mutations: +* Read-only diagnostics: + +`git fetch`, `git remote update`, and any command that updates refs must be listed under `Git ref mutations`, not read-only diagnostics. + +If `git reset --hard`, checkout, clean, worktree add/remove, merge simulation, merge abort, or similar commands occurred, report them under `Worktree/index mutations`. + +Use precise wording: + +* `File edits by author: none` +* `Worktree/index mutations: ...` +* `Git ref mutations: ...` +* `MCP/Gitea mutations: ...` + +Do not collapse issue, branch, commit, push, PR, cleanup, or external-state mutations into vague wording. + +## 26. Forbidden final-report claims unless proven + +Do not claim: + +* `next eligible issue` +* `oldest eligible issue` +* `issue claimed` +* `no duplicate work` +* `no open PR` +* `worktree clean` +* `validation passed` +* `same as master` +* `full-suite failures are pre-existing` +* `committed` +* `pushed` +* `PR created` +* `issue closed` +* `main checkout untouched` +* `no file edits` +* `no unsafe mutation` +* `all gates passed` +* `target branch up to date` + +unless the corresponding proof is included. + +If anything blocks safe work or PR creation, stop immediately and produce an executable recovery handoff. + +Do not improvise around the gates. + +## 27. Proof wording enforcement + +The following phrases are forbidden unless directly supported by current-session evidence: + +* next eligible issue +* oldest eligible issue +* inventory complete +* no duplicate work +* issue claimed +* worktree clean +* validation passed +* same as master +* full-suite failures are pre-existing +* committed +* pushed +* PR created +* issue closed +* target branch up to date +* all gates passed +* no unsafe mutation +* no file edits + +If the proof comes from prior state rather than a command/tool run in the current session, label it as prior proof, not live proof. + +If a tool call was rejected, confirmation-gated, dry-run-only, or no-op, report it separately from performed mutations. + +## 28. Final self-check before output + +Before final output, check the report for contradictions. + +Verify: + +* if any file was edited, `File edits by author` is not `none` +* if any worktree was added/removed, `Worktree/index mutations` lists it +* if any fetch happened, `Git ref mutations` lists it +* if any issue was claimed/commented/updated, `Issue mutations` lists it +* if any branch was created, `Branch mutations` lists it +* if any commit was created, `Commit mutations` lists it +* if any push occurred, `Push mutations` lists it +* if any PR was created, `PR mutations` lists it +* if any cleanup happened, `Cleanup mutations` lists it +* if any issue/PR external state changed, `External-state mutations` lists it +* if pagination is claimed complete, final-page proof is present +* if same-as-master is claimed, baseline proof is complete +* if selected issue is claimed next eligible, every earlier issue has proof-backed skip reasoning +* if PR created is claimed, PR verification proof is present +* if main checkout untouched is claimed, main checkout status proof is present + +If any contradiction exists, fix the final report before output. + +## 29. Controller handoff schema + +End every run with a controller handoff using this schema. + +Do not omit fields. Use `none` or `not verified in this session` where appropriate. + +Controller Handoff: + +* Task: +* Repo: +* Role: +* Identity: +* Active profile: +* Runtime context: +* Selected issue: +* Eligibility class: +* Issue ordering policy: +* Issue inventory pagination proof: +* Earlier issues skipped: +* Duplicate active work proof: +* Claim/lock state: +* Stable branch: +* Stable branch SHA: +* Branch name: +* Worktree path: +* Worktree inside branches: +* Worktree branch/HEAD state: +* Worktree dirty before implementation: +* Files changed: +* Validation: +* Baseline comparison: +* Commit SHA: +* Push result: +* PR number: +* PR URL: +* PR verification: +* Main checkout branch: +* Main checkout dirty state: +* Main checkout used for task work: +* File edits by author: +* Worktree/index mutations: +* Git ref mutations: +* MCP/Gitea mutations: +* Issue mutations: +* Branch mutations: +* Commit mutations: +* Push mutations: +* PR mutations: +* Cleanup mutations: +* External-state mutations: +* Read-only diagnostics: +* Blockers: +* Current status: +* Safe next action: +* Safety statement: + +## 30. Stop conditions summary + +Stop immediately and produce a recovery handoff if: + +* canonical workflow is required but cannot be loaded +* identity/profile/capability cannot be proven +* runtime context is blocked +* infra stop appears +* MCP reconnect fails +* capability state is stale +* issue inventory pagination cannot be proven +* issue ordering cannot be proven +* issue eligibility cannot be proven +* duplicate active work cannot be checked +* selected issue is already claimed by another worker +* selected issue already has an open PR +* claim/lock fails +* stable branch cannot be fetched +* task worktree cannot be created safely +* task worktree is dirty before implementation +* validation cannot run +* validation fails without allowed exception +* baseline comparison is required but incomplete +* diff review finds unrelated or unsafe changes +* commit fails +* push fails +* PR creation fails +* PR verification fails +* any report contradiction cannot be resolved + +Blocked handoffs must not include direct commit, push, or PR replay commands. + +Blocked handoffs must say to rerun the full workflow after the blocker clears. + +Do not improvise around the gates. diff --git a/subagent_gate.py b/subagent_gate.py new file mode 100644 index 0000000..f3fc18e --- /dev/null +++ b/subagent_gate.py @@ -0,0 +1,166 @@ +"""Fail-closed subagent delegation gates (#266). + +Subagents can bypass or lose context for worktree, capability, mutation, +retry, and reporting rules. These helpers make delegation an explicit, +provable decision instead of a default: deterministic write tasks stay +inline unless the parent session records why a subagent is needed and +hands the subagent the full gate context it must operate under. + +Like ``author_proofs``/``review_proofs``, the helpers are pure (no git, +no API calls): the parent workflow gathers the facts and passes them in, +so the same logic works from prompts, harness assertions, and tests. +Nothing here weakens the review/merge/permission gates — a delegated +subagent is subject to the same gates as its parent. +""" + +# AC1: deterministic write workflows a subagent must never run by default. +DETERMINISTIC_WRITE_TASKS = frozenset({ + "claim_issue", + "create_branch", + "edit_code", + "commit", + "push_branch", + "create_pr", + "review_pr", + "merge_pr", + "cleanup_branch", + "close_issue", +}) + +# Read-only delegation that needs no explicit authorization. +READ_ONLY_TASKS = frozenset({ + "read_files", + "code_search", + "inventory_prs", + "summarize_issue", + "explore_codebase", +}) + +# AC3: context a subagent must inherit from the parent session before any +# authorized write delegation may proceed. +REQUIRED_INHERITED_CONTEXT = ( + "issue_lock", + "branch", + "worktree_path", + "identity_profile", + "allowed_tool_class", + "command_deny_list", + "validation_ledger_requirement", + "final_report_schema", +) + +# AC4: proof fields a subagent final report must carry — the same fields a +# parent workflow's final report requires. +REQUIRED_SUBAGENT_REPORT_FIELDS = ( + "identity_profile", + "worktree_path", + "branch", + "changed_files", + "validation_results", + "workspace_mutations", +) + + +def _clean(value): + return (value or "").strip() if isinstance(value, str) else value + + +def _classify_task(task_type): + task = _clean(task_type) + if not task: + return "", "unknown" + if task in DETERMINISTIC_WRITE_TASKS: + return task, "deterministic_write" + if task in READ_ONLY_TASKS: + return task, "read_only" + return task, "unknown" + + +def _missing_context_fields(inherited_context): + context = inherited_context or {} + missing = [] + for field in REQUIRED_INHERITED_CONTEXT: + value = context.get(field) + if not isinstance(value, str) or not value.strip(): + missing.append(field) + return missing + + +def assess_subagent_delegation(task_type, *, explicitly_allowed=False, + justification=None, inherited_context=None): + """Decide whether delegating *task_type* to a subagent may proceed. + + Fail closed: unknown tasks are blocked; deterministic write tasks are + blocked unless explicitly allowed (AC1) with a recorded justification + (AC2) and the full inherited gate context (AC3). Read-only delegation + is allowed without explicit authorization. + + Returns {'block', 'allowed', 'task_type', 'task_class', 'reasons', + 'missing_context'}. + """ + task, task_class = _classify_task(task_type) + reasons = [] + missing_context = [] + + if task_class == "unknown": + reasons.append( + f"task type '{task}' is not a recognized delegation class; " + "run it inline in the parent session (fail closed)" + ) + elif task_class == "deterministic_write": + if not explicitly_allowed: + reasons.append( + f"deterministic write task '{task}' must run inline unless " + "subagent use is explicitly allowed by the parent session" + ) + if not _clean(justification): + reasons.append( + "no recorded justification for why a subagent is needed; " + "the parent session must record one before delegating" + ) + missing_context = _missing_context_fields(inherited_context) + if missing_context: + reasons.append( + "subagent would not inherit required gate context: " + + ", ".join(missing_context) + ) + + block = bool(reasons) + return { + "block": block, + "allowed": not block, + "task_type": task, + "task_class": task_class, + "reasons": reasons, + "missing_context": missing_context, + } + + +def validate_subagent_report(report_fields): + """AC4: accept subagent output only with the parent-grade proof fields. + + *report_fields* maps field name -> reported value. Missing or blank + proof fields make the report invalid (fail closed). + + Returns {'valid', 'block', 'reasons', 'missing_fields'}. + """ + reasons = [] + report = report_fields or {} + missing = [] + for field in REQUIRED_SUBAGENT_REPORT_FIELDS: + value = report.get(field) + if not isinstance(value, str) or not value.strip(): + missing.append(field) + if missing: + reasons.append( + "subagent final report missing required proof fields: " + + ", ".join(missing) + ) + + valid = not reasons + return { + "valid": valid, + "block": not valid, + "reasons": reasons, + "missing_fields": missing, + } diff --git a/task_capability_map.py b/task_capability_map.py index 7607170..9d162c1 100644 --- a/task_capability_map.py +++ b/task_capability_map.py @@ -28,6 +28,10 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.issue.comment", "role": "author", }, + "lock_issue": { + "permission": "gitea.issue.comment", + "role": "author", + }, "set_issue_labels": { "permission": "gitea.issue.comment", "role": "author", @@ -80,6 +84,14 @@ TASK_CAPABILITY_MAP: dict[str, dict[str, str]] = { "permission": "gitea.branch.delete", "role": "author", }, + "commit_files": { + "permission": "gitea.repo.commit", + "role": "author", + }, + "gitea_commit_files": { + "permission": "gitea.repo.commit", + "role": "author", + }, "reconcile_merged_cleanups": { "permission": "gitea.read", "role": "author", @@ -93,6 +105,7 @@ ISSUE_MUTATION_TOOL_TASKS: dict[str, str] = { "gitea_create_issue_comment": "comment_issue", "gitea_mark_issue": "mark_issue", "gitea_set_issue_labels": "set_issue_labels", + "gitea_commit_files": "commit_files", } @@ -114,4 +127,4 @@ def required_role(task: str) -> str: def tool_required_permission(tool_name: str) -> str: """Return the operation an issue-mutating tool must gate on.""" - return required_permission(ISSUE_MUTATION_TOOL_TASKS[tool_name]) \ No newline at end of file + return required_permission(ISSUE_MUTATION_TOOL_TASKS[tool_name]) diff --git a/tests/test_agent_temp_artifacts.py b/tests/test_agent_temp_artifacts.py new file mode 100644 index 0000000..2e14385 --- /dev/null +++ b/tests/test_agent_temp_artifacts.py @@ -0,0 +1,87 @@ +"""Tests for agent temp artifact detection and preflight warnings (#261).""" +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import agent_temp_artifacts +import mcp_server + + +class TestAgentTempArtifactPatterns(unittest.TestCase): + def test_detects_root_level_encode_emit_inline(self): + porcelain = ( + "?? _encode_commit_payload.py\n" + "?? _emit_payload.py\n" + "?? _inline_b64.py\n" + ) + self.assertEqual( + agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(porcelain), + ["_emit_payload.py", "_encode_commit_payload.py", "_inline_b64.py"], + ) + + def test_ignores_nested_and_unrelated_untracked(self): + porcelain = ( + "?? scripts/_encode_x.py\n" + "?? README-draft.md\n" + " M task_capability_map.py\n" + ) + self.assertEqual( + agent_temp_artifacts.find_agent_temp_artifacts_from_porcelain(porcelain), + [], + ) + + +class TestPreflightWarnings(unittest.TestCase): + def setUp(self): + self._snapshot = ( + mcp_server._preflight_whoami_called, + mcp_server._preflight_capability_called, + ) + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + + def tearDown(self): + mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = ( + self._snapshot + ) + + @patch( + "mcp_server._get_workspace_porcelain", + return_value="?? _encode_commit_payload.py\n", + ) + def test_assess_preflight_surfaces_warning_not_block(self, _porcelain): + status = mcp_server.assess_preflight_status() + self.assertTrue(status["preflight_ready"]) + self.assertEqual(status["preflight_block_reasons"], []) + self.assertEqual(len(status["preflight_warnings"]), 1) + self.assertIn("_encode_commit_payload.py", status["preflight_warnings"][0]) + + +class TestIssueLockArtifactWarning(unittest.TestCase): + @patch("mcp_server.api_get_all", return_value=[]) + @patch("mcp_server._auth", return_value="token x") + @patch("mcp_server._resolve", return_value=("h", "o", "r")) + @patch("mcp_server.ISSUE_LOCK_FILE", new_callable=lambda: tempfile.mktemp()) + @patch("issue_lock_worktree.read_worktree_git_state") + def test_lock_success_includes_artifact_warning(self, mock_state, _lock_file, *_mocks): + mock_state.return_value = { + "current_branch": "master", + "porcelain_status": "?? _emit_payload.py\n", + } + result = mcp_server.gitea_lock_issue( + issue_number=261, + branch_name="docs/issue-261-agent-artifact-cleanup", + remote="prgs", + ) + self.assertTrue(result["success"]) + self.assertIn("warnings", result) + self.assertIn("_emit_payload.py", result["warnings"][0]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_commit_files_capability.py b/tests/test_commit_files_capability.py new file mode 100644 index 0000000..d1203bd --- /dev/null +++ b/tests/test_commit_files_capability.py @@ -0,0 +1,199 @@ +"""Tests for commit_files task capability resolver mapping (#262).""" +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import mcp_server +import task_capability_map + +CONFIG = { + "version": 2, + "contexts": { + "ctx": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.example.com"}, + } + }, + "profiles": { + "commit-author": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "author-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": ["gitea.read", "gitea.repo.commit"], + "forbidden_operations": ["gitea.pr.create"], + "execution_profile": "commit-author", + }, + "pr-only-author": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "pr-author", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": ["gitea.read", "gitea.pr.create"], + "forbidden_operations": ["gitea.repo.commit"], + "execution_profile": "pr-only-author", + }, + "reviewer-profile": { + "enabled": True, + "context": "ctx", + "role": "reviewer", + "username": "reviewer-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, + "allowed_operations": ["gitea.read", "gitea.pr.review", "gitea.pr.merge"], + "forbidden_operations": [ + "gitea.repo.commit", "gitea.pr.create", "gitea.branch.push", + ], + "execution_profile": "reviewer-profile", + }, + }, + "rules": {"allow_runtime_switching": False}, +} + + +class CommitFilesCapabilityBase(unittest.TestCase): + def setUp(self): + self._preflight_snapshot = ( + mcp_server._preflight_whoami_called, + mcp_server._preflight_capability_called, + ) + mcp_server._preflight_whoami_called = False + mcp_server._preflight_capability_called = False + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + mcp_server._preflight_whoami_called, mcp_server._preflight_capability_called = ( + self._preflight_snapshot + ) + self._dir.cleanup() + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + "GITEA_TOKEN_REVIEWER": "reviewer-pass", + } + + +class TestCommitFilesResolver(CommitFilesCapabilityBase): + @patch("mcp_server.api_request", return_value={"login": "author-user"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_resolves_to_repo_commit(self, _auth, _api): + with patch.dict(os.environ, self._env("commit-author"), clear=True): + for task in ("commit_files", "gitea_commit_files"): + with self.subTest(task=task): + res = mcp_server.gitea_resolve_task_capability( + task=task, remote="prgs") + self.assertEqual(res["required_operation_permission"], + "gitea.repo.commit") + self.assertEqual(res["required_role_kind"], "author") + self.assertTrue(res["allowed_in_current_session"]) + + @patch("mcp_server.api_request", return_value={"login": "pr-author"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_create_pr_does_not_satisfy_commit_files(self, _auth, _api): + with patch.dict(os.environ, self._env("pr-only-author"), clear=True): + commit_res = mcp_server.gitea_resolve_task_capability( + task="commit_files", remote="prgs") + pr_res = mcp_server.gitea_resolve_task_capability( + task="create_pr", remote="prgs") + self.assertFalse(commit_res["allowed_in_current_session"]) + self.assertTrue(pr_res["allowed_in_current_session"]) + self.assertEqual(commit_res["required_operation_permission"], + "gitea.repo.commit") + self.assertEqual(pr_res["required_operation_permission"], + "gitea.pr.create") + + @patch("mcp_server.api_request", return_value={"login": "reviewer-user"}) + @patch("mcp_server.get_auth_header", return_value="token reviewer-pass") + def test_reviewer_denied_commit_files(self, _auth, _api): + with patch.dict(os.environ, self._env("reviewer-profile"), clear=True): + res = mcp_server.gitea_resolve_task_capability( + task="commit_files", remote="prgs") + self.assertFalse(res["allowed_in_current_session"]) + self.assertEqual(res["required_operation_permission"], + "gitea.repo.commit") + self.assertTrue(res["different_mcp_namespace_required"]) + + +class TestCommitFilesToolGate(CommitFilesCapabilityBase): + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_allowed_author_proceeds(self, _auth, mock_api, _role): + mock_api.return_value = { + "commit": {"sha": "abc"}, + "branch": {"name": "feat/x"}, + } + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + with patch.dict(os.environ, self._env("commit-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "a.txt", "content": "YQ=="}], + message="test", + new_branch="feat/x", + remote="prgs", + ) + self.assertTrue(res["success"]) + + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_reviewer_blocked_with_permission_report(self, _auth, _role): + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + with patch.dict(os.environ, self._env("reviewer-profile"), clear=True): + res = mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "a.txt", "content": "YQ=="}], + message="test", + remote="prgs", + ) + self.assertFalse(res["success"]) + self.assertFalse(res.get("performed", True)) + self.assertIn("permission_report", res) + self.assertEqual(res["permission_report"]["missing_permission"], + "gitea.repo.commit") + + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + def test_preflight_blocks_before_api(self, _role): + env = {**self._env("commit-author"), "GITEA_TEST_FORCE_DIRTY": "1"} + with patch.dict(os.environ, env, clear=True): + with self.assertRaises(RuntimeError) as ctx: + mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "a.txt", "content": "YQ=="}], + message="test", + remote="prgs", + ) + self.assertIn("gitea_whoami", str(ctx.exception)) + + +class TestCommitFilesMapAlignment(unittest.TestCase): + def test_tool_gate_matches_resolver(self): + self.assertEqual( + task_capability_map.tool_required_permission("gitea_commit_files"), + task_capability_map.required_permission("commit_files"), + ) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_commit_files_gate.py b/tests/test_commit_files_gate.py new file mode 100644 index 0000000..4c72d46 --- /dev/null +++ b/tests/test_commit_files_gate.py @@ -0,0 +1,251 @@ +"""Regression tests: gitea_commit_files tool gates match resolver and whoami verification. + +Covers Issue #262 requirements. +""" +import json +import os +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import mcp_server +import task_capability_map +import gitea_config + +CONFIG = { + "version": 2, + "contexts": { + "ctx": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.example.com"}, + } + }, + "profiles": { + "full-author": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "author-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": [ + "gitea.read", "gitea.issue.create", "gitea.repo.commit" + ], + "forbidden_operations": [], + "execution_profile": "full-author", + }, + "reviewer-no-commit": { + "enabled": True, + "context": "ctx", + "role": "reviewer", + "username": "reviewer-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_REVIEWER"}, + "allowed_operations": [ + "gitea.read", "gitea.pr.review", "gitea.pr.approve" + ], + "forbidden_operations": [ + "gitea.repo.commit", "gitea.pr.create", "gitea.branch.push" + ], + "execution_profile": "reviewer-no-commit", + }, + }, + "rules": {"allow_runtime_switching": False}, +} + + +class TestCommitFilesGate(unittest.TestCase): + def setUp(self): + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + gitea_config._active_profile_override = None + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + gitea_config._active_profile_override = None + self._dir.cleanup() + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + "GITEA_TOKEN_REVIEWER": "reviewer-pass", + } + + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_allowed_author_proceeds(self, _auth, mock_api, _role): + mock_api.return_value = { + "commit": {"sha": "abc123commit"}, + "branch": {"name": "some-branch"}, + } + with patch.dict(os.environ, self._env("full-author"), clear=True): + resolve = mcp_server.gitea_resolve_task_capability( + task="commit_files", remote="prgs" + ) + self.assertTrue(resolve["allowed_in_current_session"]) + + res = mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "x.txt", "content": "YQ=="}], + message="Add x", + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertEqual(res["commit"], "abc123commit") + + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token reviewer-pass") + def test_denied_reviewer_blocked(self, _auth, mock_api, _role): + mock_api.return_value = {"login": "reviewer-user"} + with patch.dict(os.environ, self._env("reviewer-no-commit"), clear=True): + resolve = mcp_server.gitea_resolve_task_capability( + task="commit_files", remote="prgs" + ) + self.assertFalse(resolve["allowed_in_current_session"]) + + res = mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "x.txt", "content": "YQ=="}], + message="Add x", + remote="prgs", + ) + self.assertFalse(res["success"]) + self.assertFalse(res.get("performed", True)) + self.assertIn("permission_report", res) + self.assertEqual( + res["permission_report"]["missing_permission"], "gitea.repo.commit" + ) + post_calls = [c for c in mock_api.call_args_list if len(c.args) > 0 and c.args[0] == "POST"] + self.assertFalse(post_calls) + + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token reviewer-pass") + def test_unknown_profile_fails_closed(self, _auth, mock_api, _role): + mock_api.return_value = {"login": "reviewer-user"} + with patch.dict(os.environ, self._env("non-existent"), clear=True): + res = mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "x.txt", "content": "YQ=="}], + message="Add x", + remote="prgs", + ) + self.assertFalse(res["success"]) + self.assertFalse(res.get("performed", True)) + post_calls = [c for c in mock_api.call_args_list if len(c.args) > 0 and c.args[0] == "POST"] + self.assertFalse(post_calls) + + +class TestPreflightCommitFilesGate(unittest.TestCase): + def setUp(self): + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + + self.orig_whoami_called = mcp_server._preflight_whoami_called + self.orig_capability_called = mcp_server._preflight_capability_called + self.orig_whoami_violation = mcp_server._preflight_whoami_violation + self.orig_capability_violation = mcp_server._preflight_capability_violation + self.orig_resolved_role = mcp_server._preflight_resolved_role + self.orig_process_start = mcp_server._process_start_porcelain + self.orig_whoami_baseline = mcp_server._preflight_whoami_baseline_porcelain + self.orig_capability_baseline = mcp_server._preflight_capability_baseline_porcelain + self.orig_whoami_files = mcp_server._preflight_whoami_violation_files + self.orig_capability_files = mcp_server._preflight_capability_violation_files + self.orig_reviewer_files = mcp_server._preflight_reviewer_violation_files + + mcp_server._preflight_whoami_called = False + mcp_server._preflight_capability_called = False + mcp_server._preflight_whoami_violation = False + mcp_server._preflight_capability_violation = False + mcp_server._preflight_resolved_role = None + mcp_server._process_start_porcelain = "" + mcp_server._preflight_whoami_baseline_porcelain = None + mcp_server._preflight_capability_baseline_porcelain = None + mcp_server._preflight_whoami_violation_files = [] + mcp_server._preflight_capability_violation_files = [] + mcp_server._preflight_reviewer_violation_files = [] + + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + + mcp_server._preflight_whoami_called = self.orig_whoami_called + mcp_server._preflight_capability_called = self.orig_capability_called + mcp_server._preflight_whoami_violation = self.orig_whoami_violation + mcp_server._preflight_capability_violation = self.orig_capability_violation + mcp_server._preflight_resolved_role = self.orig_resolved_role + mcp_server._process_start_porcelain = self.orig_process_start + mcp_server._preflight_whoami_baseline_porcelain = self.orig_whoami_baseline + mcp_server._preflight_capability_baseline_porcelain = self.orig_capability_baseline + mcp_server._preflight_whoami_violation_files = self.orig_whoami_files + mcp_server._preflight_capability_violation_files = self.orig_capability_files + mcp_server._preflight_reviewer_violation_files = self.orig_reviewer_files + + self._dir.cleanup() + os.environ.pop("GITEA_TEST_FORCE_DIRTY", None) + os.environ.pop("GITEA_TEST_PORCELAIN", None) + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + } + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_preflight_not_called_blocks_commit(self, _auth, mock_api): + mock_api.return_value = {"login": "author-user"} + with patch.dict(os.environ, self._env("full-author"), clear=True): + os.environ["GITEA_TEST_PORCELAIN"] = "" + with self.assertRaises(RuntimeError) as ctx: + mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "x.txt", "content": "YQ=="}], + message="Add x", + remote="prgs", + ) + self.assertIn("Identity (gitea_whoami) has not been verified", str(ctx.exception)) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_dirty_workspace_before_whoami_blocks_commit(self, _auth, mock_api): + mock_api.return_value = {"login": "author-user"} + with patch.dict(os.environ, self._env("full-author"), clear=True): + os.environ["GITEA_TEST_FORCE_DIRTY"] = "1" + mcp_server.record_preflight_check("whoami") + mcp_server.record_preflight_check("capability", resolved_role="author") + + with self.assertRaises(RuntimeError) as ctx: + mcp_server.gitea_commit_files( + files=[{"operation": "create", "path": "x.txt", "content": "YQ=="}], + message="Add x", + remote="prgs", + ) + self.assertIn("Workspace file edits occurred before gitea_whoami verification", str(ctx.exception)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_commit_payloads.py b/tests/test_commit_payloads.py new file mode 100644 index 0000000..1b3a80d --- /dev/null +++ b/tests/test_commit_payloads.py @@ -0,0 +1,251 @@ +import json +import os +import tempfile +import unittest +from unittest.mock import MagicMock, patch + +import gitea_config +import mcp_server +import task_capability_map + +CONFIG = { + "version": 2, + "contexts": { + "ctx": { + "enabled": True, + "gitea": {"enabled": True, "base_url": "https://gitea.example.com"}, + } + }, + "profiles": { + "full-author": { + "enabled": True, + "context": "ctx", + "role": "author", + "username": "author-user", + "auth": {"type": "env", "name": "GITEA_TOKEN_AUTHOR"}, + "allowed_operations": [ + "gitea.read", + "gitea.repo.commit", + "gitea.pr.create", + "gitea.branch.push", + ], + "forbidden_operations": [ + "gitea.pr.approve", + "gitea.pr.merge", + "gitea.pr.review", + ], + "execution_profile": "full-author", + }, + }, +} + + +class TestCommitPayloads(unittest.TestCase): + def setUp(self): + self._remotes = patch.dict(mcp_server.REMOTES, { + "prgs": {"host": "gitea.example.com", "org": "Example-Org", + "repo": "Example-Repo"}, + }) + self._remotes.start() + mcp_server._IDENTITY_CACHE.clear() + + # Setup temp directories and lock files + self._dir = tempfile.TemporaryDirectory() + self.config_path = os.path.join(self._dir.name, "profiles.json") + with open(self.config_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(CONFIG)) + + self.locked_worktree_dir = tempfile.TemporaryDirectory() + self.locked_worktree_path = os.path.realpath(self.locked_worktree_dir.name) + + self.lock_file_path = "/tmp/gitea_issue_lock.json" + self.lock_data = { + "issue_number": 263, + "branch_name": "feat/issue-263-native-commit-payloads", + "remote": "prgs", + "org": "Example-Org", + "repo": "Example-Repo", + "worktree_path": self.locked_worktree_path, + } + with open(self.lock_file_path, "w", encoding="utf-8") as fh: + fh.write(json.dumps(self.lock_data)) + + # Reset preflight status to bypass/pass verification in tests + self.orig_whoami_called = mcp_server._preflight_whoami_called + self.orig_capability_called = mcp_server._preflight_capability_called + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + + def tearDown(self): + self._remotes.stop() + mcp_server._IDENTITY_CACHE.clear() + + mcp_server._preflight_whoami_called = self.orig_whoami_called + mcp_server._preflight_capability_called = self.orig_capability_called + + self._dir.cleanup() + self.locked_worktree_dir.cleanup() + if os.path.exists(self.lock_file_path): + os.remove(self.lock_file_path) + + def _env(self, profile: str) -> dict: + return { + "GITEA_MCP_CONFIG": self.config_path, + "GITEA_MCP_PROFILE": profile, + "GITEA_TOKEN_AUTHOR": "author-pass", + "GITEA_TEST_PORCELAIN": "", + } + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_content_plain(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-123"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hello.txt", + "content_plain": "Hello World!", + } + ], + message="Add hello.txt", + remote="prgs", + ) + print("DEBUG RES IS:", res) + self.assertTrue(res["success"]) + self.assertEqual(res["commit"], "sha-123") + self.assertEqual(res["content_source_proof"][0]["source"], "inline_plain") + + # Verify posted base64 content + post_args = mock_api.call_args[0] + payload = post_args[3] + self.assertEqual(payload["files"][0]["content"], "SGVsbG8gV29ybGQh") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_workspace_path(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-456"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + + # Create a workspace file + file_relative = "src/code.py" + file_abs = os.path.join(self.locked_worktree_path, file_relative) + os.makedirs(os.path.dirname(file_abs), exist_ok=True) + with open(file_abs, "wb") as fh: + fh.write(b"print('hello')") + + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "src/code.py", + "workspace_path": file_relative, + } + ], + message="Add code.py", + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertEqual(res["content_source_proof"][0]["source"], "workspace_path") + + post_args = mock_api.call_args[0] + payload = post_args[3] + # "print('hello')" in base64 is cHJpbnQoJ2hlbGxvJyk= + self.assertEqual(payload["files"][0]["content"], "cHJpbnQoJ2hlbGxvJyk=") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_local_path(self, _auth, mock_api): + mock_api.return_value = { + "commit": {"sha": "sha-789"}, + "branch": {"name": "feat/issue-263-native-commit-payloads"} + } + + file_abs = os.path.join(self.locked_worktree_path, "local.bin") + with open(file_abs, "wb") as fh: + fh.write(b"\x00\x01\x02\x03\xff") + + with patch.dict(os.environ, self._env("full-author"), clear=True): + res = mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "local.bin", + "local_path": file_abs, + } + ], + message="Add local.bin", + remote="prgs", + ) + self.assertTrue(res["success"]) + self.assertEqual(res["content_source_proof"][0]["source"], "local_path") + + post_args = mock_api.call_args[0] + payload = post_args[3] + # b"\x00\x01\x02\x03\xff" in base64 is AAECA/8= + self.assertEqual(payload["files"][0]["content"], "AAECA/8=") + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_traversal_blocked(self, _auth, mock_api): + # Remove active lock file to ensure it fails on traversal/invalid locks + os.remove(self.lock_file_path) + + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(RuntimeError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hack.txt", + "workspace_path": "any.txt", + } + ], + message="Add hack", + remote="prgs", + ) + self.assertIn("Issue lock is missing", str(ctx.exception)) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_outside_scope_blocked(self, _auth, mock_api): + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(ValueError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hack.txt", + "workspace_path": "../outside.txt", + } + ], + message="Add hack", + remote="prgs", + ) + self.assertIn("falls outside of locked worktree", str(ctx.exception)) + + @patch("mcp_server.api_request") + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_commit_files_multiple_sources_blocked(self, _auth, mock_api): + with patch.dict(os.environ, self._env("full-author"), clear=True): + with self.assertRaises(ValueError) as ctx: + mcp_server.gitea_commit_files( + files=[ + { + "operation": "create", + "path": "hello.txt", + "content_plain": "Hello!", + "content": "SGVsbG8=", + } + ], + message="Add hello", + remote="prgs", + ) + self.assertIn("Multiple content sources specified", str(ctx.exception)) diff --git a/tests/test_identity_disclosure.py b/tests/test_identity_disclosure.py new file mode 100644 index 0000000..6b12e3d --- /dev/null +++ b/tests/test_identity_disclosure.py @@ -0,0 +1,102 @@ +"""Identity-disclosure checks for workflow reports (#305). + +Workflow reports sometimes included the authenticated user's personal +email even though username/profile identity is sufficient (observed: +``Identity: jcwalker3 / jcwalker3@yahoo.com`` in a reconciliation +handoff). These tests pin the no-email identity summary helper and the +final-report validator that flags unnecessary email disclosure. +""" +import sys +import unittest + +sys.path.insert(0, str(__import__("pathlib").Path(__file__).resolve().parent.parent)) + +import review_proofs + + +class TestIdentitySummary(unittest.TestCase): + def test_author_summary_uses_username_and_profile_only(self): + summary = review_proofs.format_identity_summary( + "jcwalker3", "prgs-author") + self.assertEqual(summary, "jcwalker3 / prgs-author") + self.assertNotIn("@", summary) + + def test_reviewer_summary_uses_username_and_profile_only(self): + summary = review_proofs.format_identity_summary( + "sysadmin", "prgs-reviewer") + self.assertEqual(summary, "sysadmin / prgs-reviewer") + + def test_summary_appends_role_and_remote_without_email(self): + summary = review_proofs.format_identity_summary( + "jcwalker3", "prgs-author", role="author", remote="prgs") + self.assertIn("jcwalker3 / prgs-author", summary) + self.assertIn("author", summary) + self.assertIn("prgs", summary) + self.assertNotIn("@", summary) + + def test_summary_never_leaks_email_passed_as_username(self): + """Defense in depth: an email in the username slot is reduced.""" + summary = review_proofs.format_identity_summary( + "jcwalker3@yahoo.com", "prgs-author") + self.assertNotIn("@", summary) + self.assertIn("jcwalker3", summary) + + +class TestEmailDisclosureAssessment(unittest.TestCase): + def test_report_without_email_passes(self): + report = "\n".join([ + "Controller Handoff", + "Identity: jcwalker3 / prgs-author", + "Role: author", + ]) + res = review_proofs.assess_email_disclosure(report) + self.assertTrue(res["proven"]) + self.assertFalse(res["flagged"]) + self.assertEqual(res["emails"], []) + self.assertEqual(res["reasons"], []) + + def test_unnecessary_email_is_flagged(self): + report = "\n".join([ + "Controller Handoff", + "Identity: jcwalker3 / jcwalker3@yahoo.com", + ]) + res = review_proofs.assess_email_disclosure(report) + self.assertFalse(res["proven"]) + self.assertTrue(res["flagged"]) + self.assertIn("jcwalker3@yahoo.com", res["emails"]) + self.assertTrue(res["reasons"]) + self.assertFalse(res["justified"]) + + def test_multiple_emails_all_reported(self): + report = ( + "Identity: jcwalker3@yahoo.com author\n" + "Reviewer: 913443@dadeschools.net\n" + ) + res = review_proofs.assess_email_disclosure(report) + self.assertTrue(res["flagged"]) + self.assertEqual( + sorted(res["emails"]), + ["913443@dadeschools.net", "jcwalker3@yahoo.com"], + ) + + def test_justified_email_with_explanation_is_not_flagged(self): + report = "\n".join([ + "Identity: jcwalker3 / prgs-author", + "Contact email: jcwalker3@yahoo.com", + "Email required because two accounts share the username and the", + "address is necessary to disambiguate identity.", + ]) + res = review_proofs.assess_email_disclosure(report) + self.assertFalse(res["flagged"]) + self.assertTrue(res["justified"]) + self.assertIn("jcwalker3@yahoo.com", res["emails"]) + + def test_email_without_justification_language_is_flagged(self): + report = "Contact email: jcwalker3@yahoo.com just in case.\n" + res = review_proofs.assess_email_disclosure(report) + self.assertTrue(res["flagged"]) + self.assertFalse(res["justified"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_issue_lock_worktree.py b/tests/test_issue_lock_worktree.py index ae7c027..02347f4 100644 --- a/tests/test_issue_lock_worktree.py +++ b/tests/test_issue_lock_worktree.py @@ -36,7 +36,32 @@ class TestIssueLockWorktreeAssessment(unittest.TestCase): porcelain_status="", ) self.assertFalse(result["proven"]) - self.assertIn("issue lock must be taken from base branch", result["reasons"][0]) + self.assertIn("base-equivalence could not be proven", result["reasons"][0]) + + def test_base_equivalent_feature_branch_passes(self): + result = issue_lock_worktree.assess_issue_lock_worktree( + worktree_path="/repo/branches/issue-275", + current_branch="feat/issue-275-claim-lock-branches-worktree", + porcelain_status="", + base_equivalent=True, + inspected_git_root="/repo/branches/issue-275", + base_branch="origin/master", + ) + self.assertTrue(result["proven"]) + self.assertFalse(result["block"]) + self.assertEqual(result["base_branch"], "origin/master") + + def test_non_base_equivalent_branch_fails(self): + result = issue_lock_worktree.assess_issue_lock_worktree( + worktree_path="/repo/branches/issue-275", + current_branch="feat/issue-275-claim-lock-branches-worktree", + porcelain_status="", + base_equivalent=False, + inspected_git_root="/repo/branches/issue-275", + base_branch=None, + ) + self.assertFalse(result["proven"]) + self.assertIn("must be base-equivalent", result["reasons"][0]) def test_untracked_files_do_not_block(self): result = issue_lock_worktree.assess_issue_lock_worktree( @@ -96,4 +121,4 @@ class TestPrWorktreeMatch(unittest.TestCase): if __name__ == "__main__": - unittest.main() \ No newline at end of file + unittest.main() diff --git a/tests/test_llm_workflow_split.py b/tests/test_llm_workflow_split.py new file mode 100644 index 0000000..327b17d --- /dev/null +++ b/tests/test_llm_workflow_split.py @@ -0,0 +1,100 @@ +"""Doc-contract checks for task-specific LLM workflow split (#333).""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SKILL_DIR = REPO_ROOT / "skills" / "llm-project-workflow" +SKILL = (SKILL_DIR / "SKILL.md").read_text(encoding="utf-8") + + +def test_skill_md_exists(): + assert SKILL_DIR.joinpath("SKILL.md").is_file() + + +def test_all_workflow_files_exist(): + for name in ( + "review-merge-pr.md", + "reconcile-landed-pr.md", + "create-issue.md", + "work-issue.md", + ): + assert (SKILL_DIR / "workflows" / name).is_file(), name + + +def test_skill_references_all_workflow_files(): + for name in ( + "workflows/review-merge-pr.md", + "workflows/reconcile-landed-pr.md", + "workflows/create-issue.md", + "workflows/work-issue.md", + ): + assert name in SKILL + + +def test_skill_contains_mode_isolation_language(): + assert "## Mode isolation" in SKILL + assert "review-merge-pr" in SKILL + assert "reconcile-landed-pr" in SKILL + assert "create-issue" in SKILL + assert "work-issue" in SKILL + assert "Do not mix modes" in SKILL or "do not mix modes" in SKILL.lower() + + +def test_skill_is_router_not_monolithic_review_body(): + assert "This skill is a **router**" in SKILL or "router" in SKILL.lower() + assert "## F. Review workflow" not in SKILL + assert "gitea_mark_final_review_decision" not in SKILL + assert "gitea_submit_pr_review" not in SKILL + + +def test_review_merge_workflow_contract(): + text = (SKILL_DIR / "workflows" / "review-merge-pr.md").read_text(encoding="utf-8") + assert "canonical: true" in text + assert "## 26A. Terminal review mutation hard-stop" in text + assert "## 11A. Skipped PRs are read-only" in text + assert "## 35. Duplicate request-changes prevention" in text + assert "ALREADY_LANDED_RECONCILE_REQUIRED" in text + + +def test_reconcile_landed_workflow_contract(): + text = (SKILL_DIR / "workflows" / "reconcile-landed-pr.md").read_text(encoding="utf-8") + assert "canonical: true" in text + assert "Do not review or merge normal PRs" in text + assert "Already-landed proof" in text + + +def test_create_issue_workflow_contract(): + text = (SKILL_DIR / "workflows" / "create-issue.md").read_text(encoding="utf-8") + assert "canonical: true" in text + assert "## 9. Duplicate search before mutation" in text + + +def test_work_issue_workflow_contract(): + text = (SKILL_DIR / "workflows" / "work-issue.md").read_text(encoding="utf-8") + assert "canonical: true" in text + assert "Do not merge your own PR" in text + assert "## 21. PR creation rules" in text + + +def test_final_report_schemas_exist(): + for name in ( + "review-merge-final-report.md", + "reconcile-landed-final-report.md", + "create-issue-final-report.md", + "work-issue-final-report.md", + ): + assert (SKILL_DIR / "schemas" / name).is_file(), name + + +def test_review_pr_template_references_workflow(): + text = (SKILL_DIR / "templates" / "review-pr.md").read_text(encoding="utf-8") + assert "workflows/review-merge-pr.md" in text + + +def test_merge_pr_template_references_workflow(): + text = (SKILL_DIR / "templates" / "merge-pr.md").read_text(encoding="utf-8") + assert "workflows/review-merge-pr.md" in text + + +def test_start_issue_template_references_work_issue_workflow(): + text = (SKILL_DIR / "templates" / "start-issue.md").read_text(encoding="utf-8") + assert "workflows/work-issue.md" in text \ No newline at end of file diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 6b2827f..f4597ca 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1126,9 +1126,11 @@ class TestGetFile(unittest.TestCase): # --------------------------------------------------------------------------- class TestCommitFiles(unittest.TestCase): + @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", + return_value=(True, [])) @patch("mcp_server.api_request") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) - def test_commit_files_success(self, _auth, mock_api): + def test_commit_files_success(self, _auth, mock_api, _role): mock_api.return_value = { "commit": {"sha": "commit-sha-123"}, "branch": {"name": "test-branch"} @@ -1136,11 +1138,15 @@ class TestCommitFiles(unittest.TestCase): files = [ {"operation": "create", "path": "test.txt", "content": "SGVsbG8="} ] - result = gitea_commit_files( - files=files, - message="Initial commit", - new_branch="test-branch" - ) + env = { + "GITEA_ALLOWED_OPERATIONS": "gitea.repo.commit", + } + with patch.dict(os.environ, env, clear=True): + result = gitea_commit_files( + files=files, + message="Initial commit", + new_branch="test-branch" + ) self.assertTrue(result["success"]) self.assertEqual(result["commit"], "commit-sha-123") self.assertEqual(result["branch"], "test-branch") @@ -2986,20 +2992,31 @@ class TestVerifyMutationAuthority(unittest.TestCase): mcp_server.verify_mutation_authority("prgs") +def _clean_master_git_state_for_lock(): + return { + "current_branch": "master", + "porcelain_status": "", + "base_equivalent": True, + "inspected_git_root": "/scratch/wt", + "base_branch": "origin/master", + } + + class TestIssueLocking(unittest.TestCase): """Test issue locking and PR gating constraints.""" - @staticmethod - def _clean_master_git_state(): - return {"current_branch": "master", "porcelain_status": ""} + def setUp(self): + self._env_patcher = patch.dict(os.environ, ISSUE_WRITE_ENV, clear=True) + self._env_patcher.start() def tearDown(self): + self._env_patcher.stop() if os.path.exists(ISSUE_LOCK_FILE): os.remove(ISSUE_LOCK_FILE) @patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", - return_value={"current_branch": "master", "porcelain_status": ""}, + return_value=_clean_master_git_state_for_lock(), ) @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @@ -3019,7 +3036,7 @@ class TestIssueLocking(unittest.TestCase): @patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", - return_value={"current_branch": "master", "porcelain_status": ""}, + return_value=_clean_master_git_state_for_lock(), ) @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @@ -3036,7 +3053,7 @@ class TestIssueLocking(unittest.TestCase): @patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", - return_value={"current_branch": "master", "porcelain_status": ""}, + return_value=_clean_master_git_state_for_lock(), ) @patch("mcp_server.api_get_all") @patch("mcp_server.get_auth_header", return_value=FAKE_AUTH) @@ -3057,7 +3074,7 @@ class TestIssueLocking(unittest.TestCase): scratch = "/tmp/gitea-tools-author-scratch/issue-249-clean" with patch( "mcp_server.issue_lock_worktree.read_worktree_git_state", - return_value={"current_branch": "master", "porcelain_status": ""}, + return_value=_clean_master_git_state_for_lock(), ) as mock_git: res = gitea_lock_issue( issue_number=249, @@ -3077,6 +3094,7 @@ class TestIssueLocking(unittest.TestCase): return_value={ "current_branch": "master", "porcelain_status": " M gitea_mcp_server.py\n", + "base_equivalent": True, }, ): with self.assertRaises(RuntimeError) as ctx: @@ -3096,6 +3114,7 @@ class TestIssueLocking(unittest.TestCase): return_value={ "current_branch": "feat/issue-243-forbidden-git-gaps", "porcelain_status": "", + "base_equivalent": False, }, ): with self.assertRaises(RuntimeError) as ctx: @@ -3105,7 +3124,7 @@ class TestIssueLocking(unittest.TestCase): remote="prgs", worktree_path="/tmp/scratch/wt", ) - self.assertIn("issue lock must be taken from base branch", str(ctx.exception)) + self.assertIn("base-equivalent", str(ctx.exception)) @patch("mcp_server.role_session_router.check_author_mutation_after_reviewer_stop", return_value=(True, [])) @@ -3248,7 +3267,12 @@ class TestPreflightVerification(unittest.TestCase): mcp_server._preflight_whoami_violation_files = self.orig_whoami_files mcp_server._preflight_capability_violation_files = self.orig_capability_files mcp_server._preflight_reviewer_violation_files = self.orig_reviewer_files - for key in ("GITEA_TEST_FORCE_DIRTY", "GITEA_TEST_PORCELAIN"): + for key in ( + "GITEA_TEST_FORCE_DIRTY", + "GITEA_TEST_PORCELAIN", + "GITEA_ACTIVE_WORKTREE", + "GITEA_AUTHOR_WORKTREE", + ): if key in os.environ: del os.environ[key] @@ -3338,3 +3362,35 @@ class TestPreflightVerification(unittest.TestCase): status = mcp_server.assess_preflight_status() self.assertFalse(status["preflight_ready"]) self.assertIn("gitea_whoami", status["preflight_block_reasons"][0]) + + def test_declared_clean_task_worktree_ignores_control_checkout_violation(self): + """#275: active branches/ worktree is the inspected mutation workspace.""" + import mcp_server + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + mcp_server._preflight_whoami_violation = True + mcp_server._preflight_whoami_violation_files = ["mcp_server.py"] + os.environ["GITEA_TEST_PORCELAIN"] = "" + + worktree = "/repo/branches/issue-275-clean" + status = mcp_server.assess_preflight_status(worktree_path=worktree) + self.assertTrue(status["preflight_ready"]) + self.assertEqual(status["preflight_block_reasons"], []) + self.assertIn("preflight_workspace", status) + mcp_server.verify_preflight_purity(worktree_path=worktree) + + def test_declared_dirty_task_worktree_reports_workspace_diagnostics(self): + """#275: dirty failures name the inspected task workspace, not just files.""" + import mcp_server + mcp_server._preflight_whoami_called = True + mcp_server._preflight_capability_called = True + os.environ["GITEA_TEST_PORCELAIN"] = " M task_file.py\n" + + worktree = "/repo/branches/issue-275-dirty" + with self.assertRaises(RuntimeError) as ctx: + mcp_server.verify_preflight_purity(worktree_path=worktree) + msg = str(ctx.exception) + self.assertIn("active task workspace root", msg) + self.assertIn("inspected git root", msg) + self.assertIn("dirty files: task_file.py", msg) + self.assertIn("dirty scope:", msg) diff --git a/tests/test_mutation_ledger_report.py b/tests/test_mutation_ledger_report.py new file mode 100644 index 0000000..7223e72 --- /dev/null +++ b/tests/test_mutation_ledger_report.py @@ -0,0 +1,102 @@ +"""Tests for final-report mutation ledger verifier (#331).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from review_proofs import assess_mutation_ledger_report # noqa: E402 + + +class TestMutationLedgerReport(unittest.TestCase): + def test_none_claim_with_performed_edit_blocks(self): + report = ( + "Review decision: request_changes.\n" + "File edits by reviewer: none\n" + ) + action_log = [ + {"action": "Edited", "path": "walkthrough.md", "tracked": False}, + ] + result = assess_mutation_ledger_report(report, action_log=action_log) + self.assertFalse(result["proven"]) + self.assertTrue(result["file_edits_claimed_none"]) + + def test_reported_edit_passes(self): + report = ( + "File edits by reviewer: Edited walkthrough.md (untracked)\n" + "Mutation ledger: Edited walkthrough.md untracked in review worktree\n" + ) + action_log = [ + { + "action": "Edited", + "path": "walkthrough.md", + "tracked": False, + "in_repo": True, + }, + ] + result = assess_mutation_ledger_report( + report, + action_log=action_log, + walkthrough_explicitly_requested=True, + ) + self.assertTrue(result["proven"]) + + def test_unreported_mutation_blocks(self): + report = "File edits by reviewer: none\n" + action_log = [{"action": "Wrote", "path": "/tmp/review-notes.txt"}] + result = assess_mutation_ledger_report(report, action_log=action_log) + self.assertFalse(result["proven"]) + self.assertIn("/tmp/review-notes.txt", result["unreported_paths"]) + + def test_outside_repo_requires_label(self): + report = ( + "File edits by reviewer: Wrote /tmp/review-notes.txt\n" + ) + action_log = [ + { + "action": "Wrote", + "path": "/tmp/review-notes.txt", + "outside_repo": True, + }, + ] + result = assess_mutation_ledger_report(report, action_log=action_log) + self.assertFalse(result["proven"]) + self.assertIn("outside repo", " ".join(result["reasons"]).lower()) + + def test_gated_rejection_excluded_from_performed(self): + report = "File edits by reviewer: none\nRejected gated calls: submit_pr_review blocked\n" + action_log = [ + { + "action": "Edited", + "path": "walkthrough.md", + "performed": False, + "gated_rejected": True, + }, + ] + result = assess_mutation_ledger_report(report, action_log=action_log) + self.assertTrue(result["proven"]) + self.assertEqual(result["performed_mutations"], []) + + def test_post_status_artifact_requires_final_git_status(self): + report = ( + "File edits by reviewer: Created scratch/notes.md (untracked)\n" + ) + action_log = [ + { + "action": "Created", + "path": "scratch/notes.md", + "tracked": False, + "after_git_status": True, + }, + ] + result = assess_mutation_ledger_report( + report, + action_log=action_log, + final_git_status_reported=False, + ) + self.assertFalse(result["proven"]) + self.assertIn("final git status", " ".join(result["reasons"]).lower()) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_operator_guide.py b/tests/test_operator_guide.py index 602160a..fd62809 100644 --- a/tests/test_operator_guide.py +++ b/tests/test_operator_guide.py @@ -133,7 +133,8 @@ class TestControlPlaneGuide(GuideTestBase): for key in ("hard_stops", "fail_closed", "head_sha_pinning", "merge_confirmation", "redaction", "separation", "profile_switching", "identity_verification", - "work_selection", "global_worktree"): + "work_selection", "global_worktree", + "shell_spawn_hard_stop"): self.assertIn(key, rules) self.assertIn("MERGE PR", json.dumps(rules["merge_confirmation"])) self.assertTrue(rules["hard_stops"]) diff --git a/tests/test_request_changes_approval_proof.py b/tests/test_request_changes_approval_proof.py new file mode 100644 index 0000000..95ed42e --- /dev/null +++ b/tests/test_request_changes_approval_proof.py @@ -0,0 +1,104 @@ +"""Tests for REQUEST_CHANGES override proof before approval (#326).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from review_proofs import assess_request_changes_approval_proof # noqa: E402 + +HEAD_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" +HEAD_B = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + + +def _feedback(**overrides): + base = { + "success": True, + "current_head_sha": HEAD_A, + "has_blocking_change_requests": False, + "author_pushed_after_request_changes": False, + "reviews": [], + } + base.update(overrides) + return base + + +def _blocking_review(**overrides): + entry = { + "reviewer": "reviewer-a", + "verdict": "REQUEST_CHANGES", + "body": "Tests fail on pinned head; fix test_commit_files_gate.", + "submitted_at": "2026-07-07T01:00:00-05:00", + "reviewed_head_sha": HEAD_A, + "dismissed": False, + "stale": False, + } + entry.update(overrides) + return entry + + +class TestRequestChangesApprovalProof(unittest.TestCase): + def test_missing_feedback_blocks_approval(self): + result = assess_request_changes_approval_proof(None) + self.assertFalse(result["approve_allowed"]) + self.assertTrue(result["block"]) + + def test_no_prior_request_changes_allows_approval(self): + feedback = _feedback( + reviews=[{ + "reviewer": "sysadmin", + "verdict": "APPROVED", + "body": "LGTM", + "submitted_at": "2026-07-07T02:00:00-05:00", + "reviewed_head_sha": HEAD_A, + "dismissed": False, + }] + ) + result = assess_request_changes_approval_proof(feedback) + self.assertTrue(result["approve_allowed"]) + self.assertIsNone(result["blocking_review"]) + + def test_changed_head_since_blocker_allows_approval(self): + feedback = _feedback( + current_head_sha=HEAD_B, + author_pushed_after_request_changes=True, + has_blocking_change_requests=True, + reviews=[_blocking_review(reviewed_head_sha=HEAD_A)], + ) + result = assess_request_changes_approval_proof(feedback) + self.assertTrue(result["approve_allowed"]) + self.assertTrue(result["head_changed_since_blocker"]) + + def test_unchanged_head_without_override_blocks_approval(self): + feedback = _feedback( + has_blocking_change_requests=True, + reviews=[_blocking_review()], + ) + result = assess_request_changes_approval_proof(feedback) + self.assertFalse(result["approve_allowed"]) + self.assertIn("override_reason", " ".join(result["reasons"])) + + def test_unchanged_head_with_valid_override_allows_approval(self): + blocker = _blocking_review() + feedback = _feedback( + has_blocking_change_requests=True, + reviews=[blocker], + ) + report = ( + "Blocker text: Tests fail on pinned head; fix test_commit_files_gate.\n" + "Override: wrong_validation_environment — CI used stale worktree." + ) + result = assess_request_changes_approval_proof( + feedback, + override_reason="wrong_validation_environment", + override_explanation="CI used stale worktree; local rerun passed.", + report_text=report, + ) + self.assertTrue(result["approve_allowed"]) + self.assertEqual( + result["blocking_review"]["blocking_reviewer"], "reviewer-a" + ) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_resolve_task_capability.py b/tests/test_resolve_task_capability.py index 463a618..3545675 100644 --- a/tests/test_resolve_task_capability.py +++ b/tests/test_resolve_task_capability.py @@ -141,6 +141,17 @@ class TestResolveTaskCapability(unittest.TestCase): self.assertTrue(res["allowed_in_current_session"]) self.assertIn("gitea.issue.comment", res["active_profile_allowed_operations"]) + @patch("mcp_server.api_request", return_value={"login": "author-user"}) + @patch("mcp_server.get_auth_header", return_value="token author-pass") + def test_resolve_lock_issue_author_profile_allowed(self, _auth, _api): + # #275: lock_issue must be an exact resolver task for claim/lock gates. + with patch.dict(os.environ, self._env("author-profile")): + res = mcp_server.gitea_resolve_task_capability(task="lock_issue", remote="prgs") + self.assertEqual(res["requested_task"], "lock_issue") + self.assertEqual(res["required_operation_permission"], "gitea.issue.comment") + self.assertTrue(res["allowed_in_current_session"]) + self.assertIn("gitea.issue.comment", res["active_profile_allowed_operations"]) + def test_resolve_unknown_task_fails_closed(self): with patch.dict(os.environ, self._env("author-profile")): with self.assertRaises(ValueError): diff --git a/tests/test_role_session_router.py b/tests/test_role_session_router.py index 25b21c1..6204b6d 100644 --- a/tests/test_role_session_router.py +++ b/tests/test_role_session_router.py @@ -206,28 +206,35 @@ class TestRoleSessionRouter(unittest.TestCase): 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 + # Hermetic
/branches/ layout (#283): deriving these + # paths from __file__ made the test pass only when the suite ran from + # a branches/ worktree, because skip_python_scan_walk_root skips a + # branches/ walk root only when /branches exists. + with tempfile.TemporaryDirectory() as tmp: + main_root = os.path.join(tmp, "main") + worktree_root = os.path.join(main_root, "branches", "fix-issue-1") + os.makedirs(os.path.join(worktree_root, "tests")) + + 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.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, worktree_root + ) ) - ) - self.assertFalse( - role_session_router.skip_python_scan_walk_root( - worktree_root, os.path.join(worktree_root, "tests") + 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()) diff --git a/tests/test_shell_spawn_hard_stop_docs.py b/tests/test_shell_spawn_hard_stop_docs.py new file mode 100644 index 0000000..f4596e3 --- /dev/null +++ b/tests/test_shell_spawn_hard_stop_docs.py @@ -0,0 +1,82 @@ +"""Doc-contract checks for the shell-spawn hard-stop rule (#258). + +When the shell executor fails to spawn (exit_code: -1 with empty +stdout/stderr), agents must probe once, mark shell unavailable, hard-stop +after two consecutive spawn failures, and emit a recovery report instead of +retry-spiralling. These tests pin that guidance in the runbooks, the portable +skill doc, and the operator guide so it cannot silently regress. +""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent + +RUNBOOK = REPO_ROOT / "docs" / "llm-workflow-runbooks.md" +SKILL = REPO_ROOT / "skills" / "llm-project-workflow" / "SKILL.md" + + +def _normalize(text: str) -> str: + """Collapse whitespace so phrase checks survive markdown line wrapping.""" + return " ".join(text.split()) + + +def _runbook_text() -> str: + return _normalize(RUNBOOK.read_text(encoding="utf-8")) + + +def _skill_text() -> str: + return _normalize(SKILL.read_text(encoding="utf-8")) + + +def test_runbook_has_shell_spawn_hard_stop_section(): + text = _runbook_text() + assert "## Shell Spawn Hard-Stop Rule" in text + for phrase in ( + "exit_code: -1", + "empty stdout/stderr", + "trivial probe", + "two consecutive spawn failures", + "mark shell unavailable", + "recovery report", + ): + assert phrase in text, f"runbook missing phrase: {phrase!r}" + + +def test_runbook_recovery_report_names_required_steps(): + text = _runbook_text() + for phrase in ( + "restart the session", + "kill hung background terminals", + "MCP-native", + ): + assert phrase in text, f"runbook recovery guidance missing: {phrase!r}" + + +def test_runbook_forbids_retry_spirals(): + text = _runbook_text() + assert "never retry the same failing spawn" in text + + +def test_skill_doc_declares_shell_spawn_hard_stop_rule(): + text = _skill_text() + assert "## Shell Spawn Hard-Stop Rule" in text + for phrase in ( + "exit_code: -1", + "two consecutive spawn failures", + "recovery report", + ): + assert phrase in text, f"SKILL.md missing phrase: {phrase!r}" + + +def test_operator_guide_rules_include_shell_spawn_hard_stop(): + import sys + + sys.path.insert(0, str(REPO_ROOT)) + import gitea_mcp_server + + rule = gitea_mcp_server._GUIDE_RULES["shell_spawn_hard_stop"] + for phrase in ( + "exit_code: -1", + "two consecutive", + "recovery report", + ): + assert phrase in rule, f"operator guide rule missing: {phrase!r}" diff --git a/tests/test_subagent_gate.py b/tests/test_subagent_gate.py new file mode 100644 index 0000000..a4d08fd --- /dev/null +++ b/tests/test_subagent_gate.py @@ -0,0 +1,152 @@ +"""Tests for fail-closed subagent delegation gates (#266). + +Covers the acceptance criteria: blocked write delegation, allowed read-only +delegation, missing inherited context, and invalid subagent final reports. +""" +import unittest + +import subagent_gate + + +def _full_context(): + return { + "issue_lock": "issue #266 locked to feat/issue-266-subagent-gate-inheritance", + "branch": "feat/issue-266-subagent-gate-inheritance", + "worktree_path": "branches/feat-issue-266-subagent-gate-inheritance", + "identity_profile": "jcwalker3/prgs-author", + "allowed_tool_class": "read_write_files", + "command_deny_list": "git push --force; rm -rf", + "validation_ledger_requirement": "record command/exit/output for every validation claim", + "final_report_schema": "controller-handoff-v1", + } + + +def _full_report(): + return { + "identity_profile": "jcwalker3/prgs-author", + "worktree_path": "branches/feat-issue-266-subagent-gate-inheritance", + "branch": "feat/issue-266-subagent-gate-inheritance", + "changed_files": "subagent_gate.py, tests/test_subagent_gate.py", + "validation_results": "pytest tests/test_subagent_gate.py -q: exit 0, 14 passed", + "workspace_mutations": "edited subagent_gate.py", + } + + +class TestAssessSubagentDelegation(unittest.TestCase): + # AC1: deterministic write tasks are blocked by default + def test_write_delegation_blocked_by_default(self): + for task in ("claim_issue", "create_branch", "edit_code", "commit", + "push_branch", "create_pr", "review_pr", "merge_pr", + "cleanup_branch"): + res = subagent_gate.assess_subagent_delegation(task) + self.assertTrue(res["block"], task) + self.assertFalse(res["allowed"], task) + self.assertEqual(res["task_class"], "deterministic_write", task) + self.assertTrue( + any("explicitly allowed" in r for r in res["reasons"]), task) + + # AC2: explicit allowance without a recorded justification still blocks + def test_write_delegation_requires_recorded_justification(self): + res = subagent_gate.assess_subagent_delegation( + "create_pr", explicitly_allowed=True, + inherited_context=_full_context()) + self.assertTrue(res["block"]) + self.assertTrue( + any("justification" in r for r in res["reasons"])) + + # AC3: explicit allowance + justification but missing inherited context + def test_write_delegation_blocks_on_missing_inherited_context(self): + context = _full_context() + del context["issue_lock"] + del context["command_deny_list"] + res = subagent_gate.assess_subagent_delegation( + "commit", explicitly_allowed=True, + justification="parent session proved batch commit needs isolation", + inherited_context=context) + self.assertTrue(res["block"]) + self.assertIn("issue_lock", res["missing_context"]) + self.assertIn("command_deny_list", res["missing_context"]) + + def test_write_delegation_blocks_on_empty_context_values(self): + context = _full_context() + context["worktree_path"] = " " + res = subagent_gate.assess_subagent_delegation( + "commit", explicitly_allowed=True, + justification="isolation required", + inherited_context=context) + self.assertTrue(res["block"]) + self.assertIn("worktree_path", res["missing_context"]) + + def test_write_delegation_allowed_with_authorization_and_full_context(self): + res = subagent_gate.assess_subagent_delegation( + "edit_code", explicitly_allowed=True, + justification="parallel mechanical rename across many files", + inherited_context=_full_context()) + self.assertFalse(res["block"]) + self.assertTrue(res["allowed"]) + self.assertEqual(res["missing_context"], []) + + # Allowed read-only delegation needs no explicit authorization + def test_read_only_delegation_allowed(self): + for task in ("read_files", "code_search", "inventory_prs", + "summarize_issue"): + res = subagent_gate.assess_subagent_delegation(task) + self.assertFalse(res["block"], task) + self.assertTrue(res["allowed"], task) + self.assertEqual(res["task_class"], "read_only", task) + + # Unknown task types fail closed + def test_unknown_task_fails_closed(self): + res = subagent_gate.assess_subagent_delegation("launch_missiles") + self.assertTrue(res["block"]) + self.assertEqual(res["task_class"], "unknown") + + def test_blank_task_fails_closed(self): + res = subagent_gate.assess_subagent_delegation(" ") + self.assertTrue(res["block"]) + self.assertEqual(res["task_class"], "unknown") + + +class TestValidateSubagentReport(unittest.TestCase): + # AC4: subagent output must carry the same proof fields as the parent + def test_full_report_valid(self): + res = subagent_gate.validate_subagent_report(_full_report()) + self.assertTrue(res["valid"]) + self.assertFalse(res["block"]) + self.assertEqual(res["missing_fields"], []) + + def test_missing_proof_fields_invalid(self): + report = _full_report() + del report["validation_results"] + del report["worktree_path"] + res = subagent_gate.validate_subagent_report(report) + self.assertFalse(res["valid"]) + self.assertTrue(res["block"]) + self.assertIn("validation_results", res["missing_fields"]) + self.assertIn("worktree_path", res["missing_fields"]) + + def test_empty_field_values_invalid(self): + report = _full_report() + report["changed_files"] = "" + res = subagent_gate.validate_subagent_report(report) + self.assertFalse(res["valid"]) + self.assertIn("changed_files", res["missing_fields"]) + + def test_none_report_invalid(self): + res = subagent_gate.validate_subagent_report(None) + self.assertFalse(res["valid"]) + self.assertTrue(res["block"]) + + +class TestOperatorGuideRule(unittest.TestCase): + def test_operator_guide_declares_subagent_rule(self): + import gitea_mcp_server + + rule = gitea_mcp_server._GUIDE_RULES["subagent_delegation"].lower() + for phrase in ("deterministic write", "inherit", "read-only", + "fail closed"): + self.assertIn(phrase, rule) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_validation_ledger.py b/tests/test_validation_ledger.py new file mode 100644 index 0000000..0048820 --- /dev/null +++ b/tests/test_validation_ledger.py @@ -0,0 +1,155 @@ +"""Tests for validation ledger and final-report verifier (#271).""" +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from validation_ledger import ( # noqa: E402 + assess_full_suite_claim, + assess_ledger_entry, + new_ledger_entry, + verify_final_report, +) + + +class TestLedgerEntry(unittest.TestCase): + def test_valid_entry_is_claimable(self): + entry = new_ledger_entry( + command="pytest tests/ -q", + cwd="/repo/branches/feat-issue-271", + exit_code=0, + output_summary="994 passed, 6 skipped", + ) + result = assess_ledger_entry(entry) + self.assertTrue(result["claimable"]) + self.assertEqual(result["verdict"], "strong") + + def test_missing_command_output_is_invalid(self): + entry = new_ledger_entry( + command="pytest tests/ -q", + cwd="/repo/branches/task", + exit_code=0, + output_summary="", + ) + result = assess_ledger_entry(entry) + self.assertFalse(result["claimable"]) + self.assertEqual(result["verdict"], "invalid") + + +class TestFullSuiteClaim(unittest.TestCase): + def _entry(self, **kwargs): + base = new_ledger_entry( + command="pytest tests/ -q", + cwd="/repo/branches/task", + exit_code=0, + output_summary="10 passed", + ) + base.update(kwargs) + return base + + def test_full_suite_overclaim_with_ignored_paths_fails(self): + entry = self._entry( + ignored_paths=[{"path": "tests/integration/", "justification": ""}], + ) + result = assess_full_suite_claim( + [entry], + report_text="Validation: full suite passed.", + ) + self.assertFalse(result["claimable"]) + self.assertTrue( + any("full suite" in r.lower() for r in result["reasons"]) + ) + + def test_full_suite_except_note_allows_ignored_paths(self): + entry = self._entry( + ignored_paths=[{ + "path": "tests/integration/", + "justification": "network tests require GITEA_INTEGRATION=1", + }], + ) + result = assess_full_suite_claim( + [entry], + report_text="Validation: full suite except integration tests.", + ) + self.assertTrue(result["claimable"]) + + +class TestFinalReportVerifier(unittest.TestCase): + def _base_report(self, **overrides): + report = { + "identity": "jcwalker3", + "profile": "prgs-author", + "capability_resolved": True, + "worktree_path": "/repo/branches/feat-issue-271", + "changed_files": ["validation_ledger.py"], + "validation_ledger": [ + new_ledger_entry( + command="pytest tests/test_validation_ledger.py -q", + cwd="/repo/branches/feat-issue-271", + exit_code=0, + output_summary="12 passed", + ) + ], + "validation_claim": "passed", + "review_submitted": False, + "review_visible_approved": False, + "review_pending": False, + "merge_performed": False, + "merge_blocker": "not a reviewer task", + "workspace_clean": True, + } + report.update(overrides) + return report + + def test_complete_report_is_proven(self): + result = verify_final_report(self._base_report()) + self.assertTrue(result["proven"]) + self.assertFalse(result["block"]) + + def test_validation_overclaim_fails(self): + report = self._base_report( + validation_claim="passed", + validation_ledger=[ + new_ledger_entry( + command="pytest tests/ -q", + cwd="/repo/branches/task", + exit_code=1, + output_summary="3 failed", + ) + ], + ) + result = verify_final_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(result["contradictions"]) + + def test_pending_approval_misreported_as_approved_fails(self): + report = self._base_report( + claims_approved_review=True, + review_visible_approved=False, + review_pending=True, + ) + result = verify_final_report(report) + self.assertFalse(result["proven"]) + self.assertTrue( + any("approved" in c.lower() for c in result["contradictions"]) + ) + + def test_missing_command_output_in_ledger_fails(self): + report = self._base_report( + validation_ledger=[ + new_ledger_entry( + command="pytest tests/ -q", + cwd="/repo/branches/task", + exit_code=0, + output_summary="", + ) + ], + ) + result = verify_final_report(report) + self.assertFalse(result["proven"]) + self.assertTrue(result["reasons"]) + + +if __name__ == "__main__": + unittest.main() \ No newline at end of file diff --git a/tests/test_workflow_commit_path_docs.py b/tests/test_workflow_commit_path_docs.py new file mode 100644 index 0000000..7322127 --- /dev/null +++ b/tests/test_workflow_commit_path_docs.py @@ -0,0 +1,33 @@ +"""Documentation checks for MCP-native commit path rules (#260).""" +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +RUNBOOK = REPO_ROOT / "docs" / "llm-workflow-runbooks.md" +SAFETY = REPO_ROOT / "docs" / "safety-model.md" + + +def test_runbook_requires_mcp_native_commit_path(): + text = RUNBOOK.read_text(encoding="utf-8") + assert "MCP-native commit path" in text + assert "gitea_commit_files" in text + assert "gitea.repo.commit" in text + assert "only" in text.lower() and "approved" in text.lower() + lower = text.lower() + for forbidden in ("webfetch", "playwright", "manual llm-generated base64"): + assert forbidden in lower + + +def test_runbook_forbids_fallback_loops(): + lower = RUNBOOK.read_text(encoding="utf-8").lower() + assert "retry shell encoding" in lower + assert "loop" in lower + assert "recovery report" in lower + + +def test_safety_model_documents_commit_fallback_ban(): + text = SAFETY.read_text(encoding="utf-8") + assert "Agent Commit Path" in text + assert "gitea_commit_files" in text + assert "WebFetch" in text + assert "Playwright" in text + assert "llm-workflow-runbooks.md" in text \ No newline at end of file diff --git a/validation_ledger.py b/validation_ledger.py new file mode 100644 index 0000000..a78dd6d --- /dev/null +++ b/validation_ledger.py @@ -0,0 +1,239 @@ +"""Validation command ledger and final-report verifier (#271). + +Sessions may only claim validation passed when the exact command, exit code, +and output summary are recorded. Final reports are checked against the +ledger and workflow proof fields before submission. +""" + +from __future__ import annotations + +from datetime import datetime, timezone +from typing import Any + +LEDGER_REQUIRED_FIELDS = ( + "command", + "cwd", + "exit_code", + "output_summary", + "timestamp", +) + +OPTIONAL_LEDGER_FIELDS = ( + "skipped_tests", + "ignored_paths", +) + + +def _clean(value: Any) -> str: + return (value or "").strip() if isinstance(value, str) else str(value or "").strip() + + +def normalize_ledger_entry(entry: dict | None) -> dict: + """Return a normalized ledger entry with required keys present.""" + data = dict(entry or {}) + normalized = { + "command": _clean(data.get("command")), + "cwd": _clean(data.get("cwd")), + "exit_code": data.get("exit_code"), + "output_summary": _clean(data.get("output_summary")), + "timestamp": _clean(data.get("timestamp")), + "skipped_tests": data.get("skipped_tests") or [], + "ignored_paths": data.get("ignored_paths") or [], + } + return normalized + + +def assess_ledger_entry(entry: dict | None) -> dict: + """Fail closed when a ledger entry cannot support a validation claim.""" + normalized = normalize_ledger_entry(entry) + reasons: list[str] = [] + + for field in LEDGER_REQUIRED_FIELDS: + value = normalized.get(field) + if field == "exit_code": + if value is None or not isinstance(value, int): + reasons.append("exit_code missing or not an integer") + continue + if not _clean(value): + reasons.append(f"ledger field '{field}' missing or empty") + + exit_code = normalized.get("exit_code") + if isinstance(exit_code, int) and exit_code != 0: + reasons.append(f"exit_code {exit_code} indicates failure") + + if not _clean(normalized.get("output_summary")): + reasons.append("output_summary missing; command output was not summarized") + + claimable = not reasons and isinstance(exit_code, int) and exit_code == 0 + return { + "claimable": claimable, + "entry": normalized, + "reasons": reasons, + "verdict": "strong" if claimable else "invalid", + } + + +def assess_full_suite_claim( + ledger_entries: list[dict] | None, + *, + report_text: str = "", +) -> dict: + """'Full suite passed' requires zero ignored tests unless explicitly noted.""" + entries = [normalize_ledger_entry(e) for e in (ledger_entries or [])] + reasons: list[str] = [] + lower = (report_text or "").lower() + + if not entries: + return { + "claimable": False, + "verdict": "invalid", + "reasons": ["no validation ledger entries recorded"], + } + + entry_assessments = [assess_ledger_entry(e) for e in entries] + invalid = [a for a in entry_assessments if not a["claimable"]] + if invalid: + reasons.extend(invalid[0]["reasons"]) + + ignored_total = 0 + for entry in entries: + ignored = entry.get("ignored_paths") or [] + ignored_total += len(ignored) + for item in ignored: + path = (item or {}).get("path") if isinstance(item, dict) else item + justification = (item or {}).get("justification", "") if isinstance(item, dict) else "" + if path and not _clean(justification): + reasons.append( + f"ignored path '{path}' lacks justification for full-suite claim" + ) + + claims_full_suite = "full suite passed" in lower or "full test suite passed" in lower + if claims_full_suite and ignored_total: + except_phrases = ( + "full suite except", + "except ignored", + "ignored paths:", + "ignored test", + ) + if not any(phrase in lower for phrase in except_phrases): + reasons.append( + "report claims full suite passed but ledger records ignored " + "tests/paths without an explicit 'full suite except …' note" + ) + + claimable = not reasons and not invalid + return { + "claimable": claimable, + "verdict": "strong" if claimable else ("invalid" if invalid else "weak"), + "reasons": reasons, + "entry_assessments": entry_assessments, + "ignored_path_count": ignored_total, + } + + +def new_ledger_entry( + *, + command: str, + cwd: str, + exit_code: int, + output_summary: str, + skipped_tests: list[str] | None = None, + ignored_paths: list[dict] | None = None, + timestamp: str | None = None, +) -> dict: + """Build a ledger entry with an ISO-8601 UTC timestamp.""" + ts = timestamp or datetime.now(timezone.utc).replace(microsecond=0).isoformat() + return normalize_ledger_entry({ + "command": command, + "cwd": cwd, + "exit_code": exit_code, + "output_summary": output_summary, + "timestamp": ts, + "skipped_tests": skipped_tests or [], + "ignored_paths": ignored_paths or [], + }) + + +def verify_final_report(report: dict | None) -> dict: + """Verify a structured final report against ledger and workflow proofs.""" + data = dict(report or {}) + reasons: list[str] = [] + contradictions: list[str] = [] + + identity = _clean(data.get("identity")) + profile = _clean(data.get("profile")) + capability_resolved = data.get("capability_resolved") + worktree_path = _clean(data.get("worktree_path")) + changed_files = data.get("changed_files") or [] + ledger_entries = data.get("validation_ledger") or data.get("ledger_entries") or [] + + if not identity: + reasons.append("identity missing from final report") + if not profile: + reasons.append("profile missing from final report") + if capability_resolved is not True: + reasons.append("capability_resolved must be true with exact resolver evidence") + if not worktree_path: + reasons.append("worktree_path missing from final report") + if not isinstance(changed_files, list): + reasons.append("changed_files must be a list (use [] when none)") + + ledger_verdict = assess_full_suite_claim( + ledger_entries, + report_text=_clean(data.get("report_text")), + ) + if not ledger_verdict["claimable"]: + reasons.extend(ledger_verdict["reasons"]) + + validation_claim = _clean(data.get("validation_claim")).lower() + if validation_claim in ("passed", "pass", "full suite passed"): + if ledger_verdict["verdict"] == "invalid": + contradictions.append( + "validation_claim says passed but ledger entries are invalid" + ) + + review_submitted = data.get("review_submitted") + review_visible_approved = data.get("review_visible_approved") + review_pending = data.get("review_pending") + if review_visible_approved is True and review_pending is True: + contradictions.append( + "review_visible_approved and review_pending cannot both be true" + ) + if data.get("claims_approved_review") is True: + if review_visible_approved is not True: + contradictions.append( + "report claims APPROVED review but review_visible_approved is not true" + ) + if review_pending is True: + contradictions.append( + "report claims APPROVED review while review_pending is true" + ) + + merge_performed = data.get("merge_performed") + merge_blocker = _clean(data.get("merge_blocker")) + if merge_performed is True and merge_blocker: + contradictions.append("merge_performed true but merge_blocker also set") + if merge_performed is not True and not merge_blocker and data.get("claims_merged") is True: + contradictions.append("report claims merge but merge_performed is not true") + + workspace_clean = data.get("workspace_clean") + workspace_classification = _clean(data.get("workspace_classification")) + if workspace_clean is False and not workspace_classification: + reasons.append( + "workspace is not clean but workspace_classification is missing" + ) + + if review_submitted is False and data.get("claims_review_submitted") is True: + contradictions.append( + "claims_review_submitted true but review_submitted is false" + ) + + all_reasons = reasons + contradictions + proven = not all_reasons + return { + "proven": proven, + "block": not proven, + "reasons": all_reasons, + "contradictions": contradictions, + "ledger_verdict": ledger_verdict, + } \ No newline at end of file