From 8f34c2ef4d507dae61db9fd7c52c48d215d1c7e1 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 20 May 2026 19:46:19 +0530 Subject: [PATCH 1/9] fix(claude-code): add timeout to session-start hook Entire-Checkpoint: 01d84dee8fed --- cmd/entire/cli/agent/claudecode/hooks.go | 57 ++++++++----- cmd/entire/cli/agent/claudecode/hooks_test.go | 82 +++++++++++++++++++ cmd/entire/cli/agent/claudecode/types.go | 1 + 3 files changed, 120 insertions(+), 20 deletions(-) diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index b112dd1c9d..0ec0c30b78 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -34,6 +34,8 @@ const ClaudeSettingsFileName = "settings.json" // metadataDenyRule blocks Claude from reading Entire session metadata const metadataDenyRule = "Read(./.entire/metadata/**)" +const productionSessionStartHookTimeoutSeconds = 5 + // entireHookPrefixes are command prefixes that identify Entire hooks (both old and new formats) var entireHookPrefixes = []string{ "entire ", @@ -133,9 +135,15 @@ func (c *ClaudeCodeAgent) InstallHooks(ctx context.Context, localDev bool, force count := 0 + sessionStartTimeout := 0 + if !localDev { + sessionStartTimeout = productionSessionStartHookTimeoutSeconds + } + // Add hooks if they don't exist - if !hookCommandExists(sessionStart, sessionStartCmd) { - sessionStart = addHookToMatcher(sessionStart, "", sessionStartCmd) + var sessionStartChanged bool + sessionStart, sessionStartChanged = ensureHookInMatcherWithTimeout(sessionStart, "", sessionStartCmd, sessionStartTimeout) + if sessionStartChanged { count++ } if !hookCommandExists(sessionEnd, sessionEndCmd) { @@ -426,37 +434,46 @@ func hookCommandExistsWithMatcher(matchers []ClaudeHookMatcher, matcherName, com } func addHookToMatcher(matchers []ClaudeHookMatcher, matcherName, command string) []ClaudeHookMatcher { + return addHookToMatcherWithTimeout(matchers, matcherName, command, 0) +} + +func addHookToMatcherWithTimeout(matchers []ClaudeHookMatcher, matcherName, command string, timeout int) []ClaudeHookMatcher { + matchers, _ = ensureHookInMatcherWithTimeout(matchers, matcherName, command, timeout) + return matchers +} + +func ensureHookInMatcherWithTimeout(matchers []ClaudeHookMatcher, matcherName, command string, timeout int) ([]ClaudeHookMatcher, bool) { entry := ClaudeHookEntry{ Type: "command", Command: command, + Timeout: timeout, } - // If no matcher name, add to a matcher with empty string - if matcherName == "" { - for i, matcher := range matchers { - if matcher.Matcher == "" { - matchers[i].Hooks = append(matchers[i].Hooks, entry) - return matchers - } + for i, matcher := range matchers { + if matcher.Matcher != matcherName { + continue } - return append(matchers, ClaudeHookMatcher{ - Matcher: "", - Hooks: []ClaudeHookEntry{entry}, - }) - } - // Find or create matcher with the given name - for i, matcher := range matchers { - if matcher.Matcher == matcherName { - matchers[i].Hooks = append(matchers[i].Hooks, entry) - return matchers + for j, hook := range matcher.Hooks { + if hook.Command != command { + continue + } + if hook.Type == entry.Type && hook.Timeout == entry.Timeout { + return matchers, false + } + matchers[i].Hooks[j].Type = entry.Type + matchers[i].Hooks[j].Timeout = entry.Timeout + return matchers, true } + + matchers[i].Hooks = append(matchers[i].Hooks, entry) + return matchers, true } return append(matchers, ClaudeHookMatcher{ Matcher: matcherName, Hooks: []ClaudeHookEntry{entry}, - }) + }), true } // isEntireHook checks if a command is an Entire hook (old or new format) diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 1b00cb26b4..90693d0f9f 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -3,6 +3,7 @@ package claudecode import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "slices" @@ -435,6 +436,87 @@ func readClaudeSettings(t *testing.T, tempDir string) ClaudeSettings { return settings } +func TestInstallHooks_AddsTimeoutToNewProductionSessionStart(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + agent := &ClaudeCodeAgent{} + _, err := agent.InstallHooks(context.Background(), false, false) + if err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + settings := readClaudeSettings(t, tempDir) + wantCommand := agentpkg.WrapProductionJSONWarningHookCommand( + "entire hooks claude-code session-start", + agentpkg.WarningFormatMultiLine, + ) + + for _, matcher := range settings.Hooks.SessionStart { + if matcher.Matcher != "" { + continue + } + for _, hook := range matcher.Hooks { + if hook.Command == wantCommand { + if hook.Timeout != productionSessionStartHookTimeoutSeconds { + t.Fatalf("SessionStart timeout = %d, want %d", hook.Timeout, productionSessionStartHookTimeoutSeconds) + } + return + } + } + } + + t.Fatal("production SessionStart hook not found") +} + +func TestInstallHooks_BackfillsTimeoutOnExistingProductionSessionStart(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + existingCommand := agentpkg.WrapProductionJSONWarningHookCommand( + "entire hooks claude-code session-start", + agentpkg.WarningFormatMultiLine, + ) + writeSettingsFile(t, tempDir, fmt.Sprintf(`{ + "hooks": { + "SessionStart": [ + { + "matcher": "", + "hooks": [{"type": "command", "command": %q}] + } + ] + } +}`, existingCommand)) + + before := readClaudeSettings(t, tempDir) + if before.Hooks.SessionStart[0].Hooks[0].Timeout != 0 { + t.Fatalf("precondition SessionStart timeout = %d, want 0", before.Hooks.SessionStart[0].Hooks[0].Timeout) + } + + agent := &ClaudeCodeAgent{} + _, err := agent.InstallHooks(context.Background(), false, false) + if err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + settings := readClaudeSettings(t, tempDir) + for _, matcher := range settings.Hooks.SessionStart { + if matcher.Matcher != "" { + continue + } + for _, hook := range matcher.Hooks { + if hook.Command == existingCommand { + if hook.Timeout != productionSessionStartHookTimeoutSeconds { + t.Fatalf("SessionStart timeout = %d, want %d", hook.Timeout, productionSessionStartHookTimeoutSeconds) + } + return + } + } + } + + t.Fatal("production SessionStart hook not found") +} + //nolint:tparallel // Parent uses t.Chdir() which prevents t.Parallel(); subtests only read from pre-loaded data func TestInstallHooks_PreservesUserHooksOnSameType(t *testing.T) { tempDir := t.TempDir() diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index 2a8898e1b2..ea956c3fe5 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -27,6 +27,7 @@ type ClaudeHookMatcher struct { type ClaudeHookEntry struct { Type string `json:"type"` Command string `json:"command"` + Timeout int `json:"timeout,omitempty"` } // sessionInfoRaw is the JSON structure from SessionStart/SessionEnd/Stop hooks. From 062675d75c715a767644762c36306ca8cc24d239 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Tue, 13 Oct 2026 20:06:03 +0530 Subject: [PATCH 2/9] fix(claude-code): time out Entire hooks so a stalled launch can't block Claude Entire-Checkpoint: 01M4TV293VGC1RSNYYKD2SASRQ --- .claude/settings.json | 27 ++- cmd/entire/cli/agent/claudecode/hooks.go | 91 ++++++-- cmd/entire/cli/agent/claudecode/hooks_test.go | 200 +++++++++++++++--- 3 files changed, 260 insertions(+), 58 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 17f53b2a76..b632de7b99 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -18,7 +18,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'", + "timeout": 60 } ] }, @@ -27,7 +28,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'", + "timeout": 60 } ] } @@ -38,7 +40,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code pre-task'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code pre-task'", + "timeout": 10 } ] } @@ -64,7 +67,8 @@ }, { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then printf \"%s\\n\" \"{\\\"systemMessage\\\":\\\"\\\\n\\\\nEntire CLI is enabled but not installed or not on PATH.\\\\nInstallation guide: https://docs.entire.io/cli/installation#installation-methods\\\"}\"; exit 0; fi; exec entire hooks claude-code session-start'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then printf \"%s\\n\" \"{\\\"systemMessage\\\":\\\"\\\\n\\\\nEntire CLI is enabled but not installed or not on PATH.\\\\nInstallation guide: https://docs.entire.io/cli/installation#installation-methods\\\"}\"; exit 0; fi; exec entire hooks claude-code session-start'", + "timeout": 10 } ] } @@ -75,7 +79,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop'", + "timeout": 60 } ] } @@ -86,7 +91,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop-failure'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop-failure'", + "timeout": 60 } ] } @@ -97,7 +103,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-start'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-start'", + "timeout": 10 } ] } @@ -108,7 +115,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-stop'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-stop'", + "timeout": 60 } ] } @@ -119,7 +127,8 @@ "hooks": [ { "type": "command", - "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code user-prompt-submit'" + "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code user-prompt-submit'", + "timeout": 10 } ] } diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index c20809443f..3a27b41992 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -86,7 +86,7 @@ func (c *ClaudeCodeAgent) InstallHooks(ctx context.Context, force bool) (int, er return 0, err } - count, staleDropped := installHookEntries(rawHooks, force) + count, staleDropped, timeoutsSet := installHookEntries(rawHooks, force) // Unconditional, like the stale-hook migration in installHookEntries: a // plain `entire enable` must drop the retired metadata deny rule, not just @@ -97,10 +97,10 @@ func (c *ClaudeCodeAgent) InstallHooks(ctx context.Context, force bool) (int, er return 0, fmt.Errorf("failed to update permissions in %s: %w", cfg.Path(), err) } - // staleDropped forces a write even when nothing was added: a file holding - // both a stale and a current hook adds nothing, and returning early here - // would leave the stale hook on disk. - if count == 0 && !permissionsChanged && !staleDropped { + // staleDropped and timeoutsSet force a write even when nothing was added: a + // file holding both a stale and a current hook, or a current hook without + // its timeout, adds nothing, and returning early here would leave it as is. + if count == 0 && !permissionsChanged && !staleDropped && !timeoutsSet { return 0, nil // All hooks and permissions already installed } @@ -124,7 +124,7 @@ func (c *ClaudeCodeAgent) HasStaleManagedHooks(ctx context.Context) bool { if err != nil { return false } - _, staleDropped := installHookEntries(rawHooks, false) + _, staleDropped, _ := installHookEntries(rawHooks, false) return staleDropped } @@ -192,23 +192,38 @@ func loadRawClaudeSettingsForInstall(cfg *agent.HookConfigFile) (rawSettings, ra return rawSettings, rawHooks, rawPermissions, nil } +// Claude Code waits on Entire's hooks, and an `entire` launch the OS stalls +// before main() never returns on its own, so each hook carries a timeout that +// lets Claude Code kill it and continue. +const ( + // quickHookTimeoutSec bounds hooks that only record state. They finish in + // milliseconds, and Claude Code blocks startup and every prompt on them. + quickHookTimeoutSec = 10 + // checkpointHookTimeoutSec bounds hooks that write checkpoint data, whose + // cost grows with transcript size; a kill mid-write loses that checkpoint. + checkpointHookTimeoutSec = 60 +) + type entireSimpleHook struct { hookType string matcher string command string + timeout int } // entireSimpleHooks lists the hooks Entire registers alone in their hook // type, each under one matcher (empty for all but SubagentStart). func entireSimpleHooks() []entireSimpleHook { return []entireSimpleHook{ - {"SessionStart", "", agent.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agent.WarningFormatMultiLine)}, - {"SessionEnd", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code session-end")}, - {"Stop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop")}, - {"StopFailure", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop-failure")}, - {"SubagentStart", workflowAgentMatcher, agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-start")}, - {"SubagentStop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop")}, - {"UserPromptSubmit", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code user-prompt-submit")}, + {"SessionStart", "", agent.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agent.WarningFormatMultiLine), quickHookTimeoutSec}, + // No timeout: Claude Code gives SessionEnd a short exit grace by default, + // and an explicit timeout would make it wait longer on exit. + {"SessionEnd", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code session-end"), 0}, + {"Stop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop"), checkpointHookTimeoutSec}, + {"StopFailure", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop-failure"), checkpointHookTimeoutSec}, + {"SubagentStart", workflowAgentMatcher, agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-start"), quickHookTimeoutSec}, + {"SubagentStop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop"), checkpointHookTimeoutSec}, + {"UserPromptSubmit", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code user-prompt-submit"), quickHookTimeoutSec}, } } @@ -218,6 +233,25 @@ func entireToolUseHookCommands() (preTask, postTask string) { agent.WrapProductionSilentHookCommand("entire hooks claude-code post-task") } +// setMissingTimeout gives every entry running command the timeout, leaving any +// timeout the user already chose. Reports whether it changed an entry. +func setMissingTimeout(matchers []ClaudeHookMatcher, command string, timeout int) bool { + if timeout == 0 { + return false + } + changed := false + for i := range matchers { + for j := range matchers[i].Hooks { + hook := &matchers[i].Hooks[j] + if hook.Command == command && hook.Timeout == 0 { + hook.Timeout = timeout + changed = true + } + } + } + return changed +} + // retiredPostTodoHookCommand is the post-todo command older CLIs installed. // It is no longer installed, and an install prunes it, but its subcommand stays // registered and records nothing, so a checkout that still carries it runs @@ -243,11 +277,12 @@ func EntireHookCommands() map[string][]string { } // installHookEntries mutates rawHooks in place to ensure every Entire hook is -// present, migrating stale entries (from older CLI versions or, when force is -// set, any current Entire hook) first. Returns the number of hooks newly -// added and whether any stale entry was dropped (see the staleDropped comment -// at its InstallHooks call site for why that forces a write on its own). -func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count int, staleDropped bool) { +// present with its timeout, migrating stale entries (from older CLI versions +// or, when force is set, any current Entire hook) first. Returns the number of +// hooks newly added, whether any stale entry was dropped, and whether any entry +// gained a timeout (see the staleDropped comment at its InstallHooks call site +// for why either forces a write on its own). +func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count int, staleDropped, timeoutsSet bool) { var preToolUse, postToolUse []ClaudeHookMatcher parseHookType(rawHooks, "PreToolUse", &preToolUse) parseHookType(rawHooks, "PostToolUse", &postToolUse) @@ -323,6 +358,19 @@ func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count count++ } + // Covers the entries just added and those older CLIs wrote without one. + for _, h := range simpleHooks { + if setMissingTimeout(simpleMatchers[h.hookType], h.command, h.timeout) { + timeoutsSet = true + } + } + if setMissingTimeout(preToolUse, preTaskCmd, quickHookTimeoutSec) { + timeoutsSet = true + } + if setMissingTimeout(postToolUse, postTaskCmd, checkpointHookTimeoutSec) { + timeoutsSet = true + } + // Marshal modified hook types back to rawHooks for _, h := range simpleHooks { marshalHookType(rawHooks, h.hookType, simpleMatchers[h.hookType]) @@ -330,7 +378,7 @@ func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count marshalHookType(rawHooks, "PreToolUse", preToolUse) marshalHookType(rawHooks, "PostToolUse", postToolUse) - return count, staleDropped + return count, staleDropped, timeoutsSet } // writeClaudeSettingsFile marshals rawHooks and rawPermissions into @@ -681,10 +729,7 @@ func hookCommandExistsWithMatcher(matchers []ClaudeHookMatcher, matcherName, com } // addHookToMatcher appends command to the matcher named matcherName, creating it -// if absent. No timeout is set: Timeout exists on ClaudeHookEntry to round-trip -// settings files that carry one, but Entire's own hooks all keep Claude Code's -// default. (The removed local-dev mode was the only thing that needed a longer -// SessionEnd budget, because it compiled the CLI from source.) +// if absent. installHookEntries sets the entry's timeout afterwards. func addHookToMatcher(matchers []ClaudeHookMatcher, matcherName, command string) []ClaudeHookMatcher { entry := ClaudeHookEntry{ Type: "command", diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index f056660c13..49fc0f8196 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -9,6 +9,7 @@ import ( "slices" "strings" "testing" + "time" agentpkg "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/testutil" @@ -567,10 +568,66 @@ func TestUninstallHooks_RemovesLegacyLocalDevHooks(t *testing.T) { } } -// TestInstallHooks_SessionEndIsUntimed verifies no hook carries an explicit -// timeout. The explicit SessionEnd timeout existed only for local-dev hooks, -// which compiled from source and could exceed Claude Code's exit grace. -func TestInstallHooks_SessionEndIsUntimed(t *testing.T) { +// expectedHookTimeout is one installed Entire hook and the timeout it must carry. +type expectedHookTimeout struct { + hookType string + matcher string + command string + timeout int +} + +func expectedHookTimeouts() []expectedHookTimeout { + silent := agentpkg.WrapProductionSilentHookCommand + return []expectedHookTimeout{ + {"SessionStart", "", agentpkg.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agentpkg.WarningFormatMultiLine), 10}, + {"UserPromptSubmit", "", silent("entire hooks claude-code user-prompt-submit"), 10}, + {"SubagentStart", workflowAgentMatcher, silent("entire hooks claude-code subagent-start"), 10}, + {"PreToolUse", subagentToolMatcher, silent("entire hooks claude-code pre-task"), 10}, + {"Stop", "", silent("entire hooks claude-code stop"), 60}, + {"StopFailure", "", silent("entire hooks claude-code stop-failure"), 60}, + {"SubagentStop", "", silent("entire hooks claude-code subagent-stop"), 60}, + {"PostToolUse", subagentToolMatcher, silent("entire hooks claude-code post-task"), 60}, + {"PostToolUse", skillToolMatcher, silent("entire hooks claude-code post-task"), 60}, + // Claude Code gives SessionEnd a short exit grace by default; an explicit + // timeout would make it wait longer on exit, not shorter. + {"SessionEnd", "", silent("entire hooks claude-code session-end"), 0}, + } +} + +func hookMatchersByType(settings ClaudeSettings) map[string][]ClaudeHookMatcher { + h := settings.Hooks + return map[string][]ClaudeHookMatcher{ + "SessionStart": h.SessionStart, + "SessionEnd": h.SessionEnd, + "UserPromptSubmit": h.UserPromptSubmit, + "SubagentStart": h.SubagentStart, + "SubagentStop": h.SubagentStop, + "Stop": h.Stop, + "StopFailure": h.StopFailure, + "PreToolUse": h.PreToolUse, + "PostToolUse": h.PostToolUse, + } +} + +func assertHookTimeouts(t *testing.T, settings ClaudeSettings) { + t.Helper() + byType := hookMatchersByType(settings) + for _, want := range expectedHookTimeouts() { + hook, ok := lookupHook(byType[want.hookType], want.matcher, want.command) + if !ok { + t.Errorf("%s hook not found (matcher=%q, command=%q)", want.hookType, want.matcher, want.command) + continue + } + if hook.Timeout != want.timeout { + t.Errorf("%s (matcher=%q) timeout = %d, want %d", want.hookType, want.matcher, hook.Timeout, want.timeout) + } + } +} + +// TestInstallHooks_SetsHookTimeouts pins a per-hook timeout on every Entire hook +// that Claude Code waits on, so a stalled `entire` launch cannot block startup, +// prompts, or turn ends indefinitely. +func TestInstallHooks_SetsHookTimeouts(t *testing.T) { tempDir := t.TempDir() t.Chdir(tempDir) @@ -580,21 +637,104 @@ func TestInstallHooks_SessionEndIsUntimed(t *testing.T) { } settings := readClaudeSettings(t, tempDir) - if len(settings.Hooks.SessionEnd) == 0 || len(settings.Hooks.SessionEnd[0].Hooks) == 0 { - t.Fatal("expected a SessionEnd hook to be installed") - } - if got := settings.Hooks.SessionEnd[0].Hooks[0].Timeout; got != 0 { - t.Errorf("SessionEnd timeout = %d, want 0", got) - } + assertHookTimeouts(t, settings) - // Stronger than the parsed check: prove the field is omitted entirely - // (omitempty), not written as an explicit "timeout": 0. + // SessionEnd must omit the field entirely (omitempty), not write "timeout": 0. raw, err := os.ReadFile(filepath.Join(tempDir, ".claude", "settings.json")) if err != nil { t.Fatalf("failed to read settings.json: %v", err) } - if strings.Contains(string(raw), "timeout") { - t.Errorf("settings.json must not contain any timeout field, got:\n%s", raw) + var parsed struct { + Hooks map[string][]struct { + Hooks []map[string]json.RawMessage `json:"hooks"` + } `json:"hooks"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatalf("failed to parse settings.json: %v", err) + } + for _, m := range parsed.Hooks["SessionEnd"] { + for _, h := range m.Hooks { + if _, ok := h["timeout"]; ok { + t.Errorf("SessionEnd hook must not carry a timeout field, got:\n%s", raw) + } + } + } +} + +// TestInstallHooks_BackfillsMissingTimeouts covers configs written by older CLIs: +// the hook is already present without a timeout, so nothing is added, but the +// install must still write the timeout. +func TestInstallHooks_BackfillsMissingTimeouts(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + writeClaudeStopHooks(t, tempDir, []string{agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop")}) + + ag := &ClaudeCodeAgent{} + if _, err := ag.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + assertHookTimeouts(t, readClaudeSettings(t, tempDir)) +} + +// TestInstallHooks_KeepsUserSetTimeout verifies an install never overrides a +// timeout the user chose for an Entire hook. +func TestInstallHooks_KeepsUserSetTimeout(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + writeSettingsFile(t, tempDir, fmt.Sprintf(`{ + "hooks": { + "Stop": [{"matcher": "", "hooks": [{"type": "command", "command": %q, "timeout": 300}]}] + } +}`, stopCmd)) + + ag := &ClaudeCodeAgent{} + if _, err := ag.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + hook, ok := lookupHook(readClaudeSettings(t, tempDir).Hooks.Stop, "", stopCmd) + if !ok { + t.Fatal("Stop hook not found") + } + if hook.Timeout != 300 { + t.Errorf("Stop timeout = %d, want the user's 300", hook.Timeout) + } +} + +// TestInstallHooks_TimeoutsAreIdempotent verifies a second install of an +// up-to-date config neither adds hooks nor rewrites the file. +func TestInstallHooks_TimeoutsAreIdempotent(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + ag := &ClaudeCodeAgent{} + if _, err := ag.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + settingsPath := filepath.Join(tempDir, ".claude", "settings.json") + past := time.Now().Add(-time.Hour).Truncate(time.Second) + if err := os.Chtimes(settingsPath, past, past); err != nil { + t.Fatalf("Chtimes() error = %v", err) + } + + count, err := ag.InstallHooks(context.Background(), false) + if err != nil { + t.Fatalf("second InstallHooks() error = %v", err) + } + if count != 0 { + t.Errorf("second InstallHooks() count = %d, want 0", count) + } + info, err := os.Stat(settingsPath) + if err != nil { + t.Fatalf("Stat() error = %v", err) + } + if !info.ModTime().Equal(past) { + t.Errorf("settings.json was rewritten by an install that had nothing to change") } } @@ -777,19 +917,27 @@ func TestInstallHooks_PrunesPostTodoHook(t *testing.T) { } } -// assertHookExists checks that a hook with the given matcher and command exists -func assertHookExists(t *testing.T, matchers []ClaudeHookMatcher, matcher, command, description string) { - t.Helper() +// lookupHook returns the hook with the given matcher and command. +func lookupHook(matchers []ClaudeHookMatcher, matcher, command string) (ClaudeHookEntry, bool) { for _, m := range matchers { - if m.Matcher == matcher { - for _, h := range m.Hooks { - if h.Command == command { - return - } + if m.Matcher != matcher { + continue + } + for _, h := range m.Hooks { + if h.Command == command { + return h, true } } } - t.Errorf("%s was not found (matcher=%q, command=%q)", description, matcher, command) + return ClaudeHookEntry{}, false +} + +// assertHookExists checks that a hook with the given matcher and command exists +func assertHookExists(t *testing.T, matchers []ClaudeHookMatcher, matcher, command, description string) { + t.Helper() + if _, ok := lookupHook(matchers, matcher, command); !ok { + t.Errorf("%s was not found (matcher=%q, command=%q)", description, matcher, command) + } } func TestInstallHooks_PreservesUnknownHookTypes(t *testing.T) { @@ -969,15 +1117,15 @@ func TestInstallHooks_UsesCurrentToolMatchers(t *testing.T) { wantCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop") assertHookExists(t, settings.Hooks.SubagentStop, "", wantCmd, "SubagentStop hook") - // Same shape as Stop: single command, empty matcher, no timeout. + // Same shape as Stop: single command, empty matcher, Stop's timeout. if len(settings.Hooks.SubagentStop) != 1 || settings.Hooks.SubagentStop[0].Matcher != "" { t.Fatalf("SubagentStop = %+v, want a single matcher with an empty string matcher (same shape as Stop)", settings.Hooks.SubagentStop) } if len(settings.Hooks.SubagentStop[0].Hooks) != 1 { t.Fatalf("SubagentStop hooks = %d, want 1", len(settings.Hooks.SubagentStop[0].Hooks)) } - if got := settings.Hooks.SubagentStop[0].Hooks[0].Timeout; got != 0 { - t.Errorf("SubagentStop timeout = %d, want 0 (no explicit timeout, same as Stop)", got) + if got, want := settings.Hooks.SubagentStop[0].Hooks[0].Timeout, settings.Hooks.Stop[0].Hooks[0].Timeout; got != want { + t.Errorf("SubagentStop timeout = %d, want %d (same as Stop)", got, want) } }) } From 8c32a849be72d98022ff623554891d998b0b11b8 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Tue, 13 Oct 2026 21:21:29 +0530 Subject: [PATCH 3/9] fix(claude-code): keep user hook fields, report timeout backfill, and give capture hooks 30s Entire-Checkpoint: 01M4TZCCGFH94AN9B78ZC2RHC2 --- .claude/settings.json | 6 +- cmd/entire/cli/agent/agent.go | 9 +- cmd/entire/cli/agent/claudecode/hooks.go | 21 +-- cmd/entire/cli/agent/claudecode/hooks_test.go | 126 +++++++++++++++++- cmd/entire/cli/agent/claudecode/types.go | 83 +++++++++++- cmd/entire/cli/gitrepo/status.go | 19 +-- cmd/entire/cli/setup.go | 16 +-- cmd/entire/cli/setup_prune_report_test.go | 40 +++++- 8 files changed, 281 insertions(+), 39 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index b632de7b99..7e96dc537b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -41,7 +41,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code pre-task'", - "timeout": 10 + "timeout": 30 } ] } @@ -104,7 +104,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-start'", - "timeout": 10 + "timeout": 30 } ] } @@ -128,7 +128,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code user-prompt-submit'", - "timeout": 10 + "timeout": 30 } ] } diff --git a/cmd/entire/cli/agent/agent.go b/cmd/entire/cli/agent/agent.go index ab33243328..60d23a9079 100644 --- a/cmd/entire/cli/agent/agent.go +++ b/cmd/entire/cli/agent/agent.go @@ -167,15 +167,16 @@ type HookFreshness interface { } // StaleHookReporter is implemented by hook-supporting agents whose install -// also prunes Entire hooks that older CLIs wrote and this one no longer does -// (for Claude Code, the retired post-todo hook). `entire enable` asks before -// installing, so it can say it removed them instead of reporting the hooks as +// also rewrites Entire hooks that older CLIs wrote: pruning ones this CLI no +// longer writes (for Claude Code, the retired post-todo hook) or updating ones +// whose shape changed (Claude Code's hook timeouts). `entire enable` asks before +// installing, so it can say it updated them instead of reporting the hooks as // already installed. Implementations must be read-only. type StaleHookReporter interface { Agent // HasStaleManagedHooks reports whether the agent's hook config holds - // Entire hooks the next install will remove. + // Entire hooks the next install will remove or update. HasStaleManagedHooks(ctx context.Context) bool } diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index 3a27b41992..98420eb149 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -113,7 +113,8 @@ func (c *ClaudeCodeAgent) InstallHooks(ctx context.Context, force bool) (int, er // HasStaleManagedHooks implements agent.StaleHookReporter: whether a plain // install would drop Entire hooks this CLI no longer writes (the retired -// post-todo hook, or a hook left by an older command shape). Read-only: it +// post-todo hook, or a hook left by an older command shape) or give an Entire +// hook the timeout an older CLI wrote it without. Read-only: it // runs the install's hook merge on a freshly loaded copy and discards it. func (c *ClaudeCodeAgent) HasStaleManagedHooks(ctx context.Context) bool { cfg, err := claudeHookConfig(ctx) @@ -124,8 +125,8 @@ func (c *ClaudeCodeAgent) HasStaleManagedHooks(ctx context.Context) bool { if err != nil { return false } - _, staleDropped, _ := installHookEntries(rawHooks, false) - return staleDropped + _, staleDropped, timeoutsSet := installHookEntries(rawHooks, false) + return staleDropped || timeoutsSet } // claudeHookConfig returns .claude/settings.json for the current worktree, @@ -196,9 +197,13 @@ func loadRawClaudeSettingsForInstall(cfg *agent.HookConfigFile) (rawSettings, ra // before main() never returns on its own, so each hook carries a timeout that // lets Claude Code kill it and continue. const ( - // quickHookTimeoutSec bounds hooks that only record state. They finish in - // milliseconds, and Claude Code blocks startup and every prompt on them. + // quickHookTimeoutSec bounds SessionStart, which only records state and + // finishes in milliseconds; Claude Code blocks startup on it. quickHookTimeoutSec = 10 + // captureHookTimeoutSec bounds hooks that snapshot the worktree before a + // prompt or subagent. It must outlast gitrepo.StatusWalkBudget so a slow + // walk degrades on its own instead of being killed, losing the snapshot. + captureHookTimeoutSec = 30 // checkpointHookTimeoutSec bounds hooks that write checkpoint data, whose // cost grows with transcript size; a kill mid-write loses that checkpoint. checkpointHookTimeoutSec = 60 @@ -221,9 +226,9 @@ func entireSimpleHooks() []entireSimpleHook { {"SessionEnd", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code session-end"), 0}, {"Stop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop"), checkpointHookTimeoutSec}, {"StopFailure", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code stop-failure"), checkpointHookTimeoutSec}, - {"SubagentStart", workflowAgentMatcher, agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-start"), quickHookTimeoutSec}, + {"SubagentStart", workflowAgentMatcher, agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-start"), captureHookTimeoutSec}, {"SubagentStop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop"), checkpointHookTimeoutSec}, - {"UserPromptSubmit", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code user-prompt-submit"), quickHookTimeoutSec}, + {"UserPromptSubmit", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code user-prompt-submit"), captureHookTimeoutSec}, } } @@ -364,7 +369,7 @@ func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count timeoutsSet = true } } - if setMissingTimeout(preToolUse, preTaskCmd, quickHookTimeoutSec) { + if setMissingTimeout(preToolUse, preTaskCmd, captureHookTimeoutSec) { timeoutsSet = true } if setMissingTimeout(postToolUse, postTaskCmd, checkpointHookTimeoutSec) { diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 49fc0f8196..f2ada809aa 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -13,6 +13,7 @@ import ( agentpkg "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/testutil" + "github.com/entireio/cli/cmd/entire/cli/gitrepo" "github.com/stretchr/testify/require" ) @@ -580,9 +581,9 @@ func expectedHookTimeouts() []expectedHookTimeout { silent := agentpkg.WrapProductionSilentHookCommand return []expectedHookTimeout{ {"SessionStart", "", agentpkg.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agentpkg.WarningFormatMultiLine), 10}, - {"UserPromptSubmit", "", silent("entire hooks claude-code user-prompt-submit"), 10}, - {"SubagentStart", workflowAgentMatcher, silent("entire hooks claude-code subagent-start"), 10}, - {"PreToolUse", subagentToolMatcher, silent("entire hooks claude-code pre-task"), 10}, + {"UserPromptSubmit", "", silent("entire hooks claude-code user-prompt-submit"), 30}, + {"SubagentStart", workflowAgentMatcher, silent("entire hooks claude-code subagent-start"), 30}, + {"PreToolUse", subagentToolMatcher, silent("entire hooks claude-code pre-task"), 30}, {"Stop", "", silent("entire hooks claude-code stop"), 60}, {"StopFailure", "", silent("entire hooks claude-code stop-failure"), 60}, {"SubagentStop", "", silent("entire hooks claude-code subagent-stop"), 60}, @@ -738,6 +739,125 @@ func TestInstallHooks_TimeoutsAreIdempotent(t *testing.T) { } } +// userStopMatcherJSON is a user-owned Stop matcher carrying fields Entire does +// not model: a prompt hook, an async command hook, and a matcher-level key. +const userStopMatcherJSON = `{ + "matcher": "", + "comment": "team notifications", + "hooks": [ + {"type": "prompt", "prompt": "Check the task is done", "timeout": 30}, + {"type": "command", "command": "./notify.sh > /dev/null && echo ok", "async": true, "statusMessage": "notifying"} + ] +}` + +// writeSettingsWithUserStopMatcher seeds an Entire Stop hook without a timeout +// (as older CLIs wrote it) next to userStopMatcherJSON. +func writeSettingsWithUserStopMatcher(t *testing.T, dir string) { + t.Helper() + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + writeSettingsFile(t, dir, fmt.Sprintf(`{ + "hooks": { + "Stop": [ + {"matcher": "", "hooks": [{"type": "command", "command": %q}]}, + %s + ] + } +}`, stopCmd, userStopMatcherJSON)) +} + +// assertUserStopMatcherIntact fails unless the user's Stop matcher survives +// byte-for-byte in meaning: every field kept and none added. +func assertUserStopMatcherIntact(t *testing.T, dir string) { + t.Helper() + raw, err := os.ReadFile(filepath.Join(dir, ".claude", "settings.json")) + if err != nil { + t.Fatalf("failed to read settings.json: %v", err) + } + var parsed struct { + Hooks struct { + Stop []map[string]any `json:"Stop"` + } `json:"hooks"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatalf("failed to parse settings.json: %v", err) + } + var want map[string]any + if err := json.Unmarshal([]byte(userStopMatcherJSON), &want); err != nil { + t.Fatal(err) + } + for _, m := range parsed.Hooks.Stop { + if m["comment"] != nil { + require.Equal(t, want, m, "user Stop matcher changed") + return + } + } + t.Fatalf("user Stop matcher missing:\n%s", raw) +} + +// TestInstallHooks_PreservesUserHookFields covers the upgrade rewrite: a +// timeout backfill must not strip fields from the user's own hooks. +func TestInstallHooks_PreservesUserHookFields(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + writeSettingsWithUserStopMatcher(t, tempDir) + + ag := &ClaudeCodeAgent{} + if _, err := ag.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + assertUserStopMatcherIntact(t, tempDir) + + raw, err := os.ReadFile(filepath.Join(tempDir, ".claude", "settings.json")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(raw), `\u003e`) || strings.Contains(string(raw), `\u0026`) { + t.Errorf("settings.json must not HTML-escape commands, got:\n%s", raw) + } +} + +func TestUninstallHooks_PreservesUserHookFields(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + writeSettingsWithUserStopMatcher(t, tempDir) + + ag := &ClaudeCodeAgent{} + if err := ag.UninstallHooks(context.Background()); err != nil { + t.Fatalf("UninstallHooks() error = %v", err) + } + + assertUserStopMatcherIntact(t, tempDir) +} + +// TestHasStaleManagedHooks_MissingTimeout lets enable report a timeout +// backfill instead of calling the rewritten config "already installed". +func TestHasStaleManagedHooks_MissingTimeout(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + writeClaudeStopHooks(t, tempDir, []string{agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop")}) + + c := &ClaudeCodeAgent{} + if !c.HasStaleManagedHooks(context.Background()) { + t.Fatal("HasStaleManagedHooks() = false for a hook missing its timeout, want true") + } + if _, err := c.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + if c.HasStaleManagedHooks(context.Background()) { + t.Error("HasStaleManagedHooks() = true after install, want false") + } +} + +// TestCaptureHookTimeoutExceedsStatusWalkBudget keeps Claude Code from killing +// a capture hook before its status walk can give up and degrade on its own. +func TestCaptureHookTimeoutExceedsStatusWalkBudget(t *testing.T) { + t.Parallel() + if time.Duration(captureHookTimeoutSec)*time.Second <= gitrepo.StatusWalkBudget { + t.Errorf("captureHookTimeoutSec = %ds, want more than gitrepo.StatusWalkBudget (%s)", captureHookTimeoutSec, gitrepo.StatusWalkBudget) + } +} + // seedClaudeSettings writes a .claude/settings.json whose Stop hook is stopCmd. func seedClaudeSettings(t *testing.T, dir, stopCmd string) { t.Helper() diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index bb4ba476a8..9d2e709429 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -1,6 +1,13 @@ package claudecode -import "encoding/json" +import ( + "bytes" + "encoding/json" + "maps" + "slices" + + "github.com/entireio/cli/cmd/entire/cli/jsonutil" +) // ClaudeSettings represents the .claude/settings.json structure type ClaudeSettings struct { @@ -24,15 +31,87 @@ type ClaudeHooks struct { type ClaudeHookMatcher struct { Matcher string `json:"matcher"` Hooks []ClaudeHookEntry `json:"hooks"` + extra map[string]json.RawMessage +} + +// claudeHookMatcherFields is ClaudeHookMatcher without its JSON methods. +type claudeHookMatcherFields ClaudeHookMatcher + +func (m *ClaudeHookMatcher) UnmarshalJSON(data []byte) error { + extra, err := unmarshalKeepingExtra(data, (*claudeHookMatcherFields)(m), "matcher", "hooks") + m.extra = extra + return err +} + +func (m ClaudeHookMatcher) MarshalJSON() ([]byte, error) { + return marshalWithExtra(claudeHookMatcherFields(m), m.extra) } // ClaudeHookEntry represents a single hook command type ClaudeHookEntry struct { Type string `json:"type"` - Command string `json:"command"` + Command string `json:"command,omitempty"` // Timeout is the hook's timeout in seconds. Omitted (0) leaves Claude Code's // default in place; set only where a hook needs an explicit budget. Timeout int `json:"timeout,omitempty"` + extra map[string]json.RawMessage +} + +// claudeHookEntryFields is ClaudeHookEntry without its JSON methods. +type claudeHookEntryFields ClaudeHookEntry + +func (e *ClaudeHookEntry) UnmarshalJSON(data []byte) error { + extra, err := unmarshalKeepingExtra(data, (*claudeHookEntryFields)(e), "type", "command", "timeout") + e.extra = extra + return err +} + +func (e ClaudeHookEntry) MarshalJSON() ([]byte, error) { + return marshalWithExtra(claudeHookEntryFields(e), e.extra) +} + +// unmarshalKeepingExtra decodes data into known and returns the keys outside +// knownKeys. Entire rewrites settings.json, which the user and Claude Code own, +// so every field it does not model must survive the round trip. +func unmarshalKeepingExtra(data []byte, known any, knownKeys ...string) (map[string]json.RawMessage, error) { + if err := json.Unmarshal(data, known); err != nil { + return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + } + var extra map[string]json.RawMessage + if err := json.Unmarshal(data, &extra); err != nil { + return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + } + for _, k := range knownKeys { + delete(extra, k) + } + return extra, nil +} + +// marshalWithExtra encodes known, then appends extra in key order, so modeled +// fields keep their usual position and output stays deterministic. +func marshalWithExtra(known any, extra map[string]json.RawMessage) ([]byte, error) { + data, err := jsonutil.MarshalWithNoHTMLEscape(known) + if err != nil || len(extra) == 0 { + return data, err //nolint:wrapcheck // returned from MarshalJSON; encoding/json adds context + } + var buf bytes.Buffer + buf.Write(data[:len(data)-1]) + needComma := len(data) > len("{}") + for _, k := range slices.Sorted(maps.Keys(extra)) { + key, err := jsonutil.MarshalWithNoHTMLEscape(k) + if err != nil { + return nil, err //nolint:wrapcheck // returned from MarshalJSON; encoding/json adds context + } + if needComma { + buf.WriteByte(',') + } + needComma = true + buf.Write(key) + buf.WriteByte(':') + buf.Write(extra[k]) + } + buf.WriteByte('}') + return buf.Bytes(), nil } // sessionInfoRaw is the JSON structure from SessionStart/SessionEnd/Stop hooks. diff --git a/cmd/entire/cli/gitrepo/status.go b/cmd/entire/cli/gitrepo/status.go index 9f595487c0..eff3195d8d 100644 --- a/cmd/entire/cli/gitrepo/status.go +++ b/cmd/entire/cli/gitrepo/status.go @@ -41,15 +41,16 @@ func Status(_ context.Context, repo *git.Repository) (git.Status, error) { // StatusWalkBudget bounds the wall-clock time of one worktree status read on // the agent-hook capture paths — the go-git walk in StatusWithBudget and the // first-checkpoint `git status` subprocess in the checkpoint store. Agents -// time out their hooks at roughly 60s (Claude Code's default), but a -// timed-out hook PROCESS is not killed — an unbounded walk over a -// pathological worktree (e.g. a stray `git init` in $HOME with no .gitignore) -// has been observed grinding for hours at gigabytes of RSS after the agent -// gave up. 20s is chosen as far beyond any healthy repository (a warm walk -// over a large working set finishes in seconds) while leaving the remaining -// ~40s of the agent's timeout for the rest of the capture path — transcript -// copy, tree building, state writes — so the hook still degrades gracefully -// and exits instead of being orphaned. +// time out their hooks (Claude Code's Entire capture hooks at 30s, its +// checkpoint hooks at 60s), but a timed-out hook PROCESS is not killed — an +// unbounded walk over a pathological worktree (e.g. a stray `git init` in +// $HOME with no .gitignore) has been observed grinding for hours at gigabytes +// of RSS after the agent gave up. 20s is chosen as far beyond any healthy +// repository (a warm walk over a large working set finishes in seconds) while +// leaving the rest of the hook's timeout for the remaining capture path — +// transcript copy, tree building, state writes — so the hook still degrades +// gracefully and exits instead of being orphaned. Hook timeouts must stay +// above this budget. const StatusWalkBudget = 20 * time.Second // ErrStatusBudgetExceeded reports that a worktree status walk was abandoned diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index 10abd3214b..beb9ce7a69 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -1984,10 +1984,10 @@ func setupAgentHooks(ctx context.Context, ag agent.Agent, forceHooks bool) (int, } // installAgentHooks installs ag's hooks (setupAgentHooks) and reports whether -// the install also pruned Entire hooks this version no longer writes (for -// Claude Code, the retired post-todo hook). The stale check runs before the -// install, because afterwards there is nothing left to detect. Every enable -// path reports a prune, so a rewritten config is never called "already +// the install also pruned or updated outdated Entire hooks (for Claude Code, +// the retired post-todo hook or a hook missing its timeout). The stale check +// runs before the install, because afterwards there is nothing left to detect. +// Every enable path reports it, so a rewritten config is never called "already // installed" or left unmentioned. func installAgentHooks(ctx context.Context, ag agent.Agent, forceHooks bool) (installed int, prunedStale bool, err error) { if reporter, ok := agent.AsStaleHookReporter(ag); ok { @@ -2000,10 +2000,10 @@ func installAgentHooks(ctx context.Context, ag agent.Agent, forceHooks bool) (in return installed, prunedStale, nil } -// reportPrunedStaleHooks tells the user an install removed outdated Entire -// hooks from ag's config. +// reportPrunedStaleHooks tells the user an install removed or updated outdated +// Entire hooks in ag's config. func reportPrunedStaleHooks(w io.Writer, ag agent.Agent) { - fmt.Fprintf(w, " Removed outdated Entire hooks for %s (hooks no longer used by this version)\n", ag.Description()) + fmt.Fprintf(w, " Updated outdated Entire hooks for %s\n", ag.Description()) } func setupAgentHookSet(ctx context.Context, w io.Writer, agents []agent.Agent, forceHooks bool) ([]agent.Agent, []error) { @@ -2377,7 +2377,7 @@ func setupAgentHooksNonInteractive(ctx context.Context, w io.Writer, ag agent.Ag case installedHooks == 0: fmt.Fprintf(w, " Hooks for %s already installed\n", ag.Description()) case prunedStaleHooks: - fmt.Fprintf(w, " Installed %d hooks for %s and removed outdated ones\n", installedHooks, ag.Description()) + fmt.Fprintf(w, " Installed %d hooks for %s and updated outdated ones\n", installedHooks, ag.Description()) default: fmt.Fprintf(w, " Installed %d hooks for %s\n", installedHooks, ag.Description()) } diff --git a/cmd/entire/cli/setup_prune_report_test.go b/cmd/entire/cli/setup_prune_report_test.go index e0afd1cc52..c5cba9d1ef 100644 --- a/cmd/entire/cli/setup_prune_report_test.go +++ b/cmd/entire/cli/setup_prune_report_test.go @@ -4,6 +4,9 @@ import ( "bytes" "context" "fmt" + "os" + "path/filepath" + "regexp" "testing" "github.com/stretchr/testify/require" @@ -38,11 +41,44 @@ func TestSetupAgentHookSet_ReportsPrunedStaleHooks(t *testing.T) { added, errs := setupAgentHookSet(context.Background(), &out, []agent.Agent{claude}, false) require.Empty(t, errs) require.Len(t, added, 1) - require.Contains(t, out.String(), "Removed outdated Entire hooks for") + require.Contains(t, out.String(), "Updated outdated Entire hooks for") // A second install has nothing left to prune and says nothing about it. out.Reset() _, errs = setupAgentHookSet(context.Background(), &out, []agent.Agent{claude}, false) require.Empty(t, errs) - require.NotContains(t, out.String(), "Removed outdated") + require.NotContains(t, out.String(), "Updated outdated") +} + +// An older install whose hooks only lack their timeouts is rewritten too, so +// the add/reinstall path must report it rather than stay silent. Uses t.Chdir — +// do NOT add t.Parallel(). +func TestSetupAgentHookSet_ReportsTimeoutBackfill(t *testing.T) { + dir := t.TempDir() + testutil.InitRepo(t, dir) + t.Chdir(dir) + paths.ClearWorktreeRootCache() + + claude, err := agent.Get(agent.AgentNameClaudeCode) + require.NoError(t, err) + var out bytes.Buffer + _, errs := setupAgentHookSet(context.Background(), &out, []agent.Agent{claude}, false) + require.Empty(t, errs) + + settingsPath := filepath.Join(dir, ".claude", "settings.json") + raw, err := os.ReadFile(settingsPath) + require.NoError(t, err) + stripped := regexp.MustCompile(`,\s*"timeout": \d+`).ReplaceAll(raw, nil) + require.NotEqual(t, raw, stripped, "fixture must contain timeouts to strip") + require.NoError(t, os.WriteFile(settingsPath, stripped, 0o600)) + + out.Reset() + _, errs = setupAgentHookSet(context.Background(), &out, []agent.Agent{claude}, false) + require.Empty(t, errs) + require.Contains(t, out.String(), "Updated outdated Entire hooks for") + + out.Reset() + _, errs = setupAgentHookSet(context.Background(), &out, []agent.Agent{claude}, false) + require.Empty(t, errs) + require.NotContains(t, out.String(), "Updated outdated") } From 495153073799d9a84e71d7a201bdf83c1802e4b6 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 09:41:48 +0530 Subject: [PATCH 4/9] fix(claude-code): let entire doctor add missing hook timeouts and stop enable calling new hooks outdated Entire-Checkpoint: 01M4W9QYKH916B98HYJBQKM7GC --- cmd/entire/cli/agent/agent.go | 13 ++ cmd/entire/cli/agent/capabilities.go | 6 + cmd/entire/cli/agent/claudecode/hooks.go | 114 ++++++++++++-- cmd/entire/cli/agent/claudecode/hooks_test.go | 146 ++++++++++++++++++ cmd/entire/cli/doctor.go | 39 +++++ cmd/entire/cli/doctor_test.go | 34 ++++ 6 files changed, 335 insertions(+), 17 deletions(-) diff --git a/cmd/entire/cli/agent/agent.go b/cmd/entire/cli/agent/agent.go index 60d23a9079..726ad700a0 100644 --- a/cmd/entire/cli/agent/agent.go +++ b/cmd/entire/cli/agent/agent.go @@ -180,6 +180,19 @@ type StaleHookReporter interface { HasStaleManagedHooks(ctx context.Context) bool } +// HookTimeoutRepairer is implemented by agents whose Entire hooks carry +// timeouts that configs written by older CLIs lack. `entire doctor` runs the +// repair without asking, because a user who already enabled Entire has no +// reason to run enable again. The repair must only give Entire's own hooks a +// missing timeout and leave everything the user chose as it is. +type HookTimeoutRepairer interface { + Agent + + // RepairHookTimeouts adds the missing timeouts and reports whether it + // rewrote the hook config. An absent config is not an error. + RepairHookTimeouts(ctx context.Context) (bool, error) +} + // EffectiveHookDiagnostics marks agents whose effective hook state is reported // by an agent-owned diagnostic surface rather than generic freshness output. type EffectiveHookDiagnostics interface { diff --git a/cmd/entire/cli/agent/capabilities.go b/cmd/entire/cli/agent/capabilities.go index 4743bda64d..b109546167 100644 --- a/cmd/entire/cli/agent/capabilities.go +++ b/cmd/entire/cli/agent/capabilities.go @@ -82,6 +82,12 @@ func AsStaleHookReporter(ag Agent) (StaleHookReporter, bool) { return builtinCapability[StaleHookReporter](ag) } +// AsHookTimeoutRepairer returns the agent as HookTimeoutRepairer if it can +// add the hook timeouts an older CLI wrote its config without. +func AsHookTimeoutRepairer(ag Agent) (HookTimeoutRepairer, bool) { + return builtinCapability[HookTimeoutRepairer](ag) +} + // AsEffectiveHookDiagnostics returns the agent as EffectiveHookDiagnostics if // it owns diagnostics for its effective hook configuration. func AsEffectiveHookDiagnostics(ag Agent) (EffectiveHookDiagnostics, bool) { diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index 98420eb149..313c5c412c 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io/fs" "os" "slices" "strings" @@ -238,17 +239,30 @@ func entireToolUseHookCommands() (preTask, postTask string) { agent.WrapProductionSilentHookCommand("entire hooks claude-code post-task") } -// setMissingTimeout gives every entry running command the timeout, leaving any -// timeout the user already chose. Reports whether it changed an entry. -func setMissingTimeout(matchers []ClaudeHookMatcher, command string, timeout int) bool { - if timeout == 0 { - return false +// entireHookTimeouts maps each Entire hook command to its timeout. SessionEnd +// is left out because it runs without one. +func entireHookTimeouts() map[string]int { + out := make(map[string]int) + for _, h := range entireSimpleHooks() { + if h.timeout != 0 { + out[h.command] = h.timeout + } } + preTask, postTask := entireToolUseHookCommands() + out[preTask] = captureHookTimeoutSec + out[postTask] = checkpointHookTimeoutSec + return out +} + +// setMissingTimeout gives every Entire entry in matchers its timeout from +// timeouts, leaving any timeout the user already chose. Reports whether it +// changed an entry. +func setMissingTimeout(matchers []ClaudeHookMatcher, timeouts map[string]int) bool { changed := false for i := range matchers { for j := range matchers[i].Hooks { hook := &matchers[i].Hooks[j] - if hook.Command == command && hook.Timeout == 0 { + if timeout := timeouts[hook.Command]; timeout != 0 && hook.Timeout == 0 { hook.Timeout = timeout changed = true } @@ -257,6 +271,69 @@ func setMissingTimeout(matchers []ClaudeHookMatcher, command string, timeout int return changed } +// RepairHookTimeouts implements agent.HookTimeoutRepairer. Unlike an install, +// it adds, removes and moves no hook and rewrites only the hooks block, so +// `entire doctor` can run it without asking. A hook type that does not parse is +// skipped rather than rewritten. +func (c *ClaudeCodeAgent) RepairHookTimeouts(ctx context.Context) (bool, error) { + cfg, err := claudeHookConfig(ctx) + if err != nil { + return false, err + } + data, err := cfg.Read() + if errors.Is(err, fs.ErrNotExist) { + return false, nil + } + if err != nil { + return false, err //nolint:wrapcheck // agent.HookConfigFile already names the file in its error + } + var rawSettings, rawHooks map[string]json.RawMessage + if err := json.Unmarshal(data, &rawSettings); err != nil { + return false, fmt.Errorf("failed to parse %s: %w", cfg.Path(), err) + } + hooksRaw, ok := rawSettings["hooks"] + if !ok { + return false, nil + } + if err := json.Unmarshal(hooksRaw, &rawHooks); err != nil { + return false, fmt.Errorf("failed to parse hooks in %s: %w", cfg.Path(), err) + } + + timeouts := entireHookTimeouts() + changed := false + for hookType := range EntireHookCommands() { + raw, ok := rawHooks[hookType] + if !ok { + continue + } + var matchers []ClaudeHookMatcher + if err := json.Unmarshal(raw, &matchers); err != nil || !setMissingTimeout(matchers, timeouts) { + continue + } + updated, err := jsonutil.MarshalWithNoHTMLEscape(matchers) + if err != nil { + return false, fmt.Errorf("failed to marshal %s hooks: %w", hookType, err) + } + rawHooks[hookType] = updated + changed = true + } + if !changed { + return false, nil + } + + if rawSettings["hooks"], err = jsonutil.MarshalWithNoHTMLEscape(rawHooks); err != nil { + return false, fmt.Errorf("failed to marshal hooks: %w", err) + } + output, err := jsonutil.MarshalIndentWithNewline(rawSettings, "", " ") + if err != nil { + return false, fmt.Errorf("failed to marshal settings: %w", err) + } + if err := cfg.Write(output, 0o600); err != nil { + return false, err //nolint:wrapcheck // agent.HookConfigFile already names the file in its error + } + return true, nil +} + // retiredPostTodoHookCommand is the post-todo command older CLIs installed. // It is no longer installed, and an install prunes it, but its subcommand stays // registered and records nothing, so a checkout that still carries it runs @@ -339,6 +416,7 @@ func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count postToolUse = drop(postToolUse, postTaskCmd) // Add hooks if they don't exist + timeouts := entireHookTimeouts() for _, h := range simpleHooks { m := simpleMatchers[h.hookType] exists := hookCommandExists(m, h.command) @@ -346,33 +424,34 @@ func installHookEntries(rawHooks map[string]json.RawMessage, force bool) (count exists = hookCommandExistsWithMatcher(m, h.matcher, h.command) } if !exists { - simpleMatchers[h.hookType] = addHookToMatcher(m, h.matcher, h.command) + simpleMatchers[h.hookType] = addHookToMatcher(m, h.matcher, h.command, h.timeout) count++ } } if !hookCommandExistsWithMatcher(preToolUse, subagentToolMatcher, preTaskCmd) { - preToolUse = addHookToMatcher(preToolUse, subagentToolMatcher, preTaskCmd) + preToolUse = addHookToMatcher(preToolUse, subagentToolMatcher, preTaskCmd, timeouts[preTaskCmd]) count++ } if !hookCommandExistsWithMatcher(postToolUse, subagentToolMatcher, postTaskCmd) { - postToolUse = addHookToMatcher(postToolUse, subagentToolMatcher, postTaskCmd) + postToolUse = addHookToMatcher(postToolUse, subagentToolMatcher, postTaskCmd, timeouts[postTaskCmd]) count++ } if !hookCommandExistsWithMatcher(postToolUse, skillToolMatcher, postTaskCmd) { - postToolUse = addHookToMatcher(postToolUse, skillToolMatcher, postTaskCmd) + postToolUse = addHookToMatcher(postToolUse, skillToolMatcher, postTaskCmd, timeouts[postTaskCmd]) count++ } - // Covers the entries just added and those older CLIs wrote without one. + // Entries just added already carry theirs, so this only reaches entries older + // CLIs wrote without one. for _, h := range simpleHooks { - if setMissingTimeout(simpleMatchers[h.hookType], h.command, h.timeout) { + if setMissingTimeout(simpleMatchers[h.hookType], timeouts) { timeoutsSet = true } } - if setMissingTimeout(preToolUse, preTaskCmd, captureHookTimeoutSec) { + if setMissingTimeout(preToolUse, timeouts) { timeoutsSet = true } - if setMissingTimeout(postToolUse, postTaskCmd, checkpointHookTimeoutSec) { + if setMissingTimeout(postToolUse, timeouts) { timeoutsSet = true } @@ -733,12 +812,13 @@ func hookCommandExistsWithMatcher(matchers []ClaudeHookMatcher, matcherName, com return false } -// addHookToMatcher appends command to the matcher named matcherName, creating it -// if absent. installHookEntries sets the entry's timeout afterwards. -func addHookToMatcher(matchers []ClaudeHookMatcher, matcherName, command string) []ClaudeHookMatcher { +// addHookToMatcher appends command with its timeout to the matcher named +// matcherName, creating it if absent. +func addHookToMatcher(matchers []ClaudeHookMatcher, matcherName, command string, timeout int) []ClaudeHookMatcher { entry := ClaudeHookEntry{ Type: "command", Command: command, + Timeout: timeout, } // If no matcher name, add to a matcher with empty string diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index f2ada809aa..882d14103b 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -3,7 +3,10 @@ package claudecode import ( "context" "encoding/json" + "errors" "fmt" + "io/fs" + "maps" "os" "path/filepath" "slices" @@ -849,6 +852,149 @@ func TestHasStaleManagedHooks_MissingTimeout(t *testing.T) { } } +// TestHasStaleManagedHooks_FreshInstall: hooks a first install adds are new, +// not outdated, so enable must not say it updated them. +func TestHasStaleManagedHooks_FreshInstall(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + if (&ClaudeCodeAgent{}).HasStaleManagedHooks(context.Background()) { + t.Error("HasStaleManagedHooks() = true with no settings file, want false") + } +} + +// TestRepairHookTimeouts_FillsOnlyMissingTimeouts covers the doctor repair: +// it may only give Entire hooks their missing timeout, so the user's matcher, a +// hook type that does not parse, and the permissions block all stay as written. +func TestRepairHookTimeouts_FillsOnlyMissingTimeouts(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + writeSettingsFile(t, tempDir, fmt.Sprintf(`{ + "permissions": {}, + "hooks": { + "SessionStart": {"not": "a matcher list"}, + "Stop": [ + {"matcher": "", "hooks": [{"type": "command", "command": %q}]}, + %s + ] + } +}`, stopCmd, userStopMatcherJSON)) + + c := &ClaudeCodeAgent{} + changed, err := c.RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if !changed { + t.Fatal("RepairHookTimeouts() changed = false, want true") + } + + hook, ok := lookupHook(readClaudeSettingsHooksOnly(t, tempDir, "Stop"), "", stopCmd) + if !ok { + t.Fatal("Entire Stop hook missing after repair") + } + if hook.Timeout != checkpointHookTimeoutSec { + t.Errorf("Stop timeout = %d, want %d", hook.Timeout, checkpointHookTimeoutSec) + } + assertUserStopMatcherIntact(t, tempDir) + + raw, err := os.ReadFile(filepath.Join(tempDir, ".claude", "settings.json")) + if err != nil { + t.Fatal(err) + } + var parsed struct { + Permissions json.RawMessage `json:"permissions"` + Hooks map[string]json.RawMessage `json:"hooks"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatalf("failed to parse settings.json: %v", err) + } + if got := slices.Sorted(maps.Keys(parsed.Hooks)); !slices.Equal(got, []string{"SessionStart", "Stop"}) { + t.Errorf("hook types = %v, want only [SessionStart Stop]", got) + } + require.JSONEq(t, `{"not": "a matcher list"}`, string(parsed.Hooks["SessionStart"]), "unparseable hook type changed") + require.JSONEq(t, `{}`, string(parsed.Permissions), "permissions block changed") + + changed, err = c.RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("second RepairHookTimeouts() error = %v", err) + } + if changed { + t.Error("second RepairHookTimeouts() changed = true, want false") + } +} + +// TestRepairHookTimeouts_NoSettingsFile: doctor runs where Claude Code was +// never set up, and the repair must not create a config there. +func TestRepairHookTimeouts_NoSettingsFile(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + changed, err := (&ClaudeCodeAgent{}).RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if changed { + t.Error("RepairHookTimeouts() changed = true with no settings file") + } + if _, err := os.Stat(filepath.Join(tempDir, ".claude")); !errors.Is(err, fs.ErrNotExist) { + t.Errorf(".claude exists after repair with no settings file (stat err = %v)", err) + } +} + +// TestRepairHookTimeouts_KeepsUserSetTimeout: a timeout the user chose for an +// Entire hook is not missing, so the repair leaves the file untouched. +func TestRepairHookTimeouts_KeepsUserSetTimeout(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + content := fmt.Sprintf(`{ + "hooks": { + "Stop": [{"matcher": "", "hooks": [{"type": "command", "command": %q, "timeout": 300}]}] + } +}`, stopCmd) + writeSettingsFile(t, tempDir, content) + + changed, err := (&ClaudeCodeAgent{}).RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if changed { + t.Error("RepairHookTimeouts() changed = true for a user-set timeout") + } + raw, err := os.ReadFile(filepath.Join(tempDir, ".claude", "settings.json")) + if err != nil { + t.Fatal(err) + } + if string(raw) != content { + t.Errorf("settings.json changed:\n%s", raw) + } +} + +// readClaudeSettingsHooksOnly parses one hook type, for files where another +// hook type is deliberately malformed and readClaudeSettings would fail. +func readClaudeSettingsHooksOnly(t *testing.T, dir, hookType string) []ClaudeHookMatcher { + t.Helper() + raw, err := os.ReadFile(filepath.Join(dir, ".claude", "settings.json")) + if err != nil { + t.Fatalf("failed to read settings.json: %v", err) + } + var parsed struct { + Hooks map[string]json.RawMessage `json:"hooks"` + } + if err := json.Unmarshal(raw, &parsed); err != nil { + t.Fatalf("failed to parse settings.json: %v", err) + } + var matchers []ClaudeHookMatcher + if err := json.Unmarshal(parsed.Hooks[hookType], &matchers); err != nil { + t.Fatalf("failed to parse %s: %v", hookType, err) + } + return matchers +} + // TestCaptureHookTimeoutExceedsStatusWalkBudget keeps Claude Code from killing // a capture hook before its status walk can give up and degrade on its own. func TestCaptureHookTimeoutExceedsStatusWalkBudget(t *testing.T) { diff --git a/cmd/entire/cli/doctor.go b/cmd/entire/cli/doctor.go index c81745635d..7660fe1e27 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -194,6 +194,10 @@ func runSessionsFix(cmd *cobra.Command, force bool) error { // Fixes rather than only reporting: what it removes is a rule Entire wrote. checkRetiredDenyRule(cmd) + // Hook timeouts that configs from older CLIs lack. Fixes rather than only + // reporting: it only fills in a timeout on a hook Entire wrote. + checkHookTimeouts(cmd) + // Hooks left by removed Gemini CLI support. Fixes rather than only // reporting, for the same reason: every entry it removes is Entire's own. checkRetiredGeminiHooks(cmd) @@ -1415,6 +1419,41 @@ func checkRetiredDenyRule(cmd *cobra.Command) { } } +// checkHookTimeouts gives Entire's hooks the timeouts a config written by an +// older CLI lacks, so a stalled `entire` launch cannot block the agent until +// its own default timeout. It repairs without asking for the reason +// checkRetiredDenyRule does: it only sets a timeout on a hook Entire wrote that +// has none, so nothing the user chose is touched. As there, a repair error is +// logged rather than printed. +func checkHookTimeouts(cmd *cobra.Command) { + ctx := cmd.Context() + w := cmd.OutOrStdout() + for _, name := range GetAgentsWithHooksInstalled(ctx) { + ag, err := agent.Get(name) + if err != nil { + continue + } + repairer, ok := agent.AsHookTimeoutRepairer(ag) + if !ok { + continue + } + changed, repairErr := repairer.RepairHookTimeouts(ctx) + if repairErr != nil { + logging.Warn(ctx, "could not add missing hook timeouts", + slog.String("agent", string(ag.Type())), + slog.String("error", repairErr.Error())) + continue + } + if changed { + fmt.Fprintf(w, "%s hooks: MISSING TIMEOUTS\n", ag.Type()) + fmt.Fprintln(w, " Some Entire hooks had no timeout, so a stalled `entire` launch could block") + fmt.Fprintln(w, " the agent until its own default timeout.") + fmt.Fprintln(w, " ✓ Fixed: timeouts added (your own hooks and timeouts are untouched).") + fmt.Fprintln(w, " The settings file changed — commit or revert it as you prefer.") + } + } +} + // checkRetiredGeminiHooks removes the Entire hook entries Gemini CLI support // installed in .gemini/settings.json. Nothing else will: that agent is no // longer registered, so `entire enable`, `entire agent remove` and the uninstall diff --git a/cmd/entire/cli/doctor_test.go b/cmd/entire/cli/doctor_test.go index 2dd0d8f84c..a81b4046be 100644 --- a/cmd/entire/cli/doctor_test.go +++ b/cmd/entire/cli/doctor_test.go @@ -7,6 +7,7 @@ import ( "log/slog" "os" "path/filepath" + "regexp" "runtime" "strings" "testing" @@ -1171,6 +1172,39 @@ func TestCheckHookDrift_ClaudeCodeWarnsWhenOutdated(t *testing.T) { require.Contains(t, out, "entire enable --force") } +// TestCheckHookTimeouts_RestoresWhatEnableWrites — a Claude Code config from +// before hook timeouts existed is repaired to exactly what `entire enable` +// writes today, and a repaired config is left alone on the next run. +func TestCheckHookTimeouts_RestoresWhatEnableWrites(t *testing.T) { + dir := setupGitRepoForPhaseTest(t) + t.Chdir(dir) + + if _, err := (&claudecode.ClaudeCodeAgent{}).InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + settingsPath := filepath.Join(dir, claudeDirName, claudecode.ClaudeSettingsFileName) + want, err := os.ReadFile(settingsPath) + require.NoError(t, err) + + untimed := regexp.MustCompile(`,\s*"timeout": \d+`).ReplaceAll(want, nil) + require.NotEqual(t, want, untimed, "fixture should have had timeouts to strip") + require.NoError(t, os.WriteFile(settingsPath, untimed, 0o600)) + + cmd, stdout := newTestCmd(t) + checkHookTimeouts(cmd) + out := stdout.String() + require.Contains(t, out, "Claude Code hooks: MISSING TIMEOUTS") + require.Contains(t, out, "✓ Fixed: timeouts added (your own hooks and timeouts are untouched).") + + got, err := os.ReadFile(settingsPath) + require.NoError(t, err) + require.Equal(t, string(want), string(got)) + + cmd, stdout = newTestCmd(t) + checkHookTimeouts(cmd) + require.Empty(t, stdout.String()) +} + // TestCheckCodexHookTrust_FlagsStaleHooksFile — user enabled Codex on // an older release that didn't ship PostToolUse. Their hooks.json has // only the three legacy events. Doctor must surface the gap and tell From 1d051b6de1e0fc94633a8e2bb1d9c805f5b77c18 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 10:36:55 +0530 Subject: [PATCH 5/9] fix(claude-code): give checkpoint hooks 120s, keep user hook entries Entire can't model, and report hooks an install would rewrite as outdated Entire-Checkpoint: 01M4WCWVVQS94FPTS8F8PX8FGS --- .claude/settings.json | 10 +- cmd/entire/cli/agent/claudecode/hooks.go | 51 ++++-- cmd/entire/cli/agent/claudecode/hooks_test.go | 165 ++++++++++++++++-- cmd/entire/cli/agent/claudecode/types.go | 40 ++++- cmd/entire/cli/doctor.go | 14 +- cmd/entire/cli/doctor_test.go | 8 + cmd/entire/cli/gitrepo/status.go | 10 +- 7 files changed, 238 insertions(+), 60 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 7e96dc537b..1d1a12a7cf 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -19,7 +19,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'", - "timeout": 60 + "timeout": 120 } ] }, @@ -29,7 +29,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code post-task'", - "timeout": 60 + "timeout": 120 } ] } @@ -80,7 +80,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop'", - "timeout": 60 + "timeout": 120 } ] } @@ -92,7 +92,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code stop-failure'", - "timeout": 60 + "timeout": 120 } ] } @@ -116,7 +116,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then exit 0; fi; exec entire hooks claude-code subagent-stop'", - "timeout": 60 + "timeout": 120 } ] } diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index 313c5c412c..32583d6207 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -195,19 +195,25 @@ func loadRawClaudeSettingsForInstall(cfg *agent.HookConfigFile) (rawSettings, ra } // Claude Code waits on Entire's hooks, and an `entire` launch the OS stalls -// before main() never returns on its own, so each hook carries a timeout that -// lets Claude Code kill it and continue. +// before main() never returns on its own, so each hook carries a timeout after +// which Claude Code cancels it and continues. const ( - // quickHookTimeoutSec bounds SessionStart, which only records state and - // finishes in milliseconds; Claude Code blocks startup on it. + // quickHookTimeoutSec bounds SessionStart, which records state behind a + // network refresh capped at about a second; Claude Code blocks startup on it. quickHookTimeoutSec = 10 // captureHookTimeoutSec bounds hooks that snapshot the worktree before a - // prompt or subagent. It must outlast gitrepo.StatusWalkBudget so a slow - // walk degrades on its own instead of being killed, losing the snapshot. + // prompt or subagent. UserPromptSubmit already runs this work under Claude + // Code's 30s default; SubagentStart and pre-task drop from its 600s command + // default to match. It must outlast gitrepo.StatusWalkBudget so a slow walk + // degrades on its own before Claude Code cancels the hook and drops the + // snapshot. captureHookTimeoutSec = 30 - // checkpointHookTimeoutSec bounds hooks that write checkpoint data, whose - // cost grows with transcript size; a kill mid-write loses that checkpoint. - checkpointHookTimeoutSec = 60 + // checkpointHookTimeoutSec bounds hooks that write checkpoint data. Their + // path can spend checkpoint/remote.WriteProbeFetchBudget on a remote probe + // and gitrepo.StatusWalkBudget on a first-checkpoint status before transcript + // work whose cost grows with transcript size; a hook cancelled mid-write may + // not finish that checkpoint. + checkpointHookTimeoutSec = 120 ) type entireSimpleHook struct { @@ -255,14 +261,14 @@ func entireHookTimeouts() map[string]int { } // setMissingTimeout gives every Entire entry in matchers its timeout from -// timeouts, leaving any timeout the user already chose. Reports whether it -// changed an entry. +// timeouts, leaving any timeout the user already chose, including one Entire +// cannot model and so keeps as written. Reports whether it changed an entry. func setMissingTimeout(matchers []ClaudeHookMatcher, timeouts map[string]int) bool { changed := false for i := range matchers { for j := range matchers[i].Hooks { hook := &matchers[i].Hooks[j] - if timeout := timeouts[hook.Command]; timeout != 0 && hook.Timeout == 0 { + if timeout := timeouts[hook.Command]; timeout != 0 && hook.Timeout == 0 && hook.raw == nil { hook.Timeout = timeout changed = true } @@ -682,9 +688,10 @@ func (c *ClaudeCodeAgent) AreHooksInstalled(ctx context.Context) (bool, error) { // `entire status` and `entire doctor` can treat every agent's drift check // uniformly; the names stay exported here for existing call sites. // -// For Claude Code, HooksOutdated means Entire hooks are installed but the -// current tool-use matchers no longer carry them (e.g. an older CLI wrote them -// under the now non-firing "Task"/"TodoWrite" matchers). +// For Claude Code, HooksOutdated means Entire hooks are installed but a plain +// install would change them: a required hook is missing (e.g. an older CLI +// wrote them under the now non-firing "Task"/"TodoWrite" matchers), or an +// Entire hook is in a shape this CLI no longer writes or lacks its timeout. type HookConfigState = agent.HookConfigState const ( @@ -706,10 +713,13 @@ func (c *ClaudeCodeAgent) CheckHookConfig(ctx context.Context) agent.HookConfigS // CheckHookConfig reports whether Entire's Claude Code hooks are absent, // current, or outdated. It is a read-only diagnostic used by `entire status` -// and `entire doctor`; it never modifies settings. Outdated is detected on the -// positive spec: Entire is installed (Stop hook present) yet one of the current -// tool-use matchers (Agent, Skill), SubagentStart (for -// Workflow agents), SubagentStop, or StopFailure does not carry its Entire hook. +// and `entire doctor`; it never modifies settings. Entire is installed when a +// Stop hook is present. It is outdated when one of the current tool-use +// matchers (Agent, Skill), SubagentStart (for Workflow agents), SubagentStop, +// or StopFailure does not carry its Entire hook, or when HasStaleManagedHooks +// finds an Entire hook an install would prune or give a timeout. Without that +// second check, a hook an older CLI wrote passes as current while a stalled +// `entire` launch can still block Claude Code on it. func CheckHookConfig(ctx context.Context) HookConfigState { settings, err := loadClaudeSettings(ctx) // An unreadable or malformed settings file collapses to HooksAbsent @@ -730,6 +740,9 @@ func CheckHookConfig(ctx context.Context) HookConfigState { !hasEntireHook(settings.Hooks.StopFailure) { return HooksOutdated } + if (&ClaudeCodeAgent{}).HasStaleManagedHooks(ctx) { + return HooksOutdated + } return HooksCurrent } diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 882d14103b..63423a7b20 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -1,6 +1,7 @@ package claudecode import ( + "bytes" "context" "encoding/json" "errors" @@ -9,6 +10,7 @@ import ( "maps" "os" "path/filepath" + "regexp" "slices" "strings" "testing" @@ -17,6 +19,7 @@ import ( agentpkg "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/agent/testutil" "github.com/entireio/cli/cmd/entire/cli/gitrepo" + "github.com/entireio/cli/cmd/entire/cli/jsonutil" "github.com/stretchr/testify/require" ) @@ -587,11 +590,11 @@ func expectedHookTimeouts() []expectedHookTimeout { {"UserPromptSubmit", "", silent("entire hooks claude-code user-prompt-submit"), 30}, {"SubagentStart", workflowAgentMatcher, silent("entire hooks claude-code subagent-start"), 30}, {"PreToolUse", subagentToolMatcher, silent("entire hooks claude-code pre-task"), 30}, - {"Stop", "", silent("entire hooks claude-code stop"), 60}, - {"StopFailure", "", silent("entire hooks claude-code stop-failure"), 60}, - {"SubagentStop", "", silent("entire hooks claude-code subagent-stop"), 60}, - {"PostToolUse", subagentToolMatcher, silent("entire hooks claude-code post-task"), 60}, - {"PostToolUse", skillToolMatcher, silent("entire hooks claude-code post-task"), 60}, + {"Stop", "", silent("entire hooks claude-code stop"), 120}, + {"StopFailure", "", silent("entire hooks claude-code stop-failure"), 120}, + {"SubagentStop", "", silent("entire hooks claude-code subagent-stop"), 120}, + {"PostToolUse", subagentToolMatcher, silent("entire hooks claude-code post-task"), 120}, + {"PostToolUse", skillToolMatcher, silent("entire hooks claude-code post-task"), 120}, // Claude Code gives SessionEnd a short exit grace by default; an explicit // timeout would make it wait longer on exit, not shorter. {"SessionEnd", "", silent("entire hooks claude-code session-end"), 0}, @@ -833,6 +836,83 @@ func TestUninstallHooks_PreservesUserHookFields(t *testing.T) { assertUserStopMatcherIntact(t, tempDir) } +// TestInstallHooks_KeepsUserHooksEntireCannotModel covers user hooks whose +// shape Entire's types do not fully model. A rewrite must leave them exactly as +// written: encoding/json would otherwise read a differently-cased key into a +// modeled field and write it twice, and a modeled field holding another type +// would fail the decode and drop every unmodeled field. +func TestInstallHooks_KeepsUserHooksEntireCannotModel(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + matcher string + }{ + { + name: "differently-cased keys", + matcher: `{"matcher": "user", "Matcher": "x", "hooks": [{"type": "command", "command": "./a.sh", "Command": "./b.sh", "Timeout": 5}]}`, + }, + { + name: "fractional timeout", + matcher: `{"matcher": "user", "hooks": [{"type": "command", "command": "./notify.sh", "timeout": 1.5, "async": true, "statusMessage": "notifying"}]}`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rawHooks := map[string]json.RawMessage{"Stop": json.RawMessage(`[` + tt.matcher + `]`)} + + installHookEntries(rawHooks, false) + + var stop []map[string]any + require.NoError(t, json.Unmarshal(rawHooks["Stop"], &stop)) + var want map[string]any + require.NoError(t, json.Unmarshal([]byte(tt.matcher), &want)) + require.Contains(t, stop, want, "user Stop matcher changed:\n%s", rawHooks["Stop"]) + }) + } +} + +// TestInstallHooks_KeepsFractionalTimeoutOnEntireHook: a timeout the user set on +// an Entire hook is theirs even when Entire cannot model it, so neither install +// nor doctor backfills one, and enable does not report the hook as outdated. +func TestInstallHooks_KeepsFractionalTimeoutOnEntireHook(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + entry := fmt.Sprintf(`{"type": "command", "command": %q, "timeout": 45.5}`, stopCmd) + writeSettingsFile(t, tempDir, fmt.Sprintf(`{"hooks": {"Stop": [{"matcher": "", "hooks": [%s]}]}}`, entry)) + + c := &ClaudeCodeAgent{} + if c.HasStaleManagedHooks(context.Background()) { + t.Error("HasStaleManagedHooks() = true for an Entire hook with a user-set fractional timeout") + } + changed, err := c.RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if changed { + t.Error("RepairHookTimeouts() changed = true for a user-set fractional timeout") + } + if _, err := c.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + + stop := readClaudeSettingsHooksOnly(t, tempDir, "Stop") + var hooks []map[string]any + for _, m := range stop { + data, err := json.Marshal(m.Hooks) + require.NoError(t, err) + var entries []map[string]any + require.NoError(t, json.Unmarshal(data, &entries)) + hooks = append(hooks, entries...) + } + var want map[string]any + require.NoError(t, json.Unmarshal([]byte(entry), &want)) + require.Equal(t, []map[string]any{want}, hooks, "Entire Stop hook changed or duplicated") +} + // TestHasStaleManagedHooks_MissingTimeout lets enable report a timeout // backfill instead of calling the rewritten config "already installed". func TestHasStaleManagedHooks_MissingTimeout(t *testing.T) { @@ -1703,6 +1783,59 @@ func TestCheckHookConfig_Outdated_MissingSkillHook(t *testing.T) { } } +// TestCheckHookConfig_Outdated_WhenInstallWouldRewrite: a config that has every +// required hook but that an install would still rewrite is outdated, so +// `entire status` and `entire doctor` surface it instead of calling it current. +func TestCheckHookConfig_Outdated_WhenInstallWouldRewrite(t *testing.T) { + stopCmd, err := jsonutil.MarshalWithNoHTMLEscape(agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop")) + require.NoError(t, err) + + tests := []struct { + name string + mutate func([]byte) []byte + }{ + { + name: "missing timeouts", + mutate: func(b []byte) []byte { + return regexp.MustCompile(`,\s*"timeout": \d+`).ReplaceAll(b, nil) + }, + }, + { + // Written before the sh -c wrapper existed; a stalled launch blocks it too. + name: "older command shape", + mutate: func(b []byte) []byte { + return bytes.Replace(b, stopCmd, []byte(`"entire hooks claude-code stop"`), 1) + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + a := &ClaudeCodeAgent{} + if _, err := a.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + settingsPath := filepath.Join(tempDir, ".claude", ClaudeSettingsFileName) + data, err := os.ReadFile(settingsPath) + require.NoError(t, err) + mutated := tt.mutate(data) + require.NotEqual(t, data, mutated, "fixture mutation changed nothing") + writeSettingsFile(t, tempDir, string(mutated)) + + if got := CheckHookConfig(context.Background()); got != HooksOutdated { + t.Errorf("CheckHookConfig() = %v, want HooksOutdated", got) + } + if _, err := a.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } + if got := CheckHookConfig(context.Background()); got != HooksCurrent { + t.Errorf("CheckHookConfig() after reinstall = %v, want HooksCurrent", got) + } + }) + } +} + func TestCheckHookConfig_SupersetMatchersAreCurrent(t *testing.T) { tempDir := t.TempDir() t.Chdir(tempDir) @@ -1713,23 +1846,19 @@ func TestCheckHookConfig_SupersetMatchersAreCurrent(t *testing.T) { subagentStop := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop") pre := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code pre-task") post := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code post-task") - todo := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code post-todo") // "Agent|Foo" still covers Agent; "Agent|Skill|Foo" still covers Agent and - // Skill; "TaskCreate|TaskUpdate|TaskGet" still covers TaskCreate and - // TaskUpdate; "workflow-subagent|Explore" still covers workflow-subagent. + // Skill; "workflow-subagent|Explore" still covers workflow-subagent. writeSettingsFile(t, tempDir, fmt.Sprintf(`{ "hooks": { - "Stop": [{"matcher": "", "hooks": [{"type": "command", "command": %q}]}], - "StopFailure": [{"matcher": "", "hooks": [{"type": "command", "command": %q}]}], - "SubagentStart": [{"matcher": "workflow-subagent|Explore", "hooks": [{"type": "command", "command": %q}]}], - "SubagentStop": [{"matcher": "", "hooks": [{"type": "command", "command": %q}]}], - "PreToolUse": [{"matcher": "Agent|Foo", "hooks": [{"type": "command", "command": %q}]}], - "PostToolUse": [ - {"matcher": "Agent|Skill|Foo", "hooks": [{"type": "command", "command": %q}]}, - {"matcher": "TaskCreate|TaskUpdate|TaskGet", "hooks": [{"type": "command", "command": %q}]} - ] + "Stop": [{"matcher": "", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}], + "StopFailure": [{"matcher": "", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}], + "SubagentStart": [{"matcher": "workflow-subagent|Explore", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}], + "SubagentStop": [{"matcher": "", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}], + "PreToolUse": [{"matcher": "Agent|Foo", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}], + "PostToolUse": [{"matcher": "Agent|Skill|Foo", "hooks": [{"type": "command", "command": %q, "timeout": %d}]}] } -}`, stop, stopFailure, subagentStart, subagentStop, pre, post, todo)) +}`, stop, checkpointHookTimeoutSec, stopFailure, checkpointHookTimeoutSec, subagentStart, captureHookTimeoutSec, + subagentStop, checkpointHookTimeoutSec, pre, captureHookTimeoutSec, post, checkpointHookTimeoutSec)) if got := CheckHookConfig(context.Background()); got != HooksCurrent { t.Errorf("CheckHookConfig() = %v, want HooksCurrent (superset matcher)", got) diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index 9d2e709429..0fe3a23376 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -3,6 +3,7 @@ package claudecode import ( "bytes" "encoding/json" + "errors" "maps" "slices" @@ -55,6 +56,10 @@ type ClaudeHookEntry struct { // default in place; set only where a hook needs an explicit budget. Timeout int `json:"timeout,omitempty"` extra map[string]json.RawMessage + // raw holds the entry as the user wrote it when a modeled field has a type + // Entire does not model (such as a fractional timeout). It is written back + // unchanged, since re-encoding the decoded fields would lose that value. + raw json.RawMessage } // claudeHookEntryFields is ClaudeHookEntry without its JSON methods. @@ -62,27 +67,48 @@ type claudeHookEntryFields ClaudeHookEntry func (e *ClaudeHookEntry) UnmarshalJSON(data []byte) error { extra, err := unmarshalKeepingExtra(data, (*claudeHookEntryFields)(e), "type", "command", "timeout") + if typeErr := (*json.UnmarshalTypeError)(nil); errors.As(err, &typeErr) { + // The other modeled fields still decoded, so Entire can recognise its own + // hook; only the write-back has to use the original bytes. + e.raw = slices.Clone(data) + return nil + } e.extra = extra return err } func (e ClaudeHookEntry) MarshalJSON() ([]byte, error) { + if e.raw != nil { + return e.raw, nil + } return marshalWithExtra(claudeHookEntryFields(e), e.extra) } -// unmarshalKeepingExtra decodes data into known and returns the keys outside -// knownKeys. Entire rewrites settings.json, which the user and Claude Code own, -// so every field it does not model must survive the round trip. +// unmarshalKeepingExtra decodes the knownKeys of data into known and returns +// every other key. Entire rewrites settings.json, which the user and Claude Code +// own, so every field it does not model must survive the round trip. +// +// Only exact knownKeys are decoded: encoding/json matches field names +// case-insensitively, so decoding data whole would read a key such as "Command", +// which Claude Code ignores, into the live command field. func unmarshalKeepingExtra(data []byte, known any, knownKeys ...string) (map[string]json.RawMessage, error) { - if err := json.Unmarshal(data, known); err != nil { - return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context - } var extra map[string]json.RawMessage if err := json.Unmarshal(data, &extra); err != nil { return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context } + modeled := make(map[string]json.RawMessage, len(knownKeys)) for _, k := range knownKeys { - delete(extra, k) + if v, ok := extra[k]; ok { + modeled[k] = v + delete(extra, k) + } + } + modeledJSON, err := json.Marshal(modeled) + if err != nil { + return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + } + if err := json.Unmarshal(modeledJSON, known); err != nil { + return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context } return extra, nil } diff --git a/cmd/entire/cli/doctor.go b/cmd/entire/cli/doctor.go index 7660fe1e27..16803fa3ed 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -187,6 +187,12 @@ func runSessionsFix(cmd *cobra.Command, force bool) error { // Agent-specific: does agy actually load the workspace hooks? checkAntigravityHooksLoaded(cmd) + // Hook timeouts that configs from older CLIs lack. Fixes rather than only + // reporting: it only fills in a timeout on a hook Entire wrote. Runs before + // the drift check, which does not look at timeouts, so that check reports on + // the repaired config instead of calling a config missing them OK. + checkHookTimeouts(cmd) + // Agent-specific: Claude Code hook config drift. checkHookDrift(cmd) @@ -194,10 +200,6 @@ func runSessionsFix(cmd *cobra.Command, force bool) error { // Fixes rather than only reporting: what it removes is a rule Entire wrote. checkRetiredDenyRule(cmd) - // Hook timeouts that configs from older CLIs lack. Fixes rather than only - // reporting: it only fills in a timeout on a hook Entire wrote. - checkHookTimeouts(cmd) - // Hooks left by removed Gemini CLI support. Fixes rather than only // reporting, for the same reason: every entry it removes is Entire's own. checkRetiredGeminiHooks(cmd) @@ -1359,8 +1361,8 @@ func checkHookDrift(cmd *cobra.Command) { fmt.Fprintf(w, "✓ %s hook config: OK\n", displayName) case agent.HooksOutdated: fmt.Fprintf(w, "%s hooks: OUT OF DATE\n", displayName) - fmt.Fprintln(w, " The installed hook config no longer matches what this CLI writes,") - fmt.Fprintln(w, " so some or all hooks may silently not fire.") + fmt.Fprintln(w, " The installed hook config no longer matches what this CLI writes, so") + fmt.Fprintln(w, " some hooks may not fire, or may block the agent if `entire` stalls on launch.") fmt.Fprintln(w, " Run `entire enable --force` to update it.") } } diff --git a/cmd/entire/cli/doctor_test.go b/cmd/entire/cli/doctor_test.go index a81b4046be..2e6e6bc150 100644 --- a/cmd/entire/cli/doctor_test.go +++ b/cmd/entire/cli/doctor_test.go @@ -1191,6 +1191,10 @@ func TestCheckHookTimeouts_RestoresWhatEnableWrites(t *testing.T) { require.NoError(t, os.WriteFile(settingsPath, untimed, 0o600)) cmd, stdout := newTestCmd(t) + checkHookDrift(cmd) + require.Contains(t, stdout.String(), "Claude Code hooks: OUT OF DATE") + + cmd, stdout = newTestCmd(t) checkHookTimeouts(cmd) out := stdout.String() require.Contains(t, out, "Claude Code hooks: MISSING TIMEOUTS") @@ -1203,6 +1207,10 @@ func TestCheckHookTimeouts_RestoresWhatEnableWrites(t *testing.T) { cmd, stdout = newTestCmd(t) checkHookTimeouts(cmd) require.Empty(t, stdout.String()) + + cmd, stdout = newTestCmd(t) + checkHookDrift(cmd) + require.Contains(t, stdout.String(), "Claude Code hook config: OK") } // TestCheckCodexHookTrust_FlagsStaleHooksFile — user enabled Codex on diff --git a/cmd/entire/cli/gitrepo/status.go b/cmd/entire/cli/gitrepo/status.go index eff3195d8d..d2aef5fda4 100644 --- a/cmd/entire/cli/gitrepo/status.go +++ b/cmd/entire/cli/gitrepo/status.go @@ -41,11 +41,11 @@ func Status(_ context.Context, repo *git.Repository) (git.Status, error) { // StatusWalkBudget bounds the wall-clock time of one worktree status read on // the agent-hook capture paths — the go-git walk in StatusWithBudget and the // first-checkpoint `git status` subprocess in the checkpoint store. Agents -// time out their hooks (Claude Code's Entire capture hooks at 30s, its -// checkpoint hooks at 60s), but a timed-out hook PROCESS is not killed — an -// unbounded walk over a pathological worktree (e.g. a stray `git init` in -// $HOME with no .gitignore) has been observed grinding for hours at gigabytes -// of RSS after the agent gave up. 20s is chosen as far beyond any healthy +// cancel a hook at its timeout (for Claude Code, see claudecode's +// captureHookTimeoutSec and checkpointHookTimeoutSec), but cancelling has not +// reliably ended the hook PROCESS — an unbounded walk over a pathological +// worktree (e.g. a stray `git init` in $HOME with no .gitignore) has been +// observed grinding for hours at gigabytes of RSS after the agent gave up. 20s is chosen as far beyond any healthy // repository (a warm walk over a large working set finishes in seconds) while // leaving the rest of the hook's timeout for the remaining capture path — // transcript copy, tree building, state writes — so the hook still degrades From 5aee889ba4b55224163127ee3219379f25faea0e Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 11:08:50 +0530 Subject: [PATCH 6/9] fix(claude-code): point outdated hooks to doctor, give SessionStart 30s, and reuse the settings writer for timeout repair Entire-Checkpoint: 01M4WEQA6X0YFD16XX93XQF6BG --- .claude/settings.json | 2 +- cmd/entire/cli/agent/claudecode/hooks.go | 49 ++++++----------- cmd/entire/cli/agent/claudecode/hooks_test.go | 21 ++++++-- cmd/entire/cli/agent/claudecode/types.go | 6 ++- cmd/entire/cli/doctor.go | 7 +-- cmd/entire/cli/status.go | 11 ++-- cmd/entire/cli/status_test.go | 53 ++++++++++++++----- 7 files changed, 90 insertions(+), 59 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 1d1a12a7cf..d24d786f3b 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -68,7 +68,7 @@ { "type": "command", "command": "sh -c 'if ! command -v entire >/dev/null 2>&1; then printf \"%s\\n\" \"{\\\"systemMessage\\\":\\\"\\\\n\\\\nEntire CLI is enabled but not installed or not on PATH.\\\\nInstallation guide: https://docs.entire.io/cli/installation#installation-methods\\\"}\"; exit 0; fi; exec entire hooks claude-code session-start'", - "timeout": 10 + "timeout": 30 } ] } diff --git a/cmd/entire/cli/agent/claudecode/hooks.go b/cmd/entire/cli/agent/claudecode/hooks.go index 32583d6207..b89ef351fa 100644 --- a/cmd/entire/cli/agent/claudecode/hooks.go +++ b/cmd/entire/cli/agent/claudecode/hooks.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "io/fs" "os" "slices" "strings" @@ -198,9 +197,12 @@ func loadRawClaudeSettingsForInstall(cfg *agent.HookConfigFile) (rawSettings, ra // before main() never returns on its own, so each hook carries a timeout after // which Claude Code cancels it and continues. const ( - // quickHookTimeoutSec bounds SessionStart, which records state behind a - // network refresh capped at about a second; Claude Code blocks startup on it. - quickHookTimeoutSec = 10 + // sessionStartHookTimeoutSec bounds SessionStart, which Claude Code blocks + // startup on. Besides a network refresh bounded by the cli package's + // trailEnablementSessionStartRefreshTimeout, it resolves the repository and + // reads and writes session state, which a cold start or a large repository + // can slow well past that refresh. + sessionStartHookTimeoutSec = 30 // captureHookTimeoutSec bounds hooks that snapshot the worktree before a // prompt or subagent. UserPromptSubmit already runs this work under Claude // Code's 30s default; SubagentStart and pre-task drop from its 600s command @@ -227,7 +229,7 @@ type entireSimpleHook struct { // type, each under one matcher (empty for all but SubagentStart). func entireSimpleHooks() []entireSimpleHook { return []entireSimpleHook{ - {"SessionStart", "", agent.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agent.WarningFormatMultiLine), quickHookTimeoutSec}, + {"SessionStart", "", agent.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agent.WarningFormatMultiLine), sessionStartHookTimeoutSec}, // No timeout: Claude Code gives SessionEnd a short exit grace by default, // and an explicit timeout would make it wait longer on exit. {"SessionEnd", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code session-end"), 0}, @@ -278,31 +280,18 @@ func setMissingTimeout(matchers []ClaudeHookMatcher, timeouts map[string]int) bo } // RepairHookTimeouts implements agent.HookTimeoutRepairer. Unlike an install, -// it adds, removes and moves no hook and rewrites only the hooks block, so -// `entire doctor` can run it without asking. A hook type that does not parse is -// skipped rather than rewritten. +// it adds, removes and moves no hook, so `entire doctor` can run it without +// asking. A hook type that does not parse is skipped rather than rewritten. +// The write goes through writeClaudeSettingsFile, so an emptied permissions +// block is dropped as an install would drop it. func (c *ClaudeCodeAgent) RepairHookTimeouts(ctx context.Context) (bool, error) { cfg, err := claudeHookConfig(ctx) if err != nil { return false, err } - data, err := cfg.Read() - if errors.Is(err, fs.ErrNotExist) { - return false, nil - } + rawSettings, rawHooks, rawPermissions, err := loadRawClaudeSettingsForInstall(cfg) if err != nil { - return false, err //nolint:wrapcheck // agent.HookConfigFile already names the file in its error - } - var rawSettings, rawHooks map[string]json.RawMessage - if err := json.Unmarshal(data, &rawSettings); err != nil { - return false, fmt.Errorf("failed to parse %s: %w", cfg.Path(), err) - } - hooksRaw, ok := rawSettings["hooks"] - if !ok { - return false, nil - } - if err := json.Unmarshal(hooksRaw, &rawHooks); err != nil { - return false, fmt.Errorf("failed to parse hooks in %s: %w", cfg.Path(), err) + return false, err } timeouts := entireHookTimeouts() @@ -326,16 +315,8 @@ func (c *ClaudeCodeAgent) RepairHookTimeouts(ctx context.Context) (bool, error) if !changed { return false, nil } - - if rawSettings["hooks"], err = jsonutil.MarshalWithNoHTMLEscape(rawHooks); err != nil { - return false, fmt.Errorf("failed to marshal hooks: %w", err) - } - output, err := jsonutil.MarshalIndentWithNewline(rawSettings, "", " ") - if err != nil { - return false, fmt.Errorf("failed to marshal settings: %w", err) - } - if err := cfg.Write(output, 0o600); err != nil { - return false, err //nolint:wrapcheck // agent.HookConfigFile already names the file in its error + if err := writeClaudeSettingsFile(cfg, rawSettings, rawHooks, rawPermissions); err != nil { + return false, err } return true, nil } diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 63423a7b20..84864a95ca 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -586,7 +586,7 @@ type expectedHookTimeout struct { func expectedHookTimeouts() []expectedHookTimeout { silent := agentpkg.WrapProductionSilentHookCommand return []expectedHookTimeout{ - {"SessionStart", "", agentpkg.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agentpkg.WarningFormatMultiLine), 10}, + {"SessionStart", "", agentpkg.WrapProductionJSONWarningHookCommand("entire hooks claude-code session-start", agentpkg.WarningFormatMultiLine), 30}, {"UserPromptSubmit", "", silent("entire hooks claude-code user-prompt-submit"), 30}, {"SubagentStart", workflowAgentMatcher, silent("entire hooks claude-code subagent-start"), 30}, {"PreToolUse", subagentToolMatcher, silent("entire hooks claude-code pre-task"), 30}, @@ -913,6 +913,21 @@ func TestInstallHooks_KeepsFractionalTimeoutOnEntireHook(t *testing.T) { require.Equal(t, []map[string]any{want}, hooks, "Entire Stop hook changed or duplicated") } +// TestClaudeHookEntry_RedecodeDropsKeptBytes: encoding/json reuses slice +// elements when decoding into an existing value, so bytes kept from an entry +// Entire could not model must not outlive the entry that replaced it. +func TestClaudeHookEntry_RedecodeDropsKeptBytes(t *testing.T) { + t.Parallel() + + matchers := []ClaudeHookMatcher{} + require.NoError(t, json.Unmarshal([]byte(`[{"matcher":"","hooks":[{"type":"command","command":"a","timeout":1.5}]}]`), &matchers)) + require.NoError(t, json.Unmarshal([]byte(`[{"matcher":"","hooks":[{"type":"command","command":"b","timeout":30}]}]`), &matchers)) + + got, err := json.Marshal(matchers) + require.NoError(t, err) + require.JSONEq(t, `[{"matcher":"","hooks":[{"type":"command","command":"b","timeout":30}]}]`, string(got)) +} + // TestHasStaleManagedHooks_MissingTimeout lets enable report a timeout // backfill instead of calling the rewritten config "already installed". func TestHasStaleManagedHooks_MissingTimeout(t *testing.T) { @@ -952,7 +967,7 @@ func TestRepairHookTimeouts_FillsOnlyMissingTimeouts(t *testing.T) { stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") writeSettingsFile(t, tempDir, fmt.Sprintf(`{ - "permissions": {}, + "permissions": {"allow": ["Bash(ls)"]}, "hooks": { "SessionStart": {"not": "a matcher list"}, "Stop": [ @@ -995,7 +1010,7 @@ func TestRepairHookTimeouts_FillsOnlyMissingTimeouts(t *testing.T) { t.Errorf("hook types = %v, want only [SessionStart Stop]", got) } require.JSONEq(t, `{"not": "a matcher list"}`, string(parsed.Hooks["SessionStart"]), "unparseable hook type changed") - require.JSONEq(t, `{}`, string(parsed.Permissions), "permissions block changed") + require.JSONEq(t, `{"allow": ["Bash(ls)"]}`, string(parsed.Permissions), "permissions block changed") changed, err = c.RepairHookTimeouts(context.Background()) if err != nil { diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index 0fe3a23376..d74e98a011 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -39,6 +39,7 @@ type ClaudeHookMatcher struct { type claudeHookMatcherFields ClaudeHookMatcher func (m *ClaudeHookMatcher) UnmarshalJSON(data []byte) error { + *m = ClaudeHookMatcher{} // see ClaudeHookEntry.UnmarshalJSON extra, err := unmarshalKeepingExtra(data, (*claudeHookMatcherFields)(m), "matcher", "hooks") m.extra = extra return err @@ -66,14 +67,17 @@ type ClaudeHookEntry struct { type claudeHookEntryFields ClaudeHookEntry func (e *ClaudeHookEntry) UnmarshalJSON(data []byte) error { + // encoding/json reuses slice elements without zeroing them, so bytes kept + // from a previous decode must not survive into this one. + *e = ClaudeHookEntry{} extra, err := unmarshalKeepingExtra(data, (*claudeHookEntryFields)(e), "type", "command", "timeout") + e.extra = extra if typeErr := (*json.UnmarshalTypeError)(nil); errors.As(err, &typeErr) { // The other modeled fields still decoded, so Entire can recognise its own // hook; only the write-back has to use the original bytes. e.raw = slices.Clone(data) return nil } - e.extra = extra return err } diff --git a/cmd/entire/cli/doctor.go b/cmd/entire/cli/doctor.go index 16803fa3ed..69b01707d4 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -189,8 +189,9 @@ func runSessionsFix(cmd *cobra.Command, force bool) error { // Hook timeouts that configs from older CLIs lack. Fixes rather than only // reporting: it only fills in a timeout on a hook Entire wrote. Runs before - // the drift check, which does not look at timeouts, so that check reports on - // the repaired config instead of calling a config missing them OK. + // the drift check, which counts a missing timeout as drift, so that check + // reports on the repaired config instead of telling the user to run + // `entire enable --force` for something doctor just fixed. checkHookTimeouts(cmd) // Agent-specific: Claude Code hook config drift. @@ -1374,7 +1375,7 @@ func checkHookDrift(cmd *cobra.Command) { // under the repo root need manual approval, which defeats unattended permission // modes, and it guarded a staging buffer rather than the durable copy. // -// This is the one doctor check that repairs without asking, and the reason is +// It repairs without asking, as checkHookTimeouts does, and the reason is // ownership: it deletes only a rule byte-identical to the string Entire itself // wrote, so nothing the user chose is touched. `entire enable` performs the same // removal; this exists because a user who has already enabled Entire has no diff --git a/cmd/entire/cli/status.go b/cmd/entire/cli/status.go index 4fb40c58d4..d2b7eb3674 100644 --- a/cmd/entire/cli/status.go +++ b/cmd/entire/cli/status.go @@ -241,7 +241,10 @@ func formatSettingsStatusShort(ctx context.Context, s *EntireSettings, sty statu b.WriteString(strings.Join(displayNames, ", ")) } - // Warn when installed hooks are out of date (read-only; fix is manual). + // Warn when installed hooks are out of date. Status stays read-only and + // points to doctor rather than `entire enable --force`: doctor repairs + // missing hook timeouts in place, while --force rebuilds every Entire hook + // and drops the timeouts and fields the user set on them. for _, name := range OutdatedHookAgents(ctx) { ag, err := agent.Get(name) if err != nil { @@ -249,7 +252,7 @@ func formatSettingsStatusShort(ctx context.Context, s *EntireSettings, sty statu } b.WriteString("\n") b.WriteString(sty.render(sty.yellow, " ! "+string(ag.Type())+" hooks out of date")) - b.WriteString(sty.render(sty.dim, " · run 'entire enable --force'")) + b.WriteString(sty.render(sty.dim, " · run 'entire doctor'")) } if warning := codexStatusWarning(inspectCodexHookIssue(ctx)); warning != "" { b.WriteString("\n") @@ -1102,8 +1105,8 @@ type statusJSON struct { // `entire status --json` instead of the human footer. Set only on the // success path (mirrors writeAgentHelpHint, which only renders when set up). AgentHelp string `json:"agent_help,omitempty"` - // HooksOutdated lists agents whose installed hook config is out of date and - // should be refreshed with `entire enable --force`. + // HooksOutdated lists agents whose installed hook config is out of date; + // `entire doctor` repairs it or names the fix. HooksOutdated []string `json:"hooks_outdated,omitempty"` // LegacyShadowBranches counts the strict-shape entire/- // shadow branches older versions left behind; `entire doctor` reports and diff --git a/cmd/entire/cli/status_test.go b/cmd/entire/cli/status_test.go index 12335543e8..5eb8badd1c 100644 --- a/cmd/entire/cli/status_test.go +++ b/cmd/entire/cli/status_test.go @@ -1939,7 +1939,47 @@ func TestRunStatusJSON_SecretScanners(t *testing.T) { func TestRunStatusJSON_HooksOutdated(t *testing.T) { setupTestRepo(t) writeSettings(t, testSettingsEnabled) + writeOutdatedClaudeHooks(t) + var stdout bytes.Buffer + if err := runStatus(context.Background(), &stdout, false, true); err != nil { + t.Fatalf("runStatus() error = %v", err) + } + + var result statusJSON + if err := json.Unmarshal(stdout.Bytes(), &result); err != nil { + t.Fatalf("json.Unmarshal() error = %v", err) + } + if !slices.Contains(result.HooksOutdated, "claude-code") { + t.Errorf("Expected hooks_outdated to contain 'claude-code', got %v", result.HooksOutdated) + } +} + +// TestRunStatus_HooksOutdatedPointsToDoctor: `entire enable --force` rebuilds +// every Entire hook and drops the timeouts and fields the user set on them, while +// doctor repairs what it can in place and names the fix for the rest. +func TestRunStatus_HooksOutdatedPointsToDoctor(t *testing.T) { + setupTestRepo(t) + writeSettings(t, testSettingsEnabled) + writeOutdatedClaudeHooks(t) + + var stdout bytes.Buffer + if err := runStatus(context.Background(), &stdout, false, false); err != nil { + t.Fatalf("runStatus() error = %v", err) + } + out := stdout.String() + if !strings.Contains(out, "hooks out of date · run 'entire doctor'") { + t.Errorf("status output does not point outdated hooks to doctor:\n%s", out) + } + if strings.Contains(out, "enable --force") { + t.Errorf("status output suggests enable --force:\n%s", out) + } +} + +// writeOutdatedClaudeHooks writes Claude Code hooks under the retired +// Task/TodoWrite matchers. +func writeOutdatedClaudeHooks(t *testing.T) { + t.Helper() if err := os.MkdirAll(".claude", 0o755); err != nil { t.Fatalf("MkdirAll: %v", err) } @@ -1956,19 +1996,6 @@ func TestRunStatusJSON_HooksOutdated(t *testing.T) { if err := os.WriteFile(".claude/settings.json", []byte(stale), 0o644); err != nil { t.Fatalf("WriteFile: %v", err) } - - var stdout bytes.Buffer - if err := runStatus(context.Background(), &stdout, false, true); err != nil { - t.Fatalf("runStatus() error = %v", err) - } - - var result statusJSON - if err := json.Unmarshal(stdout.Bytes(), &result); err != nil { - t.Fatalf("json.Unmarshal() error = %v", err) - } - if !slices.Contains(result.HooksOutdated, "claude-code") { - t.Errorf("Expected hooks_outdated to contain 'claude-code', got %v", result.HooksOutdated) - } } func TestRunStatusJSON_CodexLinkedWorktreeHooksReportInactiveDiscovery(t *testing.T) { From 51672b705e7d94464405a35caf6c44ea85ecb127 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 11:36:23 +0530 Subject: [PATCH 7/9] fix(claude-code): keep explicit zero hook timeouts and point hook drift to `entire agent add` An explicit "timeout": 0 on an Entire hook read the same as no timeout, so doctor and install replaced it and a rewrite dropped it. It is now written back as the user wrote it. Doctor's drift message suggested `entire enable --force`, which rebuilds Entire's hooks and drops the timeouts and fields users set on them. A bare `entire enable` returns early on an enabled repo, so the message now names `entire agent add `, with --force as a fallback. Doctor's two silent repairs share one loop, and the comment on StatusWalkBudget is rewrapped. Co-Authored-By: Claude Opus 5.5 Entire-Checkpoint: 01M4WG9RQGWP0VEB5P2HKCW8DX --- cmd/entire/cli/agent/claudecode/hooks_test.go | 80 ++++++++++-------- cmd/entire/cli/agent/claudecode/types.go | 28 ++++--- cmd/entire/cli/doctor.go | 84 ++++++++++--------- cmd/entire/cli/doctor_test.go | 2 +- cmd/entire/cli/gitrepo/status.go | 12 +-- 5 files changed, 113 insertions(+), 93 deletions(-) diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 84864a95ca..8349b676c5 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -873,44 +873,56 @@ func TestInstallHooks_KeepsUserHooksEntireCannotModel(t *testing.T) { } } -// TestInstallHooks_KeepsFractionalTimeoutOnEntireHook: a timeout the user set on -// an Entire hook is theirs even when Entire cannot model it, so neither install -// nor doctor backfills one, and enable does not report the hook as outdated. -func TestInstallHooks_KeepsFractionalTimeoutOnEntireHook(t *testing.T) { - tempDir := t.TempDir() - t.Chdir(tempDir) +// TestInstallHooks_KeepsUnmodeledTimeoutOnEntireHook: a timeout the user set on +// an Entire hook is theirs even when Entire cannot model it, either because it +// is fractional or because an explicit 0 reads the same as no timeout. Neither +// install nor doctor backfills one, enable does not report the hook as +// outdated, and a rewrite keeps the value as written. +func TestInstallHooks_KeepsUnmodeledTimeoutOnEntireHook(t *testing.T) { + for _, tt := range []struct { + name string + timeout string + }{ + {"fractional", "45.5"}, + {"explicit zero", "0"}, + } { + t.Run(tt.name, func(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) - stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") - entry := fmt.Sprintf(`{"type": "command", "command": %q, "timeout": 45.5}`, stopCmd) - writeSettingsFile(t, tempDir, fmt.Sprintf(`{"hooks": {"Stop": [{"matcher": "", "hooks": [%s]}]}}`, entry)) + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + entry := fmt.Sprintf(`{"type": "command", "command": %q, "timeout": %s}`, stopCmd, tt.timeout) + writeSettingsFile(t, tempDir, fmt.Sprintf(`{"hooks": {"Stop": [{"matcher": "", "hooks": [%s]}]}}`, entry)) - c := &ClaudeCodeAgent{} - if c.HasStaleManagedHooks(context.Background()) { - t.Error("HasStaleManagedHooks() = true for an Entire hook with a user-set fractional timeout") - } - changed, err := c.RepairHookTimeouts(context.Background()) - if err != nil { - t.Fatalf("RepairHookTimeouts() error = %v", err) - } - if changed { - t.Error("RepairHookTimeouts() changed = true for a user-set fractional timeout") - } - if _, err := c.InstallHooks(context.Background(), false); err != nil { - t.Fatalf("InstallHooks() error = %v", err) - } + c := &ClaudeCodeAgent{} + if c.HasStaleManagedHooks(context.Background()) { + t.Errorf("HasStaleManagedHooks() = true for an Entire hook with user-set timeout %s", tt.timeout) + } + changed, err := c.RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if changed { + t.Errorf("RepairHookTimeouts() changed = true for user-set timeout %s", tt.timeout) + } + if _, err := c.InstallHooks(context.Background(), false); err != nil { + t.Fatalf("InstallHooks() error = %v", err) + } - stop := readClaudeSettingsHooksOnly(t, tempDir, "Stop") - var hooks []map[string]any - for _, m := range stop { - data, err := json.Marshal(m.Hooks) - require.NoError(t, err) - var entries []map[string]any - require.NoError(t, json.Unmarshal(data, &entries)) - hooks = append(hooks, entries...) + stop := readClaudeSettingsHooksOnly(t, tempDir, "Stop") + var hooks []map[string]any + for _, m := range stop { + data, err := json.Marshal(m.Hooks) + require.NoError(t, err) + var entries []map[string]any + require.NoError(t, json.Unmarshal(data, &entries)) + hooks = append(hooks, entries...) + } + var want map[string]any + require.NoError(t, json.Unmarshal([]byte(entry), &want)) + require.Equal(t, []map[string]any{want}, hooks, "Entire Stop hook changed or duplicated") + }) } - var want map[string]any - require.NoError(t, json.Unmarshal([]byte(entry), &want)) - require.Equal(t, []map[string]any{want}, hooks, "Entire Stop hook changed or duplicated") } // TestClaudeHookEntry_RedecodeDropsKeptBytes: encoding/json reuses slice diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index d74e98a011..5d8d644882 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -40,7 +40,7 @@ type claudeHookMatcherFields ClaudeHookMatcher func (m *ClaudeHookMatcher) UnmarshalJSON(data []byte) error { *m = ClaudeHookMatcher{} // see ClaudeHookEntry.UnmarshalJSON - extra, err := unmarshalKeepingExtra(data, (*claudeHookMatcherFields)(m), "matcher", "hooks") + extra, _, err := unmarshalKeepingExtra(data, (*claudeHookMatcherFields)(m), "matcher", "hooks") m.extra = extra return err } @@ -57,9 +57,10 @@ type ClaudeHookEntry struct { // default in place; set only where a hook needs an explicit budget. Timeout int `json:"timeout,omitempty"` extra map[string]json.RawMessage - // raw holds the entry as the user wrote it when a modeled field has a type - // Entire does not model (such as a fractional timeout). It is written back - // unchanged, since re-encoding the decoded fields would lose that value. + // raw holds the entry as the user wrote it when a modeled field has a value + // Entire does not model: a fractional timeout, or an explicit 0 that would + // otherwise read and re-encode as no timeout. It is written back unchanged, + // since re-encoding the decoded fields would lose that value. raw json.RawMessage } @@ -70,7 +71,7 @@ func (e *ClaudeHookEntry) UnmarshalJSON(data []byte) error { // encoding/json reuses slice elements without zeroing them, so bytes kept // from a previous decode must not survive into this one. *e = ClaudeHookEntry{} - extra, err := unmarshalKeepingExtra(data, (*claudeHookEntryFields)(e), "type", "command", "timeout") + extra, present, err := unmarshalKeepingExtra(data, (*claudeHookEntryFields)(e), "type", "command", "timeout") e.extra = extra if typeErr := (*json.UnmarshalTypeError)(nil); errors.As(err, &typeErr) { // The other modeled fields still decoded, so Entire can recognise its own @@ -78,6 +79,9 @@ func (e *ClaudeHookEntry) UnmarshalJSON(data []byte) error { e.raw = slices.Clone(data) return nil } + if err == nil && e.Timeout == 0 && slices.Contains(present, "timeout") { + e.raw = slices.Clone(data) + } return err } @@ -89,32 +93,32 @@ func (e ClaudeHookEntry) MarshalJSON() ([]byte, error) { } // unmarshalKeepingExtra decodes the knownKeys of data into known and returns -// every other key. Entire rewrites settings.json, which the user and Claude Code +// every other key, plus the knownKeys data contained. Entire rewrites settings.json, which the user and Claude Code // own, so every field it does not model must survive the round trip. // // Only exact knownKeys are decoded: encoding/json matches field names // case-insensitively, so decoding data whole would read a key such as "Command", // which Claude Code ignores, into the live command field. -func unmarshalKeepingExtra(data []byte, known any, knownKeys ...string) (map[string]json.RawMessage, error) { - var extra map[string]json.RawMessage +func unmarshalKeepingExtra(data []byte, known any, knownKeys ...string) (extra map[string]json.RawMessage, present []string, err error) { if err := json.Unmarshal(data, &extra); err != nil { - return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + return nil, nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context } modeled := make(map[string]json.RawMessage, len(knownKeys)) for _, k := range knownKeys { if v, ok := extra[k]; ok { modeled[k] = v + present = append(present, k) delete(extra, k) } } modeledJSON, err := json.Marshal(modeled) if err != nil { - return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + return nil, nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context } if err := json.Unmarshal(modeledJSON, known); err != nil { - return nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + return nil, nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context } - return extra, nil + return extra, present, nil } // marshalWithExtra encodes known, then appends extra in key order, so modeled diff --git a/cmd/entire/cli/doctor.go b/cmd/entire/cli/doctor.go index 69b01707d4..6dca89fc7f 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -1337,7 +1337,10 @@ func checkLogSink(cmd *cobra.Command) { // checkHookDrift warns when an installed agent's Entire hook config is out of // date — an older release wrote Claude Code tool matchers that no longer fire, // or a repo committed a Pi/OpenCode extension that the template has since moved -// past. Read-only; the fix is `entire enable --force`. Stays silent for agents +// past. Read-only; the fix is `entire agent add `, which installs +// without --force so the timeouts and fields the user set on Entire's hooks +// survive. A bare `entire enable` would not do: on a repo that is already +// enabled it returns without reinstalling agent hooks. Stays silent for agents // that aren't installed here or don't implement a drift check. func checkHookDrift(cmd *cobra.Command) { ctx := cmd.Context() @@ -1364,7 +1367,9 @@ func checkHookDrift(cmd *cobra.Command) { fmt.Fprintf(w, "%s hooks: OUT OF DATE\n", displayName) fmt.Fprintln(w, " The installed hook config no longer matches what this CLI writes, so") fmt.Fprintln(w, " some hooks may not fire, or may block the agent if `entire` stalls on launch.") - fmt.Fprintln(w, " Run `entire enable --force` to update it.") + fmt.Fprintf(w, " Run `entire agent add %s` to update it.\n", ag.Name()) + fmt.Fprintln(w, " If that does not fix it, add --force, which rebuilds Entire's hooks and") + fmt.Fprintln(w, " drops any changes you made to them.") } } } @@ -1386,40 +1391,24 @@ func checkHookDrift(cmd *cobra.Command) { // left visible rather than hidden: the message says the file changed so the user // can commit or revert it. func checkRetiredDenyRule(cmd *cobra.Command) { - ctx := cmd.Context() - w := cmd.OutOrStdout() - for _, name := range GetAgentsWithHooksInstalled(ctx) { - ag, err := agent.Get(name) - if err != nil { - continue - } + // A repair error is logged rather than printed. An agent whose config will + // not parse never reaches here — AreHooksInstalled fails on it, so + // GetAgentsWithHooksInstalled leaves it out — which leaves only write + // failures, and printing "stale rule" for those would name the wrong + // problem. (That a broken config goes unreported at all is a separate, + // pre-existing gap: agentHookState.unchecked has no consumer.) + repairInstalledAgents(cmd, "could not remove retired deny rule", // One pass: Repair reports whether it found the rule, so a separate // detect call would only read and parse the same file twice. - // - // A repair error is logged rather than printed. An agent whose config - // will not parse never reaches here — AreHooksInstalled fails on it, so - // GetAgentsWithHooksInstalled leaves it out — which leaves only write - // failures, and printing "stale rule" for those would name the wrong - // problem. (That a broken config goes unreported at all is a separate, - // pre-existing gap: agentHookState.unchecked has no consumer.) - changed, repairErr := agent.RepairRetiredMetadataDenyRule(ctx, ag) - if repairErr != nil { - logging.Warn(ctx, "could not remove retired deny rule", - slog.String("agent", string(ag.Type())), - slog.String("error", repairErr.Error())) - continue - } - if changed { - displayName := string(ag.Type()) - fmt.Fprintf(w, "%s permissions: STALE RULE\n", displayName) + agent.RepairRetiredMetadataDenyRule, + func(w io.Writer, ag agent.Agent) { + fmt.Fprintf(w, "%s permissions: STALE RULE\n", ag.Type()) fmt.Fprintf(w, " A retired Entire deny rule (%s) was still present.\n", agent.MetadataDenyRule) fmt.Fprintln(w, " It makes ordinary commands (a recursive grep from the repo root, or any") fmt.Fprintln(w, " command naming that path) need manual approval, and it no longer protects") fmt.Fprintln(w, " anything: the file it guarded is removed once a session is condensed.") fmt.Fprintln(w, " ✓ Fixed: rule removed (your other deny rules are untouched).") - fmt.Fprintln(w, " The settings file changed — commit or revert it as you prefer.") - } - } + }) } // checkHookTimeouts gives Entire's hooks the timeouts a config written by an @@ -1429,6 +1418,28 @@ func checkRetiredDenyRule(cmd *cobra.Command) { // has none, so nothing the user chose is touched. As there, a repair error is // logged rather than printed. func checkHookTimeouts(cmd *cobra.Command) { + repairInstalledAgents(cmd, "could not add missing hook timeouts", + func(ctx context.Context, ag agent.Agent) (bool, error) { + repairer, ok := agent.AsHookTimeoutRepairer(ag) + if !ok { + return false, nil + } + return repairer.RepairHookTimeouts(ctx) + }, + func(w io.Writer, ag agent.Agent) { + fmt.Fprintf(w, "%s hooks: MISSING TIMEOUTS\n", ag.Type()) + fmt.Fprintln(w, " Some Entire hooks had no timeout, so a stalled `entire` launch could block") + fmt.Fprintln(w, " the agent until its own default timeout.") + fmt.Fprintln(w, " ✓ Fixed: timeouts added (your own hooks and timeouts are untouched).") + }) +} + +// repairInstalledAgents runs repair, which fixes an agent's config without +// asking, on every agent with hooks installed here. For each config it changed, +// report says what was fixed, and the user is told the settings file changed: +// it is usually tracked in git, so they can commit or revert it. A repair error +// is logged rather than printed, under failure. +func repairInstalledAgents(cmd *cobra.Command, failure string, repair func(context.Context, agent.Agent) (bool, error), report func(io.Writer, agent.Agent)) { ctx := cmd.Context() w := cmd.OutOrStdout() for _, name := range GetAgentsWithHooksInstalled(ctx) { @@ -1436,23 +1447,16 @@ func checkHookTimeouts(cmd *cobra.Command) { if err != nil { continue } - repairer, ok := agent.AsHookTimeoutRepairer(ag) - if !ok { - continue - } - changed, repairErr := repairer.RepairHookTimeouts(ctx) + changed, repairErr := repair(ctx, ag) if repairErr != nil { - logging.Warn(ctx, "could not add missing hook timeouts", + logging.Warn(ctx, failure, slog.String("agent", string(ag.Type())), slog.String("error", repairErr.Error())) continue } if changed { - fmt.Fprintf(w, "%s hooks: MISSING TIMEOUTS\n", ag.Type()) - fmt.Fprintln(w, " Some Entire hooks had no timeout, so a stalled `entire` launch could block") - fmt.Fprintln(w, " the agent until its own default timeout.") - fmt.Fprintln(w, " ✓ Fixed: timeouts added (your own hooks and timeouts are untouched).") - fmt.Fprintln(w, " The settings file changed — commit or revert it as you prefer.") + report(w, ag) + fmt.Fprintln(w, " The settings file changed. Commit or revert it as you prefer.") } } } diff --git a/cmd/entire/cli/doctor_test.go b/cmd/entire/cli/doctor_test.go index 2e6e6bc150..31bf421618 100644 --- a/cmd/entire/cli/doctor_test.go +++ b/cmd/entire/cli/doctor_test.go @@ -1169,7 +1169,7 @@ func TestCheckHookDrift_ClaudeCodeWarnsWhenOutdated(t *testing.T) { out := stdout.String() require.Contains(t, out, "Claude Code hooks: OUT OF DATE") - require.Contains(t, out, "entire enable --force") + require.Contains(t, out, "Run `entire agent add claude-code` to update it.") } // TestCheckHookTimeouts_RestoresWhatEnableWrites — a Claude Code config from diff --git a/cmd/entire/cli/gitrepo/status.go b/cmd/entire/cli/gitrepo/status.go index d2aef5fda4..c1c45f2247 100644 --- a/cmd/entire/cli/gitrepo/status.go +++ b/cmd/entire/cli/gitrepo/status.go @@ -45,12 +45,12 @@ func Status(_ context.Context, repo *git.Repository) (git.Status, error) { // captureHookTimeoutSec and checkpointHookTimeoutSec), but cancelling has not // reliably ended the hook PROCESS — an unbounded walk over a pathological // worktree (e.g. a stray `git init` in $HOME with no .gitignore) has been -// observed grinding for hours at gigabytes of RSS after the agent gave up. 20s is chosen as far beyond any healthy -// repository (a warm walk over a large working set finishes in seconds) while -// leaving the rest of the hook's timeout for the remaining capture path — -// transcript copy, tree building, state writes — so the hook still degrades -// gracefully and exits instead of being orphaned. Hook timeouts must stay -// above this budget. +// observed grinding for hours at gigabytes of RSS after the agent gave up. +// 20s is chosen as far beyond any healthy repository (a warm walk over a large +// working set finishes in seconds) while leaving the rest of the hook's +// timeout for the remaining capture path — transcript copy, tree building, +// state writes — so the hook still degrades gracefully and exits instead of +// being orphaned. Hook timeouts must stay above this budget. const StatusWalkBudget = 20 * time.Second // ErrStatusBudgetExceeded reports that a worktree status walk was abandoned From 2a01d66a08f60b67bfeefd3719d0f1d67e0bf418 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 12:14:31 +0530 Subject: [PATCH 8/9] test(claude-code): cover the doctor repair dropping an empty permissions block Entire-Checkpoint: 01M4WJFJNCGR5ZCFN2X5HVYWYT --- cmd/entire/cli/agent/claudecode/hooks_test.go | 31 +++++++++++++++++++ cmd/entire/cli/agent/claudecode/types.go | 5 +-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/agent/claudecode/hooks_test.go b/cmd/entire/cli/agent/claudecode/hooks_test.go index 8349b676c5..0c9000c18d 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -1033,6 +1033,37 @@ func TestRepairHookTimeouts_FillsOnlyMissingTimeouts(t *testing.T) { } } +// TestRepairHookTimeouts_DropsEmptyPermissions: the repair writes through the +// same helper as install, so an empty permissions block goes the way install +// drops it rather than surviving only when doctor rewrote the file. +func TestRepairHookTimeouts_DropsEmptyPermissions(t *testing.T) { + tempDir := t.TempDir() + t.Chdir(tempDir) + + stopCmd := agentpkg.WrapProductionSilentHookCommand("entire hooks claude-code stop") + writeSettingsFile(t, tempDir, fmt.Sprintf(`{ + "permissions": {}, + "hooks": {"Stop": [{"matcher": "", "hooks": [{"type": "command", "command": %q}]}]} +}`, stopCmd)) + + changed, err := (&ClaudeCodeAgent{}).RepairHookTimeouts(context.Background()) + if err != nil { + t.Fatalf("RepairHookTimeouts() error = %v", err) + } + if !changed { + t.Fatal("RepairHookTimeouts() changed = false, want true") + } + raw, err := os.ReadFile(filepath.Join(tempDir, ".claude", "settings.json")) + if err != nil { + t.Fatal(err) + } + var parsed map[string]json.RawMessage + require.NoError(t, json.Unmarshal(raw, &parsed)) + if _, ok := parsed["permissions"]; ok { + t.Errorf("empty permissions block kept after repair:\n%s", raw) + } +} + // TestRepairHookTimeouts_NoSettingsFile: doctor runs where Claude Code was // never set up, and the repair must not create a config there. func TestRepairHookTimeouts_NoSettingsFile(t *testing.T) { diff --git a/cmd/entire/cli/agent/claudecode/types.go b/cmd/entire/cli/agent/claudecode/types.go index 5d8d644882..dec6a384d6 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -93,8 +93,9 @@ func (e ClaudeHookEntry) MarshalJSON() ([]byte, error) { } // unmarshalKeepingExtra decodes the knownKeys of data into known and returns -// every other key, plus the knownKeys data contained. Entire rewrites settings.json, which the user and Claude Code -// own, so every field it does not model must survive the round trip. +// every other key, plus the knownKeys data contained. Entire rewrites +// settings.json, which the user and Claude Code own, so every field it does not +// model must survive the round trip. // // Only exact knownKeys are decoded: encoding/json matches field names // case-insensitively, so decoding data whole would read a key such as "Command", From e2a215a843c91c25b6fc02dca02065e1b7351642 Mon Sep 17 00:00:00 2001 From: Muskan Paliwal Date: Wed, 14 Oct 2026 13:01:00 +0530 Subject: [PATCH 9/9] fix(agent): list HookTimeoutRepairer among the built-in capabilities Entire-Checkpoint: 01M4WN4P3E46KC597TSR9TE0A8 --- cmd/entire/cli/agent/capabilities.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/entire/cli/agent/capabilities.go b/cmd/entire/cli/agent/capabilities.go index d37bccb520..a3762133fe 100644 --- a/cmd/entire/cli/agent/capabilities.go +++ b/cmd/entire/cli/agent/capabilities.go @@ -31,6 +31,7 @@ type CapabilityDeclarer interface { // - HomeScopedInventoryExtractor // - HookConfigLocator // - HookFreshness +// - HookTimeoutRepairer // - InventoryAwareExtractor // - LateTranscriptWriter (refuses CapabilityDeclarer agents) // - ModelExtractor