diff --git a/.github/workflows/agents-guard.yml b/.github/workflows/agents-guard.yml index a58c5ccaf..7035b86ec 100644 --- a/.github/workflows/agents-guard.yml +++ b/.github/workflows/agents-guard.yml @@ -111,7 +111,7 @@ jobs: github.event_name == 'pull_request_target' && steps.eligibility.outputs.should-run == 'true' && steps.api_client_base.outputs.available != 'true' - uses: "stranske/Workflows/.github/actions/setup-api-client@d68de1904bcdbe16bfe2462b73aa18f41f8a0a47" # v1 + uses: "stranske/Workflows/.github/actions/setup-api-client@c2537cc959f2ce05926c4639d25b90678abc97bc" # v1 with: secrets: ${{ toJSON(secrets) }} github_token: ${{ github.token }} @@ -180,7 +180,7 @@ jobs: steps.eligibility.outputs.should-run == 'true' && github.event_name == 'pull_request' && steps.api_client_head.outputs.available != 'true' - uses: "stranske/Workflows/.github/actions/setup-api-client@d68de1904bcdbe16bfe2462b73aa18f41f8a0a47" # v1 + uses: "stranske/Workflows/.github/actions/setup-api-client@c2537cc959f2ce05926c4639d25b90678abc97bc" # v1 with: secrets: ${{ toJSON(secrets) }} github_token: ${{ github.token }} diff --git a/.github/workflows/maint-76-claude-code-review.yml b/.github/workflows/maint-76-claude-code-review.yml index a86edf9f7..7f487aac6 100644 --- a/.github/workflows/maint-76-claude-code-review.yml +++ b/.github/workflows/maint-76-claude-code-review.yml @@ -189,7 +189,7 @@ jobs: - name: Run Claude Code Review id: claude continue-on-error: true - uses: anthropics/claude-code-action@51705da45eecce209d4700538bf8377d5b5fc695 # v1 + uses: anthropics/claude-code-action@2fee15510437d71399d9139ed60433470484a8fb # v1 with: claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} allowed_bots: '*' diff --git a/scripts/check_agents_md_freshness.py b/scripts/check_agents_md_freshness.py new file mode 100644 index 000000000..d7caff47b --- /dev/null +++ b/scripts/check_agents_md_freshness.py @@ -0,0 +1,190 @@ +#!/usr/bin/env python3 +"""Warn when the managed Orchestrator AGENTS.md section cites stale repo facts.""" + +from __future__ import annotations + +import argparse +import json +import re +import shlex +import shutil +import sys +from dataclasses import dataclass +from pathlib import Path + +MANAGED_START = "" +MANAGED_END = "" +PATH_SUFFIXES = { + ".cfg", + ".ini", + ".js", + ".json", + ".md", + ".py", + ".sh", + ".toml", + ".txt", + ".yaml", + ".yml", +} + + +@dataclass(frozen=True) +class Finding: + kind: str + value: str + message: str + + def as_dict(self) -> dict[str, str]: + return {"kind": self.kind, "value": self.value, "message": self.message} + + +def managed_section(text: str) -> str | None: + start = text.find(MANAGED_START) + end = text.find(MANAGED_END, start + len(MANAGED_START)) if start >= 0 else -1 + if start < 0 or end < 0 or end < start: + return None + return text[start : end + len(MANAGED_END)] + + +def _clean_ref(value: str) -> str: + value = value.strip() + if len(value) >= 2 and value[0] == value[-1] and value[0] in {"'", '"'}: + value = value[1:-1] + value = re.sub(r"[:#]L?\d+(?:-L?\d+)?$", "", value) + return value + + +def _looks_like_path(value: str) -> bool: + if value.startswith(("./", "../", ".github/", "docs/", "scripts/", "templates/", "tools/")): + return True + path = Path(value) + return "/" in value or path.suffix.lower() in PATH_SUFFIXES + + +def _resolve_repo_path(repo_root: Path, ref: str) -> Path | None: + root = repo_root.resolve() + raw_path = Path(ref) + candidate = raw_path if raw_path.is_absolute() else root / raw_path + candidate = candidate.resolve() + try: + candidate.relative_to(root) + except ValueError: + return None + return candidate + + +def _path_exists(repo_root: Path, ref: str) -> bool: + candidate = _resolve_repo_path(repo_root, ref) + return candidate.exists() if candidate else False + + +def _command_parts(ref: str) -> list[str]: + try: + return shlex.split(ref) + except ValueError: + return ref.split() + + +def _command_exists(repo_root: Path, ref: str) -> bool: + parts = _command_parts(ref) + if not parts: + return True + command = parts[0] + if command.startswith(("./", "../")) or "/" in command: + candidate = _resolve_repo_path(repo_root, command) + return candidate.exists() if candidate else False + return shutil.which(command) is not None + + +def _check_command_ref(repo_root: Path, ref: str) -> list[Finding]: + findings: list[Finding] = [] + if not _command_exists(repo_root, ref): + findings.append(Finding("command", ref, f"referenced command not found: {ref}")) + for arg in _command_parts(ref)[1:]: + arg = _clean_ref(arg) + if "=" in arg: + _, arg = arg.split("=", 1) + arg = _clean_ref(arg) + if _looks_like_path(arg) and not _path_exists(repo_root, arg): + findings.append(Finding("path", arg, f"referenced path not found: {arg}")) + return findings + + +def cited_refs(section: str) -> list[str]: + refs: list[str] = [] + for raw in re.findall(r"`([^`]+)`", section): + value = _clean_ref(raw) + if not value or value.startswith(("http://", "https://")): + continue + refs.append(value) + return refs + + +def check_agents_md(repo_root: Path, agents_md: Path | None = None) -> list[Finding]: + agents_path = agents_md or repo_root / "AGENTS.md" + if not agents_path.exists(): + return [] + section = managed_section(agents_path.read_text(encoding="utf-8")) + if section is None: + return [] + + findings: list[Finding] = [] + seen: set[tuple[str, str]] = set() + for ref in cited_refs(section): + if " " in ref: + for finding in _check_command_ref(repo_root, ref): + key = (finding.kind, finding.value) + if key not in seen: + findings.append(finding) + seen.add(key) + elif _looks_like_path(ref): + key = ("path", ref) + if key not in seen and not _path_exists(repo_root, ref): + findings.append(Finding("path", ref, f"referenced path not found: {ref}")) + seen.add(key) + return findings + + +def _emit_github_warnings(findings: list[Finding]) -> None: + for finding in findings: + message = finding.message.replace("%", "%25").replace("\n", "%0A").replace("\r", "%0D") + print(f"::warning title=AGENTS.md freshness::{message}") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--repo-root", type=Path, default=Path.cwd()) + parser.add_argument("--agents-md", type=Path) + parser.add_argument("--github-annotations", action="store_true") + parser.add_argument("--json", action="store_true", dest="as_json") + parser.add_argument( + "--strict", action="store_true", help="Exit non-zero when findings are present." + ) + args = parser.parse_args(argv) + + repo_root = args.repo_root.resolve() + if args.agents_md: + agents_md = ( + args.agents_md if args.agents_md.is_absolute() else repo_root / args.agents_md + ).resolve() + else: + agents_md = None + findings = check_agents_md(repo_root, agents_md) + + if args.as_json: + print(json.dumps({"findings": [finding.as_dict() for finding in findings]}, indent=2)) + elif findings: + for finding in findings: + print(finding.message) + else: + print("AGENTS.md managed section freshness check passed.") + + if args.github_annotations and findings: + _emit_github_warnings(findings) + + return 1 if args.strict and findings else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/langchain/progress_reviewer.py b/scripts/langchain/progress_reviewer.py index cddac1ace..ec7471f57 100755 --- a/scripts/langchain/progress_reviewer.py +++ b/scripts/langchain/progress_reviewer.py @@ -427,14 +427,11 @@ def review_progress_with_llm( rounds_without_completion, ) try: - from scripts.langchain._llm_client import build_client + from tools.langchain_client import build_chat_client except ImportError: - try: - from _llm_client import build_client - except ImportError: - build_client = None + build_chat_client = None - resolved = build_client(model=model) if build_client else None + resolved = build_chat_client(model=model) if build_chat_client else None if not resolved: score, aligned, unaligned = heuristic_alignment_check( acceptance_criteria, recent_commits, files_changed diff --git a/scripts/orchestrator_skill.py b/scripts/orchestrator_skill.py index cebe93da8..f2fc2e66c 100644 --- a/scripts/orchestrator_skill.py +++ b/scripts/orchestrator_skill.py @@ -80,7 +80,8 @@ def _require_nonempty_string(value: Any, field_name: str) -> str: def _validate_repo(repo: str) -> str: - if "/" not in repo or repo.startswith("/") or repo.endswith("/"): + parts = repo.split("/") + if len(parts) != 2 or not all(parts): raise OrchestratorSkillConfigError("repo must use owner/name format") return repo diff --git a/scripts/reference_packs.py b/scripts/reference_packs.py index 1654cc323..9bedba532 100644 --- a/scripts/reference_packs.py +++ b/scripts/reference_packs.py @@ -83,7 +83,8 @@ def _require_nonempty_string(value: Any, field_name: str) -> str: def _validate_repo(repo: str) -> str: - if "/" not in repo or repo.startswith("/") or repo.endswith("/"): + parts = repo.split("/") + if len(parts) != 2 or not all(parts): raise ReferencePackConfigError("repo must use owner/name format") return repo diff --git a/scripts/runner_lib/core.py b/scripts/runner_lib/core.py index c4933094a..4cbe97429 100644 --- a/scripts/runner_lib/core.py +++ b/scripts/runner_lib/core.py @@ -5,6 +5,7 @@ import argparse import base64 import binascii +import contextlib import dataclasses import datetime as dt import hashlib @@ -83,6 +84,34 @@ def _validate_provider(provider: str) -> str: return normalized +def _resolve_child_path(root: Path, path: str | Path, *, description: str) -> Path: + root_resolved = root.resolve() + raw_path = Path(path) + candidate = raw_path if raw_path.is_absolute() else root_resolved / raw_path + candidate = candidate.resolve() + try: + candidate.relative_to(root_resolved) + except ValueError as exc: + raise ValueError(f"{description} must stay within {root_resolved}") from exc + return candidate + + +def _resolve_reference_checkout_path(workspace_path: Path, checkout_path: str | Path) -> Path: + reference_root = (workspace_path / ".reference").resolve() + candidate = _resolve_child_path( + workspace_path, + checkout_path, + description="reference checkout path", + ) + try: + candidate.relative_to(reference_root) + except ValueError as exc: + raise ValueError("reference checkout path must stay within .reference") from exc + if candidate == reference_root: + raise ValueError("reference checkout path must identify a child of .reference") + return candidate + + def _runner_key(pr_number: int, head_sha: str, provider: str) -> str: payload = f"{provider}:{pr_number}:{head_sha}" return hashlib.sha256(payload.encode("utf-8")).hexdigest() @@ -314,7 +343,7 @@ def _materialize_single_checkout_plan( ) _run_git(["git", "-C", str(clone_dir), "sparse-checkout", "reapply"], env=git_env) - destination_root = workspace_path / checkout_path + destination_root = _resolve_reference_checkout_path(workspace_path, checkout_path) if destination_root.exists(): shutil.rmtree(destination_root) destination_root.mkdir(parents=True, exist_ok=True) @@ -356,11 +385,6 @@ def materialize_orchestrator_skill( return None if plan.pack: - materialize_reference_packs( - workspace_path, - reference_pack_name=plan.pack, - token=token, - ) reference_packs = _load_reference_packs_module() snapshot = reference_packs.load_reference_packs(workspace_path) matching = [ @@ -370,7 +394,17 @@ def materialize_orchestrator_skill( ] if not matching: raise ValueError(f"orchestrator skill reference pack not found: {plan.pack}") - checkout_path = workspace_path / matching[0].checkout_path + checkout_path = _resolve_reference_checkout_path( + workspace_path, + matching[0].checkout_path, + ) + with contextlib.suppress(FileNotFoundError): + shutil.rmtree(checkout_path) + materialize_reference_packs( + workspace_path, + reference_pack_name=plan.pack, + token=token, + ) else: checkout_path = _materialize_single_checkout_plan( workspace_path, @@ -413,12 +447,23 @@ def assemble_prompt( ) if context.get("materialize_orchestrator_skill"): - materialize_orchestrator_skill( + orchestrator_summary_path = materialize_orchestrator_skill( workspace, pack_override=context.get("orchestrator_skill_pack") or None, enabled_override=context.get("orchestrator_skill_enabled"), token=token, ) + else: + orchestrator_summary_raw = context.get("orchestrator_skill_summary_path") + orchestrator_summary_path = ( + Path(str(orchestrator_summary_raw)) if orchestrator_summary_raw else None + ) + if orchestrator_summary_path: + orchestrator_summary_path = _resolve_child_path( + workspace, + orchestrator_summary_path, + description="orchestrator_skill_summary_path", + ) output_file = str( context.get("output_file") or _prompt_output_name(provider, context.get("pr_number")) @@ -448,12 +493,11 @@ def assemble_prompt( if reference_summary.is_file(): parts.extend(["\n\n## Reference Packs\n", _read_text(reference_summary).rstrip()]) - orchestrator_summary = workspace / ".reference" / "ORCHESTRATOR_SKILL.md" - if orchestrator_summary.is_file(): + if orchestrator_summary_path and orchestrator_summary_path.is_file(): parts.extend( [ "\n\n## Orchestrator Skill Context\n", - _read_text(orchestrator_summary).rstrip(), + _read_text(orchestrator_summary_path).rstrip(), ] ) @@ -504,7 +548,14 @@ def _parse_jsonl_output(raw_output: str) -> tuple[list[str], list[str]]: event_type = str(event.get("type") or event.get("status") or "").lower() text = _extract_text_from_json_event(event) if "error" in event_type or event.get("error"): - errors.append(text or json.dumps(event, sort_keys=True)) + error_value = event.get("error") + nested_error = ( + _extract_text_from_json_event(error_value) + if isinstance(error_value, dict) + else None + ) + direct_error = error_value.strip() if isinstance(error_value, str) else None + errors.append(direct_error or nested_error or text or json.dumps(event, sort_keys=True)) elif text: messages.append(text) if not parsed_any: @@ -522,7 +573,7 @@ def parse_runner_output(provider: str, raw_output: str) -> RunnerResult: clipped = raw[:64000] if len(raw) > 64000 else raw messages, errors = _parse_jsonl_output(clipped) if provider == "codex" else ([], []) - final_message = messages[-1] if messages else clipped.strip() + final_message = errors[0] if errors else (messages[-1] if messages else clipped.strip()) if not errors and re.search( r"(^::error::|\bTraceback\b|\bError:|\bException\b)", @@ -943,6 +994,7 @@ def _cmd_assemble(args: argparse.Namespace) -> int: "materialize_orchestrator_skill": args.materialize_orchestrator_skill, "orchestrator_skill_pack": args.orchestrator_skill_pack or None, "orchestrator_skill_enabled": _parse_optional_bool(args.orchestrator_skill_enabled), + "orchestrator_skill_summary_path": os.environ.get("ORCHESTRATOR_SKILL_SUMMARY_PATH"), "github_token": os.environ.get("GH_TOKEN") or os.environ.get("GITHUB_TOKEN"), } prompt = assemble_prompt(args.reference_pack_name, context, args.provider) diff --git a/tools/langchain_client.py b/tools/langchain_client.py index 2daa62d57..1db1bcc92 100644 --- a/tools/langchain_client.py +++ b/tools/langchain_client.py @@ -278,14 +278,27 @@ def build_chat_client( # Auto-select: slot order (OpenAI -> Claude -> GitHub Models by default). slots = _resolve_slots() model_override = model or os.environ.get(ENV_MODEL) - if model_override: - override_provider = selected_provider or (slots[0].provider if slots else "") - if override_provider and _is_model_blocked(override_provider, model_override): - logger.warning("Refusing blocked LLM model override: %s/%s", override_provider, model_override) - return None used_override = False for slot in slots: + slot_available = any( + ( + slot.provider == PROVIDER_OPENAI and openai_token, + slot.provider == PROVIDER_ANTHROPIC and anthropic_token and chat_anthropic_cls, + slot.provider == PROVIDER_GITHUB and github_token, + ) + ) + if not slot_available: + continue slot_model = model_override if model_override and not used_override else slot.model + if _is_model_blocked(slot.provider, slot_model): + logger.warning("Skipping blocked LLM model override: %s/%s", slot.provider, slot_model) + if model_override and not used_override: + used_override = True + slot_model = slot.model + if _is_model_blocked(slot.provider, slot_model): + continue + else: + continue if slot.provider == PROVIDER_OPENAI and openai_token: with contextlib.suppress(Exception): client = _build_openai_client( diff --git a/tools/llm_provider.py b/tools/llm_provider.py index 4b1a0cad3..7fb0603a2 100644 --- a/tools/llm_provider.py +++ b/tools/llm_provider.py @@ -29,13 +29,6 @@ from abc import ABC, abstractmethod from dataclasses import dataclass -from tools.llm_registry import ( - PROVIDER_ANTHROPIC, - PROVIDER_GITHUB, - PROVIDER_OPENAI, - configured_model_for_provider, -) - logger = logging.getLogger(__name__) # GitHub Models API endpoint (OpenAI-compatible) @@ -344,11 +337,8 @@ def _get_client(self): logger.warning("langchain_openai not installed") return None - model = configured_model_for_provider(PROVIDER_GITHUB, fallback="gpt-4.1") - if not model: - return None return ChatOpenAI( - model=model, + model="gpt-4.1", # Battle-tested, reliable, available on GitHub Models base_url=GITHUB_MODELS_BASE_URL, api_key=os.environ.get("GITHUB_TOKEN"), temperature=0.1, # Low temperature for consistent analysis @@ -560,7 +550,7 @@ def _parse_response( confidence=adjusted_confidence, reasoning=reasoning, provider_used=self.name, - model_name=configured_model_for_provider(PROVIDER_GITHUB, fallback="gpt-4.1"), + model_name="gpt-4.1", # Actual model used by GitHubModelsProvider raw_confidence=raw_confidence if adjusted_confidence != raw_confidence else None, confidence_adjusted=adjusted_confidence != raw_confidence, quality_warnings=warnings if warnings else None, @@ -575,7 +565,7 @@ def _parse_response( confidence=0.0, reasoning=f"Failed to parse response: {e}", provider_used=self.name, - model_name=configured_model_for_provider(PROVIDER_GITHUB, fallback="gpt-4.1"), + model_name="gpt-4.1", # Actual model used by GitHubModelsProvider ) @@ -600,11 +590,8 @@ def _get_client(self): logger.warning("langchain_openai not installed") return None - model = configured_model_for_provider(PROVIDER_OPENAI, fallback="gpt-5.1-codex") - if not model: - return None return ChatOpenAI( - model=model, + model="gpt-5.1-codex", # Purpose-built for analyzing Codex coding sessions api_key=os.environ.get("OPENAI_API_KEY"), temperature=0.1, ) @@ -639,7 +626,7 @@ def analyze_completion( confidence=result.confidence, reasoning=result.reasoning, provider_used=self.name, - model_name=configured_model_for_provider(PROVIDER_OPENAI, fallback="gpt-5.1-codex"), + model_name="gpt-5.1-codex", # Actual model used by OpenAIProvider raw_confidence=result.raw_confidence, confidence_adjusted=result.confidence_adjusted, quality_warnings=result.quality_warnings, @@ -669,14 +656,8 @@ def _get_client(self): logger.warning("langchain_anthropic not installed") return None - model = configured_model_for_provider( - PROVIDER_ANTHROPIC, - fallback="claude-sonnet-4-5-20250929", - ) - if not model: - return None return ChatAnthropic( - model=model, + model="claude-sonnet-4-5-20250929", anthropic_api_key=os.environ.get(ANTHROPIC_API_KEY_ENV), temperature=0.1, ) @@ -721,10 +702,7 @@ def analyze_completion( confidence=result.confidence, reasoning=result.reasoning, provider_used=self.name, - model_name=configured_model_for_provider( - PROVIDER_ANTHROPIC, - fallback="claude-sonnet-4-5-20250929", - ), + model_name="claude-sonnet-4-5-20250929", raw_confidence=result.raw_confidence, confidence_adjusted=result.confidence_adjusted, quality_warnings=result.quality_warnings, diff --git a/tools/llm_registry.py b/tools/llm_registry.py index e6ed9a624..ec46289be 100644 --- a/tools/llm_registry.py +++ b/tools/llm_registry.py @@ -79,8 +79,13 @@ def load_model_registry() -> list[ModelRegistryEntry]: logger.warning("Invalid model registry format in %s; expected object", path) return [] + raw_models = payload.get("models", []) + if not isinstance(raw_models, list): + logger.warning("Invalid model registry format in %s; expected models list", path) + return [] + entries: list[ModelRegistryEntry] = [] - for raw_entry in payload.get("models", []): + for raw_entry in raw_models: if not isinstance(raw_entry, dict): logger.warning("Ignoring invalid model registry entry in %s; expected object", path) continue @@ -89,6 +94,14 @@ def load_model_registry() -> list[ModelRegistryEntry]: if not provider or not model: continue quality_payload = raw_entry.get("quality", {}) + if not isinstance(quality_payload, dict): + logger.warning( + "Ignoring invalid quality scores for %s/%s in %s; expected object", + provider, + model, + path, + ) + quality_payload = {} quality = { str(tier).upper(): float(score) for tier, score in quality_payload.items() @@ -173,11 +186,14 @@ def configured_model_for_provider( model = str(slot.get("model", "")).strip() slot_tier = str(slot.get("quality_tier") or slot.get("tier") or tier).strip() if not model and slot_tier: - model = select_model_for_tier( - provider=slot_provider or "", - tier=slot_tier, - registry=entries, - ) or "" + model = ( + select_model_for_tier( + provider=slot_provider or "", + tier=slot_tier, + registry=entries, + ) + or "" + ) if model and not is_model_blocked(slot_provider or "", model, registry=entries): return model @@ -249,6 +265,11 @@ def apply_slot_env_overrides( model = (model_override or slot.model).strip() if is_model_blocked(provider, model, registry=registry): logger.warning("Skipping blocked LLM slot override: %s/%s", provider, model) + override_requested = provider_override is not None or model_override is not None + if override_requested and not is_model_blocked( + slot.provider, slot.model, registry=registry + ): + updated.append(slot) continue updated.append( SlotDefinition(