Skip to content

fix: ensure process exit code is a 32-bit signed integer#148

Merged
ddneilson merged 1 commit into
OpenJobDescription:mainlinefrom
ddneilson:ddneilson/iss_147
Jun 18, 2024
Merged

fix: ensure process exit code is a 32-bit signed integer#148
ddneilson merged 1 commit into
OpenJobDescription:mainlinefrom
ddneilson:ddneilson/iss_147

Conversation

@ddneilson

@ddneilson ddneilson commented Jun 18, 2024

Copy link
Copy Markdown
Contributor

Fixes: #147

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

We've observed on Windows systems that a process can return an exit code that fits in a 32-bit integer, but it is interpreted as an unsigned integer rather than a signed integer. e.g. 0xFFFD0000 as 4294770688 rather than -199608.

What was the solution? (How)

Clamp the return code to 32-bit and ensure that the bits are interpreted as a signed integer.

What is the impact of this change?

Process exit codes should now be in the expected range. Note, as a point of comparison, that the .NET APIs do assume that exit codes are 32-bit integers.

How was this change tested?

I've added unit tests that ensure that the conversion is as expected.

Was this change documented?

N/A

Is this a breaking change?

No


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

Problem:
We've observed on Windows systems that a process can return an exit code
that fits in a 32-bit integer, but it is interpreted as an unsigned
integer rather than a signed integer. e.g. 0xFFFD0000 as 4294770688
rather than -199608.

Solution:
Clamp the return code to 32-bit and ensure that the bits are interpreted
as a signed integer.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
@ddneilson
ddneilson requested a review from a team as a code owner June 18, 2024 14:56
@ddneilson
ddneilson merged commit d831c17 into OpenJobDescription:mainline Jun 18, 2024
@ddneilson
ddneilson deleted the ddneilson/iss_147 branch June 18, 2024 15:16
ddneilson added a commit that referenced this pull request Jun 18, 2024
ddneilson added a commit that referenced this pull request Jun 20, 2024
This reverts commit d831c17.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
ddneilson added a commit that referenced this pull request Jun 20, 2024
revert: ensure process exit code is a 32-bit signed integer (#148)

This reverts commit d831c17.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 16, 2026
Change WrappedAction.Cancelation.Mode from an empty-string sentinel to
None (string?) when the wrapped action defines no <Cancelation>,
matching the EXPR semantics for optional data and the int? typing of
Cancelation.NotifyPeriodInSeconds. None renders as the empty string in
format-string interpolation, and the nullness is observable via EXPR
null-coalescing.

Tests assert `is None` for the undeclared case and add an execution
test that the null mode interpolates as MODE=<>, mirroring the updated
wrap-cancelation-mode-null-when-no-cancelation conformance fixture in
openjd-specifications and the Rust change in openjd-rs.

Addresses review feedback on openjd-rs PR OpenJobDescription#261 (discussion
r3597572812) and openjd-specifications PR OpenJobDescription#148 (r3597560515).

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 16, 2026
Change WrappedAction.Cancelation.Mode from an empty-string sentinel to
None (string?) when the wrapped action defines no <Cancelation>,
matching the EXPR semantics for optional data and the int? typing of
Cancelation.NotifyPeriodInSeconds. None renders as the empty string in
format-string interpolation, and the nullness is observable via EXPR
null-coalescing.

Also address review findings on the wrap-action implementation:

- Fail gracefully when WrappedAction.* injection cannot resolve the
  wrapped action's format strings (e.g. a wrapped onRun referencing
  {{Task.File.*}} or onEnter referencing {{Env.File.*}} — embedded
  files are not materialized on the wrap path yet, a known limitation
  shared with the Rust runtime). The action now FAILs through the
  normal callback path via a new _fail_action_before_start helper and
  the session transitions to READY_ENDING, instead of a raw
  FormatStringError escaping the public API — which, for
  enter_environment, previously left the session stuck in RUNNING
  with no runner. Also covers non-integer FEATURE_BUNDLE_1
  timeout/notifyPeriod resolutions.
- WrappedAction.Environment now carries only openjd_env-defined
  variables per RFC 0008, excluding the environment's declarative
  variables: map seed (parity with the Rust runtime, which was
  already correct).
- Log a warning when a wrap environment is active but run_task() was
  not given step_name, since {{WrappedStep.Name}} renders empty.
- Hoist the Template Schemas 5.3.2 notify-period defaults (120/30)
  into shared constants used by the seeding and both cancel paths.
- Guard _run_wrap_hook against unknown hook names so a typo cannot
  become a silent SUCCESS no-op.

Tests assert `is None` for the undeclared cancelation case, the
injection-failure and READY_ENDING behavior, the openjd_env-only
Environment contents, and the hook-name guard. All 53 WRAP_ACTIONS
conformance tests pass against the Python CLI.

Addresses review feedback on openjd-rs PR OpenJobDescription#261 (discussion
r3597572812) and openjd-specifications PR OpenJobDescription#148 (r3597560515).

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
Change WrappedAction.Cancelation.Mode from an empty-string sentinel to
None (string?) when the wrapped action defines no <Cancelation>,
matching the EXPR semantics for optional data and the int? typing of
Cancelation.NotifyPeriodInSeconds. None renders as the empty string in
format-string interpolation, and the nullness is observable via EXPR
null-coalescing.

Also address review findings on the wrap-action implementation:

- Fail gracefully when WrappedAction.* injection cannot resolve the
  wrapped action's format strings (e.g. a wrapped onRun referencing
  {{Task.File.*}} or onEnter referencing {{Env.File.*}} — embedded
  files are not materialized on the wrap path yet, a known limitation
  shared with the Rust runtime). The action now FAILs through the
  normal callback path via a new _fail_action_before_start helper and
  the session transitions to READY_ENDING, instead of a raw
  FormatStringError escaping the public API — which, for
  enter_environment, previously left the session stuck in RUNNING
  with no runner. Also covers non-integer FEATURE_BUNDLE_1
  timeout/notifyPeriod resolutions.
- WrappedAction.Environment now carries only openjd_env-defined
  variables per RFC 0008, excluding the environment's declarative
  variables: map seed (parity with the Rust runtime, which was
  already correct).
- WrappedAction.Environment is flattened cumulatively in
  environment-entry order, so a later openjd_env set yields one
  effective entry (not a duplicate) and a later unset removes the
  name — matching the real subprocess environment and Rust's single
  cumulative env_vars map.
- On the onWrapEnvExit path, the openjd_env list is captured before
  the exiting environment is removed from tracking, so the wrapped
  environment's own openjd_env variables appear in
  WrappedAction.Environment just as they do in the real subprocess
  environment.
- Log a warning when a wrap environment is active but run_task() was
  not given step_name, since {{WrappedStep.Name}} renders empty.
- Hoist the Template Schemas 5.3.2 notify-period defaults (120/30)
  into shared constants used by the seeding and both cancel paths.
- Guard _run_wrap_hook against unknown hook names so a typo cannot
  become a silent SUCCESS no-op.

Tests assert `is None` for the undeclared cancelation case, the
injection-failure and READY_ENDING behavior, the openjd_env-only
Environment contents (including override/unset flattening and the
exit-path capture), and the hook-name guard. All 53 WRAP_ACTIONS
conformance tests pass against the Python CLI.

Addresses review feedback on openjd-rs PR OpenJobDescription#261 (discussion
r3597572812) and openjd-specifications PR OpenJobDescription#148 (r3597560515).

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
…view feedback on openjd-specifications PR OpenJobDescription#148, a format-string cancelation mode gets normal format string behavior: partial interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the resolved value is checked against the two mode names at run time. Null semantics (dropping the cancelation object) now apply only when the mode is a whole-field "{{ ... }}" expression, per the Template Schemas 5.3 wording; a normal format string that resolves to the empty string is an invalid mode, matching the openjd-rs typed-resolution behavior.
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
Implement RFC 0008 WRAP_ACTIONS support in the Python sessions runtime,
allowing environments to wrap task and environment actions via the
onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit hooks.

- Run wrap hooks through EnvironmentScriptRunner (wrap_env_enter /
  wrap_env_exit) and the task-run wrap path, injecting Task.Action.*,
  Env.Wrapped.Action.*, and WrappedAction.* symbols per the final RFC
  wording.
- Seed WrappedAction.Cancelation.Mode (string?) and
  WrappedAction.Cancelation.NotifyPeriodInSeconds (int?) for all three
  hooks, applying the Template Schemas 5.3.2 notify-period defaults
  (120 seconds for a task's onRun, 30 otherwise). Mode is None when the
  wrapped action declares no <Cancelation>, preserving the distinction
  from an explicit TERMINATE and rendering as the empty string per
  RFC 0005 null semantics.
- Allow normal format string behavior for the cancelation mode per
  review feedback on openjd-specifications PR OpenJobDescription#148: partial
  interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the
  resolved value is checked against the two mode names at run time.
  Null semantics (dropping the cancelation object) apply only when the
  mode is a whole-field "{{ ... }}" expression, per the Template
  Schemas 5.3 wording; a normal format string that resolves to the
  empty string is an invalid mode, matching the openjd-rs
  typed-resolution behavior.
- Support round-trip cancelation/timeout forwarding: a deferred
  cancelation mode (CancelationMethodDeferred) resolves against the
  live symbol table via a shared resolve_effective_cancelation helper
  used by both the seed path and the cancel enforcement paths, so wrap
  scripts always see the values the runtime enforces. WrappedAction.
  Timeout is int? (None when the wrapped action has no timeout), and
  FormatString timeouts resolve at launch with null meaning "no
  timeout field".
- WrappedAction.Environment carries only openjd_env-defined variables,
  flattened cumulatively in environment-entry order (overrides and
  unsets collapse to the effective entry), with the exiting
  environment's own variables captured before removal on the
  onWrapEnvExit path.
- Fail gracefully (normal FAIL callback path, session to READY_ENDING)
  when WrappedAction.* injection cannot resolve the wrapped action's
  format strings, instead of leaking FormatStringError through the
  public API.
- Inherit Rust-backed EXPR format-string evaluation transparently from
  openjd-model; guard _run_wrap_hook against unknown hook names; warn
  when a wrap environment is active but run_task() lacks step_name.

All WRAP_ACTIONS conformance fixtures pass against the Python CLI,
along with the new wrap/cancelation unit test suites.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
Implement RFC 0008 WRAP_ACTIONS support in the Python sessions runtime,
allowing environments to wrap task and environment actions via the
onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit hooks.

- Run wrap hooks through EnvironmentScriptRunner (wrap_env_enter /
  wrap_env_exit) and the task-run wrap path, injecting Task.Action.*,
  Env.Wrapped.Action.*, and WrappedAction.* symbols per the final RFC
  wording.
- Seed WrappedAction.Cancelation.Mode (string?) and
  WrappedAction.Cancelation.NotifyPeriodInSeconds (int?) for all three
  hooks, applying the Template Schemas 5.3.2 notify-period defaults
  (120 seconds for a task's onRun, 30 otherwise). Mode is None when the
  wrapped action declares no <Cancelation>, preserving the distinction
  from an explicit TERMINATE and rendering as the empty string per
  RFC 0005 null semantics.
- Allow normal format string behavior for the cancelation mode per
  review feedback on openjd-specifications PR OpenJobDescription#148: partial
  interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the
  resolved value is checked against the two mode names at run time.
  Null semantics (dropping the cancelation object) apply only when the
  mode is a whole-field "{{ ... }}" expression, per the Template
  Schemas 5.3 wording; a normal format string that resolves to the
  empty string is an invalid mode, matching the openjd-rs
  typed-resolution behavior.
- Support round-trip cancelation/timeout forwarding: a deferred
  cancelation mode (CancelationMethodDeferred) resolves against the
  live symbol table via a shared resolve_effective_cancelation helper
  used by both the seed path and the cancel enforcement paths, so wrap
  scripts always see the values the runtime enforces. WrappedAction.
  Timeout is int? (None when the wrapped action has no timeout), and
  FormatString timeouts resolve at launch with null meaning "no
  timeout field".
- WrappedAction.Environment carries only openjd_env-defined variables,
  flattened cumulatively in environment-entry order (overrides and
  unsets collapse to the effective entry), with the exiting
  environment's own variables captured before removal on the
  onWrapEnvExit path.
- Fail gracefully (normal FAIL callback path, session to READY_ENDING)
  when WrappedAction.* injection cannot resolve the wrapped action's
  format strings, instead of leaking FormatStringError through the
  public API.
- Inherit Rust-backed EXPR format-string evaluation transparently from
  openjd-model; guard _run_wrap_hook against unknown hook names; warn
  when a wrap environment is active but run_task() lacks step_name.

All WRAP_ACTIONS conformance fixtures pass against the Python CLI,
along with the new wrap/cancelation unit test suites.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
Implement RFC 0008 WRAP_ACTIONS support in the Python sessions runtime,
allowing environments to wrap task and environment actions via the
onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit hooks.

- Run wrap hooks through EnvironmentScriptRunner (wrap_env_enter /
  wrap_env_exit) and the task-run wrap path, injecting Task.Action.*,
  Env.Wrapped.Action.*, and WrappedAction.* symbols per the final RFC
  wording.
- Seed WrappedAction.Cancelation.Mode (string?) and
  WrappedAction.Cancelation.NotifyPeriodInSeconds (int?) for all three
  hooks, applying the Template Schemas 5.3.2 notify-period defaults
  (120 seconds for a task's onRun, 30 otherwise). Mode is None when the
  wrapped action declares no <Cancelation>, preserving the distinction
  from an explicit TERMINATE and rendering as the empty string per
  RFC 0005 null semantics.
- Allow normal format string behavior for the cancelation mode per
  review feedback on openjd-specifications PR OpenJobDescription#148: partial
  interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the
  resolved value is checked against the two mode names at run time.
  Null semantics (dropping the cancelation object) apply only when the
  mode is a whole-field "{{ ... }}" expression, per the Template
  Schemas 5.3 wording; a normal format string that resolves to the
  empty string is an invalid mode, matching the openjd-rs
  typed-resolution behavior.
- Support round-trip cancelation/timeout forwarding: a deferred
  cancelation mode (CancelationMethodDeferred) resolves against the
  live symbol table via a shared resolve_effective_cancelation helper
  used by both the seed path and the cancel enforcement paths, so wrap
  scripts always see the values the runtime enforces. WrappedAction.
  Timeout is int? (None when the wrapped action has no timeout), and
  FormatString timeouts resolve at launch with null meaning "no
  timeout field".
- WrappedAction.Environment carries only openjd_env-defined variables,
  flattened cumulatively in environment-entry order (overrides and
  unsets collapse to the effective entry), with the exiting
  environment's own variables captured before removal on the
  onWrapEnvExit path.
- Fail gracefully (normal FAIL callback path, session to READY_ENDING)
  when WrappedAction.* injection cannot resolve the wrapped action's
  format strings, instead of leaking FormatStringError through the
  public API.
- Inherit Rust-backed EXPR format-string evaluation transparently from
  openjd-model; guard _run_wrap_hook against unknown hook names; warn
  when a wrap environment is active but run_task() lacks step_name.

All WRAP_ACTIONS conformance fixtures pass against the Python CLI,
along with the new wrap/cancelation unit test suites.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 17, 2026
Implement RFC 0008 WRAP_ACTIONS support in the Python sessions runtime,
allowing environments to wrap task and environment actions via the
onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit hooks.

- Run wrap hooks through EnvironmentScriptRunner (wrap_env_enter /
  wrap_env_exit) and the task-run wrap path, injecting Task.Action.*,
  Env.Wrapped.Action.*, and WrappedAction.* symbols per the final RFC
  wording.
- Seed WrappedAction.Cancelation.Mode (string?) and
  WrappedAction.Cancelation.NotifyPeriodInSeconds (int?) for all three
  hooks, applying the Template Schemas 5.3.2 notify-period defaults
  (120 seconds for a task's onRun, 30 otherwise). Mode is None when the
  wrapped action declares no <Cancelation>, preserving the distinction
  from an explicit TERMINATE and rendering as the empty string per
  RFC 0005 null semantics.
- Allow normal format string behavior for the cancelation mode per
  review feedback on openjd-specifications PR OpenJobDescription#148: partial
  interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the
  resolved value is checked against the two mode names at run time.
  Null semantics (dropping the cancelation object) apply only when the
  mode is a whole-field "{{ ... }}" expression, per the Template
  Schemas 5.3 wording; a normal format string that resolves to the
  empty string is an invalid mode, matching the openjd-rs
  typed-resolution behavior.
- Support round-trip cancelation/timeout forwarding: a deferred
  cancelation mode (CancelationMethodDeferred) resolves against the
  live symbol table via a shared resolve_effective_cancelation helper
  used by both the seed path and the cancel enforcement paths, so wrap
  scripts always see the values the runtime enforces. WrappedAction.
  Timeout is int? (None when the wrapped action has no timeout), and
  FormatString timeouts resolve at launch with null meaning "no
  timeout field".
- WrappedAction.Environment carries only openjd_env-defined variables,
  flattened cumulatively in environment-entry order (overrides and
  unsets collapse to the effective entry), with the exiting
  environment's own variables captured before removal on the
  onWrapEnvExit path.
- Fail gracefully (normal FAIL callback path, session to READY_ENDING)
  when WrappedAction.* injection cannot resolve the wrapped action's
  format strings, instead of leaking FormatStringError through the
  public API.
- Inherit Rust-backed EXPR format-string evaluation transparently from
  openjd-model; guard _run_wrap_hook against unknown hook names; warn
  when a wrap environment is active but run_task() lacks step_name.

All WRAP_ACTIONS conformance fixtures pass against the Python CLI,
along with the new wrap/cancelation unit test suites.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
leongdl added a commit to leongdl/openjd-sessions-for-python that referenced this pull request Jul 21, 2026
Implement RFC 0008 WRAP_ACTIONS support in the Python sessions runtime,
allowing environments to wrap task and environment actions via the
onWrapEnvEnter, onWrapTaskRun, and onWrapEnvExit hooks.

- Run wrap hooks through EnvironmentScriptRunner (wrap_env_enter /
  wrap_env_exit) and the task-run wrap path, injecting Task.Action.*,
  Env.Wrapped.Action.*, and WrappedAction.* symbols per the final RFC
  wording.
- Seed WrappedAction.Cancelation.Mode (string?) and
  WrappedAction.Cancelation.NotifyPeriodInSeconds (int?) for all three
  hooks, applying the Template Schemas 5.3.2 notify-period defaults
  (120 seconds for a task's onRun, 30 otherwise). Mode is None when the
  wrapped action declares no <Cancelation>, preserving the distinction
  from an explicit TERMINATE and rendering as the empty string per
  RFC 0005 null semantics.
- Allow normal format string behavior for the cancelation mode per
  review feedback on openjd-specifications PR OpenJobDescription#148: partial
  interpolation like "{{ 'NOTIFY' }}_THEN_TERMINATE" is valid, and the
  resolved value is checked against the two mode names at run time.
  Null semantics (dropping the cancelation object) apply only when the
  mode is a whole-field "{{ ... }}" expression, per the Template
  Schemas 5.3 wording; a normal format string that resolves to the
  empty string is an invalid mode, matching the openjd-rs
  typed-resolution behavior.
- Support round-trip cancelation/timeout forwarding: a deferred
  cancelation mode (CancelationMethodDeferred) resolves against the
  live symbol table via a shared resolve_effective_cancelation helper
  used by both the seed path and the cancel enforcement paths, so wrap
  scripts always see the values the runtime enforces. WrappedAction.
  Timeout is int? (None when the wrapped action has no timeout), and
  FormatString timeouts resolve at launch with null meaning "no
  timeout field".
- WrappedAction.Environment carries only openjd_env-defined variables,
  flattened cumulatively in environment-entry order (overrides and
  unsets collapse to the effective entry), with the exiting
  environment's own variables captured before removal on the
  onWrapEnvExit path.
- Fail gracefully (normal FAIL callback path, session to READY_ENDING)
  when WrappedAction.* injection cannot resolve the wrapped action's
  format strings, instead of leaking FormatStringError through the
  public API.
- Inherit Rust-backed EXPR format-string evaluation transparently from
  openjd-model; guard _run_wrap_hook against unknown hook names; warn
  when a wrap environment is active but run_task() lacks step_name.

All WRAP_ACTIONS conformance fixtures pass against the Python CLI,
along with the new wrap/cancelation unit test suites.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Process exit code is out of range

3 participants