Skip to content

fix: map WRAP_ACTIONS in the ModelExtension binding - #322

Open
wyongzhi wants to merge 1 commit into
OpenJobDescription:mainlinefrom
wyongzhi:fix-wrap-actions-binding
Open

fix: map WRAP_ACTIONS in the ModelExtension binding#322
wyongzhi wants to merge 1 commit into
OpenJobDescription:mainlinefrom
wyongzhi:fix-wrap-actions-binding

Conversation

@wyongzhi

Copy link
Copy Markdown

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

PyModelExtension was missing the WRAP_ACTIONS member, so the mandatory catch-all arm in From<ModelExtension> for PyModelExtension silently reported it as EXPR. A ModelProfile built with WRAP_ACTIONS therefore enabled EXPR instead, and templates using onWrapEnvEnter/onWrapTaskRun/onWrapEnvExit were rejected with an error that did not name the real cause.

Minimal repro on 0.11.1:

from openjd.model._v1.types import ModelExtension
assert ModelExtension.from_str("WRAP_ACTIONS") is ModelExtension.from_str("EXPR")  # holds, should not

Why it was missed: WrapActions landed in the Rust crate after this binding's enum was written. #285, #306, and #318 all touched WRAP_ACTIONS support in the template layer (template_types.rs, _model.py), but none touched profile.rs, where the extension-enablement enum lives. Because upstream ModelExtension is #[non_exhaustive], the catch-all arm is compiler-mandated, so the missing arm produced no warning.

What was the solution? (How)

  • Add WRAP_ACTIONS = 4 to PyModelExtension and both From conversion arms
  • Replace the catch-all's comment with a warning that it misrepresents unmapped variants as EXPR, and document the three edits a new upstream variant needs
  • Update the .pyi stub
  • Add TestModelExtension (drift guard): asserts the binding's member set equals openjd.model.v2023_09.ExtensionName, and that every spec name round-trips to its own member via from_str. Either test fails on the next unmapped variant. This is the guard the compiler cannot provide under #[non_exhaustive].
  • Add WRAP_ACTIONS to the pickle round-trip parametrize list

What is the impact of this change?

ModelProfile built with WRAP_ACTIONS (directly, via from_strings, or from a template's extensions: list) now actually enables WRAP_ACTIONS. This unblocks downstream consumers that declare supported_extensions from ExtensionName, for which WRAP_ACTIONS was silently never enabled.

How was this change tested?

  • Have you run the unit tests? Yes — full suite: 5453 passed, 24 skipped, 3 pre-existing xfailed; the 94% coverage gate passed
  • test_name_round_trips_to_its_own_member[WRAP_ACTIONS] fails without the fix and passes with it
  • ruff, black, mypy clean

Was this change documented?

  • Are relevant docstrings in the code base updated? Yes — the catch-all arm's comment now documents the misrepresentation hazard and the three edits required when upstream gains a variant, and points at the drift-guard test. The .pyi stub lists the new member.

Is this a breaking change?

No. It adds an enum member and corrects a mapping; no existing public contract changes. Code that (incorrectly) relied on from_str("WRAP_ACTIONS") returning EXPR would change behavior, but that behavior was the defect.

Does this change impact security?

No. No new files, directories, or privilege boundaries; no threat modeling needed.


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

PyModelExtension was missing WRAP_ACTIONS, so the catch-all arm in
From<ModelExtension> for PyModelExtension silently reported it as EXPR.
A profile built with WRAP_ACTIONS enabled EXPR instead, and templates
using onWrapEnvEnter/onWrapTaskRun/onWrapEnvExit were rejected with an
error that did not name the cause.

The catch-all cannot be removed because openjd_model::ModelExtension is
non_exhaustive, so TestModelExtension guards the mapping instead: it
fails when the binding enum drifts from ExtensionName, or when any spec
name parses back to a different member. The pickle round-trip list also
gains the new member.

Signed-off-by: Yongzhi Wei <276409147+wyongzhi@users.noreply.github.com>
@wyongzhi
wyongzhi requested a review from a team as a code owner July 31, 2026 00:48
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.

1 participant