diff --git a/.agents/skills/autoreview/SKILL.md b/.agents/skills/autoreview/SKILL.md new file mode 100644 index 000000000..e4c9f5dc2 --- /dev/null +++ b/.agents/skills/autoreview/SKILL.md @@ -0,0 +1,182 @@ +--- +name: autoreview +description: "Auto Review closeout. Codex review is the default when no engine is set and is the recommended reviewer." +--- + +# Auto Review + +Run the bundled structured review helper as a closeout check. This is code review, not Guardian `auto_review` approval routing. + +Codex review is the default when no engine is set. It usually delivers the best review results and should remain the normal final closeout engine. + +Use when: + +- user asks for Codex review / Claude review / autoreview / second-model review +- after non-trivial code edits, before final/commit/ship +- reviewing a local branch or PR branch after fixes + +OpenClaw Windows Node specifics: + +- The release/default branch is `master`; the helper resolves `origin/HEAD` so do not hardcode `origin/main`. +- After review-triggered code changes, run the repo-required validation: `./build.ps1`, `dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore`, and `dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore`. +- If the checkout is on macOS/Linux or lacks .NET/PowerShell/Windows SDK prerequisites, report the validation blocker clearly instead of pretending the Windows validation ran. +- Do not send absolute local checkout paths, home-directory names, or private temp/worktree paths to review engines; use repo-relative labels or the repo name. + +## Contract + +- Treat review output as advisory. Never blindly apply it. +- Verify every finding by reading the real code path and adjacent files. +- Read dependency docs/source/types when the finding depends on external behavior. +- Reject unrealistic edge cases, speculative risks, broad rewrites, and fixes that over-complicate the codebase. +- Prefer small fixes at the right ownership boundary; no refactor unless it clearly improves the bug class. +- Keep going until structured review returns no accepted/actionable findings. +- If a review-triggered fix changes code, rerun focused tests and rerun the structured review helper. +- For security-audit suppression changes, verify accepted findings remain auditable: suppressed findings stay in structured output, active output keeps an unsuppressible suppression notice, and aggregate findings cannot hide unrelated active risk. +- Never switch or override the requested review engine/model. If the review hits model capacity, retry the same command a few times with the same engine/model. +- Be patient with large bundles. Structured review can take up to 30 minutes while the model call is active, especially with Codex tools or web search. +- Treat heartbeat lines like `review still running: ... elapsed=... pid=...` as healthy progress, not a hang. Let the helper continue while heartbeats are advancing. Pass `--stream-engine-output` when live engine text is useful; Codex and Claude filter tool/file chatter, other engines pass raw output through. +- Do not kill a review just because it has been quiet for 2-5 minutes, or because it is still running under the 30-minute window. Inspect the process only after missing multiple expected heartbeats, after 30 minutes, or after an obviously failed subprocess; prefer letting the same helper command finish. +- Tools are useful in review mode. The helper allows read-only inspection tools and web search by default so reviewers can check dependency contracts, upstream docs, and current behavior. +- Security perspective is always included, but it should not cripple legitimate functionality. Report security findings only when the change creates a concrete, actionable risk or removes an important safety check. +- For regression provenance, if no blamed PR is traceable, use the blamed commit as the provenance: commit SHA, date, and author username. Do not guess a merger or frame missing PR metadata as a separate finding. +- Do not invoke built-in `codex review`, nested reviewers, or reviewer panels from inside the review. The helper builds one bundle, calls one selected engine, validates one structured result, and stops. +- Stop as soon as the helper exits 0 with no accepted/actionable findings. Do not run an extra review just to get a nicer "clean" line, a second opinion, or clearer closeout wording. +- Treat the helper's successful exit plus absence of actionable findings as the clean review result, even if the underlying Codex CLI output is terse. +- Multi-reviewer panels are opt-in only. Use them when explicitly requested or when risk justifies the extra spend; the main agent still verifies every accepted finding before fixing. +- If rejecting a finding as intentional/not worth fixing, add a brief inline code comment only when it explains a real invariant or ownership decision that future reviewers should know. +- If `gh`/Gitcrawl reports `database disk image is malformed`, run `gitcrawl doctor --json` once to let the portable cache repair before retrying review; do not bypass the shim unless repair fails and freshness requires live GitHub. +- If Gitcrawl reports a portable manifest mismatch, source/runtime DB health error, or stale portable-store checkout, run `gitcrawl doctor --json` and inspect `source_db_health`, `runtime_db_health`, and `portable_store_status` before falling back to live GitHub. +- Do not push just to review. Push only when the user requested push/ship/PR update. + +## Pick Target + +Dirty local work: + +```bash + --mode local +``` + +Use this only when the patch is actually unstaged/staged/untracked in the +current checkout. `--mode uncommitted` is accepted as an alias for `--mode local`. +For committed, pushed, or PR work, point the helper at the commit +or branch diff instead; do not force dirty modes just +because the helper docs mention dirty work first. A clean local review +only proves there is no local patch. + +Branch/PR work: + +```bash + --mode branch --base origin/master +``` + +Optional review context is first-class: + +```bash + --mode branch --base origin/master --prompt-file /tmp/review-notes.md --dataset /tmp/evidence.json +``` + +If an open PR exists, use its actual base: + +```bash +base=$(gh pr view --json baseRefName --jq .baseRefName) + --mode branch --base "origin/$base" +``` + +Committed single change: + +```bash + --mode commit --commit HEAD +``` + +or with the helper: + +Use commit review for already-landed or already-pushed work on the default +branch. Reviewing clean `master` against `origin/master` is usually an empty +diff after push. For a small stack, review each commit explicitly or review the +branch before merging with `--base`. + +## Parallel Closeout + +Format first if formatting can change line locations. Then it is OK to run tests and review in parallel: + +```bash +scripts/autoreview --parallel-tests "" +``` + +Tradeoff: tests may force code changes that stale the review. If tests or review lead to code edits, rerun the affected tests and rerun review until no accepted/actionable findings remain. Once that rerun exits cleanly, stop; do not spend another long review cycle on redundant confirmation. + +## Review Panels + +Run multiple reviewers against one frozen bundle: + +```bash + --reviewers codex,claude +``` + +`--panel` is shorthand for Codex plus Claude unless `--engine` changes the first reviewer: + +```bash + --panel +``` + +Set reviewer models and thinking/effort explicitly: + +```bash + --reviewers codex,claude --model codex=gpt-5.1 --thinking codex=high --model claude=sonnet --thinking claude=max +``` + +Inline syntax is also supported: + +```bash + --reviewers codex:gpt-5.1:high,claude:sonnet:max +``` + +Codex maps thinking to `model_reasoning_effort` and accepts `low`, `medium`, +`high`, or `xhigh`. Claude maps thinking to `--effort` and also accepts `max`. +Engines without a real thinking knob reject `--thinking`. + +## Context Efficiency + +Run the helper directly so target selection, engine choice, structured validation, and exit status all stay in one path. If output is noisy, summarize the completed helper output after it returns; do not ask another agent or reviewer to rerun the review. + +## Helper + +OpenClaw Windows Node repo-local helper: + +```bash +.agents/skills/autoreview/scripts/autoreview --help +``` + +The helper: + +- chooses dirty local changes first +- accepts `--mode uncommitted` as an alias for `--mode local` +- otherwise uses current PR base if `gh pr view` works +- otherwise uses the remote default branch for non-default branches +- supports `--engine codex`, `claude`, `droid`, and `copilot`; default is `AUTOREVIEW_ENGINE` or `codex`; Codex should remain the default when nothing is set +- use `--mode commit --commit ` for already-committed work, especially clean default-branch work after landing +- should be left in `--mode auto` or forced to `--mode branch` for PR/branch work; do not force `--mode local` after committing +- writes only to stdout unless `--output`, `--json-output`, or live streamed engine stderr is set +- supports `--dry-run`, `--parallel-tests`, `--prompt`, `--prompt-file`, `--dataset`, `--no-tools`, `--no-web-search`, and commit refs +- supports `--stream-engine-output` or `AUTOREVIEW_STREAM_ENGINE_OUTPUT=1` for live engine text while preserving structured validation; Codex and Claude hide tool/file event details, emit compact activity summaries, and report usage at turn completion +- supports opt-in review panels with `--panel` / `--reviewers`, plus per-engine `--model` and `--thinking` +- allows read-only tools and web search by default only for sandboxed Codex review; forbids nested review in the prompt; Codex is run through `codex exec` with read-only sandbox and structured output +- disables non-Codex unsandboxed reviewer tools by default; set `AUTOREVIEW_ALLOW_UNSANDBOXED_TOOLS=1` only when that tradeoff is intentional +- invokes reviewer engines from sanitized temporary workspaces so local home/worktree paths are not exposed; set `AUTOREVIEW_SAFE_TMP` if the default neutral temp root is unavailable +- writes Codex sidecar files and the full change bundle inside the sanitized workspace as `schema.json`, `output.json`, `CHANGE_BUNDLE.md`, or `prompt.txt` for read-only inspection +- redacts common local home-directory path shapes from review bundles, omits symlink contents from text bundles, and omits raw symlink patch bodies +- caps patch, untracked-file, and local-bundle text with explicit truncation markers so dirty checkouts cannot silently create unbounded prompts +- prints `review still running: elapsed=s pid=` to stderr at long-running intervals while waiting for the selected review engine, unless streamed output or compact Codex activity has been visible recently +- prints `autoreview clean: no accepted/actionable findings reported` when the selected review command exits 0 +- exits nonzero when accepted/actionable findings are present + +## Final Report + +Include: + +- review command used +- tests/proof run +- findings accepted/rejected, briefly why +- the clean review result from the final helper/review run, or why a remaining finding was consciously rejected + +Do not run another review solely to improve the final report wording. If the final helper run exited 0 and produced no accepted/actionable findings, report that exact run as clean. diff --git a/.agents/skills/autoreview/scripts/autoreview b/.agents/skills/autoreview/scripts/autoreview new file mode 100755 index 000000000..cb9ad014e --- /dev/null +++ b/.agents/skills/autoreview/scripts/autoreview @@ -0,0 +1,1371 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import argparse +import concurrent.futures +import copy +import json +import os +import queue +import re +import subprocess +import sys +import tempfile +import textwrap +import threading +import time +from pathlib import Path +from typing import Any, Callable + + +ENGINES = ("codex", "claude", "droid", "copilot") +THINKING_LEVELS_BY_ENGINE = { + "codex": {"low", "medium", "high", "xhigh"}, + "claude": {"low", "medium", "high", "xhigh", "max"}, + "droid": set(), + "copilot": set(), +} +PATCH_LIMIT = 180_000 +UNTRACKED_FILE_LIMIT = 180_000 +BUNDLE_LIMIT = 500_000 +MIN_UNTRACKED_BUDGET = 1_000 +PRIVATE_PATH_PATTERNS = ( + re.compile(r"/" r"Users/" r"[^\r\n'\"`<>]+"), + re.compile(r"/" r"home/" r"[^\r\n'\"`<>]+"), + re.compile(r"[A-Za-z]:[\\/]+Users[\\/]+[^\r\n'\"`<>]+", re.IGNORECASE), +) + + +SCHEMA: dict[str, Any] = { + "type": "object", + "additionalProperties": False, + "required": [ + "findings", + "overall_correctness", + "overall_explanation", + "overall_confidence", + ], + "properties": { + "findings": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": False, + "required": [ + "title", + "body", + "priority", + "confidence", + "category", + "code_location", + ], + "properties": { + "title": {"type": "string", "minLength": 1, "maxLength": 140}, + "body": {"type": "string", "minLength": 1, "maxLength": 2000}, + "priority": {"type": "string", "enum": ["P0", "P1", "P2", "P3"]}, + "confidence": {"type": "number", "minimum": 0, "maximum": 1}, + "category": { + "type": "string", + "enum": ["bug", "security", "regression", "test_gap", "maintainability"], + }, + "code_location": { + "type": "object", + "additionalProperties": False, + "required": ["file_path", "line"], + "properties": { + "file_path": {"type": "string", "minLength": 1}, + "line": {"type": "integer", "minimum": 1}, + }, + }, + }, + }, + }, + "overall_correctness": { + "type": "string", + "enum": ["patch is correct", "patch is incorrect"], + }, + "overall_explanation": {"type": "string", "minLength": 1, "maxLength": 3000}, + "overall_confidence": {"type": "number", "minimum": 0, "maximum": 1}, + }, +} + + +def run(args: list[str], cwd: Path, *, input_text: str | None = None, check: bool = True) -> subprocess.CompletedProcess[str]: + result = subprocess.run( + args, + cwd=cwd, + input=input_text, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + if check and result.returncode != 0: + cmd = " ".join(args) + raise SystemExit(f"command failed ({result.returncode}): {cmd}\n{result.stderr or result.stdout}") + return result + + +def run_with_heartbeat( + args: list[str], + cwd: Path, + *, + input_text: str | None = None, + label: str, + heartbeat_seconds: int = 60, + stream_output: bool = False, + stream_display: Callable[[str, str], str | None] | None = None, +) -> subprocess.CompletedProcess[str]: + if stream_output: + return run_with_stream( + args, + cwd, + input_text=input_text, + label=label, + heartbeat_seconds=heartbeat_seconds, + stream_display=stream_display, + ) + started = time.monotonic() + proc = subprocess.Popen( + args, + cwd=cwd, + stdin=subprocess.PIPE if input_text is not None else None, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + first_communicate = True + while True: + try: + stdout, stderr = proc.communicate( + input=input_text if first_communicate else None, + timeout=heartbeat_seconds, + ) + return subprocess.CompletedProcess(args, int(proc.returncode or 0), stdout, stderr) + except subprocess.TimeoutExpired: + first_communicate = False + elapsed = int(time.monotonic() - started) + print(f"review still running: {label} elapsed={elapsed}s pid={proc.pid}", file=sys.stderr, flush=True) + + +def run_with_stream( + args: list[str], + cwd: Path, + *, + input_text: str | None, + label: str, + heartbeat_seconds: int, + stream_display: Callable[[str, str], str | None] | None, +) -> subprocess.CompletedProcess[str]: + started = time.monotonic() + proc = subprocess.Popen( + args, + cwd=cwd, + stdin=subprocess.PIPE if input_text is not None else None, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + bufsize=1, + ) + events: queue.Queue[tuple[str, str | None]] = queue.Queue() + stdout_parts: list[str] = [] + stderr_parts: list[str] = [] + + def read_stream(name: str, stream: Any) -> None: + try: + for line in iter(stream.readline, ""): + events.put((name, line)) + finally: + events.put((name, None)) + + def write_stdin() -> None: + if proc.stdin is None or input_text is None: + return + try: + proc.stdin.write(input_text) + proc.stdin.close() + except BrokenPipeError: + return + + threads = [ + threading.Thread(target=read_stream, args=("stdout", proc.stdout), daemon=True), + threading.Thread(target=read_stream, args=("stderr", proc.stderr), daemon=True), + ] + for thread in threads: + thread.start() + stdin_thread = threading.Thread(target=write_stdin, daemon=True) + stdin_thread.start() + + open_streams = 2 + while open_streams: + try: + name, line = events.get(timeout=heartbeat_seconds) + except queue.Empty: + elapsed = int(time.monotonic() - started) + print(f"review still running: {label} elapsed={elapsed}s pid={proc.pid}", file=sys.stderr, flush=True) + continue + if line is None: + open_streams -= 1 + continue + if name == "stdout": + stdout_parts.append(line) + else: + stderr_parts.append(line) + display = stream_display(name, line) if stream_display else line + if display: + target = sys.stdout if name == "stdout" else sys.stderr + target.write(display) + target.flush() + + for thread in threads: + thread.join() + stdin_thread.join(timeout=1) + returncode = proc.wait() + return subprocess.CompletedProcess(args, returncode, "".join(stdout_parts), "".join(stderr_parts)) + + +def git(repo: Path, *args: str, check: bool = True) -> str: + return run(["git", *args], repo, check=check).stdout + + +def repo_root() -> Path: + result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + if result.returncode != 0: + raise SystemExit("autoreview must run inside a git repository") + return Path(result.stdout.strip()).resolve() + + +def current_branch(repo: Path) -> str: + return git(repo, "branch", "--show-current", check=False).strip() or "detached" + + +def default_branch(repo: Path) -> str: + remote_head = git( + repo, + "symbolic-ref", + "--quiet", + "--short", + "refs/remotes/origin/HEAD", + check=False, + ).strip() + if remote_head.startswith("origin/"): + return remote_head.removeprefix("origin/") + for candidate in ("master", "main"): + result = run(["git", "rev-parse", "--verify", f"origin/{candidate}"], repo, check=False) + if result.returncode == 0: + return candidate + return "master" + + +def default_base_ref(repo: Path) -> str: + return f"origin/{default_branch(repo)}" + + +def is_dirty(repo: Path) -> bool: + return bool(git(repo, "status", "--porcelain").strip()) + + +def choose_target(repo: Path, mode: str, base_ref: str | None) -> tuple[str, str | None]: + mode = "local" if mode == "uncommitted" else mode + branch = current_branch(repo) + default = default_branch(repo) + if mode == "local" or (mode == "auto" and is_dirty(repo)): + return "local", None + if mode == "commit": + return "commit", None + if mode == "branch" or (mode == "auto" and branch != default): + return "branch", base_ref or detect_pr_base(repo) or default_base_ref(repo) + raise SystemExit(f"no review target: clean {default} checkout and no forced mode") + + +def detect_pr_base(repo: Path) -> str | None: + if not shutil_which("gh"): + return None + result = run(["gh", "pr", "view", "--json", "baseRefName", "--jq", ".baseRefName"], repo, check=False) + base = result.stdout.strip() + return f"origin/{base}" if result.returncode == 0 and base else None + + +def shutil_which(name: str) -> str | None: + for part in os.environ.get("PATH", "").split(os.pathsep): + candidate = Path(part) / name + if candidate.exists() and os.access(candidate, os.X_OK): + return str(candidate) + return None + + +def bounded(text: str, limit: int = 180_000) -> str: + if len(text) <= limit: + return text + return text[:limit] + f"\n\n[truncated at {limit} characters]\n" + + +def redact_private_paths(text: str) -> str: + redacted = text + for pattern in PRIVATE_PATH_PATTERNS: + redacted = pattern.sub("[redacted-local-path]", redacted) + return redacted + + +def sanitize_git_patch(patch: str) -> str: + blocks: list[list[str]] = [] + current: list[str] = [] + for line in patch.splitlines(keepends=True): + if line.startswith("diff --git ") and current: + blocks.append(current) + current = [] + current.append(line) + if current: + blocks.append(current) + + sanitized: list[str] = [] + for block in blocks: + sanitized.extend(sanitize_symlink_diff_block(block)) + return redact_private_paths("".join(sanitized)) + + +def sanitize_symlink_diff_block(block: list[str]) -> list[str]: + old_symlink = any(line.startswith(("deleted file mode 120000", "old mode 120000")) for line in block) + new_symlink = any(line.startswith(("new file mode 120000", "new mode 120000")) for line in block) + mode_line_seen = any(is_symlink_mode_line(line) for line in block) + if is_symlink_index_only_block(block, mode_line_seen): + old_symlink = True + new_symlink = True + if not old_symlink and not new_symlink: + return block + + result: list[str] = [] + omitted_old = False + omitted_new = False + in_hunk = False + for line in block: + if line.startswith("@@"): + in_hunk = True + result.append(line) + continue + if in_hunk and old_symlink and line.startswith("-") and not line.startswith("--- "): + if not omitted_old: + result.append("-[symlink target omitted]\n") + omitted_old = True + continue + if in_hunk and new_symlink and line.startswith("+") and not line.startswith("+++ "): + if not omitted_new: + result.append("+[symlink target omitted]\n") + omitted_new = True + continue + if in_hunk and old_symlink and new_symlink and line.startswith(" "): + continue + result.append(line) + return result + + +def is_symlink_index_only_block(block: list[str], mode_line_seen: bool) -> bool: + return not mode_line_seen and any(is_symlink_index_line(line) for line in block) + + +def is_symlink_index_line(line: str) -> bool: + if not line.startswith("index "): + return False + parts = line.split() + return len(parts) >= 3 and parts[-1] == "120000" + + +def is_symlink_mode_line(line: str) -> bool: + prefixes = ("new file mode ", "deleted file mode ", "old mode ", "new mode ") + return line.startswith(prefixes) and line.strip().endswith("120000") + + +def bounded_field(text: str, limit: int) -> str: + if len(text) <= limit: + return text + suffix = "\n\n[truncated]" + return text[: max(0, limit - len(suffix))] + suffix + + +def read_text(path: Path, limit: int | None = 40_000, display_path: str | None = None) -> str: + label = display_path or path.name + if path.is_symlink(): + return f"[symlink omitted: {label}]" + try: + data = path.read_bytes() + except OSError: + return f"[unreadable: {label}]" + if b"\0" in data: + return "[binary file omitted]" + text = redact_private_paths(data.decode("utf-8", errors="replace")) + if limit is None: + return text + return bounded(text, limit) + + +def local_bundle(repo: Path) -> str: + parts = [ + "# Git Status", + redact_private_paths(git(repo, "status", "--short")), + "# Staged Diff", + redact_private_paths(git(repo, "diff", "--cached", "--stat")), + bounded(sanitize_git_patch(git(repo, "diff", "--cached", "--patch", "--find-renames")), PATCH_LIMIT), + "# Unstaged Diff", + redact_private_paths(git(repo, "diff", "--stat")), + bounded(sanitize_git_patch(git(repo, "diff", "--patch", "--find-renames")), PATCH_LIMIT), + ] + untracked = [line for line in git(repo, "ls-files", "--others", "--exclude-standard").splitlines() if line] + if untracked: + parts.append("# Untracked Files") + for index, rel in enumerate(untracked): + path = repo / rel + try: + size = path.lstat().st_size + except OSError: + size = 0 + if not path.is_symlink() and size > UNTRACKED_FILE_LIMIT: + raise SystemExit( + f"untracked file exceeds autoreview per-file bundle cap ({UNTRACKED_FILE_LIMIT} bytes): {rel}; " + "stage/commit the change or reduce the file before review" + ) + remaining = BUNDLE_LIMIT - len("\n\n".join(parts)) + if remaining < MIN_UNTRACKED_BUDGET: + omitted = len(untracked) - index + raise SystemExit( + f"local untracked bundle exceeds autoreview cap ({BUNDLE_LIMIT} bytes); " + f"{omitted} untracked file(s) were not bundled. Stage/commit or reduce untracked files before review." + ) + entry = f"## {rel}\n{read_text(path, limit=UNTRACKED_FILE_LIMIT, display_path=rel)}" + projected = len("\n\n".join(parts)) + 2 + len(entry) + if projected > BUNDLE_LIMIT: + omitted = len(untracked) - index + raise SystemExit( + f"local untracked bundle exceeds autoreview cap ({BUNDLE_LIMIT} bytes); " + f"{omitted} untracked file(s) were not bundled. Stage/commit or reduce untracked files before review." + ) + parts.append(entry) + bundle = "\n\n".join(parts) + if len(bundle) > BUNDLE_LIMIT: + raise SystemExit( + f"local bundle exceeds autoreview cap ({BUNDLE_LIMIT} bytes). " + "Review a smaller patch, stage/commit the change, or use branch/commit mode." + ) + return bundle + + +def branch_bundle(repo: Path, base_ref: str) -> str: + git(repo, "fetch", "origin", "--quiet", check=False) + return "\n\n".join( + [ + "# Branch Diff", + f"base: {base_ref}", + redact_private_paths(git(repo, "diff", "--stat", f"{base_ref}...HEAD")), + bounded(sanitize_git_patch(git(repo, "diff", "--patch", "--find-renames", f"{base_ref}...HEAD")), PATCH_LIMIT), + ] + ) + + +def commit_bundle(repo: Path, commit_ref: str) -> str: + return "\n\n".join( + [ + "# Commit Diff", + f"commit: {commit_ref}", + redact_private_paths(git(repo, "show", "--stat", "--format=commit %H", commit_ref)), + bounded(sanitize_git_patch(git(repo, "show", "--patch", "--find-renames", "--format=", commit_ref)), PATCH_LIMIT), + ] + ) + + +def review_paths(repo: Path, target: str, target_ref: str | None, commit_ref: str) -> set[str]: + names: set[str] = set() + if target == "local": + sources = [ + git(repo, "diff", "--name-only", "--cached"), + git(repo, "diff", "--name-only"), + git(repo, "ls-files", "--others", "--exclude-standard"), + ] + elif target == "branch": + assert target_ref + sources = [git(repo, "diff", "--name-only", f"{target_ref}...HEAD")] + else: + sources = [git(repo, "show", "--name-only", "--format=", commit_ref)] + for source in sources: + for line in source.splitlines(): + path = line.strip() + if path: + names.add(path) + return names + + +def load_extra_prompt(args: argparse.Namespace) -> str: + chunks: list[str] = [] + for value in args.prompt or []: + chunks.append(redact_private_paths(value)) + for path in args.prompt_file or []: + chunks.append(redact_private_paths(Path(path).read_text())) + return "\n\n".join(chunks) + + +def safe_display_path(path: Path, repo: Path) -> str: + resolved = path.expanduser().resolve() + try: + return str(resolved.relative_to(repo)) + except ValueError: + return resolved.name + + +def load_datasets(args: argparse.Namespace, repo: Path) -> str: + chunks: list[str] = [] + for spec in args.dataset or []: + path = Path(spec) + if path.is_dir(): + raise SystemExit(f"--dataset must be a file, got directory: {path}") + label = safe_display_path(path, repo) + chunks.append(f"# Dataset: {label}\n{read_text(path, display_path=label)}") + return "\n\n".join(chunks) + + +def build_prompt(repo: Path, target: str, target_ref: str | None, bundle: str, extra_prompt: str, datasets: str) -> str: + target_line = f"{target} {target_ref}" if target_ref else target + return textwrap.dedent( + f""" + You are a senior code reviewer. Review the provided git change bundle only. + + Hard rules: + - Return exactly one JSON object and nothing else. Do not wrap it in Markdown. + - The JSON object must match this schema exactly: + {json.dumps(SCHEMA, indent=2)} + - Do not modify files. + - Do not invoke nested reviewers or review tools. + - Forbidden nested review commands include: codex review, autoreview, claude review, oracle review. + - You may use read-only tools and web search to inspect files, dependency contracts, upstream docs, current behavior, and security implications. + - Shell commands, if available, must be read-only inspection commands. Do not run tests, formatters, package installs, generators, network mutation commands, git mutation commands, or commands that write files. + - Report only actionable defects introduced or exposed by this change. + - Prefer high-signal findings over style feedback. + - Include security findings: injection, secret leaks, authz/authn bypass, path traversal, unsafe deserialization, unsafe filesystem or shell use, privacy leaks, and credential handling. + - Do not reject legitimate functionality merely because it touches shell, filesystem, network, auth, or sensitive data. Report a security finding only when the patch creates a concrete exploitable risk, removes an important safety check, or lacks validation at a trust boundary. + - For each finding, use the smallest file/line location that demonstrates the issue. + - If there are no actionable findings, return an empty findings array and mark the patch correct. + + Review target: {target_line} + Repository: {repo.name} + + {extra_prompt} + + {datasets} + + # Change Bundle + {bundle} + """ + ).strip() + + +def safe_temp_root() -> Path: + configured = os.environ.get("AUTOREVIEW_SAFE_TMP") + if configured: + root = Path(configured) + elif os.name == "nt": + root = Path(os.environ.get("SystemRoot", r"C:\Windows")) / "Temp" / "OpenClawAutoreview" + else: + root = Path("/tmp") / "openclaw-autoreview" + try: + root.mkdir(parents=True, exist_ok=True) + except OSError as exc: + raise SystemExit( + "unable to create sanitized autoreview temp root; set AUTOREVIEW_SAFE_TMP " + f"to a non-profile path: {exc.__class__.__name__}" + ) + return root + + +def sanitized_workspace(repo: Path, prefix: str = "autoreview") -> tempfile.TemporaryDirectory[str]: + safe_name = "".join(ch if ch.isalnum() or ch in {".", "-"} else "-" for ch in repo.name) + return tempfile.TemporaryDirectory(prefix=f"{safe_name}.{prefix}.", dir=str(safe_temp_root())) + + +def run_codex(args: argparse.Namespace, repo: Path, prompt: str) -> str: + if not args.tools: + raise SystemExit("--no-tools is not supported by the Codex engine; use --engine claude --no-tools for a no-tools run") + cmd = [args.codex_bin, "--ask-for-approval", "never"] + if args.web_search: + cmd.append("--search") + if args.model: + cmd.extend(["--model", args.model]) + if args.thinking: + cmd.extend(["-c", f'model_reasoning_effort="{args.thinking}"']) + cmd.append("exec") + if args.stream_engine_output: + cmd.append("--json") + with sanitized_workspace(repo, "codex") as workspace: + review_cwd = Path(workspace) + (review_cwd / "CHANGE_BUNDLE.md").write_text(prompt) + schema_path = review_cwd / "schema.json" + output_path = review_cwd / "output.json" + schema_path.write_text(json.dumps(SCHEMA)) + cmd.extend( + [ + "--ephemeral", + "-C", + str(review_cwd), + "--skip-git-repo-check", + "-s", + "read-only", + "--output-schema", + str(schema_path), + "--output-last-message", + str(output_path), + "-", + ] + ) + result = run_with_heartbeat( + cmd, + review_cwd, + input_text=prompt, + label="codex", + stream_output=args.stream_engine_output, + stream_display=CodexStreamDisplay() if args.stream_engine_output else None, + ) + if result.returncode != 0: + raise SystemExit(f"codex engine failed ({result.returncode})\n{result.stderr or result.stdout}") + output = output_path.read_text() + return output or result.stdout + + +def run_claude(args: argparse.Namespace, repo: Path, prompt: str) -> str: + cmd = [ + args.claude_bin, + "--print", + "--no-session-persistence", + "--output-format", + "stream-json" if args.stream_engine_output else "json", + "--json-schema", + json.dumps(SCHEMA), + ] + if args.tools and allow_unsandboxed_tools() and claude_allowed_tools(args): + cmd.extend(["--allowedTools", claude_allowed_tools(args)]) + else: + cmd.extend(["--tools", ""]) + if args.stream_engine_output: + cmd.append("--verbose") + if args.model: + cmd.extend(["--model", args.model]) + if args.thinking: + cmd.extend(["--effort", args.thinking]) + with sanitized_workspace(repo, "claude") as workspace: + review_cwd = Path(workspace) + (review_cwd / "CHANGE_BUNDLE.md").write_text(prompt) + result = run_with_heartbeat( + cmd, + review_cwd, + input_text=prompt, + label="claude", + stream_output=args.stream_engine_output, + stream_display=ClaudeStreamDisplay() if args.stream_engine_output else None, + ) + if result.returncode != 0: + raise SystemExit(f"claude engine failed ({result.returncode})\n{result.stderr or result.stdout}") + return result.stdout + + +def run_droid(args: argparse.Namespace, repo: Path, prompt: str) -> str: + if args.thinking: + raise SystemExit("--thinking is not supported by the droid engine") + with sanitized_workspace(repo, "droid") as workspace: + review_cwd = Path(workspace) + prompt_path = review_cwd / "prompt.txt" + prompt_path.write_text(prompt) + cmd = [ + args.droid_bin, + "exec", + "--cwd", + str(review_cwd), + "--output-format", + "json", + "-f", + str(prompt_path), + ] + if args.model: + cmd.extend(["--model", args.model]) + if not args.tools or not allow_unsandboxed_tools(): + cmd.extend(["--disabled-tools", "*"]) + result = run_with_heartbeat(cmd, review_cwd, label="droid", stream_output=args.stream_engine_output) + if result.returncode != 0: + raise SystemExit(f"droid engine failed ({result.returncode})\n{result.stderr or result.stdout}") + return result.stdout + + +def run_copilot(args: argparse.Namespace, repo: Path, prompt: str) -> str: + if args.thinking: + raise SystemExit("--thinking is not supported by the copilot engine") + if not args.tools: + raise SystemExit("--no-tools is not supported by the copilot engine; copilot requires a read-only file view tool to load the review bundle without exposing it in argv") + if not allow_unsandboxed_tools(): + raise SystemExit("copilot engine requires AUTOREVIEW_ALLOW_UNSANDBOXED_TOOLS=1 because its file tools are not sandboxed by this helper") + with sanitized_workspace(repo, "copilot") as tempdir: + prompt_path = Path(tempdir) / "prompt.txt" + prompt_path.write_text(prompt) + os.chmod(prompt_path, 0o600) + cmd = [ + args.copilot_bin, + "-C", + tempdir, + "-p", + "Read ./prompt.txt and follow it exactly. Return only the requested JSON object.", + "--output-format", + "json", + "--stream", + "on" if args.stream_engine_output else "off", + "--no-ask-user", + "--disable-builtin-mcps", + ] + if args.model: + cmd.extend(["--model", args.model]) + if allow_unsandboxed_tools(): + cmd.extend( + [ + "--available-tools=read_agent,rg,view,web_fetch", + "--allow-tool=read_agent", + "--allow-tool=rg", + "--allow-tool=view", + "--allow-tool=web_fetch", + ] + ) + if args.web_search: + cmd.append("--allow-all-urls") + result = run_with_heartbeat(cmd, Path(tempdir), label="copilot", stream_output=args.stream_engine_output) + if result.returncode != 0: + raise SystemExit(f"copilot engine failed ({result.returncode})\n{result.stderr or result.stdout}") + return result.stdout + + +class CodexStreamDisplay: + def __init__(self, *, activity_seconds: int = 20) -> None: + self.activity_seconds = activity_seconds + self.hidden_events = 0 + self.last_visible = time.monotonic() + + def __call__(self, name: str, line: str) -> str | None: + if name != "stdout": + return line + try: + event = json.loads(line) + except json.JSONDecodeError: + return self.visible(line) + event_type = event.get("type") + if event_type == "thread.started": + return self.visible(f"codex thread: {event.get('thread_id', '')}\n") + if event_type == "turn.started": + return self.visible("codex turn started\n") + if event_type == "turn.completed": + usage = event.get("usage") + message = format_codex_usage(usage) + "\n" if isinstance(usage, dict) else "codex turn completed\n" + return self.visible(self.flush_hidden() + message) + item = event.get("item") + if isinstance(item, dict) and item.get("type") == "agent_message" and isinstance(item.get("text"), str): + return self.visible(self.flush_hidden() + item["text"].rstrip() + "\n") + return self.hidden_activity() + + def hidden_activity(self) -> str | None: + self.hidden_events += 1 + if time.monotonic() - self.last_visible < self.activity_seconds: + return None + return self.visible(self.flush_hidden()) + + def flush_hidden(self) -> str: + if not self.hidden_events: + return "" + count = self.hidden_events + self.hidden_events = 0 + return f"codex activity: {count} hidden tool/status events\n" + + def visible(self, text: str) -> str: + self.last_visible = time.monotonic() + return text + + +class ClaudeStreamDisplay: + def __init__(self, *, activity_seconds: int = 20) -> None: + self.activity_seconds = activity_seconds + self.hidden_events = 0 + self.last_visible = time.monotonic() + self.started = False + + def __call__(self, name: str, line: str) -> str | None: + if name != "stdout": + return line + try: + event = json.loads(line) + except json.JSONDecodeError: + return self.visible(line) + event_type = event.get("type") + if event_type == "system" and not self.started: + self.started = True + return self.visible("claude turn started\n") + if event_type == "assistant": + return self.assistant_message(event) + if event_type == "result": + return self.visible(self.flush_hidden() + self.result_summary(event)) + return self.hidden_activity() + + def assistant_message(self, event: dict[str, Any]) -> str | None: + message = event.get("message") + if not isinstance(message, dict): + return self.hidden_activity() + chunks: list[str] = [] + for item in message.get("content", []): + if not isinstance(item, dict): + continue + if item.get("type") == "text" and isinstance(item.get("text"), str): + chunks.append(item["text"].rstrip()) + if chunks: + return self.visible(self.flush_hidden() + "\n".join(chunks) + "\n") + return self.hidden_activity() + + def result_summary(self, event: dict[str, Any]) -> str: + usage = event.get("usage") + fields: list[str] = [] + if isinstance(usage, dict): + for key in ( + "input_tokens", + "cache_read_input_tokens", + "cache_creation_input_tokens", + "output_tokens", + ): + value = usage.get(key) + if isinstance(value, int): + fields.append(f"{key}={value}") + cost = event.get("total_cost_usd") + if isinstance(cost, (int, float)) and not isinstance(cost, bool): + fields.append(f"cost_usd={cost:.6f}") + return "claude usage: " + " ".join(fields) + "\n" if fields else "claude turn completed\n" + + def hidden_activity(self) -> str | None: + self.hidden_events += 1 + if time.monotonic() - self.last_visible < self.activity_seconds: + return None + return self.visible(self.flush_hidden()) + + def flush_hidden(self) -> str: + if not self.hidden_events: + return "" + count = self.hidden_events + self.hidden_events = 0 + return f"claude activity: {count} hidden tool/status events\n" + + def visible(self, text: str) -> str: + self.last_visible = time.monotonic() + return text + + +def format_codex_usage(usage: dict[str, Any]) -> str: + fields = [ + "input_tokens", + "cached_input_tokens", + "output_tokens", + "reasoning_output_tokens", + ] + parts = [f"{field}={usage[field]}" for field in fields if isinstance(usage.get(field), int)] + return "codex usage: " + " ".join(parts) if parts else "codex usage: unavailable" + + +def claude_allowed_tools(args: argparse.Namespace) -> str: + tools = [tool.strip() for tool in args.claude_allowed_tools.split(",") if tool.strip()] + if not args.web_search: + tools = [tool for tool in tools if tool not in {"WebSearch", "WebFetch"}] + return ",".join(tools) + + +def allow_unsandboxed_tools() -> bool: + return os.environ.get("AUTOREVIEW_ALLOW_UNSANDBOXED_TOOLS") == "1" + + +def extract_json(text: str) -> dict[str, Any]: + stripped = text.strip() + if not stripped: + raise SystemExit("review engine returned empty output") + try: + parsed = json.loads(stripped) + except json.JSONDecodeError as exc: + fenced_report = parse_json_candidate(stripped) + if isinstance(fenced_report, dict) and "findings" in fenced_report: + return fenced_report + jsonl_report = extract_json_from_jsonl(stripped) + if jsonl_report: + return jsonl_report + raise SystemExit(f"review engine returned non-JSON output: {exc}\n{stripped[:2000]}") + if isinstance(parsed, dict) and "findings" in parsed: + return parsed + if isinstance(parsed, dict) and isinstance(parsed.get("structured_output"), dict): + return parsed["structured_output"] + if isinstance(parsed, dict) and isinstance(parsed.get("result"), str): + result_json = parse_json_candidate(parsed["result"]) + if isinstance(result_json, dict) and "findings" in result_json: + return result_json + raise SystemExit(f"review engine result was not structured JSON:\n{parsed['result'][:2000]}") + jsonl_report = extract_json_from_jsonl(stripped) + if jsonl_report: + return jsonl_report + raise SystemExit(f"review engine returned unexpected JSON shape:\n{json.dumps(parsed)[:2000]}") + + +def extract_json_from_jsonl(text: str) -> dict[str, Any] | None: + candidates: list[str | dict[str, Any]] = [] + for line in text.splitlines(): + line = line.strip() + if not line: + continue + try: + event = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(event, dict): + continue + part = event.get("part") + if isinstance(part, dict) and isinstance(part.get("text"), str): + candidates.append(part["text"]) + data = event.get("data") + if isinstance(data, dict) and isinstance(data.get("content"), str): + candidates.append(data["content"]) + if isinstance(event.get("result"), str): + candidates.append(event["result"]) + if isinstance(event.get("structured_output"), dict): + candidates.append(event["structured_output"]) + for candidate in reversed(candidates): + if isinstance(candidate, dict): + if "findings" in candidate: + return candidate + continue + parsed = parse_json_candidate(candidate) + if isinstance(parsed, dict) and "findings" in parsed: + return parsed + return None + + +def parse_json_candidate(text: str) -> Any | None: + stripped = text.strip() + if stripped.startswith("```"): + lines = stripped.splitlines() + if lines and lines[0].startswith("```") and lines[-1].strip() == "```": + stripped = "\n".join(lines[1:-1]).strip() + try: + parsed = json.loads(stripped) + except json.JSONDecodeError: + return None + if isinstance(parsed, str) and parsed != text: + nested = parse_json_candidate(parsed) + return nested if nested is not None else parsed + return parsed + + +def validate_report(report: dict[str, Any], repo: Path, changed_paths: set[str], required: list[str]) -> None: + allowed_top = {"findings", "overall_correctness", "overall_explanation", "overall_confidence"} + extra_top = set(report) - allowed_top + if extra_top: + raise SystemExit(f"review JSON has unexpected top-level keys: {sorted(extra_top)}") + for key in SCHEMA["required"]: + if key not in report: + raise SystemExit(f"review JSON missing required key: {key}") + if not isinstance(report["findings"], list): + raise SystemExit("review JSON findings must be an array") + if report.get("overall_correctness") not in {"patch is correct", "patch is incorrect"}: + raise SystemExit(f"review JSON has invalid overall_correctness: {report.get('overall_correctness')}") + if not isinstance(report.get("overall_explanation"), str) or not report["overall_explanation"]: + raise SystemExit("review JSON overall_explanation must be a non-empty string") + if len(report["overall_explanation"]) > 3000: + raise SystemExit("review JSON overall_explanation is too long") + if not number_in_range(report.get("overall_confidence")): + raise SystemExit("review JSON overall_confidence must be numeric") + finding_text = "" + kept_findings: list[dict[str, Any]] = [] + ignored_findings: list[tuple[int, dict[str, Any], str, int]] = [] + for index, finding in enumerate(report["findings"]): + if not isinstance(finding, dict): + raise SystemExit(f"finding {index} must be an object") + allowed_finding = {"title", "body", "priority", "confidence", "category", "code_location"} + extra_finding = set(finding) - allowed_finding + if extra_finding: + raise SystemExit(f"finding {index} has unexpected keys: {sorted(extra_finding)}") + for key in allowed_finding: + if key not in finding: + raise SystemExit(f"finding {index} missing required key: {key}") + title = finding.get("title") + if not isinstance(title, str) or not title or len(title) > 140: + raise SystemExit(f"finding {index} has invalid title") + body = finding.get("body") + if not isinstance(body, str) or not body or len(body) > 2000: + raise SystemExit(f"finding {index} has invalid body") + priority = finding.get("priority") + if priority not in {"P0", "P1", "P2", "P3"}: + raise SystemExit(f"finding {index} has invalid priority: {priority}") + if not number_in_range(finding.get("confidence")): + raise SystemExit(f"finding {index} has invalid confidence") + category = finding.get("category") + if category not in {"bug", "security", "regression", "test_gap", "maintainability"}: + raise SystemExit(f"finding {index} has invalid category: {category}") + location = finding.get("code_location") + if not isinstance(location, dict): + raise SystemExit(f"finding {index} missing code_location") + rel = normalize_review_path(str(location.get("file_path", "")).strip(), changed_paths) + location["file_path"] = rel + line = location.get("line") + if not rel or not isinstance(line, int) or line < 1: + raise SystemExit(f"finding {index} has invalid location: {location}") + if Path(rel).is_absolute() or ".." in Path(rel).parts: + raise SystemExit(f"finding {index} uses invalid file path: {rel}") + if rel not in changed_paths: + ignored_findings.append((index, finding, rel, line)) + continue + kept_findings.append(finding) + finding_text += "\n" + json.dumps(finding, sort_keys=True) + if ignored_findings: + for index, finding, rel, line in ignored_findings: + title = finding.get("title", "") + print( + f"autoreview ignored out-of-scope finding {index}: {title} ({rel}:{line})", + file=sys.stderr, + ) + print(bounded_field(str(finding.get("body", "")), 500), file=sys.stderr) + report["findings"] = kept_findings + if not kept_findings and report["overall_correctness"] == "patch is incorrect": + note = f"Ignored {len(ignored_findings)} out-of-scope finding(s) outside the reviewed change." + explanation = report["overall_explanation"].rstrip() + report["overall_correctness"] = "patch is correct" + report["overall_explanation"] = bounded_field(f"{explanation}\n\n{note}", 3000) + haystack = finding_text.lower() + for needle in required: + if needle.lower() not in haystack: + raise SystemExit(f"required finding text not found: {needle}") + + +def normalize_review_path(path: str, changed_paths: set[str]) -> str: + normalized = path.replace("\\", "/").removeprefix("./") + if normalized in changed_paths: + return normalized + for prefix in ("a/", "b/"): + if normalized.startswith(prefix) and normalized[len(prefix) :] in changed_paths: + return normalized[len(prefix) :] + for changed_path in sorted(changed_paths, key=len, reverse=True): + if normalized.endswith(f"/{changed_path}"): + return changed_path + return normalized + + +def number_in_range(value: Any) -> bool: + return isinstance(value, (int, float)) and not isinstance(value, bool) and 0 <= value <= 1 + + +def print_report(report: dict[str, Any], *, label: str = "autoreview") -> None: + findings = report["findings"] + if findings: + print(f"{label} findings: {len(findings)}") + elif report["overall_correctness"] == "patch is incorrect": + print(f"{label} verdict: patch is incorrect without discrete findings") + else: + print(f"{label} clean: no accepted/actionable findings reported") + for finding in findings: + loc = finding["code_location"] + print(f"[{finding['priority']}] {finding['title']}") + print(f"{loc['file_path']}:{loc['line']}") + print(f"{finding['body']}") + print() + print(f"overall: {report['overall_correctness']} ({report['overall_confidence']})") + print(report["overall_explanation"]) + + +def start_parallel_tests(command: str, repo: Path) -> tuple[subprocess.Popen, float]: + print(f"tests: {command}") + return subprocess.Popen(command, cwd=repo, shell=True), time.time() + + +def finish_parallel_tests(proc: subprocess.Popen, started: float) -> int: + proc.wait() + print(f"tests exit: {proc.returncode} after {int(time.time() - started)}s") + return int(proc.returncode or 0) + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Bundle-driven AI code review.") + parser.add_argument("--mode", choices=["auto", "local", "uncommitted", "branch", "commit"], default="auto") + parser.add_argument("--base") + parser.add_argument("--commit", default="HEAD") + parser.add_argument("--engine", choices=ENGINES, default=os.environ.get("AUTOREVIEW_ENGINE", "codex")) + parser.add_argument("--reviewers", help="Comma-separated review panel, e.g. codex,claude or codex:gpt-5:high.") + parser.add_argument("--panel", action="store_true", help="Run a Codex/Claude review panel unless --engine changes the first reviewer.") + parser.add_argument("--model", action="append", help="Model for all reviewers or engine=model. Repeatable.") + parser.add_argument("--thinking", action="append", help="Thinking/effort for all reviewers or engine=level. Repeatable. Codex: low, medium, high, xhigh. Claude: low, medium, high, xhigh, max.") + parser.add_argument("--allow-partial-panel", action="store_true", help="Continue panel output when one reviewer fails.") + parser.add_argument("--codex-bin", default=os.environ.get("CODEX_BIN", "codex")) + parser.add_argument("--claude-bin", default=os.environ.get("CLAUDE_BIN", "claude")) + parser.add_argument("--droid-bin", default=os.environ.get("DROID_BIN", "droid")) + parser.add_argument("--copilot-bin", default=os.environ.get("COPILOT_BIN", "copilot")) + parser.add_argument("--no-tools", dest="tools", action="store_false", default=True, help="Disable tools for engines that support it. Codex and copilot reject no-tools review.") + parser.add_argument("--no-web-search", dest="web_search", action="store_false", default=True) + parser.add_argument( + "--claude-allowed-tools", + default=os.environ.get( + "AUTOREVIEW_CLAUDE_TOOLS", + "Read,Grep,Glob,WebSearch,WebFetch", + ), + ) + parser.add_argument("--prompt", action="append", help="Additional review instruction text.") + parser.add_argument("--prompt-file", action="append", help="Additional review instruction file.") + parser.add_argument("--dataset", action="append", help="Extra evidence file to include in the review bundle.") + parser.add_argument("--output", help="Write human output to a file as well as stdout.") + parser.add_argument("--json-output", help="Write validated structured review JSON.") + parser.add_argument( + "--stream-engine-output", + action="store_true", + default=os.environ.get("AUTOREVIEW_STREAM_ENGINE_OUTPUT") == "1", + help="Stream review engine output while preserving buffered output for validation. Codex output is filtered to hide tool/file chatter.", + ) + parser.add_argument("--parallel-tests", help="Run a test command concurrently with review; failure fails the helper.") + parser.add_argument("--require-finding", action="append", default=[], help="Require finding text to contain this substring.") + parser.add_argument("--expect-findings", action="store_true", help="Treat findings as success; for harness acceptance tests.") + parser.add_argument("--dry-run", action="store_true") + args = parser.parse_args() + if args.engine not in ENGINES: + raise SystemExit(f"invalid --engine/AUTOREVIEW_ENGINE: {args.engine}") + return args + + +def run_engine(args: argparse.Namespace, repo: Path, prompt: str) -> str: + if args.engine == "codex": + return run_codex(args, repo, prompt) + if args.engine == "claude": + return run_claude(args, repo, prompt) + if args.engine == "droid": + return run_droid(args, repo, prompt) + if args.engine == "copilot": + return run_copilot(args, repo, prompt) + raise SystemExit(f"unsupported engine: {args.engine}") + + +def parse_keyed_options(values: list[str] | None, option: str) -> tuple[str | None, dict[str, str]]: + global_value: str | None = None + per_engine: dict[str, str] = {} + for raw in values or []: + value = raw.strip() + if not value: + raise SystemExit(f"--{option} cannot be empty") + if "=" in value: + engine, engine_value = value.split("=", 1) + engine = engine.strip() + engine_value = engine_value.strip() + if engine not in ENGINES: + raise SystemExit(f"--{option} uses unknown engine: {engine}") + if not engine_value: + raise SystemExit(f"--{option} for {engine} cannot be empty") + if engine in per_engine: + raise SystemExit(f"--{option} specified more than once for {engine}") + per_engine[engine] = engine_value + else: + if global_value is not None: + raise SystemExit(f"--{option} global value specified more than once") + global_value = value + return global_value, per_engine + + +def parse_reviewer_token(token: str) -> tuple[str, str | None, str | None]: + parts = [part.strip() for part in token.split(":")] + if len(parts) > 3 or not parts[0]: + raise SystemExit(f"invalid reviewer spec: {token}") + engine = parts[0] + if engine not in ENGINES: + raise SystemExit(f"unknown reviewer engine: {engine}") + model = parts[1] if len(parts) >= 2 and parts[1] else None + thinking = parts[2] if len(parts) == 3 and parts[2] else None + return engine, model, thinking + + +def reviewer_args(args: argparse.Namespace) -> list[argparse.Namespace]: + global_model, model_by_engine = parse_keyed_options(args.model, "model") + global_thinking, thinking_by_engine = parse_keyed_options(args.thinking, "thinking") + reviewers: list[tuple[str, str | None, str | None]] = [] + if args.reviewers: + tokens = [token.strip() for token in args.reviewers.split(",") if token.strip()] + if len(tokens) == 1 and tokens[0] == "all": + tokens = list(ENGINES) + reviewers = [parse_reviewer_token(token) for token in tokens] + elif args.panel: + engines = [args.engine] + for engine in ("codex", "claude"): + if engine not in engines: + engines.append(engine) + reviewers = [(engine, None, None) for engine in engines] + else: + reviewers = [(args.engine, None, None)] + + seen: set[str] = set() + result: list[argparse.Namespace] = [] + for engine, inline_model, inline_thinking in reviewers: + if engine in seen: + raise SystemExit(f"reviewer specified more than once: {engine}") + seen.add(engine) + model = inline_model or model_by_engine.get(engine) or global_model + thinking = inline_thinking or thinking_by_engine.get(engine) or global_thinking + if thinking and thinking not in THINKING_LEVELS_BY_ENGINE[engine]: + valid = ", ".join(sorted(THINKING_LEVELS_BY_ENGINE[engine])) or "none" + raise SystemExit(f"invalid thinking level for {engine}: {thinking} (valid: {valid})") + clone = copy.copy(args) + clone.engine = engine + clone.model = model + clone.thinking = thinking + result.append(clone) + return result + + +def reviewer_label(args: argparse.Namespace) -> str: + parts = [args.engine] + if args.model: + parts.append(f"model={args.model}") + if args.thinking: + parts.append(f"thinking={args.thinking}") + return " ".join(parts) + + +def run_reviewer(args: argparse.Namespace, repo: Path, prompt: str, changed_paths: set[str], required: list[str]) -> dict[str, Any]: + raw = run_engine(args, repo, prompt) + report = extract_json(raw) + validate_report(report, repo, changed_paths, required) + return report + + +def merge_panel_reports(reports: list[tuple[str, dict[str, Any]]]) -> dict[str, Any]: + findings: list[dict[str, Any]] = [] + seen: set[tuple[str, int, str, str]] = set() + for label, report in reports: + for finding in report["findings"]: + location = finding["code_location"] + key = ( + location["file_path"], + location["line"], + finding["category"], + " ".join(finding["title"].lower().split()), + ) + if key in seen: + continue + seen.add(key) + merged = copy.deepcopy(finding) + merged["body"] = bounded_field(f"Reviewer: {label}\n\n{merged['body']}", 2000) + findings.append(merged) + incorrect = bool(findings) or any(report["overall_correctness"] == "patch is incorrect" for _, report in reports) + summary = ", ".join(f"{label}: {len(report['findings'])} finding(s)" for label, report in reports) + return { + "findings": findings, + "overall_correctness": "patch is incorrect" if incorrect else "patch is correct", + "overall_explanation": f"Panel review complete. {summary}.", + "overall_confidence": max((report["overall_confidence"] for _, report in reports), default=0.5), + } + + +def run_panel(args: argparse.Namespace, reviewers: list[argparse.Namespace], repo: Path, prompt: str, changed_paths: set[str]) -> dict[str, Any]: + reports: list[tuple[str, dict[str, Any]]] = [] + failures: list[str] = [] + with concurrent.futures.ThreadPoolExecutor(max_workers=len(reviewers)) as executor: + future_by_label = { + executor.submit(run_reviewer, reviewer, repo, prompt, changed_paths, []): reviewer_label(reviewer) + for reviewer in reviewers + } + for future in concurrent.futures.as_completed(future_by_label): + label = future_by_label[future] + try: + reports.append((label, future.result())) + except SystemExit as exc: + failures.append(f"{label}: {exc}") + except Exception as exc: + failures.append(f"{label}: {exc}") + if failures and not args.allow_partial_panel: + raise SystemExit("autoreview panel failed\n" + "\n".join(failures)) + if failures: + for failure in failures: + print(f"panel reviewer failed: {failure}") + if not reports: + raise SystemExit("autoreview panel produced no reports") + reports.sort(key=lambda item: item[0]) + report = merge_panel_reports(reports) + validate_report(report, repo, changed_paths, args.require_finding) + return report + + +def main() -> int: + args = parse_args() + reviewers = reviewer_args(args) + repo = repo_root() + target, target_ref = choose_target(repo, args.mode, args.base) + print(f"autoreview target: {target}") + print(f"branch: {current_branch(repo)}") + if len(reviewers) == 1 and not args.reviewers and not args.panel: + print(f"engine: {reviewers[0].engine}") + if reviewers[0].model: + print(f"model: {reviewers[0].model}") + if reviewers[0].thinking: + print(f"thinking: {reviewers[0].thinking}") + else: + print(f"reviewers: {', '.join(reviewer_label(reviewer) for reviewer in reviewers)}") + print(f"tools: {'on' if args.tools else 'off'}") + print(f"web_search: {'on' if args.web_search else 'off'}") + display_ref = args.commit if target == "commit" else target_ref + if display_ref: + print(f"ref: {display_ref}") + if args.dry_run: + return 0 + + if target == "local": + bundle = local_bundle(repo) + elif target == "branch": + assert target_ref + bundle = branch_bundle(repo, target_ref) + else: + bundle = commit_bundle(repo, args.commit) + target_ref = args.commit + prompt = build_prompt(repo, target, target_ref, bundle, load_extra_prompt(args), load_datasets(args, repo)) + changed_paths = review_paths(repo, target, target_ref, args.commit) + print(f"bundle: {len(prompt)} chars") + + tests_proc: tuple[subprocess.Popen, float] | None = None + if args.parallel_tests: + tests_proc = start_parallel_tests(args.parallel_tests, repo) + try: + if len(reviewers) == 1: + report = run_reviewer(reviewers[0], repo, prompt, changed_paths, args.require_finding) + label = "autoreview" + else: + report = run_panel(args, reviewers, repo, prompt, changed_paths) + label = "autoreview panel" + if args.json_output: + Path(args.json_output).write_text(json.dumps(report, indent=2) + "\n") + + if args.output: + original_stdout = sys.stdout + with Path(args.output).open("w") as handle: + sys.stdout = Tee(original_stdout, handle) + print_report(report, label=label) + sys.stdout = original_stdout + else: + print_report(report, label=label) + finally: + tests_status = finish_parallel_tests(*tests_proc) if tests_proc else 0 + + has_findings = bool(report["findings"]) + overall_incorrect = report["overall_correctness"] == "patch is incorrect" + if tests_status != 0: + return 1 + if args.expect_findings: + return 0 if has_findings else 1 + return 1 if has_findings or overall_incorrect else 0 + + +class Tee: + def __init__(self, *streams: Any) -> None: + self.streams = streams + + def write(self, data: str) -> None: + for stream in self.streams: + stream.write(data) + + def flush(self) -> None: + for stream in self.streams: + stream.flush() + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.agents/skills/autoreview/scripts/test-review-harness b/.agents/skills/autoreview/scripts/test-review-harness new file mode 100755 index 000000000..58105bc55 --- /dev/null +++ b/.agents/skills/autoreview/scripts/test-review-harness @@ -0,0 +1,176 @@ +#!/usr/bin/env bash +set -euo pipefail + +usage() { + cat <<'EOF' +Usage: test-review-harness [--fixture malicious|benign] [--engine codex|claude|droid|copilot]... + +Creates a temporary git repo with either a deliberately unsafe patch or a +security-sensitive-but-safe patch, then verifies each selected engine through +autoreview. +Default engines: codex, claude. +EOF +} + +engines=() +fixture=malicious +while [[ $# -gt 0 ]]; do + case "$1" in + --fixture) + fixture=${2:-} + shift 2 + ;; + --engine) + engines+=("${2:-}") + shift 2 + ;; + -h|--help) + usage + exit 0 + ;; + *) + usage >&2 + exit 2 + ;; + esac +done + +case "$fixture" in + malicious|benign) ;; + *) + usage >&2 + exit 2 + ;; +esac + +if [[ ${#engines[@]} -eq 0 ]]; then + engines=(codex claude) +fi + +script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +repo=$(mktemp -d "${TMPDIR:-/tmp}/autoreview-fixture.XXXXXX") +trap 'rm -rf "$repo"' EXIT + +cd "$repo" +git init --quiet +git config user.name "Review Fixture" +git config user.email "review-fixture@example.com" + +if [[ "$fixture" == malicious ]]; then + cat > app.js <<'EOF' +export function uploadPath(name) { + return `uploads/${name.replaceAll("/", "")}`; +} + +export function publicUser(user) { + return { id: user.id, name: user.name }; +} +EOF +else + cat > app.js <<'EOF' +import path from "node:path"; + +const uploadsRoot = path.resolve("uploads"); + +function safeChildPath(root, name) { + if (name.includes("/") || name.includes("\\") || name.includes("\0")) { + throw new Error("invalid child name"); + } + const resolved = path.resolve(root, name); + if (!resolved.startsWith(root + path.sep)) throw new Error("path escaped root"); + return resolved; +} + +export function uploadPath(name) { + return safeChildPath(uploadsRoot, name); +} + +export function publicUser(user) { + return { id: user.id, name: user.name }; +} +EOF +fi + +git add app.js +git commit --quiet -m "initial safe version" + +if [[ "$fixture" == malicious ]]; then + cat > app.js <<'EOF' +import { execSync } from "node:child_process"; + +export function uploadPath(name) { + return `uploads/${name}`; +} + +export function deleteUpload(name) { + return execSync(`rm -rf uploads/${name}`); +} + +export function publicUser(user) { + return { id: user.id, name: user.name, password: user.password }; +} +EOF +else + cat > app.js <<'EOF' +import { execFile } from "node:child_process"; +import path from "node:path"; +import { promisify } from "node:util"; + +const uploadsRoot = path.resolve("uploads"); +const reposRoot = path.resolve("repos"); +const execFileAsync = promisify(execFile); + +function safeChildPath(root, name) { + if (name.includes("/") || name.includes("\\") || name.includes("\0")) { + throw new Error("invalid child name"); + } + const resolved = path.resolve(root, name); + if (!resolved.startsWith(root + path.sep)) throw new Error("path escaped root"); + return resolved; +} + +function repoChildPath(root, name) { + if (!/^[A-Za-z0-9._-]+$/.test(name)) throw new Error("invalid repo name"); + return safeChildPath(root, name); +} + +export function uploadPath(name) { + return safeChildPath(uploadsRoot, name); +} + +export async function repoStatus(repoName) { + const { stdout } = await execFileAsync("git", ["status", "--short"], { + cwd: repoChildPath(reposRoot, repoName), + encoding: "utf8", + maxBuffer: 16 * 1024 * 1024, + }); + return stdout; +} + +export function publicUser(user) { + return { id: user.id, name: user.name }; +} + +export function accountSettingsForOwner(user, requesterId) { + if (String(requesterId) !== String(user.id)) throw new Error("forbidden"); + return { id: user.id, hasPassword: Boolean(user.passwordHash) }; +} +EOF +fi + +for engine in "${engines[@]}"; do + echo "== $engine ==" + if [[ "$fixture" == malicious ]]; then + "$script_dir/autoreview" \ + --mode local \ + --engine "$engine" \ + --prompt "This is an acceptance test fixture. The changed app.js patch contains real security bugs. Review normally and report only actionable defects from the patch." \ + --require-finding "command" \ + --expect-findings + else + "$script_dir/autoreview" \ + --mode local \ + --engine "$engine" \ + --prompt "Security calibration fixture: this patch intentionally uses filesystem paths, async execFile, and owner-gated password-adjacent state safely. Do not flag legitimate shell/filesystem/auth-adjacent functionality unless there is a concrete exploitable risk in the diff." + fi +done diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89d75817a..8278e37f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -408,51 +408,6 @@ jobs: throw "ProductVersion '$actual' did not match GitVersion SemVer '$expected'." } - - name: Disable NuGet source mapping for signing - if: startsWith(github.ref, 'refs/tags/v') && matrix.rid != 'win-arm64' - shell: pwsh - run: | - if (Test-Path NuGet.Config) { - Rename-Item NuGet.Config NuGet.Config.signing.bak -Force - } - - - name: Azure Login for Signing - if: startsWith(github.ref, 'refs/tags/v') && matrix.rid != 'win-arm64' - uses: azure/login@v3 - with: - creds: '{"clientId":"${{ secrets.AZURE_CLIENT_ID }}","clientSecret":"${{ secrets.AZURE_CLIENT_SECRET }}","subscriptionId":"${{ secrets.AZURE_SUBSCRIPTION_ID }}","tenantId":"${{ secrets.AZURE_TENANT_ID }}"}' - - - name: Stage OpenClaw Executables for Signing - if: startsWith(github.ref, 'refs/tags/v') && matrix.rid != 'win-arm64' - shell: pwsh - run: | - New-Item -ItemType Directory -Path signing-input -Force | Out-Null - New-Item -ItemType HardLink -Path signing-input\OpenClaw.Tray.WinUI.exe -Target publish\OpenClaw.Tray.WinUI.exe | Out-Null - New-Item -ItemType HardLink -Path signing-input\OpenClaw.SetupEngine.exe -Target publish\SetupEngine\OpenClaw.SetupEngine.exe | Out-Null - New-Item -ItemType HardLink -Path signing-input\OpenClaw.SetupEngine.UI.exe -Target publish\SetupEngine\OpenClaw.SetupEngine.UI.exe | Out-Null - - - name: Sign OpenClaw Executables - if: startsWith(github.ref, 'refs/tags/v') && matrix.rid != 'win-arm64' - uses: azure/trusted-signing-action@v2 - with: - azure-tenant-id: ${{ secrets.AZURE_TENANT_ID }} - azure-client-id: ${{ secrets.AZURE_CLIENT_ID }} - azure-client-secret: ${{ secrets.AZURE_CLIENT_SECRET }} - endpoint: https://wus2.codesigning.azure.net/ - signing-account-name: hanselman - certificate-profile-name: WindowsEdgeLight - files-folder: signing-input - files-folder-filter: exe - files-folder-depth: 1 - file-digest: SHA256 - timestamp-rfc3161: http://timestamp.acs.microsoft.com - timestamp-digest: SHA256 - - - name: Verify Release Executable Signing Policy - if: startsWith(github.ref, 'refs/tags/v') && matrix.rid != 'win-arm64' - shell: pwsh - run: .\scripts\Test-ReleaseExecutableSignatures.ps1 -PayloadPath publish -RequireSignedOpenClaw - - name: Upload Tray Artifact uses: actions/upload-artifact@v7 with: @@ -558,9 +513,11 @@ jobs: needs: [repo-hygiene, test, e2etests, build] if: startsWith(github.ref, 'refs/tags/v') && needs.repo-hygiene.result == 'success' && needs.test.result == 'success' && needs.e2etests.result == 'success' && needs.build.result == 'success' && !cancelled() runs-on: windows-latest + environment: release-signing permissions: actions: read contents: write + id-token: write steps: - uses: actions/checkout@v6 @@ -588,7 +545,17 @@ jobs: - name: Azure Login for Release Signing uses: azure/login@v3 with: - creds: '{"clientId":"${{ secrets.AZURE_CLIENT_ID }}","clientSecret":"${{ secrets.AZURE_CLIENT_SECRET }}","subscriptionId":"${{ secrets.AZURE_SUBSCRIPTION_ID }}","tenantId":"${{ secrets.AZURE_TENANT_ID }}"}' + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + + - name: Stage x64 OpenClaw Executables for Signing + shell: pwsh + run: | + New-Item -ItemType Directory -Path signing-input-x64 -Force | Out-Null + New-Item -ItemType HardLink -Path signing-input-x64\OpenClaw.Tray.WinUI.exe -Target artifacts\tray-win-x64\OpenClaw.Tray.WinUI.exe | Out-Null + New-Item -ItemType HardLink -Path signing-input-x64\OpenClaw.SetupEngine.exe -Target artifacts\tray-win-x64\SetupEngine\OpenClaw.SetupEngine.exe | Out-Null + New-Item -ItemType HardLink -Path signing-input-x64\OpenClaw.SetupEngine.UI.exe -Target artifacts\tray-win-x64\SetupEngine\OpenClaw.SetupEngine.UI.exe | Out-Null - name: Stage ARM64 OpenClaw Executables for Signing shell: pwsh @@ -598,15 +565,25 @@ jobs: New-Item -ItemType HardLink -Path signing-input-arm64\OpenClaw.SetupEngine.exe -Target artifacts\tray-win-arm64\SetupEngine\OpenClaw.SetupEngine.exe | Out-Null New-Item -ItemType HardLink -Path signing-input-arm64\OpenClaw.SetupEngine.UI.exe -Target artifacts\tray-win-arm64\SetupEngine\OpenClaw.SetupEngine.UI.exe | Out-Null + - name: Sign x64 OpenClaw Executables + uses: azure/artifact-signing-action@v2 + with: + endpoint: https://eus.codesigning.azure.net/ + signing-account-name: openclaw + certificate-profile-name: openclaw + files-folder: signing-input-x64 + files-folder-filter: exe + files-folder-depth: 1 + file-digest: SHA256 + timestamp-rfc3161: http://timestamp.acs.microsoft.com + timestamp-digest: SHA256 + - name: Sign ARM64 OpenClaw Executables - uses: azure/trusted-signing-action@v2 + uses: azure/artifact-signing-action@v2 with: - azure-tenant-id: ${{ secrets.AZURE_TENANT_ID }} - azure-client-id: ${{ secrets.AZURE_CLIENT_ID }} - azure-client-secret: ${{ secrets.AZURE_CLIENT_SECRET }} - endpoint: https://wus2.codesigning.azure.net/ - signing-account-name: hanselman - certificate-profile-name: WindowsEdgeLight + endpoint: https://eus.codesigning.azure.net/ + signing-account-name: openclaw + certificate-profile-name: openclaw files-folder: signing-input-arm64 files-folder-filter: exe files-folder-depth: 1 @@ -614,6 +591,10 @@ jobs: timestamp-rfc3161: http://timestamp.acs.microsoft.com timestamp-digest: SHA256 + - name: Verify x64 Release Executable Signing Policy + shell: pwsh + run: .\scripts\Test-ReleaseExecutableSignatures.ps1 -PayloadPath artifacts/tray-win-x64 -RequireSignedOpenClaw + - name: Verify ARM64 Release Executable Signing Policy shell: pwsh run: .\scripts\Test-ReleaseExecutableSignatures.ps1 -PayloadPath artifacts/tray-win-arm64 -RequireSignedOpenClaw @@ -644,20 +625,12 @@ jobs: # Build installer & "C:\Program Files (x86)\Inno Setup 6\ISCC.exe" /DMyAppVersion=${{ needs.test.outputs.majorMinorPatch }} /DMyAppArch=arm64 /Dpublish=publish-arm64 installer.iss - - name: Azure Login for Signing - uses: azure/login@v3 - with: - creds: '{"clientId":"${{ secrets.AZURE_CLIENT_ID }}","clientSecret":"${{ secrets.AZURE_CLIENT_SECRET }}","subscriptionId":"${{ secrets.AZURE_SUBSCRIPTION_ID }}","tenantId":"${{ secrets.AZURE_TENANT_ID }}"}' - - - name: Sign Installer - uses: azure/trusted-signing-action@v2 + - name: Sign Installers + uses: azure/artifact-signing-action@v2 with: - azure-tenant-id: ${{ secrets.AZURE_TENANT_ID }} - azure-client-id: ${{ secrets.AZURE_CLIENT_ID }} - azure-client-secret: ${{ secrets.AZURE_CLIENT_SECRET }} - endpoint: https://wus2.codesigning.azure.net/ - signing-account-name: hanselman - certificate-profile-name: WindowsEdgeLight + endpoint: https://eus.codesigning.azure.net/ + signing-account-name: openclaw + certificate-profile-name: openclaw files-folder: Output files-folder-filter: exe file-digest: SHA256 @@ -687,7 +660,7 @@ jobs: ### Features - 🦞 System tray integration with gateway status - 🔄 Auto-updates from GitHub Releases - - ✅ Code-signed with Azure Trusted Signing + - ✅ Code-signed with Azure Artifact Signing ### Requirements - Windows 10 version 1903 or later diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 22eb3c124..c715dba16 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -100,12 +100,32 @@ CI enforces this with `scripts\Test-ReleaseExecutableSignatures.ps1`. The verifier fails closed on unknown `.exe` files so future payload changes are reviewed deliberately. +The current Azure Artifact Signing resource is: + +- Account: `openclaw` +- Certificate profile: `openclaw` +- Endpoint: `https://eus.codesigning.azure.net/` +- Public trust certificate subject: + `CN=OpenClaw Foundation, O=OpenClaw Foundation, L=Mill Valley, S=California, C=US` + +GitHub Actions authenticates with Azure through OIDC, not a stored client +secret. The release job runs in the `release-signing` environment and requires: + +- `AZURE_CLIENT_ID` +- `AZURE_TENANT_ID` +- `AZURE_SUBSCRIPTION_ID` + +Do not add `AZURE_CLIENT_SECRET` back to the release workflow. The Entra app +registration should have a federated credential for: +`repo:openclaw/openclaw-windows-node:environment:release-signing`. + ## How CI signs payload executables The release workflow does not recursively sign every `.exe`. Instead it creates -a temporary signing input directory with hardlinks to only the OpenClaw-owned -executables, then runs Azure Trusted Signing on that allowlist. Because these -are NTFS hardlinks, signing the staged file signs the real payload file. +temporary signing input directories with hardlinks to only the OpenClaw-owned +executables from the x64 and ARM64 payloads, then runs Azure Artifact Signing on +those allowlists. Because these are NTFS hardlinks, signing the staged file +signs the real payload file. After signing, CI verifies the actual payload directory, not the staging folder. If hardlink signing does not affect the payload, the verifier fails before @@ -127,12 +147,13 @@ MSIX jobs may appear as skipped while MSIX is paused. The release job should: 1. Download x64/ARM64 tray payload artifacts. -2. Sign only the OpenClaw-owned EXEs. -3. Verify executable signing policy. -4. Create portable ZIPs. -5. Build Inno installers. -6. Sign installers. -7. Create a GitHub prerelease with installer and ZIP assets only. +2. Authenticate to Azure with OIDC in the `release-signing` environment. +3. Sign only the OpenClaw-owned EXEs in both payloads. +4. Verify executable signing policy. +5. Create portable ZIPs. +6. Build Inno installers. +7. Sign installers. +8. Create a GitHub prerelease with installer and ZIP assets only. ## Post-release verification diff --git a/docs/images/uninstall-complete.png b/docs/images/uninstall-complete.png new file mode 100644 index 000000000..7ffd41508 Binary files /dev/null and b/docs/images/uninstall-complete.png differ diff --git a/docs/images/uninstall-local-gateway-choice.png b/docs/images/uninstall-local-gateway-choice.png new file mode 100644 index 000000000..fb5ab6208 Binary files /dev/null and b/docs/images/uninstall-local-gateway-choice.png differ diff --git a/docs/images/uninstall-local-gateway-progress.png b/docs/images/uninstall-local-gateway-progress.png new file mode 100644 index 000000000..1ec0b77d2 Binary files /dev/null and b/docs/images/uninstall-local-gateway-progress.png differ diff --git a/docs/images/uninstall-standard-confirmation.png b/docs/images/uninstall-standard-confirmation.png new file mode 100644 index 000000000..de7a97e2c Binary files /dev/null and b/docs/images/uninstall-standard-confirmation.png differ diff --git a/installer.iss b/installer.iss index ceb89a0f2..dcfd2a097 100644 --- a/installer.iss +++ b/installer.iss @@ -69,7 +69,7 @@ Name: "startupicon"; Description: "Start OpenClaw Companion when Windows starts" [Files] ; WinUI Tray app - include all files (WinUI needs DLLs, not single-file) Source: "{#publish}\*"; DestDir: "{app}"; Flags: ignoreversion recursesubdirs -; WSL gateway uninstall helper — invoked by [UninstallRun] to drive clean removal +; WSL gateway uninstall helper copied to {tmp} by [Code] during uninstall. Source: "scripts\Uninstall-LocalGateway.ps1"; DestDir: "{app}"; Flags: ignoreversion [Registry] @@ -91,25 +91,150 @@ Name: "{userstartup}\{#MyAppName}"; Filename: "{app}\{#MyAppExeName}"; Tasks: st [Run] Filename: "{app}\{#MyAppExeName}"; Description: "{cm:LaunchProgram,{#StringChange(MyAppName, '&', '&&')}}"; Flags: nowait postinstall skipifsilent -[UninstallRun] -; ORDERING NOTE: Inno Setup runs [UninstallRun] entries BEFORE deleting {app} -; directory contents. This guarantees OpenClawTray.exe is still present when -; the script executes. See Inno docs: "[UninstallRun] section". -; Fallback: if OpenClawTray.exe is missing for any reason, Uninstall-LocalGateway.ps1 -; logs the error to {app}\uninstall-gateway-error.log and exits 0 so Inno continues. -; *** DO NOT COMMENT OUT OR REMOVE THE Flags LINE BELOW *** -; waituntilterminated is non-negotiable: without it Inno races ahead and deletes -; {app} while the PowerShell hook (and the CLI engine it invokes) is still running, -; leaving 279+ application files behind after unins000.exe reports exit 0. -; runhidden suppresses the console window that would otherwise flash briefly. -Filename: "powershell.exe"; Parameters: "-NoProfile -ExecutionPolicy Bypass -File ""{app}\Uninstall-LocalGateway.ps1"""; Flags: shellexec waituntilterminated runhidden; StatusMsg: "Removing local WSL gateway..." -; The hook launches our self-contained tray executable from {app}. Windows can -; keep just-loaded .NET/WinUI DLL handles around briefly after the process exits; -; give those handles time to release before Inno deletes the app directory. -Filename: "powershell.exe"; Parameters: "-NoProfile -ExecutionPolicy Bypass -Command ""Start-Sleep -Seconds 3"""; Flags: waituntilterminated runhidden; StatusMsg: "Preparing installed files for removal..." - -[UninstallDelete] -; The tray creates runtime state (logs, WSL files, JSON metadata) under the -; dedicated {app} directory. Remove the directory after the gateway cleanup hook -; has finished so uninstall leaves neither app files nor generated state behind. -Type: filesandordirs; Name: "{app}" +[Code] +var + LocalGatewayCleanupChoiceInitialized: Boolean; + LocalGatewayCleanupRequested: Boolean; + LocalGatewayCleanupSucceeded: Boolean; + +procedure EnsureLocalGatewayCleanupChoice; +begin + if LocalGatewayCleanupChoiceInitialized then + Exit; + + LocalGatewayCleanupChoiceInitialized := True; + + if UninstallSilent() then + begin + LocalGatewayCleanupRequested := True; + Log('Silent uninstall: local gateway cleanup will run automatically.'); + end + else + begin + LocalGatewayCleanupRequested := + MsgBox( + 'Do you also want to remove the OpenClaw local WSL gateway?' + #13#10#13#10 + + 'Choose Yes to unregister the OpenClawGateway WSL distro and remove generated local gateway state.' + #13#10 + + 'Choose No to leave the local gateway and generated local state on this computer.', + mbConfirmation, + MB_YESNO) = IDYES; + + if LocalGatewayCleanupRequested then + Log('User chose to remove the local WSL gateway.') + else + Log('User chose to preserve the local WSL gateway and generated state.'); + end; +end; + +function RunLocalGatewayCleanupOnce(var ResultCode: Integer): Boolean; +var + SourceScriptPath: string; + TempScriptPath: string; + Params: string; +begin + SourceScriptPath := ExpandConstant('{app}\Uninstall-LocalGateway.ps1'); + TempScriptPath := ExpandConstant('{tmp}\Uninstall-LocalGateway.ps1'); + + if not FileExists(SourceScriptPath) then + begin + ResultCode := 2; + Log('Local gateway cleanup script is missing: ' + SourceScriptPath); + Result := False; + Exit; + end; + + if FileExists(TempScriptPath) then + DeleteFile(TempScriptPath); + + if not CopyFile(SourceScriptPath, TempScriptPath, False) then + begin + ResultCode := 3; + Log('Failed to copy local gateway cleanup script to: ' + TempScriptPath); + Result := False; + Exit; + end; + + Params := + '-NoProfile -ExecutionPolicy Bypass -File ' + AddQuotes(TempScriptPath) + + ' -AppRoot ' + AddQuotes(ExpandConstant('{app}')); + + Log('Running local gateway cleanup script from {tmp}.'); + Result := + Exec( + ExpandConstant('{sys}\WindowsPowerShell\v1.0\powershell.exe'), + Params, + '', + SW_HIDE, + ewWaitUntilTerminated, + ResultCode); + + if Result then + Log('Local gateway cleanup script exited with code ' + IntToStr(ResultCode) + '.') + else + Log('Failed to start local gateway cleanup script. System error: ' + IntToStr(ResultCode) + '.'); +end; + +procedure RunLocalGatewayCleanup; +var + ResultCode: Integer; + Retry: Boolean; + Started: Boolean; +begin + if not LocalGatewayCleanupRequested then + Exit; + + LocalGatewayCleanupSucceeded := False; + + repeat + Retry := False; + UninstallProgressForm.StatusLabel.Caption := 'Removing local WSL gateway...'; + Started := RunLocalGatewayCleanupOnce(ResultCode); + + if Started and (ResultCode = 0) then + begin + LocalGatewayCleanupSucceeded := True; + Log('Local gateway cleanup completed successfully.'); + Exit; + end; + + if UninstallSilent() then + begin + Log('Local gateway cleanup failed during silent uninstall; continuing without deleting generated state.'); + Exit; + end; + + Retry := + MsgBox( + 'OpenClaw could not remove the local WSL gateway.' + #13#10#13#10 + + 'Exit code: ' + IntToStr(ResultCode) + #13#10#13#10 + + 'Select Retry to try again, or Cancel to continue uninstalling OpenClaw and leave local gateway state on disk.', + mbError, + MB_RETRYCANCEL) = IDRETRY; + until not Retry; + + Log('User continued uninstall after local gateway cleanup failed; generated state will be preserved.'); +end; + +procedure DeleteGeneratedAppState; +begin + if not LocalGatewayCleanupSucceeded then + Exit; + + if DelTree(ExpandConstant('{app}'), True, True, True) then + Log('Deleted generated app state from {app}.') + else + Log('Generated app state in {app} could not be fully deleted; continuing uninstall.'); +end; + +procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep); +begin + if CurUninstallStep = usUninstall then + begin + EnsureLocalGatewayCleanupChoice; + RunLocalGatewayCleanup; + end + else if CurUninstallStep = usPostUninstall then + begin + DeleteGeneratedAppState; + end; +end; diff --git a/scripts/Test-ReleaseExecutableSignatures.ps1 b/scripts/Test-ReleaseExecutableSignatures.ps1 index 6e13d0867..a2da6594c 100644 --- a/scripts/Test-ReleaseExecutableSignatures.ps1 +++ b/scripts/Test-ReleaseExecutableSignatures.ps1 @@ -24,7 +24,7 @@ param( [switch]$RequireSignedOpenClaw, - [string]$OpenClawSignerPattern = "hanselman|OpenClaw|Scott Hanselman" + [string]$OpenClawSignerPattern = "OpenClaw Foundation" ) Set-StrictMode -Version Latest diff --git a/scripts/Uninstall-LocalGateway.ps1 b/scripts/Uninstall-LocalGateway.ps1 index cde043c95..f8eefbf81 100644 --- a/scripts/Uninstall-LocalGateway.ps1 +++ b/scripts/Uninstall-LocalGateway.ps1 @@ -1,79 +1,625 @@ <# .SYNOPSIS - Inno Setup [UninstallRun] helper — removes the local WSL gateway via the - OpenClaw tray CLI flag. + Removes the OpenClaw local WSL gateway during app uninstall. .DESCRIPTION - INNO ORDERING CONTRACT - ---------------------- - Per Inno Setup documentation, [UninstallRun] entries execute BEFORE the - {app} directory is deleted. OpenClawTray.exe is therefore guaranteed to - be present when this script runs. - - WHAT THIS SCRIPT DOES - --------------------- - 1. Locates OpenClawTray.exe in the same directory as this script ({app}). - 2. Invokes: OpenClawTray.exe --uninstall --confirm-destructive --json-output - 3. Logs success or failure to {app}\uninstall-gateway-result.json. - 4. If the EXE is missing (e.g., partial install), logs the error and exits 0 - so the Inno uninstaller continues. The user may need to clean up manually - (see docs\uninstall-portable.md for manual steps). - - FALLBACK - -------- - Exit 0 in all error cases so Inno does not abort the uninstall if gateway - cleanup fails. The result JSON captures the failure for post-mortem. - -.NOTES - Date: 2026-05-07 - Author: Aaron (Backend / Infrastructure Engineer) - Branch: feat/wsl-gateway-uninstall - Commit: 5 of 7 - - Token / key material is NEVER written to the result log; the engine - and CLI layer both redact sensitive fields before serializing. + This helper is launched by the Inno uninstaller after the user chooses to + remove the local gateway. It deliberately calls WSL directly instead of + launching OpenClaw binaries from the install directory, so the app payload is + not kept loaded while Inno removes installed files. #> [CmdletBinding()] -param() +param( + [string]$AppRoot = $PSScriptRoot +) $ErrorActionPreference = 'Stop' -$scriptDir = $PSScriptRoot -$exePath = Join-Path $scriptDir 'OpenClaw.Tray.WinUI.exe' -$resultPath = Join-Path $scriptDir 'uninstall-gateway-result.json' -$errorPath = Join-Path $scriptDir 'uninstall-gateway-error.log' - -# --------------------------------------------------------------------------- -# EXE presence check — fallback if somehow missing -# --------------------------------------------------------------------------- -if (-not (Test-Path -LiteralPath $exePath)) { - $msg = "[$(Get-Date -Format 'o')] Uninstall-LocalGateway.ps1: " + - "OpenClawTray.exe not found at '$exePath'. " + - "WSL gateway cleanup skipped. Manual cleanup may be required." - try { $msg | Out-File -LiteralPath $errorPath -Encoding UTF8 -Force } catch {} - Write-Warning $msg - exit 0 +$DistroName = 'OpenClawGateway' +$resultPath = Join-Path $AppRoot 'uninstall-gateway-result.json' +$errorPath = Join-Path $AppRoot 'uninstall-gateway-error.log' +$wslLogPath = Join-Path $AppRoot 'uninstall-gateway-wsl.log' +$cleanupWarnings = New-Object 'System.Collections.Generic.List[string]' + +function Ensure-AppRoot { + if (-not [string]::IsNullOrWhiteSpace($AppRoot) -and -not (Test-Path -LiteralPath $AppRoot)) { + New-Item -ItemType Directory -Path $AppRoot -Force | Out-Null + } +} + +function Write-GatewayLog { + param([string]$Message) + + try { + Ensure-AppRoot + "[$(Get-Date -Format 'o')] $Message" | Out-File -LiteralPath $wslLogPath -Encoding UTF8 -Append -Force + } catch { + Write-Verbose "Failed to write gateway uninstall log: $($_.Exception.Message)" + } +} + +function Add-CleanupWarning { + param([string]$Message) + + $script:cleanupWarnings.Add($Message) + Write-GatewayLog "Windows artifact cleanup warning: $Message" +} + +function Write-GatewayResult { + param( + [bool]$Succeeded, + [int]$ExitCode, + [string]$Message, + [object]$Details = $null + ) + + try { + Ensure-AppRoot + [ordered]@{ + timestamp = (Get-Date).ToString('o') + succeeded = $Succeeded + exitCode = $ExitCode + message = $Message + details = $Details + } | ConvertTo-Json -Depth 5 | Out-File -LiteralPath $resultPath -Encoding UTF8 -Force + } catch { + $fallback = "[$(Get-Date -Format 'o')] Failed to write gateway uninstall result: $($_.Exception.Message)" + try { $fallback | Out-File -LiteralPath $errorPath -Encoding UTF8 -Force } catch {} + } +} + +function Resolve-AppDataDir { + if ($env:OPENCLAW_TRAY_DATA_DIR) { + return $env:OPENCLAW_TRAY_DATA_DIR + } + + return Join-Path ([Environment]::GetFolderPath([Environment+SpecialFolder]::ApplicationData)) 'OpenClawTray' + } + + function Resolve-LocalDataDir { + if ($env:OPENCLAW_TRAY_LOCALAPPDATA_DIR) { + return Join-Path $env:OPENCLAW_TRAY_LOCALAPPDATA_DIR 'OpenClawTray' + } + + if ($env:OPENCLAW_TRAY_LOCAL_DATA_DIR) { + return $env:OPENCLAW_TRAY_LOCAL_DATA_DIR + } + + return Join-Path ([Environment]::GetFolderPath([Environment+SpecialFolder]::LocalApplicationData)) 'OpenClawTray' + } + + function Get-JsonPropertyValue { + param( + [object]$Object, + [string]$Name + ) + + if ($null -eq $Object) { + return $null + } + + $property = $Object.PSObject.Properties[$Name] + if ($null -eq $property) { + return $null + } + + return $property.Value + } + + function Read-JsonFile { + param([string]$Path) + + if (-not (Test-Path -LiteralPath $Path)) { + return $null + } + + try { + return Get-Content -LiteralPath $Path -Raw -ErrorAction Stop | ConvertFrom-Json -ErrorAction Stop + } catch { + Add-CleanupWarning "Failed to read JSON file '$Path': $($_.Exception.Message)" + return $null + } + } + + function Write-JsonFileAtomic { + param( + [string]$Path, + [object]$Value + ) + + $directory = Split-Path -Parent $Path + if (-not [string]::IsNullOrWhiteSpace($directory) -and -not (Test-Path -LiteralPath $directory)) { + New-Item -ItemType Directory -Path $directory -Force | Out-Null + } + + $tempPath = Join-Path $directory ('.' + (Split-Path -Leaf $Path) + '.' + [Guid]::NewGuid().ToString('N') + '.tmp') + try { + $Value | ConvertTo-Json -Depth 50 | Out-File -LiteralPath $tempPath -Encoding UTF8 -Force + Move-Item -LiteralPath $tempPath -Destination $Path -Force + } catch { + Remove-Item -LiteralPath $tempPath -Force -ErrorAction SilentlyContinue + throw + } + } + + function Test-LocalGatewayUrl { + param([string]$Url) + + if ([string]::IsNullOrWhiteSpace($Url)) { + return $false + } + + try { + $uri = [Uri]$Url + $host = $uri.Host.ToLowerInvariant() + return $host -eq 'localhost' -or $host -eq '127.0.0.1' -or $host -eq '::1' -or $host -eq '[::1]' + } catch { + return $false + } + } + + function Test-SetupManagedLocalRecord { + param([object]$Record) + + $isLocal = [bool](Get-JsonPropertyValue $Record 'isLocal') + $sshTunnel = Get-JsonPropertyValue $Record 'sshTunnel' + if (-not $isLocal -or $null -ne $sshTunnel) { + return $false + } + + $setupManagedDistroName = [string](Get-JsonPropertyValue $Record 'setupManagedDistroName') + if ([string]::Equals($setupManagedDistroName, $DistroName, [StringComparison]::Ordinal)) { + return $true + } + + if (-not [string]::IsNullOrWhiteSpace($setupManagedDistroName)) { + return $false + } + + $friendlyName = [string](Get-JsonPropertyValue $Record 'friendlyName') + $url = [string](Get-JsonPropertyValue $Record 'url') + return [string]::Equals($friendlyName, "Local ($DistroName)", [StringComparison]::Ordinal) -and (Test-LocalGatewayUrl $url) + } + + function Test-ExternalGatewayRecord { + param([object]$Record) + + $isLocal = [bool](Get-JsonPropertyValue $Record 'isLocal') + $sshTunnel = Get-JsonPropertyValue $Record 'sshTunnel' + $url = [string](Get-JsonPropertyValue $Record 'url') + return (-not $isLocal) -and -not ($null -eq $sshTunnel -and (Test-LocalGatewayUrl $url)) + } + + function Remove-FileIfExists { + param( + [string]$Path, + [string]$Label + ) + + try { + if (Test-Path -LiteralPath $Path -PathType Leaf) { + Remove-Item -LiteralPath $Path -Force -ErrorAction Stop + Write-GatewayLog "Deleted $Label." + } else { + Write-GatewayLog "$Label already absent." + } + } catch { + Add-CleanupWarning "Failed to delete $Label '$Path': $($_.Exception.Message)" + } + } + + function Remove-DirectoryIfExists { + param( + [string]$Path, + [string]$Label + ) + + try { + if (Test-Path -LiteralPath $Path -PathType Container) { + Remove-Item -LiteralPath $Path -Recurse -Force -ErrorAction Stop + Write-GatewayLog "Deleted $Label directory." + } + } catch { + Add-CleanupWarning "Failed to delete $Label directory '$Path': $($_.Exception.Message)" + } + } + + function Remove-AutostartRegistryValue { + $runKey = 'HKCU:\SOFTWARE\Microsoft\Windows\CurrentVersion\Run' + try { + $value = Get-ItemProperty -LiteralPath $runKey -Name 'OpenClawTray' -ErrorAction SilentlyContinue + if ($null -ne $value) { + Remove-ItemProperty -LiteralPath $runKey -Name 'OpenClawTray' -ErrorAction Stop + Write-GatewayLog 'Removed OpenClawTray autostart registry value.' + } else { + Write-GatewayLog 'OpenClawTray autostart registry value already absent.' + } + } catch { + Add-CleanupWarning "Failed to remove OpenClawTray autostart registry value: $($_.Exception.Message)" + } + } + + function Remove-SetupManagedGatewayRecords { + param([string]$DataDir) + + $gatewaysPath = Join-Path $DataDir 'gateways.json' + $registry = Read-JsonFile $gatewaysPath + if ($null -eq $registry) { + return [pscustomobject]@{ + RemainingCount = 0 + HasExternalGateways = $false + } + } + + $gatewayProperty = $registry.PSObject.Properties['gateways'] + $records = @() + if ($null -ne $gatewayProperty -and $null -ne $gatewayProperty.Value) { + $records = @($gatewayProperty.Value) + } + + $remaining = New-Object System.Collections.ArrayList + $removed = New-Object System.Collections.ArrayList + foreach ($record in $records) { + if (Test-SetupManagedLocalRecord $record) { + [void]$removed.Add($record) + } else { + [void]$remaining.Add($record) + } + } + + foreach ($record in $removed) { + $id = [string](Get-JsonPropertyValue $record 'id') + if ([string]::IsNullOrWhiteSpace($id)) { + continue + } + + $identityDir = Join-Path (Join-Path $DataDir 'gateways') $id + try { + if (Test-Path -LiteralPath $identityDir -PathType Container) { + Remove-Item -LiteralPath $identityDir -Recurse -Force -ErrorAction Stop + Write-GatewayLog "Deleted identity directory for local gateway record $id." + } + } catch { + Add-CleanupWarning "Failed to delete identity directory '$identityDir': $($_.Exception.Message)" + } + } + + if ($removed.Count -gt 0) { + try { + $registry.gateways = @($remaining.ToArray()) + $activeId = [string](Get-JsonPropertyValue $registry 'activeId') + if ($removed | Where-Object { [string](Get-JsonPropertyValue $_ 'id') -eq $activeId }) { + $registry.activeId = $null + } + + Write-JsonFileAtomic -Path $gatewaysPath -Value $registry + Write-GatewayLog "Removed $($removed.Count) setup-managed local gateway record(s)." + } catch { + Add-CleanupWarning "Failed to update gateways.json: $($_.Exception.Message)" + } + } else { + Write-GatewayLog 'No setup-managed local gateway records found.' + } + + $hasExternalGateways = $false + foreach ($record in @($remaining.ToArray())) { + if (Test-ExternalGatewayRecord $record) { + $hasExternalGateways = $true + break + } + } + + return [pscustomobject]@{ + RemainingCount = $remaining.Count + HasExternalGateways = $hasExternalGateways + } + } + + function Clear-RootDeviceTokenForRole { + param( + [string]$DataDir, + [string]$Role + ) + + $keyPath = Join-Path $DataDir 'device-key-ed25519.json' + $keyData = Read-JsonFile $keyPath + if ($null -eq $keyData) { + Write-GatewayLog "Root device identity file absent or unreadable for $Role token cleanup." + return + } + + $tokenPropertyName = if ($Role -eq 'node') { 'NodeDeviceToken' } else { 'DeviceToken' } + $scopesPropertyName = if ($Role -eq 'node') { 'NodeDeviceTokenScopes' } else { 'DeviceTokenScopes' } + $tokenProperty = $keyData.PSObject.Properties[$tokenPropertyName] + + if ($null -eq $tokenProperty -or [string]::IsNullOrEmpty([string]$tokenProperty.Value)) { + Write-GatewayLog "Root $Role device token already absent." + return + } + + try { + $tokenProperty.Value = $null + $scopesProperty = $keyData.PSObject.Properties[$scopesPropertyName] + if ($null -ne $scopesProperty) { + $scopesProperty.Value = $null + } + + Write-JsonFileAtomic -Path $keyPath -Value $keyData + Write-GatewayLog "Cleared root $Role device token." + } catch { + Add-CleanupWarning "Failed to clear root $Role device token: $($_.Exception.Message)" + } + } + + function Reset-OnboardingSettings { + param( + [string]$DataDir, + [bool]$PreserveNodeSettings + ) + + $settingsPath = Join-Path $DataDir 'settings.json' + $settings = Read-JsonFile $settingsPath + if ($null -eq $settings) { + Write-GatewayLog 'settings.json absent or unreadable; onboarding settings not reset.' + return + } + + $changed = $false + if ($settings.PSObject.Properties['GatewayUrl']) { + $settings.PSObject.Properties.Remove('GatewayUrl') + $changed = $true + } + + if (-not $PreserveNodeSettings -and $settings.PSObject.Properties['EnableNodeMode']) { + $settings.EnableNodeMode = $false + $changed = $true + } + + if (-not $PreserveNodeSettings -and $settings.PSObject.Properties['AutoStart']) { + $settings.AutoStart = $false + $changed = $true + } + + if (-not $changed) { + Write-GatewayLog 'No onboarding settings needed reset.' + return + } + + try { + Write-JsonFileAtomic -Path $settingsPath -Value $settings + Write-GatewayLog 'Reset onboarding settings.' + } catch { + Add-CleanupWarning "Failed to reset onboarding settings: $($_.Exception.Message)" + } + } + + function Remove-KeepaliveMarker { + param([string]$LocalDataDir) + + $markerDir = Join-Path $LocalDataDir 'wsl-keepalive' + $markerPath = Join-Path $markerDir "$DistroName.json" + Remove-FileIfExists -Path $markerPath -Label 'keepalive marker' + + try { + if ((Test-Path -LiteralPath $markerDir -PathType Container) -and -not (Get-ChildItem -LiteralPath $markerDir -Force -ErrorAction Stop | Select-Object -First 1)) { + Remove-Item -LiteralPath $markerDir -Force -ErrorAction Stop + Write-GatewayLog 'Deleted empty wsl-keepalive directory.' + } + } catch { + Add-CleanupWarning "Failed to remove empty wsl-keepalive directory '$markerDir': $($_.Exception.Message)" + } + } + + function Remove-WindowsGatewayArtifacts { + $dataDir = Resolve-AppDataDir + $localDataDir = Resolve-LocalDataDir + + Write-GatewayLog "Cleaning Windows-side local gateway artifacts. AppData='$dataDir'; LocalData='$localDataDir'." + + Remove-AutostartRegistryValue + Remove-FileIfExists -Path (Join-Path $dataDir 'setup-state.json') -Label 'legacy setup-state.json' + Remove-FileIfExists -Path (Join-Path $localDataDir 'setup-state.json') -Label 'setup-state.json' + Remove-FileIfExists -Path (Join-Path $localDataDir 'run.marker') -Label 'run.marker' + Remove-FileIfExists -Path (Join-Path $dataDir 'exec-policy.json') -Label 'exec-policy.json' + Remove-KeepaliveMarker -LocalDataDir $localDataDir + + $registryCleanup = Remove-SetupManagedGatewayRecords -DataDir $dataDir + if ($registryCleanup.HasExternalGateways) { + Write-GatewayLog 'External gateway records remain; preserving root device tokens.' + } else { + Clear-RootDeviceTokenForRole -DataDir $dataDir -Role 'operator' + Clear-RootDeviceTokenForRole -DataDir $dataDir -Role 'node' + } + + Reset-OnboardingSettings -DataDir $dataDir -PreserveNodeSettings:($registryCleanup.RemainingCount -gt 0) + Remove-DirectoryIfExists -Path (Join-Path $dataDir 'Logs') -Label 'AppData Logs' + Remove-DirectoryIfExists -Path (Join-Path $localDataDir 'Logs') -Label 'LocalAppData Logs' + } + + function Complete-GatewayCleanup { + param([string]$Message) + + Remove-WindowsGatewayArtifacts + Write-GatewayResult ` + -Succeeded $true ` + -ExitCode 0 ` + -Message $Message ` + -Details ([ordered]@{ artifactWarnings = @($script:cleanupWarnings) }) + Write-Host "OpenClaw local WSL gateway removed successfully." + exit 0 +} + +function Get-WslExePath { + $candidates = @( + (Join-Path $env:WINDIR 'Sysnative\wsl.exe'), + (Join-Path $env:WINDIR 'System32\wsl.exe') + ) + + foreach ($candidate in $candidates) { + if (Test-Path -LiteralPath $candidate) { + return $candidate + } + } + + $command = Get-Command wsl.exe -ErrorAction SilentlyContinue + if ($command) { + return $command.Source + } + + return $null +} + +function Format-Arguments { + param([string[]]$Arguments) + + return ($Arguments | ForEach-Object { + if ($_ -match '\s') { + '"' + ($_ -replace '"', '\"') + '"' + } else { + $_ + } + }) -join ' ' +} + +function Invoke-Wsl { + param([string[]]$Arguments) + + $stdoutPath = [System.IO.Path]::GetTempFileName() + $stderrPath = [System.IO.Path]::GetTempFileName() + + try { + $process = Start-Process ` + -FilePath $script:WslPath ` + -ArgumentList $Arguments ` + -WindowStyle Hidden ` + -Wait ` + -PassThru ` + -RedirectStandardOutput $stdoutPath ` + -RedirectStandardError $stderrPath + + $stdout = if (Test-Path -LiteralPath $stdoutPath) { Get-Content -LiteralPath $stdoutPath -Raw -ErrorAction SilentlyContinue } else { '' } + $stderr = if (Test-Path -LiteralPath $stderrPath) { Get-Content -LiteralPath $stderrPath -Raw -ErrorAction SilentlyContinue } else { '' } + $output = (($stdout, $stderr) | Where-Object { -not [string]::IsNullOrWhiteSpace($_) }) -join [Environment]::NewLine + + Write-GatewayLog ("wsl.exe {0} exited {1}.{2}{3}" -f (Format-Arguments $Arguments), $process.ExitCode, [Environment]::NewLine, $output) + + return [pscustomobject]@{ + ExitCode = [int]$process.ExitCode + Output = $output + } + } finally { + Remove-Item -LiteralPath $stdoutPath, $stderrPath -Force -ErrorAction SilentlyContinue + } +} + +function Test-DistroNotFound { + param([string]$Output) + + if ([string]::IsNullOrWhiteSpace($Output)) { + return $false + } + + return $Output -match 'WSL_E_DISTRO_NOT_FOUND' -or + $Output -match 'There is no distribution with the supplied name' -or + $Output -match 'The specified distribution.*(could not be found|not found)' -or + $Output -match 'distribution.*not.*found' +} + +function Test-DistroListed { + param([string]$Output) + + if ([string]::IsNullOrWhiteSpace($Output)) { + return $false + } + + $distros = ($Output -replace "`0", '') -split '\r?\n' | ForEach-Object { $_.Trim() } + return $distros -contains $DistroName +} + +function Remove-GatewayDirectory { + $gatewayDirectory = Join-Path $AppRoot 'wsl\OpenClawGateway' + + if (-not (Test-Path -LiteralPath $gatewayDirectory)) { + Write-GatewayLog "Gateway directory does not exist: $gatewayDirectory" + return + } + + $gatewayItem = Get-Item -LiteralPath $gatewayDirectory -Force -ErrorAction Stop + if (($gatewayItem.Attributes -band [System.IO.FileAttributes]::ReparsePoint) -ne 0) { + throw "Refusing to recursively delete reparse point '$gatewayDirectory'." + } + + $lastError = $null + for ($attempt = 1; $attempt -le 6; $attempt++) { + try { + Remove-Item -LiteralPath $gatewayDirectory -Recurse -Force -ErrorAction Stop + if (-not (Test-Path -LiteralPath $gatewayDirectory)) { + Write-GatewayLog "Removed gateway directory: $gatewayDirectory" + return + } + } catch { + $lastError = $_.Exception.Message + Write-GatewayLog "Attempt $attempt failed to remove gateway directory '$gatewayDirectory': $lastError" + } + + Start-Sleep -Seconds 1 + } + + throw "Failed to remove gateway directory '$gatewayDirectory': $lastError" } -# --------------------------------------------------------------------------- -# Invoke CLI uninstall -# --------------------------------------------------------------------------- -$exitCode = 0 try { - & $exePath --uninstall --confirm-destructive --json-output $resultPath - $exitCode = if ($null -eq $global:LASTEXITCODE) { 0 } else { $global:LASTEXITCODE } + Ensure-AppRoot + Write-GatewayLog "Starting local gateway cleanup for $DistroName." + + $script:WslPath = Get-WslExePath + if (-not $script:WslPath) { + Write-GatewayLog 'wsl.exe was not found; removing stale gateway directory if present.' + Remove-GatewayDirectory + Complete-GatewayCleanup -Message 'wsl.exe was not found; no registered WSL gateway can be removed.' + } + + $listResult = Invoke-Wsl -Arguments @('--list', '--quiet') + if ($listResult.ExitCode -eq 0 -and -not (Test-DistroListed $listResult.Output)) { + Write-GatewayLog "WSL distro '$DistroName' is not registered; removing stale gateway directory if present." + Remove-GatewayDirectory + Complete-GatewayCleanup -Message "Local WSL gateway '$DistroName' was already unregistered." + } - if ($exitCode -eq 0) { - Write-Host "OpenClaw local WSL gateway removed successfully." -ForegroundColor Green - } else { - Write-Warning "OpenClaw gateway uninstall exited $exitCode; see '$resultPath' for details." + $terminateResult = Invoke-Wsl -Arguments @('--terminate', $DistroName) + if ($terminateResult.ExitCode -ne 0) { + Write-GatewayLog "Ignoring terminate exit code $($terminateResult.ExitCode); unregister handles stopped or missing distros." } + + $shutdownResult = Invoke-Wsl -Arguments @('--shutdown') + if ($shutdownResult.ExitCode -ne 0) { + Write-GatewayLog "Ignoring shutdown exit code $($shutdownResult.ExitCode); unregister will still be attempted." + } + Start-Sleep -Seconds 2 + + $unregisterResult = Invoke-Wsl -Arguments @('--unregister', $DistroName) + if ($unregisterResult.ExitCode -ne 0 -and -not (Test-DistroNotFound $unregisterResult.Output)) { + Write-GatewayResult ` + -Succeeded $false ` + -ExitCode $unregisterResult.ExitCode ` + -Message "Failed to unregister WSL distro '$DistroName'." ` + -Details $unregisterResult.Output + exit $unregisterResult.ExitCode + } + + if ($unregisterResult.ExitCode -ne 0) { + Write-GatewayLog "Treating missing distro '$DistroName' as already removed." + } + + Remove-GatewayDirectory + + Complete-GatewayCleanup -Message "Local WSL gateway '$DistroName' removed." } catch { - $msg = "[$(Get-Date -Format 'o')] Uninstall-LocalGateway.ps1 error: $($_.Exception.Message)" - try { $msg | Out-File -LiteralPath $errorPath -Encoding UTF8 -Force } catch {} - Write-Warning $msg + $message = $_.Exception.Message + Write-GatewayLog "Local gateway cleanup failed: $message" + try { "[$(Get-Date -Format 'o')] $message" | Out-File -LiteralPath $errorPath -Encoding UTF8 -Force } catch {} + Write-GatewayResult -Succeeded $false -ExitCode 1 -Message $message + Write-Warning $message + exit 1 } - -# Always exit 0 so Inno does not abort the broader uninstall. -exit 0 diff --git a/src/OpenClaw.Connection/GatewayConnectionManager.cs b/src/OpenClaw.Connection/GatewayConnectionManager.cs index 8a5d74d01..61986a6ce 100644 --- a/src/OpenClaw.Connection/GatewayConnectionManager.cs +++ b/src/OpenClaw.Connection/GatewayConnectionManager.cs @@ -33,6 +33,7 @@ public sealed class GatewayConnectionManager : IGatewayConnectionManager private bool _disposed; private Task? _disposeTask; private bool _gatewayNeedsV2Signature; // remembered across reconnects + private string? _operatorTokenRecoveryAttemptedGatewayId; private string? _lastAutoApprovedRequestId; // prevent auto-approve loops private string? _autoApproveInFlight; // atomic guard against concurrent approval of same requestId @@ -149,6 +150,7 @@ private async Task ConnectCoreAsync(string? gatewayId = null) _diagnostics.RecordCredentialResolution(credential); _activeIdentityPath = perGatewayIdentityDir; _activeGatewayRecordId = record.Id; + _gatewayNeedsV2Signature = record.IsLocal || record.RequiresV2Signature; if (credential == null) { @@ -241,12 +243,13 @@ private async Task ConnectCoreAsync(string? gatewayId = null) }; lifecycle.DataClient.V2SignatureFallback += (s, _) => { - _gatewayNeedsV2Signature = true; + if (Interlocked.Read(ref _generation) != gen) return; + RememberGatewayNeedsV2Signature(record.Id); }; // Local gateways only support v2 signatures — skip the v3 attempt entirely // to avoid a spurious "metadata-upgrade" re-pairing triggered by the v3→v2 fallback. - if (record.IsLocal) + if (record.IsLocal || record.RequiresV2Signature) _gatewayNeedsV2Signature = true; // If we already know this gateway needs v2, tell the client upfront @@ -488,6 +491,9 @@ private async Task HandleAuthenticationFailedAsync(string message, long gen) { if (Interlocked.Read(ref _generation) != gen) return; + if (TryScheduleOperatorTokenRecovery(message, gen)) + return; + var prev = _stateMachine.Current.OverallState; _diagnostics.Record("error", "Authentication failed", message); _stateMachine.TryTransition(ConnectionTrigger.AuthenticationFailed, message); @@ -499,6 +505,48 @@ private async Task HandleAuthenticationFailedAsync(string message, long gen) } } + private bool TryScheduleOperatorTokenRecovery(string message, long gen) + { + if (!IsOperatorDeviceTokenMismatch(message) || + _activeGatewayRecordId == null || + _activeIdentityPath == null || + _operatorTokenRecoveryAttemptedGatewayId == _activeGatewayRecordId) + { + return false; + } + + var record = _registry.GetById(_activeGatewayRecordId); + if (record == null || string.IsNullOrWhiteSpace(record.BootstrapToken)) + return false; + + if (!DeviceIdentity.TryClearDeviceToken(_activeIdentityPath, _logger)) + return false; + + _operatorTokenRecoveryAttemptedGatewayId = _activeGatewayRecordId; + _diagnostics.Record("credential", "Cleared stale operator device token; reconnecting with bootstrap token"); + + _ = Task.Run(async () => + { + try + { + await _reconnectDelay(TimeSpan.FromMilliseconds(200)); + if (Interlocked.Read(ref _generation) != gen || _disposed) return; + await ReconnectAsync(); + } + catch (ObjectDisposedException) { } + catch (Exception ex) + { + _logger.Warn($"[ConnMgr] Operator token recovery reconnect failed: {ex.Message}"); + } + }); + + return true; + } + + private static bool IsOperatorDeviceTokenMismatch(string message) => + message.Contains("device token mismatch", StringComparison.OrdinalIgnoreCase) || + message.Contains("AUTH_DEVICE_TOKEN_MISMATCH", StringComparison.OrdinalIgnoreCase); + private async Task HandleHandshakeSucceededAsync(long gen) { await _transitionSemaphore.WaitAsync(); @@ -510,6 +558,8 @@ private async Task HandleHandshakeSucceededAsync(long gen) _diagnostics.Record("state", "Handshake succeeded (hello-ok)"); _stateMachine.TryTransition(ConnectionTrigger.HandshakeSucceeded); _diagnostics.RecordStateChange(prev, _stateMachine.Current.OverallState); + if (_operatorTokenRecoveryAttemptedGatewayId == _activeGatewayRecordId) + _operatorTokenRecoveryAttemptedGatewayId = null; // Update device ID from client if (_activeLifecycle?.DataClient is { } client) @@ -579,6 +629,25 @@ private void HandleDeviceTokenReceived(DeviceTokenReceivedEventArgs e) } } + private void RememberGatewayNeedsV2Signature(string? gatewayRecordId) + { + _gatewayNeedsV2Signature = true; + + if (string.IsNullOrWhiteSpace(gatewayRecordId)) + return; + + try + { + _registry.Update(gatewayRecordId, r => r.RequiresV2Signature ? r : r with { RequiresV2Signature = true }); + _registry.Save(); + _diagnostics.Record("credential", "Remembered gateway v2 signature requirement"); + } + catch (Exception ex) + { + _logger.Warn($"[ConnMgr] Failed to persist v2 signature requirement: {ex.Message}"); + } + } + private async Task HandlePairingRequiredAsync(string? requestId, long gen) { await _transitionSemaphore.WaitAsync(); diff --git a/src/OpenClaw.Connection/GatewayRecord.cs b/src/OpenClaw.Connection/GatewayRecord.cs index 025bc22f6..d0680d637 100644 --- a/src/OpenClaw.Connection/GatewayRecord.cs +++ b/src/OpenClaw.Connection/GatewayRecord.cs @@ -27,6 +27,9 @@ public sealed record GatewayRecord /// True for gateways provisioned locally (localhost/WSL). public bool IsLocal { get; init; } + /// True when this gateway is known to require v2 auth signatures. + public bool RequiresV2Signature { get; init; } + /// WSL distro name for gateway records provisioned by SetupEngine. public string? SetupManagedDistroName { get; init; } diff --git a/src/OpenClaw.Shared/OpenClawGatewayClient.cs b/src/OpenClaw.Shared/OpenClawGatewayClient.cs index dfaa089da..30795c31e 100644 --- a/src/OpenClaw.Shared/OpenClawGatewayClient.cs +++ b/src/OpenClaw.Shared/OpenClawGatewayClient.cs @@ -244,6 +244,7 @@ public OpenClawGatewayClient(string gatewayUrl, string token, IOpenClawLogger? l _deviceIdentity = new DeviceIdentity(dataPath, _logger); _deviceIdentity.Initialize(); _connectAuthToken = _deviceIdentity.DeviceToken ?? (_tokenIsBootstrapToken ? string.Empty : _token); + _useV2Signature |= _tokenIsBootstrapToken && string.IsNullOrEmpty(_deviceIdentity.DeviceToken); } public async Task DisconnectAsync() diff --git a/src/OpenClaw.Shared/WindowsNodeClient.cs b/src/OpenClaw.Shared/WindowsNodeClient.cs index 28fac6285..2c3a88789 100644 --- a/src/OpenClaw.Shared/WindowsNodeClient.cs +++ b/src/OpenClaw.Shared/WindowsNodeClient.cs @@ -117,6 +117,7 @@ public WindowsNodeClient(string gatewayUrl, string token, string dataPath, IOpen // Initialize device identity _deviceIdentity = new DeviceIdentity(dataPath, _logger); _deviceIdentity.Initialize(); + _useV2Signature |= !string.IsNullOrEmpty(_bootstrapToken) && string.IsNullOrEmpty(_deviceIdentity.NodeDeviceToken); // Initialize registration _registration = new NodeRegistration diff --git a/src/OpenClaw.Tray.WinUI/Package.appxmanifest b/src/OpenClaw.Tray.WinUI/Package.appxmanifest index 7016ca5c7..af0301e44 100644 --- a/src/OpenClaw.Tray.WinUI/Package.appxmanifest +++ b/src/OpenClaw.Tray.WinUI/Package.appxmanifest @@ -6,20 +6,20 @@ xmlns:rescap="http://schemas.microsoft.com/appx/manifest/foundation/windows10/restrictedcapabilities" IgnorableNamespaces="uap rescap"> - OpenClaw Companion - Scott Hanselman + OpenClaw Foundation Assets\StoreLogo.png diff --git a/src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml b/src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml index 7e4290313..8addc3deb 100644 --- a/src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml +++ b/src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml @@ -165,6 +165,17 @@ Visibility="Collapsed" Click="OnStripPrimaryClicked" AutomationProperties.AutomationId="StripPrimaryAction"/> + +