feat!: enforce resolved-value constraints at template validation by mwiebe · Pull Request #383 · OpenJobDescription/openjd-rs

mwiebe · 2026-09-11T01:13:35Z

What was the problem/requirement? (What/Why)

The 2023-09 Template Schemas place constraints on what several format-string fields resolve to — the spec's own phrasing is "after the format string has been resolved." A job name must resolve to ≤ 128 characters (512 with FEATURE_BUNDLE_1); an attribute capability value to ≤ 100 identifier-like characters; a task-parameter range element to ≤ 1024; an environment variable value to ≤ 2048; notifyPeriodInSeconds to a positive integer ≤ 600. Call these the spec-mandated resolved-value constraints — the fields whose limits the spec applies to the value an interpolation produces, in contrast to fields the spec deliberately leaves uncapped (command, args, embedded-file data).

Before this change, none of them were enforced at template validation time for interpolated values. openjd check evaluates every format-string expression during validation (that is how static type checking works: unknown symbols are unresolved placeholders and evaluation propagates them) — but it threw the results away. Concretely:

name: "{{ 'A' * 600 }}" passed check and failed at job submission.
name: "{{ Param.X }}{{ 'A' * 600 }}" — guaranteed over-limit whatever Param.X is — passed check and job submission logic that only sees the resolved whole.
notifyPeriodInSeconds: "{{ 300 + 400 }}" (statically 700 > 600) passed check and failed on the worker, mid-job.
Environment variable values had their 2048-character limit enforced nowhere — not at validation, submission, or run time.
The TASK_CHUNKING chunks format strings were never validated in the format-string pass at all: a typo'd expression surfaced only at job creation.

Investigating the fix surfaced a second, related problem: resolution itself deviated from the spec's target-type rules. The Expression Language spec (§1.3.2 "Evaluation Within Template Schemas") derives a target type for every field's whole-field expression from its schema context — T for required fields, T? for optional ones (null = field omitted) — and the Template Schemas doc explicitly mandates int? for timeout/notifyPeriodInSeconds and string? for a deferred cancelation mode. Our gates resolved all of these untargeted (render to display text, trim, parse). Observable symptoms: timeout: "{{ 120.0 }}" failed at run time with "must be a positive integer, got Float" where the spec's coercion accepts 120; a whole-field null in a required string field silently became the empty string (which the same fields' minimum-length-1 constraints forbid); an amount bound resolving to null was a parse error instead of "field omitted".

The requirement: enforce these resolved-value constraints at the earliest stage where a violation is knowable, and make validation and resolution agree exactly on what a field accepts — full staging rationale in the design document.

What was the solution? (How)

Three commits, ordered so each layer builds on the previous. The key invariant throughout: a field's validation-time target type must equal its resolution-time target type, or validation would reject values resolution accepts (or vice versa).

fix(sessions)! — spec-mandated targets in the session runtime. resolve_action_timeout, resolve_notify_period_seconds, and resolve_effective_cancelation now pass int?/int?/string? via with_target_type, so whole-field expressions coerce per the spec ({{ 120.0 }} → 120, {{ '600' }} → 600, null → field unset). Multi-segment strings concatenate as before and now trim before parsing, matching the reference implementation's int() leniency.
feat(model)! — schema-derived targets at job creation. Job name, STRING/PATH range elements, and attribute capability values resolve with target string; amount min/max with float? (a whole-field null now means the bound is unset); chunks defaultTaskCount with int and the optional targetRuntimeSeconds with int? (so {{ 4.0 }} coerces to 4); environment variable values with string in the session runtime. Since there is no list → string or null → string coercion, a whole-field list or null in a required string field is now an error instead of silently rendering its display form.
feat(model)! — gate 1: the validation layer. validate_fs gains a per-field ResolvedConstraint applied to the StaticResolution that pass 8 already computes (the feat(expr)!: return StaticResolution from FormatString::validate_expressions #373 mechanism), in two stages:
- Lower bound: min_resolved_string_len is a guaranteed lower bound on every possible run-time resolution (unresolved segments contribute 0), so a bound past the field's limit is a certain violation and fails check without knowing the unresolved parts. This is what catches "{{ Param.X }}{{ 'A' * 600 }}".
- Full value check: when the field is fully static, the exact check job creation or the worker would run executes at check time with the same error messages (notifyPeriodInSeconds must not exceed 600., value 'linuxx' is not valid for attr.worker.os.family., …).
Numeric fields get a soft 100-character bound rather than an exact one — leading zeros are legal and whitespace is trimmed, so no exact maximum exists, but an i64 needs at most 20 characters and nothing reasonable exceeds 100. Standard attribute capabilities get a sharper bound: no resolution longer than the longest allowed value (e.g. 7 for attr.worker.os.family) can ever conform. Literal (non-interpolated) fields are excluded — the existing raw-text passes already check those, and for literals raw text and resolved value coincide.

What is the impact of this change?

No public API changes in any crate: ResolvedConstraint is private to the validation pass, and the sessions functions are pub(crate). openjd-expr is untouched (this consumes the API shipped in feat(expr)!: return StaticResolution from FormatString::validate_expressions #373).
Templates whose format strings statically violate a spec-mandated resolved-value constraint now fail openjd check instead of failing later — or, for environment variable values and chunks fields, instead of never failing.
Behavior changes at resolution time are listed under the breaking-change section below.
Validation cost is unchanged in essence: the constraint checks consume evaluation results pass 8 already produced.

How was this change tested?

Have you run the unit tests?
- Yes — cargo test --workspace: all suites pass, including 2,032 tests in openjd-model and the sessions suites. (Two pre-existing Windows cross-user logon test failures on the development machine are environment-specific and unrelated.)
47 new integration tests in tests/integration/test_resolved_value_constraints.rs cover, per field: a fully static over-limit value fails check with the exact path and message; a partially-unresolved value whose lower bound alone exceeds the limit fails; and under-limit / fully-unresolved controls pass. Includes the target-type behaviors: {{ 120.0 }} timeout and {{ 4.0 }} defaultTaskCount valid; whole-field null/list in string fields rejected with the resolution diagnostic; null inside surrounding text still interpolating as empty; {{ null }} amounts and targetRuntimeSeconds treated as unset.
Existing tests asserting the old untargeted behaviors were updated in the same commits that change them (amounts coercion diagnostics; a LIST[PATH]-parameter-as-range-element test now asserts rejection, with a comment explaining the alternative).
Full OpenJD conformance suite passes: 1,133/1,133 — run after each behavioral layer, confirming no conformance fixture relies on untargeted rendering.
cargo clippy --all-features --all-targets --workspace -- -D warnings is clean; cargo fmt clean; cargo doc builds without warnings.

Was this change documented?

Yes — specs/model/validation.md gains a "Spec-Mandated Resolved-Value Constraints" section: the two-stage check, the per-field table (target type, bound, fully-static check), the §1.3.2 general rule, the named consequences of string targets, and the literals exclusion. specs/model/job-creation.md documents the chunks/amount targeting; specs/sessions/runners.md documents the action-field targeting.
Doc comments on ResolvedConstraint and the resolution helpers explain the invariant and cite the spec sections.
Follow-up (separate branch): specs/resolved-value-limits.md on docs/resolved-value-limits-design should mark gate 1 implemented and record the remaining open question.

Is this a breaking change?

Yes — all three commits carry BREAKING CHANGE footers. Summary of observable changes:

Templates statically violating spec-mandated resolved-value constraints now fail openjd check (previously accepted, failing later or never).
Whole-field null or list-valued expressions in required string fields (job name, range elements, attribute values, environment variable values) are now resolution errors; there is no null → string or list → string coercion. A bare LIST[*] parameter reference as a single range element previously rendered the list's display form as one element and is now rejected.
timeout/notifyPeriodInSeconds/mode/chunks fields coerce whole-field expressions per their targets: whole-number floats and numeric strings are accepted where they previously errored; uncoercible values fail with coercion diagnostics instead of hand-written parse messages.
Amount min/max resolving to null now means the bound is unset (previously a parse error); non-numeric or non-finite amount strings fail with coercion diagnostics.

These changes have not reached production consumers; landing them now, before the behavior is depended upon, is the point of doing this in one coordinated change.

Does this change impact security?

No

Follow-ups

List-item targets for range/attribute elements. Under §1.3.2's list-item rule these would target string? | list[string] with skip/flatten semantics (as args does). They currently target plain string (no skip/flatten). Needs reference-implementation cross-checking and an upstream clarification issue; the chunks int reading adopted here is worth confirming upstream at the same time.
§4.4.2 reading. The 2048-character environment variable value limit is enforced here on the resolved value; the raw-text check also remains. Cross-check openjd-model-for-python behavior and file an upstream clarification (pre-existing action item from the design document).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Template Schemas mandates target types for single whole-field expressions in three action fields: `int?` for timeout (§5) and notifyPeriodInSeconds (§5.3.2), `string?` for a deferred cancelation mode. The runtime resolved all three untargeted and hand-matched the value, deviating from the spec: `timeout: "{{ 120.0 }}"` failed with "must be a positive integer, got Float" where the mandated coercion accepts it as 120. resolve_action_timeout, resolve_notify_period_seconds, and resolve_effective_cancelation now pass the mandated targets via with_target_type. The String match arms remain for multi-segment format strings (which concatenate regardless of target) and now trim before parsing, matching the reference implementation's int() leniency. BREAKING CHANGE: whole-field expressions in `timeout` and `notifyPeriodInSeconds` now coerce under `int?` — whole-number floats and numeric strings are accepted, and uncoercible values fail at resolution with coercion diagnostics instead of hand-written messages. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Expression Language §1.3.2 ("Evaluation Within Template Schemas") gives every format-string field a target type for its whole-field expressions: `T` for a required field, `T?` for an optional one (null = field omitted). Job creation and the session runtime resolved the fields with spec-mandated resolved-value constraints untargeted (resolve_string_with, then trim-and-parse), which silently rendered a whole-field null as the empty string and a list value as its display form. Adopt the schema-derived targets at resolution time: - job `name`, task-parameter STRING/PATH range elements, and hostRequirements attribute values resolve with target `string` (create_job / resolve_string_range / resolve_string_list) - hostRequirements amount `min`/`max` resolve with `float?` — a whole-field null now means "field omitted" instead of a parse error - chunks `defaultTaskCount` resolves with `int` and the optional `targetRuntimeSeconds` with `int?`, so `{{ 4.0 }}` coerces to 4 - environment variable values resolve with `string` in the session runtime Multi-segment strings concatenate as before; numeric text parses with surrounding whitespace tolerated, matching the reference implementation. BREAKING CHANGE: a whole-field `null` or list-valued expression in a required string field (job name, range elements, attribute values, environment variable values) is now a resolution error; there is no list→string or null→string coercion. A bare LIST[*] parameter reference as a single range element previously rendered the list's display form as one element and is now rejected. Amount `min`/`max` resolving to `null` now means the bound is unset instead of erroring, and non-numeric/non-finite amount strings fail with coercion diagnostics. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Gate 1 of the resolved-value limits design: template validation (pass 8) no longer discards what static evaluation computes. Each field whose spec constraints apply to the resolved value — "after the format string has been resolved" in the spec's wording — carries a ResolvedConstraint applied to the StaticResolution that validate_expressions returns, in two stages: 1. Lower bound: min_resolved_string_len holds for every possible run-time resolution (unresolved segments contribute 0), so a bound past the field's limit fails `openjd check` without knowing the unresolved parts: `"{{ Param.X }}{{ 'A' * 600 }}"` can never be a valid job name. 2. Full value check: when the field is fully static, the same check job creation or the worker would run on the resolved value runs at `check` time with matching messages: a static notifyPeriodInSeconds of 700 fails with "must not exceed 600." Fields and limits: job name (128/512 chars + Cc check), attribute capability values (100 chars / standard allowed sets, including a longest-allowed-value bound for standard capabilities), STRING/PATH range elements (1024), environment variable values (2048 — previously enforced nowhere), timeout and notifyPeriodInSeconds (positive, ≤ 600), deferred cancelation mode (enum + 21-char bound), chunks defaultTaskCount/targetRuntimeSeconds (≥ 1 / ≥ 0; these format strings were previously never validated in pass 8 at all), and amount min/max (≥ 0 / > 0, finite). Numeric fields use a soft 100-character bound (MAX_RESOLVED_NUMERIC_LEN): leading zeros preclude an exact maximum, but an i64 needs at most 20 characters. Each field validates with the same schema-derived target type gates 2/3 resolve it with (Expression Language §1.3.2), so validation accepts exactly what resolution accepts. Literal fields are excluded — the raw-text passes already check those. 47 tests cover, per field: a fully static over-limit value fails, a partially-unresolved value whose bound alone exceeds the limit fails, and under-limit / fully-unresolved controls pass. Full conformance suite passes (1,133/1,133). BREAKING CHANGE: templates whose format strings statically violate resolved-value constraints — including whole-field null or list values in required string fields — now fail template validation instead of failing at job creation or on the worker (or, for environment variable values and chunks fields, not failing at all). Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback on the gate-1 range-element constraint found two issues: 1. Wrong limit field: the constraint used max_task_param_range_len (the maximum number of elements in a list-form range) where the per-element character limit is max_task_param_string_len. Both are 1024 today, so no behavior differed — but the moment either limit moves, gate 1 would validate elements against the count cap while job creation enforces the length cap. 2. Bytes vs characters: the spec states its string limits in characters (and the reference implementation's len() counts characters), but several checks compared byte lengths — falsely rejecting non-ASCII values (a 600-character/1,200-byte range element failed job creation). Converted to character counts: range elements and float-range text in job creation (and the error message that reported bytes), the resolved job-name check, attribute capability values (whose length diagnostic also fired before the charset error for non-ASCII), and the raw-literal checks for job/step/environment names, identifiers, filenames, and literal attribute values. Tests pin both gates: 'é' * 1024 (2,048 bytes) validates and 'é' * 1025 fails at check; a 600-character non-ASCII range element now survives job creation with its character length intact. Conformance suite passes 1,133/1,133. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: float? targeting made a new state reachable — an amount requirement whose only provided bound is a whole-field expression resolving to null ends up with both min and max unset after resolution. Decode's "must have at least one of min or max." check runs on field presence, so it passes; check_resolved_amount_bounds only re-applied the sign and min <= max rules; and the job carried a boundless amount requirement that matches every worker. Re-apply the at-least-one rule on the resolved values, alongside the sign checks that are re-applied there for the same reason. The message gains an "after resolution" suffix so an author who did provide the field understands why it fired. Tests: min-only and min+max whole-field nulls are rejected at job creation; a null min alongside a concrete max still resolves to a one-sided bound. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: the gate-1 bound for environment variable values hardcoded 2048, silently duplicating the value the raw-text pass reads from EffectiveLimits — the same divergence-on-change failure mode as the range-element limit fixed earlier: if the limit ever moves, the literal and interpolated forms of the same field would disagree. Introduce a dedicated EffectiveLimits::max_env_var_value_len (2048) rather than reusing max_description_len: the raw-text pass had been borrowing the §7.2 Description limit for §4.4.2 environment variable values, another values-coincidence that would break the moment either spec section moves independently. Both the raw-text check and the gate-1 resolved-value constraint now read the same field, threaded into validate_env_format_strings by its three callers (job environments, step environments, and environment templates). Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…:Int Review feedback: after every integer field adopted its schema-derived target type, all four Int constructions passed targeted: true, so the targeted: false arm of target_type() was unreachable and the stage-2 checker ignored the flag. Worse than dead weight: a gate-1/gate-2 target mismatch is exactly the failure mode this design guards against, so an unused untargeted knob was a footgun — a future field constructed with targeted: false would get an untargeted gate 1 against a targeted gate 2 and silently diverge. Remove the field and let target_type() derive int vs int? from nullable alone; it now returns ExprType rather than Option<ExprType>, since every constraint has a target. Also scrub the three doc comments that still described untargeted numeric handling the code no longer has (the enum-level scoping paragraph, the Int variant doc, and the stage-2 trim comment). Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: the amount non-finite test had been loosened from an exact assert_eq! to a substring check — against the repo's error-message test standard — and in the process lost the only coverage of resolve_to_f64's is_finite branch: under the float? target all five "nan"/"inf" loop values fail during coercion and none reach the parse path. Restore exact full-message assertions on both amount failure tests (the loop is now explicitly a coercion test), tighten the LIST[*]-in-range rejection test to assert the full path and message, and add the multi-segment case that actually reaches the finite check: "1e{{Param.Exp}}" with Exp=999 concatenates to "1e999", which parses to infinity. Also restore the finite-check diagnostic to report the resolved text ("'1e999' is not a finite number") instead of the parsed f64 ("'inf'"), which this PR had regressed — the text is what the author can act on. The check moves into the parse arm, the only path that can produce a non-finite value (a coerced Float64 excludes NaN and the infinities by construction). Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: code comments, the test-suite header, and specs/model/validation.md referenced specs/resolved-value-limits.md — a design document that lives on an unmerged branch, not in the repo — and leaned on its "gate 1/2/3" vocabulary, which nothing in the repo defines. Use the terminology the spec already establishes instead: the three processing stages of Template Schemas §7.4 (template validation, job creation, task execution on the worker host). The invariant reads as "a field's validation-time target must always equal its resolution-time target", and the constraint machinery is described as making template validation the earliest stage to catch a violation that is already knowable. References to the missing document are repointed at the Spec-Mandated Resolved-Value Constraints section of specs/model/validation.md, which is self-contained; the open question about list-item string? | list[string] semantics now lives there too. Also describe the current state rather than the change that produced it: "Pass 8 uses the values that static evaluation can resolve" rather than "no longer discards", and the string-target consequences state what is an error and why rather than comparing against prior behavior. The change narrative lives in the commit history and PR description. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback established that Expression Language §1.3.2's list-item rule — "For list items (e.g., in `args`), the target type is `T? | list[T]`" — applies to list items generally, with `args` as its worked example, not its scope. Task-parameter STRING/PATH range elements and hostRequirements attribute `anyOf`/`allOf` values are the same list-of-format-strings shape, so their whole-field expressions target `string? | list[string]`: a `null` result skips the element and a list result flattens inline, one element per list member, mixable with literal elements: range: ["first", "{{ RawParam.Paths }}", "last"] This replaces the plain-`string` targeting these fields briefly carried on this branch (which rejected list values), and is complementary to — not redundant with — the whole-field `<ListExpressionString>` range form (Expression Language §1.3.12), which replaces an entire range with one expression and cannot mix literals. At template validation, the per-element constraints (§3.4.2's 1024 characters, §3.3.2.2's charset/allowed-set rules) apply to each element of a fully static list, and the string-length bound only applies when `StaticResolution::resolved_type` says the resolution is certainly a string — a list-valued resolution distributes its characters across elements, so the display-form length says nothing about any single element. Because null-skips can empty a list whose non-emptiness was checked on field presence at decode, job creation re-checks after resolution ("has no elements after resolution") — the same shape as the amounts both-bounds-null fix. Spec references verified against the upstream wiki: Expression Language §1.3.2 "Evaluation Within Template Schemas" and §1.3.12 "Task Parameter Range Field Extensions"; Template Schemas §3.4.2 TaskParameterStringValue, §3.3.2.2 AttributeCapabilityValue, §3.4.1.5 ChunkIntTaskParameterDefinition, §3.3.1 AmountRequirement. validation.md's per-field table now cites the owning section for every field. Tests: flatten and skip at both validation and job creation, mixed literal + expansion ranges, per-element limit and charset violations, all-null lists rejected after resolution, and the LIST[PATH]-in-range test flips to asserting the flatten. Full conformance suite passes (1,133/1,133). BREAKING CHANGE: in task-parameter range elements and attribute values, a whole-field expression resolving to `null` now skips the element and a list result flattens inline (previously an error on this branch, and the list's display form as a single element before that). A range or attribute list emptied entirely by null-skips fails job creation. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: validation.md justified rejecting whole-field nulls by asserting a minimum length of 1 on the string fields, but nothing enforced that minimum on resolved values — a whole-field expression resolving to the empty string passed every stage. The claim was also wrong for one field: §4.4.2 sets the environment variable value minimum at 0 characters, so empty env values are legal and stay unchecked. Enforce the minima the spec actually sets, at both stages: - job name (§1.1.1 minimum 1): rejected statically when fully static ("must not resolve to an empty string.") and re-checked on the resolved value at job creation. The raw-text pass only sees literal names. - STRING/PATH range elements (§3.4.2 minimum 1): the per-element check covers both a static empty string and an empty element of a flattened list at validation; job creation's existing empty check, which applied only to PATH elements, now covers STRING elements too (the spec's constraint is on <TaskParameterStringValue>, which both share). - attribute values already reject empties via validate_attribute_capability_value. Also fixes the job-name maximum-length message to report characters (it compared characters but still printed the byte count), and tightens the empty-PATH-element test to an exact-message assertion. validation.md now states which fields have a resolved minimum and that env values deliberately have none. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…rity Review feedback on the resolved-minimum-length change: it enforced §3.4.2's minimum length 1 on STRING range elements at job creation but not at template validation (the STRING arm of the raw-text pass never checked literal empties, and the literal early-return in validate_fs_with skips them too), so 'type: STRING' with 'range: [""]' passed 'check' but failed 'create_job' — a divergence between the two gates, and from the reference implementation, which accepts empty STRING elements end to end (TaskParameterStringValueAsJob is explicitly min_length=0; only PATH definitions reject empties, since an empty string is not a valid path on any OS). Match the reference implementation: restore the is_path gate on the create_job empty check, and make the TextListItem static-resolution empty check PATH-only via a forbid_empty flag. PATH elements are still rejected at every stage where they become knowable (literal at raw-text validation, static expression at check, interpolated at create_job); STRING elements are never rejected for emptiness. validation.md now documents the PATH-only rule and its provenance. A spec amendment aligning §3.4.2 with implemented behavior is under consideration. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

Review feedback: for a fully static string range element over the §3.4.2 length limit, TextListItem's bound check and its per-element check both fired at the same path ('resolves to at least 1100...' and 'resolves to 1100...'), so 'openjd check' counted one violation as two errors. The AttributeValue arm already returns after its bound error; do the same here. The per-element loop still runs for conforming-length static values, preserving the PATH empty check for non-list statics. The regression test asserts the entire error output via a new check_err_exact helper, pinning the error count — the contains-based assertions could not catch the duplication. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…ved-value design Two gaps surfaced in review of the Group A PR, neither covered by the Group B scope: - Gate 2: create_job re-applies only the length and emptiness checks to the resolved job name, so an interpolated name can carry control characters past both gates despite the SS1.1.1 no-Cc-chars rule. - Gate 1: the single-valued allOf check requires all-literal elements, but literal elements can never null-skip or flatten, so >= 2 literals is a certain violation regardless of expression elements.

Conflict in crates/openjd-model/tests/integration/test_create_job.rs only: both sides appended tests at end of file, and git factored the trailing `);` / `}` out as common context, so each side's block read as truncated. Kept both blocks whole, #383's first and this branch's round-trip block after it. Verified byte-identical to each side: lines 1-6270 match upstream/main exactly, and the final 343 lines match 1bb2aed exactly. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

… resolved-value checks Two early-detection gaps flagged in the PR OpenJobDescription#383 review: - Single-valued allOf literal count (gate 1): the single-valued standard-attribute check fired only for all-literal lists. A literal element always contributes exactly one resolved element, so more than one literal violates the rule under every possible resolution — now gated on the literal count instead of all-literal, rejecting e.g. allOf: [linux, windows, '{{ Param.X }}'] at decode instead of at job creation. - Job-name control characters (gate 2): §1.1.1 constrains the resolved name, but create_job only re-applied the length and emptiness checks. An interpolated name resolving to a value with Cc characters passed both gates; create_job now rejects it alongside the emptiness check. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…ved-value design Two gaps surfaced in review of the Group A PR, neither covered by the Group B scope: - Gate 2: create_job re-applies only the length and emptiness checks to the resolved job name, so an interpolated name can carry control characters past both gates despite the SS1.1.1 no-Cc-chars rule. - Gate 1: the single-valued allOf check requires all-literal elements, but literal elements can never null-skip or flatten, so >= 2 literals is a certain violation regardless of expression elements.

… the design Both follow-ups are implemented by the fix/resolved-value-followups branch (independent of this one): the single-valued allOf literal-count gate in structure.rs and the gate-2 job-name control-character check in create_job.

… resolved-value checks Two early-detection gaps flagged in the PR OpenJobDescription#383 review: - Single-valued allOf literal count (gate 1): the single-valued standard-attribute check fired only for all-literal lists. A literal element always contributes exactly one resolved element, so more than one literal violates the rule under every possible resolution — now gated on the literal count instead of all-literal, rejecting e.g. allOf: [linux, windows, '{{ Param.X }}'] at decode instead of at job creation. - Job-name control characters (gate 2): §1.1.1 constrains the resolved name, but create_job only re-applied the length and emptiness checks. An interpolated name resolving to a value with Cc characters passed both gates; create_job now rejects it alongside the emptiness check. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…ecks (#397) * fix(model): implement PR #383 review follow-ups for resolved-value checks Two early-detection gaps flagged in the PR #383 review: - Single-valued allOf literal count (gate 1): the single-valued standard-attribute check fired only for all-literal lists. A literal element always contributes exactly one resolved element, so more than one literal violates the rule under every possible resolution — now gated on the literal count instead of all-literal, rejecting e.g. allOf: [linux, windows, '{{ Param.X }}'] at decode instead of at job creation. - Job-name control characters (gate 2): §1.1.1 constrains the resolved name, but create_job only re-applied the length and emptiness checks. An interpolated name resolving to a value with Cc characters passed both gates; create_job now rejects it alongside the emptiness check. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> * feat(expr): add FormatString segment_count and literal_segments accessors Two small accessors for reasoning statically about resolution shape: - segment_count(): a format string with more than one segment always concatenates to a single string (Expression Language §1.3.2) — only a whole-field single-expression format string can resolve to null or to a list. is_literal() distinguishes the two single-segment cases. - literal_segments(): the literal text runs, which appear verbatim in every possible resolution; expression source text is excluded since it never appears in a resolved value. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> * fix(model): sharpen the decode-time job-name and single-valued checks Review follow-ups: - The single-valued allOf count now includes multi-segment format-string elements alongside literals: a multi-segment element always concatenates to exactly one string, so it contributes exactly one resolved element — only a whole-field single-expression element can null-skip or flatten. allOf: [linux, 'windows{{ Param.X }}'] on attr.worker.os.family is now rejected at decode. - The decode-time job-name control-character check now runs on the name's literal segments instead of the whole raw text. Literal text appears verbatim in every resolution, so a control character there is still a certain violation even in an interpolated name — but expression source text never appears in a resolved value, so a control character inside an expression (e.g. a tab in a string literal that resolution removes) no longer over-rejects. What an expression resolves to is checked on the resolved value, statically when fully static and at job creation otherwise. Comments and specs now describe all three checks accurately. Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com> --------- Signed-off-by: Mark <399551+mwiebe@users.noreply.github.com>

…ved-value design Two gaps surfaced in review of the Group A PR, neither covered by the Group B scope: - Gate 2: create_job re-applies only the length and emptiness checks to the resolved job name, so an interpolated name can carry control characters past both gates despite the SS1.1.1 no-Cc-chars rule. - Gate 1: the single-valued allOf check requires all-literal elements, but literal elements can never null-skip or flatten, so >= 2 literals is a certain violation regardless of expression elements.

… the design Both follow-ups are implemented by the fix/resolved-value-followups branch (independent of this one): the single-valued allOf literal-count gate in structure.rs and the gate-2 job-name control-character check in create_job.

…ved-value design Two gaps surfaced in review of the Group A PR, neither covered by the Group B scope: - Gate 2: create_job re-applies only the length and emptiness checks to the resolved job name, so an interpolated name can carry control characters past both gates despite the SS1.1.1 no-Cc-chars rule. - Gate 1: the single-valued allOf check requires all-literal elements, but literal elements can never null-skip or flatten, so >= 2 literals is a certain violation regardless of expression elements.

… the design Both follow-ups are implemented by the fix/resolved-value-followups branch (independent of this one): the single-valued allOf literal-count gate in structure.rs and the gate-2 job-name control-character check in create_job.

…ved-value design Two gaps surfaced in review of the Group A PR, neither covered by the Group B scope: - Gate 2: create_job re-applies only the length and emptiness checks to the resolved job name, so an interpolated name can carry control characters past both gates despite the SS1.1.1 no-Cc-chars rule. - Gate 1: the single-valued allOf check requires all-literal elements, but literal elements can never null-skip or flatten, so >= 2 literals is a certain violation regardless of expression elements.

… the design Both follow-ups are implemented by the fix/resolved-value-followups branch (independent of this one): the single-valued allOf literal-count gate in structure.rs and the gate-2 job-name control-character check in create_job.

mwiebe requested a review from a team as a code owner September 11, 2026 01:13

github-actions Bot added the waiting-on-maintainers label Sep 11, 2026