diff --git a/README.md b/README.md index e0c40eda..8d04d03d 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ The render farm management space is in an awkward moment. Legacy on-premises sys **Not tied to any cloud provider.** Workers run on Linux, macOS, and Windows — bare metal, VMs, or containers. Cloud compute locations are supported across AWS, GCP, Azure, and any provider that can run a container or a binary. Your control plane runs where you want it. -**OpenJD compatible.** `sqi` adopts the [Open Job Description](https://github.com/OpenJobDescription/openjd-specifications) format as its native job execution layer. Jobs authored for OpenJD-compatible tools work with `sqi` without reformatting. This is a real standard designed for portability, not a proprietary format. +**OpenJD compatible.** `sqi` adopts the [Open Job Description](https://github.com/OpenJobDescription/openjd-specifications) format as its native job execution layer. Jobs authored for OpenJD-compatible tools work with `sqi` without reformatting. This is a real standard designed for portability, not a proprietary format. The one caveat: a template that opts into an OpenJD extension `sqi` does not implement — such as `EXPR` — is rejected by design, rather than being accepted and misinterpreted. See [`docs/openjd-conformance.md`](docs/openjd-conformance.md). **General purpose.** Rendering is the primary use case and the domain `sqi` is designed around, but the job model is general. Any workload expressible as a command with defined inputs, outputs, and environment is a valid `sqi` job — simulation, transcoding, machine learning pipelines, data processing, software development, or anything else a studio runs at scale. diff --git a/ROADMAP.md b/ROADMAP.md index 77b356bc..42c57313 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -98,7 +98,7 @@ Configuration cascades: farm defaults → queue overrides, with retry policy (ma `sqi` adopts the [Open Job Description](https://github.com/OpenJobDescription/openjd-specifications) (OpenJD) format as its native job execution format. **Benefits:** -- Studios authoring jobs for other OpenJD-compatible systems can submit to `sqi` unchanged +- Studios authoring jobs for other OpenJD-compatible systems can submit to `sqi` unchanged, provided the template does not opt into an extension `sqi` has not implemented (e.g. `EXPR`) — those are rejected by design rather than accepted and misinterpreted; see [`docs/openjd-conformance.md`](docs/openjd-conformance.md) - Standardized path mapping, parameter spaces, and execution semantics - Clear separation between job description and job authoring (the product system) @@ -161,11 +161,31 @@ S3-compatible store reachable by the operator's chosen sync tool. ### Path Translation Modes -- **OpenJD** (preferred): Standard path mapping file written into each Session. Applications that support OpenJD natively consume it directly. -- **Resolved**: All paths resolved to concrete paths before command construction. Universal for applications with no path mapping support. -- **Command arg**: Path pairs passed as explicit arguments (e.g., Maya workspace remapping). -- **Environment**: Path mappings via environment variables. -- **Staged**: Pre-job staging to worker-local storage for cloud workers without direct access to source storage. +Path translation rides the `SQI_PATH_TRANSLATION` extension and offers five +delivery mechanisms (deliveries execute in fixed order and are mutually +compatible — a product can declare all five): + +- **`translation_file`** (preferred): Native OpenJD `pathmapping-1.0` file + written into each Session, served via `{{Session.PathMappingRulesFile}}`. + Applications that support OpenJD path mapping natively consume it directly. +- **`swap_in_place`**: String substitution of path parameters in the template. + Universal for applications with no path-mapping support. sqi convenience, + not in the OpenJD spec. +- **`command_flags`**: Individual `src`/`dest` pairs appended as command-line + flags (e.g., Maya workspace remapping). +- **`environment`**: Path mappings delivered via an environment variable. +- **`stage_locally`**: Job-level PATH parameters staged to worker-local + scratch before the run and copied back after, for cloud workers without + direct access to source storage. Works with no worker configuration — an + unconfigured worker falls back to a TEMP scratch directory and sqi's own + built-in copy — but a farm spanning multiple compute locations needs an + explicit `staging.scratch_dir` and `staging.sync_command` + (`rsync`/`aws-cli`/etc.) for real remote transfer. + +`swap_in_place` and `translation_file` are the default when no +`SQI_PATH_TRANSLATION` extension is declared. Full reference: +[`docs/products.md`](docs/products.md#path-translation) and +[`docs/openjd-extensions/path-translation.md`](docs/openjd-extensions/path-translation.md). ### What `sqi` does not do @@ -221,7 +241,7 @@ NATS can run embedded within `sqi-server` (simple mode) or as a separate cluster - Product/preset definition system (YAML/JSON) — a thin catalog over OpenJD templates, with embedded Script/Python/Container built-ins - Preset library integration — static JSON index at a configurable URL (default: official community library on GitHub Pages); browse presets in the Admin hub with per-preset status (not installed / installed / update available); preview the definition and install as a product (`source: installed`) in one click; SHA-256 integrity and update-detection check on install; read-only installed products, uninstallable, with Duplicate-to-custom available on every product - Web UI product management editor and a product-driven submission form (parameter form generated from the selected product) -- Additional path translation modes (resolved, command-arg, environment, staged) as the `SQI_PATH_TRANSLATION` vendor extension +- Path translation deliveries (`swap_in_place`, `translation_file`, `command_flags`, `environment`, `stage_locally`) as the `SQI_PATH_TRANSLATION` vendor extension - S3-compatible storage support (thin layer: derived type, root validation, path staging via operator sync tool) - DCC submitter framework — in-application submitters for Maya, Houdini, Nuke, and Blender (the `sqi-submitter` Python package), built on the Python client - Compute location registry and step-level affinity (native OpenJD `attr.worker.computelocation`) diff --git a/clients/python/src/sqi_client/models.py b/clients/python/src/sqi_client/models.py index 546102d6..7e6b1ec6 100644 --- a/clients/python/src/sqi_client/models.py +++ b/clients/python/src/sqi_client/models.py @@ -41,6 +41,7 @@ "LogPage", "Page", "ParameterUserInterface", + "PathFileFilter", "Principal", "Product", "ProductParameter", @@ -227,10 +228,6 @@ def _as_bool(value: Any) -> bool: return value if isinstance(value, bool) else False -def _opt_bool(value: Any) -> bool | None: - return value if isinstance(value, bool) else None - - def _str_dict(value: Any) -> dict[str, str]: if not isinstance(value, dict): return {} @@ -1222,7 +1219,6 @@ class ParameterUserInterface: label: str = "" group_label: str = "" decimals: int | None = None - single_step_removal: bool | None = None @classmethod def from_dict(cls, data: Mapping[str, Any]) -> ParameterUserInterface: @@ -1237,7 +1233,27 @@ def from_dict(cls, data: Mapping[str, Any]) -> ParameterUserInterface: label=_as_str(data.get("label")), group_label=_as_str(data.get("group_label")), decimals=_opt_int(data.get("decimals")), - single_step_removal=_opt_bool(data.get("single_step_removal")), + ) + + +@dataclass(frozen=True) +class PathFileFilter: + """One named file type offered by a PATH parameter's chooser dialog.""" + + label: str = "" + patterns: list[str] = field(default_factory=list) + + @classmethod + def from_dict(cls, data: Mapping[str, Any]) -> PathFileFilter: + """Build an instance from a decoded JSON response object. + + Unknown fields are ignored and missing or mistyped fields fall back to + type-appropriate defaults; see the module docstring for the full + tolerant-parsing contract. + """ + return cls( + label=_as_str(data.get("label")), + patterns=_str_list(data.get("patterns")), ) @@ -1257,6 +1273,8 @@ class ProductParameter: object_type: str = "" data_flow: str = "" user_interface: ParameterUserInterface | None = None + file_filters: list[PathFileFilter] | None = None + file_filter_default: PathFileFilter | None = None @classmethod def from_dict(cls, data: Mapping[str, Any]) -> ProductParameter: @@ -1267,6 +1285,8 @@ def from_dict(cls, data: Mapping[str, Any]) -> ProductParameter: tolerant-parsing contract. """ ui = data.get("user_interface") + raw_filters = data.get("file_filters") + default_filter = data.get("file_filter_default") return cls( name=_as_str(data.get("name")), type=_as_str(data.get("type")), @@ -1286,6 +1306,16 @@ def from_dict(cls, data: Mapping[str, Any]) -> ProductParameter: user_interface=( ParameterUserInterface.from_dict(ui) if isinstance(ui, Mapping) else None ), + file_filters=( + [PathFileFilter.from_dict(f) for f in raw_filters if isinstance(f, dict)] + if isinstance(raw_filters, list) + else None + ), + file_filter_default=( + PathFileFilter.from_dict(default_filter) + if isinstance(default_filter, Mapping) + else None + ), ) diff --git a/clients/python/tests/test_models.py b/clients/python/tests/test_models.py index db44cc7a..76368e82 100644 --- a/clients/python/tests/test_models.py +++ b/clients/python/tests/test_models.py @@ -688,3 +688,45 @@ def test_product_parameter_from_dict_without_user_interface() -> None: param = ProductParameter.from_dict({"name": "Frames", "type": "INT"}) assert param.user_interface is None assert param.default is None + + +def test_product_parameter_from_dict_with_file_filters() -> None: + from sqi_client.models import ProductParameter + + param = ProductParameter.from_dict( + { + "name": "SceneFile", + "type": "PATH", + "file_filters": [ + {"label": "Maya Scene", "patterns": ["*.ma", "*.mb"]}, + {"label": "All Files", "patterns": ["*"]}, + ], + "file_filter_default": {"label": "Maya Scene", "patterns": ["*.ma", "*.mb"]}, + } + ) + assert param.file_filters is not None + assert [f.label for f in param.file_filters] == ["Maya Scene", "All Files"] + assert param.file_filters[0].patterns == ["*.ma", "*.mb"] + assert param.file_filter_default is not None + assert param.file_filter_default.label == "Maya Scene" + assert param.file_filter_default.patterns == ["*.ma", "*.mb"] + + +def test_product_parameter_from_dict_with_null_file_filters() -> None: + """The server marshals a nil slice/pointer as JSON null, not an absent key.""" + from sqi_client.models import ProductParameter + + param = ProductParameter.from_dict( + {"name": "SceneFile", "type": "PATH", "file_filters": None, "file_filter_default": None} + ) + assert param.file_filters is None + assert param.file_filter_default is None + + +def test_path_file_filter_from_dict() -> None: + from sqi_client.models import PathFileFilter + + f = PathFileFilter.from_dict({"label": "Maya Scene", "patterns": ["*.ma", "*.mb"], "extra": 1}) + assert f == PathFileFilter(label="Maya Scene", patterns=["*.ma", "*.mb"]) + # Missing fields fall back to type defaults. + assert PathFileFilter.from_dict({}) == PathFileFilter(label="", patterns=[]) diff --git a/clients/python/tests/test_products.py b/clients/python/tests/test_products.py index b1ce04c6..74647ecd 100644 --- a/clients/python/tests/test_products.py +++ b/clients/python/tests/test_products.py @@ -74,7 +74,13 @@ def test_get_product_parameters(make_client: ClientFactory) -> None: "type": "STRING", "default": "final", "user_interface": {"control": "DROPDOWN_LIST", "label": "Quality"}, - } + }, + { + "name": "SceneFile", + "type": "PATH", + "file_filters": [{"label": "Blender Scene", "patterns": ["*.blend"]}], + "file_filter_default": {"label": "Blender Scene", "patterns": ["*.blend"]}, + }, ], ) ) @@ -83,6 +89,10 @@ def test_get_product_parameters(make_client: ClientFactory) -> None: assert params[0].name == "Quality" assert params[0].user_interface is not None assert params[0].user_interface.control == "DROPDOWN_LIST" + assert params[1].file_filters is not None + assert params[1].file_filters[0].label == "Blender Scene" + assert params[1].file_filter_default is not None + assert params[1].file_filter_default.patterns == ["*.blend"] @respx.mock diff --git a/clients/submitter/src/sqi_submitter/core/schema.py b/clients/submitter/src/sqi_submitter/core/schema.py index 43510568..cb7eab30 100644 --- a/clients/submitter/src/sqi_submitter/core/schema.py +++ b/clients/submitter/src/sqi_submitter/core/schema.py @@ -29,10 +29,12 @@ def widget(self) -> str: ui = self.parameter.user_interface control = ui.control if (ui is not None and ui.control) else "" t = self.parameter.type - # PATH is type-first: OpenJD has no picker control, so LINE_EDIT (the - # only legal control that can carry a label on a path) must not suppress - # the derived picker. An explicit non-LINE_EDIT control (e.g. HIDDEN) - # still wins. See docs/dcc-submitters.md. + # PATH is type-first: a PATH parameter with no control (or a stale + # LINE_EDIT — no longer a legal control on PATH server-side, but + # tolerated here defensively) derives the picker instead of falling + # back to a plain text field. An explicit CHOOSE_* control falls + # through to the `if control` branch below and yields the same + # result; an explicit HIDDEN still wins. See docs/dcc-submitters.md. if t == "PATH" and control in ("", "LINE_EDIT"): if self.parameter.object_type == "DIRECTORY": return "CHOOSE_DIRECTORY" diff --git a/clients/submitter/tests/test_schema.py b/clients/submitter/tests/test_schema.py index 7a4be2ac..34f3d2d0 100644 --- a/clients/submitter/tests/test_schema.py +++ b/clients/submitter/tests/test_schema.py @@ -118,8 +118,9 @@ def test_form_field_is_scene_path() -> None: @pytest.mark.parametrize( ("param", "widget"), [ - # PATH is type-first: a LINE_EDIT control (the only legal way to carry a - # label on a path) must not suppress the derived picker. + # PATH is type-first: an absent control, or a stale LINE_EDIT (no + # longer a legal control on PATH server-side, but tolerated here + # defensively), must not suppress the derived picker. (_p(type_="PATH", ui={"control": "LINE_EDIT"}), "CHOOSE_INPUT_FILE"), ( _p(type_="PATH", object_type="DIRECTORY", ui={"control": "LINE_EDIT"}), @@ -139,11 +140,14 @@ def test_path_is_type_first(param: ProductParameter, widget: str) -> None: def test_labeled_path_keeps_label() -> None: + # CHOOSE_DIRECTORY is the server-valid control for a labeled directory + # PATH parameter (LINE_EDIT is not legal on PATH); it falls through to + # `if control: return control` and keeps the label alongside it. param = _p( name="OutputDir", type_="PATH", object_type="DIRECTORY", - ui={"control": "LINE_EDIT", "label": "Output Directory"}, + ui={"control": "CHOOSE_DIRECTORY", "label": "Output Directory"}, ) field = FormModel.from_parameters([param]).fields[0] assert field.widget == "CHOOSE_DIRECTORY" diff --git a/docs/architecture.md b/docs/architecture.md index 598e5e13..07ce2375 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -238,7 +238,7 @@ never changes scheduling, only an annotation. A task that has waited longer than `scheduler.unschedulable_grace` with no eligible online worker is flagged with a human-readable `unschedulable_reason`, cleared automatically once a matching worker appears or the task leaves `ready`. See -[`docs/observability.md`](observability.md#why-isnt-my-job-running-unschedulable-tasks) +[`docs/observability.md`](observability.md#why-isnt-my-job-running--unschedulable-tasks) for the operator-facing view and [`scheduler.unschedulable_grace`](configuration.md#schedulerunschedulable_grace) for the config knob. @@ -442,7 +442,7 @@ sweeps); none of them route through `UpdateTaskStatus`. A worker-reported `failed` status no longer routes straight to a terminal state. The scheduler (`internal/scheduler/failure.go`) resolves the effective -[retry policy](configuration.md#retry-failure-limits) (Job → Queue → Farm → +[retry policy](configuration.md#retry--failure-limits) (Job → Queue → Farm → server default) and records the genuine failure via `store.RecordTaskFailure`, then picks one of three outcomes: diff --git a/docs/configuration.md b/docs/configuration.md index 4550027f..64609e3a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -408,7 +408,7 @@ scheduler: ``` See -[Why isn't my job running? — Unschedulable tasks](observability.md#why-isnt-my-job-running-unschedulable-tasks) +[Why isn't my job running? — Unschedulable tasks](observability.md#why-isnt-my-job-running--unschedulable-tasks) for what the flag means, where it surfaces (task `unschedulable_reason`, job `task_counts.unschedulable`, the job-detail badge), and how it clears. @@ -1169,7 +1169,7 @@ staging: Full detail, including the built-in copy's local/dev caveat and the `staging.defaults` behavior change, is in -[Worker configuration → `staging`](worker-configuration.md#staging-local-path-staging-stage_locally-delivery). +[Worker configuration → `staging`](worker-configuration.md#staging--local-path-staging-stage_locally-delivery). ### Diagnostics (`diagnostics.enabled`) diff --git a/docs/dcc-submitters.md b/docs/dcc-submitters.md index 7c0a8ecb..23366c50 100644 --- a/docs/dcc-submitters.md +++ b/docs/dcc-submitters.md @@ -339,16 +339,17 @@ Blender scatter files into the worker's working directory. This differs from Maya's `OutputDir`, which is a real output *directory* (`Render -rd`); Blender's `-o` is a full path prefix, not a folder. -**`PATH` is type-first.** OpenJD has no file/directory *picker* control, and the -base spec requires a `control` whenever a `userInterface` (hence a `label`) is -set — so a labeled path parameter must declare `control: LINE_EDIT`. The -submitter clients treat `PATH` as type-first: a `LINE_EDIT` (or absent) control -does **not** suppress the picker. The Qt dialog derives the picker -(`CHOOSE_DIRECTORY` / `CHOOSE_OUTPUT_FILE` / `CHOOSE_INPUT_FILE`, from -`objectType`/`dataFlow`) and honors the label, so a labeled path shows a browse -button *and* its label. An explicit `HIDDEN` still wins. Templates stay valid -OpenJD; the picker is a client rendering affordance never represented in the -document. (The web UI renders paths as a labeled text field — a browser cannot +**`PATH` has real file/directory picker controls.** A `PATH` parameter's +`userInterface.control` must be one of `CHOOSE_INPUT_FILE`, +`CHOOSE_OUTPUT_FILE`, `CHOOSE_DIRECTORY`, `DROPDOWN_LIST`, or `HIDDEN` — +`LINE_EDIT` is **not** legal there and the server rejects it with a 422. A +labeled path parameter declares the `CHOOSE_*` control matching its +`objectType`/`dataFlow`. The Qt dialog honors that control directly, so a +labeled path shows a browse button *and* its label; an explicit `HIDDEN` +suppresses the picker. Full detail, including the fallback for a `PATH` +parameter with no `userInterface` block at all, is in +[PATH parameters, labels, and pickers](#path-parameters-labels-and-pickers) +below. (The web UI renders paths as a labeled text field — a browser cannot browse the farm filesystem — and the Blender panel uses a text property.) A selected render target can also supply **exact-name** extras that aren't @@ -383,32 +384,42 @@ Rules that make this a stable contract: ### PATH parameters, labels, and pickers -This repo's OpenJD `userInterface` control enum (see -[`docs/openjd-extensions.md`](openjd-extensions.md) and -[`docs/products.md`](products.md#userinterface-parameter-hints)) has **no -file-chooser control** — the valid values are `LINE_EDIT`, `MULTILINE_EDIT`, -`DROPDOWN_LIST`, `CHECK_BOX`, `CHIP_INPUT`, `HIDDEN`, and `SPIN_BOX`. There is -no `CHOOSE_INPUT_FILE`/`CHOOSE_OUTPUT_FILE`/`CHOOSE_DIRECTORY` a template -author can declare, and a `userInterface` block that is present must carry a -`control` (the server rejects one without it). - -Because of that, every reference preset labels its `PATH` parameters with -`control: LINE_EDIT` — the only control that can carry a `label` on a path — -and the Qt dialog **still** renders a real chooser for them, because it treats -`PATH` as type-first (see [The scene file is host-managed](#the-scene-file-is-host-managed) -above): a `LINE_EDIT` (or absent) control does not suppress the picker. So a -Qt-hosted artist gets a browse button *and* a friendly label. The picker -variant follows the parameter's `objectType`/`dataFlow` (`objectType: -DIRECTORY` → folder picker, `dataFlow: OUT` → save dialog, otherwise an -open-file dialog). An explicit non-`LINE_EDIT` control (e.g. `HIDDEN`) wins -over the type fallback. +This repo's OpenJD `userInterface` control vocabulary is scoped per parameter +type (see [`docs/openjd-extensions.md`](openjd-extensions.md) and +[`docs/products.md`](products.md#userinterface-parameter-hints)). On a `PATH` +parameter the legal controls are `CHOOSE_INPUT_FILE`, `CHOOSE_OUTPUT_FILE`, +`CHOOSE_DIRECTORY`, `DROPDOWN_LIST`, and `HIDDEN` — `LINE_EDIT`, +`MULTILINE_EDIT`, `CHECK_BOX`, and `SPIN_BOX` belong to other parameter types +and the server rejects them on a `PATH` with a 422. A `userInterface` block +that is present must still carry a `control` (the server rejects one +without it). + +Every reference preset labels its `PATH` parameters with the `CHOOSE_*` +control matching the parameter's `objectType`/`dataFlow` — `CHOOSE_INPUT_FILE` +for an input like `SceneFile`, `CHOOSE_DIRECTORY` for a directory-typed +output, `CHOOSE_OUTPUT_FILE` for a file-typed output — and the Qt dialog +renders a real chooser straight from the declared control, honoring the label +alongside it. So a Qt-hosted artist gets a browse button *and* a friendly +label, both read directly off the template with no client-side guessing. The +picker variant follows `objectType`/`dataFlow` (`objectType: DIRECTORY` → +folder picker, `dataFlow: OUT` → save dialog, otherwise an open-file dialog); +an explicit `HIDDEN` suppresses the picker. + +A `PATH` parameter can also carry **no** `userInterface` block at all (so no +control and no label). The submitter clients still treat `PATH` as +type-first in that case (see [The scene file is host-managed](#the-scene-file-is-host-managed) +above): they derive the same `CHOOSE_*` picker from `objectType`/`dataFlow` +rather than falling back to a plain text field the way an unlabeled `STRING` +would. This fallback lives in `FormField.widget` in +`clients/submitter/src/sqi_submitter/core/schema.py`. The chooser is a **Qt-only** rendering affordance, never represented in the -OpenJD document. The web submission form has no equivalent — `selectWidget` -in `web/src/lib/productForm.ts` renders a `PATH` parameter as a plain labeled -text field (a browser cannot browse the farm filesystem) — and the Blender -panel uses a `bpy` string property. All three read the same `LINE_EDIT` + -`label` hint; only Qt adds the browse button on top of it. +OpenJD document beyond the declared control. The web submission form has no +equivalent — `selectWidget` in `web/src/lib/productForm.ts` renders a `PATH` +parameter as a plain labeled text field (a browser cannot browse the farm +filesystem) — and the Blender panel uses a `bpy` string property. All three +read the same `control` + `label` hint; only Qt adds the browse button on top +of it. --- diff --git a/docs/observability.md b/docs/observability.md index 520c0a8b..d4bc5bbb 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -178,7 +178,7 @@ itself is empty. `sqi-server` exposes two Prometheus counters (subsystem `scheduler`, served at `GET /metrics`) for the auto-retry + failure-limit feature — see [`scheduler.default_max_attempts` / `retry_delay` / `default_failure_limit` -and the Server → Farm → Queue → Job precedence](configuration.md#retry-failure-limits) +and the Server → Farm → Queue → Job precedence](configuration.md#retry--failure-limits) for the policy that drives them, and [the task state machine](architecture.md#state-machine-task-status) for how retry, exhaustion, and auto-park interact. diff --git a/docs/openjd-conformance.md b/docs/openjd-conformance.md new file mode 100644 index 00000000..a3209e7c --- /dev/null +++ b/docs/openjd-conformance.md @@ -0,0 +1,147 @@ +# OpenJD Spec Conformance + +This page states, in one place, exactly how far `sqi`'s OpenJD support goes: which +spec version it implements, where that's enforced in code, which extensions it +understands, and — just as importantly — what it deliberately does not implement +and why that is correct rather than a gap. Read this before re-auditing OpenJD +conformance from scratch. + +## Spec version + +`sqi` implements **`jobtemplate-2023-09`** — the OpenJD job template schema. A +template's `specificationVersion` field must equal that string exactly; anything +else (missing, a different version string) is a `422` validation error. + +`sqi` does **not** implement the standalone `environment-2023-09` template type — a +second, separate top-level document format in the OpenJD spec for defining +reusable environments outside a job template. `sqi` only ever parses +`jobtemplate-2023-09` documents; environments are supported only as they appear +embedded in a job template (`jobEnvironments`, `stepEnvironments`). This is a +missing feature, not a violated rule — nothing in the spec requires an +implementation to support both template types. + +## Where validation lives + +The pipeline is parse, then validate, then expand: + +1. **`internal/openjd/parse.go`** (`Parse`) — decodes YAML or JSON into a + `*JobTemplate` (`internal/openjd/model.go`). Parsing is strict about shape: + a field that must be a scalar but arrives as a mapping or sequence is a parse + error, not a silently-wrong value. +2. **`internal/openjd/validate.go`** (`Validate`, `ValidateWithOptions`) — walks + the parsed template and returns zero or more `ValidationError`s, each a JSON + Pointer (RFC 6901) to the offending field plus a message. `Validate(t)` is + `ValidateWithOptions(t, ValidateOptions{EnforceLimits: true})`; the submission + pipeline can run with `EnforceLimits: false` (quantitative caps such as range + size relaxed) while every *structural* correctness check — required fields, + dependency resolution, extension gating, host-requirement shape — still runs + unconditionally. A structural check that only fired under `EnforceLimits: true` + would silently vanish for any caller that flips the flag; that class of bug is + exactly what Task 4 of this cycle closed. +3. **`internal/openjd/expand.go`** — turns a validated template's parameter + spaces into concrete tasks. + +A `422 Unprocessable Entity` from `POST /api/v1/jobs` carries a `detail` string +built from `ValidationErrors.Error()` — one or more `: ` entries +joined with `; `. See [`docs/openjd-submission.md`](openjd-submission.md#validation-errors) +for a real captured example. + +## Supported extensions and the vendor-prefix rule + +OpenJD extensions are **opt-in**: a template lists the ones it needs in its +top-level `extensions: [...]` array. `sqi` validates that array unconditionally +(not gated by `EnforceLimits`) against a fixed registry +(`internal/openjd/extension.go`, `LookupExtension`): + +| Extension | Origin | What it does | +|---|---|---| +| `TASK_CHUNKING` | official | Chunked integer task parameters (`CHUNK[INT]`). See [`docs/openjd-extensions/task-chunking.md`](openjd-extensions/task-chunking.md). | +| `REDACTED_ENV_VARS` | official | The `openjd_redacted_env` stdout directive that redacts a variable's value from logs. See [`docs/openjd-extensions/redacted-env-vars.md`](openjd-extensions/redacted-env-vars.md). | +| `SQI_PATH_TRANSLATION` | vendor | Per-product path-delivery checklist (`swap_in_place`/`translation_file`/`command_flags`/`environment`/`stage_locally`). See [`docs/openjd-extensions/path-translation.md`](openjd-extensions/path-translation.md). | +| `SQI_CHUNK_BOUNDS` | vendor | Exposes a `CHUNK[INT]` chunk's first/last integer as `Task.Param..Start`/`.End`. See [`docs/openjd-extensions/sqi-chunk-bounds.md`](openjd-extensions/sqi-chunk-bounds.md). | + +Every **vendor** extension (`Origin: OriginVendor` — one `sqi` defines itself, +as opposed to one specified upstream by OpenJD) name **must** carry the `SQI_` +prefix. This is enforced by a test invariant +(`internal/openjd/extension_test.go`), not just convention: it guarantees a +vendor extension name can never collide with a future official OpenJD name, +which are always bare identifiers (e.g. `TASK_CHUNKING`, not `SQI_TASK_CHUNKING`). +If a vendor extension is later upstreamed into the OpenJD spec, the promotion +path is to drop the `SQI_` prefix and flip its `Origin` to `OriginOfficial` — the +registry entry moves, it doesn't get a second copy. + +Declaring an extension name not present in the registry is a `422` at +`/extensions/{i}`, regardless of `EnforceLimits`. Declaring `CHUNK[INT]` without +`TASK_CHUNKING` in `extensions` is likewise rejected — the extension gate and the +feature it gates are checked together. + +## What `sqi` deliberately does not implement + +Two things, both **out of scope by design**, not bugs to fix: + +- **`EXPR`** (the OpenJD Expression Language extension) — new value types + (`BOOL`, `RANGE_EXPR`, `LIST[...]`), `let` on step templates, and the + `*_LIST` `userInterface.control` variants all belong to `EXPR` and are not + implemented. A template that declares `extensions: [EXPR]` is rejected with a + `422` at `/extensions/0` — unconditionally, the same as any other unregistered + extension name. +- **Standalone `environment-2023-09` templates** — see [Spec version](#spec-version) + above. + +**Why rejecting an unimplemented opt-in extension is correct, not a gap:** +OpenJD extensions exist precisely so a template can declare "I need this +capability" and a conformant implementation that lacks it can say "then I can't +run you" instead of guessing. Accepting a template whose syntax `sqi` cannot +interpret — silently ignoring the parts it doesn't understand — is strictly +worse than refusing it outright: the alternative is a job that appears to +submit successfully and then does something other than what the template +author asked for, discovered only when the render is wrong. The spec does not +mandate rejection of an unimplemented extension either way; `sqi` chooses to +reject because a loud `422` at submission time is a better failure mode than a +silent misinterpretation at run time. + +This is also why the `README.md`/`ROADMAP.md` claim that jobs authored for +other OpenJD-compatible tools work with `sqi` "without reformatting" carries an +explicit caveat rather than being dropped: the claim is true for the base spec +and every extension `sqi` implements, and false only for the extensions listed +above — which a template must opt into by name to hit. + +## Adding a deliberate divergence + +If `sqi` ever needs to diverge from strict spec conformance on purpose — the +concrete example on the table right now is restoring something like the +non-standard `CHIP_INPUT` parameter control that Task 9 of this cycle removed +— the route is the extension mechanism above, not a silent parser change: + +1. A registry entry in `internal/openjd/extension.go` naming the divergence, + e.g. `SQI_UI_CONTROLS`, with `Origin: OriginVendor` (so the `SQI_` prefix is + mandatory and enforced by the test invariant). +2. Parse/validate support in `internal/openjd` gated on that extension being + declared — the divergent behavior only activates for a template that opts + in by name. +3. A doc under `docs/openjd-extensions/`, following the shape of the existing + entries (motivation, schema, validation, worker behavior). +4. Any worker-side behavior the divergence needs, under `internal/worker/`. + +`SQI_UI_CONTROLS` is not built — nothing declares it, nothing depends on it. +It is recorded here only as the shape a future divergence would take: opt-in, +named, registered, documented — never a bare change to what the base spec's +syntax means. + +## Verifying documentation examples against the real parser + +Every OpenJD YAML/JSON example in `docs/openjd-submission.md` is verified by +actually parsing and validating it — via `openjd.Parse` + +`openjd.Validate` — not by inspection. `internal/openjd`'s existing +`TestParse*` suite exercises the parser broadly; when correcting a specific +documented example, the fastest way to confirm it is a throwaway +`_test.go` file in `internal/openjd` that parses the exact YAML/JSON block +verbatim and asserts zero validation errors, run once, then deleted before +committing — it is not meant to become a permanent regression test. Two real +bugs surfaced this way during this conformance cycle that a read-through would +have missed: the step-level dependency key is `dependencies:` (a list of +`{dependsOn: }`), not a bare `dependsOn:` list of `{stepName: }` +as earlier documentation showed; and an explicit-but-empty +`hostRequirements: {}` is now rejected (only *omitting* the key reserves the +whole machine) now that host-requirement structural checks run +unconditionally. diff --git a/docs/openjd-extensions/path-translation.md b/docs/openjd-extensions/path-translation.md index 5b99a374..3204778b 100644 --- a/docs/openjd-extensions/path-translation.md +++ b/docs/openjd-extensions/path-translation.md @@ -58,7 +58,7 @@ worker falls back to a TEMP scratch directory (`/sqi-staging`) and sqi's own built-in copy in place of a shell `sync_command`, logging a one-time WARN the first time it does so (`staging.defaults`, on by default — see -[Worker configuration → `staging`](../worker-configuration.md#staging-local-path-staging-stage_locally-delivery)). +[Worker configuration → `staging`](../worker-configuration.md#staging--local-path-staging-stage_locally-delivery)). Set `staging.defaults: false` to restore the previous fail-hard behavior for an unconfigured worker. diff --git a/docs/openjd-submission.md b/docs/openjd-submission.md index df53c752..971b3dc2 100644 --- a/docs/openjd-submission.md +++ b/docs/openjd-submission.md @@ -12,7 +12,9 @@ This document shows three progressively richer examples: shared parameters, and step-level environments. See [`docs/api.md`](api.md) for the HTTP mechanics (endpoint, query parameters, -response shape, error handling). +response shape, error handling). See +[`docs/openjd-conformance.md`](openjd-conformance.md) for which spec version +and extensions `sqi` implements, and what it deliberately does not. --- @@ -40,8 +42,8 @@ steps: # at least one required type: INT | FLOAT | STRING | PATH range: combination: # optional; default is Cartesian product of all params - dependsOn: # optional; list of step names that must complete first - - stepName: + dependencies: # optional; list of step dependencies + - dependsOn: # name of a step that must complete first stepEnvironments: [...] # optional; per-step environment blocks jobEnvironments: [...] # optional; applied to every step ``` @@ -128,11 +130,16 @@ steps: range: "{{Param.StartFrame}}-{{Param.EndFrame}}" ``` -Submit with explicit parameter values: +Submit with explicit parameter values. Job-level parameters without a +`default` (`SceneName` and `OutputDir` below) must be supplied on the query +string as `param.=`; omitting a required parameter is a +**422**. See the OpenAPI spec (`internal/api/openapi.yaml`, served at +`GET /api/v1/openapi.yaml`) for the full `param.*` rules (type checking, +`allowedValues`, etc.): ```sh curl -s -X POST \ - "http://localhost:8080/api/v1/jobs?farm_id=$FARM_ID&queue_id=$QUEUE_ID&owner=alice&priority=70" \ + "http://localhost:8080/api/v1/jobs?farm_id=$FARM_ID&queue_id=$QUEUE_ID&owner=alice&priority=70¶m.SceneName=hero_shot¶m.OutputDir=/renders/hero_shot" \ -H "Content-Type: application/x-yaml" \ --data-binary ' specificationVersion: "jobtemplate-2023-09" @@ -283,8 +290,8 @@ steps: # ── Step 2: Composite ───────────────────────────────────────────────────── # Runs only after every Render task succeeds. - name: Composite - dependsOn: - - stepName: Render + dependencies: + - dependsOn: Render script: actions: onRun: @@ -305,8 +312,8 @@ steps: # ── Step 3: Encode ──────────────────────────────────────────────────────── # Runs after Composite completes. Single task — no parameter space. - name: Encode - dependsOn: - - stepName: Composite + dependencies: + - dependsOn: Composite script: actions: onRun: @@ -428,8 +435,9 @@ do not declare CPU requirements never oversubscribe a host. Example — a 4-core worker running one undeclared task vs. four 1-core tasks: ```yaml -# One task at a time (reserves whole machine): -hostRequirements: {} # or omit hostRequirements entirely +# One task at a time (reserves whole machine): omit hostRequirements entirely. +# An explicit-but-empty `hostRequirements: {}` is rejected — a present +# hostRequirements block must declare at least one amount or attribute. # Four tasks in parallel on a 4-core worker: hostRequirements: @@ -470,11 +478,16 @@ the offending field: "type": "about:blank", "title": "Unprocessable Entity", "status": 422, - "detail": "step 'Composite' dependsOn unknown step 'Rendur'", + "detail": "/steps/1/dependencies/0/dependsOn: references unknown step \"Rendur\"", "instance": "a1b2c3d4e5f60708" } ``` +(Captured from the real validator: a step named `Composite` at index 1 whose +`dependencies[0].dependsOn` names a step `Rendur` that does not exist in the +template. Multiple validation failures are joined with `; ` in a single +`detail` string.) + Common validation failures: - `specificationVersion` missing or not `"jobtemplate-2023-09"` diff --git a/docs/preset-library.md b/docs/preset-library.md index 01413d5b..536ed2cc 100644 --- a/docs/preset-library.md +++ b/docs/preset-library.md @@ -57,7 +57,7 @@ signature — the trust boundary is the configured index URL itself. ## Configuration -See [`docs/configuration.md`](configuration.md#preset_library-remote-preset-catalog) +See [`docs/configuration.md`](configuration.md#preset_library--remote-preset-catalog) for the full reference. The short version: | Key | Default | Env var | diff --git a/docs/products.md b/docs/products.md index 2beda292..95e1ed6f 100644 --- a/docs/products.md +++ b/docs/products.md @@ -288,8 +288,7 @@ Response: `200 OK`, array of `ProductParameter` objects in template order: "control": "LINE_EDIT", "label": "Interpreter", "group_label": "", - "decimals": null, - "single_step_removal": null + "decimals": null } } ] @@ -365,8 +364,11 @@ It rides the `SQI_PATH_TRANSLATION` extension and offers five delivery mechanism variables. 5. **`stage_locally`** — Copy job-level PATH parameters with `dataFlow` IN/INOUT to worker-local scratch before the run, and copy OUT/INOUT back after (sqi - convenience, not in OpenJD spec). Requires operator configuration of - `staging.scratch_dir` and `staging.sync_command` on the worker. + convenience, not in OpenJD spec). Works with no worker configuration — an + unconfigured worker falls back to a TEMP scratch directory and sqi's own + built-in copy — but a farm spanning multiple compute locations needs an + explicit `staging.scratch_dir` and `staging.sync_command` on the worker for + real remote transfer. Deliveries execute in fixed order and are mutually compatible — a product can declare all five simultaneously. The first two (`swap_in_place`, `translation_file`) diff --git a/docs/worker-capabilities.md b/docs/worker-capabilities.md index 6f8a642d..0724b52e 100644 --- a/docs/worker-capabilities.md +++ b/docs/worker-capabilities.md @@ -238,7 +238,7 @@ capabilities as part of its larger config-and-capabilities summary. Custom detectors are configured under `capabilities.detect` in the worker config file (see -[`docs/worker-configuration.md`](worker-configuration.md#capabilities-software-auto-detection) +[`docs/worker-configuration.md`](worker-configuration.md#capabilities--software-auto-detection) and [`config/sqi-worker.example.yaml`](https://github.com/uberware/sqi/blob/main/config/sqi-worker.example.yaml)). They use the exact same schema as the built-ins diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 25fc945d..a6e726a1 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -1429,11 +1429,26 @@ components: nullable: true type: object properties: - control: { type: string, enum: [LINE_EDIT, MULTILINE_EDIT, DROPDOWN_LIST, CHECK_BOX, CHIP_INPUT, HIDDEN, SPIN_BOX] } + control: { type: string, enum: [LINE_EDIT, MULTILINE_EDIT, DROPDOWN_LIST, CHECK_BOX, HIDDEN, SPIN_BOX, CHOOSE_INPUT_FILE, CHOOSE_OUTPUT_FILE, CHOOSE_DIRECTORY] } label: { type: string } group_label: { type: string } decimals: { type: integer, nullable: true } - single_step_removal: { type: boolean, nullable: true } + file_filters: + nullable: true + type: array + items: + type: object + required: [label, patterns] + properties: + label: { type: string } + patterns: { type: array, items: { type: string } } + file_filter_default: + nullable: true + type: object + required: [label, patterns] + properties: + label: { type: string } + patterns: { type: array, items: { type: string } } # ── Usage pools ─────────────────────────────────────────────────────────── UsagePool: diff --git a/internal/api/products.go b/internal/api/products.go index 1c639edf..35ba9eb9 100644 --- a/internal/api/products.go +++ b/internal/api/products.go @@ -74,28 +74,35 @@ func toProductResponse(p store.Product) productResponse { // parameterUserInterfaceResponse mirrors openjd.ParameterUserInterface for the // GET /products/{name}/parameters response. type parameterUserInterfaceResponse struct { - Control string `json:"control"` - Label string `json:"label"` - GroupLabel string `json:"group_label"` - Decimals *int `json:"decimals"` - SingleStepRemoval *bool `json:"single_step_removal"` + Control string `json:"control"` + Label string `json:"label"` + GroupLabel string `json:"group_label"` + Decimals *int `json:"decimals"` +} + +// pathFileFilterResponse mirrors openjd.PathFileFilter on the wire. +type pathFileFilterResponse struct { + Label string `json:"label"` + Patterns []string `json:"patterns"` } // productParameterResponse is one parsed job parameter, including userInterface // hints, returned by GET /products/{name}/parameters. type productParameterResponse struct { - Name string `json:"name"` - Type string `json:"type"` - Description string `json:"description"` - Default *string `json:"default"` - AllowedValues []string `json:"allowed_values"` - MinValue *string `json:"min_value"` - MaxValue *string `json:"max_value"` - MinLength *int `json:"min_length"` - MaxLength *int `json:"max_length"` - ObjectType string `json:"object_type"` - DataFlow string `json:"data_flow"` - UserInterface *parameterUserInterfaceResponse `json:"user_interface"` + Name string `json:"name"` + Type string `json:"type"` + Description string `json:"description"` + Default *string `json:"default"` + AllowedValues []string `json:"allowed_values"` + MinValue *string `json:"min_value"` + MaxValue *string `json:"max_value"` + MinLength *int `json:"min_length"` + MaxLength *int `json:"max_length"` + ObjectType string `json:"object_type"` + DataFlow string `json:"data_flow"` + UserInterface *parameterUserInterfaceResponse `json:"user_interface"` + FileFilters []pathFileFilterResponse `json:"file_filters"` + FileFilterDefault *pathFileFilterResponse `json:"file_filter_default"` } func toProductParameterResponse(p openjd.JobParameter) productParameterResponse { @@ -114,11 +121,19 @@ func toProductParameterResponse(p openjd.JobParameter) productParameterResponse } if p.UserInterface != nil { out.UserInterface = ¶meterUserInterfaceResponse{ - Control: string(p.UserInterface.Control), - Label: p.UserInterface.Label, - GroupLabel: p.UserInterface.GroupLabel, - Decimals: p.UserInterface.Decimals, - SingleStepRemoval: p.UserInterface.SingleStepRemoval, + Control: string(p.UserInterface.Control), + Label: p.UserInterface.Label, + GroupLabel: p.UserInterface.GroupLabel, + Decimals: p.UserInterface.Decimals, + } + } + for _, f := range p.FileFilters { + out.FileFilters = append(out.FileFilters, pathFileFilterResponse{Label: f.Label, Patterns: f.Patterns}) + } + if p.FileFilterDefault != nil { + out.FileFilterDefault = &pathFileFilterResponse{ + Label: p.FileFilterDefault.Label, + Patterns: p.FileFilterDefault.Patterns, } } return out diff --git a/internal/openjd/model.go b/internal/openjd/model.go index 4d53aaa8..f09296aa 100644 --- a/internal/openjd/model.go +++ b/internal/openjd/model.go @@ -118,6 +118,24 @@ type JobParameter struct { // UserInterface carries optional OpenJD base-spec presentation hints. // Nil when the parameter declares no userInterface object. UserInterface *ParameterUserInterface + // FileFilters are the file-type choices offered by a CHOOSE_*_FILE dialog. + // Only valid on PATH parameters; validation rejects them on other types. + FileFilters []PathFileFilter + // FileFilterDefault is the filter selected when the dialog opens. + // Only valid on PATH parameters. Nil when absent. + FileFilterDefault *PathFileFilter +} + +// PathFileFilter is one named file type offered by an input or output file +// chooser dialog, e.g. {label: "Image Files", patterns: ["*.png", "*.exr"]}. +// It is presentation metadata: the server parses, validates, and carries it but +// never acts on it. +type PathFileFilter struct { + // Label names the filter in the chooser dialog. Unlike a parameter's + // userInterface label/groupLabel, this field has no enforced length cap. + Label string + // Patterns are the glob patterns the filter matches; at least one required. + Patterns []string } // ControlType is the OpenJD base-spec userInterface control for a job parameter. @@ -132,12 +150,16 @@ const ( ControlDropdownList ControlType = "DROPDOWN_LIST" // ControlCheckBox is a two-state checkbox; requires exactly two allowedValues. ControlCheckBox ControlType = "CHECK_BOX" - // ControlChipInput is a multi-value chip/tag input. - ControlChipInput ControlType = "CHIP_INPUT" // ControlHidden hides the parameter from the generated form. ControlHidden ControlType = "HIDDEN" // ControlSpinBox is a numeric spinner; valid only on INT/FLOAT. ControlSpinBox ControlType = "SPIN_BOX" + // ControlChooseInputFile is a browse-for-an-existing-file dialog; PATH only. + ControlChooseInputFile ControlType = "CHOOSE_INPUT_FILE" + // ControlChooseOutputFile is a browse-for-an-output-file dialog; PATH only. + ControlChooseOutputFile ControlType = "CHOOSE_OUTPUT_FILE" + // ControlChooseDirectory is a browse-for-a-directory dialog; PATH only. + ControlChooseDirectory ControlType = "CHOOSE_DIRECTORY" ) // ParameterUserInterface is the OpenJD base-spec userInterface hint object on a @@ -152,8 +174,6 @@ type ParameterUserInterface struct { GroupLabel string // Decimals sets the precision for a SPIN_BOX on a FLOAT parameter. Decimals *int - // SingleStepRemoval applies to CHIP_INPUT only. - SingleStepRemoval *bool } // ─── Environments ──────────────────────────────────────────────────────────── @@ -180,7 +200,7 @@ type EnvironmentScript struct { } // EnvironmentActions are the lifecycle hooks for an [Environment]. -// At least one of OnEnter or OnExit must be non-nil. +// OnEnter is required by the OpenJD spec; only OnExit is optional. type EnvironmentActions struct { OnEnter *Action OnExit *Action @@ -311,9 +331,12 @@ type TaskParamDefinition struct { type TaskChunks struct { // DefaultTaskCount is the number of INT values to group per task. DefaultTaskCount int - // TargetRuntimeSeconds, when > 0, enables adaptive chunking. + // TargetRuntimeSeconds is parsed and carried but not acted on. The spec + // permits this explicitly: "A scheduler can ignore this, or dynamically + // adjust the chunk task count to be closer to this value." TargetRuntimeSeconds *int - // RangeConstraint is "CONTIGUOUS" (default) or "NONCONTIGUOUS". + // RangeConstraint is "CONTIGUOUS" or "NONCONTIGUOUS"; required, validated + // in [validateChunks]. RangeConstraint string } diff --git a/internal/openjd/parse.go b/internal/openjd/parse.go index 7ff1d680..8fe0e2b3 100644 --- a/internal/openjd/parse.go +++ b/internal/openjd/parse.go @@ -158,9 +158,53 @@ func decodeJobParameter(raw map[string]any) (JobParameter, error) { p.UserInterface = ui } + // fileFilters / fileFilterDefault (PATH-only chooser-dialog metadata; + // validation enforces the PATH-only constraint). Decoded by a helper to + // keep this function's cyclomatic complexity within bounds. + if err := decodeJobParamFileFilters(raw, &p); err != nil { + return p, err + } + return p, nil } +// decodeJobParamFileFilters populates the fileFilters and fileFilterDefault +// fields of p from the raw decoded map. +func decodeJobParamFileFilters(raw map[string]any, p *JobParameter) error { + if filters, ok := raw["fileFilters"].([]any); ok { + for i, v := range filters { + f, err := decodePathFileFilter(v, fmt.Sprintf("parameterDefinition.fileFilters[%d]", i)) + if err != nil { + return err + } + p.FileFilters = append(p.FileFilters, f) + } + } + if v, ok := raw["fileFilterDefault"]; ok && v != nil { + f, err := decodePathFileFilter(v, "parameterDefinition.fileFilterDefault") + if err != nil { + return err + } + p.FileFilterDefault = &f + } + return nil +} + +// decodePathFileFilter decodes one . +func decodePathFileFilter(v any, ctx string) (PathFileFilter, error) { + m, err := toMap(v, ctx) + if err != nil { + return PathFileFilter{}, err + } + f := PathFileFilter{Label: getString(m, "label")} + if raw, ok := m["patterns"].([]any); ok { + for _, p := range raw { + f.Patterns = append(f.Patterns, anyToString(p)) + } + } + return f, nil +} + // decodeJobParamConstraints populates the allowedValues, minValue/maxValue, and // minLength/maxLength fields of p from the raw decoded map. func decodeJobParamConstraints(raw map[string]any, p *JobParameter) error { @@ -217,9 +261,6 @@ func decodeParameterUserInterface(v any) (*ParameterUserInterface, error) { } else if ok { ui.Decimals = &n } - if b, ok := m["singleStepRemoval"].(bool); ok { - ui.SingleStepRemoval = &b - } return ui, nil } @@ -588,9 +629,11 @@ func decodeTaskChunks(v any) (*TaskChunks, error) { if err != nil { return nil, err } - chunks := TaskChunks{ - RangeConstraint: "CONTIGUOUS", // spec default - } + // rangeConstraint is NOT defaulted here: the spec marks it required (no + // @optional annotation, unlike targetRuntimeSeconds on the line above it in + // the schema). Defaulting it would make a missing value invisible to + // validation. + var chunks TaskChunks if n, ok, err := intFieldStrict(m, "defaultTaskCount", "chunks.defaultTaskCount"); err != nil { return nil, err } else if ok { diff --git a/internal/openjd/parse_filefilters_test.go b/internal/openjd/parse_filefilters_test.go new file mode 100644 index 00000000..409f8847 --- /dev/null +++ b/internal/openjd/parse_filefilters_test.go @@ -0,0 +1,488 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package openjd_test + +import ( + "fmt" + "strings" + "testing" + + "github.com/uberware/sqi/internal/openjd" +) + +// fileFiltersYAML builds n distinct, individually-valid fileFilters entries +// for embedding under a PATH parameter's fileFilters key. +func fileFiltersYAML(n int) string { + var b strings.Builder + for i := range n { + fmt.Fprintf(&b, " - label: F%d\n patterns: [\"*.png\"]\n", i) + } + return b.String() +} + +func TestParse_PathFileFilters(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: FilterJob +parameterDefinitions: + - name: ScenePath + type: PATH + objectType: FILE + dataFlow: IN + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: + - label: Image Files + patterns: ["*.png", "*.exr"] + - label: All Files + patterns: ["*"] + fileFilterDefault: + label: Image Files + patterns: ["*.png", "*.exr"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + if errs := openjd.Validate(tmpl); len(errs) > 0 { + t.Fatalf("expected file filters to validate; got: %v", errs) + } + p := tmpl.ParameterDefinitions[0] + if len(p.FileFilters) != 2 { + t.Fatalf("FileFilters = %d, want 2", len(p.FileFilters)) + } + if p.FileFilters[0].Label != "Image Files" { + t.Errorf("filter label = %q, want %q", p.FileFilters[0].Label, "Image Files") + } + if len(p.FileFilters[0].Patterns) != 2 || p.FileFilters[0].Patterns[1] != "*.exr" { + t.Errorf("patterns = %v, want [*.png *.exr]", p.FileFilters[0].Patterns) + } + if p.FileFilterDefault == nil || p.FileFilterDefault.Label != "Image Files" { + t.Errorf("FileFilterDefault = %+v, want the Image Files filter", p.FileFilterDefault) + } +} + +func TestValidate_FileFiltersRejectedOnNonPath(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: BadFilterJob +parameterDefinitions: + - name: Frames + type: STRING + fileFilters: + - label: Image Files + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "fileFilters") +} + +func TestValidate_FileFilterDefaultOnlyRejectedOnNonPath(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: BadFilterDefaultJob +parameterDefinitions: + - name: Frames + type: STRING + fileFilterDefault: + label: Image Files + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/parameterDefinitions/0/fileFilterDefault:") +} + +func TestValidate_FileFilterRequiresPatterns(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: EmptyFilterJob +parameterDefinitions: + - name: ScenePath + type: PATH + fileFilters: + - label: Broken +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "patterns") +} + +// ── Rule 1: label is required (structural) ───────────────────────────────── + +func TestValidate_FileFilterLabelRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: NoLabelJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: + - patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/parameterDefinitions/0/fileFilters/0/label") +} + +func TestValidate_FileFilterDefaultLabelRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: NoDefaultLabelJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilterDefault: + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/parameterDefinitions/0/fileFilterDefault/label") +} + +// ── Rule 2: label capped at 64 characters (gated) ────────────────────────── + +func TestValidate_FileFilterLabelLength(t *testing.T) { + cases := []struct { + name string + label string + wantErr bool + }{ + {"exactly 64 accepted", strings.Repeat("a", 64), false}, + {"65 rejected", strings.Repeat("a", 65), true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + yaml := fmt.Sprintf(` +specificationVersion: jobtemplate-2023-09 +name: LabelLenJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: + - label: %s + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +`, tc.label) + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := containsPointer(errs, "/parameterDefinitions/0/fileFilters/0/label") + if got != tc.wantErr { + t.Errorf("label length %d: got error=%v, want error=%v; errs=%v", len(tc.label), got, tc.wantErr, errs) + } + }) + } +} + +// ── Rule 3: maximum of 20 filters (gated) ────────────────────────────────── + +func TestValidate_FileFilterMaxCount(t *testing.T) { + cases := []struct { + name string + n int + wantErr bool + }{ + {"20 accepted", 20, false}, + {"21 rejected", 21, true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: CountJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: +` + fileFiltersYAML(tc.n) + ` +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := containsMessage(errs, "at most 20 file filters") + if got != tc.wantErr { + t.Errorf("%d filters: got error=%v, want error=%v; errs=%v", tc.n, got, tc.wantErr, errs) + } + }) + } +} + +// ── Rule 4: filters valid only with CHOOSE_INPUT_FILE / CHOOSE_OUTPUT_FILE (structural) ── + +func TestValidate_FileFilterControlPairing(t *testing.T) { + cases := []struct { + name string + control string + wantErr bool + }{ + {"CHOOSE_INPUT_FILE accepted", "CHOOSE_INPUT_FILE", false}, + {"CHOOSE_OUTPUT_FILE accepted", "CHOOSE_OUTPUT_FILE", false}, + {"CHOOSE_DIRECTORY rejected", "CHOOSE_DIRECTORY", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + yaml := fmt.Sprintf(` +specificationVersion: jobtemplate-2023-09 +name: ControlPairJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: %s, label: Scene } + fileFilters: + - label: Image Files + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +`, tc.control) + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := containsMessage(errs, "CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE") + if got != tc.wantErr { + t.Errorf("control %s: got error=%v, want error=%v; errs=%v", tc.control, got, tc.wantErr, errs) + } + }) + } +} + +func TestValidate_FileFilterControlPairing_AbsentControlRejected(t *testing.T) { + // A PATH parameter with fileFilters but no userInterface at all: absent is + // not one of the two file-choosing controls, so it is invalid. + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: NoControlJob +parameterDefinitions: + - name: ScenePath + type: PATH + fileFilters: + - label: Image Files + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE") +} + +// ── Gated (2, 3) vs structural (1, 4): EnforceLimits must only skip the gated ones ── + +func TestValidateWithOptions_FileFilters_GatedVsStructural(t *testing.T) { + t.Run("label length gated, label-required structural", func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: GateLabelJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: + - label: "" + patterns: ["*.png"] + - label: ` + strings.Repeat("a", 65) + ` + patterns: ["*.png"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + + withFalse := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + if containsPointer(withFalse, "/parameterDefinitions/0/fileFilters/1/label") { + t.Errorf("EnforceLimits=false should not report the 64-char label cap; got %v", withFalse) + } + if !containsPointer(withFalse, "/parameterDefinitions/0/fileFilters/0/label") { + t.Errorf("EnforceLimits=false should still report the required-label structural error; got %v", withFalse) + } + + withTrue := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: true}) + if !containsPointer(withTrue, "/parameterDefinitions/0/fileFilters/1/label") { + t.Errorf("EnforceLimits=true should report the 64-char label cap; got %v", withTrue) + } + if !containsPointer(withTrue, "/parameterDefinitions/0/fileFilters/0/label") { + t.Errorf("EnforceLimits=true should still report the required-label structural error; got %v", withTrue) + } + }) + + t.Run("max filter count gated, control pairing structural", func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: GateCountJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_DIRECTORY, label: Scene } + fileFilters: +` + fileFiltersYAML(21) + ` +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + + withFalse := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + if containsMessage(withFalse, "at most 20 file filters") { + t.Errorf("EnforceLimits=false should not report the 20-filter cap; got %v", withFalse) + } + if !containsMessage(withFalse, "CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE") { + t.Errorf("EnforceLimits=false should still report the control-pairing structural error; got %v", withFalse) + } + + withTrue := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: true}) + if !containsMessage(withTrue, "at most 20 file filters") { + t.Errorf("EnforceLimits=true should report the 20-filter cap; got %v", withTrue) + } + if !containsMessage(withTrue, "CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE") { + t.Errorf("EnforceLimits=true should still report the control-pairing structural error; got %v", withTrue) + } + }) +} + +// ── Rule 5: pattern string format (grammar structural, length gated) ────── + +// patternFilterYAML builds a single-parameter template whose one fileFilters +// entry carries the given pattern, and whose fileFilterDefault carries the +// same pattern -- so both fields exercise the same check. +func patternFilterYAML(pattern string) string { + return fmt.Sprintf(` +specificationVersion: jobtemplate-2023-09 +name: PatternJob +parameterDefinitions: + - name: ScenePath + type: PATH + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene } + fileFilters: + - label: Filter + patterns: [%q] + fileFilterDefault: + label: Filter + patterns: [%q] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +`, pattern, pattern) +} + +func TestValidate_FileFilterPatternFormat(t *testing.T) { + cases := []struct { + name string + pattern string + wantErr bool + }{ + {"star accepted", "*", false}, + {"star-dot-star accepted", "*.*", false}, + {"png accepted", "*.png", false}, + {"exr accepted", "*.exr", false}, + {"multi-dot extension accepted", "*.tar.gz", false}, + {"20-char pattern accepted", "*." + strings.Repeat("a", 18), false}, + {"empty string rejected", "", true}, + {"trailing dot empty extension rejected", "*.", true}, + {"missing leading star rejected", "png", true}, + {"wildcard star in extension rejected", "*.p*g", true}, + {"forward slash in extension rejected", "*.a/b", true}, + {"backslash in extension rejected", `*.a\b`, true}, + {"question mark in extension rejected", "*.a?", true}, + {"brackets in extension rejected", "*.a[b]", true}, + {"colon in extension rejected", "*.a:b", true}, + {"pipe in extension rejected", "*.a|b", true}, + {"dollar sign in extension rejected", "*.a$b", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + yaml := patternFilterYAML(tc.pattern) + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + gotFilters := containsPointer(errs, "/parameterDefinitions/0/fileFilters/0/patterns/0") + gotDefault := containsPointer(errs, "/parameterDefinitions/0/fileFilterDefault/patterns/0") + if gotFilters != tc.wantErr { + t.Errorf("pattern %q (fileFilters): got error=%v, want error=%v; errs=%v", tc.pattern, gotFilters, tc.wantErr, errs) + } + if gotDefault != tc.wantErr { + t.Errorf("pattern %q (fileFilterDefault): got error=%v, want error=%v; errs=%v", tc.pattern, gotDefault, tc.wantErr, errs) + } + }) + } +} + +// ── Rule 6: pattern max length 20 characters (gated) ─────────────────────── + +func TestValidateWithOptions_FileFilterPatternLength_GatedVsStructural(t *testing.T) { + longPattern := "*." + strings.Repeat("a", 19) // 21 characters total + yaml := patternFilterYAML(longPattern) + tmpl := mustParse(t, yaml) + + withFalse := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + if containsPointer(withFalse, "/parameterDefinitions/0/fileFilters/0/patterns/0") { + t.Errorf("EnforceLimits=false should not report the 20-char pattern cap; got %v", withFalse) + } + + withTrue := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: true}) + if !containsPointer(withTrue, "/parameterDefinitions/0/fileFilters/0/patterns/0") { + t.Errorf("EnforceLimits=true should report the 20-char pattern cap; got %v", withTrue) + } + + // A grammar-invalid pattern must still error with EnforceLimits: false. + badYAML := patternFilterYAML("*.a:b") + badTmpl := mustParse(t, badYAML) + badErrs := openjd.ValidateWithOptions(badTmpl, openjd.ValidateOptions{EnforceLimits: false}) + if !containsPointer(badErrs, "/parameterDefinitions/0/fileFilters/0/patterns/0") { + t.Errorf("EnforceLimits=false should still report a grammar-invalid pattern; got %v", badErrs) + } +} diff --git a/internal/openjd/parse_userinterface_test.go b/internal/openjd/parse_userinterface_test.go index adcedab9..9dc97eb3 100644 --- a/internal/openjd/parse_userinterface_test.go +++ b/internal/openjd/parse_userinterface_test.go @@ -60,11 +60,6 @@ parameterDefinitions: userInterface: control: SPIN_BOX decimals: 3 - - name: Tags - type: STRING - userInterface: - control: CHIP_INPUT - singleStepRemoval: true steps: - name: A script: @@ -80,8 +75,4 @@ steps: if scale == nil || scale.Decimals == nil || *scale.Decimals != 3 { t.Fatalf("Scale.Decimals = %v, want 3", scale) } - tags := tmpl.ParameterDefinitions[1].UserInterface - if tags == nil || tags.SingleStepRemoval == nil || !*tags.SingleStepRemoval { - t.Fatalf("Tags.SingleStepRemoval = %v, want true", tags) - } } diff --git a/internal/openjd/validate.go b/internal/openjd/validate.go index cbaf3e3f..fcc1d378 100644 --- a/internal/openjd/validate.go +++ b/internal/openjd/validate.go @@ -8,6 +8,7 @@ import ( "math" "regexp" "slices" + "sort" "strconv" "strings" "unicode/utf8" @@ -166,8 +167,9 @@ func validatePathTranslation(t *JobTemplate) ValidationErrors { // validateChunkBounds enforces the SQI_CHUNK_BOUNDS extension's constraints when // it is declared: TASK_CHUNKING must also be declared, and every CHUNK[INT] task -// parameter must be CONTIGUOUS (an empty rangeConstraint defaults to contiguous), -// because .Start/.End are undefined across the gaps of a NONCONTIGUOUS chunk. +// parameter must be CONTIGUOUS (rangeConstraint is required, so this only rejects +// the literal NONCONTIGUOUS value), because .Start/.End are undefined across the +// gaps of a NONCONTIGUOUS chunk. // Runs unconditionally (not gated by EnforceLimits). func validateChunkBounds(t *JobTemplate) ValidationErrors { if !t.hasExtension("SQI_CHUNK_BOUNDS") { @@ -207,13 +209,23 @@ var identifierRE = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) // underscores, and digits, 3-128 characters. Must match [A-Z_0-9]{3,128}. var extensionNameRE = regexp.MustCompile(`^[A-Z_0-9]{3,128}$`) +// fileFilterPatternRE matches the grammar of +// (§2.8): "*", "*.*", or "*." followed +// by one or more legal extension characters. Legal extension characters are +// any unicode character except the Cc category, path separators ("\" and +// "/"), wildcard characters ("*", "?", "[", "]"), and characters commonly +// disallowed in paths ("#", "%", "&", "{", "}", "<", ">", "$", "!", "'", +// "\"", ":", "@", "`", "|", "="). This also subsumes the spec's 1-character +// minimum length: an empty string does not match any alternative. +var fileFilterPatternRE = regexp.MustCompile(`^(\*|\*\.\*|\*\.[^\p{Cc}\\/*?\[\]#%&{}<>$!'":@` + "`" + `|=]+)$`) + // ─── ValidateOptions ────────────────────────────────────────────────────────── // ValidateOptions controls optional validation behavior passed to // [ValidateWithOptions]. type ValidateOptions struct { // EnforceLimits gates quantitative limit checks: maximum name lengths, - // element counts, reserved-name rules, etc. + // element counts, etc. // // When false those checks are skipped — useful in operator environments // that predate strict limit enforcement and cannot yet update all templates. @@ -221,6 +233,12 @@ type ValidateOptions struct { // NOTE: the quantitative limit checks live in [validateLimits] (and the // helpers it calls). Every limit check MUST be guarded by opts.EnforceLimits // and belong to validateLimits — do not scatter gated checks elsewhere. + // Structural correctness checks belong in the always-run path instead: + // [validateHostRequirements] is the host-requirement half, deliberately + // separate from [validateHostRequirementLimits]. Reserved-capability value + // checks ([validateReservedAmounts], [validateReservedAttributes]) are + // value-domain correctness, not a size or count cap, so they run from + // [validateHostRequirements] unconditionally too. // Resource-exhaustion guards (e.g. the [maxRangeValues] cap in // parseIntRangeExpr) are NOT limit checks: they always apply, regardless of // this flag. @@ -398,9 +416,22 @@ const ( // maxAttributeValues caps each attribute's anyOf/allOf element count (1–50). maxAttributeValues = 50 // maxUILabelLen caps userInterface label length in characters (runes). - maxUILabelLen = 256 + // The spec's is 1-64 characters. + maxUILabelLen = 64 // maxUIGroupLabelLen caps userInterface groupLabel length in characters (runes). - maxUIGroupLabelLen = 256 + maxUIGroupLabelLen = 64 + // maxFileFilterLabelLen caps a fileFilters/fileFilterDefault entry's label + // length in characters (runes). Same bound + // (1-64 characters) as maxUILabelLen (§2.6). + maxFileFilterLabelLen = maxUILabelLen + // maxFileFilters caps the number of entries in a PATH parameter's + // fileFilters list. Spec: "Maximum of 20 filters". + maxFileFilters = 20 + // maxFileFilterPatternLen caps a single pattern string's length in + // characters (runes). The spec's is + // 1-20 characters; the 1-character minimum is structural (subsumed by + // [fileFilterPatternRE]) so only the maximum is gated here. + maxFileFilterPatternLen = 20 ) // validateLimits runs every quantitative limit check. It is only invoked when @@ -530,19 +561,16 @@ func taskParamValueCount(tp TaskParamDefinition) (count int, counted bool) { } // validateHostRequirementLimits checks the gated limits on a step's -// hostRequirements: combined count, presence, capability name lengths, and -// attribute anyOf/allOf element counts. +// hostRequirements: combined count, capability name lengths, and attribute +// anyOf/allOf element counts. Structural correctness (presence, capability +// name well-formedness and prefix, and attribute anyOf/allOf presence) and +// reserved-capability value checks (reserved amount minimums, reserved +// attribute allowed values) live in [validateHostRequirements] instead and +// always run — see the invariant documented on [ValidateOptions]. func validateHostRequirementLimits(hr HostRequirements, base string) ValidationErrors { var errs ValidationErrors - combined := len(hr.Amounts) + len(hr.Attributes) - switch { - case combined == 0: - errs = append(errs, ValidationError{ - Pointer: base, - Message: "hostRequirements must declare at least one amount or attribute when present", - }) - case combined > maxHostRequirements: + if combined := len(hr.Amounts) + len(hr.Attributes); combined > maxHostRequirements { errs = append(errs, ValidationError{ Pointer: base, Message: fmt.Sprintf("at most %d host requirements are allowed (got %d)", maxHostRequirements, combined), @@ -551,14 +579,12 @@ func validateHostRequirementLimits(hr HostRequirements, base string) ValidationE for i, a := range hr.Amounts { ptr := fmt.Sprintf("%s/amounts/%d/name", base, i) - errs = append(errs, validateCapabilityName(a.Name, ptr)...) - errs = append(errs, validateCapabilityPrefix(a.Name, "amount.", ptr)...) + errs = append(errs, validateCapabilityNameLength(a.Name, ptr)...) } for i, a := range hr.Attributes { ptr := fmt.Sprintf("%s/attributes/%d", base, i) - errs = append(errs, validateCapabilityName(a.Name, ptr+"/name")...) - errs = append(errs, validateCapabilityPrefix(a.Name, "attr.", ptr+"/name")...) + errs = append(errs, validateCapabilityNameLength(a.Name, ptr+"/name")...) if len(a.AnyOf) > maxAttributeValues { errs = append(errs, ValidationError{ @@ -572,6 +598,42 @@ func validateHostRequirementLimits(hr HostRequirements, base string) ValidationE Message: fmt.Sprintf("at most %d values are allowed (got %d)", maxAttributeValues, len(a.AllOf)), }) } + } + + return errs +} + +// validateHostRequirements checks the structural correctness of a step's host +// requirements: that the block declares something, that capability names are +// non-empty and correctly prefixed, that each attribute constrains something, +// and — for reserved capability names — that the value asked for is within +// the spec-mandated domain (reserved amount minimums, reserved attribute +// allowed values). These are correctness checks, not size caps: a template +// asking for vcpu: 0 is malformed, not oversized. So they always run -- see +// the invariant documented on [ValidateOptions]. The gated size caps +// (combined count, name length, anyOf/allOf element counts) live in +// [validateHostRequirementLimits] instead. +func validateHostRequirements(hr HostRequirements, base string) ValidationErrors { + var errs ValidationErrors + + if len(hr.Amounts)+len(hr.Attributes) == 0 { + errs = append(errs, ValidationError{ + Pointer: base, + Message: "hostRequirements must declare at least one amount or attribute when present", + }) + } + + for i, a := range hr.Amounts { + ptr := fmt.Sprintf("%s/amounts/%d/name", base, i) + errs = append(errs, validateCapabilityNameRequired(a.Name, ptr)...) + errs = append(errs, validateCapabilityPrefix(a.Name, "amount.", ptr)...) + } + + for i, a := range hr.Attributes { + ptr := fmt.Sprintf("%s/attributes/%d", base, i) + errs = append(errs, validateCapabilityNameRequired(a.Name, ptr+"/name")...) + errs = append(errs, validateCapabilityPrefix(a.Name, "attr.", ptr+"/name")...) + if len(a.AnyOf)+len(a.AllOf) == 0 { errs = append(errs, ValidationError{ Pointer: ptr, @@ -580,7 +642,9 @@ func validateHostRequirementLimits(hr HostRequirements, base string) ValidationE } } - // Reserved-name checks (also gated by EnforceLimits via the call chain). + // Reserved-capability value checks: pure value-domain correctness, with + // no size or count component, so they belong here rather than in the + // gated validateHostRequirementLimits. errs = append(errs, validateReservedAmounts(hr.Amounts, base)...) errs = append(errs, validateReservedAttributes(hr.Attributes, base)...) @@ -593,8 +657,10 @@ func validateHostRequirementLimits(hr HostRequirements, base string) ValidationE // // Min/Max values that contain an unresolved format-string reference ("{{") are // skipped: the numeric value cannot be determined before job-parameter binding. -// Non-numeric values are already rejected by earlier structural checks, so a -// parse failure here is silently skipped to avoid double-reporting. +// Nothing upstream of this check rejects a non-numeric Min/Max: the decoder +// (decodeAmountBound) accepts any scalar (string, number, or boolean) without +// requiring it to parse as a number, so a non-numeric bound is reported here, +// by [checkReservedBound], not silently skipped. func validateReservedAmounts(amounts []AmountRequirement, base string) ValidationErrors { var errs ValidationErrors for i, a := range amounts { @@ -609,10 +675,11 @@ func validateReservedAmounts(amounts []AmountRequirement, base string) Validatio return errs } -// checkReservedBound validates that a *string capability bound (Min or Max), -// when present and parseable as a number, is >= the reserved minimum. Nil -// bounds and bounds containing format-string references ("{{") are skipped. -// Non-finite values (NaN, ±Inf) are rejected as validation errors. +// checkReservedBound validates that a *string capability bound (Min or Max) +// is >= the reserved minimum. Nil bounds and bounds containing format-string +// references ("{{") are skipped. A bound that fails to parse as a number, or +// that parses to a non-finite value (NaN, ±Inf), is reported as a validation +// error rather than skipped. func checkReservedBound(val *string, minReq float64, capName, ptr string) ValidationErrors { if val == nil || strings.Contains(*val, "{{") { return nil @@ -697,7 +764,8 @@ func sortedMapKeys(m map[string]bool) string { // "attr." for attributes. The match is case-insensitive, since OpenJD // jobtemplate-2023-09 defines capability names as case-insensitive and the // scheduler resolves them case-insensitively (see internal/scheduler/matcher.go). -// An empty name is left to [validateCapabilityName]'s "name is required" check. +// An empty name is left to [validateCapabilityNameRequired]'s "name is +// required" check. func validateCapabilityPrefix(name, wantPrefix, ptr string) ValidationErrors { if name == "" { return nil @@ -711,13 +779,21 @@ func validateCapabilityPrefix(name, wantPrefix, ptr string) ValidationErrors { return nil } -// validateCapabilityName checks that an amount/attribute capability name has a -// length within the spec bounds (1–100 characters). -func validateCapabilityName(name, ptr string) ValidationErrors { - switch n := utf8.RuneCountInString(name); { - case n == 0: +// validateCapabilityNameRequired checks that an amount/attribute capability +// name is present. Structural correctness: always runs, regardless of +// EnforceLimits — see the invariant documented on [ValidateOptions]. +func validateCapabilityNameRequired(name, ptr string) ValidationErrors { + if utf8.RuneCountInString(name) == 0 { return ValidationErrors{{Pointer: ptr, Message: "name is required"}} - case n > maxCapabilityNameLen: + } + return nil +} + +// validateCapabilityNameLength checks that an amount/attribute capability name +// is within the spec bound (at most 100 characters). Gated: only runs when +// EnforceLimits is set (see [validateHostRequirementLimits]). +func validateCapabilityNameLength(name, ptr string) ValidationErrors { + if n := utf8.RuneCountInString(name); n > maxCapabilityNameLen { return ValidationErrors{{ Pointer: ptr, Message: fmt.Sprintf("name must be at most %d characters (got %d)", maxCapabilityNameLen, n), @@ -726,25 +802,75 @@ func validateCapabilityName(name, ptr string) ValidationErrors { return nil } -// validateUILimits enforces length caps on userInterface labels. Gated: callers +// validateUILimits enforces length caps on userInterface labels and the +// fileFilters quantitative caps (label length, filter count). Gated: callers // run it only when EnforceLimits is set. func validateUILimits(params []JobParameter) ValidationErrors { var errs ValidationErrors for i, p := range params { - if p.UserInterface == nil { - continue - } - base := fmt.Sprintf("/parameterDefinitions/%d/userInterface", i) - if utf8.RuneCountInString(p.UserInterface.Label) > maxUILabelLen { - errs = append(errs, ValidationError{ - Pointer: base + "/label", - Message: fmt.Sprintf("label exceeds %d characters", maxUILabelLen), - }) + paramPtr := fmt.Sprintf("/parameterDefinitions/%d", i) + if p.UserInterface != nil { + base := paramPtr + "/userInterface" + if utf8.RuneCountInString(p.UserInterface.Label) > maxUILabelLen { + errs = append(errs, ValidationError{ + Pointer: base + "/label", + Message: fmt.Sprintf("label exceeds %d characters", maxUILabelLen), + }) + } + if utf8.RuneCountInString(p.UserInterface.GroupLabel) > maxUIGroupLabelLen { + errs = append(errs, ValidationError{ + Pointer: base + "/groupLabel", + Message: fmt.Sprintf("groupLabel exceeds %d characters", maxUIGroupLabelLen), + }) + } } - if utf8.RuneCountInString(p.UserInterface.GroupLabel) > maxUIGroupLabelLen { + errs = append(errs, validateFileFilterLimits(p, paramPtr)...) + } + return errs +} + +// validateFileFilterLimits checks the gated quantitative caps on a PATH +// parameter's file filters: at most [maxFileFilters] entries, each entry's +// (including fileFilterDefault's) label at most [maxFileFilterLabelLen] +// characters, and each pattern (including fileFilterDefault's) at most +// [maxFileFilterPatternLen] characters. Structural correctness (label +// required, control pairing, pattern grammar) lives in [validateFileFilters] +// instead and always runs -- see the invariant documented on +// [ValidateOptions]. +func validateFileFilterLimits(p JobParameter, ptr string) ValidationErrors { + var errs ValidationErrors + if len(p.FileFilters) > maxFileFilters { + errs = append(errs, ValidationError{ + Pointer: ptr + "/fileFilters", + Message: fmt.Sprintf("at most %d file filters are allowed (got %d)", maxFileFilters, len(p.FileFilters)), + }) + } + for i, f := range p.FileFilters { + errs = append(errs, validatePathFileFilterLimits(f, fmt.Sprintf("%s/fileFilters/%d", ptr, i))...) + } + if p.FileFilterDefault != nil { + errs = append(errs, validatePathFileFilterLimits(*p.FileFilterDefault, ptr+"/fileFilterDefault")...) + } + return errs +} + +// validatePathFileFilterLimits checks the gated quantitative caps on a +// single [PathFileFilter]: label length and each pattern's length. Extracted +// from [validateFileFilterLimits] to keep its complexity in bounds and +// reused for both fileFilters entries and fileFilterDefault. +func validatePathFileFilterLimits(f PathFileFilter, ptr string) ValidationErrors { + var errs ValidationErrors + if n := utf8.RuneCountInString(f.Label); n > maxFileFilterLabelLen { + errs = append(errs, ValidationError{ + Pointer: ptr + "/label", + Message: fmt.Sprintf("label must be at most %d characters (got %d)", maxFileFilterLabelLen, n), + }) + } + for i, pattern := range f.Patterns { + if n := utf8.RuneCountInString(pattern); n > maxFileFilterPatternLen { errs = append(errs, ValidationError{ - Pointer: base + "/groupLabel", - Message: fmt.Sprintf("groupLabel exceeds %d characters", maxUIGroupLabelLen), + Pointer: fmt.Sprintf("%s/patterns/%d", ptr, i), + Message: fmt.Sprintf("pattern must be at most %d characters (got %d)", maxFileFilterPatternLen, n), }) } } @@ -753,16 +879,54 @@ func validateUILimits(params []JobParameter) ValidationErrors { // ─── userInterface validation ───────────────────────────────────────────────── -// validControls is the set of OpenJD base-spec userInterface control values. -// Read-only after initialization. -var validControls = map[ControlType]struct{}{ - ControlLineEdit: {}, - ControlMultilineEdit: {}, - ControlDropdownList: {}, - ControlCheckBox: {}, - ControlChipInput: {}, - ControlHidden: {}, - ControlSpinBox: {}, +// controlsByType is the OpenJD base-spec userInterface control vocabulary, +// scoped per parameter type as the spec defines it. The spec does NOT share one +// vocabulary across types: LINE_EDIT is valid on STRING and invalid on PATH, +// which needs a CHOOSE_* dialog instead. Read-only after initialization. +// +// The *_LIST control variants belong to the EXPR extension and are deliberately +// absent -- sqi does not implement EXPR. +var controlsByType = map[JobParamType]map[ControlType]struct{}{ + JobParamTypeString: { + ControlLineEdit: {}, + ControlMultilineEdit: {}, + ControlDropdownList: {}, + ControlCheckBox: {}, + ControlHidden: {}, + }, + JobParamTypePath: { + ControlChooseInputFile: {}, + ControlChooseOutputFile: {}, + ControlChooseDirectory: {}, + ControlDropdownList: {}, + ControlHidden: {}, + }, + JobParamTypeInt: { + ControlSpinBox: {}, + ControlDropdownList: {}, + ControlHidden: {}, + }, + JobParamTypeFloat: { + ControlSpinBox: {}, + ControlDropdownList: {}, + ControlHidden: {}, + }, +} + +// allowedControlsFor returns the sorted control names valid for a parameter +// type, for use in error messages. A template author who writes LINE_EDIT on a +// PATH needs to be told CHOOSE_INPUT_FILE exists, not merely that they are wrong. +func allowedControlsFor(t JobParamType) string { + set, ok := controlsByType[t] + if !ok { + return "" + } + names := make([]string, 0, len(set)) + for c := range set { + names = append(names, string(c)) + } + sort.Strings(names) + return strings.Join(names, ", ") } // validateUserInterfaceControl checks control-specific constraints for a @@ -778,10 +942,6 @@ func validateUserInterfaceControl(ui *ParameterUserInterface, p JobParameter, ct if len(p.AllowedValues) != 2 { errs = append(errs, ValidationError{Pointer: ctrlPtr, Message: "CHECK_BOX requires exactly two allowedValues"}) } - case ControlSpinBox: - if p.Type != JobParamTypeInt && p.Type != JobParamTypeFloat { - errs = append(errs, ValidationError{Pointer: ctrlPtr, Message: "SPIN_BOX is valid only on INT or FLOAT parameters"}) - } } return errs } @@ -789,7 +949,7 @@ func validateUserInterfaceControl(ui *ParameterUserInterface, p JobParameter, ct // validateUserInterface checks a parameter's optional userInterface hints: // the control must be a known value, and control/constraint combinations must // be coherent (DROPDOWN_LIST/CHECK_BOX need allowedValues; SPIN_BOX is numeric; -// decimals/singleStepRemoval pair with their controls). Structural — always runs. +// decimals pairs with its control). Structural — always runs. func validateUserInterface(p JobParameter, ptr string) ValidationErrors { ui := p.UserInterface if ui == nil { @@ -802,10 +962,15 @@ func validateUserInterface(p JobParameter, ptr string) ValidationErrors { errs = append(errs, ValidationError{Pointer: ctrlPtr, Message: "required"}) return errs } - if _, ok := validControls[ui.Control]; !ok { + allowed, known := controlsByType[p.Type] + if !known { + return errs // unknown parameter type is reported by validateJobParams + } + if _, ok := allowed[ui.Control]; !ok { errs = append(errs, ValidationError{ Pointer: ctrlPtr, - Message: fmt.Sprintf("unknown control %q", ui.Control), + Message: fmt.Sprintf("control %q is not valid on a %s parameter; allowed: %s", + ui.Control, p.Type, allowedControlsFor(p.Type)), }) return errs } @@ -818,13 +983,6 @@ func validateUserInterface(p JobParameter, ptr string) ValidationErrors { Message: "decimals is valid only with SPIN_BOX on a FLOAT parameter", }) } - if ui.SingleStepRemoval != nil && ui.Control != ControlChipInput { - errs = append(errs, ValidationError{ - Pointer: ptr + "/userInterface/singleStepRemoval", - Message: "singleStepRemoval is valid only with CHIP_INPUT", - }) - } - return errs } @@ -872,6 +1030,9 @@ func validateJobParams(params []JobParameter) ValidationErrors { // userInterface validation is also structural, always runs. errs = append(errs, validateUserInterface(p, ptr)...) + + // fileFilters / fileFilterDefault are also structural, always runs. + errs = append(errs, validateFileFilters(p, ptr)...) } return errs } @@ -1047,6 +1208,107 @@ func validatePathOnlyField[T ~string](value T, ptr, field string, isPath bool, v return errs } +// fileFilterControls is the set of userInterface controls that fileFilters and +// fileFilterDefault are valid alongside, per OpenJD jobtemplate-2023-09 §2.7: +// "Can be provided when the uiControl is CHOOSE_INPUT_FILE or +// CHOOSE_OUTPUT_FILE". +var fileFilterControls = map[ControlType]struct{}{ + ControlChooseInputFile: {}, + ControlChooseOutputFile: {}, +} + +// validateFileFilters checks the PATH-only file chooser filters: they are +// valid only on PATH parameters whose userInterface.control is +// CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE, and each filter (including +// fileFilterDefault) must declare a label and at least one pattern. +// Structural -- always runs. The quantitative caps (label length, filter +// count) live in [validateFileFilterLimits] instead, gated behind +// EnforceLimits -- see the invariant documented on [ValidateOptions]. +func validateFileFilters(p JobParameter, ptr string) ValidationErrors { + var errs ValidationErrors + if len(p.FileFilters) == 0 && p.FileFilterDefault == nil { + return nil + } + // Point at whichever field was actually declared, so a + // fileFilterDefault-only parameter doesn't get an error pointing at + // the sibling fileFilters field it never set. + field := "fileFilterDefault" + if len(p.FileFilters) > 0 { + field = "fileFilters" + } + if p.Type != JobParamTypePath { + errs = append(errs, ValidationError{ + Pointer: ptr + "/" + field, + Message: "fileFilters and fileFilterDefault are valid only on PATH parameters", + }) + return errs + } + if p.UserInterface == nil { + errs = append(errs, ValidationError{ + Pointer: ptr + "/" + field, + Message: "fileFilters and fileFilterDefault require userInterface.control to be CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE", + }) + } else if _, ok := fileFilterControls[p.UserInterface.Control]; !ok { + errs = append(errs, ValidationError{ + Pointer: ptr + "/" + field, + Message: fmt.Sprintf("fileFilters and fileFilterDefault require userInterface.control to be CHOOSE_INPUT_FILE or CHOOSE_OUTPUT_FILE (got %q)", p.UserInterface.Control), + }) + } + for i, f := range p.FileFilters { + errs = append(errs, validatePathFileFilter(f, fmt.Sprintf("%s/fileFilters/%d", ptr, i))...) + } + if p.FileFilterDefault != nil { + errs = append(errs, validatePathFileFilter(*p.FileFilterDefault, ptr+"/fileFilterDefault")...) + } + return errs +} + +// validatePathFileFilter checks the structural correctness of a single +// [PathFileFilter]: label is required, at least one pattern is required, and +// each pattern must match the grammar +// (§2.8): "*", "*.*", or "*." followed by one or more legal extension +// characters. Extracted from [validateFileFilters] to keep its complexity in +// bounds and reused for both fileFilters entries and fileFilterDefault. +func validatePathFileFilter(f PathFileFilter, ptr string) ValidationErrors { + var errs ValidationErrors + if f.Label == "" { + errs = append(errs, ValidationError{ + Pointer: ptr + "/label", + Message: "required", + }) + } + if len(f.Patterns) == 0 { + errs = append(errs, ValidationError{ + Pointer: ptr + "/patterns", + Message: "at least one pattern is required", + }) + } + for i, pattern := range f.Patterns { + if !fileFilterPatternRE.MatchString(pattern) { + errs = append(errs, ValidationError{ + Pointer: fmt.Sprintf("%s/patterns/%d", ptr, i), + Message: fmt.Sprintf("pattern %q is not a valid file filter pattern; must be \"*\", \"*.*\", or \"*.\" followed by one or more legal extension characters", pattern), + }) + } + } + return errs +} + +// validateAction checks the spec-required fields on a single action. The spec +// marks command required with a minimum length of 1 character. An action with +// no command is accepted by parse, expands into tasks, and then runs nothing -- +// the step reports success having done no work -- so this is structural +// correctness and always runs, never gated behind EnforceLimits. +func validateAction(a Action, ptr string) ValidationErrors { + if a.Command == "" { + return ValidationErrors{{ + Pointer: ptr + "/command", + Message: "required; must be at least 1 character", + }} + } + return nil +} + // ─── environment validation ─────────────────────────────────────────────────── func validateEnvironments(envs []Environment, base string) ValidationErrors { @@ -1064,12 +1326,23 @@ func validateEnvironments(envs []Environment, base string) ValidationErrors { } else { seen[e.Name] = struct{}{} } + if e.Script == nil && len(e.Variables) == 0 { + errs = append(errs, ValidationError{ + Pointer: ptr, + Message: "at least one of script or variables must be provided", + }) + } if e.Script != nil { - if e.Script.Actions.OnEnter == nil && e.Script.Actions.OnExit == nil { + if e.Script.Actions.OnEnter == nil { errs = append(errs, ValidationError{ - Pointer: ptr + "/script/actions", - Message: "at least one of onEnter or onExit must be defined", + Pointer: ptr + "/script/actions/onEnter", + Message: "required", }) + } else { + errs = append(errs, validateAction(*e.Script.Actions.OnEnter, ptr+"/script/actions/onEnter")...) + } + if e.Script.Actions.OnExit != nil { + errs = append(errs, validateAction(*e.Script.Actions.OnExit, ptr+"/script/actions/onExit")...) } errs = append(errs, validateEmbeddedFiles(e.Script.EmbeddedFiles, ptr+"/script/embeddedFiles")...) } @@ -1134,9 +1407,13 @@ func validateStep(s StepTemplate, idx int, stepNames map[string]struct{}) Valida } } - // step script embedded files - if s.Script != nil { + // step script -- required by spec; without it the step has no action and + // scheduler/assign.go silently omits it, so the step runs nothing. + if s.Script == nil { + errs = append(errs, ValidationError{Pointer: base + "/script", Message: "required"}) + } else { errs = append(errs, validateEmbeddedFiles(s.Script.EmbeddedFiles, base+"/script/embeddedFiles")...) + errs = append(errs, validateAction(s.Script.Actions.OnRun, base+"/script/actions/onRun")...) } // step environments @@ -1147,6 +1424,11 @@ func validateStep(s StepTemplate, idx int, stepNames map[string]struct{}) Valida errs = append(errs, validateParameterSpace(*s.ParameterSpace, base+"/parameterSpace")...) } + // host requirements (structural; the size caps stay in validateLimits) + if s.HostRequirements != nil { + errs = append(errs, validateHostRequirements(*s.HostRequirements, base+"/hostRequirements")...) + } + return errs } @@ -1262,17 +1544,45 @@ func validateTaskParamRangeAndChunks(tp TaskParamDefinition, base string) Valida Pointer: base + "/chunks", Message: "required for CHUNK[INT] parameters", }) - } else if tp.Chunks.DefaultTaskCount <= 0 { - errs = append(errs, ValidationError{ - Pointer: base + "/chunks/defaultTaskCount", - Message: "must be a positive integer", - }) + } else { + errs = append(errs, validateChunks(*tp.Chunks, base)...) } } return errs } +// validateChunks validates a CHUNK[INT] parameter's chunks definition. It is +// extracted from [validateTaskParamRangeAndChunks] to keep that function's +// cyclomatic complexity within bounds. +func validateChunks(c TaskChunks, base string) ValidationErrors { + var errs ValidationErrors + + if c.DefaultTaskCount <= 0 { + errs = append(errs, ValidationError{ + Pointer: base + "/chunks/defaultTaskCount", + Message: "must be a positive integer", + }) + } + + switch c.RangeConstraint { + case "": + errs = append(errs, ValidationError{ + Pointer: base + "/chunks/rangeConstraint", + Message: "required; must be CONTIGUOUS or NONCONTIGUOUS", + }) + case "CONTIGUOUS", "NONCONTIGUOUS": + // valid + default: + errs = append(errs, ValidationError{ + Pointer: base + "/chunks/rangeConstraint", + Message: fmt.Sprintf("invalid value %q; must be CONTIGUOUS or NONCONTIGUOUS", c.RangeConstraint), + }) + } + + return errs +} + // validateCombination checks that a combination expression is syntactically // valid, that every identifier it references names a declared parameter, and // that no CHUNK[INT] parameter is associated (zipped) with other parameters. diff --git a/internal/openjd/validate_capability_prefix_test.go b/internal/openjd/validate_capability_prefix_test.go index 234da769..9a20b74b 100644 --- a/internal/openjd/validate_capability_prefix_test.go +++ b/internal/openjd/validate_capability_prefix_test.go @@ -12,9 +12,12 @@ import ( // with "amount.", attributes with "attr.". A mis-namespaced name — e.g. an // `amount.worker.vcpu` requirement mistakenly written as the attribute // `worker.vcpu` — resolves to an empty worker value and so can never be -// satisfied, leaving the job's tasks permanently ready. This check is gated -// behind EnforceLimits alongside the other capability-name rules. -func TestValidate_CapabilityNamePrefix_Gated(t *testing.T) { +// satisfied, leaving the job's tasks permanently ready. This is structural +// correctness, not a size cap, so it always runs — even with EnforceLimits +// false (see the invariant documented on [openjd.ValidateOptions] and +// [openjd.ValidateWithOptions]'s split between validateHostRequirements and +// validateHostRequirementLimits). +func TestValidate_CapabilityNamePrefix_Structural(t *testing.T) { cases := []struct { name string mutate func(*openjd.JobTemplate) @@ -90,13 +93,18 @@ func TestValidate_CapabilityNamePrefix_Gated(t *testing.T) { t.Fatalf("EnforceLimits=true: expected pointer %q, got %v", tc.wantPtr, errsOn) } - // ── EnforceLimits=false: the prefix check must be gated off ─────── + // ── EnforceLimits=false: the prefix check is structural and must + // still fire ─────────────────────────────────────────────────── tmplOff := mustParse(t, minimalValidYAML()) tc.mutate(tmplOff) errsOff := openjd.ValidateWithOptions(tmplOff, openjd.ValidateOptions{EnforceLimits: false}) - if tc.wantPtr != "" && containsPointer(errsOff, tc.wantPtr) { - t.Fatalf("EnforceLimits=false: pointer %q should be gated off, got %v", tc.wantPtr, errsOff) + if tc.wantPtr == "" { + if len(errsOff) != 0 { + t.Fatalf("EnforceLimits=false: expected no errors, got %v", errsOff) + } + } else if !containsPointer(errsOff, tc.wantPtr) { + t.Fatalf("EnforceLimits=false: expected pointer %q, got %v", tc.wantPtr, errsOff) } }) } diff --git a/internal/openjd/validate_combination_test.go b/internal/openjd/validate_combination_test.go index 7153c9f8..979fff0b 100644 --- a/internal/openjd/validate_combination_test.go +++ b/internal/openjd/validate_combination_test.go @@ -24,7 +24,7 @@ func chunkStep(t *testing.T, comb string) *openjd.JobTemplate { Name: "Chunked", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), - Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}, + Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}, }, {Name: "Other", Type: openjd.TaskParamTypeString, RangeList: []string{"x", "y", "z", "w", "v"}}, }, @@ -51,7 +51,7 @@ func TestValidate_ChunkAssociatedNested_Error(t *testing.T) { c := "(Chunked * Other, Extra)" tmpl.Steps[0].ParameterSpace = &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ - {Name: "Chunked", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}}, + {Name: "Chunked", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}}, {Name: "Other", Type: openjd.TaskParamTypeString, RangeList: []string{"x"}}, {Name: "Extra", Type: openjd.TaskParamTypeString, RangeList: []string{"e"}}, }, @@ -79,8 +79,8 @@ func TestValidate_MultipleChunkParams_Error(t *testing.T) { c := "ChunkA * ChunkB" tmpl.Steps[0].ParameterSpace = &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ - {Name: "ChunkA", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}}, - {Name: "ChunkB", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}}, + {Name: "ChunkA", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}}, + {Name: "ChunkB", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}}, }, Combination: &c, } diff --git a/internal/openjd/validate_conformance_test.go b/internal/openjd/validate_conformance_test.go new file mode 100644 index 00000000..43c51c31 --- /dev/null +++ b/internal/openjd/validate_conformance_test.go @@ -0,0 +1,361 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package openjd_test + +import ( + "strings" + "testing" + + "github.com/uberware/sqi/internal/openjd" +) + +func TestValidate_OnRunCommandRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: NoCommandJob +steps: + - name: Step1 + script: + actions: + onRun: + args: ["nothing"] +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/steps/0/script/actions/onRun/command") +} + +func TestValidate_EnvironmentActionCommandRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: EnvNoCommandJob +jobEnvironments: + - name: Setup + script: + actions: + onEnter: + args: ["nothing"] +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/jobEnvironments/0/script/actions/onEnter/command") +} + +func TestValidate_StepScriptRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: NoScriptJob +steps: + - name: Step1 + description: a step with no script at all +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/steps/0/script") +} + +func TestValidate_EnvironmentOnEnterRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: OnExitOnlyJob +jobEnvironments: + - name: Teardown + script: + actions: + onExit: + command: cleanup.sh +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "/jobEnvironments/0/script/actions/onEnter") +} + +func TestValidate_EnvironmentNeedsScriptOrVariables(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: EmptyEnvJob +jobEnvironments: + - name: Nothing +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "at least one of script or variables") +} + +func TestValidate_RangeConstraintRequired(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: ChunkNoConstraintJob +extensions: [TASK_CHUNKING] +steps: + - name: Step1 + parameterSpace: + taskParameterDefinitions: + - name: Frame + type: CHUNK[INT] + range: "1-10" + chunks: + defaultTaskCount: 2 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + assertValidationContains(t, tmpl, "chunks/rangeConstraint") +} + +func TestValidate_RangeConstraintValue(t *testing.T) { + for _, tc := range []struct { + name string + value string + valid bool + }{ + {"contiguous", "CONTIGUOUS", true}, + {"noncontiguous", "NONCONTIGUOUS", true}, + {"garbage", "FOO", false}, + {"lowercase", "contiguous", false}, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: ChunkConstraintJob +extensions: [TASK_CHUNKING] +steps: + - name: Step1 + parameterSpace: + taskParameterDefinitions: + - name: Frame + type: CHUNK[INT] + range: "1-10" + chunks: + defaultTaskCount: 2 + rangeConstraint: ` + tc.value + ` + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := len(errs) == 0 + if got != tc.valid { + t.Fatalf("valid = %v, want %v (errs: %v)", got, tc.valid, errs) + } + }) + } +} + +// Structural host-requirement checks must survive EnforceLimits: false. Before +// this task they lived in validateHostRequirementLimits, reachable only from +// validateLimits, so disabling limits silently disabled correctness too. +func TestValidate_HostRequirementStructuralChecksSurviveDisabledLimits(t *testing.T) { + for _, tc := range []struct { + name string + hr string + want string + }{ + { + name: "empty requirements block", + hr: " hostRequirements: {}", + want: "at least one amount or attribute", + }, + { + name: "attribute with neither anyOf nor allOf", + hr: ` hostRequirements: + attributes: + - name: attr.worker.os.family`, + want: "at least one of anyOf or allOf", + }, + { + name: "amount with a malformed capability name", + hr: ` hostRequirements: + amounts: + - name: "not a valid name!" + min: 1`, + want: "amount", + }, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: HostReqJob +steps: + - name: Step1 +` + tc.hr + ` + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + found := false + for _, e := range errs { + if strings.Contains(e.Error(), tc.want) { + found = true + } + } + if !found { + t.Fatalf("expected an error containing %q with limits disabled; got: %v", tc.want, errs) + } + }) + } +} + +// TestValidate_ReservedCapabilityValueChecksSurviveDisabledLimits guards the +// reserved-value half of the host-requirement split: validateReservedAmounts +// and validateReservedAttributes are value-domain checks with no size or +// count component (a template asking for vcpu: 0 is malformed, not +// oversized), so they must run even when EnforceLimits is false. Before this +// split they were only reachable through the gated validateHostRequirementLimits. +func TestValidate_ReservedCapabilityValueChecksSurviveDisabledLimits(t *testing.T) { + for _, tc := range []struct { + name string + hr string + want string + }{ + { + name: "reserved amount below its minimum", + hr: ` hostRequirements: + amounts: + - name: amount.worker.vcpu + min: 0`, + want: "below the reserved minimum", + }, + { + name: "reserved attribute with a disallowed value", + hr: ` hostRequirements: + attributes: + - name: attr.worker.os.family + anyOf: [plan9]`, + want: "not allowed for reserved attribute", + }, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: ReservedValueJob +steps: + - name: Step1 +` + tc.hr + ` + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + found := false + for _, e := range errs { + if strings.Contains(e.Error(), tc.want) { + found = true + } + } + if !found { + t.Fatalf("expected an error containing %q with limits disabled; got: %v", tc.want, errs) + } + }) + } +} + +// TestValidate_ReservedCapabilityValueChecksNotDoubleReported is the +// double-reporting guard: under EnforceLimits: true, a reserved amount below +// its minimum and a reserved attribute with a disallowed value must each be +// reported exactly once, at their specific pointer -- not twice from both +// the always-run structural path and the gated limits path. +func TestValidate_ReservedCapabilityValueChecksNotDoubleReported(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: ReservedDoubleReportJob +steps: + - name: Step1 + hostRequirements: + amounts: + - name: amount.worker.vcpu + min: 0 + attributes: + - name: attr.worker.os.family + anyOf: [plan9] + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) // EnforceLimits: true + + countAt := func(pointer string) int { + n := 0 + for _, e := range errs { + if e.Pointer == pointer { + n++ + } + } + return n + } + + const amountPtr = "/steps/0/hostRequirements/amounts/0/min" + const attrPtr = "/steps/0/hostRequirements/attributes/0/anyOf/0" + + if n := countAt(amountPtr); n != 1 { + t.Errorf("expected exactly 1 error at %s, got %d; errs: %v", amountPtr, n, errs) + } + if n := countAt(attrPtr); n != 1 { + t.Errorf("expected exactly 1 error at %s, got %d; errs: %v", attrPtr, n, errs) + } +} + +// TestValidate_HostRequirementCapabilityNameLengthStaysGated proves the move +// took only the reserved-value checks and left a genuinely quantitative cap +// (capability name length) behind the EnforceLimits gate. The name here is +// not a reserved capability name, so only the length cap could fire. +func TestValidate_HostRequirementCapabilityNameLengthStaysGated(t *testing.T) { + longName := "amount." + strings.Repeat("x", 101) + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: CapNameLenJob +steps: + - name: Step1 + hostRequirements: + amounts: + - name: ` + longName + ` + min: 5 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + + errs := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: false}) + for _, e := range errs { + if strings.Contains(e.Error(), "at most 100 characters") { + t.Fatalf("expected the capability name length cap to stay gated with limits disabled; got: %v", errs) + } + } + + errs = openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: true}) + found := false + for _, e := range errs { + if strings.Contains(e.Error(), "at most 100 characters") { + found = true + } + } + if !found { + t.Fatalf("expected the capability name length cap to fire with limits enabled; got: %v", errs) + } +} diff --git a/internal/openjd/validate_controls_test.go b/internal/openjd/validate_controls_test.go new file mode 100644 index 00000000..692a796a --- /dev/null +++ b/internal/openjd/validate_controls_test.go @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: AGPL-3.0-or-later + +package openjd_test + +import ( + "strings" + "testing" + + "github.com/uberware/sqi/internal/openjd" +) + +// pathParamTemplate builds a template with one PATH job parameter carrying the +// given userInterface control. +func pathParamTemplate(control string) string { + return ` +specificationVersion: jobtemplate-2023-09 +name: PathControlJob +parameterDefinitions: + - name: ScenePath + type: PATH + objectType: FILE + dataFlow: IN + userInterface: { control: ` + control + `, label: Scene } +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` +} + +func TestValidate_ChooserControlsAccepted(t *testing.T) { + for _, control := range []string{"CHOOSE_INPUT_FILE", "CHOOSE_OUTPUT_FILE", "CHOOSE_DIRECTORY"} { + t.Run(control, func(t *testing.T) { + tmpl := mustParse(t, pathParamTemplate(control)) + if errs := openjd.Validate(tmpl); len(errs) > 0 { + t.Fatalf("expected %s to be valid on a PATH parameter; got: %v", control, errs) + } + }) + } +} + +// The full control vocabulary checked against every parameter type. Every pair +// marked invalid here is accepted by sqi before this task. +func TestValidate_ControlsScopedToParameterType(t *testing.T) { + for _, tc := range []struct { + paramType string + control string + valid bool + }{ + {"STRING", "LINE_EDIT", true}, + {"STRING", "MULTILINE_EDIT", true}, + {"STRING", "HIDDEN", true}, + {"STRING", "SPIN_BOX", false}, + {"STRING", "CHOOSE_INPUT_FILE", false}, + {"PATH", "CHOOSE_INPUT_FILE", true}, + {"PATH", "CHOOSE_OUTPUT_FILE", true}, + {"PATH", "CHOOSE_DIRECTORY", true}, + {"PATH", "HIDDEN", true}, + {"PATH", "LINE_EDIT", false}, + {"PATH", "MULTILINE_EDIT", false}, + {"PATH", "CHECK_BOX", false}, + {"INT", "SPIN_BOX", true}, + {"INT", "HIDDEN", true}, + {"INT", "LINE_EDIT", false}, + {"INT", "CHECK_BOX", false}, + {"FLOAT", "SPIN_BOX", true}, + {"FLOAT", "MULTILINE_EDIT", false}, + // CHIP_INPUT is an sqi invention appearing nowhere in the spec, at any + // type. These rows go red here, where the vocabulary actually changes, + // and stay as the permanent guard against anyone re-adding it. Task 9 + // then deletes the dead constant with no new test of its own. + {"STRING", "CHIP_INPUT", false}, + {"PATH", "CHIP_INPUT", false}, + {"INT", "CHIP_INPUT", false}, + {"FLOAT", "CHIP_INPUT", false}, + } { + t.Run(tc.paramType+"/"+tc.control, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: ScopedControlJob +parameterDefinitions: + - name: P + type: ` + tc.paramType + ` + userInterface: { control: ` + tc.control + `, label: P } +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := len(errs) == 0 + if got != tc.valid { + t.Fatalf("%s on %s: valid = %v, want %v (errs: %v)", + tc.control, tc.paramType, got, tc.valid, errs) + } + }) + } +} + +func TestValidate_LabelLengthBounds(t *testing.T) { + // No minimum-length case: getString (parse.go:693-697) returns "" for both + // an absent key and an explicitly empty one, and Label is a plain string, so + // `label: ""` and no label at all are the same state after parsing -- and the + // absent case is legal (label is @optional; the spec prescribes falling back + // to the parameter name). There is no representable "present but empty" to + // reject. Ruled 2026-07-24: enforce the maximum only. + for _, tc := range []struct { + name string + label string + valid bool + }{ + {"empty is indistinguishable from absent, so valid", "", true}, + {"one char", "S", true}, + {"exactly 64", strings.Repeat("a", 64), true}, + {"65 is too long", strings.Repeat("a", 65), false}, + } { + t.Run(tc.name, func(t *testing.T) { + yaml := ` +specificationVersion: jobtemplate-2023-09 +name: LabelJob +parameterDefinitions: + - name: P + type: STRING + userInterface: { control: LINE_EDIT, label: "` + tc.label + `" } +steps: + - name: Step1 + script: + actions: + onRun: + command: echo +` + tmpl := mustParse(t, yaml) + errs := openjd.Validate(tmpl) + got := len(errs) == 0 + if got != tc.valid { + t.Fatalf("label len %d: valid = %v, want %v (errs: %v)", + len(tc.label), got, tc.valid, errs) + } + }) + } +} diff --git a/internal/openjd/validate_extensions_test.go b/internal/openjd/validate_extensions_test.go index e5af9085..f111f25f 100644 --- a/internal/openjd/validate_extensions_test.go +++ b/internal/openjd/validate_extensions_test.go @@ -28,13 +28,18 @@ func chunkTemplate(extensions []string) *openjd.JobTemplate { Steps: []openjd.StepTemplate{ { Name: "Render", + Script: &openjd.StepScript{ + Actions: openjd.StepActions{ + OnRun: openjd.Action{Command: "echo"}, + }, + }, ParameterSpace: &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ { Name: "Frames", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), - Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}, + Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}, }, }, }, @@ -247,26 +252,36 @@ func TestValidateExtensions_TwoChunkSteps_NoDeclaration(t *testing.T) { Steps: []openjd.StepTemplate{ { Name: "RenderA", + Script: &openjd.StepScript{ + Actions: openjd.StepActions{ + OnRun: openjd.Action{Command: "echo"}, + }, + }, ParameterSpace: &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ { Name: "FramesA", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), - Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}, + Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}, }, }, }, }, { Name: "RenderB", + Script: &openjd.StepScript{ + Actions: openjd.StepActions{ + OnRun: openjd.Action{Command: "echo"}, + }, + }, ParameterSpace: &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ { Name: "FramesB", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("11-20"), - Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}, + Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}, }, }, }, @@ -298,9 +313,12 @@ func TestValidateExtensions_TwoChunkSteps_NoDeclaration(t *testing.T) { // TestValidateExtensions_UnsupportedPlusDeclaredChunking confirms that when an // unsupported extension is present alongside TASK_CHUNKING, and CHUNK[INT] is -// used, we get exactly one error (for the unsupported extension) and no -// TASK_CHUNKING-required error. This confirms that the declared-set is -// populated even for unsupported entries. +// used, the unsupported extension at /extensions/0 produces exactly one error +// at that pointer, and no TASK_CHUNKING-required error is produced anywhere +// (since TASK_CHUNKING IS declared). This confirms that the declared-set is +// populated even for unsupported entries. It does not assert the total error +// count for the template, only what is checked at /extensions/0 and for the +// TASK_CHUNKING-required message. func TestValidateExtensions_UnsupportedPlusDeclaredChunking(t *testing.T) { tmpl := &openjd.JobTemplate{ SpecificationVersion: openjd.SpecVersion, @@ -309,13 +327,18 @@ func TestValidateExtensions_UnsupportedPlusDeclaredChunking(t *testing.T) { Steps: []openjd.StepTemplate{ { Name: "Render", + Script: &openjd.StepScript{ + Actions: openjd.StepActions{ + OnRun: openjd.Action{Command: "echo"}, + }, + }, ParameterSpace: &openjd.StepParameterSpace{ TaskParameterDefinitions: []openjd.TaskParamDefinition{ { Name: "Frames", Type: openjd.TaskParamTypeChunkInt, RangeExpr: new("1-10"), - Chunks: &openjd.TaskChunks{DefaultTaskCount: 2}, + Chunks: &openjd.TaskChunks{DefaultTaskCount: 2, RangeConstraint: "CONTIGUOUS"}, }, }, }, @@ -343,7 +366,7 @@ func TestValidateExtensions_UnsupportedPlusDeclaredChunking(t *testing.T) { } } - // Count total errors related to extensions (should be exactly 1) + // Count errors at /extensions/0 (should be exactly 1) var extErrors int for _, err := range errs { if err.Pointer == "/extensions/0" { @@ -351,6 +374,6 @@ func TestValidateExtensions_UnsupportedPlusDeclaredChunking(t *testing.T) { } } if extErrors != 1 { - t.Errorf("expected exactly 1 extension error; got %d: %v", extErrors, errs) + t.Errorf("expected exactly 1 extension error at /extensions/0; got %d: %v", extErrors, errs) } } diff --git a/internal/openjd/validate_limits_test.go b/internal/openjd/validate_limits_test.go index 143b5002..dab866a9 100644 --- a/internal/openjd/validate_limits_test.go +++ b/internal/openjd/validate_limits_test.go @@ -61,6 +61,12 @@ func TestValidate_Limits_Gated(t *testing.T) { name string mutate func(*openjd.JobTemplate) wantPtr string // "" means the mutation is valid (no error even when enforcing) + // structural marks a case whose check was split out of validateLimits + // (host-requirement structural correctness, always runs regardless of + // EnforceLimits — see the invariant documented on + // openjd.ValidateOptions). For these, wantPtr must fire even with + // EnforceLimits=false instead of being gated off. + structural bool }{ // ── job parameterDefinitions: upper bound 50 ── { @@ -99,7 +105,10 @@ func TestValidate_Limits_Gated(t *testing.T) { { name: "job env name 64 ok", mutate: func(t *openjd.JobTemplate) { - t.JobEnvironments = []openjd.Environment{{Name: strings.Repeat("e", 64)}} + t.JobEnvironments = []openjd.Environment{{ + Name: strings.Repeat("e", 64), + Variables: map[string]string{"K": "V"}, + }} }, }, { @@ -114,7 +123,10 @@ func TestValidate_Limits_Gated(t *testing.T) { { name: "step env name 64 ok", mutate: func(t *openjd.JobTemplate) { - t.Steps[0].StepEnvironments = []openjd.Environment{{Name: strings.Repeat("e", 64)}} + t.Steps[0].StepEnvironments = []openjd.Environment{{ + Name: strings.Repeat("e", 64), + Variables: map[string]string{"K": "V"}, + }} }, }, { @@ -233,7 +245,8 @@ func TestValidate_Limits_Gated(t *testing.T) { mutate: func(t *openjd.JobTemplate) { t.Steps[0].HostRequirements = &openjd.HostRequirements{} }, - wantPtr: "/steps/0/hostRequirements", + wantPtr: "/steps/0/hostRequirements", + structural: true, }, // ── capability name length: 1–100 ── @@ -261,7 +274,8 @@ func TestValidate_Limits_Gated(t *testing.T) { Amounts: []openjd.AmountRequirement{{Name: "", Min: new("1")}}, } }, - wantPtr: "/steps/0/hostRequirements/amounts/0/name", + wantPtr: "/steps/0/hostRequirements/amounts/0/name", + structural: true, }, { name: "attribute name 101 error", @@ -307,7 +321,8 @@ func TestValidate_Limits_Gated(t *testing.T) { Attributes: []openjd.AttributeRequirement{{Name: "attr.x"}}, } }, - wantPtr: "/steps/0/hostRequirements/attributes/0", + wantPtr: "/steps/0/hostRequirements/attributes/0", + structural: true, }, // ── INT range overlap ── @@ -359,14 +374,20 @@ func TestValidate_Limits_Gated(t *testing.T) { t.Fatalf("EnforceLimits=true: expected pointer %q, got %v", tc.wantPtr, errsOn) } - // Not enforcing: the gated limit must not fire. + // Not enforcing: a gated limit must not fire, but a structural + // check (split out of validateLimits into the always-run path) + // must still fire. tmplOff := mustParse(t, minimalValidYAML()) tc.mutate(tmplOff) errsOff := openjd.ValidateWithOptions(tmplOff, openjd.ValidateOptions{EnforceLimits: false}) - if tc.wantPtr != "" && containsPointer(errsOff, tc.wantPtr) { + switch { + case tc.structural: + if !containsPointer(errsOff, tc.wantPtr) { + t.Fatalf("EnforceLimits=false: structural pointer %q should still fire, got %v", tc.wantPtr, errsOff) + } + case tc.wantPtr != "" && containsPointer(errsOff, tc.wantPtr): t.Fatalf("EnforceLimits=false: limit pointer %q should be gated off, got %v", tc.wantPtr, errsOff) - } - if tc.wantPtr == "" && len(errsOff) != 0 { + case tc.wantPtr == "" && len(errsOff) != 0: t.Fatalf("EnforceLimits=false: expected no errors, got %v", errsOff) } }) diff --git a/internal/openjd/validate_reserved_test.go b/internal/openjd/validate_reserved_test.go index 3240ded8..a8a07017 100644 --- a/internal/openjd/validate_reserved_test.go +++ b/internal/openjd/validate_reserved_test.go @@ -4,10 +4,12 @@ package openjd_test // Tests for reserved capability-name and attribute-value checks. // -// All checks are gated behind EnforceLimits (part of the limits bucket). -// Each table case is exercised twice: -// - EnforceLimits=true → if wantPtr != "", the error must appear at that pointer. -// - EnforceLimits=false → the reserved-name error must NOT appear, proving the gate. +// These are value-domain correctness checks, not size/count caps, so they +// run unconditionally from validateHostRequirements regardless of +// EnforceLimits — see the invariant documented on ValidateOptions. Each +// table case is exercised under both EnforceLimits=true and +// EnforceLimits=false and must produce the identical result either way: if +// wantPtr != "", the error must appear at that pointer in both modes. import ( "testing" @@ -15,7 +17,7 @@ import ( "github.com/uberware/sqi/internal/openjd" ) -func TestValidate_ReservedNames_Gated(t *testing.T) { +func TestValidate_ReservedNames_AlwaysEnforced(t *testing.T) { cases := []struct { name string mutate func(*openjd.JobTemplate) @@ -312,29 +314,22 @@ func TestValidate_ReservedNames_Gated(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - // ── EnforceLimits=true: reserved-name errors must fire ──────────── - tmplOn := mustParse(t, minimalValidYAML()) - tc.mutate(tmplOn) - errsOn := openjd.ValidateWithOptions(tmplOn, openjd.ValidateOptions{EnforceLimits: true}) + // Reserved-value checks are structural correctness, not a gated + // limit, so both EnforceLimits settings must behave identically. + for _, enforce := range []bool{true, false} { + tmpl := mustParse(t, minimalValidYAML()) + tc.mutate(tmpl) + errs := openjd.ValidateWithOptions(tmpl, openjd.ValidateOptions{EnforceLimits: enforce}) - if tc.wantPtr == "" { - if len(errsOn) != 0 { - t.Fatalf("EnforceLimits=true: expected no errors, got %v", errsOn) + if tc.wantPtr == "" { + if len(errs) != 0 { + t.Fatalf("EnforceLimits=%v: expected no errors, got %v", enforce, errs) + } + continue + } + if !containsPointer(errs, tc.wantPtr) { + t.Fatalf("EnforceLimits=%v: expected pointer %q, got %v", enforce, tc.wantPtr, errs) } - } else if !containsPointer(errsOn, tc.wantPtr) { - t.Fatalf("EnforceLimits=true: expected pointer %q, got %v", tc.wantPtr, errsOn) - } - - // ── EnforceLimits=false: reserved-name check must be gated off ─── - tmplOff := mustParse(t, minimalValidYAML()) - tc.mutate(tmplOff) - errsOff := openjd.ValidateWithOptions(tmplOff, openjd.ValidateOptions{EnforceLimits: false}) - - if tc.wantPtr != "" && containsPointer(errsOff, tc.wantPtr) { - t.Fatalf("EnforceLimits=false: reserved-name pointer %q should be gated off, got %v", tc.wantPtr, errsOff) - } - if tc.wantPtr == "" && len(errsOff) != 0 { - t.Fatalf("EnforceLimits=false: expected no errors, got %v", errsOff) } }) } diff --git a/internal/openjd/validate_userinterface_test.go b/internal/openjd/validate_userinterface_test.go index c8ad1046..081d180f 100644 --- a/internal/openjd/validate_userinterface_test.go +++ b/internal/openjd/validate_userinterface_test.go @@ -78,18 +78,10 @@ func TestValidateUserInterface(t *testing.T) { name: "decimals without spinbox", param: JobParameter{ Name: "Q", Type: JobParamTypeFloat, - UserInterface: &ParameterUserInterface{Control: ControlLineEdit, Decimals: new(2)}, + UserInterface: &ParameterUserInterface{Control: ControlHidden, Decimals: new(2)}, }, wantPointer: "/parameterDefinitions/0/userInterface/decimals", }, - { - name: "singleStepRemoval without chip input", - param: JobParameter{ - Name: "Q", Type: JobParamTypeString, - UserInterface: &ParameterUserInterface{Control: ControlLineEdit, SingleStepRemoval: new(true)}, - }, - wantPointer: "/parameterDefinitions/0/userInterface/singleStepRemoval", - }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -182,12 +174,54 @@ func TestValidateUIGroupLabelLengthLimit(t *testing.T) { } } +// TestValidateUIGroupLabelExactBoundary proves the groupLabel cap is enforced +// at exactly maxUIGroupLabelLen characters, not merely somewhere below a +// clearly-over-limit value. TestValidateUIGroupLabelLengthLimit above only +// exercises a 257-character groupLabel, which would still fail an off-by-one +// in the comparison (e.g. >= instead of >); this test pins the boundary +// itself: maxUIGroupLabelLen characters must be accepted, maxUIGroupLabelLen+1 +// must be rejected. +func TestValidateUIGroupLabelExactBoundary(t *testing.T) { + base := func(groupLabel string) *JobTemplate { + return &JobTemplate{ + SpecificationVersion: SpecVersion, + Name: "x", + ParameterDefinitions: []JobParameter{{ + Name: "Q", Type: JobParamTypeString, + UserInterface: &ParameterUserInterface{Control: ControlLineEdit, GroupLabel: groupLabel}, + }}, + Steps: []StepTemplate{{Name: "A"}}, + } + } + + // Exactly maxUIGroupLabelLen characters — must be accepted. + groupLabelAtLimit := strings.Repeat("x", maxUIGroupLabelLen) + if errs := ValidateWithOptions(base(groupLabelAtLimit), ValidateOptions{EnforceLimits: true}); strings.Contains(errs.Error(), "groupLabel") { + t.Errorf("%d-character groupLabel was incorrectly rejected; got %v", maxUIGroupLabelLen, errs) + } + + // maxUIGroupLabelLen+1 characters — must be rejected with a pointer on /groupLabel. + groupLabelOverLimit := strings.Repeat("x", maxUIGroupLabelLen+1) + errs := ValidateWithOptions(base(groupLabelOverLimit), ValidateOptions{EnforceLimits: true}) + found := false + for _, e := range errs { + if e.Pointer == "/parameterDefinitions/0/userInterface/groupLabel" { + found = true + break + } + } + if !found { + t.Errorf("%d-character groupLabel was not flagged at expected pointer; got %v", maxUIGroupLabelLen+1, errs) + } +} + // TestValidateUILabelRuneCounting proves the label limit is counted in Unicode // runes (characters), not bytes. "é" (U+00E9) is 2 bytes but 1 rune. // -// With byte counting (the old len() approach) a 256-rune label of "é" would be -// 512 bytes and would be wrongly rejected. With correct rune counting it must -// be accepted; 257 runes must be rejected. +// With byte counting (the old len() approach) a maxUILabelLen-rune label of +// "é" would be 2*maxUILabelLen bytes and would be wrongly rejected. With +// correct rune counting it must be accepted; maxUILabelLen+1 runes must be +// rejected. func TestValidateUILabelRuneCounting(t *testing.T) { base := func(label string) *JobTemplate { return &JobTemplate{ @@ -201,15 +235,15 @@ func TestValidateUILabelRuneCounting(t *testing.T) { } } - // Exactly 256 multibyte runes — must be accepted. - label256 := strings.Repeat("é", 256) // 512 bytes, 256 runes - if errs := ValidateWithOptions(base(label256), ValidateOptions{EnforceLimits: true}); strings.Contains(errs.Error(), "label") { - t.Errorf("256-rune multibyte label was incorrectly rejected; got %v", errs) + // Exactly maxUILabelLen multibyte runes — must be accepted. + labelAtLimit := strings.Repeat("é", maxUILabelLen) // 2*maxUILabelLen bytes, maxUILabelLen runes + if errs := ValidateWithOptions(base(labelAtLimit), ValidateOptions{EnforceLimits: true}); strings.Contains(errs.Error(), "label") { + t.Errorf("%d-rune multibyte label was incorrectly rejected; got %v", maxUILabelLen, errs) } - // 257 multibyte runes — must be rejected with a pointer on /label. - label257 := strings.Repeat("é", 257) - errs := ValidateWithOptions(base(label257), ValidateOptions{EnforceLimits: true}) + // maxUILabelLen+1 multibyte runes — must be rejected with a pointer on /label. + labelOverLimit := strings.Repeat("é", maxUILabelLen+1) + errs := ValidateWithOptions(base(labelOverLimit), ValidateOptions{EnforceLimits: true}) found := false for _, e := range errs { if e.Pointer == "/parameterDefinitions/0/userInterface/label" { @@ -218,6 +252,6 @@ func TestValidateUILabelRuneCounting(t *testing.T) { } } if !found { - t.Errorf("257-rune multibyte label was not flagged at expected pointer; got %v", errs) + t.Errorf("%d-rune multibyte label was not flagged at expected pointer; got %v", maxUILabelLen+1, errs) } } diff --git a/mkdocs.yml b/mkdocs.yml index f75333a6..9ebc4349 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -63,6 +63,7 @@ nav: - Compute Locations: compute-locations.md - Operations: - Overview: operations.md + - Authentication: auth.md - Observability: observability.md - Release Runbook: release-runbook.md - Reference: @@ -70,6 +71,7 @@ nav: - Overview: api.md - OpenJD: - Submission Guide: openjd-submission.md + - Spec Conformance: openjd-conformance.md - Extensions Overview: openjd-extensions.md - Extension Details: - Task Chunking: openjd-extensions/task-chunking.md @@ -88,3 +90,10 @@ markdown_extensions: - pymdownx.superfences - toc: permalink: true + # GitHub-compatible heading slugs. The docs are read both on the MkDocs + # site and directly on GitHub, and every intra-doc anchor in the tree was + # written GitHub-style — a heading like "Roles & permissions" anchors as + # "roles--permissions" (the removed "&" leaves a double dash), where + # Python-Markdown's default slugify would produce "roles-permissions". + # Without this, those links resolve on GitHub and 404 on the built site. + slugify: !!python/object/apply:pymdownx.slugs.slugify {kwds: {case: lower}} diff --git a/presets/sqi/blender-batch-render.yaml b/presets/sqi/blender-batch-render.yaml index c361e145..e0ff3ffc 100644 --- a/presets/sqi/blender-batch-render.yaml +++ b/presets/sqi/blender-batch-render.yaml @@ -18,7 +18,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -27,7 +27,7 @@ template: objectType: FILE dataFlow: OUT default: "" - userInterface: { control: LINE_EDIT, label: Output Path } + userInterface: { control: CHOOSE_OUTPUT_FILE, label: Output Path } steps: - name: Render hostRequirements: diff --git a/presets/sqi/houdini-rop-render.yaml b/presets/sqi/houdini-rop-render.yaml index ee6a7eaa..0cc367c0 100644 --- a/presets/sqi/houdini-rop-render.yaml +++ b/presets/sqi/houdini-rop-render.yaml @@ -16,7 +16,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -36,6 +36,7 @@ template: range: "{{Param.Frames}}" chunks: defaultTaskCount: 10 + rangeConstraint: CONTIGUOUS script: embeddedFiles: - name: render_chunk.py diff --git a/presets/sqi/maya-layer-render.yaml b/presets/sqi/maya-layer-render.yaml index 98a8b761..6f12d38e 100644 --- a/presets/sqi/maya-layer-render.yaml +++ b/presets/sqi/maya-layer-render.yaml @@ -18,7 +18,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -26,7 +26,7 @@ template: type: PATH objectType: DIRECTORY dataFlow: OUT - userInterface: { control: LINE_EDIT, label: Output Directory } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory } - name: Renderer type: STRING default: file diff --git a/presets/sqi/maya-scene-render.yaml b/presets/sqi/maya-scene-render.yaml index 1da64633..c41f2ffd 100644 --- a/presets/sqi/maya-scene-render.yaml +++ b/presets/sqi/maya-scene-render.yaml @@ -19,7 +19,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -27,7 +27,7 @@ template: type: PATH objectType: DIRECTORY dataFlow: OUT - userInterface: { control: LINE_EDIT, label: Output Directory } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory } - name: Renderer type: STRING default: file diff --git a/presets/sqi/nuke-script-render.yaml b/presets/sqi/nuke-script-render.yaml index 295be30c..07762dea 100644 --- a/presets/sqi/nuke-script-render.yaml +++ b/presets/sqi/nuke-script-render.yaml @@ -17,7 +17,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -34,6 +34,7 @@ template: range: "{{Param.Frames}}" chunks: defaultTaskCount: 10 + rangeConstraint: CONTIGUOUS script: actions: onRun: diff --git a/presets/sqi/nuke-write-render.yaml b/presets/sqi/nuke-write-render.yaml index f539b7ef..f0d9aa0a 100644 --- a/presets/sqi/nuke-write-render.yaml +++ b/presets/sqi/nuke-write-render.yaml @@ -16,7 +16,7 @@ template: type: PATH objectType: FILE dataFlow: IN - userInterface: { control: LINE_EDIT, label: Scene File } + userInterface: { control: CHOOSE_INPUT_FILE, label: Scene File } - name: Frames type: STRING userInterface: { control: LINE_EDIT, label: Frame Range } @@ -36,6 +36,7 @@ template: range: "{{Param.Frames}}" chunks: defaultTaskCount: 10 + rangeConstraint: CONTIGUOUS script: actions: onRun: diff --git a/presets/testing/test-render-bash.yaml b/presets/testing/test-render-bash.yaml index e9ae6005..01cb244f 100644 --- a/presets/testing/test-render-bash.yaml +++ b/presets/testing/test-render-bash.yaml @@ -46,7 +46,7 @@ template: objectType: DIRECTORY dataFlow: OUT default: "" - userInterface: { control: LINE_EDIT, label: Output Directory, groupLabel: Output } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory, groupLabel: Output } steps: - name: render parameterSpace: diff --git a/presets/testing/test-render-powershell.yaml b/presets/testing/test-render-powershell.yaml index 06ac35bb..4f59ea1d 100644 --- a/presets/testing/test-render-powershell.yaml +++ b/presets/testing/test-render-powershell.yaml @@ -46,7 +46,7 @@ template: objectType: DIRECTORY dataFlow: OUT default: "" - userInterface: { control: LINE_EDIT, label: Output Directory, groupLabel: Output } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory, groupLabel: Output } steps: - name: render hostRequirements: diff --git a/presets/testing/test-steps-bash.yaml b/presets/testing/test-steps-bash.yaml index 8c02fe81..b8609f0a 100644 --- a/presets/testing/test-steps-bash.yaml +++ b/presets/testing/test-steps-bash.yaml @@ -44,7 +44,7 @@ template: objectType: DIRECTORY dataFlow: OUT default: "" - userInterface: { control: LINE_EDIT, label: Output Directory, groupLabel: Output } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory, groupLabel: Output } steps: - name: render parameterSpace: diff --git a/presets/testing/test-steps-powershell.yaml b/presets/testing/test-steps-powershell.yaml index bfb11a30..306cc1cc 100644 --- a/presets/testing/test-steps-powershell.yaml +++ b/presets/testing/test-steps-powershell.yaml @@ -45,7 +45,7 @@ template: objectType: DIRECTORY dataFlow: OUT default: "" - userInterface: { control: LINE_EDIT, label: Output Directory, groupLabel: Output } + userInterface: { control: CHOOSE_DIRECTORY, label: Output Directory, groupLabel: Output } steps: - name: render hostRequirements: diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 094474a7..7dc55a28 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -429,9 +429,11 @@ export type ControlType = | 'MULTILINE_EDIT' | 'DROPDOWN_LIST' | 'CHECK_BOX' - | 'CHIP_INPUT' | 'HIDDEN' | 'SPIN_BOX' + | 'CHOOSE_INPUT_FILE' + | 'CHOOSE_OUTPUT_FILE' + | 'CHOOSE_DIRECTORY' /** OpenJD base-spec userInterface hints on a job parameter. */ export interface ParameterUserInterface { @@ -439,7 +441,12 @@ export interface ParameterUserInterface { label: string group_label: string decimals: number | null - single_step_removal: boolean | null +} + +/** One named file type offered by a PATH parameter's chooser dialog. */ +export interface PathFileFilter { + label: string + patterns: string[] } /** Parsed job parameter from GET /products/{name}/parameters. */ @@ -456,6 +463,8 @@ export interface ProductParameter { object_type: string data_flow: string user_interface: ParameterUserInterface | null + file_filters: PathFileFilter[] | null + file_filter_default: PathFileFilter | null } /** Input for the submitProductJob mutation. */ diff --git a/web/src/components/ProductParamField.module.css b/web/src/components/ProductParamField.module.css index f1fd7a7b..70f45e73 100644 --- a/web/src/components/ProductParamField.module.css +++ b/web/src/components/ProductParamField.module.css @@ -22,7 +22,7 @@ /* * Theme-aware control styling. Scoped to .field so the generated widgets - * (text/number/select/textarea/chips) follow the palette in both light and + * (text/number/select/textarea) follow the palette in both light and * dark mode instead of falling back to UA defaults. Checkboxes are excluded — * the global stylesheet already themes them. */ diff --git a/web/src/components/ProductParamField.test.tsx b/web/src/components/ProductParamField.test.tsx index 4ad5dbdb..73070ba7 100644 --- a/web/src/components/ProductParamField.test.tsx +++ b/web/src/components/ProductParamField.test.tsx @@ -18,6 +18,8 @@ function param(over: Partial): ProductParameter { object_type: '', data_flow: '', user_interface: null, + file_filters: [], + file_filter_default: null, ...over, } } @@ -51,7 +53,6 @@ describe('ProductParamField', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, })} value="" diff --git a/web/src/components/ProductParamField.tsx b/web/src/components/ProductParamField.tsx index 7e018035..1a816195 100644 --- a/web/src/components/ProductParamField.tsx +++ b/web/src/components/ProductParamField.tsx @@ -48,17 +48,6 @@ export default function ProductParamField({ param, value, error, onChange }: Pro return ( onChange(e.target.value)} /> ) - case 'chips': - // v1: comma-separated entry; server receives the raw string. - return ( - onChange(e.target.value)} - placeholder="comma-separated" - /> - ) default: return ( onChange(e.target.value)} /> diff --git a/web/src/lib/productForm.test.ts b/web/src/lib/productForm.test.ts index 9ff5c64c..b75dc371 100644 --- a/web/src/lib/productForm.test.ts +++ b/web/src/lib/productForm.test.ts @@ -17,6 +17,8 @@ function param(over: Partial): ProductParameter { object_type: '', data_flow: '', user_interface: null, + file_filters: [], + file_filter_default: null, ...over, } } @@ -31,7 +33,6 @@ describe('selectWidget', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, allowed_values: ['a', 'b'], }), @@ -45,7 +46,6 @@ describe('selectWidget', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, allowed_values: ['off', 'on'], }), @@ -59,24 +59,10 @@ describe('selectWidget', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, }), ), ).toBe('textarea') - expect( - selectWidget( - param({ - user_interface: { - control: 'CHIP_INPUT', - label: '', - group_label: '', - decimals: null, - single_step_removal: null, - }, - }), - ), - ).toBe('chips') expect( selectWidget( param({ @@ -85,7 +71,6 @@ describe('selectWidget', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, type: 'INT', }), @@ -99,13 +84,34 @@ describe('selectWidget', () => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, }), ), ).toBe('hidden') }) + it('renders chooser controls as text inputs until a picker exists', () => { + for (const control of [ + 'CHOOSE_INPUT_FILE', + 'CHOOSE_OUTPUT_FILE', + 'CHOOSE_DIRECTORY', + ] as const) { + expect( + selectWidget( + param({ + type: 'PATH', + user_interface: { + control, + label: '', + group_label: '', + decimals: null, + }, + }), + ), + ).toBe('text') + } + }) + it('falls back by type when no userInterface', () => { expect(selectWidget(param({ allowed_values: ['a', 'b'] }))).toBe('select') expect(selectWidget(param({ type: 'INT' }))).toBe('number') @@ -126,7 +132,6 @@ describe('helpers', () => { label: 'Scene file', group_label: '', decimals: null, - single_step_removal: null, }, }), ), diff --git a/web/src/lib/productForm.ts b/web/src/lib/productForm.ts index c618378e..ed423e0c 100644 --- a/web/src/lib/productForm.ts +++ b/web/src/lib/productForm.ts @@ -1,16 +1,20 @@ // SPDX-License-Identifier: AGPL-3.0-or-later import type { ProductParameter } from '@/api/types' -export type Widget = 'text' | 'textarea' | 'select' | 'checkbox' | 'chips' | 'number' | 'hidden' +export type Widget = 'text' | 'textarea' | 'select' | 'checkbox' | 'number' | 'hidden' const CONTROL_WIDGET: Record = { LINE_EDIT: 'text', MULTILINE_EDIT: 'textarea', DROPDOWN_LIST: 'select', CHECK_BOX: 'checkbox', - CHIP_INPUT: 'chips', SPIN_BOX: 'number', HIDDEN: 'hidden', + // No file-picker widget exists yet; these render as text inputs, which is + // exactly what these fields rendered as before the controls were recognised. + CHOOSE_INPUT_FILE: 'text', + CHOOSE_OUTPUT_FILE: 'text', + CHOOSE_DIRECTORY: 'text', } /** Choose a form widget for a parameter: explicit userInterface control first, diff --git a/web/src/lib/productValidation.test.ts b/web/src/lib/productValidation.test.ts index 5b170ca8..209121be 100644 --- a/web/src/lib/productValidation.test.ts +++ b/web/src/lib/productValidation.test.ts @@ -17,6 +17,8 @@ function param(over: Partial): ProductParameter { object_type: '', data_flow: '', user_interface: null, + file_filters: [], + file_filter_default: null, ...over, } } diff --git a/web/src/pages/ProductSubmit.test.tsx b/web/src/pages/ProductSubmit.test.tsx index c475b972..ff3a5093 100644 --- a/web/src/pages/ProductSubmit.test.tsx +++ b/web/src/pages/ProductSubmit.test.tsx @@ -50,6 +50,8 @@ const h = vi.hoisted(() => { object_type: 'FILE', data_flow: '', user_interface: null, + file_filters: [], + file_filter_default: null, }) const internalParam = () => ({ name: 'InternalPath', @@ -68,8 +70,9 @@ const h = vi.hoisted(() => { label: '', group_label: '', decimals: null, - single_step_removal: null, }, + file_filters: [], + file_filter_default: null, }) const defaultParams = () => [sceneParam(null), internalParam()] const makeJob = (overrides: Partial> = {}) => ({