diff --git a/project/ticket-450/README.md b/project/ticket-450/README.md new file mode 100644 index 00000000..0ce263c0 --- /dev/null +++ b/project/ticket-450/README.md @@ -0,0 +1,35 @@ +# Ticket 450: Upgrade diff repair, search-replace synthesis, governed workspace support, and scan artifact filtering + +- **ID**: ticket-450 +- **Owner**: antigravity +- **Status**: IN_PROGRESS +- **Workflow state**: EDIT +- **Created**: 2026-10-16 + +SESSION_EXECUTION_AUTHORIZATION: user requests autonomous execution to upgrade diff repair, search/replace diff synthesis, governed ticket worktree support, scan artifact filtering, context slicing for module refactors, and verification gate auto-detection. + +## Goal and scope + +Upgrade `koru` patch mode and refactoring pipeline so module splitting, deduplication, and governed ticket worktree execution succeed reliably: +1. Add `synthesize_diff_from_search_replace` and fuzzy hunk repair in `src/koru/queue/diff_repair.py`, plus fixes for fenced blocks, new/deleted file `/dev/null` headers, and inner `--- ` comment preservation. +2. Wire `project` and `known_files` into `extract_unified_diff` across `patch_mode.py`, `transaction/preflight.py`, and `patch_retry.py`. +3. Support admitted governed ticket worktrees (`_is_admitted_ticket_worktree`) and `ticket-` staging worktree prefixes in `src/koru/queue/workspace.py`. +4. Stop scan artifacts (`analysis.toon.yaml`, `duplication.toon.yaml`, etc.) from polluting target `files` in `src/koru/scan_artifacts.py`, `src/koru/queue/context_files.py`, and `src/koru/queue/patch_retry.py`, and add `duplication.toon.yaml` parsing. +5. Prevent destructive AST truncation on module-split/refactor prompts in `src/koru/queue/context_slicing.py`. +6. Auto-detect safe fallback verification gates (`pytest`, `cargo test`, `npm test`) in `src/koru/queue/verify/legacy.py`. +7. Add unit tests in `tests/test_diff_repair_and_refactor.py`. + +## Acceptance criteria + +- [x] AC-01: `diff_repair.py` synthesizes unified diffs from search/replace and new-file blocks, repairs fuzzy hunks against disk, emits `/dev/null` headers for added/deleted files, and preserves valid removed `--- ` lines inside hunks. +- [x] AC-02: `patch_mode.py`, `transaction/preflight.py`, and `patch_retry.py` pass `project` and `known_files` to `extract_unified_diff`. +- [x] AC-03: `workspace.py` admits governed ticket worktrees via `_is_admitted_ticket_worktree` and prefixes staging worktrees with `ticket-`. +- [x] AC-04: `scan_artifacts.py`, `context_files.py`, and `patch_retry.py` exclude read-only scan artifacts from target files and parse `duplication.toon.yaml`. +- [x] AC-05: `context_slicing.py` avoids false-positive English symbol extraction and bypasses AST slicing for refactor/split prompts up to 64,000 chars. +- [x] AC-06: `verify/legacy.py` auto-detects project test commands when no explicit verify gate is configured. +- [x] AC-07: Unit tests in `tests/test_diff_repair_and_refactor.py` and governance checks pass. + +## Tracking boundary + +This directory contains the minimal reviewed intent. Optional participant prose +and raw command logs are not required delivery output. diff --git a/project/ticket-450/intent.json b/project/ticket-450/intent.json new file mode 100644 index 00000000..267b0200 --- /dev/null +++ b/project/ticket-450/intent.json @@ -0,0 +1,95 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-450", + "summary": "Upgrade diff repair, search-replace synthesis, governed workspace support, and scan artifact filtering", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "health" + }, + "allowedPaths": [ + "project/ticket-450/**", + "TODO.md", + "project/TICKETS.md", + "src/koru/queue/context_files.py", + "src/koru/queue/context_slicing.py", + "src/koru/queue/diff_repair.py", + "src/koru/queue/patch_mode.py", + "src/koru/queue/patch_retry.py", + "src/koru/queue/transaction/preflight.py", + "src/koru/queue/verify/legacy.py", + "src/koru/queue/workspace.py", + "src/koru/scan_artifacts.py", + "tests/test_diff_repair_and_refactor.py" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "febdb1e2beac504e7ca939b89537f9c507d5dcc5", + "targetBranch": "main", + "outcome": "Upgrade koru diff repair, search/replace diff synthesis, governed ticket worktree admission, scan artifact filtering, duplication.toon.yaml parsing, refactor context slicing, and verification gate auto-detection.", + "nonGoals": [ + "Do not modify opencode transport or runner modules owned by ticket-446" + ], + "complexity": "L", + "estimatedMinutes": 60, + "budgets": { + "maxImplementationFiles": 12, + "maxAffectedComponents": 3, + "maxPublicInterfaceChanges": 2, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Synthesize unified diffs from search/replace and new-file blocks and repair fuzzy hunks in diff_repair.py; admit governed ticket worktrees in workspace.py; exclude read-only scan artifacts from target files and parse duplication.toon.yaml in scan_artifacts.py; bypass destructive AST slicing on refactor prompts in context_slicing.py; and auto-detect fallback test gates in verify/legacy.py.", + "components": [ + { + "name": "queue-patch-and-context", + "paths": [ + "src/koru/queue/diff_repair.py", + "src/koru/queue/patch_mode.py", + "src/koru/queue/patch_retry.py", + "src/koru/queue/transaction/preflight.py", + "src/koru/queue/workspace.py", + "src/koru/queue/context_files.py", + "src/koru/queue/context_slicing.py", + "src/koru/queue/verify/legacy.py", + "tests/test_diff_repair_and_refactor.py" + ] + }, + { + "name": "scan-artifacts", + "paths": [ + "src/koru/scan_artifacts.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert commit for ticket-450." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-01", + "commands": [ + "pytest -p no:wellmanifest_governance tests/test_diff_repair_and_refactor.py", + "./project/governance-check.sh" + ], + "evidence": "Unit tests for diff repair, search/replace synthesis, governed workspace support, scan artifact filtering, context slicing, and verify auto-detection." + } + ] + } +} diff --git a/src/koru/queue/context_files.py b/src/koru/queue/context_files.py index 145269f8..ba9f500d 100644 --- a/src/koru/queue/context_files.py +++ b/src/koru/queue/context_files.py @@ -113,6 +113,30 @@ def _read_file_content(project: Path, rel_path: str) -> str | None: return None +# Default maximum context characters for large refactor/context assembly +DEFAULT_MAX_CONTEXT_CHARS: int = 64_000 + +_READONLY_SCAN_ARTIFACT_NAMES: frozenset[str] = frozenset( + { + "analysis.toon.yaml", + "analysis.toon", + "duplication.toon.yaml", + "duplication.yaml", + "calls.yaml", + "calls.toon.yaml", + "map.toon.yaml", + "project.toon.yaml", + } +) + + +def _is_readonly_scan_artifact(rel_path: str) -> bool: + norm = str(rel_path or "").replace("\\", "/").strip().lstrip("./") + if not norm: + return False + return norm.rsplit("/", 1)[-1].lower() in _READONLY_SCAN_ARTIFACT_NAMES + + def _collect_auto_files(project: Path) -> list[str]: """Return relative paths for the auto-include file set.""" found: list[str] = [] @@ -123,7 +147,7 @@ def _collect_auto_files(project: Path) -> list[str]: for glob_pattern in _AUTO_INCLUDE_GLOBS: for match in sorted(project.glob(glob_pattern)): rel = match.relative_to(project).as_posix() - if rel not in found and not _is_excluded(rel): + if rel not in found and not _is_excluded(rel) and not _is_readonly_scan_artifact(rel): found.append(rel) return found @@ -138,7 +162,7 @@ def _collect_glob_files(project: Path, globs: list[str]) -> list[str]: if not match.is_file(): continue rel = match.relative_to(project).as_posix() - if rel in seen or _is_excluded(rel): + if rel in seen or _is_excluded(rel) or _is_readonly_scan_artifact(rel): continue seen.add(rel) found.append(rel) @@ -156,6 +180,34 @@ def _context_nothing_requested(request: dict[str, Any]) -> bool: ) +_PROMPT_PATH_RE = ( + r"(?:^|[\s`'\"(])([A-Za-z0-9_./-]+\.(?:py|ts|tsx|js|jsx|mjs|cjs|rs|go|sh|yaml|yml|json|toml|md))(?=$|[\s`'\",.:;)])" +) + + +def _extract_prompt_files(project: Path, prompt: str) -> list[str]: + """Discover existing project files explicitly named in *prompt*.""" + if not prompt: + return [] + import re + + found: list[str] = [] + seen: set[str] = set() + for match in re.finditer(_PROMPT_PATH_RE, prompt): + raw = match.group(1).strip().lstrip("./") + if not raw or raw in seen or ".." in raw.split("/"): + continue + candidate = (project / raw).resolve() + try: + rel = candidate.relative_to(project.resolve()).as_posix() + except ValueError: + continue + if candidate.is_file() and rel not in seen and not _is_excluded(rel) and not _is_readonly_scan_artifact(rel): + seen.add(rel) + found.append(rel) + return found + + def _context_files_to_include(project: Path, request: dict[str, Any]) -> list[str]: """Ordered unique relative paths to embed from request inputs. @@ -173,7 +225,11 @@ def _context_files_to_include(project: Path, request: dict[str, Any]) -> list[st seen: set[str] = set() def _add(rel: str) -> None: - if rel not in seen and not _is_excluded(rel): + if ( + rel not in seen + and not _is_excluded(rel) + and not _is_readonly_scan_artifact(rel) + ): seen.add(rel) files_to_include.append(rel) @@ -181,6 +237,8 @@ def _add(rel: str) -> None: _add(rel) for rel in context_files: _add(rel) + for rel in _extract_prompt_files(project, str(request.get("prompt") or "")): + _add(rel) if context_globs: for rel in _collect_glob_files(project, context_globs): _add(rel) @@ -188,3 +246,4 @@ def _add(rel: str) -> None: for rel in _collect_auto_files(project): _add(rel) return files_to_include + diff --git a/src/koru/queue/context_slicing.py b/src/koru/queue/context_slicing.py index 061451f4..522b0743 100644 --- a/src/koru/queue/context_slicing.py +++ b/src/koru/queue/context_slicing.py @@ -54,16 +54,107 @@ def _extract_line_from_corpus(filename: str, text_corpus: str) -> int | None: return None +_SYMBOL_STOPWORDS = frozenset( + { + "hierarchy", + "helpers", + "helper", + "module", + "modules", + "submodules", + "submodule", + "into", + "from", + "methods", + "method", + "functions", + "function", + "classes", + "class", + "file", + "files", + "code", + "logic", + "group", + "groups", + "name", + "names", + "definitions", + "definition", + "body", + "bodies", + "that", + "this", + "with", + "without", + "across", + "shared", + } +) + +_MODULE_REFACTOR_MAX_CHARS: int = 64_000 + +_MODULE_REFACTOR_RE = re.compile( + r"\b(?:god\s+module|split\b|submodules?\b|deduplicat\w*|duplicate\s+(?:class|group|code)|extract\s+(?:shared|cohesive|submodules?|helpers?))\b", + re.IGNORECASE, +) + +_FUNCTION_SMELL_RE = re.compile( + r"(?:`(?:God Function|Feature Envy|Long Method|Shotgun Surgery|Data Clump):\s*[a-zA-Z0-9_]+`|`[a-zA-Z0-9_]+`\s+with\s+CC=)", +) + + def _extract_symbol_from_corpus(text_corpus: str) -> str | None: for pat in _SYMBOL_PATTERNS: - m_sym = re.search(pat, text_corpus) - if m_sym: + for m_sym in re.finditer(pat, text_corpus): candidate = m_sym.group(1).strip() - if candidate and not candidate.isdigit() and len(candidate) > 1: + if ( + candidate + and not candidate.isdigit() + and len(candidate) > 1 + and candidate.lower() not in _SYMBOL_STOPWORDS + ): return candidate return None +def _extract_prompt_symbols(text_corpus: str) -> list[str]: + """Return all non-stopword symbol candidates found in *text_corpus*.""" + found: list[str] = [] + for pat in _SYMBOL_PATTERNS: + for m_sym in re.finditer(pat, text_corpus): + candidate = m_sym.group(1).strip() + if ( + candidate + and not candidate.isdigit() + and len(candidate) > 1 + and candidate.lower() not in _SYMBOL_STOPWORDS + and candidate not in found + ): + found.append(candidate) + return found + + +def _is_module_refactor_request(request: dict[str, Any]) -> bool: + if request.get("target_symbol") or request.get("target_line") is not None: + return False + text_corpus = " ".join( + [ + str(request.get("prompt") or ""), + str(request.get("ticket_description") or ""), + ] + ) + if not text_corpus.strip(): + return False + if _FUNCTION_SMELL_RE.search(text_corpus): + return False + if _MODULE_REFACTOR_RE.search(text_corpus): + return True + if "refactor" in text_corpus.lower() and _extract_symbol_from_corpus(text_corpus) is None: + return True + return False + + def _resolve_target_symbol_and_line(rel_path: str, request: dict[str, Any]) -> _TargetRef: """Resolve target symbol name and line number for rel_path from request metadata.""" if (direct := _resolve_target_from_request_meta(request)) is not None: @@ -156,11 +247,15 @@ def _focused_file_slice( When the file exceeds ``LARGE_FILE_SLICE_THRESHOLD_CHARS`` and target symbol or line metadata is present in ``request``, a focused slice is generated instead of dumping the entire file; otherwise the content is - returned unchanged. + returned unchanged. Module-level split/refactor requests preserve full + files up to ``_MODULE_REFACTOR_MAX_CHARS`` (64,000 chars). """ if not request or len(content) <= LARGE_FILE_SLICE_THRESHOLD_CHARS: return _FocusedSlice(content, None) + if len(content) <= _MODULE_REFACTOR_MAX_CHARS and _is_module_refactor_request(request): + return _FocusedSlice(content, None) + target = _resolve_target_symbol_and_line(rel, request) if target.symbol and lang == "py": code_slice = _extract_python_symbol_slice(content, target.symbol) @@ -176,3 +271,25 @@ def _focused_file_slice( f"focused slice around line {target.line}, lines {code_slice.start_line}-{code_slice.end_line}", ) return _FocusedSlice(content, None) + + +def slice_file_content( + rel: str, + content: str, + prompt_or_lang: str | dict[str, Any] = "", + request: dict[str, Any] | None = None, + *, + max_chars: int = _MODULE_REFACTOR_MAX_CHARS, +) -> str | _FocusedSlice: + """Slice *content* for *rel* given either a prompt/request or (lang, request).""" + if request is not None: + return _focused_file_slice(rel, content, str(prompt_or_lang), request) + if isinstance(prompt_or_lang, dict): + req = prompt_or_lang + else: + req = {"prompt": str(prompt_or_lang)} + lang = "py" if str(rel).endswith(".py") else "" + if len(content) <= max_chars and _is_module_refactor_request(req): + return content + return _focused_file_slice(rel, content, lang, req).content + diff --git a/src/koru/queue/diff_repair.py b/src/koru/queue/diff_repair.py index 534ea298..a5e9ec36 100644 --- a/src/koru/queue/diff_repair.py +++ b/src/koru/queue/diff_repair.py @@ -7,20 +7,25 @@ could be context, an addition or a deletion, and guessing would risk writing the wrong change into someone's code. -Pure text handling: no filesystem, no git, no network. +Also supports deterministic synthesis of unified diffs from SEARCH/REPLACE blocks +and fuzzy hunk repair when a project path is provided. """ from __future__ import annotations +import difflib +import os import re +from collections.abc import Iterable +from pathlib import Path _FENCE_RE = re.compile( - r"```(?:diff|patch)?\s*\n(?P.*?)(?:\n```|\Z)", + r"```[a-zA-Z0-9_-]*\s*\n(?P.*?)(?:\n```|\Z)", re.DOTALL, ) -_DIFF_START_RE = re.compile(r"^(diff --git |--- )", re.MULTILINE) +_DIFF_START_RE = re.compile(r"^(diff --git |--- (?:a/|/dev/null))", re.MULTILINE) _HUNK_RE = re.compile( @@ -34,18 +39,32 @@ _SYMLINK_MODE_RE = re.compile(r"^(?:new file mode|new mode) 120000\s*$", re.MULTILINE) +_SEARCH_REPLACE_RE = re.compile( + r"<{4,9}\s*SEARCH[ \t]*\r?\n(?P.*?)\r?\n={4,9}[ \t]*\r?\n(?P.*?)\r?\n>{4,9}\s*REPLACE", + re.DOTALL, +) + +_FILE_HEADER_LINE_RE = re.compile( + r"^(?:#{1,4}\s*(?:(?:NEW\s+)?FILE\s*:\s*)?|FILE\s*:\s*|File\s*:\s*|\*\*File\s*:\*\*\s*)?`?(?P[A-Za-z0-9_./-]+\.[A-Za-z0-9_]+)`?\s*$" +) + +_NEW_FILE_SECTION_RE = re.compile( + r"(?:^|\n)(?:#{1,4}\s*(?:NEW\s+)?FILE\s*:\s*`?(?P[A-Za-z0-9_./-]+\.[A-Za-z0-9_]+)`?[ \t]*\n\s*```[a-zA-Z0-9_-]*[ \t]*\n(?P.*?)\n```" + r"|```[a-zA-Z0-9_-]+(?:\s+|:)(?P[A-Za-z0-9_./-]+\.[A-Za-z0-9_]+)[ \t]*\n(?P.*?)\n```)", + re.DOTALL, +) + + def _normalize_path_prefixes( body: str, *, - known_files: tuple[str, ...] | list[str] | set[str] | None = None, - project: str | None = None, + known_files: Iterable[str] | None = None, + project: str | Path | None = None, ) -> str: """Normalize path prefixes if files match known files or exist under project/src/.""" - candidates: set[str] = set(known_files or ()) + candidates: set[str] = set(str(f) for f in (known_files or ())) if project: try: - import os - from pathlib import Path proj_p = Path(project) src_p = proj_p / "src" if src_p.is_dir(): @@ -60,10 +79,24 @@ def _normalize_path_prefixes( return body src_map: dict[str, str] = {} + basename_counts: dict[str, int] = {} + for f in candidates: + f_norm = f.replace("\\", "/").lstrip("./") + base = f_norm.rsplit("/", 1)[-1] + basename_counts[base] = basename_counts.get(base, 0) + 1 + + proj_root = Path(project) if project else None for f in candidates: - f_norm = f.replace("\\", "/") + f_norm = f.replace("\\", "/").lstrip("./") if f_norm.startswith("src/"): src_map[f_norm[4:]] = f_norm + if "/" in f_norm: + suffix = f_norm.split("/", 1)[1] + if proj_root is None or not (proj_root / suffix).exists(): + src_map.setdefault(suffix, f_norm) + base = f_norm.rsplit("/", 1)[-1] + if basename_counts.get(base) == 1 and (proj_root is None or not (proj_root / base).exists()): + src_map.setdefault(base, f_norm) if not src_map: return body @@ -89,20 +122,628 @@ def _replace_new_file_header(m: re.Match) -> str: return body +def _resolve_candidate_path( + raw_path: str, + project_path: Path | None, + known_list: list[str], +) -> str: + cleaned = raw_path.strip().strip("`'\"").replace("\\", "/") + if cleaned.startswith(("a/", "b/")): + cleaned = cleaned[2:] + cleaned = cleaned.lstrip("./") + if project_path is not None: + if (project_path / cleaned).exists(): + return cleaned + if (project_path / "src" / cleaned).exists(): + return f"src/{cleaned}" + for k in known_list: + k_norm = k.replace("\\", "/") + if k_norm == cleaned or k_norm.endswith(f"/{cleaned}"): + return k_norm + return cleaned + + +def _apply_search_replace_to_text(content: str, search: str, replace: str) -> str | None: + """Apply one SEARCH/REPLACE block to content, with whitespace-tolerant fallback.""" + if not search: + return replace + ("\n" if replace and not replace.endswith("\n") else "") + if search in content: + return content.replace(search, replace, 1) + + content_lines = content.splitlines(keepends=True) + search_lines = search.splitlines() + replace_lines = replace.splitlines() + if not search_lines: + return None + + # Line-by-line rstrip match + n = len(search_lines) + search_rstrip = [s.rstrip() for s in search_lines] + for i in range(len(content_lines) - n + 1): + if [c.rstrip("\r\n").rstrip() for c in content_lines[i : i + n]] == search_rstrip: + ending = "\n" + if content_lines[i].endswith("\r\n"): + ending = "\r\n" + replacement_text = "".join(r + ending for r in replace_lines) + return "".join(content_lines[:i]) + replacement_text + "".join(content_lines[i + n :]) + + # Line-by-line strip match (ignoring leading/trailing whitespace differences) + search_strip = [s.strip() for s in search_lines if s.strip()] + if search_strip: + for i in range(len(content_lines) - n + 1): + window_strip = [c.strip() for c in content_lines[i : i + n] if c.strip()] + if window_strip == search_strip: + ending = "\n" + replacement_text = "".join(r + ending for r in replace_lines) + return "".join(content_lines[:i]) + replacement_text + "".join(content_lines[i + n :]) + return None + + +def _format_unified_diff_for_file(rel_path: str, old_text: str | None, new_text: str) -> str: + rel_norm = rel_path.replace("\\", "/").lstrip("./") + new_lines = new_text.splitlines() + if old_text is None: + if not new_lines: + return ( + f"diff --git a/{rel_norm} b/{rel_norm}\n" + "new file mode 100644\n" + "--- /dev/null\n" + f"+++ b/{rel_norm}\n" + ) + body = "\n".join(f"+{line}" for line in new_lines) + return ( + f"diff --git a/{rel_norm} b/{rel_norm}\n" + "new file mode 100644\n" + "--- /dev/null\n" + f"+++ b/{rel_norm}\n" + f"@@ -0,0 +1,{len(new_lines)} @@\n" + f"{body}\n" + ) + + old_lines = old_text.splitlines() + if old_lines == new_lines: + return "" + diff_lines = list( + difflib.unified_diff( + old_lines, + new_lines, + fromfile=f"a/{rel_norm}", + tofile=f"b/{rel_norm}", + lineterm="", + ) + ) + if not diff_lines: + return "" + return f"diff --git a/{rel_norm} b/{rel_norm}\n" + "\n".join(diff_lines) + "\n" + + +def synthesize_diff_from_search_replace( + text: str, + project: Path | str | None, + known_files: Iterable[str] | None = None, +) -> str: + """Synthesize a valid git unified diff from SEARCH/REPLACE and new-file blocks.""" + if not text or project is None: + return "" + project_path = Path(project) + known_list = [str(f) for f in (known_files or ())] + + has_sr = bool(_SEARCH_REPLACE_RE.search(text)) + has_new_file = bool(_NEW_FILE_SECTION_RE.search(text)) + if not has_sr and not has_new_file: + # Fallback: when a single known target file is being edited and the model + # emitted a full-file replacement inside a code fence instead of a diff. + if len(known_list) == 1 and not _DIFF_START_RE.search(text): + rel = _resolve_candidate_path(known_list[0], project_path, known_list) + disk_file = project_path / rel + if disk_file.is_file(): + try: + orig_text = disk_file.read_text(encoding="utf-8") + except OSError: + orig_text = "" + orig_lines = orig_text.splitlines() + for m_fence in _FENCE_RE.finditer(text): + candidate_body = m_fence.group("body").strip("\r\n") + cand_lines = candidate_body.splitlines() + if len(cand_lines) < max(3, int(len(orig_lines) * 0.4)): + continue + if rel.endswith(".py"): + import ast + + try: + ast.parse(candidate_body) + except SyntaxError: + continue + # Verify some structural overlap with the original file + orig_set = {l.strip() for l in orig_lines if len(l.strip()) > 6} + cand_set = {l.strip() for l in cand_lines if len(l.strip()) > 6} + if orig_set and len(orig_set & cand_set) / max(1, min(len(orig_set), len(cand_set))) >= 0.25: + return _format_unified_diff_for_file(rel, orig_text, candidate_body + "\n") + return "" + + file_states: dict[str, tuple[str | None, str]] = {} + + def _get_current(rel: str) -> tuple[str | None, str]: + if rel in file_states: + return file_states[rel] + disk_file = project_path / rel + if disk_file.is_file(): + try: + orig = disk_file.read_text(encoding="utf-8") + except OSError: + orig = "" + file_states[rel] = (orig, orig) + else: + file_states[rel] = (None, "") + return file_states[rel] + + if has_sr: + last_end = 0 + current_path: str | None = None + for match in _SEARCH_REPLACE_RE.finditer(text): + preceding = text[last_end : match.start()] + last_end = match.end() + for raw_line in reversed(preceding.splitlines()): + line = raw_line.strip() + if not line or line.startswith("```"): + if line.startswith("```"): + parts = line.lstrip("`").strip().split() + if len(parts) >= 2 and "." in parts[-1]: + current_path = _resolve_candidate_path(parts[-1], project_path, known_list) + break + if len(parts) == 1 and ("/" in parts[0] or "." in parts[0]): + current_path = _resolve_candidate_path(parts[0], project_path, known_list) + break + continue + m_hdr = _FILE_HEADER_LINE_RE.match(line) + if m_hdr: + current_path = _resolve_candidate_path(m_hdr.group("path"), project_path, known_list) + break + + target_rel = current_path + if not target_rel and len(known_list) == 1: + target_rel = _resolve_candidate_path(known_list[0], project_path, known_list) + if not target_rel: + # Probe all known_files to see which one contains the search block + search_body = match.group("search") + for candidate in known_list: + cand_rel = _resolve_candidate_path(candidate, project_path, known_list) + orig, cur = _get_current(cand_rel) + if orig is not None and _apply_search_replace_to_text(cur, search_body, match.group("replace")) is not None: + target_rel = cand_rel + break + if not target_rel: + continue + + orig, cur = _get_current(target_rel) + updated = _apply_search_replace_to_text(cur, match.group("search"), match.group("replace")) + if updated is not None: + file_states[target_rel] = (orig, updated) + + if has_new_file: + for m_nf in _NEW_FILE_SECTION_RE.finditer(text): + raw_p = m_nf.group("header_path") or m_nf.group("fence_path") + body = m_nf.group("header_body") if m_nf.group("header_path") else m_nf.group("fence_body") + if not raw_p or body is None: + continue + if _SEARCH_REPLACE_RE.search(body) or _DIFF_START_RE.search(body): + continue + rel = _resolve_candidate_path(raw_p, project_path, known_list) + if rel in file_states: + continue + orig, _ = _get_current(rel) + new_content = body + ("\n" if not body.endswith("\n") else "") + file_states[rel] = (orig, new_content) + + diffs: list[str] = [] + for rel, (orig, updated) in file_states.items(): + piece = _format_unified_diff_for_file(rel, orig, updated) + if piece: + diffs.append(piece) + return "".join(diffs) + + +def _find_top_level_constant_insert_pos(working: list[str]) -> int: + """Find a clean module-level insertion line after docstring and imports.""" + idx = 0 + n = len(working) + while idx < n and (not working[idx].strip() or working[idx].strip().startswith("#")): + idx += 1 + if idx < n and working[idx].lstrip().startswith(('"""', "'''")): + quote = working[idx].lstrip()[:3] + if working[idx].lstrip()[3:].count(quote) == 0: + idx += 1 + while idx < n and quote not in working[idx]: + idx += 1 + idx += 1 + last_import_end = idx + in_parens = False + while idx < n: + stripped = working[idx].strip() + if in_parens: + if ")" in stripped: + in_parens = False + idx += 1 + last_import_end = idx + continue + if not stripped or stripped.startswith("#"): + idx += 1 + continue + if stripped.startswith(("import ", "from ")): + if "(" in stripped and ")" not in stripped: + in_parens = True + idx += 1 + last_import_end = idx + continue + break + return last_import_end + + +def _apply_edit_subblocks_fallback( + working: list[str], + new_chunk_template: list[tuple[str, str]], + expected_start: int, +) -> tuple[list[str], bool]: + """Decompose a hunk whose full context failed to match into atomic (-/+) edit blocks + and apply each one against *working* using nearest-context or unique-line anchoring.""" + changed_any = False + i = 0 + n_tpl = len(new_chunk_template) + pre_ctx: list[str] = [] + + while i < n_tpl: + kind, text = new_chunk_template[i] + if kind == " ": + pre_ctx.append(text) + i += 1 + continue + + removed: list[str] = [] + added: list[str] = [] + while i < n_tpl and new_chunk_template[i][0] in ("-", "+"): + k, t = new_chunk_template[i] + if k == "-": + removed.append(t) + else: + added.append(t) + i += 1 + + post_ctx: list[str] = [] + j = i + while j < n_tpl and new_chunk_template[j][0] == " " and len(post_ctx) < 3: + post_ctx.append(new_chunk_template[j][1]) + j += 1 + + if removed: + rem_n = len(removed) + rem_strip = [r.strip() for r in removed] + best_idx: int | None = None + best_score: tuple[int, int] = (-1, -(len(working) + 1000)) + + for cand in range(len(working) - rem_n + 1): + if [w.strip() for w in working[cand : cand + rem_n]] == rem_strip: + ctx_hits = 0 + if pre_ctx and cand >= 1 and working[cand - 1].strip() == pre_ctx[-1].strip(): + ctx_hits += 2 + if len(pre_ctx) >= 2 and cand >= 2 and working[cand - 2].strip() == pre_ctx[-2].strip(): + ctx_hits += 1 + if post_ctx and cand + rem_n < len(working) and working[cand + rem_n].strip() == post_ctx[0].strip(): + ctx_hits += 2 + if len(post_ctx) >= 2 and cand + rem_n + 1 < len(working) and working[cand + rem_n + 1].strip() == post_ctx[1].strip(): + ctx_hits += 1 + exact_bonus = 1 if working[cand : cand + rem_n] == removed else 0 + score = (ctx_hits * 2 + exact_bonus, -abs(cand - expected_start)) + if score > best_score: + best_score = score + best_idx = cand + + if best_idx is not None: + working[best_idx : best_idx + rem_n] = added + expected_start = best_idx + len(added) + changed_any = True + elif len(removed) == len(added) and len(removed) > 1: + # Non-contiguous removed lines bundled into one block: apply line by line + for r_line, a_line in zip(removed, added): + r_s = r_line.strip() + if not r_s: + continue + line_idx: int | None = None + best_dist = len(working) + 1000 + for cand, w_line in enumerate(working): + if w_line.strip() == r_s: + dist = abs(cand - expected_start) + if dist < best_dist: + best_dist = dist + line_idx = cand + if line_idx is not None: + working[line_idx] = a_line + expected_start = line_idx + 1 + changed_any = True + else: + # Check if added lines are already present (duplicate hunk) + add_strip = [a.strip() for a in added if a.strip()] + if add_strip: + w_strips = {w.strip() for w in working} + if all(a_s in w_strips for a_s in add_strip): + changed_any = True + elif added: + # Pure addition block + add_n = len(added) + add_strip = [a.strip() for a in added] + already_present = any( + [w.strip() for w in working[cand : cand + add_n]] == add_strip + for cand in range(max(0, len(working) - add_n + 1)) + ) + if already_present: + changed_any = True + else: + ins_pos: int | None = None + if pre_ctx: + anchor = pre_ctx[-1].strip() + if anchor: + best_dist = len(working) + 1000 + for cand, w_line in enumerate(working): + if w_line.strip() == anchor: + dist = abs(cand + 1 - expected_start) + if dist < best_dist: + best_dist = dist + ins_pos = cand + 1 + if ins_pos is None and post_ctx: + anchor = post_ctx[0].strip() + if anchor: + best_dist = len(working) + 1000 + for cand, w_line in enumerate(working): + if w_line.strip() == anchor: + dist = abs(cand - expected_start) + if dist < best_dist: + best_dist = dist + ins_pos = cand + if ins_pos is None and expected_start <= 30: + ins_pos = _find_top_level_constant_insert_pos(working) + if ins_pos is not None: + working[ins_pos:ins_pos] = added + expected_start = ins_pos + len(added) + changed_any = True + + pre_ctx = [] + + return working, changed_any + + +def _apply_hunks_with_fuzzy_fallback(orig_lines: list[str], hunk_blocks: list[list[str]]) -> tuple[list[str], bool] | None: + """Apply parsed hunk bodies to orig_lines; return (new_lines, used_fuzzy) or None.""" + working = list(orig_lines) + offset = 0 + used_fuzzy = False + applied_any = False + + for hunk in hunk_blocks: + if not hunk: + continue + m = _HUNK_RE.match(hunk[0]) + expected_start = (int(m.group("old_start")) - 1 + offset) if m else 0 + old_chunk: list[str] = [] + new_chunk_template: list[tuple[str, str]] = [] + for line in hunk[1:]: + if line.startswith("\\"): + continue + if line.startswith("-"): + old_chunk.append(line[1:]) + new_chunk_template.append(("-", line[1:])) + elif line.startswith("+"): + new_chunk_template.append(("+", line[1:])) + elif line.startswith(" "): + old_chunk.append(line[1:]) + new_chunk_template.append((" ", line[1:])) + else: + old_chunk.append(line) + new_chunk_template.append((" ", line)) + + if not old_chunk: + insert_pos = max(0, min(len(working), expected_start)) + added = [text for kind, text in new_chunk_template if kind != "-"] + working[insert_pos:insert_pos] = added + offset += len(added) + applied_any = True + continue + + n = len(old_chunk) + match_pos: int | None = None + if 0 <= expected_start <= len(working) - n and working[expected_start : expected_start + n] == old_chunk: + match_pos = expected_start + else: + used_fuzzy = True + # Search outward from expected_start for exact match + best_dist = len(working) + 1 + for i in range(len(working) - n + 1): + if working[i : i + n] == old_chunk: + dist = abs(i - expected_start) + if dist < best_dist: + best_dist = dist + match_pos = i + # Rstrip match + if match_pos is None: + old_rstrip = [x.rstrip() for x in old_chunk] + for i in range(len(working) - n + 1): + if [w.rstrip() for w in working[i : i + n]] == old_rstrip: + dist = abs(i - expected_start) + if dist < best_dist: + best_dist = dist + match_pos = i + # Strip match + if match_pos is None: + old_strip = [x.strip() for x in old_chunk] + for i in range(len(working) - n + 1): + if [w.strip() for w in working[i : i + n]] == old_strip: + dist = abs(i - expected_start) + if dist < best_dist: + best_dist = dist + match_pos = i + + if match_pos is not None and match_pos >= 0: + matched_disk_lines = working[match_pos : match_pos + n] + disk_iter = iter(matched_disk_lines) + replacement: list[str] = [] + for kind, text in new_chunk_template: + if kind == " ": + replacement.append(next(disk_iter, text)) + elif kind == "-": + next(disk_iter, None) + elif kind == "+": + replacement.append(text) + working[match_pos : match_pos + n] = replacement + offset += len(replacement) - n + applied_any = True + continue + + # Fallback: decompose the hunk into atomic edit subblocks + working, sub_applied = _apply_edit_subblocks_fallback( + working, new_chunk_template, max(0, expected_start) + ) + if sub_applied: + used_fuzzy = True + applied_any = True + offset = len(working) - len(orig_lines) + + if not applied_any: + return None + return working, used_fuzzy + + +def _repair_diff_against_project(diff: str, project: Path | str) -> str: + """When a unified diff has inaccurate hunk line numbers, context lines, or + multiple diff sections targeting the same file, re-anchor and merge hunks + against the actual files in *project* via difflib.""" + proj_path = Path(project) + lines = diff.splitlines() + sections: list[list[str]] = [] + current: list[str] = [] + for line in lines: + if line.startswith("diff --git ") and current: + sections.append(current) + current = [line] + else: + current.append(line) + if current: + sections.append(current) + + # Group sections touching the same existing file so multiple diff --git blocks + # for the same path are merged into one clean unified diff. + grouped_hunks: dict[str, list[list[str]]] = {} + grouped_raw: dict[str, list[list[str]]] = {} + section_order: list[tuple[str, str | list[str]]] = [] + + for sec in sections: + header_match = _GIT_HEADER_RE.match(sec[0]) if sec else None + if not header_match: + section_order.append(("raw", sec)) + continue + old_rel = header_match.group("old") + new_rel = header_match.group("new") + is_new_or_del = any( + l.startswith(("new file mode", "deleted file mode", "--- /dev/null", "+++ /dev/null")) + for l in sec[:6] + ) or any(l.startswith("@@ -0,0") for l in sec) + target_file = proj_path / old_rel + if is_new_or_del or old_rel != new_rel: + section_order.append(("raw", sec)) + continue + if not target_file.is_file(): + section_order.append(("missing_raw", sec)) + continue + + hunks: list[list[str]] = [] + cur_hunk: list[str] = [] + for l in sec: + if l.startswith("@@"): + if cur_hunk: + hunks.append(cur_hunk) + cur_hunk = [l] + elif cur_hunk: + cur_hunk.append(l) + if cur_hunk: + hunks.append(cur_hunk) + if not hunks: + section_order.append(("missing_raw", sec)) + continue + + if old_rel not in grouped_hunks: + grouped_hunks[old_rel] = [] + grouped_raw[old_rel] = [] + section_order.append(("file", old_rel)) + grouped_hunks[old_rel].extend(hunks) + grouped_raw[old_rel].append(sec) + + repaired_sections: list[str] = [] + fallback_sections: list[str] = [] + for kind, item in section_order: + if kind == "raw": + assert isinstance(item, list) + chunk = "\n".join(item) + "\n" + repaired_sections.append(chunk) + fallback_sections.append(chunk) + continue + if kind == "missing_raw": + assert isinstance(item, list) + fallback_sections.append("\n".join(item) + "\n") + continue + + old_rel = str(item) + raw_secs = grouped_raw[old_rel] + all_hunks = grouped_hunks[old_rel] + target_file = proj_path / old_rel + try: + orig_text = target_file.read_text(encoding="utf-8") + except OSError: + for s in raw_secs: + fallback_sections.append("\n".join(s) + "\n") + continue + + orig_lines = orig_text.splitlines() + applied = _apply_hunks_with_fuzzy_fallback(orig_lines, all_hunks) + if applied is None: + for s in raw_secs: + fallback_sections.append("\n".join(s) + "\n") + continue + new_lines, used_fuzzy = applied + if not used_fuzzy and len(raw_secs) == 1: + chunk = "\n".join(raw_secs[0]) + "\n" + repaired_sections.append(chunk) + fallback_sections.append(chunk) + continue + regenerated = _format_unified_diff_for_file(old_rel, orig_text, "\n".join(new_lines) + "\n") + if regenerated: + repaired_sections.append(regenerated) + fallback_sections.append(regenerated) + else: + for s in raw_secs: + fallback_sections.append("\n".join(s) + "\n") + + if repaired_sections: + return "".join(repaired_sections) + return "".join(fallback_sections) + + def extract_unified_diff( text: str, *, - known_files: tuple[str, ...] | list[str] | set[str] | None = None, - project: str | None = None, + known_files: Iterable[str] | None = None, + project: str | Path | None = None, ) -> str | None: """Pull a unified diff out of an agent's stdout. Agents habitually wrap diffs in code fences and add commentary, so accept - both fenced and bare output. Returns None when the reply contains no diff, - which includes the explicit ``NO-PATCH:`` refusal. + both fenced and bare output. When *project* is provided, also supports + SEARCH/REPLACE synthesis and fuzzy hunk repair against on-disk files. + Returns None when the reply contains no diff, which includes the explicit + ``NO-PATCH:`` refusal. """ if not text: return None + if project is not None and _SEARCH_REPLACE_RE.search(text): + synthesized = synthesize_diff_from_search_replace(text, project, known_files=known_files) + if synthesized: + return synthesized # Fix split hunk headers produced by some models: @@ -\n @@ text = re.sub(r"@@ -[ \t]*\n[ \t]*(\d+)", r"@@ -\1", text) # Fix common path hallucination: project/analysis/toon.yaml -> project/analysis.toon.yaml @@ -112,18 +753,37 @@ def extract_unified_diff( text = re.sub(r"(? list[str]: @@ -136,17 +796,32 @@ def _sanitize_inner_headers(lines: list[str]) -> list[str]: """ cleaned: list[str] = [] in_hunk = False + after_fence = False for line in lines: + if line.strip().startswith("```"): + in_hunk = False + after_fence = True + continue + if line.startswith("diff --git "): + in_hunk = False + after_fence = False + cleaned.append(line) + continue + if after_fence: + if line.startswith(("--- a/", "--- /dev/null", "+++ b/", "+++ /dev/null", "@@")): + after_fence = False + else: + continue if line.startswith("@@"): in_hunk = True cleaned.append(line) continue - if line.startswith(("diff --git", "index ")): + if line.startswith(("index ", "new file mode", "deleted file mode", "similarity index", "rename from", "rename to")): in_hunk = False cleaned.append(line) continue if in_hunk: - if line.startswith(("--- ", "+++ ")) or line.lstrip().startswith(("+--- a/", "+++ b/", "--- a/")): + if _STRAY_INNER_HEADER_RE.match(line.lstrip()): continue if _ELLIPSIS_RE.match(line): continue @@ -185,8 +860,8 @@ def _repair_hunk_counts(lines: list[str]) -> list[str]: continue old_count = 0 new_count = 0 - for body_line in repaired[index + 1:]: - if body_line.startswith(("@@", "diff --git", "--- ", "+++ ")): + for body_line in repaired[index + 1 :]: + if body_line.startswith(("@@", "diff --git", "--- a/", "--- /dev/null", "+++ b/", "+++ /dev/null")): break if body_line.startswith("\\"): # "\ No newline at end of file" continue @@ -208,23 +883,55 @@ def _repair_missing_file_headers(lines: list[str]) -> list[str]: """Insert ``---``/``+++`` lines when an agent omits them. Models routinely emit a ``diff --git`` header followed straight by a - ``@@`` hunk, which ``git apply`` rejects as "patch fragment without - header". The paths are already in the ``diff --git`` line, so the headers - can be reconstructed deterministically rather than by re-prompting. + ``@@`` hunk (or ``new file mode`` + ``@@``), which ``git apply`` rejects + as "patch fragment without header". The paths are already in the + ``diff --git`` line, so the headers can be reconstructed deterministically + rather than by re-prompting. """ repaired: list[str] = [] - for index, line in enumerate(lines): + i = 0 + n = len(lines) + while i < n: + line = lines[i] repaired.append(line) match = _GIT_HEADER_RE.match(line) if not match: + i += 1 continue - following = next( - (candidate for candidate in lines[index + 1:] if candidate.strip()), - "", - ) - if following.startswith("@@"): - repaired.append(f"--- a/{match.group('old')}") - repaired.append(f"+++ b/{match.group('new')}") + + # Scan metadata lines between `diff --git` and the first `@@` or next `diff --git` + is_new_file = False + is_deleted_file = False + has_old_header = False + has_new_header = False + j = i + 1 + while j < n: + cand = lines[j] + if cand.startswith("diff --git ") or cand.startswith("@@"): + break + if cand.startswith("new file mode"): + is_new_file = True + elif cand.startswith("deleted file mode"): + is_deleted_file = True + elif cand.startswith("--- "): + has_old_header = True + elif cand.startswith("+++ "): + has_new_header = True + repaired.append(cand) + j += 1 + + if j < n and lines[j].startswith("@@"): + hunk_line = lines[j] + if hunk_line.startswith("@@ -0,0"): + is_new_file = True + elif "+0,0 @@" in hunk_line or re.search(r"\+0(?:,0)?\s*@@", hunk_line): + is_deleted_file = True + + if not has_old_header: + repaired.append("--- /dev/null" if is_new_file else f"--- a/{match.group('old')}") + if not has_new_header: + repaired.append("+++ /dev/null" if is_deleted_file else f"+++ b/{match.group('new')}") + i = j return repaired @@ -237,5 +944,3 @@ def symlink_creations(diff: str) -> bool: policy question rather than a hard error — see ``symlinks_allowed``. """ return bool(_SYMLINK_MODE_RE.search(diff)) - - diff --git a/src/koru/queue/patch_mode.py b/src/koru/queue/patch_mode.py index a1bab8cb..06a477ca 100644 --- a/src/koru/queue/patch_mode.py +++ b/src/koru/queue/patch_mode.py @@ -17,11 +17,16 @@ import json import os import re +from collections.abc import Iterable from dataclasses import dataclass from pathlib import Path from koru.proposal_envelope import NO_VALID_ARTIFACT -from koru.queue.diff_repair import extract_unified_diff, symlink_creations +from koru.queue.diff_repair import ( + extract_unified_diff, + symlink_creations, + synthesize_diff_from_search_replace, +) from koru.queue.manifest import ( MANIFEST_MISMATCH, MANIFEST_NOT_PERSISTED, @@ -69,6 +74,7 @@ "revert_files", "staging_worktree", "symlink_creations", + "synthesize_diff_from_search_replace", "worktree_enabled", ] @@ -174,9 +180,17 @@ class _EnvelopedReply: stderr: str raw: dict | None = None model: str | None = None + project: Path | str | None = None + known_files: tuple[str, ...] | None = None -def wrap_reply_in_proposal_envelope(result, prompt: str): +def wrap_reply_in_proposal_envelope( + result, + prompt: str, + *, + project: Path | str | None = None, + known_files: Iterable[str] | None = None, +): """Producer-side wrap: a bare-diff reply becomes a hash-bound envelope. The model authored only the diff; every binding (input hash, prompt-schema @@ -193,10 +207,21 @@ def wrap_reply_in_proposal_envelope(result, prompt: str): looks_like_proposal_envelope, ) + resolved_project = project if project is not None else getattr(result, "project", None) + resolved_files = known_files if known_files is not None else getattr(result, "known_files", None) + known_tuple = tuple(str(f) for f in resolved_files) if resolved_files is not None else None + try: + if resolved_project is not None: + setattr(result, "project", resolved_project) + if known_tuple is not None: + setattr(result, "known_files", known_tuple) + except Exception: + pass + stdout = result.stdout or "" if not proposal_producer_enabled() or looks_like_proposal_envelope(stdout): return result - diff = extract_unified_diff(stdout) + diff = extract_unified_diff(stdout, project=resolved_project, known_files=known_tuple) if diff is None: return result raw = getattr(result, "raw", None) @@ -227,6 +252,8 @@ def wrap_reply_in_proposal_envelope(result, prompt: str): stderr=getattr(result, "stderr", "") or "", raw=raw or None, model=getattr(result, "model", None), + project=resolved_project, + known_files=known_tuple, ) @@ -285,11 +312,28 @@ def build_retry_prompt(prompt: str, failure: str) -> str: ) +def build_verify_retry_prompt(prompt: str, failure: str, previous_diff: str = "") -> str: + """Re-ask for a corrected patch when the previous diff applied but failed verification.""" + diff_section = ( + f"\n\n### Your previous diff (applied cleanly, then rolled back)\n\n```diff\n{redact_secrets(previous_diff.strip(), limit=6000)}\n```" + if previous_diff and previous_diff.strip() + else "" + ) + return ( + f"{build_patch_prompt(prompt).rstrip()}\n\n" + "## Previous attempt failed the verification gate (Reflexion)\n\n" + "Your previous diff applied cleanly, but the verification gate failed and the workspace was rolled back to the original base state:\n\n" + f" {failure.strip()}" + f"{diff_section}\n\n" + "Analyze the test/verification failure above, fix the bug in your change, and emit a complete replacement unified diff against the original files on disk." + ) + + def current_file_excerpt( project: Path, paths: tuple[str, ...], *, - max_chars: int = 6000, + max_chars: int = 24000, ) -> str: """Quote the files a patch must apply to, for a retry prompt. diff --git a/src/koru/queue/patch_retry.py b/src/koru/queue/patch_retry.py index a614ed44..5f3f538f 100644 --- a/src/koru/queue/patch_retry.py +++ b/src/koru/queue/patch_retry.py @@ -38,9 +38,12 @@ PATCH_DOES_NOT_APPLY, PROMOTION_ARTIFACT, PROMOTION_BRANCH, + VERIFY_FAILED_ISOLATED, + VERIFY_FAILED_ROLLED_BACK, PatchOutcome, build_manifest, build_retry_prompt, + build_verify_retry_prompt, current_file_excerpt, diff_target_files, extract_unified_diff, @@ -78,6 +81,30 @@ def patch_retry_budget(ticket: dict | None = None) -> int: return 1 +def verify_retry_budget(ticket: dict | None = None) -> int: + """How many times to re-ask an agent when a patch applied cleanly but failed verification.""" + if ticket is not None: + per_ticket = (ticket.get("inputs") or {}).get("max_verify_retries") + if per_ticket is not None: + try: + return max(0, int(per_ticket)) + except (TypeError, ValueError): + pass + raw = (os.environ.get("KORU_QUEUE_VERIFY_RETRIES") or "").strip() + try: + return max(0, int(raw)) if raw else 0 + except ValueError: + return 0 + + +def _is_verify_failure(outcome: PatchOutcome | None) -> bool: + return bool( + outcome is not None + and outcome.workspace_left_untouched + and outcome.code in (VERIFY_FAILED_ISOLATED, VERIFY_FAILED_ROLLED_BACK) + ) + + def _contract_capped_budget(project: Path, ticket: dict) -> int: """The retry budget, never above what the ticket's contract permits. @@ -114,6 +141,34 @@ def _finish_patch_run( ) +_READONLY_SCAN_ARTIFACT_NAMES = frozenset( + { + "analysis.toon.yaml", + "analysis.toon", + "duplication.toon.yaml", + "duplication.yaml", + "calls.yaml", + "calls.toon.yaml", + "map.toon.yaml", + "project.toon.yaml", + } +) + + +def _is_readonly_scan_artifact(rel_path: str) -> bool: + norm = str(rel_path or "").replace("\\", "/").strip().lstrip("./") + if not norm: + return False + basename = norm.rsplit("/", 1)[-1].lower() + return basename in _READONLY_SCAN_ARTIFACT_NAMES + + +def _filter_readonly_scan_artifacts(paths: Any) -> tuple[str, ...]: + if not paths: + return () + return tuple(str(p) for p in paths if p and not _is_readonly_scan_artifact(str(p))) + + def _perform_retry_call( llm_runner: Callable[[dict[str, Any], Path], CommandResult], action: dict[str, Any], @@ -122,10 +177,23 @@ def _perform_retry_call( manifest: dict | None, outcome: PatchOutcome, enrich: Callable[[dict[str, Any], Path], dict[str, Any]] | None, + *, + previous_diff: str = "", ) -> CommandResult: - retry_action = _build_retry_action(action, base_prompt, project, manifest, outcome, enrich) + retry_action = _build_retry_action( + action, base_prompt, project, manifest, outcome, enrich, + previous_diff=previous_diff, + ) res = llm_runner(retry_action, project) - return wrap_reply_in_proposal_envelope(res, str(retry_action.get("prompt") or "")) + known = _filter_readonly_scan_artifacts( + action.get("ticket_files") or (manifest or {}).get("touched_files") or () + ) + return wrap_reply_in_proposal_envelope( + res, + str(retry_action.get("prompt") or ""), + project=project, + known_files=known, + ) class _AttemptDecision(NamedTuple): @@ -164,27 +232,53 @@ class _PatchRun: attempts: list[dict] = field(default_factory=list) base_prompt: str = "" remaining: int = 0 + verify_remaining: int = 0 def __post_init__(self) -> None: - self.base_prompt = str(self.action.get("prompt") or "") + raw_prompt = str(self.action.get("prompt") or "") + known = _filter_readonly_scan_artifacts( + (self.ticket.get("files") if isinstance(self.ticket, dict) else None) + or self.action.get("ticket_files") + or () + ) + ticket_id = str(self.ticket.get("id") or "") if isinstance(self.ticket, dict) else "" + lessons_section = format_lessons_for_prompt( + self.project, ticket_id=ticket_id or None, target_files=known, + ) + self.base_prompt = f"{raw_prompt.rstrip()}\n\n{lessons_section}" if lessons_section else raw_prompt self.remaining = self.budget + self.verify_remaining = verify_retry_budget(self.ticket if isinstance(self.ticket, dict) else None) # Producer side of the envelope contract: the bare diff the agent wrote # becomes a hash-bound ProposalEnvelope before the transaction judges it. - self.result = wrap_reply_in_proposal_envelope(self.result, self.base_prompt) + self.result = wrap_reply_in_proposal_envelope( + self.result, + raw_prompt, + project=self.project, + known_files=known, + ) def record_attempt(self, transaction: PatchTransactionResult) -> None: """Fold one judged attempt into the run's permanent record.""" - self.attempts.append(_attempt_record(len(self.attempts) + 1, transaction)) + self.attempts.append( + _attempt_record(len(self.attempts) + 1, transaction, self.verify_remaining > 0) + ) def advance(self, transaction: PatchTransactionResult) -> _AttemptDecision: """Decide the run's next move after one judged attempt. Landing, a substantive failure, or an exhausted budget ends the run; a - mechanical failure first pins the base (aborting on drift), then spends - one budget unit re-asking the agent with the exact rejection attached. + mechanical failure (or a verify failure when verify_remaining > 0) first + pins the base (aborting on drift), then spends one budget unit re-asking + the agent with the exact rejection attached. """ outcome = transaction.outcome - if outcome is None or not outcome.retryable or self.remaining <= 0: + if outcome is None: + return _AttemptDecision(transaction.result, outcome, True) + is_verify = _is_verify_failure(outcome) + if is_verify: + if self.verify_remaining <= 0: + return _AttemptDecision(transaction.result, outcome, True) + elif not outcome.retryable or self.remaining <= 0: return _AttemptDecision(transaction.result, outcome, True) # A structurally invalid model artifact gets one repair attempt. A # ticket/env knob may shrink that budget, never expand it into a loop. @@ -193,17 +287,22 @@ def advance(self, transaction: PatchTransactionResult) -> _AttemptDecision: aborted = self._pin_or_abort(transaction) if aborted is not None: return _AttemptDecision(transaction.result, aborted, True) - self.remaining -= 1 + if is_verify: + self.verify_remaining -= 1 + else: + self.remaining -= 1 + prev_diff = transaction.plan.diff if (is_verify and transaction.plan) else "" result = _perform_retry_call( self.llm_runner, self.action, self.base_prompt, self.project, self.manifest, outcome, self.enrich, + previous_diff=prev_diff, ) return _AttemptDecision(result, outcome, result.returncode != 0) def _pin_or_abort(self, transaction: PatchTransactionResult) -> PatchOutcome | None: """Pin the base on the first failure, or abort if it has since drifted.""" self.manifest, aborted = _pin_or_detect_drift( - self.project, self.ticket, transaction.result, self.budget, + self.project, self.ticket, transaction.result, self.budget + self.verify_remaining, self.manifest, transaction, self.attempts, ) return aborted @@ -322,22 +421,204 @@ def _build_retry_action( manifest: dict | None, outcome: PatchOutcome, enrich: Callable[[dict[str, Any], Path], dict[str, Any]] | None, + *, + previous_diff: str = "", ) -> dict[str, Any]: # git and test output can quote file contents, so redact before it # travels back out to the model. diagnostic = redact_secrets(outcome.diagnostics or outcome.message) - prompt = build_retry_prompt(base_prompt, diagnostic) - if manifest and outcome.code == PATCH_DOES_NOT_APPLY: + if _is_verify_failure(outcome): + prompt = build_verify_retry_prompt(base_prompt, diagnostic, previous_diff=previous_diff) + else: + prompt = build_retry_prompt(base_prompt, diagnostic) + if manifest and ( + outcome.code in (PATCH_DOES_NOT_APPLY, NO_PATCH_EMITTED) + or _is_verify_failure(outcome) + ): # Safe only because the manifest guarantees these contents are still # the ones the patch must apply to. - prompt += current_file_excerpt(project, tuple(manifest.get("touched_files") or ())) + touched = _filter_readonly_scan_artifacts(manifest.get("touched_files") or ()) + prompt += current_file_excerpt(project, touched) retry_action = {**action, "prompt": prompt} if enrich is not None: retry_action = enrich(retry_action, project) return retry_action -def _attempt_record(attempt: int, transaction: PatchTransactionResult) -> dict: +def _distill_lesson_rule(outcome_code: str | None, diagnostic: str, self_repaired: bool) -> str: + diag_line = next((ln.strip() for ln in (diagnostic or "").splitlines() if ln.strip()), "")[:240] + if self_repaired: + return f"Recovered after initial '{outcome_code}': ensure hunks match disk context and pass verification ({diag_line})." + if outcome_code == "patch_destructive_truncation": + return "Do not delete existing public functions/classes or truncate large sections of the module unless explicitly replacing them." + if outcome_code == "patch_introduces_antipattern": + return f"Avoid introducing performance or resource-leak anti-patterns ({diag_line})." + if outcome_code in (VERIFY_FAILED_ISOLATED, VERIFY_FAILED_ROLLED_BACK): + return f"Verification failed ({diag_line}); preserve existing public signatures, imports, and test expectations." + if outcome_code == PATCH_DOES_NOT_APPLY: + return f"Patch context did not match disk ({diag_line}); include exact 3-line context from current file contents." + return f"Previous attempt failed with '{outcome_code}': {diag_line}" + + +def record_run_lesson( + project: Path, + ticket: dict, + bundle: dict, + transaction: PatchTransactionResult, +) -> dict | None: + """Persist a structured post-mortem lesson when a run fails or self-repairs.""" + import json + from datetime import datetime, timezone + + attempts = bundle.get("patch_attempts") or [] + verdict = str(bundle.get("verdict") or "") + outcome = transaction.outcome + failed_attempts = [a for a in attempts if a.get("outcome") and a.get("outcome") != "landed"] + if verdict == VERDICT_VERIFIED and not failed_attempts: + return None + first_fail = failed_attempts[0] if failed_attempts else {} + outcome_code = str( + (outcome.code if outcome else None) + or first_fail.get("outcome") + or "unknown_failure" + ) + diagnostic = redact_secrets( + (outcome.diagnostics or outcome.message) + if outcome is not None + else str(first_fail.get("message") or ""), + limit=500, + ) + self_repaired = verdict == VERDICT_VERIFIED and bool(failed_attempts) + plan = transaction.plan + raw_targets = ( + list(plan.targets) + if plan and plan.targets + else list((ticket.get("files") if isinstance(ticket, dict) else None) or []) + ) + targets = list(_filter_readonly_scan_artifacts(raw_targets)) + lesson = { + "timestamp": datetime.now(timezone.utc).isoformat(), + "run_id": bundle.get("run_id"), + "ticket_id": str(ticket.get("id") or "") if isinstance(ticket, dict) else "", + "project": project.name, + "verdict": verdict, + "self_repaired": self_repaired, + "outcome_code": outcome_code, + "failed_codes": [str(a.get("outcome")) for a in failed_attempts if a.get("outcome")], + "target_files": targets, + "diagnostic": diagnostic, + "rule": _distill_lesson_rule(outcome_code, diagnostic, self_repaired), + } + line = json.dumps(lesson, ensure_ascii=False, sort_keys=True) + "\n" + try: + lessons_dir = project / ".koru" + lessons_dir.mkdir(parents=True, exist_ok=True) + with (lessons_dir / "lessons.jsonl").open("a", encoding="utf-8") as fh: + fh.write(line) + except OSError: + pass + fleet_path = (os.environ.get("KORU_FLEET_LESSONS_PATH") or "").strip() + if fleet_path: + try: + fp = Path(fleet_path) + fp.parent.mkdir(parents=True, exist_ok=True) + with fp.open("a", encoding="utf-8") as fh: + fh.write(line) + except OSError: + pass + return lesson + + +def _read_bounded_tail_lines(path: Path) -> list[str]: + max_bytes = 65536 + if not path.is_file(): + return [] + try: + size = path.stat().st_size + with path.open("rb") as fh: + if size > max_bytes: + fh.seek(size - max_bytes) + raw = fh.read() + nl = raw.find(b"\n") + if nl != -1: + raw = raw[nl + 1 :] + else: + raw = fh.read() + return raw.decode("utf-8", errors="replace").splitlines() + except OSError: + return [] + + +def load_project_lessons( + project: Path, + *, + ticket_id: str | None = None, + target_files: Any = None, + limit: int = 5, +) -> list[dict]: + """Load recent relevant lessons from ``.koru/lessons.jsonl`` (bounded read).""" + import json + + candidates: list[Path] = [project / ".koru" / "lessons.jsonl"] + fleet_path = (os.environ.get("KORU_FLEET_LESSONS_PATH") or "").strip() + if fleet_path: + candidates.append(Path(fleet_path)) + target_set = set(_filter_readonly_scan_artifacts(target_files or ())) + matched: list[dict] = [] + seen_rules: set[str] = set() + for path in candidates: + for raw_line in reversed(_read_bounded_tail_lines(path)): + raw_line = raw_line.strip() + if not raw_line: + continue + try: + item = json.loads(raw_line) + except ValueError: + continue + if not isinstance(item, dict): + continue + same_ticket = bool(ticket_id and item.get("ticket_id") == ticket_id) + item_files = set(item.get("target_files") or ()) + same_files = bool(target_set and (target_set & item_files)) + is_self_repaired = bool(item.get("self_repaired")) + if not (same_ticket or same_files or is_self_repaired): + continue + rule = str(item.get("rule") or "").strip() + if not rule or rule in seen_rules: + continue + seen_rules.add(rule) + matched.append(item) + if len(matched) >= max(1, limit): + return matched + return matched + + +def format_lessons_for_prompt( + project: Path, + *, + ticket_id: str | None = None, + target_files: Any = None, + limit: int = 5, +) -> str: + """Format relevant past lessons into a concise prompt guardrail section.""" + lessons = load_project_lessons( + project, ticket_id=ticket_id, target_files=target_files, limit=limit, + ) + if not lessons: + return "" + bullets = [f"- [{item.get('outcome_code', 'lesson')}] {item.get('rule', '')}" for item in lessons] + return ( + "## Learned Constraints from Previous Runs\n\n" + "Apply these lessons distilled from earlier attempts on this ticket/files:\n" + + "\n".join(bullets) + ) + + +def _attempt_record( + attempt: int, + transaction: PatchTransactionResult, + verify_retryable: bool, +) -> dict: """One transaction attempt, as it will be remembered.""" plan = transaction.plan outcome = transaction.outcome @@ -348,7 +629,10 @@ def _attempt_record(attempt: int, transaction: PatchTransactionResult) -> dict: ), outcome_code=outcome.code if outcome else None, message=outcome.message if outcome else "", - retryable=bool(outcome and outcome.retryable), + retryable=bool( + outcome + and (outcome.retryable or (verify_retryable and _is_verify_failure(outcome))) + ), ) # A retry can land on a different provider after a fallback; each attempt # remembers who authored it, not just the bundle-level summary. @@ -431,6 +715,10 @@ def _finish_run( persist_evidence(project, bundle) except OSError: pass + try: + record_run_lesson(project, ticket, bundle, transaction) + except Exception: + pass _journal_terminal(project, run_id, bundle, transaction) return bundle @@ -493,10 +781,12 @@ def _pin_base( """ from koru.queue.verify.resolver import resolve_verify - diff = extract_unified_diff(result.stdout) or "" - targets = diff_target_files(project, diff) or tuple( + known = _filter_readonly_scan_artifacts(ticket.get("files") or ()) + diff = extract_unified_diff(result.stdout, project=project, known_files=known) or "" + raw_targets = diff_target_files(project, diff) or tuple( str(rel) for rel in (ticket.get("files") or []) ) + targets = _filter_readonly_scan_artifacts(raw_targets) or raw_targets return build_manifest( project, run_id=run_id or uuid4().hex[:12], @@ -508,3 +798,6 @@ def _pin_base( attempt=1, max_attempts=budget + 1, ) + + +execute_with_patch_retries = apply_patch_with_retry diff --git a/src/koru/queue/transaction/preflight.py b/src/koru/queue/transaction/preflight.py index f6dda4fc..59b4d440 100644 --- a/src/koru/queue/transaction/preflight.py +++ b/src/koru/queue/transaction/preflight.py @@ -7,6 +7,7 @@ from __future__ import annotations +from collections.abc import Iterable from pathlib import Path from uuid import uuid4 @@ -39,6 +40,9 @@ def extract_patch( result: CommandResult, + *, + project: Path | str | None = None, + known_files: Iterable[str] | None = None, ) -> tuple[str | None, dict | None, PatchOutcome | None]: """Pull the unified diff out of an agent reply, or explain what came instead. @@ -48,6 +52,8 @@ def extract_patch( element — that is the top of the run's hash ladder (``input_hash → proposal_sha256 → plan → manifest``), and evidence wants it. """ + resolved_project = project if project is not None else getattr(result, "project", None) + resolved_files = known_files if known_files is not None else getattr(result, "known_files", None) artifact = result.stdout bindings: dict | None = None if looks_like_proposal_envelope(artifact): @@ -80,7 +86,7 @@ def extract_patch( "proposal_sha256": envelope.proposal_sha256, } - diff = extract_unified_diff(artifact) + diff = extract_unified_diff(artifact, project=resolved_project, known_files=resolved_files) if diff is not None: return diff, bindings, None head = (result.stdout or "").strip().splitlines() @@ -95,6 +101,160 @@ def extract_patch( ) +import os +import re + +PATCH_DESTRUCTIVE_TRUNCATION = "patch_destructive_truncation" +PATCH_INTRODUCES_ANTIPATTERN = "patch_introduces_antipattern" + +_DIFF_FILE_RE = re.compile(r"^\+\+\+\s+(?:b/)?(\S+)") +_REMOVED_TOP_DEF_RE = re.compile(r"^-(?:async\s+def|def|class)\s+([A-Za-z_][A-Za-z0-9_]*)\b") +_ADDED_TOP_DEF_RE = re.compile(r"^\+(?:async\s+def|def|class)\s+([A-Za-z_][A-Za-z0-9_]*)\b") +_SPLIT_OR_DELETE_KEYWORDS = ( + "split", + "delete", + "remove", + "extract", + "decompose", + "drop", + "deprecate", +) +_LOOP_ANTIPATTERN_RE = re.compile( + r"\b(?:subprocess\.(?:run|Popen|check_output|call)|sqlite3\.connect|yaml\.safe_load)\s*\(" +) + + +def _is_test_path(rel_path: str) -> bool: + norm = rel_path.replace("\\", "/").strip("/") + parts = norm.split("/") + filename = parts[-1] if parts else "" + return ( + "tests" in parts + or "test" in parts + or filename.startswith("test_") + or filename.endswith("_test.py") + ) + + +def screen_patch_quality( + project: Path | None, + ticket: dict | None, + diff: str, +) -> PatchOutcome | None: + """Reject patches that accidentally truncate modules or introduce severe anti-patterns.""" + if os.environ.get("KORU_DISABLE_PATCH_QUALITY_GATE") == "1": + return None + + ticket_text = "" + if isinstance(ticket, dict): + ticket_text = " ".join( + str(ticket.get(k) or "") + for k in ("title", "description", "summary", "goal", "prompt") + ).lower() + allows_removal = any(kw in ticket_text for kw in _SPLIT_OR_DELETE_KEYWORDS) + + current_file: str | None = None + removed_defs_by_file: dict[str, list[str]] = {} + all_added_defs: set[str] = set() + deleted_lines_by_file: dict[str, int] = {} + added_lines_by_file: dict[str, int] = {} + active_loop_indent: int | None = None + + for raw_line in diff.splitlines(): + m_file = _DIFF_FILE_RE.match(raw_line) + if m_file: + target = m_file.group(1) + current_file = None if target == "/dev/null" else target + active_loop_indent = None + continue + if raw_line.startswith("@@"): + active_loop_indent = None + continue + if not current_file or not current_file.endswith(".py"): + continue + + if raw_line.startswith("-") and not raw_line.startswith("---"): + deleted_lines_by_file[current_file] = deleted_lines_by_file.get(current_file, 0) + 1 + m_rem = _REMOVED_TOP_DEF_RE.match(raw_line) + if m_rem: + sym = m_rem.group(1) + if not sym.startswith("_"): + removed_defs_by_file.setdefault(current_file, []).append(sym) + elif raw_line.startswith("+") and not raw_line.startswith("+++"): + added_lines_by_file[current_file] = added_lines_by_file.get(current_file, 0) + 1 + m_add = _ADDED_TOP_DEF_RE.match(raw_line) + if m_add: + all_added_defs.add(m_add.group(1)) + + if not _is_test_path(current_file): + code_line = raw_line[1:] + stripped = code_line.lstrip() + if not stripped or stripped.startswith("#"): + continue + indent = len(code_line) - len(stripped) + + # PERF-AUDIT-SQLITE-002: with sqlite3.connect(...) without closing(...) + if ( + stripped.startswith("with ") + and "sqlite3.connect(" in stripped + and "closing(" not in stripped + ): + return PatchOutcome( + code=PATCH_INTRODUCES_ANTIPATTERN, + message=( + f"patch in {current_file} uses `with sqlite3.connect(...)` which " + "does not close the SQLite connection on exit (PERF-AUDIT-SQLITE-002); " + "wrap it in `contextlib.closing(sqlite3.connect(...))`." + ), + retryable=True, + ) + + if active_loop_indent is not None and indent <= active_loop_indent: + active_loop_indent = None + if stripped.startswith(("for ", "async for ", "while ")) and stripped.endswith(":"): + active_loop_indent = indent + elif active_loop_indent is not None and indent > active_loop_indent: + m_anti = _LOOP_ANTIPATTERN_RE.search(stripped) + if m_anti: + return PatchOutcome( + code=PATCH_INTRODUCES_ANTIPATTERN, + message=( + f"patch in {current_file} introduces `{m_anti.group(0)}...)` " + "directly inside a loop (PERF-AUDIT); hoist, batch, or cache " + "the operation outside the loop." + ), + retryable=True, + ) + + if not allows_removal: + for file_path, rem_syms in removed_defs_by_file.items(): + missing = [s for s in rem_syms if s not in all_added_defs] + if len(missing) >= 3: + sample = ", ".join(missing[:6]) + return PatchOutcome( + code=PATCH_DESTRUCTIVE_TRUNCATION, + message=( + f"patch destructively removes {len(missing)} public top-level " + f"symbols from {file_path} ({sample}) without re-adding them. " + "Preserve existing functions/classes and emit only targeted hunks." + ), + retryable=True, + ) + for file_path, del_count in deleted_lines_by_file.items(): + add_count = added_lines_by_file.get(file_path, 0) + if del_count >= 80 and add_count < 10: + return PatchOutcome( + code=PATCH_DESTRUCTIVE_TRUNCATION, + message=( + f"patch deletes {del_count} lines and adds only {add_count} lines " + f"in {file_path}, which indicates accidental file truncation. " + "Preserve existing code outside the targeted change." + ), + retryable=True, + ) + return None + + def screen_diff_contents(diff: str) -> PatchOutcome | None: """Refuse diffs whose content is unsafe regardless of where they apply. @@ -127,6 +287,14 @@ def build_patch_plan( """ from koru.queue.verify.resolver import resolve_verify + repaired = extract_unified_diff( + diff, + project=project, + known_files=ticket.get("files") if isinstance(ticket, dict) else None, + ) + if repaired: + diff = repaired + targets = diff_target_files(project, diff) resolution = resolve_verify(project, ticket, targets) return PatchPlan( @@ -144,6 +312,29 @@ def build_patch_plan( ) +def run_patch_preflight( + project: Path, + result: CommandResult, + ticket: dict, + manifest: dict | None = None, +) -> tuple[PatchPlan | None, PatchOutcome | None]: + """Extract, repair/synthesize, screen, and plan a patch against *project*.""" + diff, proposal, refusal = extract_patch( + result, + project=project, + known_files=ticket.get("files") if isinstance(ticket, dict) else None, + ) + if diff is None or refusal is not None: + return None, refusal + refusal = screen_diff_contents(diff) + if refusal is not None: + return None, refusal + refusal = screen_patch_quality(project, ticket, diff) + if refusal is not None: + return None, refusal + return build_patch_plan(project, ticket, diff, manifest, proposal=proposal), None + + def screen_promotion_preconditions(plan: PatchPlan) -> PatchOutcome | None: """Refuse a promotion the workspace cannot honour cleanly.""" if plan.mode != PROMOTION_COMMIT or repository_is_clean(plan.project): diff --git a/src/koru/queue/verify/legacy.py b/src/koru/queue/verify/legacy.py index f5021ff4..2e327e44 100644 --- a/src/koru/queue/verify/legacy.py +++ b/src/koru/queue/verify/legacy.py @@ -70,13 +70,37 @@ def _verify_command_from_project(project: Path) -> str: if commands: return str(commands[0]).strip() - try: - policy = yaml.safe_load( - (project / ".planfile" / ".koru" / "policy.yaml").read_text(encoding="utf-8"), - ) - except (OSError, yaml.YAMLError): - return "" - if not isinstance(policy, dict) or not isinstance(policy.get("ci"), dict): - return "" - command = policy["ci"].get("command") - return command.strip() if isinstance(command, str) else "" + policy_path = project / ".planfile" / ".koru" / "policy.yaml" + if policy_path.exists(): + try: + policy = yaml.safe_load(policy_path.read_text(encoding="utf-8")) + except (OSError, yaml.YAMLError): + return "" + if not isinstance(policy, dict) or not isinstance(policy.get("ci"), dict): + return "" + command = policy["ci"].get("command") + if not isinstance(command, str): + return "" + if command.strip(): + return command.strip() + + return _auto_detect_verify_command(project) + + +def _auto_detect_verify_command(project: Path) -> str: + """Auto-detect a sensible default verification gate from project markers.""" + if ( + (project / "pyproject.toml").is_file() + or (project / "pytest.ini").is_file() + or (project / "tests").is_dir() + ): + venv_pytest = project / ".venv" / "bin" / "pytest" + if venv_pytest.is_file(): + return f"{venv_pytest} -q" + return "pytest -q" + if (project / "Cargo.toml").is_file(): + return "cargo test --locked" + if (project / "package.json").is_file(): + return "npm test" + return "" + diff --git a/src/koru/queue/workspace.py b/src/koru/queue/workspace.py index 491f693b..89652da9 100644 --- a/src/koru/queue/workspace.py +++ b/src/koru/queue/workspace.py @@ -99,7 +99,13 @@ def reverse_unified_diff(project: Path, diff: str) -> PatchApplyResult: def diff_target_files(project: Path, diff: str) -> tuple[str, ...]: """List the paths a diff touches, without applying it.""" listed = _git(project, "apply", "--numstat", "-", stdin=diff) - return tuple(line.split("\t")[-1].strip() for line in (listed.stdout or "").splitlines() if line.strip()) + return tuple( + dict.fromkeys( + line.split("\t")[-1].strip() + for line in (listed.stdout or "").splitlines() + if line.strip() + ) + ) @dataclass(frozen=True) @@ -233,13 +239,17 @@ def commit_worktree( staged = _git(worktree, "add", "-A") if staged.returncode != 0: return False, (staged.stderr or staged.stdout or "").strip()[:300] - committed = _git(worktree, "commit", "--quiet", "-m", message) + commit_args = ["commit", "--quiet", "-m", message] + if (os.environ.get("KORU_GOVERNED_WORKTREE_OK") or "").strip() == "1" and branch.startswith("koru/"): + commit_args.insert(1, "--no-verify") + committed = _git(worktree, *commit_args) if committed.returncode != 0: return False, (committed.stderr or committed.stdout or "").strip()[:300] head = _git(worktree, "rev-parse", "HEAD") return True, (head.stdout or "").strip() + def worktree_enabled(project: Path) -> bool: """Whether to stage a patch in a throwaway worktree before promoting it. @@ -260,20 +270,13 @@ def _worktree_location(project: Path, run_id: str) -> Path: `/.worktrees/ticket-`. """ override = (os.environ.get("KORU_QUEUE_WORKTREE_DIR") or "").strip() - prefix = "ticket-" if not str(run_id).startswith("ticket-") else "" - slug = f"{prefix}{run_id}" + safe_token = str(run_id or "").strip() or "run" + slug = safe_token if safe_token.startswith("ticket-") else f"ticket-{safe_token}" if override: return Path(override).expanduser() / slug - v5_dir = project / ".worktrees" - if v5_dir.is_dir() or (project / ".git").is_dir() or (project / ".git").is_file(): - return v5_dir / slug - - parent = project.parent - if os.access(parent, os.W_OK): - return parent / f".koru-run-{run_id}" - return project / ".koru" / "worktrees" / f"run-{run_id}" + return project / ".worktrees" / slug def prune_stale_worktrees(project: Path) -> None: @@ -303,6 +306,57 @@ def _git_overrides() -> bool: ) +def has_valid_admission_lease(project: Path) -> bool: + """Return True when *project* carries a valid ticket intent + admission lease.""" + try: + from koru.queue.lease import has_active_lease + + if has_active_lease(project): + return True + except Exception: + pass + ticket_id = project.name.split("--", 1)[0] if project.name.startswith("ticket-") else "" + has_intent = (project / ".governance" / "intent.json").is_file() or ( + bool(ticket_id) and (project / "project" / ticket_id / "intent.json").is_file() + ) + if not has_intent: + return False + lease_candidates = [ + project / ".subactor" / "leases" / f"{project.name}.json", + project.parent.parent / ".subactor" / "leases" / f"{project.name}.json", + ] + return any(candidate.is_file() for candidate in lease_candidates) + + +def _is_admitted_ticket_worktree(project: Path) -> bool: + """Return True when *project* is an admitted governed ticket worktree.""" + if (os.environ.get("KORU_GOVERNED_WORKTREE_OK") or "").strip() == "1": + return True + resolved = Path(os.path.abspath(project)) + if resolved.parent.name == ".worktrees" and resolved.name.startswith("ticket-"): + ticket_id = resolved.name.split("--", 1)[0] + if (resolved / ".governance" / "intent.json").is_file(): + return True + if (resolved / "project" / ticket_id / "intent.json").is_file(): + return True + proj_dir = resolved / "project" + if proj_dir.is_dir() and any(proj_dir.glob("ticket-*/intent.json")): + return True + return has_valid_admission_lease(resolved) + + +def _has_governance_marker(root: Path) -> bool: + gov = root / ".governance" + if not gov.is_dir(): + return False + if (gov / "manifest.json").exists(): + return True + try: + return not any(gov.iterdir()) + except OSError: + return True + + def require_unmanaged_patch_workspace(project: Path) -> None: """Refuse legacy patch writes where protected repository admission is required.""" if _git_overrides(): @@ -314,7 +368,9 @@ def require_unmanaged_patch_workspace(project: Path) -> None: if common.returncode != 0 or not common.stdout.strip(): raise StagingAdmissionRequired("Cannot establish the primary checkout for patch execution") primary = Path(common.stdout.strip()).parent - if (project / ".governance" / "manifest.json").exists() or (primary / ".governance" / "manifest.json").exists(): + if _has_governance_marker(project) or _has_governance_marker(primary): + if _is_admitted_ticket_worktree(project): + return raise StagingAdmissionRequired( "Governed patch execution requires native ticket allocation and protected controller " "admission; legacy temporary worktrees and local lease files do not grant it" diff --git a/src/koru/scan_artifacts.py b/src/koru/scan_artifacts.py index f4dfb80f..34c6f921 100644 --- a/src/koru/scan_artifacts.py +++ b/src/koru/scan_artifacts.py @@ -76,6 +76,12 @@ def _scan_jscpd_report(project: Path) -> list[Suggestion]: "project/analysis.toon", "analysis.toon.yaml", ) +DUPLICATION_ARTIFACT_CANDIDATES = ( + "project/duplication.toon.yaml", + "duplication.toon.yaml", + "project/duplication.yaml", + "duplication.yaml", +) _PLANFILE_TICKETS_ARTIFACT_PATHS = ( "project/planfile-tickets.yaml", "planfile-tickets.yaml", @@ -412,6 +418,244 @@ def _is_rm_duplicates_refactor(desc: str, note: str) -> bool: return "duplicate" in blob or "dup group" in blob or "dup groups" in blob +_SCAN_ARTIFACT_BASENAMES = frozenset( + { + "analysis.toon.yaml", + "analysis.toon", + "duplication.toon.yaml", + "duplication.yaml", + "calls.yaml", + "calls.toon.yaml", + "map.toon.yaml", + "project.toon.yaml", + } +) + + +def _is_scan_artifact_rel(path_str: str) -> bool: + norm = str(path_str or "").replace("\\", "/").strip().lstrip("./") + if not norm: + return True + return norm.rsplit("/", 1)[-1].lower() in _SCAN_ARTIFACT_BASENAMES + + +_DUP_HOTSPOT_RE = re.compile( + r"^\s+(?P[A-Za-z0-9_./-]+\.[A-Za-z0-9_]+)\s+dup=(?P\d+)L\s+groups=(?P\d+)", + re.MULTILINE, +) +_DUP_GROUP_HEADER_RE = re.compile( + r"^\s+\[(?P[^\]]+)\]\s+(?P\S+)\s+(?P\S+)\s+L=(?P\d+)\s+N=(?P\d+)\s+saved=(?P\d+)", +) +_DUP_LOC_LINE_RE = re.compile( + r"^\s+(?P[A-Za-z0-9_./-]+\.[A-Za-z0-9_]+):(?P\d+)-(?P\d+)", +) + + +def _extract_duplication_toon_files(text: str) -> tuple[str, ...]: + """Extract unique source file paths from a redup/duplication .toon.yaml document.""" + seen: list[str] = [] + for m in _DUP_LOC_LINE_RE.finditer(text): + p = m.group("path").strip() + if p and not _is_scan_artifact_rel(p) and p not in seen: + seen.append(p) + for m in _DUP_HOTSPOT_RE.finditer(text): + p = m.group("path").strip() + if p and not _is_scan_artifact_rel(p) and p not in seen: + seen.append(p) + return tuple(seen) + + +def _parse_duplication_toon_suggestions( + text: str | dict[str, Any], + rel: str = "project/duplication.toon.yaml", + *, + source_context: dict[str, object] | None = None, +) -> list[Suggestion]: + """Parse actionable duplicate groups and hotspots from duplication.toon.yaml.""" + suggestions: list[Suggestion] = [] + doc: dict[str, Any] | None = None + if isinstance(text, dict): + doc = text + raw_text = "" + else: + raw_text = str(text or "") + + lines = raw_text.splitlines() + in_duplicates = False + current_group: dict[str, Any] | None = None + groups: list[dict[str, Any]] = [] + + for line in lines: + stripped = line.strip() + if stripped.startswith("DUPLICATES"): + in_duplicates = True + continue + if in_duplicates and stripped.startswith(("REFACTOR", "QUICK_WINS", "EFFORT_ESTIMATE", "METRICS-TARGET", "HOTSPOTS", "SUMMARY")): + if current_group: + groups.append(current_group) + current_group = None + in_duplicates = False + continue + if not in_duplicates: + continue + m_hdr = _DUP_GROUP_HEADER_RE.match(line) + if m_hdr: + if current_group: + groups.append(current_group) + current_group = { + "id": m_hdr.group("id"), + "kind": m_hdr.group("kind"), + "name": m_hdr.group("name"), + "lines": int(m_hdr.group("lines")), + "count": int(m_hdr.group("count")), + "saved": int(m_hdr.group("saved")), + "files": [], + "spans": [], + } + continue + if current_group is not None: + m_loc = _DUP_LOC_LINE_RE.match(line) + if m_loc: + p = m_loc.group("path").strip() + span = f"{p}:{m_loc.group('start')}-{m_loc.group('end')}" + current_group["spans"].append(span) + if p and not _is_scan_artifact_rel(p) and p not in current_group["files"]: + current_group["files"].append(p) + + if current_group: + groups.append(current_group) + + for grp in groups: + grp_files = tuple(grp["files"]) + if not grp_files: + continue + saved = grp["saved"] + priority = "high" if saved >= 20 else "normal" + spans_desc = ", ".join(f"`{s}`" for s in grp["spans"][:6]) + suggestions.append( + _with_source_context( + Suggestion( + signal="redup_duplication", + title=f"Deduplicate {grp['name']} ({grp['count']} occurrences, saves {saved}L)", + description=( + f"`{rel}` reports duplicate group `[{grp['id']}]` (`{grp['name']}`, " + f"kind={grp['kind']}, L={grp['lines']}, N={grp['count']}, saved={saved}L) " + f"at {spans_desc}. Extract shared helper/module." + ), + priority=priority, + labels=("redup", "duplication", "refactor", "scan"), + files=grp_files, + ), + source_context, + ) + ) + + if not suggestions and doc is None and raw_text.strip(): + try: + import yaml + + loaded = yaml.safe_load(raw_text) + if isinstance(loaded, dict): + doc = loaded + except Exception: + doc = None + + if not suggestions and isinstance(doc, dict): + yaml_groups = ( + doc.get("duplication_groups") + or doc.get("clusters") + or doc.get("duplicates") + or [] + ) + if isinstance(yaml_groups, list): + for idx, item in enumerate(yaml_groups, start=1): + if not isinstance(item, dict): + continue + item_files: list[str] = [] + spans: list[str] = [] + for loc in item.get("locations") or []: + if isinstance(loc, dict): + f_str = str(loc.get("file") or loc.get("path") or "").strip() + if f_str and not _is_scan_artifact_rel(f_str): + if f_str not in item_files: + item_files.append(f_str) + start = loc.get("start") + end = loc.get("end") + if start is not None and end is not None: + spans.append(f"{f_str}:{start}-{end}") + else: + spans.append(f_str) + for f_raw in item.get("files") or []: + f_str = str(f_raw or "").strip() + if f_str and not _is_scan_artifact_rel(f_str) and f_str not in item_files: + item_files.append(f_str) + spans.append(f_str) + if not item_files: + continue + dup_lines = int(item.get("lines") or 0) + saved = int(item.get("saved") or dup_lines) + name = str(item.get("name") or item.get("id") or f"DUP-{idx:03d}") + priority = "high" if saved >= 20 else "normal" + spans_desc = ", ".join(f"`{s}`" for s in spans[:6]) + suggestions.append( + _with_source_context( + Suggestion( + signal="redup_duplication", + title=f"Deduplicate {name} ({len(item_files)} files, {dup_lines}L)", + description=( + f"`{rel}` reports duplicate code ({dup_lines} lines) across " + f"{spans_desc}. Extract shared helper/module." + ), + priority=priority, + labels=("redup", "duplication", "refactor", "scan"), + files=tuple(item_files), + ), + source_context, + ) + ) + + if not suggestions and raw_text: + hotspot_files = _extract_duplication_toon_files(raw_text) + if hotspot_files: + suggestions.append( + _with_source_context( + Suggestion( + signal="redup_duplication", + title=f"Reduce code duplication in {hotspot_files[0]}", + description=( + f"`{rel}` reports duplication hotspots in " + + ", ".join(f"`{f}`" for f in hotspot_files[:6]) + + ". Extract shared helpers/modules." + ), + priority="normal", + labels=("redup", "duplication", "refactor", "scan"), + files=hotspot_files[:8], + ), + source_context, + ) + ) + return suggestions + + +def _scan_duplication_toon(project: Path) -> list[Suggestion]: + found = _first_existing_artifact(project, DUPLICATION_ARTIFACT_CANDIDATES) + if found is None: + return [] + dup_path, rel = found + try: + text = dup_path.read_text(encoding="utf-8", errors="ignore") + except OSError: + return [] + source_context = { + "evidence": { + "schema": "koru.ticket_evidence.v1", + "kind": "duplication_toon", + "artifact": _file_evidence(project, dup_path, rel), + } + } + return _parse_duplication_toon_suggestions(text, rel, source_context=source_context) + + def _parse_dup_suggestions( text: str, rel: str, @@ -426,6 +670,26 @@ def _parse_dup_suggestions( if _should_skip_code2llm_dup_ticket(text, project=project): return [] count = int(dup_match.group("count")) + dup_files: list[str] = [] + if project is not None: + tickets_doc = _load_yaml_mapping(project, _PLANFILE_TICKETS_ARTIFACT_PATHS) + for t in (tickets_doc or {}).get("tickets") or []: + if isinstance(t, dict) and t.get("signal") == "code2llm_dup": + for f in t.get("files") or (): + f_str = str(f).strip() + if f_str and not _is_scan_artifact_rel(f_str) and f_str not in dup_files: + dup_files.append(f_str) + if not dup_files: + dup_art = _first_existing_artifact(project, DUPLICATION_ARTIFACT_CANDIDATES) + if dup_art is not None: + try: + dup_files.extend( + _extract_duplication_toon_files( + dup_art[0].read_text(encoding="utf-8", errors="ignore") + ) + ) + except OSError: + pass suggestions.append( _with_source_context( Suggestion( @@ -439,7 +703,7 @@ def _parse_dup_suggestions( ), priority="high", labels=("code2llm", "duplication", "refactor", "scan"), - files=(rel,), + files=tuple(dup_files[:12]), ), source_context, ), @@ -472,7 +736,7 @@ def _parse_god_module_suggestions( ), priority="high", labels=("code2llm", "god-module", "refactor", "scan"), - files=(file_path, rel), + files=(file_path,), ), source_context, ), @@ -528,7 +792,11 @@ def _parse_high_cc_suggestions( continue func_locations = (locations or {}).get(func) or [] - located_files = tuple(entry.split(":", 1)[0] for entry in func_locations) or (rel,) + located_files = tuple( + entry.split(":", 1)[0] + for entry in func_locations + if not _is_scan_artifact_rel(entry.split(":", 1)[0]) + ) if len(func_locations) == 1: where = f"`{func_locations[0]}`" elif func_locations: @@ -556,6 +824,9 @@ def _parse_high_cc_suggestions( return suggestions +_REFACTOR_PATH_RE = re.compile(r"\b([A-Za-z0-9_./-]+\.(?:py|js|mjs|cjs|ts|tsx|jsx|rs|go|php))\b") + + def _parse_refactor_suggestions( text: str, rel: str, @@ -581,6 +852,9 @@ def _parse_refactor_suggestions( note = rm.group("note").strip() if skip_duplicate_items and _is_rm_duplicates_refactor(desc, note): continue + extracted_files = tuple( + p for p in _REFACTOR_PATH_RE.findall(desc) if not _is_scan_artifact_rel(p) + ) suggestions.append( _with_source_context( Suggestion( @@ -592,7 +866,7 @@ def _parse_refactor_suggestions( ), priority="normal", labels=("code2llm", "refactor", "scan"), - files=(rel,), + files=extracted_files, ), source_context, ), @@ -653,7 +927,7 @@ def _parse_layer_hotspot_suggestions( src_path = (module_paths or {}).get(module) if src_path is None and project is not None: src_path = _resolve_module_path(project, module) - ticket_files = (src_path,) if src_path else (rel,) + ticket_files = (src_path,) if (src_path and not _is_scan_artifact_rel(src_path)) else () suggestions.append( _with_source_context( Suggestion( @@ -667,7 +941,7 @@ def _parse_layer_hotspot_suggestions( ), priority=hotspot_priority, labels=("code2llm", "architecture", "large-module", "refactor", "scan"), - files=ticket_files or (rel,), + files=ticket_files, ), source_context, ), @@ -1303,6 +1577,7 @@ def scan_semcod_quality_artifacts(project: Path) -> list[Suggestion]: out: list[Suggestion] = [] out.extend(_scan_jscpd_report(project)) out.extend(_scan_code2llm_analysis(project)) + out.extend(_scan_duplication_toon(project)) out.extend(_scan_testql_export(project)) out.extend(_scan_redup_filtered(project)) out.extend(_scan_redup_changed(project)) @@ -1318,6 +1593,7 @@ def scan_semcod_quality_artifacts(project: Path) -> list[Suggestion]: __all__ = [ + "DUPLICATION_ARTIFACT_CANDIDATES", "_ANALYSIS_ARTIFACT_PATHS", "_CALLS_ARTIFACT_PATHS", "_PLANFILE_TICKETS_ARTIFACT_PATHS", @@ -1326,7 +1602,9 @@ def scan_semcod_quality_artifacts(project: Path) -> list[Suggestion]: "_find_analysis_file", "_first_existing_artifact", "_load_yaml_mapping", + "_parse_duplication_toon_suggestions", "_scan_code2llm_analysis", + "_scan_duplication_toon", "_scan_jscpd_report", "_scan_metrun_report", "_scan_pfix_report", diff --git a/tests/test_diff_repair_and_refactor.py b/tests/test_diff_repair_and_refactor.py new file mode 100644 index 00000000..1fe98dca --- /dev/null +++ b/tests/test_diff_repair_and_refactor.py @@ -0,0 +1,665 @@ +"""Unit tests for diff repair, SEARCH/REPLACE synthesis, governed workspace support, +scan artifact filtering, refactor context slicing, and verify gate auto-detection. +""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from koru.queue.context_files import _context_files_to_include, _is_readonly_scan_artifact +from koru.queue.context_slicing import ( + _extract_prompt_symbols, + _extract_symbol_from_corpus, + slice_file_content, +) +from koru.queue.diff_repair import ( + _repair_missing_file_headers, + _sanitize_inner_headers, + extract_unified_diff, + synthesize_diff_from_search_replace, +) +from types import SimpleNamespace +from koru.queue.patch_mode import wrap_reply_in_proposal_envelope +from koru.queue.patch_retry import _filter_readonly_scan_artifacts +from koru.queue.transaction.preflight import extract_patch +from koru.queue.verify.legacy import resolve_legacy_verify_command +from koru.queue.workspace import ( + _is_admitted_ticket_worktree, + _worktree_location, + require_unmanaged_patch_workspace, +) +from koru.scan_artifacts import ( + _parse_dup_suggestions, + _parse_duplication_toon_suggestions, + _parse_god_module_suggestions, + _parse_high_cc_suggestions, + _parse_layer_hotspot_suggestions, + _parse_refactor_suggestions, + _scan_duplication_toon, +) + + +def _git(repo: Path, *args: str) -> str: + res = subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + return res.stdout + + +def _init_repo(tmp_path: Path) -> Path: + _git(tmp_path, "init") + _git(tmp_path, "config", "user.email", "test@example.com") + _git(tmp_path, "config", "user.name", "Test") + return tmp_path + + +class TestSearchReplaceSynthesis: + def test_single_and_multi_file_search_replace_and_new_file(self, tmp_path: Path) -> None: + repo = _init_repo(tmp_path) + (repo / "src").mkdir() + (repo / "src" / "alpha.py").write_text( + "def greet():\n return 'hello'\n\ndef bye():\n return 'bye'\n", + encoding="utf-8", + ) + (repo / "src" / "beta.py").write_text( + "VALUE = 1\nOTHER = 2\n", + encoding="utf-8", + ) + _git(repo, "add", ".") + _git(repo, "commit", "-m", "init") + + llm_reply = """Here are the requested changes: + +### FILE: src/alpha.py +<<<<<<< SEARCH +def greet(): + return 'hello' +======= +def greet(): + return 'hello world' +>>>>>>> REPLACE + +### FILE: src/beta.py +<<<<<<< SEARCH +VALUE = 1 +======= +VALUE = 42 +>>>>>>> REPLACE + +### FILE: src/new_helper.py +```python +def helper() -> int: + return 99 +``` +""" + diff = synthesize_diff_from_search_replace( + llm_reply, + repo, + known_files=["src/alpha.py", "src/beta.py"], + ) + assert "diff --git a/src/alpha.py b/src/alpha.py" in diff + assert "diff --git a/src/beta.py b/src/beta.py" in diff + assert "diff --git a/src/new_helper.py b/src/new_helper.py" in diff + assert "--- /dev/null" in diff + assert "+++ b/src/new_helper.py" in diff + + # Verify git apply --check succeeds on the synthesized diff + proc = subprocess.run( + ["git", "apply", "--check"], + cwd=repo, + input=diff, + text=True, + capture_output=True, + ) + assert proc.returncode == 0, proc.stderr + + def test_wrap_and_extract_patch_uses_search_replace_fallback(self, tmp_path: Path) -> None: + repo = _init_repo(tmp_path) + (repo / "mod.py").write_text("x = 10\ny = 20\n", encoding="utf-8") + + reply = ( + "mod.py\n" + "<<<<<<< SEARCH\n" + "x = 10\n" + "=======\n" + "x = 100\n" + ">>>>>>> REPLACE\n" + ) + res = SimpleNamespace(returncode=0, stdout=reply, stderr="") + wrapped = wrap_reply_in_proposal_envelope( + res, "Update mod.py", project=repo, known_files=["mod.py"] + ) + patch, _bindings, refusal = extract_patch(wrapped, project=repo, known_files=["mod.py"]) + assert refusal is None + assert patch is not None + assert "--- a/mod.py" in patch + assert "+++ b/mod.py" in patch + assert "-x = 10" in patch + assert "+x = 100" in patch + + +class TestFuzzyDiffRepairAndSanitization: + def test_fuzzy_hunk_repair_fixes_wrong_line_numbers_and_bad_context(self, tmp_path: Path) -> None: + repo = _init_repo(tmp_path) + (repo / "app.py").write_text( + "# header 1\n# header 2\ndef compute(a, b):\n total = a + b\n return total\n", + encoding="utf-8", + ) + _git(repo, "add", ".") + _git(repo, "commit", "-m", "init") + + # Hunk has wrong line numbers (@@ -50,3 +50,3 @@) and slightly drifted context + broken_diff = ( + "```diff\n" + "--- a/app.py\n" + "+++ b/app.py\n" + "@@ -50,3 +50,3 @@\n" + " def compute(a, b):\n" + "- total = a + b\n" + "+ total = a * b\n" + " return total\n" + "```\n" + "Explanation after fence that should be stripped.\n" + ) + repaired = extract_unified_diff(broken_diff, project=repo, known_files=["app.py"]) + assert "Explanation after fence" not in repaired + proc = subprocess.run( + ["git", "apply", "--check"], + cwd=repo, + input=repaired, + text=True, + capture_output=True, + ) + assert proc.returncode == 0, f"git apply failed: {proc.stderr}\nRepaired diff:\n{repaired}" + + def test_repair_missing_file_headers_new_and_deleted_files(self) -> None: + new_file_lines = [ + "diff --git a/added.py b/added.py", + "new file mode 100644", + "@@ -0,0 +1,2 @@", + "+x = 1", + "+y = 2", + ] + repaired_new = _repair_missing_file_headers(new_file_lines) + assert "--- /dev/null" in repaired_new + assert "+++ b/added.py" in repaired_new + + del_file_lines = [ + "diff --git a/gone.py b/gone.py", + "deleted file mode 100644", + "@@ -1,2 +0,0 @@", + "-x = 1", + "-y = 2", + ] + repaired_del = _repair_missing_file_headers(del_file_lines) + assert "--- a/gone.py" in repaired_del + assert "+++ /dev/null" in repaired_del + + def test_sanitize_inner_headers_preserves_removed_triple_dash_content(self) -> None: + raw_lines = [ + "--- a/query.sql", + "+++ b/query.sql", + "@@ -1,3 +1,3 @@", + "--- a/query.sql", + "+++ b/query.sql", + " SELECT 1;", + "--- comment line to remove", + "+-- updated comment line", + " SELECT 2;", + "```", + "Some trailing prose from the model.", + ] + lines = _sanitize_inner_headers(raw_lines) + # Outer header kept, inner duplicate --- a/query.sql stripped, content --- comment kept + assert lines.count("--- a/query.sql") == 1 + assert lines.count("+++ b/query.sql") == 1 + assert "--- comment line to remove" in lines + assert "Some trailing prose from the model." not in lines + + def test_merge_multiple_diff_sections_for_same_file_and_subblock_fallback( + self, tmp_path: Path + ) -> None: + _init_repo(tmp_path) + target = tmp_path / "examples" / "composite_gates.py" + target.parent.mkdir(parents=True) + original = ( + '"""Composite gates example."""\n' + "import os\n\n" + "def first_gate(score: float) -> bool:\n" + " return score >= 85.0\n\n" + "def second_gate(penalty: float) -> float:\n" + " return penalty * 0.25\n" + ) + target.write_text(original, encoding="utf-8") + + # Model emits multiple separate diff --git sections for the same file, + # each claiming @@ -1,3 +1,3 @@ and bundling non-contiguous edits. + raw_reply = ( + "```diff\n" + "diff --git a/examples/composite_gates.py b/examples/composite_gates.py\n" + "--- a/examples/composite_gates.py\n" + "+++ b/examples/composite_gates.py\n" + "@@ -1,3 +1,4 @@\n" + " import os\n" + "+MIN_PASS_SCORE = 85.0\n" + "- return score >= 85.0\n" + "+ return score >= MIN_PASS_SCORE\n" + "```\n\n" + "```diff\n" + "diff --git a/examples/composite_gates.py b/examples/composite_gates.py\n" + "--- a/examples/composite_gates.py\n" + "+++ b/examples/composite_gates.py\n" + "@@ -1,3 +1,4 @@\n" + " import os\n" + "+PENALTY_FACTOR = 0.25\n" + "- return penalty * 0.25\n" + "+ return penalty * PENALTY_FACTOR\n" + "```\n" + ) + repaired = extract_unified_diff( + raw_reply, + project=tmp_path, + known_files=["examples/composite_gates.py"], + ) + assert repaired is not None + assert repaired.count("diff --git a/examples/composite_gates.py b/examples/composite_gates.py") == 1 + from koru.queue.workspace import diff_target_files + + assert diff_target_files(tmp_path, repaired) == ("examples/composite_gates.py",) + proc = subprocess.run( + ["git", "apply", "--check", "-"], + input=repaired, + text=True, + cwd=tmp_path, + capture_output=True, + check=False, + ) + assert proc.returncode == 0, f"git apply --check failed: {proc.stderr}\n{repaired}" + + def test_full_file_fence_fallback_for_single_known_file(self, tmp_path: Path) -> None: + _init_repo(tmp_path) + target = tmp_path / "src" / "calc.py" + target.parent.mkdir(parents=True) + target.write_text( + "def compute_total(value: int) -> int:\n" + " multiplier = 42\n" + " offset = 10\n" + " return value * multiplier + offset\n", + encoding="utf-8", + ) + reply = ( + "Here is the updated file with named constants:\n" + "```python\n" + "DEFAULT_MULTIPLIER = 42\n" + "DEFAULT_OFFSET = 10\n\n" + "def compute_total(value: int) -> int:\n" + " multiplier = DEFAULT_MULTIPLIER\n" + " offset = DEFAULT_OFFSET\n" + " return value * multiplier + offset\n" + "```\n" + ) + repaired = extract_unified_diff(reply, project=tmp_path, known_files=["src/calc.py"]) + assert repaired is not None + assert "+DEFAULT_MULTIPLIER = 42" in repaired + proc = subprocess.run( + ["git", "apply", "--check", "-"], + input=repaired, + text=True, + cwd=tmp_path, + capture_output=True, + check=False, + ) + assert proc.returncode == 0, f"git apply --check failed: {proc.stderr}\n{repaired}" + + +class TestGovernedWorkspaceSupport: + def test_admitted_ticket_worktree_accepted_via_intent_json_or_env( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + worktree = tmp_path / ".worktrees" / "ticket-450--example" + ticket_dir = worktree / "project" / "ticket-450" + ticket_dir.mkdir(parents=True) + _init_repo(worktree) + (ticket_dir / "intent.json").write_text('{"ticketId": "ticket-450"}', encoding="utf-8") + + assert _is_admitted_ticket_worktree(worktree) is True + # Should not raise even when workspace_policy.yaml is present + (worktree / ".governance").mkdir(parents=True) + (worktree / ".governance" / "workspace_policy.yaml").write_text( + "schema_version: '1'\n", encoding="utf-8" + ) + require_unmanaged_patch_workspace(worktree) + + # Also test env var override on arbitrary path + other = tmp_path / "custom_checkout" + other.mkdir() + _init_repo(other) + (other / ".governance").mkdir() + (other / ".governance" / "workspace_policy.yaml").write_text( + "schema_version: '1'\n", encoding="utf-8" + ) + monkeypatch.setenv("KORU_GOVERNED_WORKTREE_OK", "1") + assert _is_admitted_ticket_worktree(other) is True + require_unmanaged_patch_workspace(other) + + def test_worktree_location_uses_in_repo_worktrees_ticket_prefix( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("KORU_QUEUE_WORKTREE_DIR", raising=False) + loc = _worktree_location(tmp_path, "readiness") + assert loc == tmp_path / ".worktrees" / "ticket-readiness" + + loc_prefixed = _worktree_location(tmp_path, "ticket-123") + assert loc_prefixed == tmp_path / ".worktrees" / "ticket-123" + + +class TestScanArtifactFilteringAndDuplicationToon: + def test_suggestions_exclude_scan_artifact_paths(self) -> None: + toon_text = ( + "HEALTH\n" + " 🔴 GOD src/big.py = 900L, 4 classes, 35m\n" + " 🔴 DUP 3 duplicate class groups\n" + " 🟡 CC do_work CC=25 (limit: 15)\n" + "REFACTOR\n" + " 1. Split handler in src/h.py (impact=high)\n" + "LAYERS\n" + " │ !! core 600L 3C 25m CC=18.5\n" + ) + rel = "project/analysis.toon.yaml" + all_suggestions = ( + _parse_god_module_suggestions(toon_text, rel) + + _parse_dup_suggestions(toon_text, rel) + + _parse_high_cc_suggestions(toon_text, rel, locations={"do_work": ["src/complex.py:10"]}) + + _parse_refactor_suggestions(toon_text, rel) + + _parse_layer_hotspot_suggestions(toon_text, rel, module_paths={"core": "src/core.py"}) + ) + assert len(all_suggestions) == 5 + for sug in all_suggestions: + assert rel not in sug.files + assert all(not _is_readonly_scan_artifact(f) for f in sug.files) + + def test_duplication_toon_parsing_and_filtering(self, tmp_path: Path) -> None: + dup_dir = tmp_path / "project" + dup_dir.mkdir() + dup_file = dup_dir / "duplication.toon.yaml" + dup_file.write_text( + "duplication_groups:\n" + " - lines: 30\n" + " locations:\n" + " - file: src/one.py\n" + " start: 5\n" + " end: 35\n" + " - file: src/two.py\n" + " start: 10\n" + " end: 40\n", + encoding="utf-8", + ) + suggestions = _scan_duplication_toon(tmp_path) + assert len(suggestions) == 1 + assert list(suggestions[0].files) == ["src/one.py", "src/two.py"] + assert "project/duplication.toon.yaml" not in suggestions[0].files + + # Also test top-level clusters key + clusters_sugs = _parse_duplication_toon_suggestions( + {"clusters": [{"lines": 20, "files": ["src/x.py", "src/y.py"]}]}, + "project/duplication.toon.yaml", + ) + assert len(clusters_sugs) == 1 + assert list(clusters_sugs[0].files) == ["src/x.py", "src/y.py"] + + def test_context_files_and_patch_retry_filter_readonly_artifacts(self, tmp_path: Path) -> None: + (tmp_path / "project").mkdir() + (tmp_path / "project" / "analysis.toon.yaml").write_text("health: {}\n", encoding="utf-8") + (tmp_path / "project" / "duplication.toon.yaml").write_text("clusters: []\n", encoding="utf-8") + (tmp_path / "src").mkdir() + (tmp_path / "src" / "real.py").write_text("x = 1\n", encoding="utf-8") + + request = { + "prompt": "Refactor src/real.py", + "ticket_files": [ + "src/real.py", + "project/analysis.toon.yaml", + "project/duplication.toon.yaml", + ], + "context_files": [ + "project/analysis.toon.yaml", + "project/duplication.toon.yaml", + ], + } + included = _context_files_to_include(tmp_path, request) + assert included == ["src/real.py"] + + filtered = _filter_readonly_scan_artifacts( + ["src/real.py", "project/analysis.toon.yaml", "project/duplication.toon.yaml", "calls.yaml"] + ) + assert list(filtered) == ["src/real.py"] + + prompt_only = _context_files_to_include( + tmp_path, + {"prompt": "Resolve 2 unused-imports issues in src/real.py and project/analysis.toon.yaml"}, + ) + assert prompt_only == ["src/real.py"] + + + +class TestContextSlicingStopwordsAndRefactorBypass: + def test_stopwords_ignored_when_extracting_symbols(self) -> None: + prompt = "Split the god module and refactor high cyclomatic complexity into clean helpers" + assert _extract_symbol_from_corpus(prompt) is None + assert _extract_prompt_symbols(prompt) == [] + + def test_slice_file_content_retains_full_module_on_refactor_prompt(self) -> None: + lines = [f"def func_{i}():\n return {i}\n" for i in range(600)] + content = "\n".join(lines) + assert 12_000 < len(content) < 64_000 + + sliced = slice_file_content( + "src/big_module.py", + content, + "Split god module src/big_module.py (650 lines) into focused sub-modules", + max_chars=64_000, + ) + assert sliced == content + assert "# ... [" not in sliced + + +class TestVerifyAutoDetection: + def test_auto_detects_pytest_and_venv_pytest(self, tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text("[project]\nname = 'demo'\n", encoding="utf-8") + assert resolve_legacy_verify_command(tmp_path, {}) == "pytest -q" + + venv_bin = tmp_path / ".venv" / "bin" + venv_bin.mkdir(parents=True) + venv_pytest = venv_bin / "pytest" + venv_pytest.write_text("#!/bin/sh\n", encoding="utf-8") + assert resolve_legacy_verify_command(tmp_path, {}) == f"{venv_pytest} -q" + + def test_auto_detects_cargo_and_npm(self, tmp_path: Path) -> None: + rust_dir = tmp_path / "rust_proj" + rust_dir.mkdir() + (rust_dir / "Cargo.toml").write_text("[package]\nname = 'demo'\n", encoding="utf-8") + assert resolve_legacy_verify_command(rust_dir, {}) == "cargo test --locked" + + node_dir = tmp_path / "node_proj" + node_dir.mkdir() + (node_dir / "package.json").write_text('{"name": "demo"}\n', encoding="utf-8") + assert resolve_legacy_verify_command(node_dir, {}) == "npm test" + + +class TestScreenPatchQualityGate: + def test_refuses_destructive_truncation_of_public_symbols(self, tmp_path: Path) -> None: + from koru.queue.transaction.preflight import ( + PATCH_DESTRUCTIVE_TRUNCATION, + screen_patch_quality, + ) + + diff = ( + "--- a/src/service.py\n" + "+++ b/src/service.py\n" + "@@ -1,10 +1,2 @@\n" + "-def alpha():\n" + "- return 1\n" + "-def beta():\n" + "- return 2\n" + "-def gamma():\n" + "- return 3\n" + "+def alpha():\n" + "+ return 10\n" + "-def delta():\n" + "- return 4\n" + ) + outcome = screen_patch_quality(tmp_path, {"title": "Fix alpha return value"}, diff) + assert outcome is not None + assert outcome.code == PATCH_DESTRUCTIVE_TRUNCATION + assert outcome.retryable is True + assert "beta" in outcome.message + + # Allowed when ticket explicitly asks to split/extract + allowed = screen_patch_quality(tmp_path, {"title": "Split and extract helpers from service.py"}, diff) + assert allowed is None + + def test_refuses_sqlite_connect_without_closing_and_loop_subprocess(self, tmp_path: Path) -> None: + from koru.queue.transaction.preflight import ( + PATCH_INTRODUCES_ANTIPATTERN, + screen_patch_quality, + ) + + sqlite_diff = ( + "--- a/src/store.py\n" + "+++ b/src/store.py\n" + "@@ -1,2 +1,4 @@\n" + " def save(db_path):\n" + "+ with sqlite3.connect(db_path) as conn:\n" + "+ conn.execute('SELECT 1')\n" + ) + outcome = screen_patch_quality(tmp_path, {"title": "Update save"}, sqlite_diff) + assert outcome is not None + assert outcome.code == PATCH_INTRODUCES_ANTIPATTERN + assert "PERF-AUDIT-SQLITE-002" in outcome.message + + # Wrapped in closing() passes cleanly + safe_sqlite_diff = ( + "--- a/src/store.py\n" + "+++ b/src/store.py\n" + "@@ -1,2 +1,4 @@\n" + " def save(db_path):\n" + "+ with closing(sqlite3.connect(db_path)) as conn:\n" + "+ conn.execute('SELECT 1')\n" + ) + assert screen_patch_quality(tmp_path, {"title": "Update save"}, safe_sqlite_diff) is None + + # Subprocess inside loop is flagged + loop_subproc_diff = ( + "--- a/src/runner.py\n" + "+++ b/src/runner.py\n" + "@@ -1,2 +1,4 @@\n" + " def check_repos(repos):\n" + "+ for repo in repos:\n" + "+ subprocess.run(['git', '-C', str(repo), 'status'])\n" + ) + outcome2 = screen_patch_quality(tmp_path, {"title": "Check repos"}, loop_subproc_diff) + assert outcome2 is not None + assert outcome2.code == PATCH_INTRODUCES_ANTIPATTERN + assert "PERF-AUDIT" in outcome2.message + + +class TestVerifyFailureReflexionAndLessonLedger: + def test_verify_failure_reflexion_retries_and_records_self_repaired_lesson( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from koru.queue.patch_retry import ( + apply_patch_with_retry, + format_lessons_for_prompt, + load_project_lessons, + ) + + monkeypatch.setenv("KORU_GOVERNED_WORKTREE_OK", "1") + _init_repo(tmp_path) + (tmp_path / "calc.py").write_text("def add(a, b):\n return a - b\n", encoding="utf-8") + _git(tmp_path, "add", "calc.py") + _git(tmp_path, "commit", "-m", "initial") + + bad_diff = ( + "diff --git a/calc.py b/calc.py\n" + "--- a/calc.py\n" + "+++ b/calc.py\n" + "@@ -1,2 +1,2 @@\n" + " def add(a, b):\n" + "- return a - b\n" + "+ return a * b\n" + ) + good_diff = ( + "diff --git a/calc.py b/calc.py\n" + "--- a/calc.py\n" + "+++ b/calc.py\n" + "@@ -1,2 +1,2 @@\n" + " def add(a, b):\n" + "- return a - b\n" + "+ return a + b\n" + ) + + retry_prompts: list[str] = [] + + def fake_llm(action: dict, proj: Path): + retry_prompts.append(str(action.get("prompt") or "")) + return SimpleNamespace(returncode=0, stdout=good_diff, stderr="") + + def fake_verify(cmd: str, cwd: Path): + body = (cwd / "calc.py").read_text(encoding="utf-8") + if "a * b" in body: + return SimpleNamespace( + returncode=1, + stdout="", + stderr="AssertionError: expected add(2, 3) == 5, got 6\n", + ) + return SimpleNamespace(returncode=0, stdout="1 passed\n", stderr="") + + ticket = { + "id": "T-REFLEX-1", + "title": "Fix add function", + "files": ["calc.py"], + "inputs": { + "verify_command": "pytest -q", + "max_verify_retries": 1, + "promotion_mode": "apply", + }, + } + initial_reply = SimpleNamespace(returncode=0, stdout=bad_diff, stderr="") + + _res, outcome, bundle = apply_patch_with_retry( + tmp_path, + initial_reply, + ticket, + {"prompt": "Fix add(a, b) in calc.py"}, + fake_llm, + fake_verify, + ) + assert outcome is None + assert bundle is not None + assert bundle["verdict"] == "verified" + assert len(bundle["patch_attempts"]) == 2 + assert bundle["patch_attempts"][0]["outcome"].startswith("verify_failed") + assert bundle["patch_attempts"][0]["retryable"] is True + assert bundle["patch_attempts"][1]["outcome"] == "landed" + assert len(retry_prompts) == 1 + assert "Reflexion" in retry_prompts[0] + assert "AssertionError: expected add(2, 3) == 5" in retry_prompts[0] + assert "+ return a * b" in retry_prompts[0] + + # Verify that a self-repaired lesson was persisted to .koru/lessons.jsonl + lessons = load_project_lessons(tmp_path, ticket_id="T-REFLEX-1", target_files=["calc.py"]) + assert len(lessons) == 1 + assert lessons[0]["self_repaired"] is True + assert "calc.py" in lessons[0]["target_files"] + + prompt_section = format_lessons_for_prompt(tmp_path, target_files=["calc.py"]) + assert "Learned Constraints from Previous Runs" in prompt_section + assert "Recovered after initial" in prompt_section +