diff --git a/internal/config/config.go b/internal/config/config.go index 22fd9d2..33dba2b 100644 --- a/internal/config/config.go +++ b/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. - Timeout string `yaml:"timeout"` + // Defaults to 60s if zero. + 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 _, err := time.ParseDuration(cfg.Timeout); err != nil { - return fmt.Errorf("invalid timeout: %w", err) - } + if cfg.Timeout < 0 { + return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout) } 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. - Timeout string `yaml:"timeout"` + // Defaults to 5 minutes if zero. + 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 _, err := time.ParseDuration(cfg.Timeout); err != nil { - return fmt.Errorf("invalid timeout: %w", err) - } + if cfg.Timeout < 0 { + return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout) } 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", ""} + // + // The missing message is therefore passed to echo as the text + // ""; 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 string `yaml:"timeout"` + // Timeout limits execution time (e.g., "30s", "5m"). Zero means no limit. + 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 _, err := time.ParseDuration(cfg.Timeout); err != nil { - return fmt.Errorf("invalid timeout: %v", err) - } + if cfg.Timeout < 0 { + return fmt.Errorf("invalid timeout %v: must not be negative", cfg.Timeout) } 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 is the parsed duration, set during Load. - shutdownTimeout time.Duration `yaml:"-"` + ShutdownTimeout time.Duration `yaml:"shutdown_timeout"` // 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 == "" { - cfg.ShutdownTimeout = "30s" + if cfg.ShutdownTimeout == 0 { + 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 { - return fmt.Errorf("invalid shutdown_timeout %q: %w", cfg.ShutdownTimeout, err) + if cfg.ShutdownTimeout < 0 { + 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 } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 502b08c..0f34dbb 100644 --- a/internal/config/config_test.go +++ b/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", - cfg.ShutdownTimeout, "60s", - ) - } - if cfg.GetShutdownTimeout() != 60*time.Second { - t.Errorf( - "GetShutdownTimeout() = %v, want 60s", - cfg.GetShutdownTimeout(), + "ShutdownTimeout = %v, want %v", + cfg.ShutdownTimeout, 60*time.Second, ) } }, @@ -104,16 +98,10 @@ tts: cfg.Address, ":8080", ) } - if cfg.ShutdownTimeout != "30s" { + if cfg.ShutdownTimeout != 30*time.Second { t.Errorf( - "ShutdownTimeout = %q, want %q", - cfg.ShutdownTimeout, "30s", - ) - } - if cfg.GetShutdownTimeout() != 30*time.Second { - t.Errorf( - "GetShutdownTimeout() = %v, want 30s", - cfg.GetShutdownTimeout(), + "ShutdownTimeout = %v, want %v", + cfg.ShutdownTimeout, 30*time.Second, ) } }, @@ -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", - cfg.ShutdownTimeout, "30s", + "ShutdownTimeout = %v, want %v", + 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", - cfg.ShutdownTimeout, "120s", + "ShutdownTimeout = %v, want %v", + 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 err != nil { - t.Fatalf("ParseDuration(%q) error = %v", j.Timeout, err) - } - if timeout.Minutes() != 10 { - t.Errorf("Timeout = %v, want 10m", timeout) + if j.Timeout != 10*time.Minute { + t.Errorf("Timeout = %v, want 10m", j.Timeout) } if j.MaxIterations != 20 { t.Errorf("MaxIterations = %d, want 20", j.MaxIterations)