fix(review): post fresh comment on re-review instead of silent PATCH

CLAUDE.md

@@ -160,7 +160,7 @@ Config additions in `config.py`: `is_api_enabled()`, `get_api_host()` (default `
 Extensible command plugin system. Each skill lives in `skills/<scope>/<skill-name>/` with a `SKILL.md` (YAML frontmatter defining commands, aliases, metadata) and an optional `handler.py`.
 
 - **`skills.py`** — Registry that discovers SKILL.md files, parses frontmatter (custom lite YAML parser, no PyYAML), maps commands/aliases to skills, and dispatches execution.
-- **Core skills** live in `koan/skills/core/` (abort, add_project, ai, alias, ask, audit, autoreview, brainstorm, branches, cancel, changelog, chat, check, check_need, check_notifications, checkup, ci_check, claudemd, config_check, dead_code, deepplan, delete_project, diagnose, doc, doctor, done, email, explore, fix, focus, gh_request, gha_audit, idea, implement, inbox, incident, journal, language, list, live, logs, magic, mission, models, passive, plan, plan_implement, pr, priority, private_security_audit, profile, projects, quota, rebase, recreate, recurring, refactor, reflect, rename, rescan, restart, review, review_rebase, rtk, scaffold_skill, security_audit, shutdown, snapshot, sparring, spec_audit, squash, stats, status, tech_debt, tracker, verbose)
+- **Core skills** live in `koan/skills/core/` (abort, add_project, ai, alias, ask, audit, autoreview, brainstorm, branches, cancel, changelog, chat, check, check_need, check_notifications, checkup, ci_check, claudemd, config_check, dead_code, deepplan, delete_project, diagnose, doc, doctor, done, email, explain, explore, fix, focus, gh_request, gha_audit, idea, implement, inbox, incident, journal, language, list, live, logs, magic, mission, models, passive, plan, plan_implement, pr, priority, private_security_audit, profile, projects, quota, rebase, recreate, recurring, refactor, reflect, rename, rescan, restart, review, review_rebase, rtk, scaffold_skill, security_audit, shutdown, snapshot, sparring, spec_audit, squash, stats, status, tech_debt, tracker, verbose)
 - **Custom skills** loaded from `instance/skills/<scope>/` — each scope directory can be a cloned Git repo for team sharing.
 - **Handler pattern**: `def handle(ctx: SkillContext) -> Optional[str]` — return string for Telegram reply, empty string for "already handled", None for no message.
 - **`worker: true`** flag in SKILL.md marks blocking skills (Claude calls, API requests) that run in a background thread.

docs/architecture/providers.md

@@ -11,6 +11,9 @@ Providers are responsible for:
 - resolving the executable and authentication assumptions;
 - mapping Koan tool permissions to provider-specific flags;
 - building commands for print or streaming execution;
+- declaring how prompts can be moved from argv to stdin;
+- declaring whether invocations must be serialized to protect shared provider
+  state such as rotating auth tokens;
 - normalizing output handling enough for mission execution code;
 - exposing provider capabilities without leaking provider details into unrelated
   modules.

docs/providers/codex.md

@@ -113,6 +113,16 @@ Koan tracks token usage from Codex JSON output when available. This internal
 estimate drives autonomous mode downgrades (`deep` -> `implement` -> `review`
 -> `wait`) but is separate from hard provider quota detection.
 
+Recognized Codex JSONL usage events are:
+
+- `turn.completed` events with a `usage` object.
+- `event_msg` rollout events where `payload.type` is `token_count` and
+  `payload.info.total_token_usage` contains the token counters.
+
+In both formats, Koan treats `cached_input_tokens` as cache-read tokens and
+subtracts them from counted input tokens before updating the internal budget.
+When multiple usage-bearing events are present, the latest snapshot wins.
+
 For Codex subscription accounts where you want to ignore internal estimates and
 only react to real provider quota/session-limit errors, set:
 
@@ -172,6 +182,24 @@ Install the CLI: `npm install -g @openai/codex`
 
 Re-authenticate: `codex login --device-auth`
 
+When debugging Kōan, test the same non-interactive path Kōan uses. A short
+plain command like `codex exec 'say hello'` can succeed while the daemon path
+fails during JSON streaming, stdin prompt passing, or final-message capture:
+
+```bash
+printf 'say hello' | codex exec --json --output-last-message /tmp/koan-codex-last-message --sandbox workspace-write -
+```
+
+If Kōan runs as a background service, also verify that the service user has the
+same `HOME`, `PATH`, `.codex/auth.json`, and `.env` values as your interactive
+shell. Restart Kōan after re-authenticating so the daemon uses the refreshed
+Codex auth state.
+
+Kōan serializes its own Codex CLI subprocesses to avoid concurrent token-refresh
+races between preflight probes, message formatting, and long-running review
+sessions. If Codex still reports `401 Unauthorized` or refresh-token reuse, Kōan
+will pause for auth and requeue the active mission instead of marking it failed.
+
 ### Rate limits
 
 Codex shares quota with your ChatGPT subscription. If you hit limits,

docs/users/skills.md

@@ -37,6 +37,7 @@ Complete reference for all Koan slash commands. Use these via Telegram, Slack, o
 | `/implement <issue>` | `/impl` | Queue implementation for a GitHub or Jira issue; never bails — resolves ambiguity with simplest viable solution, retries once before surfacing a problem | Yes |
 | `/fix <issue>` | — | Understand → plan → test → implement → submit PR | Yes |
 | `/review <PR>` | `/rv` | Review a pull request | Yes |
+| `/explain <PR>` | `/xp` | Explain a PR's changes in plain language with examples and alternative approaches | Yes |
 | `/rebase <PR>` | `/rb` | Rebase a PR onto its base branch | Yes |
 | `/planimplement <issue>` | `/planimp`, `/planimpl`, `/planit`, `/plandoit` | Plan then implement an issue (combo: /plan → /implement) | Yes |
 | `/recreate <PR>` | `/rc` | Re-implement a PR from scratch on a fresh branch | Yes |

docs/users/user-manual.md

@@ -514,6 +514,21 @@ Use this before `/plan` when the idea is architecturally complex, when you want
 - `/review https://github.com/org/repo/pull/55 --architecture --errors` — Both passes
 </details>
 
+**`/explain`** — Explain a PR's intent and changes in plain, simple language.
+
+- **Usage:** `/explain <github-pr-url>`
+- **Aliases:** `/xp`
+- **GitHub @mention:** `@koan-bot /explain` on a PR
+
+Produces a pedagogical walkthrough of the PR: what problem it solves (with examples), how the fix works step-by-step, the data flow after the change, and whether a simpler approach could have worked.
+
+<details>
+<summary>Use cases</summary>
+
+- `/explain https://github.com/org/repo/pull/55` — Get a plain-language explanation
+- `/xp https://github.com/org/repo/pull/55` — Same thing, shorter
+</details>
+
 **`/refactor`** — Queue a targeted refactoring mission.
 
 - **Usage:** `/refactor <github-url-or-path>`
@@ -661,6 +676,15 @@ review_dispatch:
   cooldown_minutes: 30     # Min minutes between checks per project (default: 30)
 ```
 
+To prevent the bot from replying to itself in review comment threads (and to cap thread depth), configure:
+
+```yaml
+review_reply:
+  max_thread_depth: 5      # Stop replying after this many comments per thread (default: 5)
+```
+
+When the bot is the last poster in a thread and no human has followed up, the thread is excluded from future replies. This prevents self-reply loops where repeated `/review` runs keep adding replies to the same comment.
+
 <details>
 <summary>Use cases</summary>
 
@@ -1877,6 +1901,7 @@ All commands at a glance. **Tier:** B = Beginner, I = Intermediate, P = Power Us
 | `/implement <issue>` | `/impl` | I | Implement a GitHub or Jira issue |
 | `/fix <issue>` | — | I | Full bug-fix pipeline (understand → plan → test → fix → PR) |
 | `/review <PR> [--architecture] [--errors]` | `/rv` | I | Review a pull request |
+| `/explain <PR>` | `/xp` | I | Explain a PR in plain language with examples |
 | `/refactor <desc>` | `/rf` | I | Targeted refactoring mission |
 | `/ask <comment-url>` | — | I | Ask a question about a PR/issue — posts AI reply to GitHub |
 | `/rebase <PR>` | `/rb` | I | Rebase a PR onto its base branch |

instance.example/config.yaml

@@ -262,6 +262,13 @@ skill_timeout: 7200
 #   enabled: false                # Master switch (default: false)
 #   cooldown_minutes: 30          # Min minutes between checks per project (default: 30)
 
+# Review reply guard — prevents the bot from replying to itself in PR
+# comment threads and caps the total depth of any single thread.
+# When the bot is the last poster in a thread and no human followed up,
+# the thread is excluded from the repliable list on the next review run.
+# review_reply:
+#   max_thread_depth: 5           # Stop replying after this many comments in a thread (default: 5)
+
 # Contemplative mode trigger chance (0-100%)
 # When no mission is pending, this is the probability of running a reflective
 # session instead of autonomous work. Allows regular moments of introspection

koan/app/claude_step.py

@@ -8,6 +8,7 @@
 
 import json
 import logging
+import os
 import re
 import shlex
 import subprocess
@@ -19,10 +20,12 @@
 from app.cli_exec import popen_cli, stream_with_timeout
 from app.cli_provider import build_full_command, run_command
 from app.config import get_model_config
+from app.git_utils import GitCommandError
 from app.git_utils import get_current_branch as _git_utils_get_current_branch
 from app.git_utils import ordered_remotes, run_git_strict
 from app.github import pr_create, run_gh, sanitize_github_comment
 from app.prompts import load_prompt_or_skill
+from app.run_log import log_safe
 
 
 class StepResult:
@@ -366,6 +369,87 @@ def run_claude(
     }
 
 
+def _precommit_hook_path(repo_path: str) -> Optional[Path]:
+    """Return the path to an executable pre-commit hook, or ``None``.
+
+    Checks the standard git hook location plus Husky (the common JS toolchain
+    layout). Only files that exist and are executable count.
+    """
+    candidates = [
+        Path(repo_path) / ".git" / "hooks" / "pre-commit",
+        Path(repo_path) / ".husky" / "pre-commit",
+    ]
+    for path in candidates:
+        try:
+            if path.is_file() and os.access(path, os.X_OK):
+                return path
+        except OSError:
+            continue
+    return None
+
+
+def is_hook_rejection(exc: GitCommandError, repo_path: str) -> bool:
+    """Heuristically decide whether *exc* came from a pre-commit hook objecting.
+
+    git reserves exit code 128 for its *own* fatal errors (bad ref, lock
+    contention, etc.); a hook rejection propagates the hook's own exit code
+    (typically 1/2). git's "nothing to commit" is the one common exit-1
+    false-positive, so it is filtered out. Finally, an executable pre-commit
+    hook must actually be present. Together these separate "the hook ran and
+    said no" from "git itself failed".
+    """
+    if exc.returncode == 128:
+        return False
+    if "nothing to commit" in (exc.stderr or ""):
+        return False
+    return _precommit_hook_path(repo_path) is not None
+
+
+def _commit_with_hook_fallback(commit_args: list, cwd: str, run_git=None) -> None:
+    """Commit, attempting target-repo pre-commit hooks first.
+
+    Project pre-commit hooks (lint/format/test) can exceed the git timeout on
+    first run — a cold env install (nvm/node) easily outlasts the default. A
+    blanket ``--no-verify`` would never let hooks run; an unguarded hooked
+    commit crashes the whole pipeline on timeout.
+
+    The two failure modes are distinct and handled differently:
+
+    * **Timeout** (``subprocess.TimeoutExpired``) — the hook *hung* (e.g. a
+      watch-mode test runner, or a cold env install). Retry with ``--no-verify``
+      so the pipeline makes progress; CI remains the real gate.
+    * **Fast non-zero exit** (:class:`GitCommandError`) — when this is a hook
+      *rejection* (see :func:`is_hook_rejection`), the hook evaluated quickly and
+      objected. Surface its output and re-raise — do *not* bypass it. Genuine
+      git errors (exit 128, etc.) also re-raise unchanged.
+
+    ``run_git`` defaults to :func:`_run_git`; callers may pass their own
+    module-level reference so patches in tests resolve correctly.
+    """
+    runner = run_git or _run_git
+    try:
+        runner(["git", "commit", *commit_args], cwd=cwd, timeout=180)
+    except subprocess.TimeoutExpired:
+        # Hook hung past the budget — bypass it so the pipeline can proceed.
+        runner(["git", "commit", "--no-verify", *commit_args], cwd=cwd)
+    except GitCommandError as exc:
+        if is_hook_rejection(exc, cwd):
+            # Hook ran, evaluated quickly, and objected — respect it.
+            log_safe(
+                "git",
+                f"pre-commit hook rejected the commit (exit {exc.returncode}): "
+                f"{(exc.stderr or '').strip()[:200]}",
+            )
+        raise
+    except RuntimeError as exc:
+        # A non-GitCommandError runner (e.g. a test double) that reports a
+        # timeout in its message still means the hook hung — bypass it.
+        if "timed out" in str(exc).lower():
+            runner(["git", "commit", "--no-verify", *commit_args], cwd=cwd)
+            return
+        raise
+
+
 def commit_if_changes(project_path: str, message: str) -> bool:
     """Stage all changes and commit if there are any.
 
@@ -380,7 +464,7 @@ def commit_if_changes(project_path: str, message: str) -> bool:
         return False
 
     _run_git(["git", "add", "-A"], cwd=project_path)
-    _run_git(["git", "commit", "-m", message], cwd=project_path)
+    _commit_with_hook_fallback(["-m", message], project_path, _run_git)
     return True
 
 

koan/app/claudemd_refresh.py

@@ -22,6 +22,7 @@
 from datetime import datetime, timezone
 from pathlib import Path
 
+from app.claude_step import _commit_with_hook_fallback
 from app.git_sync import run_git
 from app.git_utils import run_git_strict
 from app.prompts import load_skill_prompt
@@ -126,7 +127,12 @@ def _commit_and_push(project_path: str, branch_name: str, message: str) -> bool:
     if not _has_changes(project_path):
         return False
     run_git_strict("add", "CLAUDE.md", cwd=project_path)
-    run_git_strict("commit", "-m", message, cwd=project_path)
+
+    def _runner(cmd, cwd=None, timeout=60):
+        # Adapt _run_git convention (cmd list) to run_git_strict (*args).
+        return run_git_strict(*cmd[1:], cwd=cwd, timeout=timeout)
+
+    _commit_with_hook_fallback(["-m", message], project_path, _runner)
     run_git_strict("push", "-u", "origin", branch_name, cwd=project_path)
     return True
 
@@ -140,6 +146,9 @@ def _create_pr(
     """Create a draft PR for the CLAUDE.md update. Returns the PR URL."""
     from app.github import pr_create, resolve_target_repo
 
+    from app.pr_footer import build_koan_footer
+
+    footer = build_koan_footer()
     if mode == "INIT":
         title = f"docs: create CLAUDE.md for {project_name}"
         body = (
@@ -148,7 +157,7 @@ def _create_pr(
             "## Why\n"
             "No CLAUDE.md existed — this bootstraps the reference document "
             "for AI coding assistants working on this project.\n\n"
-            "---\n_Generated by Kōan `/claudemd`_"
+            f"---\n{footer}"
         )
     else:
         title = f"docs: update CLAUDE.md for {project_name}"
@@ -158,7 +167,7 @@ def _create_pr(
             "## Why\n"
             "Architecturally significant changes landed since the last update. "
             "This keeps the AI reference document in sync with the codebase.\n\n"
-            "---\n_Generated by Kōan `/claudemd`_"
+            f"---\n{footer}"
         )
 
     pr_kwargs = {

koan/app/cli_errors.py

@@ -13,8 +13,8 @@
 """
 
 import re
+import sys
 from enum import Enum
-from typing import Optional
 
 
 class ErrorCategory(Enum):
@@ -79,6 +79,47 @@ class ErrorCategory(Enum):
 _TERMINAL_RE = re.compile("|".join(_TERMINAL_PATTERNS), re.IGNORECASE)
 
 
+def _detect_auth_for_provider(
+    *,
+    stdout_text: str,
+    stderr_text: str,
+    provider_name: str,
+    exit_code: int,
+) -> bool:
+    """Delegate provider-specific auth detection to the provider object.
+
+    Mirrors ``quota_handler._detect_quota_for_provider``: resolve via the public
+    ``get_provider_by_name`` API (which normalizes the name and raises a clean
+    ``KeyError`` on unknown names), and degrade conservatively to ``False`` on
+    any failure. ``classify_cli_error`` runs on some unwrapped call paths, so a
+    provider-side bug must never crash the caller — it is logged and swallowed.
+    """
+    if not provider_name:
+        return False
+    try:
+        from app.provider import get_provider_by_name
+
+        provider = get_provider_by_name(provider_name)
+        return provider.detect_auth_failure(
+            stdout_text=stdout_text,
+            stderr_text=stderr_text,
+            exit_code=exit_code,
+        )
+    except KeyError as e:
+        print(
+            f"[cli_errors] unknown provider {provider_name!r}: {e}",
+            file=sys.stderr,
+        )
+        return False
+    except Exception as e:
+        print(
+            f"[cli_errors] provider auth detector failed "
+            f"for {provider_name!r}: {e}",
+            file=sys.stderr,
+        )
+        return False
+
+
 def classify_cli_error(
     exit_code: int,
     stdout: str = "",
@@ -128,6 +169,14 @@ def classify_cli_error(
     if _AUTH_RE.search(combined):
         return ErrorCategory.AUTH
 
+    if _detect_auth_for_provider(
+        stdout_text=stdout,
+        stderr_text=stderr,
+        provider_name=provider_name,
+        exit_code=exit_code,
+    ):
+        return ErrorCategory.AUTH
+
     # Terminal errors — don't retry
     if _TERMINAL_RE.search(combined):
         return ErrorCategory.TERMINAL