From c6af00dc5a33f48c66ab588d0baa77fc1d8035ae Mon Sep 17 00:00:00 2001 From: Matt Van Horn Date: Sat, 7 Mar 2026 16:20:13 -0800 Subject: [PATCH] refactor(agent): extract hasTTY() env checks to NonInteractiveDetector interface Replace hardcoded GEMINI_CLI, COPILOT_CLI, and GIT_TERMINAL_PROMPT checks in hasTTY() with an optional agent interface. Agents whose hook subprocesses inherit the user's TTY but can't respond to prompts now implement NonInteractiveDetector with their env var, so adding a new agent no longer requires changes to the strategy layer. Closes #601 Co-Authored-By: Claude Opus 4.6 --- cmd/entire/cli/agent/agent.go | 14 ++++ cmd/entire/cli/agent/copilotcli/copilotcli.go | 7 ++ .../agent/factoryaidroid/factoryaidroid.go | 8 ++ cmd/entire/cli/agent/geminicli/gemini.go | 7 ++ cmd/entire/cli/agent/registry.go | 28 +++++++ cmd/entire/cli/agent/registry_test.go | 78 +++++++++++++++++++ .../cli/strategy/manual_commit_hooks.go | 24 +----- 7 files changed, 146 insertions(+), 20 deletions(-) diff --git a/cmd/entire/cli/agent/agent.go b/cmd/entire/cli/agent/agent.go index b5503bc17..dc3974497 100644 --- a/cmd/entire/cli/agent/agent.go +++ b/cmd/entire/cli/agent/agent.go @@ -196,6 +196,20 @@ type TestOnly interface { IsTestOnly() bool } +// NonInteractiveDetector is optionally implemented by agents whose hook +// subprocesses inherit the user's TTY but can't respond to interactive +// prompts. Returns the env var name and value that indicate a non-interactive +// subprocess (e.g., "GEMINI_CLI", "" means any non-empty value). +type NonInteractiveDetector interface { + Agent + + // NonInteractiveEnvVar returns the environment variable name that signals + // a non-interactive subprocess. If value is empty, any non-empty env var + // value triggers non-interactive mode. If value is non-empty, only that + // exact value triggers it. + NonInteractiveEnvVar() (name string, value string) +} + // SubagentAwareExtractor provides methods for extracting files and tokens including subagents. // Agents that support spawning subagents (like Claude Code's Task tool) should implement this // to ensure subagent contributions are included in checkpoints. diff --git a/cmd/entire/cli/agent/copilotcli/copilotcli.go b/cmd/entire/cli/agent/copilotcli/copilotcli.go index 99d717644..19875c586 100644 --- a/cmd/entire/cli/agent/copilotcli/copilotcli.go +++ b/cmd/entire/cli/agent/copilotcli/copilotcli.go @@ -46,6 +46,13 @@ func (c *CopilotCLIAgent) Description() string { // IsPreview returns true because this is a new integration. func (c *CopilotCLIAgent) IsPreview() bool { return true } +// NonInteractiveEnvVar returns the env var that signals a Copilot CLI subprocess. +// Copilot sets COPILOT_CLI=1 when running hook subprocesses (v0.0.421+). +// The subprocess may inherit the user's TTY but can't respond to prompts. +func (c *CopilotCLIAgent) NonInteractiveEnvVar() (string, string) { + return "COPILOT_CLI", "" +} + // DetectPresence checks if Entire hooks are installed in the Copilot CLI config. // Delegates to AreHooksInstalled which checks .github/hooks/entire.json for Entire hook entries. func (c *CopilotCLIAgent) DetectPresence(ctx context.Context) (bool, error) { diff --git a/cmd/entire/cli/agent/factoryaidroid/factoryaidroid.go b/cmd/entire/cli/agent/factoryaidroid/factoryaidroid.go index 862d84aae..746bdc613 100644 --- a/cmd/entire/cli/agent/factoryaidroid/factoryaidroid.go +++ b/cmd/entire/cli/agent/factoryaidroid/factoryaidroid.go @@ -52,6 +52,14 @@ func (f *FactoryAIDroidAgent) Description() string { // IsPreview returns true as Factory AI Droid integration is in preview. func (f *FactoryAIDroidAgent) IsPreview() bool { return true } +// NonInteractiveEnvVar returns the env var that signals a non-interactive subprocess. +// Factory AI Droid (and other CI environments) set GIT_TERMINAL_PROMPT=0 to disable +// git's own terminal prompts. Since hooks run as git subprocesses, this signals +// that interactive prompting should be skipped. +func (f *FactoryAIDroidAgent) NonInteractiveEnvVar() (string, string) { + return "GIT_TERMINAL_PROMPT", "0" +} + // ProtectedDirs returns directories that Factory AI Droid uses for config/state. func (f *FactoryAIDroidAgent) ProtectedDirs() []string { return []string{".factory"} } diff --git a/cmd/entire/cli/agent/geminicli/gemini.go b/cmd/entire/cli/agent/geminicli/gemini.go index 0cbc92ba4..47a53b3fa 100644 --- a/cmd/entire/cli/agent/geminicli/gemini.go +++ b/cmd/entire/cli/agent/geminicli/gemini.go @@ -50,6 +50,13 @@ func (g *GeminiCLIAgent) Description() string { func (g *GeminiCLIAgent) IsPreview() bool { return true } +// NonInteractiveEnvVar returns the env var that signals a Gemini CLI subprocess. +// Gemini sets GEMINI_CLI=1 when running shell commands. The subprocess may +// inherit the user's TTY but can't respond to interactive prompts. +func (g *GeminiCLIAgent) NonInteractiveEnvVar() (string, string) { + return "GEMINI_CLI", "" +} + // DetectPresence checks if Gemini CLI is configured in the repository. func (g *GeminiCLIAgent) DetectPresence(ctx context.Context) (bool, error) { // Get worktree root to check for .gemini directory diff --git a/cmd/entire/cli/agent/registry.go b/cmd/entire/cli/agent/registry.go index 5d518470f..e14937c4e 100644 --- a/cmd/entire/cli/agent/registry.go +++ b/cmd/entire/cli/agent/registry.go @@ -3,6 +3,7 @@ package agent import ( "context" "fmt" + "os" "slices" "sync" @@ -170,6 +171,33 @@ func AllProtectedDirs() []string { return dirs } +// IsNonInteractiveEnv checks whether any registered agent signals that the +// current process is a non-interactive subprocess (e.g., a git hook run by +// an agent that inherits the user's TTY but can't respond to prompts). +func IsNonInteractiveEnv() bool { + registryMu.RLock() + factories := make([]Factory, 0, len(registry)) + for _, f := range registry { + factories = append(factories, f) + } + registryMu.RUnlock() + + for _, factory := range factories { + ag := factory() + if detector, ok := ag.(NonInteractiveDetector); ok { + envName, envValue := detector.NonInteractiveEnvVar() + actual := os.Getenv(envName) + if envValue == "" && actual != "" { + return true + } + if envValue != "" && actual == envValue { + return true + } + } + } + return false +} + // Default returns the default agent. // Returns nil if the default agent is not registered. // diff --git a/cmd/entire/cli/agent/registry_test.go b/cmd/entire/cli/agent/registry_test.go index c456e4287..a0eaa151a 100644 --- a/cmd/entire/cli/agent/registry_test.go +++ b/cmd/entire/cli/agent/registry_test.go @@ -249,6 +249,84 @@ func TestAllProtectedDirs(t *testing.T) { }) } +func TestIsNonInteractiveEnv(t *testing.T) { + // Save original registry state + originalRegistry := make(map[types.AgentName]Factory) + registryMu.Lock() + for k, v := range registry { + originalRegistry[k] = v + } + registry = make(map[types.AgentName]Factory) + registryMu.Unlock() + + defer func() { + registryMu.Lock() + registry = originalRegistry + registryMu.Unlock() + }() + + t.Run("returns false with no agents", func(t *testing.T) { + if IsNonInteractiveEnv() { + t.Error("expected false with empty registry") + } + }) + + t.Run("returns false when env var not set", func(t *testing.T) { + registryMu.Lock() + registry = make(map[types.AgentName]Factory) + registryMu.Unlock() + + Register(types.AgentName("ni-agent"), func() Agent { + return &nonInteractiveAgent{envName: "TEST_NI_AGENT_ENV_XYZ", envValue: ""} + }) + + if IsNonInteractiveEnv() { + t.Error("expected false when env var is not set") + } + }) + + t.Run("returns true when any-value env var is set", func(t *testing.T) { + t.Setenv("TEST_NI_AGENT_ENV_XYZ", "1") + + if !IsNonInteractiveEnv() { + t.Error("expected true when env var is set") + } + }) + + t.Run("returns true when exact-value env var matches", func(t *testing.T) { + registryMu.Lock() + registry = make(map[types.AgentName]Factory) + registryMu.Unlock() + + Register(types.AgentName("exact-agent"), func() Agent { + return &nonInteractiveAgent{envName: "TEST_EXACT_ENV_XYZ", envValue: "0"} + }) + + t.Setenv("TEST_EXACT_ENV_XYZ", "0") + if !IsNonInteractiveEnv() { + t.Error("expected true when env var matches exact value") + } + }) + + t.Run("returns false when exact-value env var does not match", func(t *testing.T) { + t.Setenv("TEST_EXACT_ENV_XYZ", "1") + if IsNonInteractiveEnv() { + t.Error("expected false when env var does not match exact value") + } + }) +} + +// nonInteractiveAgent is a mock that implements NonInteractiveDetector. +type nonInteractiveAgent struct { + mockAgent + envName string + envValue string +} + +func (n *nonInteractiveAgent) NonInteractiveEnvVar() (string, string) { + return n.envName, n.envValue +} + // protectedDirAgent is a mock that returns configurable protected dirs. type protectedDirAgent struct { mockAgent diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index 910c9daf8..1995e320d 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -47,26 +47,10 @@ func hasTTY() bool { return v == "1" } - // Gemini CLI sets GEMINI_CLI=1 when running shell commands. - // Gemini subprocesses may have access to the user's TTY, but they can't - // actually respond to interactive prompts. Treat them as non-TTY. - // See: https://geminicli.com/docs/tools/shell/ - if os.Getenv("GEMINI_CLI") != "" { - return false - } - - // Copilot CLI sets COPILOT_CLI=1 when running hook subprocesses (v0.0.421+). - // Like Gemini, the subprocess may inherit the user's TTY but can't respond - // to interactive prompts. - if os.Getenv("COPILOT_CLI") != "" { - return false - } - - // GIT_TERMINAL_PROMPT=0 disables git's own terminal prompts. - // Factory AI Droid (and other non-interactive environments like CI) set this. - // Since we run as a git hook, respect it — if the environment doesn't want - // git prompting, our hook shouldn't prompt either. - if os.Getenv("GIT_TERMINAL_PROMPT") == "0" { + // Check if any registered agent signals a non-interactive subprocess. + // Agents whose hook subprocesses inherit the user's TTY but can't respond + // to prompts implement NonInteractiveDetector with their env var. + if agent.IsNonInteractiveEnv() { return false }