From 027e2fc9e6d91947ad0f640bb8c2d25aeaea9070 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 14:55:21 -0500 Subject: [PATCH 01/10] feat(cli): add file collection and LoadPlugin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CollectFiles walks an input file or directory, filters by accepted extensions, enforces a max-size limit, and returns a map of relative-path keys to file content (forward slashes on all platforms). Single-file inputs bypass the extension filter — the user explicitly chose the file, so the plugin's Accepts list is not applied. Empty accepts means all files are included (used by from_osi which has no Accepts field). LoadPlugin exports the existing unexported loadPlugin, used when --plugin bypasses name-based discovery. --- cli/internal/plugin/collect.go | 87 ++++++++++++++++++++ cli/internal/plugin/collect_test.go | 122 ++++++++++++++++++++++++++++ cli/internal/plugin/discover.go | 7 ++ 3 files changed, 216 insertions(+) create mode 100644 cli/internal/plugin/collect.go create mode 100644 cli/internal/plugin/collect_test.go diff --git a/cli/internal/plugin/collect.go b/cli/internal/plugin/collect.go new file mode 100644 index 0000000..a582503 --- /dev/null +++ b/cli/internal/plugin/collect.go @@ -0,0 +1,87 @@ +package plugin + +import ( + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" +) + +// CollectFiles builds the files map for a plugin Request. +// +// If input is a regular file, it is read unconditionally (no extension filter) +// and keyed by its bare filename. The caller explicitly chose the file, so the +// plugin's Accepts list is not applied. +// +// If input is a directory, it is walked recursively. Only files whose extension +// (case-insensitive) appears in accepts are included. If accepts is empty, all +// files are included. Keys are forward-slash relative paths from the directory root. +// +// maxBytes enforces a cumulative size limit. Pass maxBytes <= 0 to disable it. +func CollectFiles(input string, accepts []string, maxBytes int64) (map[string]string, error) { + info, err := os.Stat(input) + if err != nil { + return nil, fmt.Errorf("cannot access input path %q: %w", input, err) + } + + if !info.IsDir() { + data, err := os.ReadFile(input) + if err != nil { + return nil, fmt.Errorf("cannot read input file %q: %w", input, err) + } + if maxBytes > 0 && int64(len(data)) > maxBytes { + return nil, fmt.Errorf("total input size exceeds limit (%dMB). Use --max-input-size to override", + maxBytes/(1024*1024)) + } + return map[string]string{filepath.Base(input): string(data)}, nil + } + + // Build a lowercase set for O(1) extension lookup. + acceptSet := make(map[string]struct{}, len(accepts)) + for _, ext := range accepts { + acceptSet[strings.ToLower(ext)] = struct{}{} + } + + files := make(map[string]string) + var total int64 + + err = filepath.WalkDir(input, func(path string, d fs.DirEntry, werr error) error { + if werr != nil { + return werr + } + if d.IsDir() { + return nil + } + if len(acceptSet) > 0 { + ext := strings.ToLower(filepath.Ext(path)) + if _, ok := acceptSet[ext]; !ok { + return nil + } + } + + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("cannot read %q: %w", path, err) + } + + if maxBytes > 0 { + total += int64(len(data)) + if total > maxBytes { + return fmt.Errorf("total input size exceeds limit (%dMB). Use --max-input-size to override", + maxBytes/(1024*1024)) + } + } + + rel, err := filepath.Rel(input, path) + if err != nil { + return err + } + files[filepath.ToSlash(rel)] = string(data) + return nil + }) + if err != nil { + return nil, err + } + return files, nil +} diff --git a/cli/internal/plugin/collect_test.go b/cli/internal/plugin/collect_test.go new file mode 100644 index 0000000..46f71be --- /dev/null +++ b/cli/internal/plugin/collect_test.go @@ -0,0 +1,122 @@ +package plugin_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/open-semantic-interchange/ossie/cli/internal/plugin" +) + +// writeFile is a test helper that creates a file with the given content. +// Distinct from writePlugin in discover_test.go which creates a full plugin dir. +func writeFile(t *testing.T, dir, name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil { + t.Fatalf("could not write %s: %v", name, err) + } +} + +func TestCollectFiles_singleFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "model.yaml") + if err := os.WriteFile(path, []byte("content"), 0644); err != nil { + t.Fatal(err) + } + + // Single file: extension filter (.json) does NOT apply. + files, err := plugin.CollectFiles(path, []string{".json"}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file, got %d", len(files)) + } + if v, ok := files["model.yaml"]; !ok || v != "content" { + t.Errorf("unexpected files map: %v", files) + } +} + +func TestCollectFiles_directory(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "a.yaml", "a") + writeFile(t, dir, "b.yaml", "b") + + files, err := plugin.CollectFiles(dir, []string{".yaml"}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 2 { + t.Errorf("expected 2 files, got %d: %v", len(files), files) + } +} + +func TestCollectFiles_directoryFiltered(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "model.yaml", "yaml-content") + writeFile(t, dir, "readme.md", "md-content") + + files, err := plugin.CollectFiles(dir, []string{".yaml"}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 1 { + t.Fatalf("expected 1 file after filter, got %d: %v", len(files), files) + } + if _, ok := files["model.yaml"]; !ok { + t.Errorf("expected model.yaml in files, got: %v", files) + } +} + +func TestCollectFiles_forwardSlashKeys(t *testing.T) { + dir := t.TempDir() + sub := filepath.Join(dir, "subdir") + if err := os.MkdirAll(sub, 0755); err != nil { + t.Fatal(err) + } + writeFile(t, sub, "model.yaml", "content") + + files, err := plugin.CollectFiles(dir, []string{".yaml"}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + for k := range files { + for _, ch := range k { + if ch == '\\' { + t.Errorf("key contains backslash: %q", k) + } + } + if k != "subdir/model.yaml" { + t.Errorf("expected key %q, got %q", "subdir/model.yaml", k) + } + } +} + +func TestCollectFiles_exceedsMaxSize(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "big.yaml", "12345") // 5 bytes + + _, err := plugin.CollectFiles(dir, []string{".yaml"}, 4) // limit 4 bytes + if err == nil { + t.Fatal("expected error for oversized input, got nil") + } +} + +func TestCollectFiles_emptyDirectory(t *testing.T) { + dir := t.TempDir() + + files, err := plugin.CollectFiles(dir, []string{".yaml"}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 0 { + t.Errorf("expected empty map, got: %v", files) + } +} + +func TestCollectFiles_nonExistentInput(t *testing.T) { + _, err := plugin.CollectFiles(filepath.Join(t.TempDir(), "nonexistent.yaml"), nil, 0) + if err == nil { + t.Fatal("expected error for non-existent path, got nil") + } +} diff --git a/cli/internal/plugin/discover.go b/cli/internal/plugin/discover.go index 7263e32..2d22352 100644 --- a/cli/internal/plugin/discover.go +++ b/cli/internal/plugin/discover.go @@ -74,3 +74,10 @@ func loadPlugin(dir string) (*Plugin, error) { return raw.toPlugin(dir), nil } + +// LoadPlugin reads, parses, and validates the plugin.yaml inside dir. +// It is the exported equivalent of loadPlugin, used when --plugin bypasses +// name-based discovery. +func LoadPlugin(dir string) (*Plugin, error) { + return loadPlugin(dir) +} From 7fc9ec2455c7cae501f762e2637e917b406a54b0 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 14:56:37 -0500 Subject: [PATCH 02/10] feat(cli): embed osi schema for output validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds cli/internal/schema/ with the OSI JSON schema embedded via //go:embed. Validate() accepts YAML or JSON, round-trips through encoding/json to normalise yaml.v3 types to what jsonschema/v6 expects, then validates against the compiled schema. The schema is compiled once in init() — a panic here indicates a build defect (corrupted embed), not a runtime error. New dependency: github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 (draft 2020-12 support, matching the schema's $schema declaration). --- cli/go.mod | 2 + cli/go.sum | 6 + cli/internal/schema/osi-schema.json | 330 ++++++++++++++++++++++++++++ cli/internal/schema/schema.go | 54 +++++ cli/internal/schema/schema_test.go | 55 +++++ 5 files changed, 447 insertions(+) create mode 100644 cli/internal/schema/osi-schema.json create mode 100644 cli/internal/schema/schema.go create mode 100644 cli/internal/schema/schema_test.go diff --git a/cli/go.mod b/cli/go.mod index d17729d..afb2f28 100644 --- a/cli/go.mod +++ b/cli/go.mod @@ -3,6 +3,7 @@ module github.com/open-semantic-interchange/ossie/cli go 1.22 require ( + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 github.com/spf13/cobra v1.10.2 gopkg.in/yaml.v3 v3.0.1 ) @@ -10,4 +11,5 @@ require ( require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/pflag v1.0.9 // indirect + golang.org/x/text v0.14.0 // indirect ) diff --git a/cli/go.sum b/cli/go.sum index 47edb24..f11d3e5 100644 --- a/cli/go.sum +++ b/cli/go.sum @@ -1,12 +1,18 @@ github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= +golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/cli/internal/schema/osi-schema.json b/cli/internal/schema/osi-schema.json new file mode 100644 index 0000000..72cb164 --- /dev/null +++ b/cli/internal/schema/osi-schema.json @@ -0,0 +1,330 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/open-semantic-interchange/OSI/core-spec/osi-schema.json", + "title": "OSI Core Metadata Specification", + "description": "JSON Schema for validating OSI (Open Semantic Interoperability) semantic model definitions", + "type": "object", + "properties": { + "version": { + "type": "string", + "const": "0.2.0.dev0", + "description": "OSI specification version" + }, + "semantic_model": { + "type": "array", + "description": "Collection of semantic model definitions", + "items": { + "$ref": "#/$defs/SemanticModel" + } + } + }, + "required": ["version", "semantic_model"], + "additionalProperties": false, + "$defs": { + "Dialect": { + "type": "string", + "enum": ["ANSI_SQL", "SNOWFLAKE", "MDX", "TABLEAU", "DATABRICKS", "MAQL"], + "description": "Supported SQL and expression language dialects" + }, + "Vendor": { + "type": "string", + "examples": ["COMMON", "SNOWFLAKE", "SALESFORCE", "DBT", "DATABRICKS", "GOODDATA"], + "description": "Vendor name for custom extensions. Any string value is accepted." + }, + "AIContext": { + "description": "Additional context for AI tools", + "oneOf": [ + { + "type": "string" + }, + { + "type": "object", + "properties": { + "instructions": { + "type": "string", + "description": "Instructions for AI on how to use this entity" + }, + "synonyms": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Alternative names and terms" + }, + "examples": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Sample questions or use cases" + } + }, + "additionalProperties": true + } + ] + }, + "CustomExtension": { + "type": "object", + "description": "Vendor-specific attributes for extensibility", + "properties": { + "vendor_name": { + "$ref": "#/$defs/Vendor" + }, + "data": { + "type": "string", + "description": "JSON string containing vendor-specific data" + } + }, + "required": ["vendor_name", "data"], + "additionalProperties": false + }, + "DialectExpression": { + "type": "object", + "description": "Expression in a specific dialect", + "properties": { + "dialect": { + "$ref": "#/$defs/Dialect" + }, + "expression": { + "type": "string", + "description": "SQL or dialect-specific expression" + } + }, + "required": ["dialect", "expression"], + "additionalProperties": false + }, + "Expression": { + "type": "object", + "description": "Expression definition with multi-dialect support", + "properties": { + "dialects": { + "type": "array", + "items": { + "$ref": "#/$defs/DialectExpression" + }, + "minItems": 1 + } + }, + "required": ["dialects"], + "additionalProperties": false + }, + "Dimension": { + "type": "object", + "description": "Dimension metadata", + "properties": { + "is_time": { + "type": "boolean", + "description": "Indicates if this is a time-based dimension for temporal filtering" + } + }, + "additionalProperties": false + }, + "Field": { + "type": "object", + "description": "Row-level attribute for grouping, filtering, and metric expressions", + "properties": { + "name": { + "type": "string", + "description": "Unique identifier for the field within the dataset" + }, + "expression": { + "$ref": "#/$defs/Expression" + }, + "dimension": { + "$ref": "#/$defs/Dimension" + }, + "label": { + "type": "string", + "description": "Label for categorization" + }, + "description": { + "type": "string", + "description": "Human-readable description" + }, + "ai_context": { + "$ref": "#/$defs/AIContext" + }, + "custom_extensions": { + "type": "array", + "items": { + "$ref": "#/$defs/CustomExtension" + } + } + }, + "required": ["name", "expression"], + "additionalProperties": false + }, + "Dataset": { + "type": "object", + "description": "Logical dataset representing a business entity (fact or dimension table)", + "properties": { + "name": { + "type": "string", + "description": "Unique identifier for the dataset" + }, + "source": { + "type": "string", + "description": "Reference to underlying physical table/view (database.schema.table) or query" + }, + "primary_key": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Primary key columns (single or composite)" + }, + "unique_keys": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + }, + "description": "Array of unique key definitions (each can be single or composite)" + }, + "description": { + "type": "string", + "description": "Human-readable description" + }, + "ai_context": { + "$ref": "#/$defs/AIContext" + }, + "fields": { + "type": "array", + "items": { + "$ref": "#/$defs/Field" + } + }, + "custom_extensions": { + "type": "array", + "items": { + "$ref": "#/$defs/CustomExtension" + } + } + }, + "required": ["name", "source"], + "additionalProperties": false + }, + "Relationship": { + "type": "object", + "description": "Foreign key relationship between datasets", + "properties": { + "name": { + "type": "string", + "description": "Unique identifier for the relationship" + }, + "from": { + "type": "string", + "description": "Dataset on the many side of the relationship" + }, + "to": { + "type": "string", + "description": "Dataset on the one side of the relationship" + }, + "from_columns": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "description": "Foreign key columns in the 'from' dataset" + }, + "to_columns": { + "type": "array", + "items": { + "type": "string" + }, + "minItems": 1, + "description": "Primary/unique key columns in the 'to' dataset" + }, + "ai_context": { + "$ref": "#/$defs/AIContext" + }, + "custom_extensions": { + "type": "array", + "items": { + "$ref": "#/$defs/CustomExtension" + } + } + }, + "required": ["name", "from", "to", "from_columns", "to_columns"], + "additionalProperties": false + }, + "Metric": { + "type": "object", + "description": "Quantitative measure defined on business data", + "properties": { + "name": { + "type": "string", + "description": "Unique identifier for the metric" + }, + "expression": { + "$ref": "#/$defs/Expression" + }, + "description": { + "type": "string", + "description": "Human-readable description of what the metric measures" + }, + "ai_context": { + "$ref": "#/$defs/AIContext" + }, + "custom_extensions": { + "type": "array", + "items": { + "$ref": "#/$defs/CustomExtension" + } + } + }, + "required": ["name", "expression"], + "additionalProperties": false + }, + "SemanticModel": { + "type": "object", + "description": "Top-level container representing a complete semantic model", + "properties": { + "name": { + "type": "string", + "description": "Unique identifier for the semantic model" + }, + "description": { + "type": "string", + "description": "Human-readable description" + }, + "ai_context": { + "$ref": "#/$defs/AIContext" + }, + "datasets": { + "type": "array", + "items": { + "$ref": "#/$defs/Dataset" + }, + "minItems": 1, + "description": "Collection of logical datasets" + }, + "relationships": { + "type": "array", + "items": { + "$ref": "#/$defs/Relationship" + }, + "description": "Defines how datasets are connected" + }, + "metrics": { + "type": "array", + "items": { + "$ref": "#/$defs/Metric" + }, + "description": "Quantifiable measures spanning datasets" + }, + "custom_extensions": { + "type": "array", + "items": { + "$ref": "#/$defs/CustomExtension" + } + } + }, + "required": ["name", "datasets"], + "additionalProperties": false + } + } +} diff --git a/cli/internal/schema/schema.go b/cli/internal/schema/schema.go new file mode 100644 index 0000000..c3c0efd --- /dev/null +++ b/cli/internal/schema/schema.go @@ -0,0 +1,54 @@ +package schema + +import ( + "bytes" + _ "embed" // blank import required to activate //go:embed + "encoding/json" + "fmt" + + "github.com/santhosh-tekuri/jsonschema/v6" + "gopkg.in/yaml.v3" +) + +//go:embed osi-schema.json +var schemaJSON []byte + +// schemaID must exactly match the $id in osi-schema.json. +// The compiler uses this string as both the resource URL and the compile target. +const schemaID = "https://github.com/open-semantic-interchange/OSI/core-spec/osi-schema.json" + +var compiledSchema *jsonschema.Schema + +func init() { + schemaSrc, err := jsonschema.UnmarshalJSON(bytes.NewReader(schemaJSON)) + if err != nil { + panic(fmt.Sprintf("schema: failed to parse embedded OSI schema: %v", err)) + } + c := jsonschema.NewCompiler() + if err := c.AddResource(schemaID, schemaSrc); err != nil { + panic(fmt.Sprintf("schema: failed to register embedded OSI schema: %v", err)) + } + compiledSchema = c.MustCompile(schemaID) +} + +// Validate validates data (YAML or JSON bytes) against the embedded OSI schema. +// YAML is converted to JSON via a round-trip through yaml.v3 and encoding/json +// to normalise Go types to the representation jsonschema/v6 expects. +// Returns nil if valid, an error describing the violation otherwise. +func Validate(data []byte) error { + var doc any + if err := yaml.Unmarshal(data, &doc); err != nil { + return fmt.Errorf("failed to parse document: %w", err) + } + // Round-trip through JSON to normalise yaml.v3's native int/float64 + // to json.Number, which jsonschema/v6 expects. + jsonBytes, err := json.Marshal(doc) + if err != nil { + return fmt.Errorf("failed to serialise document to JSON: %w", err) + } + inst, err := jsonschema.UnmarshalJSON(bytes.NewReader(jsonBytes)) + if err != nil { + return fmt.Errorf("failed to prepare document for validation: %w", err) + } + return compiledSchema.Validate(inst) +} diff --git a/cli/internal/schema/schema_test.go b/cli/internal/schema/schema_test.go new file mode 100644 index 0000000..2a4ac85 --- /dev/null +++ b/cli/internal/schema/schema_test.go @@ -0,0 +1,55 @@ +package schema_test + +import ( + "testing" + + "github.com/open-semantic-interchange/ossie/cli/internal/schema" +) + +// minimalValidYAML satisfies all schema-required fields: +// root: version (const) + semantic_model (array) +// SemanticModel: name + datasets (minItems 1) +// Dataset: name + source +const minimalValidYAML = ` +version: "0.2.0.dev0" +semantic_model: + - name: test_model + datasets: + - name: test_dataset + source: db.schema.table +` + +const minimalValidJSON = `{ + "version": "0.2.0.dev0", + "semantic_model": [ + { + "name": "test_model", + "datasets": [ + { + "name": "test_dataset", + "source": "db.schema.table" + } + ] + } + ] +}` + +func TestValidate_validYAML(t *testing.T) { + if err := schema.Validate([]byte(minimalValidYAML)); err != nil { + t.Errorf("expected valid YAML to pass, got: %v", err) + } +} + +func TestValidate_validJSON(t *testing.T) { + if err := schema.Validate([]byte(minimalValidJSON)); err != nil { + t.Errorf("expected valid JSON to pass, got: %v", err) + } +} + +func TestValidate_invalidDocument(t *testing.T) { + // Missing required fields (version, semantic_model) and additionalProperties:false + // means unknown top-level keys are also rejected. + if err := schema.Validate([]byte(`name: foo`)); err == nil { + t.Error("expected invalid document to fail, got nil") + } +} From 85f3d00396d133f2d89ad4d70131fa214fbdda07 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 14:57:55 -0500 Subject: [PATCH 03/10] feat(cli): implement osi convert base command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wires together file collection (F2), plugin invocation (F3), schema validation, and output writing into a working end-to-end convert command. Flow: resolve plugin by name or --plugin path → collect input files → invoke plugin with timeout context → validate to_osi output against embedded OSI schema → write passing files to ossie-output/ → render issues to stderr → exit non-zero on any error-severity issue. Schema validation failures remove the offending file from output (rather than writing it with a warning) — the spec says validate before writing. Info-severity issues are suppressed unless --verbose. parseMaxInputSize supports MB/GB suffixes (case-insensitive) and raw byte counts. renderIssues formats each issue as: severity: message severity: path: message (when path is present) --- cli/cmd/convert.go | 186 +++++++++++++++++++++++++++++++++++++++- cli/cmd/convert_test.go | 124 +++++++++++++++++++++++++++ 2 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 cli/cmd/convert_test.go diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index 92722ee..bb285ae 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -1,8 +1,18 @@ package cmd import ( + "context" "fmt" + "io" + "os" + "path/filepath" + "strconv" + "strings" + "time" + "github.com/open-semantic-interchange/ossie/cli/internal/ossiedir" + "github.com/open-semantic-interchange/ossie/cli/internal/plugin" + "github.com/open-semantic-interchange/ossie/cli/internal/schema" "github.com/spf13/cobra" ) @@ -26,14 +36,184 @@ func init() { } func runConvert(cmd *cobra.Command, args []string) error { + // --- 1. Parse flags --- from, _ := cmd.Flags().GetString("from") to, _ := cmd.Flags().GetString("to") - - // MarkFlagsMutuallyExclusive handles the both-set case; handle neither here. if from == "" && to == "" { return fmt.Errorf("exactly one of --from or --to must be specified") } - fmt.Fprintln(cmd.OutOrStdout(), "not yet implemented") + input, _ := cmd.Flags().GetString("input") + outputFlag, _ := cmd.Flags().GetString("output") + pluginFlag, _ := cmd.Flags().GetString("plugin") + timeoutSecs, _ := cmd.Flags().GetInt("timeout") + maxInputSizeStr, _ := cmd.Flags().GetString("max-input-size") + // verbose is a PersistentFlag on root; Cobra injects it into child flag sets. + verbose, _ := cmd.Flags().GetBool("verbose") + + // --- 2. Resolve plugin --- + var p *plugin.Plugin + if pluginFlag != "" { + loaded, err := plugin.LoadPlugin(pluginFlag) + if err != nil { + return fmt.Errorf("could not load plugin from %q: %w", pluginFlag, err) + } + p = loaded + } else { + platformName := from + if to != "" { + platformName = to + } + pluginsDir, err := ossiedir.PluginDir() + if err != nil { + return err + } + plugins, err := plugin.Discover(pluginsDir, cmd.ErrOrStderr()) + if err != nil { + return err + } + var matches []*plugin.Plugin + for _, candidate := range plugins { + if candidate.Platform.Name == platformName { + matches = append(matches, candidate) + } + } + switch len(matches) { + case 0: + return fmt.Errorf("no plugin found for %q; run 'ossie plugin install %s' to install it", platformName, platformName) + case 1: + p = matches[0] + default: + return fmt.Errorf("multiple plugins found for %q; use --plugin to specify a path", platformName) + } + } + + // --- 3. Select direction --- + var direction string + var dir plugin.Direction + if from != "" { + direction = "to_osi" + dir = p.Convert.ToOSI + } else { + direction = "from_osi" + dir = p.Convert.FromOSI + } + + // --- 4. Parse max-input-size --- + maxBytes, err := parseMaxInputSize(maxInputSizeStr) + if err != nil { + return fmt.Errorf("invalid --max-input-size %q: %w", maxInputSizeStr, err) + } + + // --- 5. Collect input files --- + files, err := plugin.CollectFiles(input, dir.Accepts, maxBytes) + if err != nil { + return fmt.Errorf("input collection failed: %w", err) + } + + // --- 6. Build context with timeout --- + ctx := cmd.Context() + if ctx == nil { + ctx = context.Background() + } + ctx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSecs)*time.Second) + defer cancel() + + // --- 7. Route plugin stderr --- + var pluginStderr io.Writer = io.Discard + if verbose { + pluginStderr = cmd.ErrOrStderr() + } + + // --- 8. Invoke plugin --- + resp, err := plugin.Invoke(ctx, p.Path, dir.Invoke, plugin.Request{Files: files}, pluginStderr) + if err != nil { + return fmt.Errorf("plugin invocation failed: %w", err) + } + + // --- 9. Validate output (to_osi direction only) --- + // Schema validation runs before any files are written to disk. + // Files that fail validation are removed from resp.Files and reported + // as error-severity issues. + if direction == "to_osi" { + for name, content := range resp.Files { + if verr := schema.Validate([]byte(content)); verr != nil { + resp.Issues = append(resp.Issues, plugin.Issue{ + Severity: "error", + Path: name, + Message: fmt.Sprintf("output validation failed: %v", verr), + }) + delete(resp.Files, name) + } + } + } + + // --- 10. Write output files --- + outDir := outputFlag + if outDir == "" { + outDir = filepath.Join("ossie-output", p.Platform.Name, direction) + } + if err := os.MkdirAll(outDir, 0755); err != nil { + return fmt.Errorf("could not create output directory %q: %w", outDir, err) + } + for name, content := range resp.Files { + dest := filepath.Join(outDir, filepath.FromSlash(name)) + if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { + return fmt.Errorf("could not create output subdirectory for %q: %w", name, err) + } + if err := os.WriteFile(dest, []byte(content), 0644); err != nil { + return fmt.Errorf("could not write output file %q: %w", dest, err) + } + } + + // --- 11. Render issues and set exit code --- + if hasErrors := renderIssues(resp.Issues, cmd.ErrOrStderr(), verbose); hasErrors { + return fmt.Errorf("conversion completed with errors") + } return nil } + +// parseMaxInputSize parses a size string like "100MB", "2GB", or a raw integer +// (interpreted as bytes). The suffix is case-insensitive. +func parseMaxInputSize(s string) (int64, error) { + upper := strings.ToUpper(strings.TrimSpace(s)) + if strings.HasSuffix(upper, "GB") { + n, err := strconv.ParseInt(strings.TrimSuffix(upper, "GB"), 10, 64) + if err != nil { + return 0, fmt.Errorf("invalid GB value: %w", err) + } + return n * 1024 * 1024 * 1024, nil + } + if strings.HasSuffix(upper, "MB") { + n, err := strconv.ParseInt(strings.TrimSuffix(upper, "MB"), 10, 64) + if err != nil { + return 0, fmt.Errorf("invalid MB value: %w", err) + } + return n * 1024 * 1024, nil + } + n, err := strconv.ParseInt(upper, 10, 64) + if err != nil { + return 0, fmt.Errorf("unsupported size format %q (use e.g. 100MB, 2GB, or a raw byte count)", s) + } + return n, nil +} + +// renderIssues writes issues to w and returns true if any error-severity issue +// is present. info-severity issues are suppressed unless verbose is true. +func renderIssues(issues []plugin.Issue, w io.Writer, verbose bool) bool { + hasErrors := false + for _, iss := range issues { + if iss.Severity == "info" && !verbose { + continue + } + if iss.Path != "" { + fmt.Fprintf(w, "%s: %s: %s\n", iss.Severity, iss.Path, iss.Message) + } else { + fmt.Fprintf(w, "%s: %s\n", iss.Severity, iss.Message) + } + if iss.Severity == "error" { + hasErrors = true + } + } + return hasErrors +} diff --git a/cli/cmd/convert_test.go b/cli/cmd/convert_test.go new file mode 100644 index 0000000..adc7190 --- /dev/null +++ b/cli/cmd/convert_test.go @@ -0,0 +1,124 @@ +package cmd + +import ( + "strings" + "testing" + + "github.com/open-semantic-interchange/ossie/cli/internal/plugin" +) + +// --- parseMaxInputSize tests --- + +func TestParseMaxInputSize_MB(t *testing.T) { + got, err := parseMaxInputSize("100MB") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := int64(100 * 1024 * 1024) + if got != want { + t.Errorf("got %d, want %d", got, want) + } +} + +func TestParseMaxInputSize_MB_lowercase(t *testing.T) { + got, err := parseMaxInputSize("50mb") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := int64(50 * 1024 * 1024) + if got != want { + t.Errorf("got %d, want %d", got, want) + } +} + +func TestParseMaxInputSize_GB(t *testing.T) { + got, err := parseMaxInputSize("2GB") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := int64(2 * 1024 * 1024 * 1024) + if got != want { + t.Errorf("got %d, want %d", got, want) + } +} + +func TestParseMaxInputSize_rawBytes(t *testing.T) { + got, err := parseMaxInputSize("1048576") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1048576 { + t.Errorf("got %d, want 1048576", got) + } +} + +func TestParseMaxInputSize_invalid(t *testing.T) { + _, err := parseMaxInputSize("fooMB") + if err == nil { + t.Fatal("expected error for invalid input, got nil") + } +} + +func TestParseMaxInputSize_invalidNoSuffix(t *testing.T) { + _, err := parseMaxInputSize("not-a-number") + if err == nil { + t.Fatal("expected error for non-numeric input without suffix, got nil") + } +} + +// --- renderIssues tests --- + +func TestRenderIssues_errorSetsReturnTrue(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "error", Message: "something failed"}} + if !renderIssues(issues, &buf, false) { + t.Error("expected hasErrors=true for error-severity issue") + } + if !strings.Contains(buf.String(), "error: something failed") { + t.Errorf("unexpected output: %q", buf.String()) + } +} + +func TestRenderIssues_warningSetsReturnFalse(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "warning", Message: "a warning"}} + if renderIssues(issues, &buf, false) { + t.Error("expected hasErrors=false for warning-only issue") + } +} + +func TestRenderIssues_infoSuppressedWhenNotVerbose(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "info", Message: "some info"}} + renderIssues(issues, &buf, false) + if buf.Len() != 0 { + t.Errorf("expected no output for info issue when not verbose, got: %q", buf.String()) + } +} + +func TestRenderIssues_infoPrintedWhenVerbose(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "info", Message: "some info"}} + renderIssues(issues, &buf, true) + if !strings.Contains(buf.String(), "info: some info") { + t.Errorf("expected info output when verbose, got: %q", buf.String()) + } +} + +func TestRenderIssues_pathIncludedInOutput(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "warning", Path: "models/orders.yaml", Message: "deprecated field"}} + renderIssues(issues, &buf, false) + if !strings.Contains(buf.String(), "warning: models/orders.yaml: deprecated field") { + t.Errorf("expected path in output, got: %q", buf.String()) + } +} + +func TestRenderIssues_emptyPathNoDoubleColon(t *testing.T) { + var buf strings.Builder + issues := []plugin.Issue{{Severity: "error", Message: "fatal"}} + renderIssues(issues, &buf, false) + if strings.Contains(buf.String(), ": : ") { + t.Errorf("empty path must not produce double colon, got: %q", buf.String()) + } +} From 1c40d07e8ac28c29d5531749ce550177eb2143aa Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 15:18:47 -0500 Subject: [PATCH 04/10] chore(cli): remove duplicate schema copy, generate from core-spec cli/internal/schema/osi-schema.json was a committed copy of core-spec/osi-schema.json that could drift silently. The file is now gitignored and generated by `make prepare`, which copies from the canonical source before any build or test step. CI switches from bare go commands to make targets so the copy step runs automatically in all environments. --- .github/workflows/cli-ci.yml | 12 +- .gitignore | 3 + cli/Makefile | 14 +- cli/internal/schema/osi-schema.json | 330 ---------------------------- 4 files changed, 18 insertions(+), 341 deletions(-) delete mode 100644 cli/internal/schema/osi-schema.json diff --git a/.github/workflows/cli-ci.yml b/.github/workflows/cli-ci.yml index e729089..e182e77 100644 --- a/.github/workflows/cli-ci.yml +++ b/.github/workflows/cli-ci.yml @@ -28,11 +28,11 @@ jobs: go-version-file: cli/go.mod cache-dependency-path: cli/go.sum - - name: go build - run: go build ./... + - name: make build + run: make build - - name: go vet - run: go vet ./... + - name: make lint + run: make lint - - name: go test - run: go test ./... + - name: make test + run: make test diff --git a/.gitignore b/.gitignore index 5f39ab3..46098ed 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,6 @@ # Go CLI cli/dist/ cli/ossie + +# Generated from core-spec/osi-schema.json by `make prepare` +cli/internal/schema/osi-schema.json diff --git a/cli/Makefile b/cli/Makefile index d1f7d66..7575875 100644 --- a/cli/Makefile +++ b/cli/Makefile @@ -1,18 +1,22 @@ BINARY_NAME := ossie BUILD_DIR := dist -.PHONY: build install test lint release-dry-run clean +.PHONY: prepare build install test lint release-dry-run clean -build: +# Copy the canonical schema from the repo root before any Go step that needs it. +prepare: + cp ../core-spec/osi-schema.json internal/schema/osi-schema.json + +build: prepare go build -o $(BUILD_DIR)/$(BINARY_NAME) . -install: +install: prepare go build -o $(shell go env GOPATH)/bin/$(BINARY_NAME) . -test: +test: prepare go test ./... -lint: +lint: prepare go vet ./... release-dry-run: diff --git a/cli/internal/schema/osi-schema.json b/cli/internal/schema/osi-schema.json deleted file mode 100644 index 72cb164..0000000 --- a/cli/internal/schema/osi-schema.json +++ /dev/null @@ -1,330 +0,0 @@ -{ - "$schema": "https://json-schema.org/draft/2020-12/schema", - "$id": "https://github.com/open-semantic-interchange/OSI/core-spec/osi-schema.json", - "title": "OSI Core Metadata Specification", - "description": "JSON Schema for validating OSI (Open Semantic Interoperability) semantic model definitions", - "type": "object", - "properties": { - "version": { - "type": "string", - "const": "0.2.0.dev0", - "description": "OSI specification version" - }, - "semantic_model": { - "type": "array", - "description": "Collection of semantic model definitions", - "items": { - "$ref": "#/$defs/SemanticModel" - } - } - }, - "required": ["version", "semantic_model"], - "additionalProperties": false, - "$defs": { - "Dialect": { - "type": "string", - "enum": ["ANSI_SQL", "SNOWFLAKE", "MDX", "TABLEAU", "DATABRICKS", "MAQL"], - "description": "Supported SQL and expression language dialects" - }, - "Vendor": { - "type": "string", - "examples": ["COMMON", "SNOWFLAKE", "SALESFORCE", "DBT", "DATABRICKS", "GOODDATA"], - "description": "Vendor name for custom extensions. Any string value is accepted." - }, - "AIContext": { - "description": "Additional context for AI tools", - "oneOf": [ - { - "type": "string" - }, - { - "type": "object", - "properties": { - "instructions": { - "type": "string", - "description": "Instructions for AI on how to use this entity" - }, - "synonyms": { - "type": "array", - "items": { - "type": "string" - }, - "description": "Alternative names and terms" - }, - "examples": { - "type": "array", - "items": { - "type": "string" - }, - "description": "Sample questions or use cases" - } - }, - "additionalProperties": true - } - ] - }, - "CustomExtension": { - "type": "object", - "description": "Vendor-specific attributes for extensibility", - "properties": { - "vendor_name": { - "$ref": "#/$defs/Vendor" - }, - "data": { - "type": "string", - "description": "JSON string containing vendor-specific data" - } - }, - "required": ["vendor_name", "data"], - "additionalProperties": false - }, - "DialectExpression": { - "type": "object", - "description": "Expression in a specific dialect", - "properties": { - "dialect": { - "$ref": "#/$defs/Dialect" - }, - "expression": { - "type": "string", - "description": "SQL or dialect-specific expression" - } - }, - "required": ["dialect", "expression"], - "additionalProperties": false - }, - "Expression": { - "type": "object", - "description": "Expression definition with multi-dialect support", - "properties": { - "dialects": { - "type": "array", - "items": { - "$ref": "#/$defs/DialectExpression" - }, - "minItems": 1 - } - }, - "required": ["dialects"], - "additionalProperties": false - }, - "Dimension": { - "type": "object", - "description": "Dimension metadata", - "properties": { - "is_time": { - "type": "boolean", - "description": "Indicates if this is a time-based dimension for temporal filtering" - } - }, - "additionalProperties": false - }, - "Field": { - "type": "object", - "description": "Row-level attribute for grouping, filtering, and metric expressions", - "properties": { - "name": { - "type": "string", - "description": "Unique identifier for the field within the dataset" - }, - "expression": { - "$ref": "#/$defs/Expression" - }, - "dimension": { - "$ref": "#/$defs/Dimension" - }, - "label": { - "type": "string", - "description": "Label for categorization" - }, - "description": { - "type": "string", - "description": "Human-readable description" - }, - "ai_context": { - "$ref": "#/$defs/AIContext" - }, - "custom_extensions": { - "type": "array", - "items": { - "$ref": "#/$defs/CustomExtension" - } - } - }, - "required": ["name", "expression"], - "additionalProperties": false - }, - "Dataset": { - "type": "object", - "description": "Logical dataset representing a business entity (fact or dimension table)", - "properties": { - "name": { - "type": "string", - "description": "Unique identifier for the dataset" - }, - "source": { - "type": "string", - "description": "Reference to underlying physical table/view (database.schema.table) or query" - }, - "primary_key": { - "type": "array", - "items": { - "type": "string" - }, - "description": "Primary key columns (single or composite)" - }, - "unique_keys": { - "type": "array", - "items": { - "type": "array", - "items": { - "type": "string" - } - }, - "description": "Array of unique key definitions (each can be single or composite)" - }, - "description": { - "type": "string", - "description": "Human-readable description" - }, - "ai_context": { - "$ref": "#/$defs/AIContext" - }, - "fields": { - "type": "array", - "items": { - "$ref": "#/$defs/Field" - } - }, - "custom_extensions": { - "type": "array", - "items": { - "$ref": "#/$defs/CustomExtension" - } - } - }, - "required": ["name", "source"], - "additionalProperties": false - }, - "Relationship": { - "type": "object", - "description": "Foreign key relationship between datasets", - "properties": { - "name": { - "type": "string", - "description": "Unique identifier for the relationship" - }, - "from": { - "type": "string", - "description": "Dataset on the many side of the relationship" - }, - "to": { - "type": "string", - "description": "Dataset on the one side of the relationship" - }, - "from_columns": { - "type": "array", - "items": { - "type": "string" - }, - "minItems": 1, - "description": "Foreign key columns in the 'from' dataset" - }, - "to_columns": { - "type": "array", - "items": { - "type": "string" - }, - "minItems": 1, - "description": "Primary/unique key columns in the 'to' dataset" - }, - "ai_context": { - "$ref": "#/$defs/AIContext" - }, - "custom_extensions": { - "type": "array", - "items": { - "$ref": "#/$defs/CustomExtension" - } - } - }, - "required": ["name", "from", "to", "from_columns", "to_columns"], - "additionalProperties": false - }, - "Metric": { - "type": "object", - "description": "Quantitative measure defined on business data", - "properties": { - "name": { - "type": "string", - "description": "Unique identifier for the metric" - }, - "expression": { - "$ref": "#/$defs/Expression" - }, - "description": { - "type": "string", - "description": "Human-readable description of what the metric measures" - }, - "ai_context": { - "$ref": "#/$defs/AIContext" - }, - "custom_extensions": { - "type": "array", - "items": { - "$ref": "#/$defs/CustomExtension" - } - } - }, - "required": ["name", "expression"], - "additionalProperties": false - }, - "SemanticModel": { - "type": "object", - "description": "Top-level container representing a complete semantic model", - "properties": { - "name": { - "type": "string", - "description": "Unique identifier for the semantic model" - }, - "description": { - "type": "string", - "description": "Human-readable description" - }, - "ai_context": { - "$ref": "#/$defs/AIContext" - }, - "datasets": { - "type": "array", - "items": { - "$ref": "#/$defs/Dataset" - }, - "minItems": 1, - "description": "Collection of logical datasets" - }, - "relationships": { - "type": "array", - "items": { - "$ref": "#/$defs/Relationship" - }, - "description": "Defines how datasets are connected" - }, - "metrics": { - "type": "array", - "items": { - "$ref": "#/$defs/Metric" - }, - "description": "Quantifiable measures spanning datasets" - }, - "custom_extensions": { - "type": "array", - "items": { - "$ref": "#/$defs/CustomExtension" - } - } - }, - "required": ["name", "datasets"], - "additionalProperties": false - } - } -} From bcd3d7066a181434fe0d7069c37d02e8545ee9b3 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 15:21:32 -0500 Subject: [PATCH 05/10] test(cli): add test for CollectFiles with empty accepts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty accepts means no extension filter — all files in the directory are included. The godoc stated this but it was untested. --- cli/internal/plugin/collect_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cli/internal/plugin/collect_test.go b/cli/internal/plugin/collect_test.go index 46f71be..bb0f550 100644 --- a/cli/internal/plugin/collect_test.go +++ b/cli/internal/plugin/collect_test.go @@ -120,3 +120,19 @@ func TestCollectFiles_nonExistentInput(t *testing.T) { t.Fatal("expected error for non-existent path, got nil") } } + +func TestCollectFiles_emptyAcceptsIncludesAllFiles(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "model.yaml", "yaml-content") + writeFile(t, dir, "readme.md", "md-content") + writeFile(t, dir, "data.json", "json-content") + + // Empty accepts means no filter — all files should be included. + files, err := plugin.CollectFiles(dir, []string{}, 0) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 3 { + t.Errorf("expected 3 files with empty accepts, got %d: %v", len(files), files) + } +} From b5f0b0ebd02d927677a52b490e189b115d3b2dfa Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 15:23:05 -0500 Subject: [PATCH 06/10] fix(cli): skip output dir creation when no files to write If all to_osi output files fail schema validation, resp.Files is empty. Creating the output directory in that case leaves an empty ossie-output/ on disk with no explanation. Guard os.MkdirAll behind len(resp.Files) > 0 so the directory only appears when there is something to write. --- cli/cmd/convert.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index bb285ae..8005f86 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -153,8 +153,10 @@ func runConvert(cmd *cobra.Command, args []string) error { if outDir == "" { outDir = filepath.Join("ossie-output", p.Platform.Name, direction) } - if err := os.MkdirAll(outDir, 0755); err != nil { - return fmt.Errorf("could not create output directory %q: %w", outDir, err) + if len(resp.Files) > 0 { + if err := os.MkdirAll(outDir, 0755); err != nil { + return fmt.Errorf("could not create output directory %q: %w", outDir, err) + } } for name, content := range resp.Files { dest := filepath.Join(outDir, filepath.FromSlash(name)) From 0c0e5573396ee1fe9ff46ee1ffda1ff5521b02a9 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 15:24:37 -0500 Subject: [PATCH 07/10] fix(cli): use backticks for inline command in error message Consistent with Go error message conventions and the rest of the codebase. Single quotes were a shell quoting style. --- cli/cmd/convert.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index 8005f86..27c2331 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -80,7 +80,7 @@ func runConvert(cmd *cobra.Command, args []string) error { } switch len(matches) { case 0: - return fmt.Errorf("no plugin found for %q; run 'ossie plugin install %s' to install it", platformName, platformName) + return fmt.Errorf("no plugin found for %q; run `ossie plugin install %s` to install it", platformName, platformName) case 1: p = matches[0] default: From a8b8a5675b10cc4f1e26c1b0ea77206731e7ed3f Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Tue, 16 Jun 2026 15:29:12 -0500 Subject: [PATCH 08/10] refactor(cli): break runConvert into named helper functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit runConvert was a single function with 11 numbered sections. Extract sections 2, 3, 9, and 10 into named functions so the orchestrator reads as a clean sequence of calls: resolvePlugin — discover or load plugin by path/name selectDirection — map --from/--to to direction + Direction struct validateOutput — schema-validate to_osi files, remove invalid ones writeOutput — create outDir and write response files to disk Sections 4 (parseMaxInputSize), 5 (CollectFiles), 8 (Invoke), and 11 (renderIssues) were already named functions. Sections 1, 6, and 7 are too small to warrant extraction. --- cli/cmd/convert.go | 181 ++++++++++++++++++++++++++------------------- 1 file changed, 104 insertions(+), 77 deletions(-) diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index 27c2331..c61e284 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -36,13 +36,12 @@ func init() { } func runConvert(cmd *cobra.Command, args []string) error { - // --- 1. Parse flags --- + // 1. Parse flags from, _ := cmd.Flags().GetString("from") to, _ := cmd.Flags().GetString("to") if from == "" && to == "" { return fmt.Errorf("exactly one of --from or --to must be specified") } - input, _ := cmd.Flags().GetString("input") outputFlag, _ := cmd.Flags().GetString("output") pluginFlag, _ := cmd.Flags().GetString("plugin") @@ -51,67 +50,32 @@ func runConvert(cmd *cobra.Command, args []string) error { // verbose is a PersistentFlag on root; Cobra injects it into child flag sets. verbose, _ := cmd.Flags().GetBool("verbose") - // --- 2. Resolve plugin --- - var p *plugin.Plugin - if pluginFlag != "" { - loaded, err := plugin.LoadPlugin(pluginFlag) - if err != nil { - return fmt.Errorf("could not load plugin from %q: %w", pluginFlag, err) - } - p = loaded - } else { - platformName := from - if to != "" { - platformName = to - } - pluginsDir, err := ossiedir.PluginDir() - if err != nil { - return err - } - plugins, err := plugin.Discover(pluginsDir, cmd.ErrOrStderr()) - if err != nil { - return err - } - var matches []*plugin.Plugin - for _, candidate := range plugins { - if candidate.Platform.Name == platformName { - matches = append(matches, candidate) - } - } - switch len(matches) { - case 0: - return fmt.Errorf("no plugin found for %q; run `ossie plugin install %s` to install it", platformName, platformName) - case 1: - p = matches[0] - default: - return fmt.Errorf("multiple plugins found for %q; use --plugin to specify a path", platformName) - } + // 2. Resolve plugin + platformName := from + if to != "" { + platformName = to } - - // --- 3. Select direction --- - var direction string - var dir plugin.Direction - if from != "" { - direction = "to_osi" - dir = p.Convert.ToOSI - } else { - direction = "from_osi" - dir = p.Convert.FromOSI + p, err := resolvePlugin(platformName, pluginFlag, cmd.ErrOrStderr()) + if err != nil { + return err } - // --- 4. Parse max-input-size --- + // 3. Select direction + direction, dir := selectDirection(p, from) + + // 4. Parse max-input-size maxBytes, err := parseMaxInputSize(maxInputSizeStr) if err != nil { return fmt.Errorf("invalid --max-input-size %q: %w", maxInputSizeStr, err) } - // --- 5. Collect input files --- + // 5. Collect input files files, err := plugin.CollectFiles(input, dir.Accepts, maxBytes) if err != nil { return fmt.Errorf("input collection failed: %w", err) } - // --- 6. Build context with timeout --- + // 6. Build context with timeout ctx := cmd.Context() if ctx == nil { ctx = context.Background() @@ -119,46 +83,114 @@ func runConvert(cmd *cobra.Command, args []string) error { ctx, cancel := context.WithTimeout(ctx, time.Duration(timeoutSecs)*time.Second) defer cancel() - // --- 7. Route plugin stderr --- + // 7. Route plugin stderr var pluginStderr io.Writer = io.Discard if verbose { pluginStderr = cmd.ErrOrStderr() } - // --- 8. Invoke plugin --- + // 8. Invoke plugin resp, err := plugin.Invoke(ctx, p.Path, dir.Invoke, plugin.Request{Files: files}, pluginStderr) if err != nil { return fmt.Errorf("plugin invocation failed: %w", err) } - // --- 9. Validate output (to_osi direction only) --- - // Schema validation runs before any files are written to disk. - // Files that fail validation are removed from resp.Files and reported - // as error-severity issues. - if direction == "to_osi" { - for name, content := range resp.Files { - if verr := schema.Validate([]byte(content)); verr != nil { - resp.Issues = append(resp.Issues, plugin.Issue{ - Severity: "error", - Path: name, - Message: fmt.Sprintf("output validation failed: %v", verr), - }) - delete(resp.Files, name) - } - } - } + // 9. Validate output + resp.Issues = append(resp.Issues, validateOutput(resp, direction)...) - // --- 10. Write output files --- + // 10. Write output files outDir := outputFlag if outDir == "" { outDir = filepath.Join("ossie-output", p.Platform.Name, direction) } - if len(resp.Files) > 0 { - if err := os.MkdirAll(outDir, 0755); err != nil { - return fmt.Errorf("could not create output directory %q: %w", outDir, err) + if err := writeOutput(resp.Files, outDir); err != nil { + return err + } + + // 11. Render issues and set exit code + if hasErrors := renderIssues(resp.Issues, cmd.ErrOrStderr(), verbose); hasErrors { + return fmt.Errorf("conversion completed with errors") + } + return nil +} + +// resolvePlugin finds the plugin to use for the conversion. +// If pluginFlag is set, it loads from that path directly. Otherwise it +// discovers all installed plugins and filters by platformName. +func resolvePlugin(platformName, pluginFlag string, stderr io.Writer) (*plugin.Plugin, error) { + if pluginFlag != "" { + loaded, err := plugin.LoadPlugin(pluginFlag) + if err != nil { + return nil, fmt.Errorf("could not load plugin from %q: %w", pluginFlag, err) + } + return loaded, nil + } + + pluginsDir, err := ossiedir.PluginDir() + if err != nil { + return nil, err + } + plugins, err := plugin.Discover(pluginsDir, stderr) + if err != nil { + return nil, err + } + + var matches []*plugin.Plugin + for _, candidate := range plugins { + if candidate.Platform.Name == platformName { + matches = append(matches, candidate) } } + switch len(matches) { + case 0: + return nil, fmt.Errorf("no plugin found for %q; run `ossie plugin install %s` to install it", platformName, platformName) + case 1: + return matches[0], nil + default: + return nil, fmt.Errorf("multiple plugins found for %q; use --plugin to specify a path", platformName) + } +} + +// selectDirection returns the direction string ("to_osi" or "from_osi") and +// the corresponding Direction based on whether --from or --to was specified. +func selectDirection(p *plugin.Plugin, from string) (string, plugin.Direction) { + if from != "" { + return "to_osi", p.Convert.ToOSI + } + return "from_osi", p.Convert.FromOSI +} + +// validateOutput validates to_osi response files against the embedded OSI schema. +// Invalid files are removed from resp.Files in place and returned as +// error-severity issues for the caller to append to resp.Issues. +func validateOutput(resp *plugin.Response, direction string) []plugin.Issue { + if direction != "to_osi" { + return nil + } + var issues []plugin.Issue for name, content := range resp.Files { + if verr := schema.Validate([]byte(content)); verr != nil { + issues = append(issues, plugin.Issue{ + Severity: "error", + Path: name, + Message: fmt.Sprintf("output validation failed: %v", verr), + }) + delete(resp.Files, name) + } + } + return issues +} + +// writeOutput writes files to outDir, creating the directory only when there +// are files to write. Subdirectories within outDir are created as needed. +func writeOutput(files map[string]string, outDir string) error { + if len(files) == 0 { + return nil + } + if err := os.MkdirAll(outDir, 0755); err != nil { + return fmt.Errorf("could not create output directory %q: %w", outDir, err) + } + for name, content := range files { dest := filepath.Join(outDir, filepath.FromSlash(name)) if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { return fmt.Errorf("could not create output subdirectory for %q: %w", name, err) @@ -167,11 +199,6 @@ func runConvert(cmd *cobra.Command, args []string) error { return fmt.Errorf("could not write output file %q: %w", dest, err) } } - - // --- 11. Render issues and set exit code --- - if hasErrors := renderIssues(resp.Issues, cmd.ErrOrStderr(), verbose); hasErrors { - return fmt.Errorf("conversion completed with errors") - } return nil } From 5af2f09b3333bad22df5c7b78698be336d39f999 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Wed, 24 Jun 2026 16:12:02 -0500 Subject: [PATCH 09/10] fix(convert): update stale to_osi/from_osi references in convert.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selectDirection and validateOutput were written against the pre-rename field names (ToOSI/FromOSI, to_osi/from_osi). The direction string mismatch in validateOutput was a silent bug — output validation was never reached because "to_ossie" != "to_osi". --- cli/cmd/convert.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/cmd/convert.go b/cli/cmd/convert.go index c61e284..d1dc157 100644 --- a/cli/cmd/convert.go +++ b/cli/cmd/convert.go @@ -151,20 +151,20 @@ func resolvePlugin(platformName, pluginFlag string, stderr io.Writer) (*plugin.P } } -// selectDirection returns the direction string ("to_osi" or "from_osi") and +// selectDirection returns the direction string ("to_ossie" or "from_ossie") and // the corresponding Direction based on whether --from or --to was specified. func selectDirection(p *plugin.Plugin, from string) (string, plugin.Direction) { if from != "" { - return "to_osi", p.Convert.ToOSI + return "to_ossie", p.Convert.ToOssie } - return "from_osi", p.Convert.FromOSI + return "from_ossie", p.Convert.FromOssie } -// validateOutput validates to_osi response files against the embedded OSI schema. +// validateOutput validates to_ossie response files against the embedded OSI schema. // Invalid files are removed from resp.Files in place and returned as // error-severity issues for the caller to append to resp.Issues. func validateOutput(resp *plugin.Response, direction string) []plugin.Issue { - if direction != "to_osi" { + if direction != "to_ossie" { return nil } var issues []plugin.Issue From f47ae2ae3aad3d53538201ccd2e984794b9ee476 Mon Sep 17 00:00:00 2001 From: Quigley Malcolm Date: Wed, 24 Jun 2026 16:46:48 -0500 Subject: [PATCH 10/10] refactor(collect): extract directory walk into collectDir helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CollectFiles was doing two distinct things — single-file read and directory walk — in one flat function body. Extracting the walk into collectDir makes each path easier to follow and test independently. --- cli/internal/plugin/collect.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/internal/plugin/collect.go b/cli/internal/plugin/collect.go index a582503..2098481 100644 --- a/cli/internal/plugin/collect.go +++ b/cli/internal/plugin/collect.go @@ -43,10 +43,14 @@ func CollectFiles(input string, accepts []string, maxBytes int64) (map[string]st acceptSet[strings.ToLower(ext)] = struct{}{} } + return collectDir(input, acceptSet, maxBytes) +} + +func collectDir(root string, acceptSet map[string]struct{}, maxBytes int64) (map[string]string, error) { files := make(map[string]string) var total int64 - err = filepath.WalkDir(input, func(path string, d fs.DirEntry, werr error) error { + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, werr error) error { if werr != nil { return werr } @@ -73,7 +77,7 @@ func CollectFiles(input string, accepts []string, maxBytes int64) (map[string]st } } - rel, err := filepath.Rel(input, path) + rel, err := filepath.Rel(root, path) if err != nil { return err }