From e95f3fdf0a66ba05ba2c8fb7e755e107f9cf7991 Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 11 Jun 2026 08:34:41 +0300 Subject: Thread context.Context through blocking I/O entry points Accept ctx as the first parameter on the blocking I/O entry points and propagate it to downstream blocking calls so the work is cancellable from the process entry point: - appconfig.Load / LoadWithOptions: honor ctx before the blocking file reads, returning defaults on a cancelled context. - LSP: lsp.Server.Run(ctx) ties the serve loop to the caller context via a new watchParentContext bridge (cancels the server context, aborting in-flight LLM work). Threaded through hexailsp.Run/RunWithConfig/ RunWithFactory and runtimeconfig.Store.Reload. - MCP: mcp.Server.Run(ctx) stops accepting requests once ctx is cancelled; threaded through hexaimcp.Run/RunWithFactory/RunBackfill. - editor: RunEditor/OpenTempAndEdit/OpenFile take ctx and use exec.CommandContext so a cancelled context kills the editor subprocess; threaded through hexaicli, hexaiaction and askcli call sites. Top-level callers (cmd/hexai-lsp-server, cmd/hexai-mcp-server) now build a signal-cancelled context (SIGINT/SIGTERM) so shutdown tears the run down cleanly. Updated comments to explain the cancellation flow and added cancellation tests for the LSP/MCP loops, editor, and config load. All tests pass with -race; cross-package coverage 86.2%. Co-Authored-By: Claude Opus 4.8 --- internal/appconfig/config_alias_test.go | 3 +- internal/appconfig/config_env_model_test.go | 9 +++-- internal/appconfig/config_features_test.go | 15 ++++---- internal/appconfig/config_load.go | 18 +++++++-- internal/appconfig/config_test.go | 47 ++++++++++++++++------- internal/appconfig/custom_validation_more_test.go | 5 ++- 6 files changed, 66 insertions(+), 31 deletions(-) (limited to 'internal/appconfig') diff --git a/internal/appconfig/config_alias_test.go b/internal/appconfig/config_alias_test.go index da7909e..764611b 100644 --- a/internal/appconfig/config_alias_test.go +++ b/internal/appconfig/config_alias_test.go @@ -1,6 +1,7 @@ package appconfig import ( + "context" "log" "os" "path/filepath" @@ -28,7 +29,7 @@ codex = "gpt-5-codex" if err := os.WriteFile(path, []byte(toml), 0o644); err != nil { t.Fatalf("write: %v", err) } - cfg := Load(log.New(os.Stderr, "test ", 0)) + cfg := Load(context.Background(), log.New(os.Stderr, "test ", 0)) if cfg.OpenAIModel != "gpt-5-codex" { t.Fatalf("expected alias to resolve to gpt-5-codex, got %q", cfg.OpenAIModel) } diff --git a/internal/appconfig/config_env_model_test.go b/internal/appconfig/config_env_model_test.go index e10fa5d..9856a4e 100644 --- a/internal/appconfig/config_env_model_test.go +++ b/internal/appconfig/config_env_model_test.go @@ -1,6 +1,7 @@ package appconfig import ( + "context" "log" "os" "testing" @@ -12,14 +13,14 @@ func TestEnv_GenericModelOverrideAndPrecedence(t *testing.T) { t.Setenv("HEXAI_MODEL", "gpt-5-codex") t.Setenv("HEXAI_PROVIDER", "openai") // No provider-specific env set yet: HEXAI_MODEL should flow into OpenAIModel - cfg := Load(log.New(os.Stderr, "test ", 0)) + cfg := Load(context.Background(), log.New(os.Stderr, "test ", 0)) if cfg.OpenAIModel != "gpt-5-codex" { t.Fatalf("expected OpenAIModel=gpt-5-codex via HEXAI_MODEL, got %q", cfg.OpenAIModel) } // Now set a provider-specific model; it should win over HEXAI_MODEL t.Setenv("HEXAI_OPENAI_MODEL", "gpt-5-thinking") - cfg2 := Load(log.New(os.Stderr, "test ", 0)) + cfg2 := Load(context.Background(), log.New(os.Stderr, "test ", 0)) if cfg2.OpenAIModel != "gpt-5-thinking" { t.Fatalf("expected OpenAIModel from HEXAI_OPENAI_MODEL to win, got %q", cfg2.OpenAIModel) } @@ -30,7 +31,7 @@ func TestEnv_ModelForce_OverridesProviderSpecific(t *testing.T) { t.Setenv("HEXAI_OPENAI_MODEL", "gpt-5-main") t.Setenv("HEXAI_MODEL_FORCE", "gpt-5-codex") t.Setenv("HEXAI_PROVIDER", "openai") - cfg := Load(log.New(os.Stderr, "test ", 0)) + cfg := Load(context.Background(), log.New(os.Stderr, "test ", 0)) if cfg.OpenAIModel != "gpt-5-codex" { t.Fatalf("expected OpenAIModel forced to gpt-5-codex, got %q", cfg.OpenAIModel) } @@ -43,7 +44,7 @@ func TestEnv_SurfaceModelOverrides(t *testing.T) { t.Setenv("HEXAI_MODEL_CLI", "gpt-cli") t.Setenv("HEXAI_TEMPERATURE_CLI", "0.22") t.Setenv("HEXAI_PROVIDER_CLI", "ollama") - cfg := Load(log.New(os.Stderr, "test ", 0)) + cfg := Load(context.Background(), log.New(os.Stderr, "test ", 0)) if len(cfg.CompletionConfigs) != 1 { t.Fatalf("expected single completion entry, got %+v", cfg.CompletionConfigs) } diff --git a/internal/appconfig/config_features_test.go b/internal/appconfig/config_features_test.go index 123283a..77beb07 100644 --- a/internal/appconfig/config_features_test.go +++ b/internal/appconfig/config_features_test.go @@ -3,6 +3,7 @@ package appconfig import ( + "context" "os" "path/filepath" "reflect" @@ -11,7 +12,7 @@ import ( func TestIgnoreConfig_Defaults(t *testing.T) { clearHexaiEnv(t) - cfg := Load(nil) + cfg := Load(context.Background(), nil) if cfg.IgnoreGitignore == nil || !*cfg.IgnoreGitignore { t.Error("expected IgnoreGitignore default true") } @@ -33,7 +34,7 @@ gitignore = false extra_patterns = ["*.min.js", "dist/**"] lsp_notify_ignored = false `) - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) if cfg.IgnoreGitignore == nil || *cfg.IgnoreGitignore { t.Error("expected IgnoreGitignore false from file") } @@ -58,7 +59,7 @@ lsp_notify_ignored = true withEnv(t, "HEXAI_IGNORE_GITIGNORE", "false") withEnv(t, "HEXAI_IGNORE_LSP_NOTIFY", "0") withEnv(t, "HEXAI_IGNORE_EXTRA_PATTERNS", "*.bak,*.tmp") - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) if cfg.IgnoreGitignore == nil || *cfg.IgnoreGitignore { t.Error("expected IgnoreGitignore false from env override") } @@ -90,7 +91,7 @@ gitignore = true gitignore = false extra_patterns = ["build/**"] `) - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: projectDir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: projectDir}) if cfg.IgnoreGitignore == nil || *cfg.IgnoreGitignore { t.Error("expected project override to set IgnoreGitignore false") } @@ -108,7 +109,7 @@ func TestIgnoreConfig_DisableGitignore(t *testing.T) { [ignore] gitignore = false `) - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) if cfg.IgnoreGitignore == nil || *cfg.IgnoreGitignore { t.Error("expected IgnoreGitignore false") } @@ -149,7 +150,7 @@ clear_keys = "C-u" newline_keys = "S-Enter" submit_keys = "Enter" `) - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) if cfg.TmuxEditPopupWidth != "90%" { t.Errorf("PopupWidth = %q, want 90%%", cfg.TmuxEditPopupWidth) } @@ -212,7 +213,7 @@ func TestTmuxEditConfig_SkipsEmptyName(t *testing.T) { name = "" display_name = "Empty" `) - cfg := LoadWithOptions(newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), newLogger(), LoadOptions{ConfigPath: cfgPath, ProjectRoot: dir}) if len(cfg.TmuxEditAgents) != 0 { t.Errorf("got %d agents, want 0 (empty name should be skipped)", len(cfg.TmuxEditAgents)) } diff --git a/internal/appconfig/config_load.go b/internal/appconfig/config_load.go index 3ed1654..1e4c6b7 100644 --- a/internal/appconfig/config_load.go +++ b/internal/appconfig/config_load.go @@ -1,6 +1,7 @@ package appconfig import ( + "context" "fmt" "log" "os" @@ -15,15 +16,26 @@ import ( const ProjectConfigFilename = ".hexaiconfig.toml" // Load reads configuration from a file and merges with defaults. -// It respects the XDG Base Directory Specification. -func Load(logger *log.Logger) App { return LoadWithOptions(logger, LoadOptions{}) } +// It respects the XDG Base Directory Specification. The context lets callers +// abort the (blocking) file reads on shutdown/cancellation. +func Load(ctx context.Context, logger *log.Logger) App { + return LoadWithOptions(ctx, logger, LoadOptions{}) +} // LoadWithOptions reads configuration and applies the requested loading options. -func LoadWithOptions(logger *log.Logger, opts LoadOptions) App { +// ctx is honored before performing the blocking file I/O so a cancelled caller +// gets a clean default config back instead of stalling on disk access. +func LoadWithOptions(ctx context.Context, logger *log.Logger, opts LoadOptions) App { cfg := newDefaultConfig() if logger == nil { return cfg // Return defaults if no logger is provided (e.g. in tests) } + // Respect cancellation up front: config loading touches several files and + // there is no value in starting that work once the caller has given up. + if ctx != nil && ctx.Err() != nil { + logger.Printf("config load cancelled: %v", ctx.Err()) + return cfg + } // Step 1: Load global config file configPath := strings.TrimSpace(opts.ConfigPath) diff --git a/internal/appconfig/config_test.go b/internal/appconfig/config_test.go index d4e7739..06e73a1 100644 --- a/internal/appconfig/config_test.go +++ b/internal/appconfig/config_test.go @@ -2,6 +2,7 @@ package appconfig import ( "bytes" + "context" "errors" "io" "log" @@ -14,6 +15,24 @@ import ( func newLogger() *log.Logger { return log.New(io.Discard, "", 0) } +// TestLoadWithOptions_CancelledContextReturnsDefaults verifies that a +// pre-cancelled context short-circuits config loading (skipping the blocking +// file reads) and returns the default config instead. +func TestLoadWithOptions_CancelledContextReturnsDefaults(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "config.toml") + writeFile(t, cfgPath, "[core]\nmax_tokens = 9999\n") + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + cfg := LoadWithOptions(ctx, newLogger(), LoadOptions{ConfigPath: cfgPath}) + def := newDefaultConfig() + if cfg.MaxTokens != def.MaxTokens { + t.Fatalf("expected default config on cancelled ctx, got MaxTokens=%d", cfg.MaxTokens) + } +} + func writeFile(t *testing.T, path, content string) { t.Helper() if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { @@ -46,7 +65,7 @@ func withEnv(t *testing.T, k, v string) { } func TestLoad_Defaults_NoLogger(t *testing.T) { - cfg := Load(nil) + cfg := Load(context.Background(), nil) if cfg.MaxTokens == 0 || cfg.ContextMode == "" || cfg.ContextWindowLines == 0 || cfg.MaxContextTokens == 0 { t.Fatalf("expected defaults populated, got %+v", cfg) } @@ -60,7 +79,7 @@ func TestLoad_Defaults_WithLogger_NoFile_NoEnv(t *testing.T) { dir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", dir) logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: dir}) def := newDefaultConfig() if cfg.MaxTokens != def.MaxTokens || cfg.ContextMode != def.ContextMode || cfg.ContextWindowLines != def.ContextWindowLines { t.Fatalf("expected defaults; got %+v want %+v", cfg, def) @@ -186,7 +205,7 @@ temperature = 0.0 withEnv(t, "HEXAI_PROVIDER_CLI", "ollama") logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: dir}) // Check overrides if cfg.MaxTokens != 321 || cfg.ContextMode != "always-full" || cfg.ContextWindowLines != 77 || cfg.MaxContextTokens != 888 { @@ -255,7 +274,7 @@ temperature = 0.0 } { t.Setenv(k, "") } - cfg2 := LoadWithOptions(logger, LoadOptions{ProjectRoot: dir}) + cfg2 := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: dir}) if cfg2.MaxTokens != 123 || cfg2.ContextMode != "file-on-new-func" || cfg2.ContextWindowLines != 50 || cfg2.MaxContextTokens != 999 || cfg2.LogPreviewLimit != 0 { t.Fatalf("file merge not applied: %+v", cfg2) } @@ -434,7 +453,7 @@ temperature = 0.0 // Ensure no env override interferes with manual_invoke_min_prefix in this test t.Setenv("HEXAI_MANUAL_INVOKE_MIN_PREFIX", "") logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{ProjectRoot: dir}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: dir}) if cfg.MaxTokens != 111 || cfg.ContextMode != "window" || cfg.ContextWindowLines != 42 || cfg.MaxContextTokens != 777 { t.Fatalf("sectioned basics wrong: %+v", cfg) @@ -495,7 +514,7 @@ explain_system = "CLI-EXPLAIN" ` writeFile(t, cfgPath, content) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) // completion if cfg.PromptCompletionSystemGeneral != "SYS-GENERAL" || cfg.PromptCompletionSystemParams != "SYS-PARAMS" || cfg.PromptCompletionSystemInline != "SYS-INLINE" { @@ -557,7 +576,7 @@ user = "Diagnostics to resolve (selection only):\n{{diagnostics}}\n\nSelected co custom_menu_hotkey = "a" ` writeFile(t, cfgPath, content) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err != nil { t.Fatalf("validate: %v", err) } @@ -592,7 +611,7 @@ id = "DUP" title = "B" instruction = "y" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "duplicate custom action id") { t.Fatalf("expected duplicate id error, got %v", err) } @@ -616,7 +635,7 @@ title = "B" instruction = "y" hotkey = "E" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "duplicate custom action hotkey") { t.Fatalf("expected duplicate hotkey error, got %v", err) } @@ -635,7 +654,7 @@ title = "A" instruction = "x" scope = "bad" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "invalid scope") { t.Fatalf("expected invalid scope error, got %v", err) } @@ -650,7 +669,7 @@ func TestTmuxMenuHotkey_Clash_Error(t *testing.T) { [tmux] custom_menu_hotkey = "r" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "clashes with built-in") { t.Fatalf("expected clash error, got %v", err) } @@ -727,7 +746,7 @@ name = "anthropic" // Load using explicit ProjectRoot (avoids needing to chdir) logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{ProjectRoot: projectDir}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: projectDir}) // Project config should override global values if cfg.MaxTokens != 8000 { @@ -768,7 +787,7 @@ max_tokens = 8000 withEnv(t, "HEXAI_MAX_TOKENS", "9999") logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{ProjectRoot: projectDir}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{ProjectRoot: projectDir}) if cfg.MaxTokens != 9999 { t.Fatalf("expected env max_tokens=9999 to override project, got %d", cfg.MaxTokens) @@ -799,7 +818,7 @@ max_tokens = 2000 } logger := newLogger() - cfg := LoadWithOptions(logger, LoadOptions{}) + cfg := LoadWithOptions(context.Background(), logger, LoadOptions{}) // Should get global config values, not defaults if cfg.MaxTokens != 2000 { diff --git a/internal/appconfig/custom_validation_more_test.go b/internal/appconfig/custom_validation_more_test.go index 36212aa..255963b 100644 --- a/internal/appconfig/custom_validation_more_test.go +++ b/internal/appconfig/custom_validation_more_test.go @@ -1,6 +1,7 @@ package appconfig import ( + "context" "path/filepath" "strings" "testing" @@ -19,7 +20,7 @@ instruction = "x" id = "no-title" instruction = "x" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || (!strings.Contains(err.Error(), "missing required field id") && !strings.Contains(err.Error(), "missing required field title")) { t.Fatalf("expected missing field error, got %v", err) } @@ -40,7 +41,7 @@ hotkey = "too" [tmux] custom_menu_hotkey = "ab" `) - cfg := Load(newLogger()) + cfg := Load(context.Background(), newLogger()) if err := cfg.Validate(); err == nil || (!strings.Contains(err.Error(), "hotkey must be a single character") && !strings.Contains(err.Error(), "invalid tmux.custom_menu_hotkey")) { t.Fatalf("expected invalid hotkey error, got %v", err) } -- cgit v1.2.3