Skip to content

chore(deps): Bump openjd-model to 0.7.1 and openjd-sessions to 0.5.8 #363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wyongzhi wants to merge 1 commit into
base: mainline
Choose a base branch
from
+74 −8
Open

chore(deps): Bump openjd-model to 0.7.1 and openjd-sessions to 0.5.8 #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions THIRD-PARTY-LICENSES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2535,8 +2535,8 @@ limitations under the License.
** libc; version 0.2.189 -- https://crates.io/crates/libc
** manyhow-macros; version 0.11.4 -- https://crates.io/crates/manyhow-macros
** openjd-expr; version 0.7.0 -- https://crates.io/crates/openjd-expr
** openjd-model; version 0.7.0 -- https://crates.io/crates/openjd-model
** openjd-sessions; version 0.5.7 -- https://crates.io/crates/openjd-sessions
** openjd-model; version 0.7.1 -- https://crates.io/crates/openjd-model
** openjd-sessions; version 0.5.8 -- https://crates.io/crates/openjd-sessions
** pin-project-lite; version 0.2.17 -- https://crates.io/crates/pin-project-lite
** portable-atomic; version 1.15.0 -- https://crates.io/crates/portable-atomic
** proc-macro2; version 1.0.107 -- https://crates.io/crates/proc-macro2
Expand Down
4 changes: 2 additions & 2 deletions rust-bindings/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ crate-type = ["cdylib", "rlib"]

[dependencies]
openjd-expr = "0.7.0"
openjd-model = "0.7.0"
openjd-sessions = "0.5.7"
openjd-model = "0.7.1"

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process note on release visibility: this bump is a user-visible validation relaxation — a template where two Steps each declare a stepEnvironment named StepEnv was rejected on the v1 path and now decodes — but it is very likely to ship with no changelog entry describing it.

[tool.semantic_release.commit_parser_options] lists chore in patch_tags, so a chore(deps): subject does bump the version. But .semantic_release/CHANGELOG.md.j2 only renders the breaking, features, bug fixes, and performance improvements element groups — there is no chore section, so a chore-typed commit produces a release with nothing written about it. That is the observed history: #349 (chore(deps): Bump openjd-* Rust crates to the 0.6.0 release) appears in neither the 0.11.8 nor the 0.11.9 CHANGELOG section. The 0.11.11 entry for #359 exists only because its text was hand-written (it carries no commit-hash link, unlike every template-generated line).

Since this bump changes what templates are accepted rather than just refreshing a pin, consider a fix:-typed subject so the behavior change lands in the released notes on its own. The relaxation is exactly the kind of thing a downstream consumer pinning this library would want to read in the changelog.

openjd-sessions = "0.5.8"
tokio = { version = "1", features = ["rt-multi-thread"] }
uuid = { version = "1", features = ["v4"] }
serde_json = "1"
Expand Down
66 changes: 66 additions & 0 deletions test/openjd/model_v1/test_parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,3 +405,69 @@ def test_json_explicit(self) -> None:
def test_invalid_yaml_raises(self) -> None:
with pytest.raises(DecodeValidationError):
decode_environment_template_str(": not a mapping")


class TestStepEnvironmentNameScope(object):
"""A Step Environment's ``name`` is scoped to the Step that defines it (Template
Schemas §3 StepTemplate, §4 Environment): unique within that Step's list, and
distinct from every Job Environment. Different Steps may reuse a name.

openjd-model 0.7.1 (openjd-rs#381) relaxed an over-strict check that held every
environment name in the template in one set, so the second Step to declare
``StepEnv`` was rejected. The v0 path always accepted this; this is the v1 path,
which had no coverage.
"""

@staticmethod
def _environment(name: str) -> dict[str, Any]:
return {
"name": name,
"script": {"actions": {"onEnter": {"command": "echo", "args": [name]}}},
}

@classmethod
def _step(cls, name: str, environment_names: list[str]) -> dict[str, Any]:
return {
"name": name,
"stepEnvironments": [cls._environment(n) for n in environment_names],
"script": {"actions": {"onRun": {"command": "echo", "args": [name]}}},
}

@classmethod
def _template(cls, steps: list[dict[str, Any]]) -> dict[str, Any]:
return {
"specificationVersion": "jobtemplate-2023-09",
"name": "T",
"jobEnvironments": [cls._environment("JobEnv")],
"steps": steps,
}

def test_same_name_across_steps_is_accepted(self) -> None:
"""Four Steps each declare ``StepEnv``. Only one Step's environments are ever
active in a Session, so these names never collide."""
template = self._template([self._step(f"Step{i}", ["StepEnv"]) for i in range(4)])
job_template = decode_job_template(template=template, supported_extensions=[])
names = [[e.name for e in (s.step_environments or [])] for s in job_template.steps]
assert names == [["StepEnv"]] * 4

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upstream #381 carried a second behavior change that this PR neither mentions nor covers: max_env_count enforcement.

From the upstream PR body: "max_env_count now counts environments (env_count) instead of HashSet::len(). The two were equal only because duplicates were always rejected; with names allowed to repeat across steps, counting distinct names would under-count."

That limit is reachable from this repo — it is a public CallerLimits field (rust-bindings/src/model/profile.rs:506, _openjd_rs.pyi:314) passed to decode_job_template(..., caller_limits=...). And the new relaxation is precisely what makes the two counting strategies diverge: the template shape added in test_same_name_across_steps_is_accepted above (4 steps x StepEnv) is the first shape where distinct-name count (1) and total environment count (5, with JobEnv) differ at all.

Searching test/ for max_env_count turns up only test_pickle.py:141,247 — round-tripping the field through __reduce__. Nothing asserts the limit is actually enforced, so the counting change is invisible to this suite in both directions: an upstream regression back to HashSet::len() would go unnoticed, and so would one that double-counted.

A test in this class would pin it, reusing the helpers already here (4 steps x StepEnv plus JobEnv is 5 environments but only 2 distinct names, so a limit of 4 must reject):

def test_max_env_count_counts_repeated_names_separately(self) -> None:
    template = self._template([self._step(f"Step{i}", ["StepEnv"]) for i in range(4)])
    with pytest.raises(ModelValidationError):
        decode_job_template(
            template=template,
            supported_extensions=[],
            caller_limits=CallerLimits(max_env_count=4),
        )

(CallerLimits is importable from openjd.model._v1; it is not currently imported in this module.)


def test_duplicate_within_one_step_is_rejected(self) -> None:
"""Control for §3 rule 1: the per-Step uniqueness check must survive the relaxation."""
template = self._template(
[self._step("Step0", ["StepEnv"]), self._step("Step1", ["StepEnv", "StepEnv"])]
)
with pytest.raises(ModelValidationError) as excinfo:
decode_job_template(template=template, supported_extensions=[])
message = str(excinfo.value)
assert "steps[1] -> stepEnvironments[1]" in message
assert "duplicate environment name: 'StepEnv'" in message

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two control tests here cover the §3 per-Step rule and the Step-vs-Job rule, but not the rule most at risk from this particular relaxation: uniqueness within jobEnvironments.

Upstream #381 relaxed a check that "held every environment name in the template in one set". Collapsing that single set is exactly the kind of change that can drop the job-level duplicate check along with the cross-step one, and there is currently no v1 coverage of it — grepping jobEnvironments under test/openjd/model_v1/ finds only this new _template() helper, test_fuzz.py:199 (an empty {} entry), and test_template_types.py (constructor round-trip, no validation). The v0 path does pin it (test/openjd/model_v0/v2023_09/test_job_template.py:388, id="duplicate environment names", expecting exactly 1 error), so v1 is the side without a guard.

A third control would close that:

def test_duplicate_job_env_names_is_rejected(self) -> None:
    template = self._template([self._step("Step0", ["StepEnv"])])
    template["jobEnvironments"] = [self._environment("JobEnv"), self._environment("JobEnv")]
    with pytest.raises(ModelValidationError):
        decode_job_template(template=template, supported_extensions=[])


def test_step_env_named_like_job_env_is_rejected(self) -> None:
"""Control for §3 rule 2: a Step Environment may not reuse a Job Environment name."""
template = self._template(
[self._step("Step0", ["StepEnv"]), self._step("Step1", ["JobEnv"])]
)
with pytest.raises(ModelValidationError) as excinfo:
decode_job_template(template=template, supported_extensions=[])
message = str(excinfo.value)
assert "steps[1] -> stepEnvironments[0]" in message
assert "duplicate environment name: 'JobEnv'" in message

@github-actions github-actions Bot Sep 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions newly pin v1 error text for the step-vs-job rule that does not match the v0 reference — worth a deliberate decision rather than being locked in as a side effect of a dependency bump.

AGENTS.md:176-180 ("Test Quality Standard") states the goal is that failure messages "match the pure-Python reference where one exists", and that "the Rust bindings reproduce the same format" as pydantic's path-prefixed output. For this specific rule the two diverge on both halves of what is asserted here:

  • Message. v0 raises Name JobEnv must differ from the names of Environments defined at the root of the template. (src/openjd/model/v2023_09/_model.py:4488). v1 says duplicate environment name: 'JobEnv'. The v0 wording names the rule; the v1 wording reads as if the name were duplicated inside one list, which is the other rule — the one test_duplicate_within_one_step_is_rejected covers two tests up. Both tests here assert the same duplicate environment name: string for what are two distinct spec rules, so neither assertion can distinguish them.
  • Path. v0's loc is ("step", i, "stepEnvironments", j, "name") (_model.py:4485), rendering through _convert_pydantic_error._loc_to_str as step[1] -> stepEnvironments[0] -> name. The asserted v1 path is steps[1] -> stepEnvironments[0] — plural steps, and no trailing -> name segment. (v0's singular step looks like the v0-side bug of the pair, since the field is steps; either way they are not equal.)

If the divergence is intended, a note in the class docstring would keep the next reader from reading these as parity assertions. If not, it is a real gap to file — but pinning the current strings in a test makes it harder to notice later.

Loading