diff --git a/docs/config.md b/docs/config.md index 9078066..3e2545b 100644 --- a/docs/config.md +++ b/docs/config.md @@ -106,9 +106,13 @@ remote state with an unsafe legacy identity must be migrated before use. - An omitted profile, an empty profile option, and the exact name `default` select the base configuration. Any other name selects `profiles/.yml` relative to the pipeline root. +- Campaign, session, and canonical-party documents are strict single-source + ConfigKit schemas. They do not enable imports, profiles, or `configkit` + directives. - Configured timeout and retry-delay durations must be positive. An omitted artifact timeout continues to inherit its configured Scriptorium timeout. - Session files must be concrete; unresolved `{{ ... }}` placeholders fail load. +- Session and party inputs inspected before typed loading are limited to 8 MiB. - Pipeline defaults are applied before validation. - Campaign and session identities must agree. - Required stable files (`speakers_file`, `autocorrect_file`, `glossary_file`, diff --git a/docs/internal/configuration.md b/docs/internal/configuration.md index 6457c21..78be729 100644 --- a/docs/internal/configuration.md +++ b/docs/internal/configuration.md @@ -44,6 +44,14 @@ download retain that exact pipeline object and its private provenance. Removing a temporary downloaded session file therefore cannot invalidate the resolved pipeline or campaign context. +Campaign and session files use independent ConfigKit definitions with imports +and profiles disabled. Campaign files normally load directly from their source; +session files are first read through the 8 MiB bounded-read boundary because the +application must reject unresolved template placeholders before passing an exact +byte snapshot to ConfigKit. Local and remotely downloaded session bytes then use +the same typed loader and retain the existing post-load identity expectations, +stable-input precedence, and source-relative path semantics. + The application also has a separate read-only inspection resolver for `config validate`, `config show`, and `config sources`. It uses the same production root/profile and campaign selection functions, but never routes through session discovery, @@ -88,13 +96,16 @@ configured roster authority; player and session-party fields are unsupported. ## Canonical Party Domain -`ParseParty` is the package-owned boundary for parsing a party source. It -requires a top-level `schema_version` and strictly validates the -`narratio.party.v1` contract into ordered character domain values. The -canonical value retains a separate exact byte copy of its source so consumers -can materialize the authored party document without reserializing it. Its -`PlayersYAML` method deterministically derives the versioned players-only -projection. +`ParseParty` is the package-owned boundary for parsing a party source. Its +single-source ConfigKit definition disables imports and profiles, requires +`schema_version` through `VersionRule`, and strictly decodes the +character-keyed typed schema before applying Narratio's party-domain checks. +Successful metadata coordinates recover authored character-map order without a +second YAML parse; class and alias slice order comes directly from typed decode. +The canonical value retains a separate exact byte copy of its source so +consumers can materialize the authored party document without reserializing it. +Its `PlayersYAML` method deterministically derives the ID-sorted, versioned +players-only projection. An unversioned source is rejected before resolution, family expansion, or prepare-stage materialization. @@ -159,8 +170,9 @@ metadata, and root-relative path behavior. Application configuration-loader test single-read boundary by changing the pipeline file after its initial load and confirming local session resolution retains the original pipeline. Other configuration tests continue to protect defaults and validation after assembly. -`party_test.go` protects the versioned party schema, domain invariants, and -deterministic players projection without involving campaign or runtime wiring. +`party_test.go` protects the versioned typed party schema, domain invariants, +authored character order, retained bytes, and deterministic players projection +without involving campaign or runtime wiring. `party_resolution_test.go` protects campaign-owned party loading, canonical input restrictions, source provenance, and virtual players input selection. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 88e176a..8542cae 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -240,7 +240,10 @@ still fail. Commands use the new pipeline loader with no format fallback. Update ## Stage 5 — Migrate campaign, session, and canonical party loading -Status: Pending +Status: Complete — Migrated campaign, session, and canonical-party documents to +independent ConfigKit definitions, preserved bounded session inspection, exact +party bytes and metadata-derived authored order, and retained identity, source, +family-expansion, and validation behavior; verified all tests and focused races. Read: `internal/config/load.go`, `party.go`, `party_resolution.go`, `artifact_families.go`, session-template tests, `internal/app/config_loader.go`, diff --git a/internal/config/campaign_config_test.go b/internal/config/campaign_config_test.go index ee0786a..880cf69 100644 --- a/internal/config/campaign_config_test.go +++ b/internal/config/campaign_config_test.go @@ -60,7 +60,7 @@ func TestCampaignStrictDecodeRejectsLegacyCampaignField(t *testing.T) { if err == nil { t.Fatal("expected load error, got nil") } - if !strings.Contains(err.Error(), "campaign file") || !strings.Contains(err.Error(), "strict decode failed") { + if !strings.Contains(err.Error(), "load campaign config") || !strings.Contains(err.Error(), "decode.unknown_field") { t.Fatalf("error = %q, want campaign strict decode context", err.Error()) } } @@ -75,7 +75,7 @@ func TestCampaignStrictDecodeRejectsUnknownFields(t *testing.T) { if err == nil { t.Fatal("expected load error, got nil") } - if !strings.Contains(err.Error(), "campaign file") || !strings.Contains(err.Error(), "strict decode failed") { + if !strings.Contains(err.Error(), "load campaign config") || !strings.Contains(err.Error(), "decode.unknown_field") { t.Fatalf("error = %q, want campaign strict decode context", err.Error()) } } diff --git a/internal/config/document_configkit.go b/internal/config/document_configkit.go new file mode 100644 index 0000000..6d74f8f --- /dev/null +++ b/internal/config/document_configkit.go @@ -0,0 +1,32 @@ +package config + +import ( + "fmt" + + configkit "gitea.maximumdirect.net/eric/ConfigKit" +) + +const maxConfigDocumentBytes int64 = 8 << 20 + +func loadCampaignWithConfigKit(path string) (*CampaignConfig, error) { + result, err := configkit.Load(configkit.File(path), configkit.Options[CampaignConfig]{ + Definition: configkit.Definition[CampaignConfig]{}, + }) + if err != nil { + return nil, describeConfigKitLoadError("campaign", err) + } + return &result.Config, nil +} + +func loadSessionWithConfigKit(label string, data []byte) (*SessionConfig, error) { + if int64(len(data)) > maxConfigDocumentBytes { + return nil, fmt.Errorf("load session config: session file %q exceeds %d-byte limit", label, maxConfigDocumentBytes) + } + result, err := configkit.Load(configkit.Bytes(label, data), configkit.Options[SessionConfig]{ + Definition: configkit.Definition[SessionConfig]{}, + }) + if err != nil { + return nil, describeConfigKitLoadError("session", err) + } + return &result.Config, nil +} diff --git a/internal/config/document_configkit_test.go b/internal/config/document_configkit_test.go new file mode 100644 index 0000000..2e64291 --- /dev/null +++ b/internal/config/document_configkit_test.go @@ -0,0 +1,56 @@ +package config + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSingleSourceDocumentsRejectConfigKitDirectives(t *testing.T) { + dir := t.TempDir() + campaignPath := filepath.Join(dir, "campaign.yml") + if err := os.WriteFile(campaignPath, []byte("configkit:\n imports: []\ncampaign_id: sample\n"), 0o644); err != nil { + t.Fatal(err) + } + tests := []struct { + name string + load func() error + }{ + {name: "campaign", load: func() error { _, err := LoadCampaign(campaignPath); return err }}, + {name: "session", load: func() error { + _, err := LoadSessionBytesWithOptions("session.yml", []byte("configkit:\n imports: []\nsession_id: sample\n"), SessionLoadOptions{}) + return err + }}, + {name: "party", load: func() error { + _, err := ParseParty([]byte("configkit:\n imports: []\nschema_version: narratio.party.v1\ncharacters: {}\n")) + return err + }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if err := test.load(); err == nil || !strings.Contains(err.Error(), "import.disabled") { + t.Fatalf("load error = %v, want disabled-import diagnostic", err) + } + }) + } +} + +func TestInspectedDocumentsEnforceConfigKitSizeLimit(t *testing.T) { + tooLarge := bytes.Repeat([]byte{' '}, int(maxConfigDocumentBytes)+1) + if _, err := LoadSessionBytesWithOptions("session.yml", tooLarge, SessionLoadOptions{}); err == nil || !strings.Contains(err.Error(), "exceeds") { + t.Fatalf("oversized session error = %v", err) + } + if _, err := ParseParty(tooLarge); err == nil || !strings.Contains(err.Error(), "exceeds") { + t.Fatalf("oversized party error = %v", err) + } + + path := filepath.Join(t.TempDir(), "session.yml") + if err := os.WriteFile(path, tooLarge, 0o644); err != nil { + t.Fatal(err) + } + if _, err := LoadSession(path); err == nil || !strings.Contains(err.Error(), "limit") { + t.Fatalf("oversized local session error = %v", err) + } +} diff --git a/internal/config/load.go b/internal/config/load.go index 6be6eaf..872344c 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -2,14 +2,12 @@ package config import ( "fmt" - "io" - "os" "path/filepath" "regexp" "sort" "strings" - "gopkg.in/yaml.v3" + "gitea.maximumdirect.net/eric/narratio/internal/fileops" ) // LoadPipeline loads pipeline configuration from a YAML file with strict field checking. @@ -65,11 +63,7 @@ func finalizeLoadedPipeline(path string, cfg *PipelineConfig) (*PipelineConfig, // LoadCampaign loads campaign configuration from a YAML file with strict field checking. func LoadCampaign(path string) (*CampaignConfig, error) { - var cfg CampaignConfig - if err := decodeStrictYAML("campaign", path, &cfg); err != nil { - return nil, fmt.Errorf("load campaign config: %w", err) - } - return &cfg, nil + return loadCampaignWithConfigKit(path) } // LoadSession loads session configuration from a YAML file with strict field checking. @@ -87,7 +81,7 @@ type SessionLoadOptions struct { // LoadSessionWithOptions loads session configuration from a YAML file with // strict field checking. func LoadSessionWithOptions(path string, opts SessionLoadOptions) (*SessionConfig, error) { - sessionBytes, err := os.ReadFile(path) + sessionBytes, err := fileops.ReadFileBounded(path, maxConfigDocumentBytes) if err != nil { return nil, fmt.Errorf("load session config: session file %q: open: %w", path, err) } @@ -101,9 +95,9 @@ func LoadSessionBytesWithOptions(label string, data []byte, opts SessionLoadOpti return nil, fmt.Errorf("load session config: %w", err) } - var cfg SessionConfig - if err := decodeStrictYAMLFromReader("session", label, strings.NewReader(string(data)), &cfg); err != nil { - return nil, fmt.Errorf("load session config: %w", err) + cfg, err := loadSessionWithConfigKit(label, data) + if err != nil { + return nil, err } if strings.TrimSpace(opts.SessionID) != "" && strings.TrimSpace(cfg.SessionID) != "" && strings.TrimSpace(cfg.SessionID) != strings.TrimSpace(opts.SessionID) { return nil, fmt.Errorf( @@ -131,7 +125,7 @@ func LoadSessionBytesWithOptions(label string, data []byte, opts SessionLoadOpti ) } } - return &cfg, nil + return cfg, nil } // Load loads and resolves combined pipeline, campaign, and session configuration. @@ -394,33 +388,6 @@ func selectStableInput(campaignValue, sessionValue, campaignPath, sessionPath st } } -func decodeStrictYAML(kind, path string, out any) error { - f, err := os.Open(path) - if err != nil { - return fmt.Errorf("%s file %q: open: %w", kind, path, err) - } - defer f.Close() - - return decodeStrictYAMLFromReader(kind, path, f, out) -} - -func decodeStrictYAMLFromReader(kind, path string, r io.Reader, out any) error { - dec := yaml.NewDecoder(r) - dec.KnownFields(true) - if err := dec.Decode(out); err != nil { - return fmt.Errorf("%s file %q: strict decode failed: %w", kind, path, err) - } - - var extra yaml.Node - if err := dec.Decode(&extra); err == nil { - return fmt.Errorf("%s file %q: must contain exactly one YAML document", kind, path) - } else if err != io.EOF { - return fmt.Errorf("%s file %q: trailing content decode failed: %w", kind, path, err) - } - - return nil -} - var sessionTemplatePlaceholderPattern = regexp.MustCompile(`\{\{[^}]*\}\}`) var sessionTemplateVariablePattern = regexp.MustCompile(`\{\{\s*([a-zA-Z_][a-zA-Z0-9_]*)\s*\}\}`) diff --git a/internal/config/loader_strict_test.go b/internal/config/loader_strict_test.go index 2f4d0b2..f156da7 100644 --- a/internal/config/loader_strict_test.go +++ b/internal/config/loader_strict_test.go @@ -54,31 +54,3 @@ func TestLoadersRejectTrailingYAMLDocuments(t *testing.T) { }) } } - -func TestStrictYAMLRejectsParserExposedEmptyTrailingDocument(t *testing.T) { - var target struct { - Name string `yaml:"name"` - } - err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("name: one\n---\n"), &target) - if err == nil { - t.Fatal("decodeStrictYAMLFromReader() error = nil, want trailing-document error") - } - if !strings.Contains(err.Error(), "exactly one YAML document") { - t.Fatalf("decodeStrictYAMLFromReader() error = %v, want exactly-one-document error", err) - } -} - -func TestStrictYAMLAcceptsSingleDocumentAndRejectsUnknownFields(t *testing.T) { - var target struct { - Name string `yaml:"name"` - } - if err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("name: one\n"), &target); err != nil { - t.Fatalf("decodeStrictYAMLFromReader(single document) error = %v", err) - } - if target.Name != "one" { - t.Fatalf("Name = %q, want one", target.Name) - } - if err := decodeStrictYAMLFromReader("test", "memory", strings.NewReader("unknown: one\n"), &target); err == nil || !strings.Contains(err.Error(), "strict decode failed") { - t.Fatalf("decodeStrictYAMLFromReader(unknown field) error = %v, want strict decode error", err) - } -} diff --git a/internal/config/party.go b/internal/config/party.go index 4d5e867..b3975cb 100644 --- a/internal/config/party.go +++ b/internal/config/party.go @@ -3,12 +3,12 @@ package config import ( "bytes" "fmt" - "io" + "math" "sort" - "strconv" "strings" "unicode" + configkit "gitea.maximumdirect.net/eric/ConfigKit" "gitea.maximumdirect.net/eric/narratio/internal/artifactpolicy" "gopkg.in/yaml.v3" ) @@ -54,264 +54,141 @@ type PartyClass struct { Level *int } -// ParseParty strictly parses one supported party document. +type partyDocument struct { + SchemaVersion string `yaml:"schema_version"` + Characters map[string]partyCharacter `yaml:"characters"` +} + +type partyCharacter struct { + Player partyPlayer `yaml:"player"` + Character partyCharacterDetails `yaml:"character"` +} + +type partyPlayer struct { + Name string `yaml:"name"` +} + +type partyCharacterDetails struct { + Name string `yaml:"name"` + Aliases []string `yaml:"alias"` + Classes []partyClass `yaml:"classes"` +} + +type partyClass struct { + Name string `yaml:"name"` + Level *int `yaml:"level"` +} + +type partyDomainError struct { + path configkit.Path + message string +} + +// ParseParty strictly parses one supported party document through ConfigKit. func ParseParty(data []byte) (*CanonicalParty, error) { - root, decoder, err := parsePartyFirstDocument(data) + if int64(len(data)) > maxConfigDocumentBytes { + return nil, fmt.Errorf("parse party config: party document exceeds %d-byte limit", maxConfigDocumentBytes) + } + definition := configkit.Definition[partyDocument]{ + Version: &configkit.VersionRule{ + Path: configkit.Keys("schema_version"), + Check: validatePartyVersion, + }, + Validate: validatePartyDocument, + } + result, err := configkit.Load(configkit.Bytes("party.yml", data), configkit.Options[partyDocument]{Definition: definition}) if err != nil { - return nil, err + return nil, describeConfigKitLoadError("party", err) } - if err := requireNoTrailingPartyDocuments(decoder); err != nil { - return nil, err - } - party, err := parseCanonicalParty(root, data) - if err != nil { - return nil, err - } - return party, nil + return canonicalPartyFromResult(result, data), nil } -func parsePartyFirstDocument(data []byte) (*yaml.Node, *yaml.Decoder, error) { - decoder := yaml.NewDecoder(bytes.NewReader(data)) - var document yaml.Node - if err := decoder.Decode(&document); err != nil { - if err == io.EOF { - return nil, nil, fmt.Errorf("party YAML document is empty") - } - return nil, nil, fmt.Errorf("decode party YAML: %w", err) - } - if document.Kind != yaml.DocumentNode || len(document.Content) != 1 { - return nil, nil, fmt.Errorf("party must contain one YAML document") - } - return document.Content[0], decoder, nil -} - -func requireNoTrailingPartyDocuments(decoder *yaml.Decoder) error { - var trailing yaml.Node - if err := decoder.Decode(&trailing); err == nil { - return fmt.Errorf("party must contain exactly one YAML document") - } else if err != io.EOF { - return fmt.Errorf("decode trailing party YAML: %w", err) +func validatePartyVersion(version configkit.Version, validation *configkit.Validation) error { + path := configkit.Keys("schema_version") + if !version.Present || version.Null || version.Text == "" { + validation.Error(path, fmt.Sprintf("schema_version is required; supported schema_version is %q", PartySchemaVersion)) + } else if version.Text != PartySchemaVersion { + validation.Error(path, fmt.Sprintf("schema_version is unsupported; supported schema_version is %q", PartySchemaVersion)) } return nil } -func parseCanonicalParty(root *yaml.Node, raw []byte) (*CanonicalParty, error) { - fields, err := partyMappingFields(root, "party") - if err != nil { - return nil, err +func validatePartyDocument(document *partyDocument, validation *configkit.Validation) { + if problem := firstPartyDomainError(document); problem != nil { + validation.Error(problem.path, problem.message) } +} - version, ok := fields["schema_version"] - if !ok { - return nil, fmt.Errorf("party.schema_version is required; supported schema_version is %q", PartySchemaVersion) +func firstPartyDomainError(document *partyDocument) *partyDomainError { + charactersPath := configkit.Keys("characters") + if document == nil || len(document.Characters) == 0 { + return &partyDomainError{path: charactersPath, message: "characters must be non-empty"} } - schemaVersion, err := partyRequiredString(map[string]*yaml.Node{"schema_version": version}, "schema_version", "party") - if err != nil { - return nil, fmt.Errorf("party.schema_version must be a non-empty string; supported schema_version is %q", PartySchemaVersion) + ids := make([]string, 0, len(document.Characters)) + for id := range document.Characters { + ids = append(ids, id) } - if schemaVersion != PartySchemaVersion { - return nil, fmt.Errorf("party.schema_version %q is unsupported; supported schema_version is %q", schemaVersion, PartySchemaVersion) - } - if err := partyKnownFields(fields, "party", "schema_version", "characters"); err != nil { - return nil, err - } - charactersNode, ok := fields["characters"] - if !ok { - return nil, fmt.Errorf("party.characters is required") - } - charactersFields, err := partyMappingFields(charactersNode, "party.characters") - if err != nil { - return nil, err - } - if len(charactersFields) == 0 { - return nil, fmt.Errorf("party.characters must be non-empty") - } - - party := &CanonicalParty{Raw: append([]byte(nil), raw...)} - seenNames := make([]string, 0, len(charactersFields)) - for _, id := range partyMappingOrder(charactersNode) { - entry, err := parsePartyCharacter(id, charactersFields[id], seenNames) - if err != nil { - return nil, err + sort.Strings(ids) + seenNames := make([]string, 0, len(ids)*2) + for _, id := range ids { + path := charactersPath.Key(id) + character := document.Characters[id] + if id != strings.TrimSpace(id) || !artifactpolicy.IsConfiguredKey(id) { + return &partyDomainError{path: path, message: "invalid character id"} } - party.Characters = append(party.Characters, entry) - seenNames = append(seenNames, entry.Character.Name) - seenNames = append(seenNames, entry.Character.Aliases...) - } - return party, nil -} - -func parsePartyCharacter(id string, node *yaml.Node, seenNames []string) (PartyCharacter, error) { - path := "party.characters." + id - if id != strings.TrimSpace(id) || !artifactpolicy.IsConfiguredKey(id) { - return PartyCharacter{}, fmt.Errorf("%s has invalid character id %q", path, id) - } - fields, err := partyMappingFields(node, path) - if err != nil { - return PartyCharacter{}, err - } - if err := partyKnownFields(fields, path, "player", "character"); err != nil { - return PartyCharacter{}, err - } - player, err := parsePartyPlayer(fields["player"], path+".player") - if err != nil { - return PartyCharacter{}, err - } - character, err := parsePartyCharacterDetails(fields["character"], path+".character") - if err != nil { - return PartyCharacter{}, err - } - if partyNameAmbiguous(character.Name, seenNames) { - return PartyCharacter{}, fmt.Errorf("%s name or alias %q is ambiguous", path, character.Name) - } - visibleNames := append(append([]string(nil), seenNames...), character.Name) - for _, name := range character.Aliases { - if partyNameAmbiguous(name, visibleNames) { - return PartyCharacter{}, fmt.Errorf("%s name or alias %q is ambiguous", path, name) + if err := validatePartyDisplayName(character.Player.Name); err != "" { + return &partyDomainError{path: path.Key("player").Key("name"), message: err} } - visibleNames = append(visibleNames, name) - } - return PartyCharacter{ID: id, Player: player, Character: character}, nil -} - -func parsePartyPlayer(node *yaml.Node, path string) (PartyPlayer, error) { - fields, err := partyMappingFields(node, path) - if err != nil { - return PartyPlayer{}, err - } - if err := partyKnownFields(fields, path, "name"); err != nil { - return PartyPlayer{}, err - } - name, err := partyRequiredDisplayString(fields, "name", path) - if err != nil { - return PartyPlayer{}, err - } - return PartyPlayer{Name: name}, nil -} - -func parsePartyCharacterDetails(node *yaml.Node, path string) (PartyCharacterDetails, error) { - fields, err := partyMappingFields(node, path) - if err != nil { - return PartyCharacterDetails{}, err - } - if err := partyKnownFields(fields, path, "name", "alias", "classes"); err != nil { - return PartyCharacterDetails{}, err - } - name, err := partyRequiredDisplayString(fields, "name", path) - if err != nil { - return PartyCharacterDetails{}, err - } - aliases, err := partyAliases(fields["alias"], path+".alias") - if err != nil { - return PartyCharacterDetails{}, err - } - classes, err := partyClasses(fields["classes"], path+".classes") - if err != nil { - return PartyCharacterDetails{}, err - } - return PartyCharacterDetails{Name: name, Aliases: aliases, Classes: classes}, nil -} - -func partyAliases(node *yaml.Node, path string) ([]string, error) { - if node == nil { - return nil, nil - } - if node.Kind != yaml.SequenceNode { - return nil, fmt.Errorf("%s must be a list", path) - } - aliases := make([]string, 0, len(node.Content)) - for index, item := range node.Content { - alias, err := partyDisplayString(item, fmt.Sprintf("%s[%d]", path, index)) - if err != nil { - return nil, err + if err := validatePartyDisplayName(character.Character.Name); err != "" { + return &partyDomainError{path: path.Key("character").Key("name"), message: err} } - aliases = append(aliases, alias) + if partyNameAmbiguous(character.Character.Name, seenNames) { + return &partyDomainError{path: path.Key("character").Key("name"), message: "name or alias is ambiguous"} + } + visibleNames := append(append([]string(nil), seenNames...), character.Character.Name) + for index, alias := range character.Character.Aliases { + aliasPath := path.Key("character").Key("alias").Index(index) + if err := validatePartyDisplayName(alias); err != "" { + return &partyDomainError{path: aliasPath, message: err} + } + if partyNameAmbiguous(alias, visibleNames) { + return &partyDomainError{path: aliasPath, message: "name or alias is ambiguous"} + } + visibleNames = append(visibleNames, alias) + } + classesPath := path.Key("character").Key("classes") + if len(character.Character.Classes) == 0 { + return &partyDomainError{path: classesPath, message: "classes must be a non-empty list"} + } + classNames := make([]string, 0, len(character.Character.Classes)) + for index, class := range character.Character.Classes { + classPath := classesPath.Index(index) + if err := validatePartyDisplayName(class.Name); err != "" { + return &partyDomainError{path: classPath.Key("name"), message: err} + } + if partyNameAmbiguous(class.Name, classNames) { + return &partyDomainError{path: classPath.Key("name"), message: "duplicate class"} + } + classNames = append(classNames, class.Name) + if class.Level != nil && *class.Level <= 0 { + return &partyDomainError{path: classPath.Key("level"), message: "level must be a positive integer"} + } + } + seenNames = visibleNames } - return aliases, nil + return nil } -func partyClasses(node *yaml.Node, path string) ([]PartyClass, error) { - if node == nil { - return nil, fmt.Errorf("%s is required", path) - } - if node.Kind != yaml.SequenceNode || len(node.Content) == 0 { - return nil, fmt.Errorf("%s must be a non-empty list", path) - } - classes := make([]PartyClass, 0, len(node.Content)) - classNames := make([]string, 0, len(node.Content)) - for index, item := range node.Content { - classPath := fmt.Sprintf("%s[%d]", path, index) - fields, err := partyMappingFields(item, classPath) - if err != nil { - return nil, err - } - if err := partyKnownFields(fields, classPath, "name", "level"); err != nil { - return nil, err - } - name, err := partyRequiredDisplayString(fields, "name", classPath) - if err != nil { - return nil, err - } - if partyNameAmbiguous(name, classNames) { - return nil, fmt.Errorf("%s has duplicate class %q", path, name) - } - classNames = append(classNames, name) - level, err := partyLevel(fields["level"], classPath+".level") - if err != nil { - return nil, err - } - classes = append(classes, PartyClass{Name: name, Level: level}) - } - return classes, nil -} - -func partyLevel(node *yaml.Node, path string) (*int, error) { - if node == nil { - return nil, nil - } - if node.Kind != yaml.ScalarNode || node.Tag != "!!int" { - return nil, fmt.Errorf("%s must be a positive integer", path) - } - level, err := strconv.Atoi(node.Value) - if err != nil || level <= 0 { - return nil, fmt.Errorf("%s must be a positive integer", path) - } - return &level, nil -} - -func partyRequiredDisplayString(fields map[string]*yaml.Node, key, path string) (string, error) { - node, ok := fields[key] - if !ok { - return "", fmt.Errorf("%s.%s is required", path, key) - } - return partyDisplayString(node, path+"."+key) -} - -func partyRequiredString(fields map[string]*yaml.Node, key, path string) (string, error) { - node, ok := fields[key] - if !ok { - return "", fmt.Errorf("%s.%s is required", path, key) - } - if node.Kind != yaml.ScalarNode || node.Tag != "!!str" || node.Value == "" { - return "", fmt.Errorf("%s.%s must be a non-empty string", path, key) - } - return node.Value, nil -} - -func partyDisplayString(node *yaml.Node, path string) (string, error) { - if node == nil || node.Kind != yaml.ScalarNode || node.Tag != "!!str" { - return "", fmt.Errorf("%s must be a display string", path) - } - value := node.Value +func validatePartyDisplayName(value string) string { if value == "" || strings.TrimSpace(value) != value { - return "", fmt.Errorf("%s must be non-empty and trimmed", path) + return "display name must be non-empty and trimmed" } for _, runeValue := range value { if unicode.IsControl(runeValue) { - return "", fmt.Errorf("%s must not contain control characters", path) + return "display name must not contain control characters" } } - return value, nil + return "" } func partyNameAmbiguous(name string, existing []string) bool { @@ -323,70 +200,45 @@ func partyNameAmbiguous(name string, existing []string) bool { return false } -func partyMappingFields(node *yaml.Node, path string) (map[string]*yaml.Node, error) { - if node == nil || node.Kind != yaml.MappingNode { - return nil, fmt.Errorf("%s must be a mapping", path) +func canonicalPartyFromResult(result *configkit.Result[partyDocument], raw []byte) *CanonicalParty { + type orderedCharacter struct { + id string + location configkit.Location } - if len(node.Content)%2 != 0 { - return nil, fmt.Errorf("%s has an incomplete mapping", path) - } - fields := make(map[string]*yaml.Node, len(node.Content)/2) - for index := 0; index < len(node.Content); index += 2 { - key, value := node.Content[index], node.Content[index+1] - if key.Kind != yaml.ScalarNode || key.Tag != "!!str" { - return nil, fmt.Errorf("%s has a non-string field name", path) + metadata := result.Metadata() + ordered := make([]orderedCharacter, 0, len(result.Config.Characters)) + for id := range result.Config.Characters { + entry := orderedCharacter{id: id, location: configkit.Location{Line: math.MaxInt}} + if info, ok := metadata.Lookup(configkit.Keys("characters", id)); ok && len(info.Origin.Locations) != 0 { + entry.location = info.Origin.Locations[0] } - if key.Value == "" { - return nil, fmt.Errorf("%s has an empty field name", path) - } - if _, duplicate := fields[key.Value]; duplicate { - return nil, fmt.Errorf("%s has duplicate field %q", path, key.Value) - } - if partyContainsAlias(value) { - return nil, fmt.Errorf("%s.%s must not use YAML aliases", path, key.Value) - } - fields[key.Value] = value + ordered = append(ordered, entry) } - return fields, nil -} - -func partyMappingOrder(node *yaml.Node) []string { - keys := make([]string, 0, len(node.Content)/2) - for index := 0; index < len(node.Content); index += 2 { - keys = append(keys, node.Content[index].Value) - } - return keys -} - -func partyKnownFields(fields map[string]*yaml.Node, path string, allowed ...string) error { - for name := range fields { - known := false - for _, candidate := range allowed { - if name == candidate { - known = true - break - } + sort.Slice(ordered, func(left, right int) bool { + if ordered[left].location.Line != ordered[right].location.Line { + return ordered[left].location.Line < ordered[right].location.Line } - if !known { - return fmt.Errorf("%s has unknown field %q", path, name) + if ordered[left].location.Column != ordered[right].location.Column { + return ordered[left].location.Column < ordered[right].location.Column } - } - return nil -} - -func partyContainsAlias(node *yaml.Node) bool { - if node == nil { - return false - } - if node.Kind == yaml.AliasNode { - return true - } - for _, child := range node.Content { - if partyContainsAlias(child) { - return true + return ordered[left].id < ordered[right].id + }) + party := &CanonicalParty{Raw: bytes.Clone(raw), Characters: make([]PartyCharacter, 0, len(ordered))} + for _, orderedCharacter := range ordered { + input := result.Config.Characters[orderedCharacter.id] + classes := make([]PartyClass, len(input.Character.Classes)) + for index, class := range input.Character.Classes { + classes[index] = PartyClass{Name: class.Name, Level: class.Level} } + party.Characters = append(party.Characters, PartyCharacter{ + ID: orderedCharacter.id, + Player: PartyPlayer{Name: input.Player.Name}, + Character: PartyCharacterDetails{ + Name: input.Character.Name, Aliases: append([]string(nil), input.Character.Aliases...), Classes: classes, + }, + }) } - return false + return party } // ClassSummary preserves the declaration order required by the party contract. @@ -429,16 +281,12 @@ func (p *CanonicalParty) PlayersYAML() ([]byte, error) { projection := struct { SchemaVersion string `yaml:"schema_version"` Players []projectionPlayer `yaml:"players"` - }{ - SchemaVersion: PlayersSchemaVersion, - Players: make([]projectionPlayer, 0, len(characters)), - } + }{SchemaVersion: PlayersSchemaVersion, Players: make([]projectionPlayer, 0, len(characters))} for _, character := range characters { projection.Players = append(projection.Players, projectionPlayer{ Name: character.Player.Name, Character: projectionCharacter{ - ID: character.ID, - Name: character.Character.Name, + ID: character.ID, Name: character.Character.Name, Aliases: append([]string(nil), character.Character.Aliases...), }, }) diff --git a/internal/config/party_resolution.go b/internal/config/party_resolution.go index 8781e46..fae7055 100644 --- a/internal/config/party_resolution.go +++ b/internal/config/party_resolution.go @@ -8,8 +8,6 @@ import ( "gitea.maximumdirect.net/eric/narratio/internal/fileops" ) -const maxPartyDocumentBytes = 8 << 20 - func resolvePartyInput(input ResolvedInputFile) (ResolvedParty, error) { configuredPath := strings.TrimSpace(input.Path) if configuredPath == "" { @@ -24,7 +22,7 @@ func resolvePartyInput(input ResolvedInputFile) (ResolvedParty, error) { path = filepath.Join(filepath.Dir(configPath), path) } path = filepath.Clean(path) - raw, err := fileops.ReadFileBounded(path, maxPartyDocumentBytes) + raw, err := fileops.ReadFileBounded(path, maxConfigDocumentBytes) if err != nil { return ResolvedParty{}, fmt.Errorf("read party input %q: %w", path, err) } diff --git a/internal/config/party_resolution_test.go b/internal/config/party_resolution_test.go index 04ee0c4..5df3a98 100644 --- a/internal/config/party_resolution_test.go +++ b/internal/config/party_resolution_test.go @@ -65,9 +65,9 @@ func TestCanonicalPartyRejectsSeparatePlayersAndSessionPartyOverrides(t *testing sessionExtra string want string }{ - {name: "campaign players", campaignExtra: " players_file: players.yml\n", want: "field players_file not found"}, - {name: "session players", sessionExtra: " players_file: players.yml\n", want: "field players_file not found"}, - {name: "session party", sessionExtra: " party_file: party-override.yml\n", want: "field party_file not found"}, + {name: "campaign players", campaignExtra: " players_file: players.yml\n", want: "/inputs/players_file"}, + {name: "session players", sessionExtra: " players_file: players.yml\n", want: "/inputs/players_file"}, + {name: "session party", sessionExtra: " party_file: party-override.yml\n", want: "/inputs/party_file"}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/internal/config/party_test.go b/internal/config/party_test.go index 8ad2c43..fb64cae 100644 --- a/internal/config/party_test.go +++ b/internal/config/party_test.go @@ -60,26 +60,26 @@ characters: want string }{ {name: "unsupported version", yaml: strings.Replace(valid, PartySchemaVersion, "narratio.party.v2", 1), want: "unsupported"}, - {name: "non-string version", yaml: strings.Replace(valid, PartySchemaVersion, "1", 1), want: "must be a non-empty string"}, - {name: "unknown top field", yaml: valid + "unknown: true\n", want: "unknown field"}, - {name: "unknown nested field", yaml: strings.Replace(valid, " name: Eric\n", " name: Eric\n role: GM\n", 1), want: "unknown field"}, - {name: "trailing document", yaml: valid + "---\ncharacters: {}\n", want: "exactly one YAML document"}, + {name: "non-string version", yaml: strings.Replace(valid, PartySchemaVersion, "1", 1), want: "version.type"}, + {name: "unknown top field", yaml: valid + "unknown: true\n", want: "decode.unknown_field"}, + {name: "unknown nested field", yaml: strings.Replace(valid, " name: Eric\n", " name: Eric\n role: GM\n", 1), want: "decode.unknown_field"}, + {name: "trailing document", yaml: valid + "---\ncharacters: {}\n", want: "yaml.multiple_documents"}, {name: "empty characters", yaml: "schema_version: narratio.party.v1\ncharacters: {}\n", want: "must be non-empty"}, {name: "invalid id", yaml: strings.Replace(valid, " arannis:", " bad-id:", 1), want: "invalid character id"}, {name: "whitespace id", yaml: strings.Replace(valid, " arannis:", " ' arannis':", 1), want: "invalid character id"}, - {name: "missing player name", yaml: strings.Replace(valid, "player:\n name: Eric", "player: {}", 1), want: "player.name is required"}, - {name: "missing character name", yaml: strings.Replace(valid, " name: Arannis\n", "", 1), want: "character.name is required"}, + {name: "missing player name", yaml: strings.Replace(valid, "player:\n name: Eric", "player: {}", 1), want: "/characters/arannis/player/name"}, + {name: "missing character name", yaml: strings.Replace(valid, " name: Arannis\n", "", 1), want: "/characters/arannis/character/name"}, {name: "blank player name", yaml: strings.Replace(valid, "name: Eric", "name: ' Eric'", 1), want: "non-empty and trimmed"}, {name: "control character", yaml: strings.Replace(valid, "name: Arannis", "name: \"Ara\\tnnis\"", 1), want: "control characters"}, - {name: "non-string display", yaml: strings.Replace(valid, "name: Eric", "name: 42", 1), want: "display string"}, - {name: "missing classes", yaml: strings.Replace(valid, " classes:\n - name: wizard\n level: 8\n", "", 1), want: "classes is required"}, + {name: "non-string display", yaml: strings.Replace(valid, "name: Eric", "name: 42", 1), want: "decode.type"}, + {name: "missing classes", yaml: strings.Replace(valid, " classes:\n - name: wizard\n level: 8\n", "", 1), want: "classes must be a non-empty list"}, {name: "empty classes", yaml: strings.Replace(valid, " classes:\n - name: wizard\n level: 8\n", " classes: []\n", 1), want: "non-empty list"}, {name: "zero level", yaml: strings.Replace(valid, "level: 8", "level: 0", 1), want: "positive integer"}, {name: "negative level", yaml: strings.Replace(valid, "level: 8", "level: -2", 1), want: "positive integer"}, - {name: "non-integer level", yaml: strings.Replace(valid, "level: 8", "level: eight", 1), want: "positive integer"}, + {name: "non-integer level", yaml: strings.Replace(valid, "level: 8", "level: eight", 1), want: "decode.type"}, {name: "duplicate class", yaml: strings.Replace(valid, " level: 8\n", " level: 8\n - name: WIZARD\n", 1), want: "duplicate class"}, - {name: "duplicate field", yaml: strings.Replace(valid, " name: Eric\n", " name: Eric\n name: Erin\n", 1), want: "duplicate field"}, - {name: "yaml alias", yaml: strings.Replace(strings.Replace(valid, " name: Eric", " name: &player Eric", 1), " name: Arannis", " name: *player", 1), want: "must not use YAML aliases"}, + {name: "duplicate field", yaml: strings.Replace(valid, " name: Eric\n", " name: Eric\n name: Erin\n", 1), want: "yaml.duplicate_key"}, + {name: "yaml alias", yaml: strings.Replace(strings.Replace(valid, " name: Eric", " name: &player Eric", 1), " name: Arannis", " name: *player", 1), want: "yaml.anchor"}, } for _, test := range tests { @@ -242,6 +242,9 @@ characters: if err != nil { t.Fatalf("ParseParty() error = %v", err) } + if len(document.Characters) != 2 || document.Characters[0].ID != "zeta" || document.Characters[1].ID != "alpha" { + t.Fatalf("authored character order = %#v, want zeta then alpha", document.Characters) + } got, err := document.PlayersYAML() if err != nil { t.Fatalf("PlayersYAML() error = %v", err) diff --git a/internal/config/pipeline_configkit.go b/internal/config/pipeline_configkit.go index d688af0..0a8839b 100644 --- a/internal/config/pipeline_configkit.go +++ b/internal/config/pipeline_configkit.go @@ -65,9 +65,13 @@ func loadPipelineWithConfigKit(path string, opts PipelineLoadOptions) (*Pipeline } func describePipelineLoadError(err error) error { + return describeConfigKitLoadError("pipeline", err) +} + +func describeConfigKitLoadError(kind string, err error) error { var loadError *configkit.LoadError if !errors.As(err, &loadError) { - return fmt.Errorf("load pipeline config: %w", err) + return fmt.Errorf("load %s config: %w", kind, err) } var diagnostics bytes.Buffer resolved := loadError.Diagnostics() @@ -92,9 +96,9 @@ func describePipelineLoadError(err error) error { } } if writeErr := configkit.WriteDiagnostics(&diagnostics, resolved); writeErr != nil { - return fmt.Errorf("load pipeline config: %w", err) + return fmt.Errorf("load %s config: %w", kind, err) } - return fmt.Errorf("load pipeline config: strict decode failed or validation failed: %w: %s", err, strings.TrimSpace(diagnostics.String())) + return fmt.Errorf("load %s config: strict decode failed or validation failed: %w: %s", kind, err, strings.TrimSpace(diagnostics.String())) } func selectedPipelineProfileName(opts PipelineLoadOptions) string {