diff --git a/docs/cli.md b/docs/cli.md index d1519c7..8551145 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -26,9 +26,13 @@ go run ./cmd/scriptorium render \ - `scriptorium inspect prompt`: inspect one prompt definition without model execution. - `scriptorium inspect profile`: inspect one effective profile without model execution. -All commands accept `--config ` and reject positional arguments. An -effective `prompt_dir` is required for every command. Supply it through the -configuration contract or the command's `--prompt-dir` flag. +All commands accept `--config ` and reject positional arguments. `run`, +`render`, `serve`, and `inspect prompt` require an effective `prompt_dir`; +`inspect profile` does not. Supply the directory through the configuration +contract or a supported `--prompt-dir` flag. Explicit application flags, +including empty values, override file settings. Configuration load diagnostics +go to stderr and exit with status 1; successful load warnings are printed once +before command work. ## `scriptorium run` @@ -144,7 +148,7 @@ scriptorium render [flags] ``` `--prompt ` is required. The following optional flags are supported: -`--config`, `--prompt-dir`, `--profile-dir`, `--prompt-version`, `--profile`, +`--config`, `--prompt-dir`, `--profile-dir`, `--schema-dir`, `--prompt-version`, `--profile`, `--input`, `--var`, `--out`, `--llm-base-url`, `--model`, `--api-key-env`, `--temperature`, `--max-tokens`, `--top-p`, `--reasoning-effort`, `--session-id`, `--timeout`, `--transient-retries`, and `--format text|json`. Their meanings match the corresponding @@ -156,8 +160,7 @@ The same session and reasoning inheritance, replacement, and clearing behavior also applies. `render` validates the retry override but does not generate model output or include a retry field in prepared output. -`render` does not accept `--schema-dir`; configure `schema_dir` through the -configuration file. It resolves profiles and schemas as part of preparation but +`render` resolves profiles and schemas as part of preparation but does not call an LLM. ## `scriptorium serve` diff --git a/docs/config.md b/docs/config.md index c9844eb..4f86b7f 100644 --- a/docs/config.md +++ b/docs/config.md @@ -25,12 +25,24 @@ The maintained [minimal configuration](../examples/config.yml) and ## Application Configuration File -Configuration is strict YAML: unknown fields are rejected. Empty string values -do not override a prior value. Raw API-key fields are not accepted. +Configuration is decoded by ConfigKit v0.4.0 as strict YAML. Unknown fields, +empty files, multiple YAML documents, and YAML aliases/anchors are rejected. +An explicit empty string is a value: it clears a file-provided optional path +or, for required `prompt_dir`, fails the command's prerequisite. A scalar +`null` is not an empty string and is rejected for scalar fields; nullable +fields such as `transient_retries` retain their documented meaning. Raw +API-key fields are not accepted. Imports and selected overlays are not yet +enabled for application configuration. + +ConfigKit applies defaults, the selected file, then only explicitly supplied +application CLI flags before validating the result. Paths in the application +document remain relative to the process working directory, not to the file's +directory. Load diagnostics are printed safely on stderr; invalid application +configuration exits with status 1. | Field | Default | Meaning | | --- | --- | --- | -| `prompt_dir` | unset | Promptkit prompt-definition source directory. `run`, `render`, and `serve` require an effective value. | +| `prompt_dir` | unset | Promptkit prompt-definition source directory. `run`, `render`, `serve`, and `inspect prompt` require an effective value. | | `profile_dir` | unset | Optional custom Promptkit profile source directory overlaid on Promptkit built-ins. | | `schema_dir` | `.` | Promptkit schema source directory for relative schema paths. | | `server.addr` | `:8080` | Address used by `serve`. | diff --git a/docs/development.md b/docs/development.md index 006d79e..4791c92 100644 --- a/docs/development.md +++ b/docs/development.md @@ -3,7 +3,8 @@ This is the contributor entry point for Scriptorium. Scriptorium is an application that consumes the public [Promptkit v0.11.0 package](https://gitea.maximumdirect.net/eric/promptkit/src/tag/v0.11.0/docs/consumers/pkg-promptkit.md); -framework implementation work belongs in Promptkit. +framework implementation work belongs in Promptkit. Application configuration +uses ConfigKit v0.4.0; keep Scriptorium's schema and source selection local. ## Initial Orientation @@ -26,7 +27,7 @@ tests. | Repository orientation or component responsibility | [Internal component overview](internal/overview.md) and [architecture policy](policy/architecture.md) | | CLI commands, flags, output, or exit behavior | [CLI contract](cli.md) and [adapter internals](internal/adapters.md) | | HTTP routes, DTOs, limits, status mapping, or artifact policy | [HTTP API contract](api.md), [adapter internals](internal/adapters.md), and [source internals](internal/sources.md) | -| Application configuration or precedence | [Configuration contract](config.md), [adapter internals](internal/adapters.md), and [source internals](internal/sources.md) | +| Application configuration or precedence | [Configuration contract](config.md), [adapter internals](internal/adapters.md), [source internals](internal/sources.md), and `internal/config` definition/load tests | | Prepared-run presentation | [CLI contract](cli.md), [adapter internals](internal/adapters.md), and `internal/format` | | Prompt, profile, schema, generation, or validation semantics | [Promptkit framework formats](https://gitea.maximumdirect.net/eric/promptkit/src/tag/v0.11.0/docs/formats.md) and the [Promptkit consumer guide](https://gitea.maximumdirect.net/eric/promptkit/src/tag/v0.11.0/docs/consumers/pkg-promptkit.md) | | OpenAI-compatible outbound behavior or timeout layering | [Promptkit integration contract](https://gitea.maximumdirect.net/eric/promptkit/src/tag/v0.11.0/docs/integrations/openai-compatible-chat.md) | diff --git a/docs/internal/adapters.md b/docs/internal/adapters.md index cd69fc1..14e787d 100644 --- a/docs/internal/adapters.md +++ b/docs/internal/adapters.md @@ -18,6 +18,10 @@ described by its tagged - `internal/adapter/cli` resolves settings through `internal/config`, constructs `promptkit.Engine`, maps CLI values to `promptkit.RunRequest`, and owns output files, summaries, and exit codes. +- `internal/config` delegates application YAML decoding, default resolution, + overrides, and validation ordering to ConfigKit. The CLI supplies only + explicitly visited application flags as overrides and renders safe structured + load diagnostics on stderr. - `internal/adapter/http` strictly decodes request DTOs, maps them to Promptkit public values, calls its adapter-owned `Runner` interface, and maps results and errors to HTTP DTOs. @@ -53,7 +57,7 @@ not require prompt, schema, or artifact sources. with `promptkit.WithArtifactReader`, passes the engine through the HTTP adapter's consumer-owned `Runner` interface, and starts the server. -All three CLI paths assemble the engine from the same resolved prompt, profile, +The command paths assemble the engine from the same resolved prompt, profile, and schema directories plus configured custom backends. Each backend is mapped to Promptkit's public `Backend` value and registered during engine construction, so one constructed server engine retains one immutable backend registry and its diff --git a/docs/internal/sources.md b/docs/internal/sources.md index aca5b25..5d2fba1 100644 --- a/docs/internal/sources.md +++ b/docs/internal/sources.md @@ -24,9 +24,9 @@ The [configuration reference](../config.md) owns Scriptorium's source-location fields and precedence. Maintained files under `examples/` are application inputs that use Promptkit's tagged formats. -## Native Application Definition In Preparation +## Application Configuration Boundary -`internal/config` also contains a ConfigKit-native `Document` definition. Its +`internal/config` contains a ConfigKit-native `Document` definition. Its fresh defaults come from Scriptorium's existing default constants; its resolver records only actual trimming, path cleaning, and render-format normalization. The validator reports application retry, size-limit, and format errors without @@ -34,7 +34,7 @@ mutating the document or exposing rejected values. Application path values keep working-directory-relative meaning. ConfigKit owns strict YAML decoding and presence semantics, including dynamic integer values in backend parameters. -The new `Load` boundary selects an explicit file, the first existing implicit +The `Load` boundary selects an explicit file, the first existing implicit configuration file, or ConfigKit's defaults-only source. Only absence advances implicit discovery. One ConfigKit load applies explicit application overrides before validation and returns a result containing the resolved document and @@ -43,9 +43,10 @@ existing runtime settings with backend IDs sorted; it does not read files or apply another merge. Imports and selected overlays remain disabled until their own user-facing integration is ready. -The executable commands still use the existing loader. The new definition is -internal preparation and is not yet an alternative user-selectable configuration -mode or a change to the current [configuration contract](../config.md). +Every configuration-loading command uses this boundary. The CLI maps only +explicitly visited application flags to ConfigKit overrides and prints +structured load diagnostics before engine construction. The old application +YAML decoder and merge path have been removed. ## Restricted HTTP Artifact Reader @@ -78,7 +79,8 @@ codes. Inspect: -- `internal/config/config_test.go` +- `internal/config/document_test.go` +- `internal/config/load_test.go` - `internal/adapter/cli/run_test.go` - `internal/adapter/http/artifact_reader_test.go` - `internal/adapter/http/handler_test.go` diff --git a/docs/policy/architecture.md b/docs/policy/architecture.md index e45b79c..dbc9bd2 100644 --- a/docs/policy/architecture.md +++ b/docs/policy/architecture.md @@ -32,6 +32,12 @@ validation. Scriptorium owns application configuration, executable adapters, prepared-run and definition-inspection presentation, process behavior, and HTTP deployment policy. +Scriptorium also consumes ConfigKit v0.4.0 for application configuration +decoding, resolution, and validation flow. Scriptorium owns its application +schema, defaults, normalization, validation rules, source selection, and CLI +flag mapping; ConfigKit does not own Promptkit framework sources or HTTP +artifact containment. + The concrete package inventory is maintained in the [internal overview](../internal/overview.md). @@ -54,7 +60,7 @@ cmd/scriptorium CLI and HTTP adapters, configuration, defaults, and formatting | v -gitea.maximumdirect.net/eric/promptkit +Promptkit (execution) and ConfigKit (application configuration) ``` - Retained application packages may import Promptkit's root package. @@ -80,8 +86,8 @@ vendored Promptkit implementation, or unpublished dependency assumptions. process streams, output files, summaries, and exit codes. - `internal/adapter/http` owns routes, strict JSON DTOs, size limits, response mapping, status mapping, and the restricted artifact reader. -- `internal/config` owns discovery and strict decoding of Scriptorium - application configuration. +- `internal/config` owns discovery and Scriptorium's application definition; + ConfigKit owns strict YAML decoding and the load/override pipeline. - `internal/defaults` owns Scriptorium application and HTTP defaults only. - `internal/format` owns deterministic prepared-run and definition-inspection text and JSON presentation. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 25c257b..c5bed84 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -197,7 +197,10 @@ with no new user-visible mode. ## Stage 3: Cut Existing Commands Over And Remove The Legacy Loader -**Status: Not started.** +**Status: Complete.** All five command modes use the shared ConfigKit loader; +the legacy decoder and merge path are removed. CLI and configuration contracts +now describe native YAML and override behavior. Full test, race, vet, build, +and diff checks passed. Primary files: `internal/adapter/cli/run.go`, `inspect.go`, `internal/config/config.go`, and associated tests. diff --git a/go.mod b/go.mod index d0889eb..2a278e6 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,6 @@ go 1.25.5 require ( gitea.maximumdirect.net/eric/ConfigKit v0.4.0 gitea.maximumdirect.net/eric/promptkit v0.11.0 - gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -14,4 +13,5 @@ require ( github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.6 // indirect golang.org/x/text v0.14.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/internal/adapter/cli/application_config.go b/internal/adapter/cli/application_config.go new file mode 100644 index 0000000..04fcf32 --- /dev/null +++ b/internal/adapter/cli/application_config.go @@ -0,0 +1,71 @@ +package cli + +import ( + "errors" + "flag" + "fmt" + "io" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" + appconfig "gitea.maximumdirect.net/eric/scriptorium/internal/config" +) + +// applicationOverrides keeps explicit application flags separate from invocation state. +func applicationOverrides(fs *flag.FlagSet) []configkit.Override { + var overrides []configkit.Override + fs.Visit(func(selected *flag.Flag) { + var path configkit.Path + switch selected.Name { + case "prompt-dir": + path = configkit.Keys("prompt_dir") + case "profile-dir": + path = configkit.Keys("profile_dir") + case "schema-dir": + path = configkit.Keys("schema_dir") + case "addr": + path = configkit.Keys("server", "addr") + case "artifact-root": + path = configkit.Keys("server", "artifact_root") + case "max-request-bytes": + path = configkit.Keys("server", "max_request_bytes") + case "max-artifact-bytes": + path = configkit.Keys("server", "max_artifact_bytes") + case "max-response-bytes": + path = configkit.Keys("server", "max_response_bytes") + default: + return + } + overrides = append(overrides, configkit.Override{ + Path: path, Value: selected.Value.(flag.Getter).Get(), Flag: "--" + selected.Name, + }) + }) + return overrides +} + +func loadApplicationSettings(configPath string, explicit bool, overrides []configkit.Override) (appconfig.AppSettings, []configkit.Diagnostic, error) { + result, err := appconfig.Load(appconfig.LoadOptions{ + Path: configPath, Explicit: explicit, Overrides: overrides, + }) + if err != nil { + return appconfig.AppSettings{}, nil, err + } + return appconfig.Settings(result.Config), result.Diagnostics(), nil +} + +func writeApplicationError(output io.Writer, label string, err error) { + var loadError *configkit.LoadError + if errors.As(err, &loadError) { + if writeErr := configkit.WriteDiagnostics(output, loadError.Diagnostics()); writeErr != nil { + _, _ = fmt.Fprintf(output, "%s diagnostic write error: %v\n", label, writeErr) + } + return + } + _, _ = fmt.Fprintf(output, "%s error: %v\n", label, err) +} + +func writeApplicationWarnings(output io.Writer, diagnostics []configkit.Diagnostic) error { + if len(diagnostics) == 0 { + return nil + } + return configkit.WriteDiagnostics(output, diagnostics) +} diff --git a/internal/adapter/cli/inspect.go b/internal/adapter/cli/inspect.go index ff411ff..e9af707 100644 --- a/internal/adapter/cli/inspect.go +++ b/internal/adapter/cli/inspect.go @@ -10,8 +10,8 @@ import ( "path/filepath" "strings" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/promptkit" - appconfig "gitea.maximumdirect.net/eric/scriptorium/internal/config" appformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" ) @@ -19,12 +19,16 @@ type promptInspectionConfig struct { configPath, promptDir, promptID, promptVersion, outputPath string configExplicit bool outputFormat appformat.OutputFormat + overrides []configkit.Override + diagnostics []configkit.Diagnostic } type profileInspectionConfig struct { configPath, profileDir, profileID, outputPath string configExplicit bool outputFormat appformat.OutputFormat + overrides []configkit.Override + diagnostics []configkit.Diagnostic } // emptyPromptDefinitionFS satisfies Promptkit's engine-level prompt-source @@ -50,12 +54,15 @@ func inspectCommand(args []string, stdout, stderr io.Writer) int { } cfg, err := parsePromptInspectionArgs(args[1:]) if err != nil { - fmt.Fprintf(stderr, "inspect parse error: %v\n", err) + writeApplicationError(stderr, "inspect parse", err) return ExitRuntimeError } settings, err := resolveAppSettingsForPromptInspection(cfg) if err != nil { - fmt.Fprintf(stderr, "inspect error: %v\n", err) + writeApplicationError(stderr, "inspect", err) + return ExitRuntimeError + } + if err := writeApplicationWarnings(stderr, cfg.diagnostics); err != nil { return ExitRuntimeError } engine, err := newEngine(settings) @@ -83,12 +90,15 @@ func inspectCommand(args []string, stdout, stderr io.Writer) int { func inspectProfileCommand(args []string, stdout, stderr io.Writer) int { cfg, err := parseProfileInspectionArgs(args) if err != nil { - fmt.Fprintf(stderr, "inspect parse error: %v\n", err) + writeApplicationError(stderr, "inspect parse", err) return ExitRuntimeError } settings, err := resolveAppSettingsForProfileInspection(cfg) if err != nil { - fmt.Fprintf(stderr, "inspect error: %v\n", err) + writeApplicationError(stderr, "inspect", err) + return ExitRuntimeError + } + if err := writeApplicationWarnings(stderr, cfg.diagnostics); err != nil { return ExitRuntimeError } engine, err := newEngine(settings, promptkit.WithPromptFS(emptyPromptDefinitionFS{}, ".")) @@ -127,6 +137,7 @@ func parseProfileInspectionArgs(args []string) (*profileInspectionConfig, error) return nil, err } cfg.configExplicit = flagWasSet(fs, "config") + cfg.overrides = applicationOverrides(fs) if fs.NArg() > 0 { return nil, fmt.Errorf("unexpected positional args: %v", fs.Args()) } @@ -145,12 +156,11 @@ func parseProfileInspectionArgs(args []string) (*profileInspectionConfig, error) } func resolveAppSettingsForProfileInspection(cfg *profileInspectionConfig) (engineSettings, error) { - settings, err := resolveAppSettingsWithConfigPresence(cfg.configPath, cfg.configExplicit, appconfig.CLIOverrides{ - ProfileDir: cfg.profileDir, - }) + settings, diagnostics, err := loadApplicationSettings(cfg.configPath, cfg.configExplicit, cfg.overrides) if err != nil { return engineSettings{}, err } + cfg.diagnostics = diagnostics return engineSettings{profileDir: settings.ProfileDir, backends: settings.Backends, transientRetries: settings.TransientRetries}, nil } @@ -169,6 +179,7 @@ func parsePromptInspectionArgs(args []string) (*promptInspectionConfig, error) { return nil, err } cfg.configExplicit = flagWasSet(fs, "config") + cfg.overrides = applicationOverrides(fs) if fs.NArg() > 0 { return nil, fmt.Errorf("unexpected positional args: %v", fs.Args()) } @@ -187,12 +198,11 @@ func parsePromptInspectionArgs(args []string) (*promptInspectionConfig, error) { } func resolveAppSettingsForPromptInspection(cfg *promptInspectionConfig) (engineSettings, error) { - settings, err := resolveAppSettingsWithConfigPresence(cfg.configPath, cfg.configExplicit, appconfig.CLIOverrides{ - PromptDir: cfg.promptDir, - }) + settings, diagnostics, err := loadApplicationSettings(cfg.configPath, cfg.configExplicit, cfg.overrides) if err != nil { return engineSettings{}, err } + cfg.diagnostics = diagnostics if strings.TrimSpace(settings.PromptDir) == "" { return engineSettings{}, errors.New(errPromptDirRequired) } diff --git a/internal/adapter/cli/run.go b/internal/adapter/cli/run.go index f15ea09..96de21f 100644 --- a/internal/adapter/cli/run.go +++ b/internal/adapter/cli/run.go @@ -13,6 +13,7 @@ import ( "strings" "time" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/promptkit" httpadapter "gitea.maximumdirect.net/eric/scriptorium/internal/adapter/http" appconfig "gitea.maximumdirect.net/eric/scriptorium/internal/config" @@ -57,6 +58,7 @@ type runConfig struct { runTimeout time.Duration transientRetries int configuredTransientRetries *int + diagnostics []configkit.Diagnostic defaultRenderFormat renderformat.PreparedRunOutputFormat @@ -89,6 +91,7 @@ type serveConfig struct { maxResponseBytes int64 backends []appconfig.BackendSettings transientRetries *int + diagnostics []configkit.Diagnostic } type commonCommandSettings struct { @@ -103,6 +106,7 @@ type commonCommandSettings struct { defaultRenderFormat renderformat.PreparedRunOutputFormat backends []appconfig.BackendSettings transientRetries *int + diagnostics []configkit.Diagnostic } type listFlag []string @@ -141,7 +145,10 @@ func Run(args []string, stdout, stderr io.Writer) int { func runCommand(args []string, stdout, stderr io.Writer) int { cfg, err := parseRunArgs(args) if err != nil { - fmt.Fprintf(stderr, "run parse error: %v\n", err) + writeApplicationError(stderr, "run parse", err) + return ExitRuntimeError + } + if err := writeApplicationWarnings(stderr, cfg.diagnostics); err != nil { return ExitRuntimeError } @@ -216,7 +223,10 @@ func writeExecutionReport(path string, value *executionreport.Execution) error { func renderCommand(args []string, stdout, stderr io.Writer) int { cfg, err := parseRenderArgs(args) if err != nil { - fmt.Fprintf(stderr, "render parse error: %v\n", err) + writeApplicationError(stderr, "render parse", err) + return ExitRuntimeError + } + if err := writeApplicationWarnings(stderr, cfg.diagnostics); err != nil { return ExitRuntimeError } @@ -254,7 +264,10 @@ func renderCommand(args []string, stdout, stderr io.Writer) int { func serveCommand(args []string, stderr io.Writer) int { cfg, err := parseServeArgs(args) if err != nil { - fmt.Fprintf(stderr, "serve parse error: %v\n", err) + writeApplicationError(stderr, "serve parse", err) + return ExitRuntimeError + } + if err := writeApplicationWarnings(stderr, cfg.diagnostics); err != nil { return ExitRuntimeError } @@ -323,8 +336,6 @@ func parseRunArgs(args []string) (*runConfig, error) { } } } - cfg.schemaDir = filepath.Clean(cfg.schemaDir) - return cfg, nil } @@ -354,6 +365,7 @@ func parseRenderArgs(args []string) (*renderConfig, error) { registerConfigPathFlag(fs, &cfg.configPath) registerExecutionRequestFlags(fs, &cfg.runConfig) + fs.StringVar(&cfg.schemaDir, "schema-dir", "", "base directory for validation schemas") var rawFormat string fs.StringVar(&rawFormat, "format", "", "render output format (text|json)") @@ -399,16 +411,7 @@ func parseServeArgs(args []string) (*serveConfig, error) { return nil, fmt.Errorf("unexpected positional args: %v", fs.Args()) } - settings, err := resolveCommonSettings(fs, cfg.configPath, appconfig.CLIOverrides{ - PromptDir: cfg.promptDirIfSet(fs), - ProfileDir: cfg.profileDirIfSet(fs), - SchemaDir: cfg.schemaDirIfSet(fs), - ServerAddr: cfg.addrIfSet(fs), - ArtifactRoot: cfg.artifactRootIfSet(fs), - MaxRequestBytes: cfg.maxRequestBytesIfSet(fs), - MaxArtifactBytes: cfg.maxArtifactBytesIfSet(fs), - MaxResponseBytes: cfg.maxResponseBytesIfSet(fs), - }) + settings, err := resolveCommonSettings(fs, cfg.configPath) if err != nil { return nil, err } @@ -423,19 +426,12 @@ func parseServeArgs(args []string) (*serveConfig, error) { cfg.maxResponseBytes = settings.maxResponseBytes cfg.backends = settings.backends cfg.transientRetries = settings.transientRetries + cfg.diagnostics = settings.diagnostics if err := validateRequiredLibraryDirs(cfg.promptDir); err != nil { return nil, err } - cfg.promptDir = filepath.Clean(cfg.promptDir) - if strings.TrimSpace(cfg.profileDir) != "" { - cfg.profileDir = filepath.Clean(cfg.profileDir) - } - cfg.schemaDir = filepath.Clean(cfg.schemaDir) - if strings.TrimSpace(cfg.artifactRoot) != "" { - cfg.artifactRoot = filepath.Clean(cfg.artifactRoot) - } return cfg, nil } @@ -467,11 +463,7 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error { return fmt.Errorf("unexpected positional args: %v", fs.Args()) } - settings, err := resolveCommonSettings(fs, cfg.configPath, appconfig.CLIOverrides{ - PromptDir: cfg.promptDirIfSet(fs), - ProfileDir: cfg.profileDirIfSet(fs), - SchemaDir: cfg.schemaDirIfSet(fs), - }) + settings, err := resolveCommonSettings(fs, cfg.configPath) if err != nil { return err } @@ -482,6 +474,7 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error { cfg.defaultRenderFormat = settings.defaultRenderFormat cfg.backends = settings.backends cfg.configuredTransientRetries = settings.transientRetries + cfg.diagnostics = settings.diagnostics if err := validateRequiredLibraryDirs(cfg.promptDir); err != nil { return err @@ -489,10 +482,6 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error { if strings.TrimSpace(cfg.promptID) == "" { return errors.New("--prompt is required") } - cfg.promptDir = filepath.Clean(cfg.promptDir) - if strings.TrimSpace(cfg.profileDir) != "" { - cfg.profileDir = filepath.Clean(cfg.profileDir) - } if cfg.outputPath != "" { cfg.outputPath = filepath.Clean(cfg.outputPath) } @@ -511,83 +500,6 @@ func finalizeExecutionRequestConfig(fs *flag.FlagSet, cfg *runConfig) error { return nil } -func (c *runConfig) promptDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "prompt-dir") { - return c.promptDir - } - return "" -} - -func (c *runConfig) profileDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "profile-dir") { - return c.profileDir - } - return "" -} - -func (c *runConfig) schemaDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "schema-dir") { - return c.schemaDir - } - return "" -} - -func (c *serveConfig) promptDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "prompt-dir") { - return c.promptDir - } - return "" -} - -func (c *serveConfig) profileDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "profile-dir") { - return c.profileDir - } - return "" -} - -func (c *serveConfig) schemaDirIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "schema-dir") { - return c.schemaDir - } - return "" -} - -func (c *serveConfig) addrIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "addr") { - return c.addr - } - return "" -} - -func (c *serveConfig) artifactRootIfSet(fs *flag.FlagSet) string { - if flagWasSet(fs, "artifact-root") { - return c.artifactRoot - } - return "" -} - -func (c *serveConfig) maxRequestBytesIfSet(fs *flag.FlagSet) *int64 { - if flagWasSet(fs, "max-request-bytes") { - return &c.maxRequestBytes - } - return nil -} - -func (c *serveConfig) maxArtifactBytesIfSet(fs *flag.FlagSet) *int64 { - if flagWasSet(fs, "max-artifact-bytes") { - return &c.maxArtifactBytes - } - return nil -} - -func (c *serveConfig) maxResponseBytesIfSet(fs *flag.FlagSet) *int64 { - if flagWasSet(fs, "max-response-bytes") { - return &c.maxResponseBytes - } - return nil -} - func registerConfigPathFlag(fs *flag.FlagSet, target *string) { fs.StringVar( target, @@ -601,26 +513,8 @@ func registerConfigPathFlag(fs *flag.FlagSet, target *string) { ) } -func resolveAppSettings(fs *flag.FlagSet, configPath string, overrides appconfig.CLIOverrides) (appconfig.AppSettings, error) { - return resolveAppSettingsWithConfigPresence(configPath, flagWasSet(fs, "config"), overrides) -} - -func resolveAppSettingsWithConfigPresence(configPath string, configExplicit bool, overrides appconfig.CLIOverrides) (appconfig.AppSettings, error) { - settings, err := appconfig.LoadConfig(configPath, configExplicit) - if err != nil { - return appconfig.AppSettings{}, fmt.Errorf("application config error: %w", err) - } - - merged, err := appconfig.ApplyCLIOverrides(settings, overrides) - if err != nil { - return appconfig.AppSettings{}, fmt.Errorf("application config error: %w", err) - } - - return merged, nil -} - -func resolveCommonSettings(fs *flag.FlagSet, configPath string, overrides appconfig.CLIOverrides) (commonCommandSettings, error) { - settings, err := resolveAppSettings(fs, configPath, overrides) +func resolveCommonSettings(fs *flag.FlagSet, configPath string) (commonCommandSettings, error) { + settings, diagnostics, err := loadApplicationSettings(configPath, flagWasSet(fs, "config"), applicationOverrides(fs)) if err != nil { return commonCommandSettings{}, err } @@ -636,6 +530,7 @@ func resolveCommonSettings(fs *flag.FlagSet, configPath string, overrides appcon defaultRenderFormat: settings.DefaultRenderFormat, backends: settings.Backends, transientRetries: settings.TransientRetries, + diagnostics: diagnostics, }, nil } diff --git a/internal/adapter/cli/run_test.go b/internal/adapter/cli/run_test.go index d21caf6..212f238 100644 --- a/internal/adapter/cli/run_test.go +++ b/internal/adapter/cli/run_test.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "errors" - "flag" "fmt" "io" "net/http" @@ -18,8 +17,8 @@ import ( "testing" "time" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/promptkit" - appconfig "gitea.maximumdirect.net/eric/scriptorium/internal/config" "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" renderformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" ) @@ -65,7 +64,7 @@ func TestParseMappingsMalformed(t *testing.T) { } func TestParseRunArgsRequiredFlags(t *testing.T) { - configPath := writeAppConfigFile(t, "") + configPath := writeAppConfigFile(t, "{}\n") _, err := parseRunArgs([]string{"--config", configPath, "--profile-dir", "./profiles", "--prompt", "p", "--input", "a=b"}) if err == nil { @@ -161,7 +160,7 @@ func TestParseRunArgsRejectsRawLLMAPIKeyFlag(t *testing.T) { } func TestParseServeArgsRequiredFlags(t *testing.T) { - configPath := writeAppConfigFile(t, "") + configPath := writeAppConfigFile(t, "{}\n") _, err := parseServeArgs([]string{"--config", configPath, "--profile-dir", "./profiles"}) if err == nil { @@ -364,22 +363,9 @@ func TestParseRunArgsInvalidExplicitConfigReturnsError(t *testing.T) { if err == nil { t.Fatal("expected invalid explicit config error") } - if !strings.Contains(err.Error(), "application config error") { - t.Fatalf("expected application config context, got %v", err) - } -} - -func TestResolveAppSettingsMissingImplicitConfigDoesNotError(t *testing.T) { - fs := flag.NewFlagSet("test", flag.ContinueOnError) - settings, err := resolveAppSettings(fs, filepath.Join(t.TempDir(), "missing.yml"), appconfig.CLIOverrides{}) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if settings.SchemaDir != defaults.SchemaDirDefault { - t.Fatalf("expected built-in schema dir, got %q", settings.SchemaDir) - } - if settings.ServerAddr != defaults.HTTPAddrDefault { - t.Fatalf("expected built-in server addr, got %q", settings.ServerAddr) + var loadError *configkit.LoadError + if !errors.As(err, &loadError) { + t.Fatalf("expected structured application config error, got %v", err) } } @@ -413,6 +399,85 @@ schema_dir: ./from-config/schemas } } +func TestApplicationFlagsPreserveExplicitEmptyValues(t *testing.T) { + configPath := writeAppConfigFile(t, `prompt_dir: ./configured/prompts +profile_dir: ./configured/profiles +schema_dir: ./configured/schemas +server: + addr: ":9090" + artifact_root: ./configured/artifacts +`) + if _, err := parseRunArgs([]string{"--config", configPath, "--prompt", "p", "--prompt-dir="}); err == nil || !strings.Contains(err.Error(), "prompt directory is required") { + t.Fatalf("explicit empty prompt directory restored lower value: %v", err) + } + render, err := parseRenderArgs([]string{"--config", configPath, "--prompt", "p", "--profile-dir=", "--schema-dir="}) + if err != nil || render.profileDir != "" || render.schemaDir != "" { + t.Fatalf("render did not preserve explicit empty sources: cfg=%+v err=%v", render, err) + } + serve, err := parseServeArgs([]string{"--config", configPath, "--addr=", "--artifact-root="}) + if err != nil || serve.addr != "" || serve.artifactRoot != "" { + t.Fatalf("serve did not preserve explicit empty settings: cfg=%+v err=%v", serve, err) + } + + prompt, err := parsePromptInspectionArgs([]string{"--config", configPath, "--prompt", "p", "--prompt-dir="}) + if err != nil { + t.Fatal(err) + } + if _, err := resolveAppSettingsForPromptInspection(prompt); err == nil || !strings.Contains(err.Error(), "prompt directory is required") { + t.Fatalf("prompt inspection restored lower directory: %v", err) + } + profile, err := parseProfileInspectionArgs([]string{"--config", configPath, "--profile", "p", "--profile-dir="}) + if err != nil { + t.Fatal(err) + } + settings, err := resolveAppSettingsForProfileInspection(profile) + if err != nil || settings.profileDir != "" { + t.Fatalf("profile inspection required a source or restored lower value: settings=%+v err=%v", settings, err) + } +} + +func TestApplicationLoadUsesNativeYAMLAndSafeDiagnostics(t *testing.T) { + for _, body := range []string{ + "", + "server: null\n", + "server:\n addr: null\n", + "server:\n unknown: 1\n", + "prompt_dir: &alias ./prompts\n", + "prompt_dir: ./prompts\n---\nprofile_dir: ./profiles\n", + } { + path := writeAppConfigFile(t, body) + _, err := parseRunArgs([]string{"--config", path, "--prompt-dir", "./prompts", "--prompt", "p"}) + var loadError *configkit.LoadError + if !errors.As(err, &loadError) { + t.Fatalf("native YAML failure %q lost structured diagnostics: %v", body, err) + } + } + + const rejected = "sensitive-synthetic-marker" + path := writeAppConfigFile(t, "defaults:\n render_format: "+rejected+"\n") + code, stdout, stderr := runCLICommand(t, renderCommand, []string{"--config", path, "--prompt-dir", "./prompts", "--prompt", "p"}) + if code != ExitRuntimeError || stdout != "" || !strings.Contains(stderr, "validate.error") || strings.Contains(stderr, rejected) { + t.Fatalf("unsafe or misclassified load diagnostic: code=%d stdout=%q stderr=%q", code, stdout, stderr) + } +} + +type failingDiagnosticWriter struct{} + +func (failingDiagnosticWriter) Write([]byte) (int, error) { + return 0, errors.New("diagnostic sink unavailable") +} + +func TestApplicationDiagnosticWriterFailure(t *testing.T) { + diagnostics := []configkit.Diagnostic{{Code: "test.error", Message: "safe diagnostic"}} + if err := writeApplicationWarnings(failingDiagnosticWriter{}, diagnostics); err == nil { + t.Fatal("expected diagnostic writer failure") + } + path := writeAppConfigFile(t, "unknown_field: secret-value\n") + if code := runCommand([]string{"--config", path, "--prompt", "sample"}, io.Discard, failingDiagnosticWriter{}); code != ExitRuntimeError { + t.Fatalf("diagnostic writer failure must retain runtime error exit status, got %d", code) + } +} + func TestParseRenderArgsWithExplicitConfigLoadsDirectoriesAndFormat(t *testing.T) { configPath := writeAppConfigFile(t, ` prompt_dir: ./from-config/prompts @@ -1533,7 +1598,7 @@ func TestExecutionReportPathValidationAndPreInvocationFailure(t *testing.T) { if err != nil { t.Fatal(err) } - if code != ExitRuntimeError || stdout != "" || !strings.Contains(stderr, "run parse error") || string(data) != "old report" { + if code != ExitRuntimeError || stdout != "" || !strings.Contains(stderr, "source.missing") || string(data) != "old report" { t.Fatalf("pre-invocation failure changed report: code=%d stdout=%q stderr=%q report=%q", code, stdout, stderr, data) } } diff --git a/internal/config/config.go b/internal/config/config.go deleted file mode 100644 index 9907451..0000000 --- a/internal/config/config.go +++ /dev/null @@ -1,312 +0,0 @@ -package config - -import ( - "bytes" - "errors" - "fmt" - "io" - "os" - "path/filepath" - "sort" - "strings" - - "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" - renderformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" - "gopkg.in/yaml.v3" -) - -const ( - DefaultConfigPath = "/etc/scriptorium/config.yml" - DefaultConfigPathLocal = "/usr/local/etc/scriptorium/config.yml" -) - -var defaultConfigSearchPaths = []string{ - DefaultConfigPathLocal, - DefaultConfigPath, -} - -var ( - ErrConfigNotFound = errors.New("config file not found") - ErrInvalidConfigYAML = errors.New("invalid config YAML") - ErrInvalidConfig = errors.New("invalid config") -) - -// Config is the on-disk YAML shape for application-level settings. -type Config struct { - PromptDir string `yaml:"prompt_dir"` - ProfileDir string `yaml:"profile_dir"` - SchemaDir string `yaml:"schema_dir"` - TransientRetries *strictInteger `yaml:"transient_retries"` - Server ServerConfig `yaml:"server"` - Defaults DefaultsConfig `yaml:"defaults"` - Backends map[string]BackendConfig `yaml:"backends"` -} - -type BackendConfig struct { - Endpoint string `yaml:"endpoint"` - APIKeyEnv string `yaml:"api_key_env"` - ExtraParams map[string]any `yaml:"extra_params"` - ConcurrencyLimit int `yaml:"concurrency_limit"` - QueueCapacity *int `yaml:"queue_capacity"` -} - -type strictInteger int - -func (value *strictInteger) UnmarshalYAML(node *yaml.Node) error { - if node.Tag != "!!int" { - return fmt.Errorf("expected integer, got %s", node.Tag) - } - var decoded int - if err := node.Decode(&decoded); err != nil { - return err - } - *value = strictInteger(decoded) - return nil -} - -type BackendSettings struct { - ID string - Endpoint string - APIKeyEnv string - ExtraParams map[string]any - ConcurrencyLimit int - QueueCapacity *int -} - -type ServerConfig struct { - Addr string `yaml:"addr"` - ArtifactRoot string `yaml:"artifact_root"` - MaxRequestBytes *int64 `yaml:"max_request_bytes"` - MaxArtifactBytes *int64 `yaml:"max_artifact_bytes"` - MaxResponseBytes *int64 `yaml:"max_response_bytes"` -} - -type DefaultsConfig struct { - RenderFormat string `yaml:"render_format"` -} - -// AppSettings is the resolved application settings used by adapters. -type AppSettings struct { - PromptDir string - ProfileDir string - SchemaDir string - ServerAddr string - ArtifactRoot string - MaxRequestBytes int64 - MaxArtifactBytes int64 - MaxResponseBytes int64 - DefaultRenderFormat renderformat.PreparedRunOutputFormat - Backends []BackendSettings - TransientRetries *int -} - -// CLIOverrides can be applied after config load to enforce precedence. -type CLIOverrides struct { - PromptDir string - ProfileDir string - SchemaDir string - ServerAddr string - ArtifactRoot string - MaxRequestBytes *int64 - MaxArtifactBytes *int64 - MaxResponseBytes *int64 - RenderFormat string -} - -// BuiltInDefaults returns compile-time application defaults. -func BuiltInDefaults() AppSettings { - return AppSettings{ - SchemaDir: defaults.SchemaDirDefault, - ServerAddr: defaults.HTTPAddrDefault, - MaxRequestBytes: defaults.HTTPMaxRequestBytesDefault, - MaxArtifactBytes: defaults.HTTPMaxArtifactBytesDefault, - MaxResponseBytes: defaults.HTTPMaxResponseBytesDefault, - DefaultRenderFormat: renderformat.DefaultPreparedRunOutputFormat, - } -} - -// LoadConfig loads and resolves app settings from YAML. -// -// If explicit is false and the file does not exist, built-in defaults are returned. -// If explicit is true, missing file is an error. -func LoadConfig(path string, explicit bool) (AppSettings, error) { - resolved := BuiltInDefaults() - configPath := strings.TrimSpace(path) - if configPath == "" { - if explicit { - configPath = DefaultConfigPath - } else { - for _, candidate := range defaultConfigSearchPaths { - if _, err := os.Stat(candidate); err == nil { - configPath = candidate - break - } else if !errors.Is(err, os.ErrNotExist) { - return AppSettings{}, fmt.Errorf("failed to stat config file %q: %w", candidate, err) - } - } - } - } - if configPath == "" { - if explicit { - return AppSettings{}, fmt.Errorf("%w: %s", ErrConfigNotFound, DefaultConfigPath) - } - return resolved, nil - } - configPath = filepath.Clean(configPath) - - raw, err := os.ReadFile(configPath) - if err != nil { - if errors.Is(err, os.ErrNotExist) { - if explicit { - return AppSettings{}, fmt.Errorf("%w: %s", ErrConfigNotFound, configPath) - } - return resolved, nil - } - return AppSettings{}, fmt.Errorf("failed to read config file %q: %w", configPath, err) - } - - cfg, err := decodeConfig(raw) - if err != nil { - return AppSettings{}, err - } - - resolved, err = applyConfig(resolved, cfg) - if err != nil { - return AppSettings{}, err - } - return resolved, nil -} - -// ApplyCLIOverrides applies CLI-provided overrides over resolved config settings. -func ApplyCLIOverrides(base AppSettings, overrides CLIOverrides) (AppSettings, error) { - out := base - - if v := strings.TrimSpace(overrides.PromptDir); v != "" { - out.PromptDir = filepath.Clean(v) - } - if v := strings.TrimSpace(overrides.ProfileDir); v != "" { - out.ProfileDir = filepath.Clean(v) - } - if v := strings.TrimSpace(overrides.SchemaDir); v != "" { - out.SchemaDir = filepath.Clean(v) - } - if v := strings.TrimSpace(overrides.ServerAddr); v != "" { - out.ServerAddr = v - } - if v := strings.TrimSpace(overrides.ArtifactRoot); v != "" { - out.ArtifactRoot = filepath.Clean(v) - } - if overrides.MaxRequestBytes != nil { - if *overrides.MaxRequestBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_request_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxRequestBytes = *overrides.MaxRequestBytes - } - if overrides.MaxArtifactBytes != nil { - if *overrides.MaxArtifactBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_artifact_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxArtifactBytes = *overrides.MaxArtifactBytes - } - if overrides.MaxResponseBytes != nil { - if *overrides.MaxResponseBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_response_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxResponseBytes = *overrides.MaxResponseBytes - } - if rawFormat := strings.TrimSpace(overrides.RenderFormat); rawFormat != "" { - parsed, err := renderformat.ParsePreparedRunOutputFormat(rawFormat) - if err != nil { - return AppSettings{}, fmt.Errorf("%w: defaults.render_format: %v", ErrInvalidConfig, err) - } - out.DefaultRenderFormat = parsed - } - - return out, nil -} - -func decodeConfig(raw []byte) (Config, error) { - var cfg Config - decoder := yaml.NewDecoder(bytes.NewReader(raw)) - decoder.KnownFields(true) - if err := decoder.Decode(&cfg); err != nil { - if errors.Is(err, io.EOF) { - return Config{}, nil - } - return Config{}, fmt.Errorf("%w: %v", ErrInvalidConfigYAML, err) - } - return cfg, nil -} - -func applyConfig(base AppSettings, cfg Config) (AppSettings, error) { - out := base - if cfg.TransientRetries != nil { - if *cfg.TransientRetries < 0 { - return AppSettings{}, fmt.Errorf("%w: transient_retries must be greater than or equal to 0", ErrInvalidConfig) - } - value := int(*cfg.TransientRetries) - out.TransientRetries = &value - } - - if v := strings.TrimSpace(cfg.PromptDir); v != "" { - out.PromptDir = filepath.Clean(v) - } - if v := strings.TrimSpace(cfg.ProfileDir); v != "" { - out.ProfileDir = filepath.Clean(v) - } - if v := strings.TrimSpace(cfg.SchemaDir); v != "" { - out.SchemaDir = filepath.Clean(v) - } - if v := strings.TrimSpace(cfg.Server.Addr); v != "" { - out.ServerAddr = v - } - if v := strings.TrimSpace(cfg.Server.ArtifactRoot); v != "" { - out.ArtifactRoot = filepath.Clean(v) - } - if cfg.Server.MaxRequestBytes != nil { - if *cfg.Server.MaxRequestBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_request_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxRequestBytes = *cfg.Server.MaxRequestBytes - } - if cfg.Server.MaxArtifactBytes != nil { - if *cfg.Server.MaxArtifactBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_artifact_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxArtifactBytes = *cfg.Server.MaxArtifactBytes - } - if cfg.Server.MaxResponseBytes != nil { - if *cfg.Server.MaxResponseBytes < 0 { - return AppSettings{}, fmt.Errorf("%w: server.max_response_bytes must be greater than or equal to 0", ErrInvalidConfig) - } - out.MaxResponseBytes = *cfg.Server.MaxResponseBytes - } - if rawFormat := strings.TrimSpace(cfg.Defaults.RenderFormat); rawFormat != "" { - parsed, err := renderformat.ParsePreparedRunOutputFormat(rawFormat) - if err != nil { - return AppSettings{}, fmt.Errorf("%w: defaults.render_format: %v", ErrInvalidConfig, err) - } - out.DefaultRenderFormat = parsed - } - if len(cfg.Backends) > 0 { - ids := make([]string, 0, len(cfg.Backends)) - for id := range cfg.Backends { - ids = append(ids, id) - } - sort.Strings(ids) - out.Backends = make([]BackendSettings, 0, len(ids)) - for _, id := range ids { - backend := cfg.Backends[id] - out.Backends = append(out.Backends, BackendSettings{ - ID: id, - Endpoint: backend.Endpoint, - APIKeyEnv: backend.APIKeyEnv, - ExtraParams: backend.ExtraParams, - ConcurrencyLimit: backend.ConcurrencyLimit, - QueueCapacity: backend.QueueCapacity, - }) - } - } - - return out, nil -} diff --git a/internal/config/config_test.go b/internal/config/config_test.go deleted file mode 100644 index 5835a3e..0000000 --- a/internal/config/config_test.go +++ /dev/null @@ -1,461 +0,0 @@ -package config - -import ( - "errors" - "os" - "path/filepath" - "reflect" - "testing" - - "gitea.maximumdirect.net/eric/scriptorium/internal/defaults" - renderformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" -) - -func TestLoadConfigMissingImplicitPathUsesBuiltInDefaults(t *testing.T) { - tmp := t.TempDir() - missing := filepath.Join(tmp, "missing.yml") - - got, err := LoadConfig(missing, false) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - want := BuiltInDefaults() - if !reflect.DeepEqual(got, want) { - t.Fatalf("unexpected settings: got=%+v want=%+v", got, want) - } -} - -func TestBuiltInDefaultsIncludeHTTPSizeLimits(t *testing.T) { - got := BuiltInDefaults() - - if got.MaxRequestBytes != defaults.HTTPMaxRequestBytesDefault { - t.Fatalf("unexpected max request bytes: %d", got.MaxRequestBytes) - } - if got.MaxArtifactBytes != defaults.HTTPMaxArtifactBytesDefault { - t.Fatalf("unexpected max artifact bytes: %d", got.MaxArtifactBytes) - } - if got.MaxResponseBytes != defaults.HTTPMaxResponseBytesDefault { - t.Fatalf("unexpected max response bytes: %d", got.MaxResponseBytes) - } -} - -func TestLoadConfigMissingExplicitPathReturnsError(t *testing.T) { - tmp := t.TempDir() - missing := filepath.Join(tmp, "missing.yml") - - _, err := LoadConfig(missing, true) - if err == nil { - t.Fatal("expected error for missing explicit config") - } - if !errors.Is(err, ErrConfigNotFound) { - t.Fatalf("expected ErrConfigNotFound, got %v", err) - } -} - -func TestLoadConfigInvalidYAMLReturnsError(t *testing.T) { - path := writeConfigFile(t, "config.yml", "prompt_dir: [") - - _, err := LoadConfig(path, true) - if err == nil { - t.Fatal("expected invalid YAML error") - } - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } -} - -func TestLoadConfigInvalidYAMLImplicitPathReturnsError(t *testing.T) { - path := writeConfigFile(t, "config.yml", "prompt_dir: [") - - _, err := LoadConfig(path, false) - if err == nil { - t.Fatal("expected invalid YAML error") - } - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } -} - -func TestLoadConfigUnknownFieldReturnsError(t *testing.T) { - path := writeConfigFile(t, "config.yml", "unknown_field: true\n") - - _, err := LoadConfig(path, true) - if err == nil { - t.Fatal("expected unknown field error") - } - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } -} - -func TestLoadConfigAPIKeyFieldIsRejectedAsUnknown(t *testing.T) { - path := writeConfigFile(t, "config.yml", "api_key: secret\n") - - _, err := LoadConfig(path, true) - if err == nil { - t.Fatal("expected unknown field error for api_key") - } - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } -} - -func TestLoadConfigBackendsRetainsResolvedSettingsInSortedOrder(t *testing.T) { - path := writeConfigFile(t, "config.yml", ` -backends: - zebra: - endpoint: https://zebra.example/v1 - api_key_env: ZEBRA_API_KEY - extra_params: - provider_option: enabled - nested: - enabled: true - attempts: 2 - concurrency_limit: 2 - alpha: - endpoint: http://alpha.example/v1 - queue_capacity: 0 -`) - - got, err := LoadConfig(path, true) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if len(got.Backends) != 2 { - t.Fatalf("expected two backends, got %#v", got.Backends) - } - if got.Backends[0].ID != "alpha" || got.Backends[1].ID != "zebra" { - t.Fatalf("expected sorted backend IDs, got %#v", got.Backends) - } - if got.Backends[0].QueueCapacity == nil || *got.Backends[0].QueueCapacity != 0 { - t.Fatalf("expected explicit zero queue capacity, got %#v", got.Backends[0].QueueCapacity) - } - if got.Backends[1].QueueCapacity != nil { - t.Fatalf("expected omitted queue capacity to remain nil, got %#v", got.Backends[1].QueueCapacity) - } - wantParams := map[string]any{ - "provider_option": "enabled", - "nested": map[string]any{ - "enabled": true, - "attempts": 2, - }, - } - if !reflect.DeepEqual(got.Backends[1].ExtraParams, wantParams) { - t.Fatalf("unexpected extra params: got=%#v want=%#v", got.Backends[1].ExtraParams, wantParams) - } - if got.Backends[1].Endpoint != "https://zebra.example/v1" || got.Backends[1].APIKeyEnv != "ZEBRA_API_KEY" || got.Backends[1].ConcurrencyLimit != 2 { - t.Fatalf("unexpected zebra backend: %#v", got.Backends[1]) - } -} - -func TestLoadConfigRejectsUnknownOrSecretBackendFields(t *testing.T) { - for name, body := range map[string]string{ - "unknown": "backends:\n local:\n endpoint: http://localhost:11434/v1\n unexpected: value\n", - "secret": "backends:\n local:\n endpoint: http://localhost:11434/v1\n api_key: secret\n", - } { - t.Run(name, func(t *testing.T) { - path := writeConfigFile(t, "config.yml", body) - _, err := LoadConfig(path, true) - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } - }) - } -} - -func TestLoadConfigRejectsInvalidBackendFieldTypes(t *testing.T) { - path := writeConfigFile(t, "config.yml", "backends:\n local:\n endpoint: http://localhost:11434/v1\n queue_capacity: not-a-number\n") - - _, err := LoadConfig(path, true) - if !errors.Is(err, ErrInvalidConfigYAML) { - t.Fatalf("expected ErrInvalidConfigYAML, got %v", err) - } -} - -func TestLoadConfigValidConfigSetsDirectoriesAndServerAddr(t *testing.T) { - path := writeConfigFile(t, "config.yml", ` -prompt_dir: ./prompts -profile_dir: ./profiles -schema_dir: ./schemas -server: - addr: 127.0.0.1:9090 - artifact_root: ./artifacts - max_request_bytes: 1024 - max_artifact_bytes: 2048 - max_response_bytes: 4096 -defaults: - render_format: json -`) - - got, err := LoadConfig(path, true) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if got.PromptDir != filepath.Clean("./prompts") { - t.Fatalf("unexpected prompt_dir: %q", got.PromptDir) - } - if got.ProfileDir != filepath.Clean("./profiles") { - t.Fatalf("unexpected profile_dir: %q", got.ProfileDir) - } - if got.SchemaDir != filepath.Clean("./schemas") { - t.Fatalf("unexpected schema_dir: %q", got.SchemaDir) - } - if got.ServerAddr != "127.0.0.1:9090" { - t.Fatalf("unexpected server.addr: %q", got.ServerAddr) - } - if got.ArtifactRoot != filepath.Clean("./artifacts") { - t.Fatalf("unexpected server.artifact_root: %q", got.ArtifactRoot) - } - if got.MaxRequestBytes != 1024 { - t.Fatalf("unexpected server.max_request_bytes: %d", got.MaxRequestBytes) - } - if got.MaxArtifactBytes != 2048 { - t.Fatalf("unexpected server.max_artifact_bytes: %d", got.MaxArtifactBytes) - } - if got.MaxResponseBytes != 4096 { - t.Fatalf("unexpected server.max_response_bytes: %d", got.MaxResponseBytes) - } - if got.DefaultRenderFormat != renderformat.PreparedRunFormatJSON { - t.Fatalf("unexpected defaults.render_format: %q", got.DefaultRenderFormat) - } -} - -func TestLoadConfigAcceptsZeroHTTPSizeLimits(t *testing.T) { - path := writeConfigFile(t, "config.yml", ` -server: - max_request_bytes: 0 - max_artifact_bytes: 0 - max_response_bytes: 0 -`) - - got, err := LoadConfig(path, true) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if got.MaxRequestBytes != 0 || got.MaxArtifactBytes != 0 || got.MaxResponseBytes != 0 { - t.Fatalf("expected zero limits to be preserved, got request=%d artifact=%d response=%d", got.MaxRequestBytes, got.MaxArtifactBytes, got.MaxResponseBytes) - } -} - -func TestLoadConfigPreservesTransientRetryPresence(t *testing.T) { - for _, tc := range []struct { - name string - body string - want *int - }{ - {name: "omitted", body: "prompt_dir: ./prompts\n"}, - {name: "null", body: "transient_retries: null\n"}, - {name: "zero", body: "transient_retries: 0\n", want: intPointer(0)}, - {name: "positive", body: "transient_retries: 3\n", want: intPointer(3)}, - } { - t.Run(tc.name, func(t *testing.T) { - settings, err := LoadConfig(writeConfigFile(t, "config.yml", tc.body), true) - if err != nil { - t.Fatal(err) - } - if !reflect.DeepEqual(settings.TransientRetries, tc.want) { - t.Fatalf("retry setting: got %v, want %v", settings.TransientRetries, tc.want) - } - }) - } - for _, tc := range []struct { - body string - want error - }{ - {body: "transient_retries: -1\n", want: ErrInvalidConfig}, - {body: "transient_retries: wrong\n", want: ErrInvalidConfigYAML}, - {body: "transient_retries: 1.5\n", want: ErrInvalidConfigYAML}, - } { - _, err := LoadConfig(writeConfigFile(t, "config.yml", tc.body), true) - if !errors.Is(err, tc.want) { - t.Fatalf("body %q: expected %v, got %v", tc.body, tc.want, err) - } - } -} - -func intPointer(value int) *int { return &value } - -func TestLoadConfigRejectsNegativeHTTPSizeLimits(t *testing.T) { - tests := []struct { - name string - body string - }{ - {name: "request", body: "server:\n max_request_bytes: -1\n"}, - {name: "artifact", body: "server:\n max_artifact_bytes: -1\n"}, - {name: "response", body: "server:\n max_response_bytes: -1\n"}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - path := writeConfigFile(t, "config.yml", tc.body) - _, err := LoadConfig(path, true) - if !errors.Is(err, ErrInvalidConfig) { - t.Fatalf("expected ErrInvalidConfig, got %v", err) - } - }) - } -} - -func TestLoadConfigEmptyFileResolvesToBuiltInDefaults(t *testing.T) { - path := writeConfigFile(t, "config.yml", "") - - got, err := LoadConfig(path, true) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - want := BuiltInDefaults() - if !reflect.DeepEqual(got, want) { - t.Fatalf("unexpected settings: got=%+v want=%+v", got, want) - } -} - -func TestLoadConfigImplicitSearchPrefersUsrLocalEtcOverEtc(t *testing.T) { - tmp := t.TempDir() - localPath := filepath.Join(tmp, "usr-local.yml") - etcPath := filepath.Join(tmp, "etc.yml") - - if err := os.WriteFile(localPath, []byte("prompt_dir: ./from-usr-local\n"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(etcPath, []byte("prompt_dir: ./from-etc\n"), 0o644); err != nil { - t.Fatal(err) - } - - orig := defaultConfigSearchPaths - defaultConfigSearchPaths = []string{localPath, etcPath} - t.Cleanup(func() { defaultConfigSearchPaths = orig }) - - got, err := LoadConfig("", false) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if got.PromptDir != filepath.Clean("./from-usr-local") { - t.Fatalf("expected usr-local config to win, got prompt_dir=%q", got.PromptDir) - } -} - -func TestLoadConfigImplicitSearchFallsBackToEtcWhenUsrLocalMissing(t *testing.T) { - tmp := t.TempDir() - missingLocal := filepath.Join(tmp, "missing-local.yml") - etcPath := filepath.Join(tmp, "etc.yml") - - if err := os.WriteFile(etcPath, []byte("prompt_dir: ./from-etc\n"), 0o644); err != nil { - t.Fatal(err) - } - - orig := defaultConfigSearchPaths - defaultConfigSearchPaths = []string{missingLocal, etcPath} - t.Cleanup(func() { defaultConfigSearchPaths = orig }) - - got, err := LoadConfig("", false) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - if got.PromptDir != filepath.Clean("./from-etc") { - t.Fatalf("expected etc fallback config, got prompt_dir=%q", got.PromptDir) - } -} - -func TestApplyCLIOverridesAppliesPrecedence(t *testing.T) { - base := AppSettings{ - PromptDir: "/from/config/prompts", - ProfileDir: "/from/config/profiles", - SchemaDir: "/from/config/schemas", - ServerAddr: ":1234", - ArtifactRoot: "/from/config/artifacts", - MaxRequestBytes: 111, - MaxArtifactBytes: 222, - MaxResponseBytes: 333, - DefaultRenderFormat: renderformat.PreparedRunFormatJSON, - } - maxRequestBytes := int64(0) - maxArtifactBytes := int64(444) - maxResponseBytes := int64(555) - - got, err := ApplyCLIOverrides(base, CLIOverrides{ - PromptDir: "./prompts-cli", - ProfileDir: "./profiles-cli", - SchemaDir: "./schemas-cli", - ServerAddr: ":8081", - ArtifactRoot: "./artifacts-cli", - MaxRequestBytes: &maxRequestBytes, - MaxArtifactBytes: &maxArtifactBytes, - MaxResponseBytes: &maxResponseBytes, - RenderFormat: "text", - }) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if got.PromptDir != filepath.Clean("./prompts-cli") { - t.Fatalf("unexpected prompt dir: %q", got.PromptDir) - } - if got.ProfileDir != filepath.Clean("./profiles-cli") { - t.Fatalf("unexpected profile dir: %q", got.ProfileDir) - } - if got.SchemaDir != filepath.Clean("./schemas-cli") { - t.Fatalf("unexpected schema dir: %q", got.SchemaDir) - } - if got.ServerAddr != ":8081" { - t.Fatalf("unexpected server addr: %q", got.ServerAddr) - } - if got.ArtifactRoot != filepath.Clean("./artifacts-cli") { - t.Fatalf("unexpected artifact root: %q", got.ArtifactRoot) - } - if got.MaxRequestBytes != 0 { - t.Fatalf("unexpected max request bytes: %d", got.MaxRequestBytes) - } - if got.MaxArtifactBytes != 444 { - t.Fatalf("unexpected max artifact bytes: %d", got.MaxArtifactBytes) - } - if got.MaxResponseBytes != 555 { - t.Fatalf("unexpected max response bytes: %d", got.MaxResponseBytes) - } - if got.DefaultRenderFormat != renderformat.PreparedRunFormatText { - t.Fatalf("unexpected render format: %q", got.DefaultRenderFormat) - } -} - -func TestApplyCLIOverridesRejectsNegativeHTTPSizeLimits(t *testing.T) { - negative := int64(-1) - - tests := []struct { - name string - overrides CLIOverrides - }{ - {name: "request", overrides: CLIOverrides{MaxRequestBytes: &negative}}, - {name: "artifact", overrides: CLIOverrides{MaxArtifactBytes: &negative}}, - {name: "response", overrides: CLIOverrides{MaxResponseBytes: &negative}}, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - _, err := ApplyCLIOverrides(BuiltInDefaults(), tc.overrides) - if !errors.Is(err, ErrInvalidConfig) { - t.Fatalf("expected ErrInvalidConfig, got %v", err) - } - }) - } -} - -func TestApplyCLIOverridesInvalidRenderFormatReturnsError(t *testing.T) { - _, err := ApplyCLIOverrides(BuiltInDefaults(), CLIOverrides{RenderFormat: "yaml"}) - if err == nil { - t.Fatal("expected invalid render format error") - } - if !errors.Is(err, ErrInvalidConfig) { - t.Fatalf("expected ErrInvalidConfig, got %v", err) - } -} - -func writeConfigFile(t *testing.T, name, content string) string { - t.Helper() - path := filepath.Join(t.TempDir(), name) - if err := os.WriteFile(path, []byte(content), 0o644); err != nil { - t.Fatalf("failed to write config file: %v", err) - } - return path -} diff --git a/internal/config/load.go b/internal/config/load.go index 33f6e1a..b60ab10 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -18,7 +18,7 @@ type LoadOptions struct { Overrides []configkit.Override } -// Load resolves one ConfigKit document without changing the executable's legacy loader. +// Load resolves one ConfigKit document and retains its inspection snapshot. func Load(options LoadOptions) (*configkit.Result[Document], error) { var source configkit.Source if options.Explicit { diff --git a/internal/config/settings.go b/internal/config/settings.go new file mode 100644 index 0000000..df10488 --- /dev/null +++ b/internal/config/settings.go @@ -0,0 +1,37 @@ +package config + +import renderformat "gitea.maximumdirect.net/eric/scriptorium/internal/format" + +const ( + DefaultConfigPath = "/etc/scriptorium/config.yml" + DefaultConfigPathLocal = "/usr/local/etc/scriptorium/config.yml" +) + +var defaultConfigSearchPaths = []string{ + DefaultConfigPathLocal, + DefaultConfigPath, +} + +// AppSettings holds the validated application values used by executable adapters. +type AppSettings struct { + PromptDir string + ProfileDir string + SchemaDir string + ServerAddr string + ArtifactRoot string + MaxRequestBytes int64 + MaxArtifactBytes int64 + MaxResponseBytes int64 + DefaultRenderFormat renderformat.PreparedRunOutputFormat + Backends []BackendSettings + TransientRetries *int +} + +type BackendSettings struct { + ID string + Endpoint string + APIKeyEnv string + ExtraParams map[string]any + ConcurrencyLimit int + QueueCapacity *int +}