diff --git a/.agents/skills/reviewing-before-pr/SKILL.md b/.agents/skills/reviewing-before-pr/SKILL.md index f3472fc56b61..163caed60a6c 100644 --- a/.agents/skills/reviewing-before-pr/SKILL.md +++ b/.agents/skills/reviewing-before-pr/SKILL.md @@ -1,49 +1,36 @@ --- name: reviewing-before-pr description: > - Run one local Greptile review of a branch before its PR opens, with `hogli review`. - Opt-in: it needs a seat on PostHog's Greptile workspace, which most people do not have yet, - so use it only when someone asks for a local Greptile review by name. Trigger terms: - hogli review, greptile, local review, pre-PR review. + Review a branch with the harness's own code review skill before its PR opens, and record the pass + in the PR description. Use when someone asks for a local review, a self-review, or a review before + opening or pushing a PR. Trigger terms: local review, self-review, review before PR, + review my branch, pre-PR review. --- # Reviewing before opening a PR -`hogli review` runs the same Greptile reviewer that comments on every PostHog PR, locally, against the committed branch diff. -A finding handled here is an ordinary pre-push edit; the same finding after the PR opens is a bot comment, a fix push, a stale-thread cleanup, and a CI re-run. +Run your harness's code review over the branch diff before `gh pr create`. +A finding handled here is an ordinary pre-push edit; the same finding after the PR opens costs a bot comment, a fix push, a stale-thread cleanup, and a CI re-run. + +This is the weaker pass, not a replacement for review: it reads code your own session may have written, with no independent context. +The PR review bot still reviews every PR, and human reviewers still read it. ## When to run -- Only when someone asks for it. A Greptile seat is per-person and hand-granted today, so opening a PR does not call for this review by default — the PR bot still reviews every PR. -- When asked, run it **once**, on the committed branch, right before `gh pr create`. -- Do not loop it after every push, and do not re-run it after addressing findings — each run is a paid review, and the PR bot passes over the final state anyway. `hogli review` enforces this by re-printing HEAD's existing completed review instead of starting a new one. -- Skip it for changes trivial enough for the `skip-agent-review` label. +- When someone asks for it. Opening a PR does not call for this review by default. +- Run it **once**, on the branch as it stands, right before `gh pr create`. +- Do not loop it after every push, and do not re-run it after addressing findings. ## The flow -1. Finish the work and commit everything. Greptile reviews committed changes only; uncommitted edits are invisible to it. -2. Run `hogli review`. -3. Verify each finding's premise against the code before acting on it. Findings carry severity and confidence, and they can be false positives — rejecting one, with a reason, is a valid outcome. -4. Fix what you agree with and commit the fixes. -5. Skip the duplicate bot review. Run `hogli review --check` (on a stacked branch, `-b ` scopes it to this layer's commits): it exits 0 when any commit on the branch has a completed review — that is, the local loop ran, even if fix commits landed after it. On exit 0, open the PR with `--label no-greptile`. On a nonzero exit (no local review ever ran on this branch, or signed out), omit the label and let the bot review. -6. Either way, record the local review in the PR description's Agent context section: the review ID and each finding's disposition (fixed, or rejected with the reason). The findings are otherwise invisible to reviewers — they only appeared in the terminal. -7. Continue the normal PR-opening flow (`hogli ci:preflight`, `gh pr create`). - -## Fallback without Greptile access - -When `hogli review` cannot run (exit 78 with no way to sign in, or no CLI in a sandbox), do not block the PR on it. -Run your harness's own review instead (Claude Code: `/code-review`) over the branch diff, and triage its findings through the same flow: verify each premise, fix what holds, commit. -Treat it as the weaker pass — it reviews code your own session may have written, with no independent context — so verify premises strictly. -A fallback review never earns the `no-greptile` label: `hogli review --check` stays nonzero without a Greptile review, so the gate enforces this on its own and the bot reviews the PR as usual. -Say in the PR description's receipt that the local pass was the harness fallback, so reviewers know the independent review is still the bot's. +1. Finish the work. Commit first when the harness's review takes a branch or commit range rather than the working tree. +2. Run the harness's review over the branch diff. In Claude Code that is `/code-review`; on a stacked branch, scope it to this layer's commits against its base. +3. Verify each finding's premise against the code before acting on it. Findings can be false positives, and rejecting one with a reason is a valid outcome. +4. Fix what holds and commit the fixes. +5. Record the pass in the PR description's Agent context section: which review ran, and each finding's disposition (fixed, or rejected with the reason). The findings are otherwise invisible to reviewers, because they only appeared in the terminal. +6. Continue the normal PR-opening flow (`hogli ci:preflight`, `gh pr create`). ## Notes -- **Exit 78 means not signed in.** Ask the user to run `greptile login` and sign in with Google, using their @posthog.com account — that is what grants access to the PostHog Greptile org, so pick "Continue with Google" in the browser window rather than creating an email-and-password account. Headless environments set `GREPTILE_API_KEY` in `.env.local` instead (see `.env.local.example`). Never attempt the interactive login yourself; when sign-in is not available, use the fallback above. -- **Missing CLI.** Flox activation installs the version pinned in `.flox/env/on-activate.sh` into the machine-shared store `~/.config/posthog/tools/greptile/`, so re-entering the environment usually fixes it. Outside flox: `brew install greptileai/tap/greptile` or `npm install -g greptile`. -- **Held-back files.** Greptile holds back files that look like they contain secrets. Leave them held back; pass `--include ` only when certain the file is safe to send. -- **Focus.** `--instructions ""` steers the reviewer, the same way an `@greptile` comment does on a PR. -- **`--force`** starts a fresh paid review even when HEAD already has one — only when the user asks for it. -- **The `no-greptile` label** works because `.greptile/config.json` lists it in `disabledLabels`. Apply it only through the `--check` gate above — never to silence a bot review you have not run locally. When in doubt, leave the label off; the default is that the bot reviews. -- Review behavior is configured in `.greptile/` (shared with the PR bot): `config.json` for settings, `files.json` for context files, with nested per-directory configs (`products/desktop/.greptile/` exists). -- The diff goes to Greptile's API for review. This repo is public, so that is fine for repo content — but it is one more reason not to `--include` held-back files. +- **The bot pass is separate.** A local review never substitutes for the PR review bot, so never label a PR to skip the bot on the strength of this pass. +- **Focus it when the diff has a risky part.** Most harness reviews accept instructions or a path target; pointing the review at the part you are least sure about beats a broad sweep. diff --git a/.env.local.example b/.env.local.example index cf97830a0651..24fe906a0a2d 100644 --- a/.env.local.example +++ b/.env.local.example @@ -15,11 +15,6 @@ # AZURE_INFERENCE_ENDPOINT=op://General/yw6efvcz5ooxx5fgll3kyang5a/username # AZURE_INFERENCE_CREDENTIAL=op://General/yw6efvcz5ooxx5fgll3kyang5a/credential -# ── Local code review (PostHog employees) ───────────────────────────── -# hogli review normally authenticates via `greptile login` (Google SSO). -# Set the key only for headless environments. -# GREPTILE_API_KEY= - # ── Workflows / messaging (PostHog employees) ───────────────────────── # Server-side rendering of email template designs (Unlayer export API). # Without it, design-only template saves over the API return a 400. diff --git a/.flox/env/on-activate.sh b/.flox/env/on-activate.sh index baf9e18b18c6..f0aec9ae4144 100755 --- a/.flox/env/on-activate.sh +++ b/.flox/env/on-activate.sh @@ -365,52 +365,6 @@ if [[ "$_PHROCS_SKIP" -eq 0 ]]; then _BG_PHROCS_START=$(date +%s) fi -# Greptile CLI: machine-global, version-addressed store. Greptile is not in -# the flox catalog (proprietary npm package), and `hogli review` needs it. -# Not a root devDependency on purpose: that would pull a review-only CLI into -# every pnpm install, including CI and sandboxes that never review. One npm -# install per machine per pinned version serves every checkout and survives -# .flox/cache wipes; each activation only ensures the version and symlinks -# the venv bin (Step 2b), so worktrees on different branches resolve their -# own pin. A failed install must not break activation; the CLI is only needed -# at PR-open time and `hogli review` prints install guidance when absent. -_GREPTILE_VERSION="3.4.1" -_GREPTILE_STORE="$HOME/.config/posthog/tools/greptile/$_GREPTILE_VERSION" -_GREPTILE_BIN="$_GREPTILE_STORE/node_modules/.bin/greptile" -_GREPTILE_STAMP="$_GREPTILE_STORE/.complete" - -_install_greptile() { - # Explicit `|| return`/`|| exit`: callers suppress errexit, so a failed - # install would otherwise fall through and stamp the broken state. - mkdir -p "$_GREPTILE_STORE" || return 1 - ( - # The store is shared across checkouts, so serialize concurrent - # activations (fresh worktrees) installing the same version. - flock 9 || exit 1 - if [[ ! -x "$_GREPTILE_BIN" || ! -f "$_GREPTILE_STAMP" ]]; then - if [[ "$_DEV_SANDBOX_INSTALLS" -eq 1 ]]; then - # printf %q: dev-sandbox re-parses its command string, so the path - # must survive a $HOME with spaces or quotes. - "$FLOX_ENV_PROJECT/bin/dev-sandbox" "npm install --prefix $(printf '%q' "$_GREPTILE_STORE") --no-fund --no-audit greptile@$_GREPTILE_VERSION" || exit 1 - else - npm install --prefix "$_GREPTILE_STORE" --no-fund --no-audit "greptile@$_GREPTILE_VERSION" || exit 1 - fi - [[ -x "$_GREPTILE_BIN" ]] || exit 1 - touch "$_GREPTILE_STAMP" || exit 1 - fi - ) 9>"$_GREPTILE_STORE/.install.lock" -} - -_GREPTILE_SKIP=0 -[[ -x "$_GREPTILE_BIN" && -f "$_GREPTILE_STAMP" ]] && _GREPTILE_SKIP=1 -if [[ "$_GREPTILE_SKIP" -eq 0 ]]; then - _BG_GREPTILE_LOG=$(mktemp) - _ACTIVATION_TMPFILES+=("$_BG_GREPTILE_LOG") - ( _install_greptile ) >"$_BG_GREPTILE_LOG" 2>&1 & - _BG_GREPTILE_PID=$! - _BG_GREPTILE_START=$(date +%s) -fi - # ── Step 1: Python packages (must run before hogli — it needs Click) ─ if [[ "$_UV_SKIP" -eq 1 ]]; then done_step "Python packages (cached)" @@ -463,17 +417,6 @@ else wait_bg_step "Node packages" "$_BG_PNPM_PID" "$_BG_PNPM_START" "$_BG_PNPM_LOG" fi -# ── Step 2b: Greptile CLI (reap; launched above with the other jobs) ── -if [[ "$_GREPTILE_SKIP" -eq 1 ]]; then - done_step "Greptile CLI (cached)" -else - wait_bg_step "Greptile CLI" "$_BG_GREPTILE_PID" "$_BG_GREPTILE_START" "$_BG_GREPTILE_LOG" \ - || warn_step "Greptile CLI install failed ${C_DIM}(hogli review prints manual install steps)${C_RESET}" -fi -if [[ -x "$_GREPTILE_BIN" && -d "$UV_PROJECT_ENVIRONMENT/bin" ]]; then - ln -sf "$_GREPTILE_BIN" "$UV_PROJECT_ENVIRONMENT/bin/greptile" -fi - # ── Step 3: /etc/hosts ────────────────────────────────────────────── POSTHOG_HOSTS="127.0.0.1 db redis7 kafka clickhouse clickhouse-coordinator objectstorage seaweedfs temporal # posthog" if grep -qF "$POSTHOG_HOSTS" /etc/hosts; then diff --git a/hogli.yaml b/hogli.yaml index 803cb42d54fa..e5c8241a5363 100644 --- a/hogli.yaml +++ b/hogli.yaml @@ -1042,10 +1042,6 @@ tools: ci:preflight: click: hogli_commands.ci_preflight:ci_preflight description: Catch deterministic CI failures reachable from your diff before you push (advisory; --fix, --strict) - review: - click: hogli_commands.review:review - description: One local Greptile review of the branch before opening the PR (skips when HEAD is already reviewed; --force, --instructions, --check) - needs_secrets: true desktop: desktop:dev: cmd: pnpm --dir products/desktop dev diff --git a/tools/hogli-commands/hogli_commands/review.py b/tools/hogli-commands/hogli_commands/review.py deleted file mode 100644 index 4dfcbe81fde3..000000000000 --- a/tools/hogli-commands/hogli_commands/review.py +++ /dev/null @@ -1,158 +0,0 @@ -"""One local Greptile review of the branch, before its PR opens. - -``hogli review`` wraps the Greptile CLI. Greptile already reviews every PostHog -PR after it opens; running the first pass locally turns its findings into -pre-push edits instead of bot comments, stale-thread cleanup, and CI re-runs. - - hogli review # review committed changes vs the repo default base - hogli review --instructions "..." # focus the reviewer, like an @greptile PR comment - hogli review --force # start a new review even when HEAD has one - hogli review --check # exit 0 when any branch commit has a completed review - -``--check`` gates the ``no-greptile`` PR label: a completed review anywhere on -the branch means the local loop ran, and the label skips the duplicate bot -pass even when fix commits landed after the review. Only a branch no review -ever ran on (or a state where that cannot be told: signed out, no CLI) exits -nonzero, leaving the PR bot as the reviewer. On a stacked branch, pass -``-b `` so the walk covers only this layer's commits. - -On top of ``greptile review`` itself, the wrapper: - -- skips the paid review when HEAD already has a completed one (re-printing it) - and resumes one still in progress, so "review once before opening the PR" - stays once across agent retries; ``--force`` overrides. -- exits ``78`` (sysexits ``EX_CONFIG``) with setup guidance when not signed in, - matching ``ci:insights`` so skills branch on the exit code, not message text. - -Greptile reviews committed changes only, so commit everything first. The -reviewing-before-pr skill covers where this sits in the PR-opening flow. -""" - -from __future__ import annotations - -import shutil -import subprocess - -import click -from hogli.manifest import REPO_ROOT - -from hogli_commands import posthog_auth - -EX_CONFIG = posthog_auth.EXIT_NOT_CONFIGURED - -_INSTALL_HINT = ( - "Re-enter the flox environment (activation installs it), " - "or install it with `brew install greptileai/tap/greptile` or `npm install -g greptile`, then re-run." -) -_SIGNIN_HINT = ( - "Run `greptile login`, or set GREPTILE_API_KEY in .env.local (see .env.local.example). " - "No access? The reviewing-before-pr skill has a harness-review fallback." -) - -# From the Greptile CLI reference for `review status`: exit 0 means a completed -# review exists for the commit, 3 means one is still running. -_STATUS_COMPLETED = 0 -_STATUS_RUNNING = 3 - -# The probes are short subprocess calls; the review itself runs uncapped -# because a large diff legitimately takes minutes. -_PROBE_TIMEOUT_SECONDS = 60 - -# Newest-first probe window for --check. Each probe is a network call, and the -# gate only asserts "the local loop ran on this branch", so a hit is expected -# within the last few commits. -_CHECK_COMMIT_LIMIT = 20 - - -def _probe(cmd: list[str]) -> subprocess.CompletedProcess[str] | None: - try: - return subprocess.run(cmd, capture_output=True, text=True, timeout=_PROBE_TIMEOUT_SECONDS) - except (OSError, subprocess.SubprocessError): - return None - - -def _signed_in(binary: str) -> bool: - """False only on Greptile's explicit signed-out error; any other ``config`` - failure falls through to the review call, which reports the real problem. - The string match is forced: greptile exits 1 for signed-out and for - ordinary failures alike.""" - result = _probe([binary, "config"]) - return result is None or result.returncode == 0 or "not signed in" not in (result.stdout + result.stderr).lower() - - -def _branch_commits(base: str | None) -> list[str]: - # Match change_detection's base convention: origin/master, then master for - # clones without the remote ref. - for ref in [base] if base is not None else ["origin/master", "master"]: - result = _probe(["git", "-C", str(REPO_ROOT), "rev-list", f"--max-count={_CHECK_COMMIT_LIMIT}", f"{ref}..HEAD"]) - if result is not None and result.returncode == 0: - return result.stdout.split() or ["HEAD"] - return ["HEAD"] - - -def check(binary: str, base: str | None) -> int: - for commit in _branch_commits(base): - status = _probe([binary, "review", "status", "--commit", commit]) - if status is None: - # A hung or broken probe would hang or break for every commit too. - break - if status.returncode == _STATUS_COMPLETED: - click.secho( - f"Commit {commit[:11]} has a completed review. Open the PR with `--label no-greptile`.", - fg="green", - err=True, - ) - return 0 - click.secho("No commit on this branch has a completed review.", fg="yellow", err=True) - return 1 - - -def run(branch: str | None, instructions: str | None, force: bool, do_check: bool) -> int: - binary = shutil.which("greptile") - if binary is None: - click.secho(f"Greptile CLI not found. {_INSTALL_HINT}", fg="red", err=True) - return 1 - if not _signed_in(binary): - click.secho(f"Not signed in to Greptile. {_SIGNIN_HINT}", fg="yellow", err=True) - return EX_CONFIG - if do_check: - return check(binary, branch) - - if not force: - status = _probe([binary, "review", "status", "--commit", "HEAD"]) - if status is not None and status.returncode == _STATUS_COMPLETED: - click.secho( - "HEAD already has a completed review. Showing it. Pass --force to start a new one.", - fg="cyan", - err=True, - ) - click.echo(status.stdout, nl=False) - return 0 - if status is not None and status.returncode == _STATUS_RUNNING: - click.secho("A review for this branch is still running. Resuming it.", fg="cyan", err=True) - return subprocess.run([binary, "review", "--resume"]).returncode - - cmd = [binary, "review"] - if branch is not None: - cmd += ["--branch", branch] - if instructions is not None: - cmd += ["--instructions", instructions] - # Inherit stdio so Greptile's own progress and interactive review view work. - return subprocess.run(cmd).returncode - - -@click.command( - name="review", - help="One local Greptile review of the branch, before you open the PR or mark it ready.", -) -@click.option("-b", "--branch", default=None, help="Base branch to review against. Omit to use the repository default.") -@click.option("--instructions", default=None, help="Extra instructions for this review, like an @greptile PR comment.") -@click.option("--force", is_flag=True, help="Start a new review even when HEAD already has a completed one.") -@click.option( - "--check", - "do_check", - is_flag=True, - help="Only report whether any commit on this branch has a completed review (exit 0 when one does); reviews nothing.", -) -def review(branch: str | None, instructions: str | None, force: bool, do_check: bool) -> None: - raise SystemExit(run(branch, instructions, force, do_check)) diff --git a/tools/hogli-commands/hogli_commands/tests/test_review.py b/tools/hogli-commands/hogli_commands/tests/test_review.py deleted file mode 100644 index ad41082a2479..000000000000 --- a/tools/hogli-commands/hogli_commands/tests/test_review.py +++ /dev/null @@ -1,172 +0,0 @@ -from __future__ import annotations - -import subprocess -from collections.abc import Callable, Iterator - -import pytest -from unittest.mock import MagicMock, patch - -from click.testing import CliRunner -from hogli.cli import cli - -runner = CliRunner() - -_BINARY = "/usr/local/bin/greptile" - - -def _proc(returncode: int, stdout: str = "", stderr: str = "") -> MagicMock: - return MagicMock(returncode=returncode, stdout=stdout, stderr=stderr) - - -def _fake_greptile( - status: int | dict[str, int] = 1, review_exit: int = 0, rev_list_exit: int = 0 -) -> Callable[..., MagicMock]: - """subprocess.run side effect: git rev-list, the config auth probe, status - probes (one exit code, or one per commit), and the streamed review call.""" - - def fake_run(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[0] == "git": - return _proc(rev_list_exit, stdout="tip\nbase\n") - if cmd[1] == "config": - return _proc(0) - if cmd[1:3] == ["review", "status"]: - code = status if isinstance(status, int) else status[cmd[cmd.index("--commit") + 1]] - return _proc(code, stdout="stored review\n") - return _proc(review_exit) - - return fake_run - - -def _status_probes(mock_run: MagicMock) -> list[list[str]]: - return [call.args[0] for call in mock_run.call_args_list if call.args[0][1:3] == ["review", "status"]] - - -def _reviews(mock_run: MagicMock) -> list[list[str]]: - return [ - call.args[0] - for call in mock_run.call_args_list - if call.args[0][:2] == [_BINARY, "review"] and call.args[0][1:3] != ["review", "status"] - ] - - -@pytest.fixture(autouse=True) -def greptile_on_path() -> Iterator[MagicMock]: - with patch("hogli_commands.review.shutil.which", return_value=_BINARY) as mock_which: - yield mock_which - - -class TestReview: - def test_missing_cli_names_the_install_command(self, greptile_on_path: MagicMock) -> None: - greptile_on_path.return_value = None - result = runner.invoke(cli, ["review"]) - assert result.exit_code == 1 - assert "brew install greptileai/tap/greptile" in result.output - - @patch("hogli_commands.review.subprocess.run") - def test_signed_out_exits_ex_config_without_starting_a_review(self, mock_run: MagicMock) -> None: - mock_run.return_value = _proc(1, stderr="error: not signed in. Set GREPTILE_API_KEY or run `greptile login`.") - result = runner.invoke(cli, ["review"]) - assert result.exit_code == 78 - assert "greptile login" in result.output - # The auth probe must be the only subprocess: a signed-out run may never - # reach the paid review call. - assert [call.args[0] for call in mock_run.call_args_list] == [[_BINARY, "config"]] - - @patch("hogli_commands.review.subprocess.run") - def test_completed_review_is_reprinted_not_rebought(self, mock_run: MagicMock) -> None: - mock_run.side_effect = _fake_greptile(status=0) - result = runner.invoke(cli, ["review"]) - assert result.exit_code == 0 - assert "stored review" in result.output - assert _reviews(mock_run) == [] - - @pytest.mark.parametrize( - "status_exit,review_exit,expected_exit,expected_flag", - [ - # An interrupted review is resumed, not restarted. - (3, 0, 0, "--resume"), - # No prior review: fresh run, and greptile's exit code passes through. - (1, 4, 4, None), - ], - ) - @patch("hogli_commands.review.subprocess.run") - def test_head_status_picks_the_review_call( - self, - mock_run: MagicMock, - status_exit: int, - review_exit: int, - expected_exit: int, - expected_flag: str | None, - ) -> None: - mock_run.side_effect = _fake_greptile(status=status_exit, review_exit=review_exit) - result = runner.invoke(cli, ["review"]) - assert result.exit_code == expected_exit - (review,) = _reviews(mock_run) - if expected_flag is not None: - assert expected_flag in review - - @patch("hogli_commands.review.subprocess.run") - def test_force_skips_the_status_probe(self, mock_run: MagicMock) -> None: - mock_run.side_effect = _fake_greptile() - result = runner.invoke(cli, ["review", "--force"]) - assert result.exit_code == 0 - assert _status_probes(mock_run) == [] - assert len(_reviews(mock_run)) == 1 - - @pytest.mark.parametrize( - "statuses,expected_exit,expected_probes", - [ - ({"tip": 0, "base": 1}, 0, 1), # tip reviewed: pass, and stop probing - ({"tip": 1, "base": 0}, 0, 2), # review ran earlier on the branch: still pass - ({"tip": 3, "base": 1}, 1, 2), # running or absent everywhere: no label - ], - ) - @patch("hogli_commands.review.subprocess.run") - def test_check_passes_when_any_branch_commit_was_reviewed( - self, - mock_run: MagicMock, - statuses: dict[str, int], - expected_exit: int, - expected_probes: int, - ) -> None: - mock_run.side_effect = _fake_greptile(status=statuses) - result = runner.invoke(cli, ["review", "--check"]) - assert result.exit_code == expected_exit - assert len(_status_probes(mock_run)) == expected_probes - # --check must never fall through to a paid review. - assert _reviews(mock_run) == [] - - @patch("hogli_commands.review.subprocess.run") - def test_check_falls_back_to_head_when_rev_list_fails(self, mock_run: MagicMock) -> None: - mock_run.side_effect = _fake_greptile(status=0, rev_list_exit=128) - result = runner.invoke(cli, ["review", "--check"]) - assert result.exit_code == 0 - assert _status_probes(mock_run) == [[_BINARY, "review", "status", "--commit", "HEAD"]] - - @patch("hogli_commands.review.subprocess.run") - def test_check_stops_walking_when_a_probe_hangs(self, mock_run: MagicMock) -> None: - fake = _fake_greptile() - - def fake_run(cmd: list[str], **kwargs: object) -> MagicMock: - if cmd[1:3] == ["review", "status"]: - raise subprocess.TimeoutExpired(cmd, 60) - return fake(cmd, **kwargs) - - mock_run.side_effect = fake_run - result = runner.invoke(cli, ["review", "--check"]) - # One timeout means they would all time out; a single 60s wait, not 20. - assert result.exit_code == 1 - assert len(_status_probes(mock_run)) == 1 - - @patch("hogli_commands.review.subprocess.run") - def test_probe_failure_still_reviews(self, mock_run: MagicMock) -> None: - def fake_run(cmd: list[str], **kwargs: object) -> MagicMock: - if kwargs.get("capture_output"): - raise subprocess.TimeoutExpired(cmd, 60) - return _proc(0) - - mock_run.side_effect = fake_run - result = runner.invoke(cli, ["review"]) - # A hung or broken probe must degrade to a plain review, not block it. - assert result.exit_code == 0 - assert len(_reviews(mock_run)) == 1