fix(gate): address P1-P3 audit findings (May 13 2026)

gate/fixer.py

@@ -1014,7 +1014,9 @@ def _loop() -> None:
     def run(self) -> FixResult:
         """Execute the fix pipeline."""
         if self._cancelled.is_set():
-            return FixResult(success=False, summary="Cancelled before start")
+            return FixResult(
+                success=False, skipped=True, summary="Cancelled before start"
+            )
 
         self._fix_start_monotonic = time.monotonic()
         self._start_watchdog()
@@ -1046,15 +1048,21 @@ def run(self) -> FixResult:
         )
         finding_count = len(actionable)
 
-        # Check fix attempt limits
+        # Check fix attempt limits — cooldown / soft / lifetime caps
+        # are policy decisions, not failures, so the result is marked
+        # ``skipped`` so reviews.jsonl can distinguish "we chose not to
+        # try" from "we tried and failed". ``log_fix_result`` maps that
+        # to ``decision: "fix_skipped"`` (audit P3.1, May 13).
         allowed, reason = state.check_fix_limits(self.pr_number, self.config, repo=self.repo)
         if not allowed:
             github.comment_pr(
                 self.repo,
                 self.pr_number,
-                f"**Gate Auto-Fix: Attempt limit reached** — {reason}",
+                f"**Gate Auto-Fix: skipped** — {reason}",
+            )
+            return FixResult(
+                success=False, skipped=True, reason=reason, summary=reason
             )
-            return FixResult(success=False, reason=reason, summary=reason)
 
         triage = self._read_json("triage.json") or {}
         risk_level = triage.get("risk_level", "medium")
@@ -1140,7 +1148,9 @@ def run(self) -> FixResult:
 
             for iteration in range(1, MAX_ITERATIONS + 1):
                 if self._cancelled.is_set():
-                    return FixResult(success=False, summary="Cancelled")
+                    return FixResult(
+                        success=False, skipped=True, summary="Cancelled"
+                    )
 
                 write_live_log(
                     self.pr_number,
@@ -1290,7 +1300,12 @@ def run(self) -> FixResult:
 
         except FileNotFoundError as e:
             logger.warning(f"Fix pipeline aborted (workspace deleted): {e}")
-            return FixResult(success=False, summary="Workspace deleted (cancelled)")
+            # Worktree teardown is the cancellation cascade — not a
+            # real failure. Mark skipped so reviews.jsonl distinguishes
+            # this from iteration-exhausted / crash failures.
+            return FixResult(
+                success=False, skipped=True, summary="Workspace deleted (cancelled)"
+            )
         except Exception as e:
             logger.exception(f"Fix pipeline failed for PR #{self.pr_number}")
             notify.fix_failed(self.pr_number, str(e), 0, self.repo)
@@ -1607,10 +1622,15 @@ def _run_rereview(self) -> bool:
 
         result = run_with_retry(
             lambda: StructuredRunner().run(
-                "fix-rereview", assembled, self.workspace, self.config
+                "fix-rereview",
+                assembled,
+                self.workspace,
+                self.config,
+                cancelled=self._cancelled,
             ),
             "fix-rereview",
             self.config,
+            cancelled=self._cancelled,
         )
 
         # Write result

gate/fixer_polish.py

@@ -511,10 +511,16 @@ def _run_fix_polish_audit(pipeline: "FixPipeline") -> dict | None:
 
     def _run() -> StageResult:
         return StructuredRunner().run(
-            "fix-polish", assembled, pipeline.workspace, pipeline.config
+            "fix-polish",
+            assembled,
+            pipeline.workspace,
+            pipeline.config,
+            cancelled=pipeline._cancelled,
         )
 
-    result = run_with_retry(_run, "fix-polish", pipeline.config)
+    result = run_with_retry(
+        _run, "fix-polish", pipeline.config, cancelled=pipeline._cancelled
+    )
     data = result.data or {}
     (pipeline.workspace / "fix-polish.json").write_text(json.dumps(data, indent=2))
     clean = data.get("clean", True)

gate/github.py

@@ -463,13 +463,31 @@ def _build_comment(verdict: dict, build: dict | None) -> str:
     return md
 
 
+def _is_own_pr(pr_author: str, config: dict | None) -> bool:
+    """True when the PR was opened by the configured bot account.
+
+    Used by ``post_review`` / ``approve_pr`` to short-circuit the
+    ``gh pr review`` round-trip that GitHub rejects with
+    ``GraphQL: Review Can not approve / request changes on your own
+    pull request``. The existing stderr-fallback (parse the error,
+    re-post as a comment) still acts as belt-and-suspenders for the
+    case where ``bot_account`` is misconfigured or ``pr_author`` is
+    empty (e.g. health.py orphan-cleanup paths).
+    """
+    if not pr_author or not config:
+        return False
+    bot = (config.get("repo") or {}).get("bot_account") or ""
+    return bool(bot) and pr_author.lower() == bot.lower()
+
+
 def post_review(
     repo: str,
     pr_number: int,
     verdict: dict,
     build: dict | None,
     sha: str,
     config: dict | None = None,
+    pr_author: str = "",
 ) -> None:
     """Post a review to the PR. Always enforcement mode.
 
@@ -478,12 +496,29 @@ def post_review(
     Also upserts a sticky human-readable summary comment (separate from
     the GitHub review object) so the team always has one canonical
     place to read Gate's verdict for this PR.
+
+    ``pr_author`` (when supplied) plus ``config["repo"]["bot_account"]``
+    let us detect bot-authored PRs up front and route directly to
+    ``comment_pr`` instead of making the ``gh pr review`` call that
+    GitHub rejects for self-reviews. The legacy stderr fallback is
+    preserved for the misconfigured / unknown-author case.
     """
     comment = _build_comment(verdict, build)
     decision = verdict.get("decision", "approve")
     findings = verdict.get("findings", [])
 
-    if decision in ("approve", "approve_with_notes"):
+    own_pr = _is_own_pr(pr_author, config)
+
+    if own_pr:
+        # Bot opened this PR — GitHub forbids self-reviews. Post the
+        # verdict as a comment instead of trying (and failing) the
+        # ``gh pr review`` call. No WARNING, no wasted round-trip, no
+        # race with the sticky-comment update.
+        comment_pr(repo, pr_number, comment)
+        logger.info(
+            f"PR #{pr_number}: bot-authored PR, posted review as comment ({decision})"
+        )
+    elif decision in ("approve", "approve_with_notes"):
         try:
             _gh(["pr", "review", str(pr_number), "--repo", repo, "--approve", "--body", comment])
             logger.info(f"PR #{pr_number} approved ({decision})")
@@ -509,6 +544,11 @@ def post_review(
             else:
                 raise
 
+    # Escalation runs for any non-approve verdict regardless of who
+    # opened the PR — humans still need to be paged when there's a
+    # critical finding or low-confidence verdict, and the label /
+    # reviewer assignment paths work the same on bot-authored PRs.
+    if decision not in ("approve", "approve_with_notes"):
         has_critical = any(
             f.get("severity") == "critical" and f.get("introduced_by_pr") is not False
             for f in findings
@@ -631,8 +671,28 @@ def complete_check_run(
 # ── Simple PR Operations ────────────────────────────────────
 
 
-def approve_pr(repo: str, pr_number: int, body: str) -> None:
-    """Approve a PR, falling back to a comment if we own the PR."""
+def approve_pr(
+    repo: str,
+    pr_number: int,
+    body: str,
+    pr_author: str = "",
+    config: dict | None = None,
+) -> None:
+    """Approve a PR, falling back to a comment if we own the PR.
+
+    ``pr_author`` + ``config["repo"]["bot_account"]`` let us detect
+    bot-authored PRs and route directly to ``comment_pr`` without
+    incurring the failing ``gh pr review --approve`` round-trip. The
+    legacy stderr fallback below is retained as belt-and-suspenders
+    for the case where one or both args are unavailable (e.g.
+    health.py orphan-cleanup paths that don't have a config in hand).
+    """
+    if _is_own_pr(pr_author, config):
+        comment_pr(repo, pr_number, body)
+        logger.info(
+            f"PR #{pr_number}: bot-authored PR, posted approval as comment"
+        )
+        return
     try:
         _gh(["pr", "review", str(pr_number), "--repo", repo, "--approve", "--body", body])
     except subprocess.CalledProcessError as e:

gate/health.py

@@ -376,9 +376,19 @@ def _process_marker(pr_dir: Path, default_repo: str, label: str) -> None:
                 )
             try:
                 pr_number = int(pr_num)
+                # ``pr_author`` is intentionally omitted here: the
+                # active_review.json marker does not persist it (the
+                # orchestrator writes the marker before fetching PR
+                # info), and this orphan-cleanup path is a fail-open
+                # recovery that runs at most a few times per day. The
+                # stderr-fallback in ``approve_pr`` still handles bot
+                # PRs correctly — it just costs one extra GraphQL
+                # round-trip and a WARNING log line per orphaned bot
+                # PR, which is acceptable for a recovery path.
                 github.approve_pr(
                     repo, pr_number,
                     "**Gate (error)** — review process died. Auto-approving.",
+                    config=config,
                 )
             except ValueError:
                 pass

gate/logger.py

@@ -162,13 +162,23 @@ def log_fix_result(
 ) -> None:
     """Append a fix result entry to reviews.jsonl.
 
-    ``status`` may be one of ``"succeeded"``, ``"failed"``, or ``"no_op"``.
-    When not provided it is derived from ``fix_success`` (legacy callers
-    that have not been updated yet). ``no_op`` lets log consumers
-    distinguish "the fix pipeline intentionally did nothing" (e.g. a
-    graceful no-op on an approve_with_notes PR with no mechanical work)
-    from "the fix pipeline landed commits" and from "the fix pipeline
-    failed" (audit A10).
+    ``status`` may be one of ``"succeeded"``, ``"failed"``, ``"no_op"``,
+    or ``"skipped"``. When not provided it is derived from
+    ``fix_success`` (legacy callers that have not been updated yet).
+
+    The four statuses let log consumers distinguish:
+
+    - ``no_op`` — the pipeline intentionally did nothing (e.g. graceful
+      no-op on an approve_with_notes PR with no mechanical work)
+    - ``succeeded`` — the pipeline landed commits
+    - ``failed`` — the pipeline attempted real work and could not
+      produce a valid diff (iteration exhaustion, crash)
+    - ``skipped`` — the pipeline short-circuited before attempting any
+      work because of cancellation (supersede / operator cancel /
+      workspace teardown) or policy (cooldown, soft / lifetime limit).
+      ``fix_skipped`` is excluded from the success-rate denominator in
+      ``gate.reports`` the same way ``fix_no_op`` is, since neither
+      represents an actual fix attempt with an outcome.
 
     Hopper-mode kwargs (``pipeline_mode``, ``sub_scope_*``,
     ``wall_clock_seconds``, ``runaway_guard_hit``, ``fixed_count``,
@@ -177,12 +187,13 @@ def log_fix_result(
     """
     if status is None:
         status = "succeeded" if fix_success else "failed"
-    if status == "no_op":
-        decision = "fix_no_op"
-    elif status == "succeeded":
-        decision = "fix_succeeded"
-    else:
-        decision = "fix_failed"
+    decision_map = {
+        "succeeded": "fix_succeeded",
+        "no_op": "fix_no_op",
+        "skipped": "fix_skipped",
+        "failed": "fix_failed",
+    }
+    decision = decision_map.get(status, "fix_failed")
     entry = {
         "timestamp": datetime.now(timezone.utc).isoformat(),
         "repo": repo,