fix(VCOP-13): correct Gemini hook contract and apply review feedback

Reviewer B (functionality + reference audit) flagged two Critical bugs
that pre-dated this branch but were in scope to fix while we're in the
hook code: vibecop's Gemini install wrote a snake_case `before_tool`
string, but Gemini CLI requires PascalCase `BeforeTool` as an array of
matcher groups (same shape as Claude). Vibecop's Gemini stdin parser
read `tool`/`input`/`project`, but real Gemini sends snake_case
`tool_name`/`tool_input`/`cwd` (same as Claude). End-to-end, vibecop on
Gemini was silently broken before this fix.

Verified against
github.com/google-gemini/gemini-cli/blob/main/docs/hooks/reference.md.

- internal/hooks/install.go:
  - geminiHooks now uses [BeforeTool array of matcher groups]; install
    upsert mirrors Claude.
  - uninstallGeminiHooks strips legacy `before_tool` snake_case key on
    upgrade.
  - copilotSettingsPath honors COPILOT_HOME env override (Reviewer B/H1).
  - upsertCodexEntry / upsertCopilotEntry now return a `changed` bool
    so installs are idempotent at the file-write level (Reviewer A/H2).
- internal/hooks/hooks.go:
  - GeminiCLIPayload uses `tool_name`/`tool_input`/`cwd`/`session_id`.
  - normalizeGeminiCLI uses toolInputSummary on the object input.
  - Auto-detection branches on hook_event_name for snake_case payloads
    (PermissionRequest → Codex, BeforeTool → Gemini, PreToolUse +
    turn_id → Codex, else Claude). Removed unused hasAnyKey helper.
  - Softened normalizeCodex hook_event_name fallback comment to match
    Codex docs language (Reviewer A/M).
- cmd/hook.go: small comment on the unreachable `return nil` (cobra
  signature requires it).
- docs/spec.md: corrected Gemini install example block.
- docs/superpowers/specs/...: updated stale Copilot allow shape in the
  design doc to match implementation (Reviewer B/H3).
- Tests: gofmt-clean on touched files; new fixtures cover Gemini wire
  shape, legacy migration, and the snake_case auto-detection branches;
  switched new install tests to t.Setenv (Reviewer A nit).

go test ./... → 157 pass. go vet ./... → clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: Eugene Archibald

cmd/hook.go

@@ -77,7 +77,7 @@ If the daemon is unreachable, exits 0 silently (fail-open).`,
 		}
 
 		os.Exit(hooks.WriteVerdict(detected, nr.Event, resp, os.Stdout, os.Stderr))
-		return nil
+		return nil // unreachable; cobra signature requires it
 	},
 }
 

docs/spec.md

@@ -188,11 +188,13 @@ After subscribing, the daemon streams newline-terminated event objects to the TU
 }
 ```
 
-**Gemini CLI** (`~/.gemini/settings.json` — equivalent hook config):
+**Gemini CLI** (`~/.gemini/settings.json` — `BeforeTool` (PascalCase) hook, array of matcher groups, same shape as Claude):
 ```json
 {
   "hooks": {
-    "before_tool": "vibecop hook"
+    "BeforeTool": [
+      { "hooks": [{ "type": "command", "command": "vibecop hook" }] }
+    ]
   }
 }
 ```

docs/superpowers/specs/2026-05-08-per-harness-hook-responses-design.md

@@ -55,7 +55,7 @@ Emit the correct **per-harness JSON response** on stdout so that `approve` actua
 | `codex`, `PreToolUse`        | (no stdout — Codex PreToolUse cannot allow; PermissionRequest is the approval channel)                                 | same JSON shape as Claude PreToolUse deny + stderr                                                                           | (no stdout)  |
 | `codex`, `PermissionRequest` | `{"hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"allow"}}}`                         | `{"hookSpecificOutput":{"hookEventName":"PermissionRequest","decision":{"behavior":"deny","message":"…"}}}` + stderr         | (no stdout)  |
 | `gemini`, `BeforeTool`       | `{"decision":"allow","reason":"…"}`                                                                                    | `{"decision":"deny","reason":"…"}` + stderr                                                                                  | (no stdout)  |
-| `copilot`, `preToolUse`      | `{"permissionDecision":"allow","permissionDecisionReason":"…"}`                                                        | `{"permissionDecision":"deny","permissionDecisionReason":"…"}` + stderr                                                      | (no stdout)  |
+| `copilot`, `preToolUse`      | `{"permissionDecision":"allow"}`                                                                                       | `{"permissionDecision":"deny","permissionDecisionReason":"…"}` + stderr                                                      | (no stdout)  |
 
 The `permissionDecisionReason` / `reason` / `message` fields are omitted from JSON when the daemon's reason is empty.
 

internal/daemon/daemon_test.go

@@ -90,8 +90,8 @@ func TestPermissionRequest(t *testing.T) {
 }
 
 func TestPermissionRequestNoHandler(t *testing.T) {
-		dir := shortTempDir(t)
-		socketPath := filepath.Join(dir, "d.sock")
+	dir := shortTempDir(t)
+	socketPath := filepath.Join(dir, "d.sock")
 	cfg := config.DefaultConfig()
 	d := New(socketPath, cfg)
 	// No handler registered.

internal/hooks/hooks.go

@@ -18,12 +18,16 @@ type ClaudeCodePayload struct {
 	Cwd           string         `json:"cwd,omitempty"`
 }
 
-// GeminiCLIPayload is a Gemini CLI before_tool hook payload.
+// GeminiCLIPayload is a Gemini CLI BeforeTool hook payload.
+//
+// Same snake_case shape as Claude — the canonical reference is
+// https://github.com/google-gemini/gemini-cli/blob/main/docs/hooks/reference.md.
 type GeminiCLIPayload struct {
-	HookEventName string `json:"hook_event_name,omitempty"`
-	Tool          string `json:"tool"`
-	Input         string `json:"input"`
-	Project       string `json:"project,omitempty"`
+	HookEventName string         `json:"hook_event_name,omitempty"`
+	ToolName      string         `json:"tool_name"`
+	ToolInput     map[string]any `json:"tool_input"`
+	Cwd           string         `json:"cwd,omitempty"`
+	SessionID     string         `json:"session_id,omitempty"`
 }
 
 // CodexPayload is a Codex CLI PreToolUse / PermissionRequest hook payload.
@@ -135,29 +139,30 @@ func DetectAndParse(r io.Reader, harnessHint string) (*NormalizedRequest, string
 		return nil, HarnessUnknown, fmt.Errorf("unrecognized payload format: %s", truncate(raw, 200))
 	}
 
-	// Codex: hook_event_name == "PermissionRequest" is the unambiguous
-	// signal — only Codex emits that event. Otherwise Codex-distinctive
-	// fields (turn_id / transcript_path / model) tell us Codex even when
-	// hook_event_name is "PreToolUse" (which Claude also uses).
-	if eventName := stringField(probe, "hook_event_name"); eventName == EventPermissionRequest ||
-		(eventName == EventPreToolUse && hasAnyKey(probe, "turn_id", "transcript_path", "model")) {
-		return parseWithFormat(raw, HarnessCodex)
-	}
-
-	// Copilot: camelCase toolName and toolArgs distinguish it from snake_case
+	// Copilot: camelCase toolName distinguishes it from the snake_case
 	// payloads. No hook_event_name on the wire.
 	if _, ok := probe["toolName"]; ok {
 		return parseWithFormat(raw, HarnessCopilot)
 	}
 
-	// Claude Code: snake_case tool_name + tool_input.
+	// Claude / Codex / Gemini all use snake_case tool_name + tool_input.
+	// Disambiguate by hook_event_name, with a Codex-vs-Claude fallback for
+	// PreToolUse (which both honor) keyed on turn_id — the one Codex-only
+	// field. transcript_path and model overlap with Claude in newer versions.
 	if _, ok := probe["tool_name"]; ok {
-		return parseWithFormat(raw, HarnessClaude)
-	}
-
-	// Gemini CLI: bare "tool" + "input".
-	if _, ok := probe["tool"]; ok {
-		return parseWithFormat(raw, HarnessGemini)
+		switch stringField(probe, "hook_event_name") {
+		case EventPermissionRequest:
+			return parseWithFormat(raw, HarnessCodex)
+		case EventBeforeTool:
+			return parseWithFormat(raw, HarnessGemini)
+		case EventPreToolUse:
+			if _, ok := probe["turn_id"]; ok {
+				return parseWithFormat(raw, HarnessCodex)
+			}
+			return parseWithFormat(raw, HarnessClaude)
+		default:
+			return parseWithFormat(raw, HarnessClaude)
+		}
 	}
 
 	return nil, HarnessUnknown, fmt.Errorf("unrecognized payload format: %s", truncate(raw, 200))
@@ -179,8 +184,8 @@ func parseWithFormat(raw, harness string) (*NormalizedRequest, string, error) {
 		if err := json.Unmarshal([]byte(raw), &p); err != nil {
 			return nil, harness, fmt.Errorf("gemini payload: %w", err)
 		}
-		if p.Tool == "" {
-			return nil, harness, fmt.Errorf("gemini payload missing tool")
+		if p.ToolName == "" {
+			return nil, harness, fmt.Errorf("gemini payload missing tool_name")
 		}
 		return normalizeGeminiCLI(p), harness, nil
 	case HarnessCodex:
@@ -228,15 +233,15 @@ func normalizeClaudeCode(p ClaudeCodePayload) *NormalizedRequest {
 
 func normalizeGeminiCLI(p GeminiCLIPayload) *NormalizedRequest {
 	nr := &NormalizedRequest{
-		Tool:  p.Tool,
-		Input: p.Input,
+		Tool:  p.ToolName,
+		Input: toolInputSummary(p.ToolInput),
 		Event: p.HookEventName,
 	}
 	if nr.Event == "" {
 		nr.Event = defaultEventFor(HarnessGemini)
 	}
-	if p.Project != "" {
-		nr.ProjectPath = p.Project
+	if p.Cwd != "" {
+		nr.ProjectPath = p.Cwd
 	} else {
 		nr.ProjectPath = detectProjectDir()
 	}
@@ -249,8 +254,9 @@ func normalizeCodex(p CodexPayload) *NormalizedRequest {
 		Input: toolInputSummary(p.ToolInput),
 		Event: p.HookEventName,
 	}
-	// Codex always sends hook_event_name; if it's missing we still try the
-	// PreToolUse default rather than failing — fail-open is the contract.
+	// Codex documents hook_event_name as always present
+	// (developers.openai.com/codex/hooks); we still default to PreToolUse on
+	// absence rather than rejecting, to honor fail-open.
 	if nr.Event == "" {
 		nr.Event = EventPreToolUse
 	}
@@ -351,12 +357,3 @@ func stringField(m map[string]json.RawMessage, key string) string {
 	}
 	return s
 }
-
-func hasAnyKey(m map[string]json.RawMessage, keys ...string) bool {
-	for _, k := range keys {
-		if _, ok := m[k]; ok {
-			return true
-		}
-	}
-	return false
-}

internal/hooks/hooks_test.go

@@ -74,9 +74,10 @@ func TestDetectClaudeCodeRead(t *testing.T) {
 
 func TestDetectGeminiCLI(t *testing.T) {
 	input := `{
-		"tool": "Bash",
-		"input": "go test ./...",
-		"project": "/Users/test/src/my-project"
+		"hook_event_name": "BeforeTool",
+		"tool_name": "Bash",
+		"tool_input": { "command": "go test ./..." },
+		"cwd": "/Users/test/src/my-project"
 	}`
 
 	nr, harness, err := DetectAndParse(strings.NewReader(input), "")
@@ -95,15 +96,22 @@ func TestDetectGeminiCLI(t *testing.T) {
 	if nr.ProjectPath != "/Users/test/src/my-project" {
 		t.Errorf("expected /Users/test/src/my-project, got %s", nr.ProjectPath)
 	}
+	if nr.Event != EventBeforeTool {
+		t.Errorf("expected event BeforeTool, got %s", nr.Event)
+	}
 }
 
 func TestDetectWithHint(t *testing.T) {
-	// Valid Gemini payload forced as Claude — should fail.
-	input := `{ "tool": "Bash", "input": "echo hi" }`
+	// Valid Gemini payload forced as Claude — should still parse, since
+	// both share the snake_case shape; the hint forces claude harness.
+	input := `{ "hook_event_name": "BeforeTool", "tool_name": "Bash", "tool_input": { "command": "echo hi" } }`
 
-	_, _, err := DetectAndParse(strings.NewReader(input), HarnessClaude)
-	if err == nil {
-		t.Fatal("expected error when parsing gemini payload as claude")
+	_, harness, err := DetectAndParse(strings.NewReader(input), HarnessClaude)
+	if err != nil {
+		t.Fatalf("hint parse failed: %v", err)
+	}
+	if harness != HarnessClaude {
+		t.Errorf("hint should force claude, got %s", harness)
 	}
 }
 
@@ -254,16 +262,32 @@ func TestDetectCopilotMalformedToolArgs(t *testing.T) {
 }
 
 func TestDetectGeminiSetsBeforeToolEvent(t *testing.T) {
-	input := `{ "tool": "Bash", "input": "go test ./..." }`
-	nr, _, err := DetectAndParse(strings.NewReader(input), "")
+	input := `{ "hook_event_name": "BeforeTool", "tool_name": "Bash", "tool_input": {} }`
+	nr, harness, err := DetectAndParse(strings.NewReader(input), "")
 	if err != nil {
 		t.Fatal(err)
 	}
+	if harness != HarnessGemini {
+		t.Errorf("expected gemini, got %s", harness)
+	}
 	if nr.Event != EventBeforeTool {
 		t.Errorf("expected default BeforeTool, got %s", nr.Event)
 	}
 }
 
+func TestDetectClaudeWhenNoHookEventName(t *testing.T) {
+	// snake_case payload with tool_name and no hook_event_name → Claude
+	// (Claude's payload sometimes omits the field on older versions).
+	input := `{ "tool_name": "Bash", "tool_input": { "command": "ls" } }`
+	_, harness, err := DetectAndParse(strings.NewReader(input), "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if harness != HarnessClaude {
+		t.Errorf("expected claude default, got %s", harness)
+	}
+}
+
 func TestParseWithFormatCodexMissingEvent(t *testing.T) {
 	// Codex always sends hook_event_name, but if it's missing we fall to
 	// the PreToolUse default rather than fail.