fix(copilot): address PR review of VS Code hook capture

- Register both agent-stop AND session-end under VS Code's single "Stop"
  event. validateVSCodeEvent routes terminal Stop payloads to session-end and
  rejects them for agent-stop, so registering only agent-stop dropped
  SessionEnd for VS Code-driven sessions. The non-matching handler no-ops.
- Include VS Code hook additions in the count InstallHooks returns, so a fresh
  entire-vscode.json write is reported as an install rather than "already
  installed".
- On uninstall, preserve unknown top-level fields: only delete
  entire-vscode.json when no hooks remain AND no user-added top-level keys
  exist, instead of deleting unconditionally.

Updates tests for the two Stop handlers and adds top-level-field preservation
coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: a6b05de34ff9

Author: Soph

cmd/entire/cli/agent/copilotcli/hooks.go

@@ -44,7 +44,7 @@ var hookConfigKey = map[string]string{
 // InstallHooks installs Copilot CLI hooks in .github/hooks/entire.json and the
 // VS Code-native hook file .github/hooks/entire-vscode.json (see vscode_hooks.go).
 // If force is true, removes existing Entire hooks before installing.
-// Returns the number of Copilot CLI hooks installed.
+// Returns the total number of hooks installed across both files.
 // Unknown top-level fields and hook types are preserved on round-trip.
 func (c *CopilotCLIAgent) InstallHooks(ctx context.Context, localDev bool, force bool) (int, error) {
 	worktreeRoot, err := paths.WorktreeRoot(ctx)
@@ -54,7 +54,10 @@ func (c *CopilotCLIAgent) InstallHooks(ctx context.Context, localDev bool, force
 
 	// Install the VS Code-native hook file alongside the Copilot CLI file so
 	// Copilot sessions run from VS Code's agent hooks (Preview) are captured.
-	if err := c.installVSCodeHooks(worktreeRoot, localDev, force); err != nil {
+	// Its additions count toward the total so a fresh VS Code-file write is
+	// reported as an install rather than "already installed".
+	vsCodeCount, err := c.installVSCodeHooks(worktreeRoot, localDev, force)
+	if err != nil {
 		return 0, err
 	}
 
@@ -137,7 +140,8 @@ func (c *CopilotCLIAgent) InstallHooks(ctx context.Context, localDev bool, force
 	}
 
 	if count == 0 {
-		return 0, nil
+		// No Copilot CLI changes, but the VS Code file may have been updated.
+		return vsCodeCount, nil
 	}
 
 	// Marshal modified hook types back into rawHooks
@@ -169,7 +173,7 @@ func (c *CopilotCLIAgent) InstallHooks(ctx context.Context, localDev bool, force
 		return 0, fmt.Errorf("failed to write %s: %w", HooksFileName, err)
 	}
 
-	return count, nil
+	return count + vsCodeCount, nil
 }
 
 // UninstallHooks removes Entire hooks from Copilot CLI's entire.json.

cmd/entire/cli/agent/copilotcli/hooks_test.go

@@ -22,8 +22,9 @@ func TestInstallHooks_FreshInstall(t *testing.T) {
 		t.Fatalf("InstallHooks() error = %v", err)
 	}
 
-	if count != 8 {
-		t.Errorf("InstallHooks() count = %d, want 8", count)
+	// 8 Copilot CLI hooks + 3 VS Code hooks (user-prompt-submitted, agent-stop, session-end).
+	if count != 11 {
+		t.Errorf("InstallHooks() count = %d, want 11", count)
 	}
 
 	hooksFile := readHooksFile(t, tempDir)
@@ -87,8 +88,8 @@ func TestInstallHooks_Idempotent(t *testing.T) {
 	if err != nil {
 		t.Fatalf("first InstallHooks() error = %v", err)
 	}
-	if count1 != 8 {
-		t.Errorf("first InstallHooks() count = %d, want 8", count1)
+	if count1 != 11 {
+		t.Errorf("first InstallHooks() count = %d, want 11", count1)
 	}
 
 	// Second install
@@ -189,8 +190,8 @@ func TestInstallHooks_ForceReinstall(t *testing.T) {
 	if err != nil {
 		t.Fatalf("force InstallHooks() error = %v", err)
 	}
-	if count != 8 {
-		t.Errorf("force InstallHooks() count = %d, want 8", count)
+	if count != 11 {
+		t.Errorf("force InstallHooks() count = %d, want 11", count)
 	}
 
 	// Verify no duplicates
@@ -271,8 +272,8 @@ func TestInstallHooks_PreservesUnknownFields(t *testing.T) {
 	if err != nil {
 		t.Fatalf("InstallHooks() error = %v", err)
 	}
-	if count != 8 {
-		t.Errorf("InstallHooks() count = %d, want 8", count)
+	if count != 11 {
+		t.Errorf("InstallHooks() count = %d, want 11", count)
 	}
 
 	// Read the raw JSON to verify unknown fields are preserved

cmd/entire/cli/agent/copilotcli/vscode_hooks.go

@@ -19,20 +19,27 @@ const VSCodeHooksFileName = "entire-vscode.json"
 // vsCodeManagedEvents lists the VS Code events Entire registers in
 // entire-vscode.json, paired with the CLI hook verb each one invokes.
 //
-// Only the two turn hooks are registered here. VS Code reads Copilot CLI
-// configs by converting lowerCamelCase event names to PascalCase
-// (userPromptSubmitted -> UserPromptSubmitted, agentStop -> AgentStop). Those
-// converted names are not real VS Code events, so the capture-critical turn
-// hooks never fire from entire.json inside VS Code. The events whose converted
-// names line up with a real VS Code event (SessionStart, SubagentStop,
-// PreToolUse, PostToolUse) are already delivered by VS Code from entire.json,
-// so registering them here too would only double-fire them.
+// Only the turn hooks are registered here. VS Code reads Copilot CLI configs by
+// converting lowerCamelCase event names to PascalCase (userPromptSubmitted ->
+// UserPromptSubmitted, agentStop -> AgentStop). Those converted names are not
+// real VS Code events, so the capture-critical turn hooks never fire from
+// entire.json inside VS Code. The events whose converted names line up with a
+// real VS Code event (SessionStart, SubagentStop, PreToolUse, PostToolUse) are
+// already delivered by VS Code from entire.json, so registering them here too
+// would only double-fire them.
+//
+// VS Code uses a single "Stop" event for both end-of-turn and terminal
+// session-stop, where Copilot CLI distinguishes agent-stop from session-end.
+// We therefore register BOTH verbs under "Stop"; validateVSCodeEvent (compat.go)
+// routes each payload to the matching handler by reason, and the non-matching
+// handler no-ops. Omitting session-end here would drop terminal SessionEnd
+// events for VS Code-driven sessions.
 var vsCodeManagedEvents = []struct {
-	Event string // VS Code hookEventName (PascalCase)
-	Verb  string // Entire CLI hook verb
+	Event string   // VS Code hookEventName (PascalCase)
+	Verbs []string // Entire CLI hook verbs registered under that event
 }{
-	{VSCodeEventUserPromptSubmit, HookNameUserPromptSubmitted},
-	{VSCodeEventStop, HookNameAgentStop},
+	{VSCodeEventUserPromptSubmit, []string{HookNameUserPromptSubmitted}},
+	{VSCodeEventStop, []string{HookNameAgentStop, HookNameSessionEnd}},
 }
 
 // VSCodeHookEntry represents a single VS Code hook command. VS Code uses the
@@ -58,12 +65,13 @@ func vsCodeHookCommand(verb string, localDev bool) string {
 // installVSCodeHooks writes/updates .github/hooks/entire-vscode.json with the
 // VS Code turn hooks. If force is true, existing Entire entries are removed
 // before installing. Unknown fields and event types are preserved on round-trip.
-func (c *CopilotCLIAgent) installVSCodeHooks(worktreeRoot string, localDev bool, force bool) error {
+// Returns the number of hook entries newly added.
+func (c *CopilotCLIAgent) installVSCodeHooks(worktreeRoot string, localDev bool, force bool) (int, error) {
 	hooksPath := filepath.Join(worktreeRoot, hooksDir, VSCodeHooksFileName)
 
 	rawFile, rawHooks, err := readVSCodeHooksFile(hooksPath)
 	if err != nil {
-		return err
+		return 0, err
 	}
 	if rawFile == nil {
 		rawFile = map[string]json.RawMessage{"version": json.RawMessage(`1`)}
@@ -72,33 +80,45 @@ func (c *CopilotCLIAgent) installVSCodeHooks(worktreeRoot string, localDev bool,
 		rawHooks = make(map[string]json.RawMessage)
 	}
 
+	count := 0
 	for _, h := range vsCodeManagedEvents {
 		var entries []VSCodeHookEntry
 		if err := parseVSCodeHookEvent(rawHooks, h.Event, &entries); err != nil {
-			return err
+			return 0, err
 		}
 		if force {
 			entries = removeEntireVSCodeHooks(entries)
 		}
-		cmd := vsCodeHookCommand(h.Verb, localDev)
-		if !vsCodeCommandExists(entries, cmd) {
-			entries = append(entries, VSCodeHookEntry{
-				Type:    "command",
-				Command: cmd,
-				Comment: "Entire CLI",
-			})
+		// All verbs share one event, so remove-on-force happens once above; then
+		// add each missing verb. Doing the force-removal per verb would wipe a
+		// sibling verb added earlier in this same loop.
+		for _, verb := range h.Verbs {
+			cmd := vsCodeHookCommand(verb, localDev)
+			if !vsCodeCommandExists(entries, cmd) {
+				entries = append(entries, VSCodeHookEntry{
+					Type:    "command",
+					Command: cmd,
+					Comment: "Entire CLI",
+				})
+				count++
+			}
 		}
 		if err := marshalVSCodeHookEvent(rawHooks, h.Event, entries); err != nil {
-			return err
+			return 0, err
 		}
 	}
 
-	return writeVSCodeHooksFile(hooksPath, rawFile, rawHooks)
+	if err := writeVSCodeHooksFile(hooksPath, rawFile, rawHooks); err != nil {
+		return 0, err
+	}
+	return count, nil
 }
 
-// uninstallVSCodeHooks removes Entire entries from entire-vscode.json. When no
-// hooks of any event type remain afterwards (nothing user-owned), the file is
-// deleted rather than left as an empty shell.
+// uninstallVSCodeHooks removes Entire entries from entire-vscode.json. When the
+// file holds nothing user-owned afterwards (no hooks of any event type and no
+// top-level fields beyond the structural version/hooks keys Entire writes), it
+// is deleted rather than left as an empty shell. If the user added their own
+// hooks or top-level fields, those are preserved and the file is rewritten.
 func (c *CopilotCLIAgent) uninstallVSCodeHooks(worktreeRoot string) error {
 	hooksPath := filepath.Join(worktreeRoot, hooksDir, VSCodeHooksFileName)
 
@@ -121,8 +141,9 @@ func (c *CopilotCLIAgent) uninstallVSCodeHooks(worktreeRoot string) error {
 		}
 	}
 
-	// If nothing user-owned remains, remove the file entirely.
-	if len(rawHooks) == 0 {
+	// Delete the file only when nothing user-owned remains: no hooks left and no
+	// top-level fields other than the structural ones Entire itself writes.
+	if len(rawHooks) == 0 && !hasUserTopLevelFields(rawFile) {
 		if err := os.Remove(hooksPath); err != nil && !errors.Is(err, os.ErrNotExist) {
 			return fmt.Errorf("failed to remove %s: %w", VSCodeHooksFileName, err)
 		}
@@ -132,6 +153,18 @@ func (c *CopilotCLIAgent) uninstallVSCodeHooks(worktreeRoot string) error {
 	return writeVSCodeHooksFile(hooksPath, rawFile, rawHooks)
 }
 
+// hasUserTopLevelFields reports whether rawFile carries any top-level key beyond
+// the structural "version" and "hooks" keys Entire manages — i.e. fields a user
+// added that must survive uninstall.
+func hasUserTopLevelFields(rawFile map[string]json.RawMessage) bool {
+	for key := range rawFile {
+		if key != "version" && key != "hooks" {
+			return true
+		}
+	}
+	return false
+}
+
 // areVSCodeHooksInstalled reports whether any Entire hook is present in
 // entire-vscode.json.
 func (c *CopilotCLIAgent) areVSCodeHooksInstalled(worktreeRoot string) bool {

cmd/entire/cli/agent/copilotcli/vscode_hooks_test.go

@@ -36,14 +36,17 @@ func TestInstallVSCodeHooks_FreshInstall(t *testing.T) {
 	tempDir := t.TempDir()
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
+	count, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
+	require.Equal(t, 3, count, "user-prompt-submitted + agent-stop + session-end")
 
 	hooks := readVSCodeFile(t, tempDir)
 
 	// Only the two turn events are registered; the rest come from entire.json.
 	require.Len(t, hooks, 2, "only UserPromptSubmit and Stop should be registered")
 	require.Len(t, hooks[VSCodeEventUserPromptSubmit], 1)
-	require.Len(t, hooks[VSCodeEventStop], 1)
+	// VS Code's single Stop event carries both agent-stop and session-end.
+	require.Len(t, hooks[VSCodeEventStop], 2)
 
 	ups := hooks[VSCodeEventUserPromptSubmit][0]
 	require.Equal(t, "command", ups.Type)
@@ -53,22 +56,37 @@ func TestInstallVSCodeHooks_FreshInstall(t *testing.T) {
 		ups.Command,
 		"VS Code uses the command field, not bash")
 
-	require.Equal(t,
-		agent.WrapProductionSilentHookCommand("entire hooks copilot-cli agent-stop"),
-		hooks[VSCodeEventStop][0].Command)
+	stopCommands := commandsOf(hooks[VSCodeEventStop])
+	require.Contains(t, stopCommands,
+		agent.WrapProductionSilentHookCommand("entire hooks copilot-cli agent-stop"))
+	require.Contains(t, stopCommands,
+		agent.WrapProductionSilentHookCommand("entire hooks copilot-cli session-end"),
+		"terminal VS Code Stop must route to session-end")
+}
+
+// commandsOf extracts the command strings from a slice of hook entries.
+func commandsOf(entries []VSCodeHookEntry) []string {
+	cmds := make([]string, len(entries))
+	for i, e := range entries {
+		cmds[i] = e.Command
+	}
+	return cmds
 }
 
 func TestInstallVSCodeHooks_Idempotent(t *testing.T) {
 	t.Parallel()
 	tempDir := t.TempDir()
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
+	_, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
+	count, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
+	require.Equal(t, 0, count, "reinstall adds nothing")
 
 	hooks := readVSCodeFile(t, tempDir)
 	require.Len(t, hooks[VSCodeEventUserPromptSubmit], 1, "must not duplicate on reinstall")
-	require.Len(t, hooks[VSCodeEventStop], 1)
+	require.Len(t, hooks[VSCodeEventStop], 2, "agent-stop + session-end, no duplicates")
 }
 
 func TestInstallVSCodeHooks_PreservesUserHooks(t *testing.T) {
@@ -92,12 +110,14 @@ func TestInstallVSCodeHooks_PreservesUserHooks(t *testing.T) {
 	require.NoError(t, os.WriteFile(vsCodeHooksPath(tempDir), []byte(seed), 0o600))
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
+	_, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
 
 	hooks := readVSCodeFile(t, tempDir)
-	require.Len(t, hooks[VSCodeEventStop], 2, "user Stop hook retained alongside Entire's")
+	require.Len(t, hooks[VSCodeEventStop], 3, "user Stop hook retained alongside Entire's agent-stop + session-end")
 	require.Len(t, hooks[VSCodeEventPreToolUse], 1, "unknown event type preserved")
 	require.Equal(t, "echo user-pretool", hooks[VSCodeEventPreToolUse][0].Command)
+	require.Contains(t, commandsOf(hooks[VSCodeEventStop]), "echo user-stop", "user Stop hook survives")
 
 	// Unknown top-level field is preserved on round-trip.
 	data, err := os.ReadFile(vsCodeHooksPath(tempDir))
@@ -110,12 +130,13 @@ func TestUninstallVSCodeHooks_DeletesWhenEmpty(t *testing.T) {
 	tempDir := t.TempDir()
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
+	_, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
 	require.True(t, ag.areVSCodeHooksInstalled(tempDir))
 
 	require.NoError(t, ag.uninstallVSCodeHooks(tempDir))
 
-	_, err := os.Stat(vsCodeHooksPath(tempDir))
+	_, err = os.Stat(vsCodeHooksPath(tempDir))
 	require.ErrorIs(t, err, os.ErrNotExist, "file removed when nothing user-owned remains")
 	require.False(t, ag.areVSCodeHooksInstalled(tempDir))
 }
@@ -125,7 +146,8 @@ func TestUninstallVSCodeHooks_KeepsUserHooks(t *testing.T) {
 	tempDir := t.TempDir()
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
+	_, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
 
 	// Add a user-owned hook to the Stop event.
 	rawFile, rawHooks, err := readVSCodeHooksFile(vsCodeHooksPath(tempDir))
@@ -158,12 +180,42 @@ func TestInstallVSCodeHooks_ForceReinstall(t *testing.T) {
 	tempDir := t.TempDir()
 
 	ag := &CopilotCLIAgent{}
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, false))
-	require.NoError(t, ag.installVSCodeHooks(tempDir, false, true))
+	_, err := ag.installVSCodeHooks(tempDir, false, false)
+	require.NoError(t, err)
+	_, err = ag.installVSCodeHooks(tempDir, false, true)
+	require.NoError(t, err)
 
 	hooks := readVSCodeFile(t, tempDir)
 	require.Len(t, hooks[VSCodeEventUserPromptSubmit], 1)
-	require.Len(t, hooks[VSCodeEventStop], 1)
+	require.Len(t, hooks[VSCodeEventStop], 2, "force reinstall keeps both agent-stop + session-end")
+}
+
+func TestUninstallVSCodeHooks_PreservesUserTopLevelFields(t *testing.T) {
+	t.Parallel()
+	tempDir := t.TempDir()
+
+	// File with only Entire-managed hooks but a user-added top-level field.
+	seed := `{
+  "version": 1,
+  "customField": "keep-me",
+  "hooks": {
+    "Stop": [
+      {"type": "command", "command": "` + agent.WrapProductionSilentHookCommand("entire hooks copilot-cli agent-stop") + `"}
+    ]
+  }
+}`
+	require.NoError(t, os.MkdirAll(filepath.Join(tempDir, hooksDir), 0o755))
+	require.NoError(t, os.WriteFile(vsCodeHooksPath(tempDir), []byte(seed), 0o600))
+
+	ag := &CopilotCLIAgent{}
+	require.NoError(t, ag.uninstallVSCodeHooks(tempDir))
+
+	// File must survive (custom field) but Entire's hook is gone.
+	require.FileExists(t, vsCodeHooksPath(tempDir), "file kept because customField is user-owned")
+	data, err := os.ReadFile(vsCodeHooksPath(tempDir))
+	require.NoError(t, err)
+	require.Contains(t, string(data), "keep-me")
+	require.False(t, ag.areVSCodeHooksInstalled(tempDir))
 }
 
 func TestInstallHooks_AlsoInstallsVSCodeFile(t *testing.T) {