Review comment navigator, gate deny split button, and a pinned policy self-deny

app/src/api/gate.ts

@@ -1,7 +1,12 @@
 import type { GateApprovalRequestedData } from "../types";
 import { getApiUrl } from "./api";
 
-export type GateDecision = "once" | "session" | "deny";
+/**
+ * `deny-session` caches the denial under the request's CacheKey for the
+ * container's lifetime, so an agent that retries the same blocked operation
+ * is refused without re-prompting the operator. See Manager.applyResolution.
+ */
+export type GateDecision = "once" | "session" | "deny" | "deny-session";
 
 /** One pending approval as returned by GET /api/gate/approvals. */
 export interface PendingApproval extends GateApprovalRequestedData {

app/src/components/chat/ApprovalCard.tsx

@@ -1,9 +1,10 @@
 import { useEffect, useRef, useState } from "react";
 import { GateApprovalGoneError, type GateDecision } from "../../api/gate";
-import { resolveGateApproval, sendMessage } from "../../api/loopApi";
+import { resolveGateApproval, sendCommand, sendMessage } from "../../api/loopApi";
 import { useTheme } from "../../ThemeContext";
 import { fonts } from "../../theme";
 import type { GateApprovalRequestedData } from "../../types";
+import { ContextMenu, type MenuItem } from "../shared/ContextMenu";
 
 export function ApprovalCard({
   data,
@@ -38,11 +39,24 @@ export function ApprovalCard({
     });
     return () => cancelAnimationFrame(id);
   }, []);
-  const [sending, setSending] = useState<GateDecision | "deny-with-prompt" | null>(null);
+  const [sending, setSending] = useState<GateDecision | "deny-with-prompt" | "deny-and-stop" | null>(null);
   const [error, setError] = useState<string | null>(null);
   const [showPrompt, setShowPrompt] = useState(false);
   const [prompt, setPrompt] = useState("");
 
+  // The deny variants live behind a caret next to Deny rather than as their
+  // own pills: plain Deny is the only non-terminal one and by far the common
+  // choice, and four side-by-side deny buttons made the card read as if they
+  // were unrelated options.
+  const caretRef = useRef<HTMLButtonElement | null>(null);
+  const [menuPos, setMenuPos] = useState<{ x: number; y: number } | null>(null);
+  const openDenyMenu = () => {
+    const el = caretRef.current;
+    if (!el) return;
+    const r = el.getBoundingClientRect();
+    setMenuPos({ x: r.left, y: r.bottom + 2 });
+  };
+
   // Expiry: the gate auto-denies at data.expires_at. Track it locally so the
   // card greys out on time even if the gate.approval_resolved event was
   // missed (WS drop, subscription race), then retract shortly after so a
@@ -87,6 +101,30 @@ export function ApprovalCard({
     }
   };
 
+  // Deny the request and end the run outright, rather than letting the agent
+  // carry on from the denial. The orchestrator's drain loop claims the next
+  // queued message as soon as the cancelled run returns, so this is the way
+  // to abandon what the agent is doing and move on to what's waiting behind
+  // it. Deny lands first so the agent sees a clean tool-denied result while
+  // its container is torn down, matching the deny-with-prompt ordering.
+  const denyAndStop = async () => {
+    setSending("deny-and-stop");
+    setError(null);
+    try {
+      await resolveGateApproval(data.req_id, "deny");
+      await sendCommand(channelId, "stop");
+      onResolved?.();
+    } catch (e) {
+      if (e instanceof GateApprovalGoneError) {
+        setExpired(true);
+        setSending(null);
+        return;
+      }
+      setError(e instanceof Error ? e.message : String(e));
+      setSending(null);
+    }
+  };
+
   const denyWithPrompt = async () => {
     const text = prompt.trim();
     if (!text) return;
@@ -109,6 +147,44 @@ export function ApprovalCard({
     }
   };
 
+  // Each entry re-checks `sending`: the caret is disabled while a decision is
+  // in flight, but one can start between opening the menu and clicking an item
+  // (the card also resolves on a peer's click via gate.approval_resolved).
+  const denyMenuItems: MenuItem[] = [
+    {
+      label: "Deny for session",
+      danger: true,
+      onClick: () => {
+        if (sending === null) void resolve("deny-session");
+      },
+    },
+    // Chat/review only: `/loop stop` cancels the orchestrator run that owns the
+    // message queue. A terminal pane's agent isn't that run (it's a TUI on the
+    // pane's stdin, with nothing queued behind it), and those panes are exactly
+    // the ones that pass onDenyWithPrompt.
+    ...(onDenyWithPrompt
+      ? []
+      : [
+          {
+            label: "Deny & stop run",
+            danger: true,
+            onClick: () => {
+              if (sending === null) void denyAndStop();
+            },
+          },
+        ]),
+    {
+      label: "Deny with prompt…",
+      danger: true,
+      separator: true,
+      onClick: () => {
+        if (sending !== null) return;
+        setShowPrompt(true);
+        setError(null);
+      },
+    },
+  ];
+
   const label = data.kind ? data.kind.toUpperCase() : "APPROVAL";
 
   return (
@@ -155,27 +231,45 @@ export function ApprovalCard({
         <div style={{ display: "flex", gap: 8, flexWrap: "wrap" }}>
           <ApprovalButton label="Allow once" decision="once" busy={sending === "once"} disabled={sending !== null} onClick={resolve} variant="primary" />
           <ApprovalButton label="Allow for session" decision="session" busy={sending === "session"} disabled={sending !== null} onClick={resolve} variant="secondary" />
-          <ApprovalButton label="Deny" decision="deny" busy={sending === "deny"} disabled={sending !== null} onClick={resolve} variant="danger" />
-          <button
-            onClick={() => {
-              setShowPrompt((s) => !s);
-              setError(null);
-            }}
-            disabled={sending !== null}
-            style={{
-              padding: "4px 12px",
-              fontSize: 12,
-              fontFamily: fonts.mono,
-              border: `1px solid ${colors.warning}`,
-              borderRadius: 12,
-              backgroundColor: "transparent",
-              color: colors.warning,
-              cursor: sending !== null ? "default" : "pointer",
-              opacity: sending !== null ? 0.5 : 1,
-            }}
-          >
-            Deny with prompt…
-          </button>
+          {/* Split button: the default action is the plain, non-terminal deny —
+              block this one call and let the agent carry on. The variants that
+              change what happens *after* the denial hang off the caret. */}
+          <div style={{ display: "flex", alignItems: "stretch" }}>
+            <ApprovalButton
+              label="Deny"
+              decision="deny"
+              busy={sending === "deny"}
+              disabled={sending !== null}
+              onClick={resolve}
+              variant="danger"
+              title="Deny this request; the agent keeps going from the denial"
+              style={{ borderTopRightRadius: 0, borderBottomRightRadius: 0, borderRight: "none" }}
+            />
+            <button
+              ref={caretRef}
+              data-testid="approval-deny-caret"
+              onClick={openDenyMenu}
+              disabled={sending !== null}
+              aria-label="More deny options"
+              title="More deny options"
+              style={{
+                padding: "4px 6px",
+                fontSize: 12,
+                fontFamily: fonts.mono,
+                border: `1px solid ${colors.warning}`,
+                borderRadius: 12,
+                borderTopLeftRadius: 0,
+                borderBottomLeftRadius: 0,
+                backgroundColor: colors.warning,
+                color: "#fff",
+                cursor: sending !== null ? "default" : "pointer",
+                opacity: sending !== null ? 0.5 : 1,
+              }}
+            >
+              ▾
+            </button>
+          </div>
+          {sending === "deny-session" || sending === "deny-and-stop" ? <span style={{ alignSelf: "center", fontSize: 12, fontFamily: fonts.mono, color: colors.textDim }}>...</span> : null}
         </div>
       )}
       {!expired && showPrompt && (
@@ -265,6 +359,7 @@ export function ApprovalCard({
           </button>
         </div>
       )}
+      {menuPos && <ContextMenu x={menuPos.x} y={menuPos.y} onClose={() => setMenuPos(null)} items={denyMenuItems} />}
     </div>
   );
 }
@@ -276,13 +371,18 @@ function ApprovalButton({
   disabled,
   onClick,
   variant,
+  title,
+  style,
 }: {
   label: string;
   decision: GateDecision;
   busy: boolean;
   disabled: boolean;
   onClick: (d: GateDecision) => void;
   variant: "primary" | "secondary" | "danger";
+  title?: string;
+  /** Merged last, so a split-button caller can flatten the adjoining corners. */
+  style?: React.CSSProperties;
 }) {
   const { colors } = useTheme();
   const accent = variant === "primary" ? colors.active : variant === "danger" ? colors.warning : colors.border;
@@ -292,6 +392,7 @@ function ApprovalButton({
     <button
       onClick={() => onClick(decision)}
       disabled={disabled}
+      title={title}
       style={{
         padding: "4px 12px",
         fontSize: 12,
@@ -302,6 +403,7 @@ function ApprovalButton({
         color: textColor,
         cursor: disabled ? "default" : "pointer",
         opacity: disabled && !busy ? 0.5 : busy ? 0.7 : 1,
+        ...style,
       }}
     >
       {busy ? "..." : label}

app/src/components/panels/ReviewDiffView.test.ts

@@ -0,0 +1,101 @@
+import { describe, expect, it } from "vitest";
+import type { ReviewComment } from "../../api/review";
+import { parseUnifiedDiff } from "./DiffViewer";
+import { type FileSummary, orderedComments } from "./ReviewDiffView";
+
+const DIFF = `diff --git a/a.go b/a.go
+index 111..222 100644
+--- a/a.go
++++ b/a.go
+@@ -1,3 +1,4 @@
+ package main
+-func old() {}
++func one() {}
++func two() {}
+@@ -20,2 +21,3 @@
+ var x = 1
++var y = 2
+diff --git a/b.go b/b.go
+index 333..444 100644
+--- a/b.go
++++ b/b.go
+@@ -1,2 +1,3 @@
+ package b
++var z = 3
+`;
+
+function comment(id: string, path: string, line: number, extra: Partial<ReviewComment> = {}): ReviewComment {
+  return { id, path, line, side: "RIGHT", body: id, pushed: false, ...extra };
+}
+
+// Mirror what ReviewDiffView builds from the parsed diff, so ordering is
+// exercised against real hunk geometry rather than hand-written line lists.
+function summarize(rawDiff: string, comments: ReviewComment[]) {
+  const parsedFiles = parseUnifiedDiff(rawDiff);
+  const known = new Set(parsedFiles.map((p) => p.path));
+  const byFile = new Map<string, ReviewComment[]>();
+  const orphans: ReviewComment[] = [];
+  for (const c of comments) {
+    if (known.has(c.path)) byFile.set(c.path, [...(byFile.get(c.path) ?? []), c]);
+    else orphans.push(c);
+  }
+  const summaries: FileSummary[] = parsedFiles.map((p) => ({
+    path: p.path,
+    additions: 0,
+    deletions: 0,
+    parsed: p,
+    agentCount: (byFile.get(p.path) ?? []).length,
+    ghCount: 0,
+  }));
+  return { summaries, byFile, orphans };
+}
+
+function ids(rawDiff: string, comments: ReviewComment[]): string[] {
+  const { summaries, byFile, orphans } = summarize(rawDiff, comments);
+  return orderedComments(summaries, byFile, orphans).map((a) => a.id);
+}
+
+describe("orderedComments", () => {
+  it("orders by file, then by the diff line each comment anchors to", () => {
+    // Supplied deliberately out of order: b.go before a.go, and within a.go
+    // the second hunk's line before the first hunk's.
+    const out = ids(DIFF, [comment("b1", "b.go", 2), comment("a-late", "a.go", 22), comment("a-early", "a.go", 2)]);
+    expect(out).toEqual(["a-early", "a-late", "b1"]);
+  });
+
+  it("keeps several comments on one line in array order", () => {
+    const out = ids(DIFF, [comment("first", "a.go", 2), comment("second", "a.go", 2), comment("third", "a.go", 2)]);
+    expect(out).toEqual(["first", "second", "third"]);
+  });
+
+  it("puts out-of-diff comments last, after every file", () => {
+    const out = ids(DIFF, [comment("orphan", "gone.go", 5), comment("a1", "a.go", 2), comment("b1", "b.go", 2)]);
+    expect(out).toEqual(["a1", "b1", "orphan"]);
+  });
+
+  it("drops a comment whose line falls outside every hunk", () => {
+    // Line 900 is in no hunk, so FileSection renders no card for it —
+    // counting it would leave the navigator with a dead target.
+    expect(ids(DIFF, [comment("a1", "a.go", 2), comment("nowhere", "a.go", 900)])).toEqual(["a1"]);
+  });
+
+  it("anchors a LEFT-side comment on the deleted line's old number", () => {
+    // `func old() {}` is old line 2 and has no new number.
+    expect(ids(DIFF, [comment("del", "a.go", 2, { side: "LEFT" })])).toEqual(["del"]);
+    // The same line number on the RIGHT is a different, also-valid anchor.
+    expect(ids(DIFF, [comment("add", "a.go", 2, { side: "RIGHT" })])).toEqual(["add"]);
+  });
+
+  it("carries the file index so navigation can expand the right section", () => {
+    const { summaries, byFile, orphans } = summarize(DIFF, [comment("a1", "a.go", 2), comment("b1", "b.go", 2), comment("orphan", "gone.go", 1)]);
+    expect(orderedComments(summaries, byFile, orphans)).toEqual([
+      { id: "a1", path: "a.go", fileIdx: 0 },
+      { id: "b1", path: "b.go", fileIdx: 1 },
+      { id: "orphan", path: "gone.go", fileIdx: -1 },
+    ]);
+  });
+
+  it("returns nothing when there are no comments", () => {
+    expect(ids(DIFF, [])).toEqual([]);
+  });
+});