feat: Commit Review, micro AI reviews in the Changes panel (v3.7.0)

ROADMAP.md

@@ -16,7 +16,7 @@ _Inspired by [git-lrc](https://github.com/HexmosTech/git-lrc) (HexmosTech). Comm
 
 - **Review staged changes** — one button in the commit area: AI pass over the staged diff, inline findings with severity badges anchored in the diff + a short summary. Generalize `usePrHunkCritique` from PR hunks to any `GitDiff` — the same engine as the v3.5.0 pre-review pass, pointed at the index
 - **Issue navigation** — cycle finding-to-finding (reuses the v3.5.0 keyboard model), per-file finding counts in the staged list
-- **Fix with agent** — git-lrc makes you copy-paste issues back to your agent; we pipe them: "Fix with agent" sends the findings to Claude Code / opencode / Codex (Agent Sessions), optionally in an AI-task scratch worktree; re-review triggers on the next staging change
+- **Fix with agent** — git-lrc makes you copy-paste issues back to your agent; we pipe them: "Fix with agent" sends the findings to Claude Code / opencode / Codex (Agent Sessions), always against the current repo; re-review triggers on the next staging change. The originally-planned "optionally in an AI-task scratch worktree" variant was cut from PR2 after manual QA against real claude/codex CLIs found a brand-new scratch worktree always hits a first-run "trust this directory?" onboarding screen that misinterprets the piped prompt as menu navigation (drove a real `brew upgrade --cask codex` in testing). Revisit once there's a real fix — pre-trusting the directory before launching the agent, or detecting the onboarding screen before writing — tracked as a v3.7.x/v3.8.0 follow-up
 - **Iterations & coverage** — track review→fix→review cycles and the share of the final staged diff already reviewed (`iter:N`, `coverage:X%`)
 - **Review / Vouch / Skip** — explicit three-state decision at commit time, non-blocking (same UX contract as the v3.5.0 secrets scanner): reviewed by AI, vouched personally, or skipped — recorded as a commit trailer `GitWand-Review: ran|vouched|skipped (iter:N, coverage:X%)` via the existing trailers support (v1.9.0), so the team sees review status right in `git log`
 - **Opt-in & scoped** — per-repo enable in `.gitwandrc` + Settings; optional pre-commit hook wiring via Settings > Hooks alongside the v3.5.0 scanner

apps/desktop/dev-server.mjs

@@ -4715,13 +4715,13 @@ async function handleRequest(req, res) {
         // 1. .gitwandrc
         const rcPath = join(base, ".gitwandrc");
         if (existsSync(rcPath)) {
-          return res.writeHead(200, { "Content-Type": "text/plain" }).end(readFileSync(rcPath, "utf-8"));
+          return res.writeHead(200, { ...corsHeaders(req), "Content-Type": "text/plain" }).end(readFileSync(rcPath, "utf-8"));
         }
 
         // 2. .gitwandrc.json
         const rcJsonPath = join(base, ".gitwandrc.json");
         if (existsSync(rcJsonPath)) {
-          return res.writeHead(200, { "Content-Type": "text/plain" }).end(readFileSync(rcJsonPath, "utf-8"));
+          return res.writeHead(200, { ...corsHeaders(req), "Content-Type": "text/plain" }).end(readFileSync(rcJsonPath, "utf-8"));
         }
 
         // 3. "gitwand" key in package.json
@@ -4730,13 +4730,13 @@ async function handleRequest(req, res) {
           try {
             const pkg = JSON.parse(readFileSync(pkgPath, "utf-8"));
             if (pkg.gitwand) {
-              return res.writeHead(200, { "Content-Type": "text/plain" }).end(JSON.stringify(pkg.gitwand));
+              return res.writeHead(200, { ...corsHeaders(req), "Content-Type": "text/plain" }).end(JSON.stringify(pkg.gitwand));
             }
           } catch { /* ignore parse errors */ }
         }
 
         // Not found — return empty string (same as Rust backend)
-        return res.writeHead(200, { "Content-Type": "text/plain" }).end("");
+        return res.writeHead(200, { ...corsHeaders(req), "Content-Type": "text/plain" }).end("");
       } catch (err) {
         return jsonResponse(req, res, { error: err.stderr?.toString() || err.message }, 500);
       }

apps/desktop/src/components/BaseModal.vue

@@ -1,6 +1,7 @@
 <script setup lang="ts">
-import { computed, onMounted, onUnmounted } from "vue";
+import { computed, onMounted, onUnmounted, ref } from "vue";
 import { useI18n } from "../composables/useI18n";
+import { focusableWithin, nextTrapTarget } from "../utils/focusTrap";
 
 type ModalSize = "sm" | "md" | "lg" | "xl" | "full" | "2x";
 type ModalPosition = "center" | "top";
@@ -29,6 +30,23 @@ const props = withDefaults(
     bodyFlush?: boolean;
     /** If true, the body is not scrollable; child content owns its own scrolling. */
     scrollOwn?: boolean;
+    /**
+     * v3.7.0 review-round fix (finding #11): trap Tab/Shift+Tab focus
+     * inside the panel, wrapping at the ends. Escape hatch for a modal that
+     * embeds something that must own its own Tab handling in a way the
+     * generic trap can't anticipate.
+     */
+    trapFocus?: boolean;
+    /**
+     * v3.7.0 review-round fix (finding #11): focus the panel itself on
+     * mount. Set to `false` for a modal that already self-focuses one of
+     * its own inputs from its own `onMounted` (that self-focus runs AFTER
+     * this component's synchronous `onMounted`, Vue fires a child's
+     * `mounted` before its parent's, so leaving `autoFocus` at its
+     * default `true` would still be harmless, but the prop exists so a
+     * modal can opt out explicitly and document its own focus target).
+     */
+    autoFocus?: boolean;
   }>(),
   {
     title: "",
@@ -42,6 +60,8 @@ const props = withDefaults(
     hideClose: false,
     bodyFlush: false,
     scrollOwn: false,
+    trapFocus: true,
+    autoFocus: true,
   }
 );
 
@@ -70,12 +90,64 @@ function onKeyDown(e: KeyboardEvent) {
   }
 }
 
+// v3.7.0 review-round fix (finding #11): focus trap, initial focus, focus
+// restore. See docs/superpowers/plans/2026-08-19-v3.7.0-commit-review-review-fixes.md,
+// Task 11, for the full design rationale (nested modals, CodeMirror/xterm).
+const panelRef = ref<HTMLElement | null>(null);
+let previouslyFocused: HTMLElement | null = null;
+
+/**
+ * Bubble-phase `keydown` listener on the PANEL element (not `window`, unlike
+ * the Escape handler above): nested modals (e.g. `askConfirm` layered over
+ * another modal) each get their own trap, and only the one that actually
+ * contains focus reacts. An inner component that owns Tab itself
+ * (CodeMirror indent, an xterm terminal) runs first in the bubble phase, so
+ * this bails on `e.defaultPrevented`.
+ */
+function onPanelKeyDown(e: KeyboardEvent) {
+  if (e.key !== "Tab" || !props.trapFocus || e.defaultPrevented) return;
+  const panel = panelRef.value;
+  if (!panel) return;
+
+  const focusables = focusableWithin(panel);
+  if (focusables.length === 0) {
+    // Nothing to wrap between. Keep focus pinned on the panel itself.
+    e.preventDefault();
+    panel.focus();
+    return;
+  }
+
+  const target = nextTrapTarget(focusables, document.activeElement, e.shiftKey);
+  if (target) {
+    e.preventDefault();
+    target.focus();
+  }
+  // target === null means `active` is a middle element: the browser's own
+  // default Tab behavior already moves focus correctly, so this is a no-op.
+}
+
 onMounted(() => {
   window.addEventListener("keydown", onKeyDown);
+
+  // Synchronous, NOT wrapped in `nextTick`: Vue fires a child's `mounted`
+  // before its parent's, and several modals self-focus an input from their
+  // OWN `onMounted` + `nextTick` (AiTaskNameModal.vue, CloneModal.vue,
+  // ForkModal.vue, FolderPicker.vue's own overlay opened from CloneModal).
+  // A `nextTick` here would run in the same microtask flush and could win
+  // the race against, or after, those, clobbering their explicit input
+  // focus. Doing this synchronously means it always happens BEFORE the
+  // parent component's own `onMounted` runs, so a parent's `nextTick`-deferred
+  // input focus always wins over this panel focus, exactly like before this
+  // fix existed.
+  previouslyFocused = document.activeElement instanceof HTMLElement ? document.activeElement : null;
+  if (props.autoFocus) panelRef.value?.focus();
 });
 
 onUnmounted(() => {
   window.removeEventListener("keydown", onKeyDown);
+  if (previouslyFocused && document.contains(previouslyFocused)) {
+    previouslyFocused.focus();
+  }
 });
 </script>
 
@@ -87,11 +159,14 @@ onUnmounted(() => {
       @click.self="onBackdropClick"
     >
       <div
+        ref="panelRef"
         class="base-modal"
         :class="sizeClass"
         :role="role"
         aria-modal="true"
         :aria-label="ariaLabel || title || undefined"
+        tabindex="-1"
+        @keydown="onPanelKeyDown"
       >
         <!-- Header -->
         <header v-if="!hideHeader" class="base-modal__header">
@@ -180,6 +255,12 @@ onUnmounted(() => {
   animation: bm-slide-in var(--transition-slow) ease;
 }
 
+/* v3.7.0 review-round fix (finding #11): the panel is `tabindex="-1"` so it
+   is programmatically focusable (initial focus, empty-focusable-set fallback);
+   suppress the focus ring that would otherwise draw on the whole panel.
+   A single class + pseudo-class, does not touch `.bm-btn` (AGENTS.md). */
+.base-modal:focus { outline: none; }
+
 .base-modal--sm   { width: min(400px, 92vw); }
 .base-modal--md   { width: min(520px, 92vw); }
 .base-modal--lg   { width: min(640px, 92vw); }

apps/desktop/src/components/CommitReviewDecisionModal.vue

@@ -0,0 +1,142 @@
+<script setup lang="ts">
+/**
+ * CommitReviewDecisionModal.vue
+ *
+ * Task 5 (v3.7.0) — the Review / Vouch / Skip decision, shown before a
+ * commit goes through when Commit Review is on. Same non-blocking UX
+ * contract as the v3.5.0 secrets scanner: this never hard-stops a commit.
+ *
+ * "Review now" runs the pass and keeps the commit pending (App.vue re-opens
+ * CommitReviewModal so the user can look at findings, then commits again
+ * when satisfied). "Vouch"/"Skip" record an explicit decision and let the
+ * commit proceed immediately. Cancelling (Escape/backdrop/Cancel button, all
+ * routed through BaseModal's `close`) cancels the commit outright — it never
+ * silently records "skipped" (decision D8: skipping must be a deliberate,
+ * explicit click, or the trailer would lie about intent).
+ *
+ * No custom keyboard shortcuts are added here — `BaseModal` already maps
+ * Escape/backdrop-click to `close`, which is exactly the "cancel" behavior
+ * this modal wants, so there's nothing else to wire (and nothing else that
+ * could get a modifier-key guard wrong).
+ */
+import { computed } from "vue";
+import { useI18n } from "../composables/useI18n";
+import BaseModal from "./BaseModal.vue";
+
+const props = withDefaults(
+  defineProps<{
+    /** Active (filtered) findings count on the staged diff, for context. */
+    findingsCount?: number;
+    /** Review passes already completed this cycle. */
+    iterations?: number;
+    /** Share (0-100) of the current staged diff already reviewed. */
+    coverage?: number;
+    /**
+     * Live, undismissed risk-severity finding count (v3.7.0 review-round fix
+     * #2/#5). When > 0, shown as a distinct warning line above the actions.
+     */
+    riskCount?: number;
+  }>(),
+  { findingsCount: 0, iterations: 0, coverage: 0, riskCount: 0 },
+);
+
+const emit = defineEmits<{
+  "review-now": [];
+  vouch: [];
+  skip: [];
+  close: [];
+}>();
+
+const { t } = useI18n();
+
+/**
+ * Three mutually exclusive context strings, driven by iterations/findingsCount
+ * (v3.7.0 review-round fix #5): the plain "{0} finding(s)" string used to
+ * render identically whether a review ran and found nothing, or no review
+ * ever ran at all.
+ */
+const contextLabel = computed(() => {
+  if (props.iterations === 0) return t("commitReview.decisionNotReviewed");
+  if (props.findingsCount === 0) return t("commitReview.decisionReviewedClean");
+  return t("commitReview.modalSubtitle", props.findingsCount);
+});
+</script>
+
+<template>
+  <BaseModal
+    :title="t('commitReview.decisionTitle')"
+    size="md"
+    role="alertdialog"
+    @close="emit('close')"
+  >
+    <p class="crdm-message">{{ t('commitReview.decisionMessage') }}</p>
+    <div class="crdm-context">
+      <span>{{ contextLabel }}</span>
+      <template v-if="iterations > 0">
+        <span class="crdm-context__sep">·</span>
+        <span>{{ t('commitReview.iterations', iterations) }}</span>
+        <span class="crdm-context__sep">·</span>
+        <span>{{ t('commitReview.coverage', coverage) }}</span>
+      </template>
+    </div>
+    <p v-if="riskCount > 0" class="crdm-risk">{{ t('commitReview.decisionRiskWarning', riskCount) }}</p>
+    <p class="crdm-hint">{{ t('commitReview.trailerHint') }}</p>
+
+    <template #footer>
+      <button type="button" class="bm-btn bm-btn--ghost crdm-cancel" @click="emit('close')">
+        {{ t('commitReview.decisionCancel') }}
+      </button>
+      <button
+        type="button"
+        class="bm-btn bm-btn--ghost crdm-skip"
+        :title="t('commitReview.skipHint')"
+        @click="emit('skip')"
+      >
+        {{ t('commitReview.decisionSkip') }}
+      </button>
+      <button
+        type="button"
+        class="bm-btn bm-btn--ghost crdm-vouch"
+        :title="t('commitReview.vouchHint')"
+        @click="emit('vouch')"
+      >
+        {{ t('commitReview.decisionVouch') }}
+      </button>
+      <button type="button" class="bm-btn bm-btn--primary crdm-review-now" @click="emit('review-now')">
+        {{ t('commitReview.decisionReviewNow') }}
+      </button>
+    </template>
+  </BaseModal>
+</template>
+
+<style scoped>
+.crdm-message {
+  color: var(--color-text);
+  margin: 0 0 var(--space-3);
+}
+
+.crdm-context {
+  display: flex;
+  flex-wrap: wrap;
+  align-items: center;
+  gap: var(--space-2);
+  color: var(--color-text-muted);
+  font-size: var(--font-size-sm);
+  margin-bottom: var(--space-3);
+}
+
+.crdm-hint {
+  color: var(--color-text-muted);
+  font-size: var(--font-size-xs);
+  margin: 0;
+}
+
+.crdm-risk {
+  color: var(--color-danger);
+  font-size: var(--font-size-sm);
+  margin: 0 0 var(--space-3);
+}
+
+/* Flat, single-class modifiers — never prefix `.bm-btn` with an ancestor
+   selector (AGENTS.md modal-CSS rule). */
+</style>