diff --git a/.claude/settings.json b/.claude/settings.json index 17f53b2a76..d24d786f3b 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": 120 } ] }, @@ -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": 120 } ] } @@ -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": 30 } ] } @@ -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": 30 } ] } @@ -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": 120 } ] } @@ -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": 120 } ] } @@ -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": 30 } ] } @@ -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": 120 } ] } @@ -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": 30 } ] } diff --git a/cmd/entire/cli/agent/agent.go b/cmd/entire/cli/agent/agent.go index ab33243328..726ad700a0 100644 --- a/cmd/entire/cli/agent/agent.go +++ b/cmd/entire/cli/agent/agent.go @@ -167,18 +167,32 @@ 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 } +// 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 1cd5d7ebf5..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 @@ -131,6 +132,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 c20809443f..b89ef351fa 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 } @@ -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, @@ -192,23 +193,51 @@ 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 after +// which Claude Code cancels it and continues. +const ( + // 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 + // 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. 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 { 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), 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}, + {"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"), captureHookTimeoutSec}, + {"SubagentStop", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code subagent-stop"), checkpointHookTimeoutSec}, + {"UserPromptSubmit", "", agent.WrapProductionSilentHookCommand("entire hooks claude-code user-prompt-submit"), captureHookTimeoutSec}, } } @@ -218,6 +247,80 @@ func entireToolUseHookCommands() (preTask, postTask string) { agent.WrapProductionSilentHookCommand("entire hooks claude-code post-task") } +// 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, 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 && hook.raw == nil { + hook.Timeout = timeout + changed = true + } + } + } + return changed +} + +// RepairHookTimeouts implements agent.HookTimeoutRepairer. Unlike an install, +// 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 + } + rawSettings, rawHooks, rawPermissions, err := loadRawClaudeSettingsForInstall(cfg) + if err != nil { + return false, 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 err := writeClaudeSettingsFile(cfg, rawSettings, rawHooks, rawPermissions); err != nil { + return false, err + } + 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 @@ -243,11 +346,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) @@ -299,6 +403,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) @@ -306,23 +411,37 @@ 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++ } + // 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], timeouts) { + timeoutsSet = true + } + } + if setMissingTimeout(preToolUse, timeouts) { + timeoutsSet = true + } + if setMissingTimeout(postToolUse, timeouts) { + timeoutsSet = true + } + // Marshal modified hook types back to rawHooks for _, h := range simpleHooks { marshalHookType(rawHooks, h.hookType, simpleMatchers[h.hookType]) @@ -330,7 +449,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 @@ -550,9 +669,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 ( @@ -574,10 +694,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 @@ -598,6 +721,9 @@ func CheckHookConfig(ctx context.Context) HookConfigState { !hasEntireHook(settings.Hooks.StopFailure) { return HooksOutdated } + if (&ClaudeCodeAgent{}).HasStaleManagedHooks(ctx) { + return HooksOutdated + } return HooksCurrent } @@ -680,15 +806,13 @@ func hookCommandExistsWithMatcher(matchers []ClaudeHookMatcher, matcherName, com return false } -// 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.) -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 f056660c13..0c9000c18d 100644 --- a/cmd/entire/cli/agent/claudecode/hooks_test.go +++ b/cmd/entire/cli/agent/claudecode/hooks_test.go @@ -1,17 +1,25 @@ package claudecode import ( + "bytes" "context" "encoding/json" + "errors" "fmt" + "io/fs" + "maps" "os" "path/filepath" + "regexp" "slices" "strings" "testing" + "time" 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" ) @@ -567,10 +575,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), 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}, + {"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}, + } +} + +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 +644,501 @@ 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") + assertHookTimeouts(t, settings) + + // 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) + } + var parsed struct { + Hooks map[string][]struct { + Hooks []map[string]json.RawMessage `json:"hooks"` + } `json:"hooks"` } - if got := settings.Hooks.SessionEnd[0].Hooks[0].Timeout; got != 0 { - t.Errorf("SessionEnd timeout = %d, want 0", got) + 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) } - // Stronger than the parsed check: prove the field is omitted entirely - // (omitempty), not written as an explicit "timeout": 0. + 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") + } +} + +// 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) +} + +// 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_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": %s}`, stopCmd, tt.timeout) + writeSettingsFile(t, tempDir, fmt.Sprintf(`{"hooks": {"Stop": [{"matcher": "", "hooks": [%s]}]}}`, entry)) + + 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...) + } + 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 +// 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) { + 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") + } +} + +// 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": {"allow": ["Bash(ls)"]}, + "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, `{"allow": ["Bash(ls)"]}`, 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_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) { + 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) } - 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]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) { + t.Parallel() + if time.Duration(captureHookTimeoutSec)*time.Second <= gitrepo.StatusWalkBudget { + t.Errorf("captureHookTimeoutSec = %ds, want more than gitrepo.StatusWalkBudget (%s)", captureHookTimeoutSec, gitrepo.StatusWalkBudget) } } @@ -777,19 +1321,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 +1521,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) } }) } @@ -1289,6 +1841,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) @@ -1299,23 +1904,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 bb4ba476a8..dec6a384d6 100644 --- a/cmd/entire/cli/agent/claudecode/types.go +++ b/cmd/entire/cli/agent/claudecode/types.go @@ -1,6 +1,14 @@ package claudecode -import "encoding/json" +import ( + "bytes" + "encoding/json" + "errors" + "maps" + "slices" + + "github.com/entireio/cli/cmd/entire/cli/jsonutil" +) // ClaudeSettings represents the .claude/settings.json structure type ClaudeSettings struct { @@ -24,15 +32,121 @@ 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 { + *m = ClaudeHookMatcher{} // see ClaudeHookEntry.UnmarshalJSON + 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 + // 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 +} + +// claudeHookEntryFields is ClaudeHookEntry without its JSON methods. +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, 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 + // hook; only the write-back has to use the original bytes. + e.raw = slices.Clone(data) + return nil + } + if err == nil && e.Timeout == 0 && slices.Contains(present, "timeout") { + e.raw = slices.Clone(data) + } + return err +} + +func (e ClaudeHookEntry) MarshalJSON() ([]byte, error) { + if e.raw != nil { + return e.raw, nil + } + return marshalWithExtra(claudeHookEntryFields(e), e.extra) +} + +// 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. +// +// 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) (extra map[string]json.RawMessage, present []string, err error) { + if err := json.Unmarshal(data, &extra); err != nil { + 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, nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + } + if err := json.Unmarshal(modeledJSON, known); err != nil { + return nil, nil, err //nolint:wrapcheck // returned from UnmarshalJSON; encoding/json adds context + } + return extra, present, 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/doctor.go b/cmd/entire/cli/doctor.go index c81745635d..6dca89fc7f 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -187,6 +187,13 @@ 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 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. checkHookDrift(cmd) @@ -1330,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() @@ -1355,9 +1365,11 @@ 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, " Run `entire enable --force` to update it.") + 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.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.") } } } @@ -1368,7 +1380,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 @@ -1379,6 +1391,55 @@ 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) { + // 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. + 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).") + }) +} + +// 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) { + 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) { @@ -1386,31 +1447,16 @@ func checkRetiredDenyRule(cmd *cobra.Command) { if err != nil { continue } - // 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) + changed, repairErr := repair(ctx, ag) if repairErr != nil { - logging.Warn(ctx, "could not remove retired deny rule", + logging.Warn(ctx, failure, 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) - 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.") + 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 2dd0d8f84c..31bf421618 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" @@ -1168,7 +1169,48 @@ 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 +// 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) + 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") + 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()) + + 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 9f595487c0..c1c45f2247 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. +// 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 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") } 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) {