Refactor config pkg

internal/config/config.go

@@ -19,8 +19,8 @@ type TTS struct {
 	// Voice is the default voice ID to use (e.g., "af_heart").
 	Voice string `yaml:"voice"`
 	// Timeout is the maximum duration for a synthesis request.
-	// Defaults to 60s if empty.
+	// Defaults to 60s if zero.
-	Timeout string `yaml:"timeout"`
+	Timeout time.Duration `yaml:"timeout"`
 }
 
 func (cfg TTS) Validate() error {
@@ -33,10 +33,8 @@ func (cfg TTS) Validate() error {
 	if cfg.Voice == "" {
 		return fmt.Errorf("missing voice")
 	}
-	if cfg.Timeout != "" {
+	if cfg.Timeout < 0 {
-		if _, err := time.ParseDuration(cfg.Timeout); err != nil {
+		return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout)
-			return fmt.Errorf("invalid timeout: %w", err)
-		}
 	}
 	return nil
 }
@@ -80,8 +78,8 @@ type ProviderConfig struct {
 	// Key is the API key for authentication.
 	Key string `yaml:"key"`
 	// Timeout is the maximum duration for a query (e.g., "5m").
-	// Defaults to 5 minutes if empty.
+	// Defaults to 5 minutes if zero.
-	Timeout string `yaml:"timeout"`
+	Timeout time.Duration `yaml:"timeout"`
 	// Models is an optional whitelist of model IDs for this provider.
 	Models []string `yaml:"models,omitempty"`
 }
@@ -96,10 +94,8 @@ func (cfg ProviderConfig) Validate() error {
 	if _, err := url.Parse(cfg.URL); err != nil {
 		return fmt.Errorf("invalid URL %q: %w", cfg.URL, err)
 	}
-	if cfg.Timeout != "" {
+	if cfg.Timeout < 0 {
-		if _, err := time.ParseDuration(cfg.Timeout); err != nil {
+		return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout)
-			return fmt.Errorf("invalid timeout: %w", err)
-		}
 	}
 	return nil
 }
@@ -127,7 +123,7 @@ type JobConfig struct {
 	// Task is the user-level instruction to give the LLM.
 	Task string `yaml:"task"`
 	// Timeout limits the job execution time (e.g., "10m").
-	Timeout string `yaml:"timeout,omitempty"`
+	Timeout time.Duration `yaml:"timeout,omitempty"`
 	// UserId identifies the user that owns conversations created by this job.
 	// It is assigned by the service until jobs are stored in the database.
 	UserId string `yaml:"user_id,omitempty"`
@@ -146,6 +142,9 @@ func (cfg JobConfig) Validate() error {
 	if cfg.MaxIterations < 0 {
 		return fmt.Errorf("max_iterations must be non-negative")
 	}
+	if cfg.Timeout < 0 {
+		return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout)
+	}
 
 	return nil
 }
@@ -158,12 +157,85 @@ type ToolConfig struct {
 	Description string `yaml:"description"`
 	// Command is the executable path or name.
 	Command string `yaml:"command"`
-	// Arguments are Go templates expanded with LLM-provided parameters.
+	// Arguments defines the arguments passed to Command when the LLM calls
+	// this tool. Each entry is a Go text/template template evaluated with the
+	// JSON object supplied by the LLM.
+	//
+	// For example, this configuration:
+	//
+	//	arguments:
+	//	  - "--message"
+	//	  - "{{.message}}"
+	//	  - "{{if .verbose}}--verbose{{end}}"
+	//
+	// combines with these tool-call arguments:
+	//
+	//	{"message":"hello world","verbose":true}
+	//
+	// to run Command with:
+	//
+	//	[]string{"--message", "hello world", "--verbose"}
+	//
+	// Each configured entry becomes one element of that []string. Spaces in
+	// "hello world" do not divide it into separate arguments. If verbose were
+	// false, its template would produce an empty string, and ParseArguments
+	// would leave that entry out, resulting in:
+	//
+	//	[]string{"--message", "hello world"}
+	//
+	// Template output is passed directly to the program. It is not interpreted
+	// by a shell. Each entry in Arguments adds either zero or one element to
+	// the argument vector, regardless of the type of the value it renders.
+	//
+	// For example, suppose Command is "cat", Arguments contains:
+	//
+	//	- "{{.files}}"
+	//
+	// and the tool-call JSON is:
+	//
+	//	{"files":["one.txt","two.txt"]}
+	//
+	// The program is invoked with:
+	//
+	//	[]string{"cat", "[one.txt two.txt]"}
+	//
+	// This asks cat to read one file named "[one.txt two.txt]". It does not
+	// produce the following argument vector, which would ask cat to read two
+	// files:
+	//
+	//	[]string{"cat", "one.txt", "two.txt"}
+	//
+	// Note that templates may accept missing values without reporting an
+	// error. For example, suppose Command is "echo", Arguments contains:
+	//
+	//	- "--message"
+	//	- "{{.message}}"
+	//
+	// and the tool-call JSON is {}. The program is invoked with:
+	//
+	//	[]string{"echo", "--message", "<no value>"}
+	//
+	// The missing message is therefore passed to echo as the text
+	// "<no value>"; it does not cause the tool call to fail.
+	//
+	// Each template is compiled when the tool is constructed, so a template
+	// syntax error is reported at configuration load, not on the tool's
+	// first call.
+	//
+	// See https://pkg.go.dev/text/template.
 	Arguments []string `yaml:"arguments"`
-	// Parameters is a JSON Schema describing expected input from the LLM.
+	// Parameters is the JSON Schema included in the tool definition sent to
+	// the LLM. It describes the values the LLM should provide when calling the
+	// tool, including their names, types, and which values are required.
+	//
+	// The schema is not used to validate a tool call before Arguments is
+	// evaluated. For example, if Parameters requires "message" but the LLM
+	// supplies {}, evaluation continues without first reporting that
+	// "message" is missing. Values that do not have the types described by the
+	// schema may likewise reach the argument templates.
 	Parameters map[string]any `yaml:"parameters"`
-	// Timeout limits execution time (e.g., "30s", "5m").
+	// Timeout limits execution time (e.g., "30s", "5m"). Zero means no limit.
-	Timeout string `yaml:"timeout"`
+	Timeout time.Duration `yaml:"timeout"`
 }
 
 func (cfg ToolConfig) Validate() error {
@@ -176,10 +248,8 @@ func (cfg ToolConfig) Validate() error {
 	if cfg.Command == "" {
 		return fmt.Errorf("missing command")
 	}
-	if cfg.Timeout != "" {
+	if cfg.Timeout < 0 {
-		if _, err := time.ParseDuration(cfg.Timeout); err != nil {
+		return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout)
-			return fmt.Errorf("invalid timeout: %v", err)
-		}
 	}
 	return nil
 }
@@ -191,9 +261,7 @@ type Config struct {
 	// Database configures the SQLite database path.
 	Database Database `yaml:"database"`
 	// ShutdownTimeout is the maximum time to wait for graceful shutdown.
-	ShutdownTimeout string `yaml:"shutdown_timeout"`
+	ShutdownTimeout time.Duration `yaml:"shutdown_timeout"`
-	// shutdownTimeout is the parsed duration, set during Load.
-	shutdownTimeout time.Duration `yaml:"-"`
 	// SystemMessage is the default system prompt prepended to all conversations.
 	SystemMessage string `yaml:"system_message"`
 	// DefaultModel specifies the default provider and model.
@@ -212,19 +280,14 @@ type Config struct {
 
 // ApplyDefaults sets default values for optional configuration fields.
 func (cfg *Config) ApplyDefaults() {
-	if cfg.ShutdownTimeout == "" {
+	if cfg.ShutdownTimeout == 0 {
-		cfg.ShutdownTimeout = "30s"
+		cfg.ShutdownTimeout = 30 * time.Second
 	}
 	if cfg.Address == "" {
 		cfg.Address = ":8080"
 	}
 }
 
-// GetShutdownTimeout returns the parsed shutdown timeout duration.
-func (cfg *Config) GetShutdownTimeout() time.Duration {
-	return cfg.shutdownTimeout
-}
-
 // Validate checks that all required fields are present and valid.
 func (cfg *Config) Validate() error {
 	if cfg.Address == "" {
@@ -253,8 +316,8 @@ func (cfg *Config) Validate() error {
 	if err := cfg.TTS.Validate(); err != nil {
 		return fmt.Errorf("invalid tts config: %w", err)
 	}
-	if _, err := time.ParseDuration(cfg.ShutdownTimeout); err != nil {
+	if cfg.ShutdownTimeout < 0 {
-		return fmt.Errorf("invalid shutdown_timeout %q: %w", cfg.ShutdownTimeout, err)
+		return fmt.Errorf("invalid shutdown_timeout %v: must not be negative", cfg.ShutdownTimeout)
 	}
 	for i, jc := range cfg.Jobs {
 		if err := jc.Validate(); err != nil {
@@ -294,11 +357,5 @@ func Load(path string) (*Config, error) {
 		return nil, fmt.Errorf("invalid config: %v", err)
 	}
 
-	d, err := time.ParseDuration(cfg.ShutdownTimeout)
-	if err != nil {
-		return nil, fmt.Errorf("parse shutdown timeout: %v", err)
-	}
-	cfg.shutdownTimeout = d
-
 	return &cfg, nil
 }

internal/config/config_test.go

@@ -62,16 +62,10 @@ func TestLoad(t *testing.T) {
 						cfg.Address, ":9090",
 					)
 				}
-				if cfg.ShutdownTimeout != "60s" {
+				if cfg.ShutdownTimeout != 60*time.Second {
 					t.Errorf(
-						"ShutdownTimeout = %q, want %q",
+						"ShutdownTimeout = %v, want %v",
-						cfg.ShutdownTimeout, "60s",
+						cfg.ShutdownTimeout, 60*time.Second,
-					)
-				}
-				if cfg.GetShutdownTimeout() != 60*time.Second {
-					t.Errorf(
-						"GetShutdownTimeout() = %v, want 60s",
-						cfg.GetShutdownTimeout(),
 					)
 				}
 			},
@@ -104,16 +98,10 @@ tts:
 						cfg.Address, ":8080",
 					)
 				}
-				if cfg.ShutdownTimeout != "30s" {
+				if cfg.ShutdownTimeout != 30*time.Second {
 					t.Errorf(
-						"ShutdownTimeout = %q, want %q",
+						"ShutdownTimeout = %v, want %v",
-						cfg.ShutdownTimeout, "30s",
+						cfg.ShutdownTimeout, 30*time.Second,
-					)
-				}
-				if cfg.GetShutdownTimeout() != 30*time.Second {
-					t.Errorf(
-						"GetShutdownTimeout() = %v, want 30s",
-						cfg.GetShutdownTimeout(),
 					)
 				}
 			},
@@ -419,6 +407,32 @@ tools:
   - name: ""
     description: missing name
     command: echo
+`,
+			shouldErr: true,
+		},
+		{
+			name: "invalid tool timeout",
+			config: `
+default_model:
+  provider: testprovider
+  model: test-model
+
+providers:
+  - name: testprovider
+    url: http://localhost:8080
+
+stt:
+  url: http://localhost:8178
+
+tts:
+  url: http://localhost:8880
+  voice: af_heart
+
+tools:
+  - name: echo
+    description: echoes input
+    command: echo
+    timeout: "-1s"
 `,
 			shouldErr: true,
 		},
@@ -464,10 +478,10 @@ func TestApplyDefaults(t *testing.T) {
 	if cfg.Address != ":8080" {
 		t.Errorf("Address = %q, want %q", cfg.Address, ":8080")
 	}
-	if cfg.ShutdownTimeout != "30s" {
+	if cfg.ShutdownTimeout != 30*time.Second {
 		t.Errorf(
-			"ShutdownTimeout = %q, want %q",
+			"ShutdownTimeout = %v, want %v",
-			cfg.ShutdownTimeout, "30s",
+			cfg.ShutdownTimeout, 30*time.Second,
 		)
 	}
 }
@@ -475,17 +489,17 @@ func TestApplyDefaults(t *testing.T) {
 func TestApplyDefaults_NoOverwrite(t *testing.T) {
 	cfg := &Config{
 		Address:         ":9999",
-		ShutdownTimeout: "120s",
+		ShutdownTimeout: 120 * time.Second,
 	}
 	cfg.ApplyDefaults()
 
 	if cfg.Address != ":9999" {
 		t.Errorf("Address = %q, want %q", cfg.Address, ":9999")
 	}
-	if cfg.ShutdownTimeout != "120s" {
+	if cfg.ShutdownTimeout != 120*time.Second {
 		t.Errorf(
-			"ShutdownTimeout = %q, want %q",
+			"ShutdownTimeout = %v, want %v",
-			cfg.ShutdownTimeout, "120s",
+			cfg.ShutdownTimeout, 120*time.Second,
 		)
 	}
 }
@@ -540,12 +554,8 @@ jobs:
 				if j.Schedule != "0 9 * * *" {
 					t.Errorf("Schedule = %q, want %q", j.Schedule, "0 9 * * *")
 				}
-				timeout, err := time.ParseDuration(j.Timeout)
+				if j.Timeout != 10*time.Minute {
-				if err != nil {
+					t.Errorf("Timeout = %v, want 10m", j.Timeout)
-					t.Fatalf("ParseDuration(%q) error = %v", j.Timeout, err)
-				}
-				if timeout.Minutes() != 10 {
-					t.Errorf("Timeout = %v, want 10m", timeout)
 				}
 				if j.MaxIterations != 20 {
 					t.Errorf("MaxIterations = %d, want 20", j.MaxIterations)