Skip to content

fix(deps): Accept a stepEnvironment name reused across steps on the v1 path - #363

Merged
wyongzhi merged 2 commits into
OpenJobDescription:mainlinefrom
wyongzhi:deps/openjd-rs-0.7.1
Sep 14, 2026
Merged

fix(deps): Accept a stepEnvironment name reused across steps on the v1 path#363
wyongzhi merged 2 commits into
OpenJobDescription:mainlinefrom
wyongzhi:deps/openjd-rs-0.7.1

Conversation

@wyongzhi

@wyongzhi wyongzhi commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What changed

Bumps two openjd-* Rust crate pins in rust-bindings/Cargo.toml, refreshes Cargo.lock, and regenerates THIRD-PARTY-LICENSES.txt (scripts/check_third_party_licenses.sh --update):

Crate From To
openjd-model 0.7.0 0.7.1
openjd-sessions 0.5.7 0.5.8

openjd-expr stays at 0.7.0. cargo update -p openjd-model -p openjd-sessions moved only these two packages; no new transitive dependency.

Upstream changes

openjd-rs#381 (openjd-model 0.7.1) — stepEnvironment name uniqueness is scoped to its step. The 2023-09 spec requires environment names to be unique within jobEnvironments, within each step's stepEnvironments, and requires a stepEnvironment name to differ from every jobEnvironment name. It does not forbid two different steps from reusing a name, and the v0 (Python) path has always accepted that. The v1 (Rust) path held every environment name in one set and rejected the second step to declare the same name. The same fix changed max_env_count to count environments rather than distinct names; the two were equal only while duplicates were always rejected. The pin brings both; no binding code change was needed (patch release, no API change).

openjd-sessions 0.5.7 → 0.5.8 carries no source change to the crate (release-train version bump only).

Tests

TestStepEnvironmentNameScope in test/openjd/model_v1/test_parse.py pins the behaviour on the v1 path:

  • positive: four steps each declaring StepEnv decode successfully
  • three controls asserting the spec rules survive the relaxation: duplicate within one step; step environment named like a job environment; duplicate within jobEnvironments (the rule most exposed by splitting the single set, previously untested on v1)
  • max_env_count: 4 x StepEnv + JobEnv is 5 environments under 2 names; a limit of 4 rejects with total environments (5) exceeds caller limit of 4

The class docstring records that the asserted v1 error text for the step-vs-job rule differs from v0 (Name X must differ from ... at step[i] -> stepEnvironments[j] -> name); aligning them is an openjd-rs concern, not introduced here.

Mutation-checked by rebuilding the extension against the previous pins (openjd-model 0.7.0): 2 failed, 3 passed — the positive case fails with duplicate environment name: 'StepEnv', the max_env_count case fails because 0.7.0 never reaches the count, and all three controls pass. Rebuilt on 0.7.1: 5 passed.

Full run: hatch run test 5969 passed, 24 skipped, 3 xfailed, coverage 94.25%; hatch run lint and hatch run typing clean.

Changelog

Subject is fix(deps): rather than chore(deps): on purpose. chore is in patch_tags but .semantic_release/CHANGELOG.md.j2 renders no chore group, so a chore-typed bump ships with no release note (as #349 did). This bump changes which templates decode, so it should appear in the notes.

Breaking change / security

No public interface changes. No security impact.


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

Comment thread test/openjd/model_v1/test_parse.py
Comment thread rust-bindings/Cargo.toml
Comment thread test/openjd/model_v1/test_parse.py
Comment thread test/openjd/model_v1/test_parse.py
@wyongzhi wyongzhi changed the title chore(deps): Bump openjd-model to 0.7.1 and openjd-sessions to 0.5.8 fix(deps): Accept a stepEnvironment name reused across steps on the v1 path Sep 14, 2026
@wyongzhi
wyongzhi force-pushed the deps/openjd-rs-0.7.1 branch from ed27a76 to cb5342a Compare September 14, 2026 18:07
leongdl
leongdl previously approved these changes Sep 14, 2026
Comment thread test/openjd/model_v1/test_parse.py
…1 path

Bump openjd-model to 0.7.1 and openjd-sessions to 0.5.8. openjd-rs#381
scopes stepEnvironment name uniqueness to its step, matching the Python
implementation and Template Schemas section 3: a template in which two
steps each declare a stepEnvironment named the same was rejected by the
v1 decoder and now decodes. The per-step rule, the step-vs-job rule,
and jobEnvironments uniqueness are unchanged. max_env_count now counts
environments rather than distinct names, which only differ once names
may repeat.

openjd-sessions 0.5.8 is a release-train bump with no source change.

Signed-off-by: wyongzhi <276409147+wyongzhi@users.noreply.github.com>
"""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=[])

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 relaxation is only exercised at decode_job_template. Nothing covers create_job on the same template, which is where a name collision would actually bite.

decode_job_template validates the JobTemplate/StepTemplate shapes. But the environment names that matter operationally are the ones on the instantiated JobStep.step_environments (src/openjd/_openjd_rs.pyi:2226) and Job.job_environments (:1166), reached via create_job, and then handed to Session.enter_environment (rust-bindings/src/sessions/session.rs:443) which tracks them by name in environments_entered (:117, :407).

That instantiation path is a second place a name set could be held. openjd-rs#381 split one template-wide set into a job set plus a per-Step set on the decode side; if create_job (or preprocess_job_parameters, which py_create_job routes through at rust-bindings/src/model/create_job_fns.rs:106) holds its own collection keyed by environment name, decode_job_template would now accept the four-StepEnv template while create_job on that same template still fails — or worse, silently collapses four distinct Environment objects into one. test_same_name_across_steps_is_accepted returns before ever finding out; it reads job_template.steps, not a created Job.

test/openjd/model_v1/test_create_job.py has no stepEnvironments at all (grep is empty across all 1583 lines), so there is no existing case that would catch this incidentally. Extending test_same_name_across_steps_is_accepted to feed its decoded template through create_job and assert the four Steps still each carry their own StepEnv would close the gap cheaply — it reuses the template already built by _template, and it is the assertion that actually demonstrates the reused name is usable rather than merely parseable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Checked the instantiation path in openjd-rs 0.7.1 for the hypothesized name-keyed collection; there is none. create_job handles step environments positionally: instantiate.rs maps each step's stepEnvironments Vec element-by-element through convert_environment (no name lookup), and the only pass over all environments in create_job/mod.rs is the max_environment_size check, where env.name appears solely in the error message. The only HashSet in that module collects accessed symbol names, not environment names. On the session side, environments_entered is name-keyed but a session only ever enters a single step's environments, so cross-step reuse cannot coexist there — which is the spec rationale for allowing it. Leaving this PR scoped to the decode-side relaxation it ships; a create_job-level pin would be guarding a structure that does not exist today.

@wyongzhi
wyongzhi enabled auto-merge (squash) September 14, 2026 20:24
@wyongzhi
wyongzhi merged commit d8174e3 into OpenJobDescription:mainline Sep 14, 2026
27 of 31 checks passed
@wyongzhi
wyongzhi deleted the deps/openjd-rs-0.7.1 branch September 14, 2026 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants