From a95714a22dea8bf4ec9205f56d47f326d72c6d76 Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Fri, 8 May 2026 10:39:14 +0300 Subject: [PATCH] fix(kit): resolve system-prompt file path before PromptBuilder (#25) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When system-prompt was a file path (via --system-prompt, config entry, or SDK Options.SystemPrompt), the path string itself was used as the base prompt because config.LoadSystemPrompt only ran later in BuildProviderConfig — by which point viper had been overwritten with the path-augmented composed text. The LLM received the path instead of the prompt contents. - Call config.LoadSystemPrompt on the raw viper value in New() before PromptBuilder composes runtime context (AGENTS.md / skills / date). - Add HasCustomSystemPrompt() and GetSystemPromptSource() so SDK callers can inspect prompt state without reaching into viper. - Display 'System Prompt loaded: ' at startup in CLI and TUI modes, paralleling the per-server 'MCP server loaded' notice. - Add regression tests covering both file-path and inline prompt paths. Fixes #25 --- cmd/root.go | 16 ++++++++ pkg/kit/kit.go | 36 +++++++++++++++++- pkg/kit/kit_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index 4b9e90ab..7a51bbcc 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -784,6 +784,16 @@ func runNormalMode(ctx context.Context) error { } defer func() { _ = kitInstance.Close() }() + // Build the "System Prompt loaded" notice shown at startup, paralleling the + // per-server "MCP server loaded" notifications so users can confirm that a + // configured prompt file was found and applied. + var systemPromptLoadedMsg string + if kitInstance.HasCustomSystemPrompt() { + if src := kitInstance.GetSystemPromptSource(); src != "" { + systemPromptLoadedMsg = "System Prompt loaded: " + src + } + } + // Extract metadata for display and app options. parsedProvider, modelName, serverNames, toolNames, mcpToolCount, extensionToolCount := CollectAgentMetadata(kitInstance, mcpConfig) @@ -801,6 +811,9 @@ func runNormalMode(ctx context.Context) error { } DisplayDebugConfig(cli, kitInstance, mcpConfig, parsedProvider) + if systemPromptLoadedMsg != "" { + cli.DisplayInfo(systemPromptLoadedMsg) + } } // Load existing messages from resumed/continued sessions. @@ -840,6 +853,9 @@ func runNormalMode(ctx context.Context) error { // Buffer for extension messages during startup (printed after startup banner). var startupExtensionMessages []string + if systemPromptLoadedMsg != "" { + startupExtensionMessages = append(startupExtensionMessages, systemPromptLoadedMsg) + } // Set up extension context and emit SessionStart. if kitInstance.Extensions().HasExtensions() { diff --git a/pkg/kit/kit.go b/pkg/kit/kit.go index 25e6484f..b42f469f 100644 --- a/pkg/kit/kit.go +++ b/pkg/kit/kit.go @@ -58,6 +58,9 @@ type Kit struct { // When false, per-model system prompts from modelSettings/customModels // can replace the default prompt on model switch. hasCustomSystemPrompt bool + // systemPromptSource holds the raw configured value (file path or text) + // when hasCustomSystemPrompt is true; empty when the built-in default is in use. + systemPromptSource string // Hook registries — interception layer (see hooks.go). beforeToolCall *hookRegistry[BeforeToolCallHook, BeforeToolCallResult] @@ -632,6 +635,21 @@ func (m *Kit) SetModel(ctx context.Context, modelString string) error { return nil } +// HasCustomSystemPrompt reports whether the user explicitly configured a system +// prompt via --system-prompt, a config file entry, or SDK Options.SystemPrompt. +// When false, the built-in default (or a per-model override) is in use and can +// be replaced transparently on model switch. +func (m *Kit) HasCustomSystemPrompt() bool { + return m.hasCustomSystemPrompt +} + +// GetSystemPromptSource returns the raw configured value — a file path or +// inline text — when HasCustomSystemPrompt is true; returns an empty string +// when the built-in default prompt is active. +func (m *Kit) GetSystemPromptSource() string { + return m.systemPromptSource +} + // composeSystemPrompt takes a base system prompt and composes it with the // current runtime context: AGENTS.md content, skills metadata, and date/cwd. // This mirrors the composition done during Kit.New() initialization. @@ -1179,6 +1197,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { maxSteps int streaming bool hasCustomSystemPrompt bool + systemPromptSource string ) if err := func() error { @@ -1285,13 +1304,27 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { // explicitly set system-prompt, use the per-model prompt as the // base instead of the global default. { - basePrompt := viper.GetString("system-prompt") + rawPromptInput := viper.GetString("system-prompt") + + // Resolve a file path to its content so PromptBuilder receives the + // actual prompt text rather than a literal path string. Without this, + // when system-prompt is set to a file path in the config file or via + // --system-prompt, the path itself becomes the effective system prompt + // sent to the model (LoadSystemPrompt only ran later, after viper had + // been overwritten with the augmented base text). + basePrompt, _ := config.LoadSystemPrompt(rawPromptInput) + if basePrompt == "" { + basePrompt = rawPromptInput + } // Track whether the user explicitly configured a custom system // prompt. When they haven't (basePrompt is the built-in default // or empty), per-model system prompts can replace it on switch. userSetSystemPrompt := basePrompt != "" && basePrompt != defaultSystemPrompt hasCustomSystemPrompt = userSetSystemPrompt + if hasCustomSystemPrompt { + systemPromptSource = rawPromptInput + } // Check for per-model system prompt override when no explicit // global system-prompt was configured by the user. @@ -1500,6 +1533,7 @@ func New(ctx context.Context, opts *Options) (*Kit, error) { opts: opts, mcpConfig: mcpConfig, hasCustomSystemPrompt: hasCustomSystemPrompt, + systemPromptSource: systemPromptSource, beforeToolCall: beforeToolCall, afterToolResult: afterToolResult, beforeTurn: beforeTurn, diff --git a/pkg/kit/kit_test.go b/pkg/kit/kit_test.go index 7eead8a2..04ca2cb2 100644 --- a/pkg/kit/kit_test.go +++ b/pkg/kit/kit_test.go @@ -3,6 +3,7 @@ package kit_test import ( "context" "os" + "strings" "testing" "github.com/spf13/viper" @@ -306,3 +307,92 @@ func TestSessionManagement(t *testing.T) { // resetViper wipes viper's global state so a test case doesn't leak // viper.Set() calls into the next one. Used via defer in subtests. func resetViper() { viper.Reset() } + +// TestNewSystemPromptFilePath is a regression test for issue #25. +// +// When Options.SystemPrompt (or the --system-prompt flag / config entry) is a +// file path, Kit must resolve the path to its file contents *before* the +// PromptBuilder composes the runtime context. Previously the path string +// itself was used verbatim as the base prompt, so the LLM received the path — +// not the prompt — as its system message. +func TestNewSystemPromptFilePath(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + defer resetViper() + + const promptContent = "You are a strict regression-test persona. Marker: KIT-25-OK" + + tmpFile, err := os.CreateTemp(t.TempDir(), "kit-system-prompt-*.md") + if err != nil { + t.Fatalf("failed to create temp prompt file: %v", err) + } + if _, err := tmpFile.WriteString(promptContent); err != nil { + t.Fatalf("failed to write temp prompt file: %v", err) + } + if err := tmpFile.Close(); err != nil { + t.Fatalf("failed to close temp prompt file: %v", err) + } + + ctx := context.Background() + host, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + SystemPrompt: tmpFile.Name(), + Quiet: true, + NoSession: true, + }) + if err != nil { + t.Fatalf("Failed to create Kit with system-prompt file: %v", err) + } + defer func() { _ = host.Close() }() + + if !host.HasCustomSystemPrompt() { + t.Error("HasCustomSystemPrompt() = false; want true when --system-prompt is set") + } + if got, want := host.GetSystemPromptSource(), tmpFile.Name(); got != want { + t.Errorf("GetSystemPromptSource() = %q; want %q", got, want) + } + + // The composed system prompt is written back to viper after PromptBuilder + // runs. It must contain the file's contents, not the file path. + composed := viper.GetString("system-prompt") + if !strings.Contains(composed, promptContent) { + t.Errorf("composed system-prompt does not contain file contents\n composed = %q\n want substring = %q", composed, promptContent) + } + if strings.TrimSpace(composed) == tmpFile.Name() { + t.Errorf("composed system-prompt is the file path verbatim (%q); LoadSystemPrompt was not applied before PromptBuilder", composed) + } +} + +// TestNewSystemPromptInline confirms that inline system-prompt strings still +// flow through unchanged after the file-path resolution change. +func TestNewSystemPromptInline(t *testing.T) { + if os.Getenv("ANTHROPIC_API_KEY") == "" { + t.Skip("Skipping test: ANTHROPIC_API_KEY not set") + } + defer resetViper() + + const inline = "You are a concise inline-prompt persona." + + ctx := context.Background() + host, err := kit.New(ctx, &kit.Options{ + Model: "anthropic/claude-sonnet-4-5-20250929", + SystemPrompt: inline, + Quiet: true, + NoSession: true, + }) + if err != nil { + t.Fatalf("Failed to create Kit with inline system-prompt: %v", err) + } + defer func() { _ = host.Close() }() + + if !host.HasCustomSystemPrompt() { + t.Error("HasCustomSystemPrompt() = false; want true for inline prompt") + } + if got := host.GetSystemPromptSource(); got != inline { + t.Errorf("GetSystemPromptSource() = %q; want %q", got, inline) + } + if composed := viper.GetString("system-prompt"); !strings.Contains(composed, inline) { + t.Errorf("composed system-prompt missing inline content; got %q", composed) + } +}