Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 47 additions & 0 deletions conformance-tests/2023-09/WRAP_ACTIONS/README.md

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.

Quorum finding: NOT-GOOD as-is — the added tree diagram documents proposed/ containing wrap-repr-py-escapes-newline-in-wrapped-args.test.yaml and a proposed/README.md, but neither exists on this branch (they live on PR #162's branch). Either note the dependency on #162 or drop those lines; the 24 real fixture entries in the README are complete and correctly grouped.

Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,53 @@ rule beyond action args (`env_templates/`):
└── 4--wrappedaction-in-embedded-file.invalid.yaml
```

A second audit round added fixtures for the remaining coverage gaps
(RFC 0008 test-coverage review):

```
WRAP_ACTIONS/
├── env_templates/
│ │ # Variable scope negatives (RFC §Template variables)
│ ├── 4--wrappedenv-name-in-onwraptaskrun.invalid.yaml
│ ├── 4--wrappedstep-name-in-onwrapenventer.invalid.yaml
│ ├── 4--wrappedstep-name-in-onwrapenvexit.invalid.yaml
│ ├── 4--wrappedaction-in-expr-funcall-outside-hook.invalid.yaml
│ ├── 4--wrappedaction-in-let-outside-hook.invalid.yaml
│ │ # All-or-nothing rule — remaining reject subsets
│ ├── 4.3--wrap-only-onwrap-env-enter.invalid.yaml
│ ├── 4.3--wrap-only-onwrap-env-exit.invalid.yaml
│ ├── 4.3--wrap-enter-and-exit-missing-task-run.invalid.yaml
│ ├── 4.3--wrap-run-and-exit-missing-enter.invalid.yaml
│ │ # FEATURE_BUNDLE_1 accept-side twin of the gated-form rejects
│ └── 4.2--wrap-fmtstring-timeout-and-mode-with-feature-bundle.yaml
├── jobs/
│ │ # Absent-by-design: host env vars excluded (RFC §Host environment
│ │ # variables and embedded file paths)
│ ├── wrap-environment-excludes-host-vars.test.yaml
│ │ # Macro propagation × emitter (RFC §Stdout forwarding)
│ ├── wrap-openjd-env-from-wrap-script-itself.test.yaml
│ ├── wrap-openjd-env-task-grand-child-visible-next-task.test.yaml
│ ├── wrap-openjd-env-grand-child-under-exit-hook.test.yaml
│ ├── wrap-progress-status-macros-forwarded.test.yaml
│ ├── wrap-stderr-forwarded-from-grand-child.test.yaml
│ ├── wrap-discarded-grand-child-stdout-loses-macro.test.yaml
│ │ # Nesting depth 2 and the nothing-to-replace rule
│ ├── wrap-two-inner-envs-wrappedenv-name-per-invocation.test.yaml
│ ├── wrap-variables-only-inner-env-skips-hooks.test.yaml
│ │ # Failure-path breadth (RFC §Failure semantics, §Lifecycle)
│ ├── wrap-failed-exit-hook-own-onexit-still-runs.test.yaml
│ ├── wrap-grand-child-fails-under-enter-hook.test.yaml
│ ├── wrap-unwrapped-parity-task-failure.test.yaml
│ │ # Session composition (How Jobs Are Run)
│ ├── wrap-two-steps-wrappedstep-name-per-step.test.yaml
│ ├── wrap-job-env-wraps-step-env.test.yaml
│ └── wrap-no-wrap-control.test.yaml
```

(A `jobs/proposed/` directory of spec-correct fixtures that fail against the
current reference implementations is added separately by the
expected-failures PR; it is documented there.)

Most execution tests use POSIX shell commands (`sh`, `bash`, `echo`,
`printf`) and are gated to `runOn: [posix]`. Fixtures that carry no
`runOn` gate use `python`, the suite's portable interpreter — including
Expand Down

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.

Quorum verdict: GOOD (5/5) — spec: §4.3.1 / RFC 0008 scope rule (WrappedAction.* only inside the three wrap hooks). Single defect (the onEnter funcall); the in-hook repr_py uses are legal, so a correct implementation can only reject via the scope rule. Targets validator expression-walking blind spots untested on mainline.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# RFC 0008 Template variables: "Templates MUST NOT reference
# WrappedAction.* outside the three wrap hooks. Schedulers MUST reject
# templates that violate this scope rule." Here the out-of-scope reference
# is buried inside an EXPR function call — repr_py(WrappedAction.Command)
# in the env's own onEnter — not written as a bare {{WrappedAction.X}}
# format string. Catches a validator that only scope-checks direct
# variable interpolations and does not walk expression arguments (the
# same blind spot as the historical Task.File.Run.name bug, which behaved
# differently inside a function call than as a direct reference). Single
# defect: the template is valid except for the WrappedAction.Command
# reference inside the function call in onEnter.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapActionInFuncall
script:
actions:
onEnter:
command: python
args: ["-c", "print({{repr_py(WrappedAction.Command)}})"]
onWrapEnvEnter:
command: python
args: ["-c", "print({{repr_py(WrappedAction.Command)}})"]
onWrapTaskRun:
command: python
args: ["-c", "print({{repr_py(WrappedAction.Command)}})"]
onWrapEnvExit:
command: python
args: ["-c", "print({{repr_py(WrappedAction.Command)}})"]

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.

Quorum verdict: GOOD-WITH-NITS (3 reviewers flagged the same nit) — spec: §4.3.1 scope rule + §3.6.2 let-scope table. Fixture is invalid under any reading, but the header's "single defect = onEnter's use of the binding; the hooks' use is fine" framing conflicts with §3.6.2: WrappedAction.* is not in EnvironmentScript.let's symbol table at all, so the let binding itself is the violation. Reword the comment; the fixture can't distinguish let-walking validators from generic let-scope checks.

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# RFC 0008 Template variables: "Templates MUST NOT reference
# WrappedAction.* outside the three wrap hooks. Schedulers MUST reject
# templates that violate this scope rule." Here the out-of-scope reference
# is buried inside a script-level `let` binding. Per Template Schemas
# 3.6.2, <EnvironmentScript>.let's symbol scope does not include
# WrappedAction.* at all — the binding itself is the violation (script-
# level let is evaluated once at environment entry and shared by ALL of
# the script's actions, including onEnter/onExit where WrappedAction.*
# never exists). Catches a validator that scope-checks format strings in
# action fields but never walks let-binding expressions (the
# `let`-indirection blind spot). The template is valid except for the
# WrappedAction-derived binding and its consumption.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapActionInLet
script:
let:
- wrapped_cmd = WrappedAction.Command
actions:
onEnter:
command: python
args: ["-c", "print({{repr_py(wrapped_cmd)}})"]
onWrapEnvEnter:
command: python
args: ["-c", "print({{repr_py(wrapped_cmd)}})"]
onWrapTaskRun:
command: python
args: ["-c", "print({{repr_py(wrapped_cmd)}})"]
onWrapEnvExit:
command: python
args: ["-c", "print({{repr_py(wrapped_cmd)}})"]

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.

Quorum verdict: GOOD-WITH-NITS — spec: §4.3.1 (WrappedEnv.* only in env hooks). Clean single defect. Nit: repr_py is gratuitous (bare {{WrappedEnv.Name}} suffices) and adds a wrong-reason surface — an implementation lacking repr_py rejects for an unrelated reason and still passes this .invalid test. Mitigated suite-wide by valid fixtures using repr_py.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# RFC 0008 Template variables: WrappedEnv.Name is available only in
# onWrapEnvEnter and onWrapEnvExit. "Templates MUST NOT reference
# WrappedEnv.* outside onWrapEnvEnter and onWrapEnvExit. Schedulers MUST
# reject templates that violate this scope rule." Referencing it in
# onWrapTaskRun — a wrap hook, but the wrong one — must be rejected at
# validation time. Catches a validator that treats all three hooks as one
# scope. Single defect: the template is valid except for the WrappedEnv.Name
# reference in onWrapTaskRun.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapEnvNameInTaskRun
script:
actions:
onWrapEnvEnter:
command: python
args: ["-c", "print(r'{{WrappedEnv.Name}}')"]
onWrapTaskRun:
command: python
args: ["-c", "print(r'{{WrappedEnv.Name}}')"]
onWrapEnvExit:
command: python
args: ["-c", "print(r'{{WrappedEnv.Name}}')"]

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.

Quorum verdict: GOOD-WITH-NITS — spec: §4.3.1 (WrappedStep.* only in onWrapTaskRun). Single defect in the enter hook. Same gratuitous-repr_py nit as siblings.

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# RFC 0008 Template variables: WrappedStep.Name is available only in
# onWrapTaskRun. "Templates MUST NOT reference ... WrappedStep.* outside
# onWrapTaskRun. Schedulers MUST reject templates that violate this scope
# rule." Referencing it in onWrapEnvEnter must be rejected at validation
# time — an environment's lifecycle action has no associated step. Catches
# a validator that injects WrappedStep.* into all three hook scopes.
# Single defect: the template is valid except for the WrappedStep.Name
# reference in onWrapEnvEnter.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapStepNameInEnter
script:
actions:
onWrapEnvEnter:
command: python
args: ["-c", "print(r'{{WrappedStep.Name}}')"]
onWrapTaskRun:
command: python
args: ["-c", "print(r'{{WrappedStep.Name}}')"]
onWrapEnvExit:
command: python
args: ["-c", "print(r'{{WrappedEnv.Name}}')"]

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.

Quorum verdict: GOOD-WITH-NITS — spec: §4.3.1. Correct twin of the enter-hook fixture; coverage reviewer notes it's a near-duplicate (same validator scope-table path) — cheap enough to keep, but the pair likely catches the same bugs. Same repr_py nit.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# RFC 0008 Template variables: WrappedStep.Name is available only in
# onWrapTaskRun. Referencing it in onWrapEnvExit must be rejected at
# validation time. The exit hook is the twin of the enter hook, but a
# validator can plausibly scope the two env hooks differently (exit hooks
# fire during teardown when a "current step" may exist in the runtime's
# state), so both directions are pinned. Single defect: the template is
# valid except for the WrappedStep.Name reference in onWrapEnvExit.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapStepNameInExit
script:
actions:
onWrapEnvEnter:
command: python
args: ["-c", "print(r'{{WrappedEnv.Name}}')"]
onWrapTaskRun:
command: python
args: ["-c", "print(r'{{WrappedStep.Name}}')"]
onWrapEnvExit:
command: python
args: ["-c", "print(r'{{WrappedStep.Name}}')"]

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.

Quorum verdict: GOOD-WITH-NITS — spec: §5 (timeout fmtstring @fb1) + §5.3 (mode/notifyPeriod fmtstring @fb1), accept-side twin of two mainline reject fixtures — a real gap filled. Nit: it sits in WRAP_ACTIONS/env_templates but additionally requires FEATURE_BUNDLE_1, and the runner does not gate by declared extensions — an implementation with WRAP_ACTIONS+EXPR but no FB1 falsely fails this "valid" fixture. Consider noting the extra requirement or relocating.

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# RFC 0008 Cancelation behavior + Template Schemas 5/5.3 (FEATURE_BUNDLE_1):
# the positive twin of 4.2--wrap-fmtstring-timeout-no-feature-bundle and
# wrap-cancelation-fmtstring-mode-no-feature-bundle. A format-string
# `timeout` and a format-string `cancelation.mode` on a wrap hook are the
# FEATURE_BUNDLE_1-gated forms of the round-trip forwarding pattern; with
# the extension listed, validation MUST ACCEPT them. Catches a validator
# that rejects the format-string forms unconditionally ("format strings in
# timeout are not allowed") instead of gating them on the extension — the
# reject-side fixtures alone cannot distinguish "gated" from "never
# permitted".
#
# NOTE: unlike its WRAP_ACTIONS siblings, this valid fixture additionally
# requires FEATURE_BUNDLE_1 support. The runner does not gate fixtures by
# declared extensions, so an implementation with WRAP_ACTIONS+EXPR but no
# FEATURE_BUNDLE_1 will (correctly, per its own capabilities) reject this
# template and fail the fixture.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
- FEATURE_BUNDLE_1
environment:
name: WrapFmtStringGatedAccepted
script:
actions:
onWrapEnvEnter:
command: echo
args: ["{{WrappedEnv.Name}}"]
timeout: "{{WrappedAction.Timeout}}"
onWrapTaskRun:
command: echo
args: ["{{WrappedAction.Command}}"]
timeout: "{{WrappedAction.Timeout}}"
cancelation:
mode: "{{WrappedAction.Cancelation.Mode}}"
notifyPeriodInSeconds: "{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}"
onWrapEnvExit:
command: echo
args: ["{{WrappedEnv.Name}}"]
timeout: "{{WrappedAction.Timeout}}"

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.

Quorum verdict: GOOD (5/5) — spec: §4.3 constraint 1 (all-or-nothing). Minimal, schema-valid but for the missing onWrapTaskRun; rejection can only come from the named rule. Completes the invalid matrix with its three siblings (mainline had only 2 of 6 cells).

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# RFC 0008 all-or-nothing rule: defining onWrapEnvEnter and onWrapEnvExit
# without onWrapTaskRun is invalid. This is the "env lifecycle only" subset
# an author writes when they only want to intercept environment setup and
# teardown — plausible, but rejected: the rule guarantees execution context
# parity across every wrapped lifecycle phase, tasks included. Single
# defect: the template is valid except for the missing onWrapTaskRun.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: EnterExitNoTaskRun
script:
actions:
onWrapEnvEnter:
command: echo
args: ["{{WrappedEnv.Name}}"]
onWrapEnvExit:
command: echo
args: ["{{WrappedEnv.Name}}"]

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.

Quorum verdict: GOOD (5/5) — spec: §4.3 constraint 1 (all-or-nothing), enter-only subset. Clean single defect.

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# RFC 0008 all-or-nothing rule: an environment that defines any wrap hook
# must define all three (onWrapEnvEnter, onWrapTaskRun, onWrapEnvExit).
# Defining only onWrapEnvEnter is invalid. Catches a validator that only
# checks for the presence of onWrapTaskRun when deciding whether the
# all-or-nothing rule applies. Single defect: the template is valid except
# for the missing onWrapTaskRun and onWrapEnvExit hooks.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: OnlyWrapEnter
script:
actions:
onWrapEnvEnter:
command: echo
args: ["{{WrappedEnv.Name}}"]

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.

Quorum verdict: GOOD (5/5) — spec: §4.3 constraint 1, exit-only subset. Coverage reviewer rates it the weakest matrix cell (siblings almost certainly catch the same validators) but completing the matrix is cheap and guards asymmetric validators.

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# RFC 0008 all-or-nothing rule: an environment that defines any wrap hook
# must define all three (onWrapEnvEnter, onWrapTaskRun, onWrapEnvExit).
# Defining only onWrapEnvExit is invalid. Catches a validator that treats
# the exit hook as optional teardown rather than part of the atomic hook
# set. Single defect: the template is valid except for the missing
# onWrapEnvEnter and onWrapTaskRun hooks.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: OnlyWrapExit
script:
actions:
onWrapEnvExit:
command: echo
args: ["{{WrappedEnv.Name}}"]

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.

Quorum verdict: GOOD (5/5) — spec: §4.3 constraint 1, missing-enter direction. All refs in legal hooks; only the missing onWrapEnvEnter can trigger rejection in a correct implementation.

Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# RFC 0008 all-or-nothing rule: defining onWrapTaskRun and onWrapEnvExit
# without onWrapEnvEnter is invalid. Catches a validator that only demands
# the hooks needed for the phases it has seen so far (tasks and teardown)
# instead of the full atomic set. Single defect: the template is valid
# except for the missing onWrapEnvEnter.
specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: RunExitNoEnter
script:
actions:
onWrapTaskRun:
command: echo
args: ["{{WrappedAction.Command}}"]
onWrapEnvExit:
command: echo
args: ["{{WrappedEnv.Name}}"]

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.

Quorum verdict: GOOD-WITH-NITS — spec: RFC 0008 stdout forwarding (runtime scans the wrap script's stdout only, never the grand-child's stream directly). Effect-based negative control, correctly paired with the excludes-host-vars sibling. Nit (service-compat reviewer): the forbidden literal LOST_VAR=should-not-surface also occurs inside the resolved hook command line via repr_py(WrappedAction.Args); a runner that echoes resolved commands into the session log false-fails. Dropping that forbidden line (keeping the WAENV=LOST_VAR one) makes it robust.

Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# RFC 0008 "Stdout forwarding and macro propagation" (negative control):
# runtimes MUST scan the WRAP SCRIPT's stdout for macros and "MUST NOT
# scan the grand-child's stdout directly". If the wrap script discards the
# grand-child's stdout (here: redirected to DEVNULL), a macro the
# grand-child emitted never reaches the wrap script's stdout, so the
# runtime must never see it and the variable must NOT surface in
# WrappedAction.Environment. Catches a runtime that locates and scans the
# grand-child process's output stream directly — the design the RFC
# explicitly rejected ("would require locating the grand-child across
# heterogeneous runtimes").
template:
specificationVersion: jobtemplate-2023-09
name: WrapDiscardedGrandChildStdoutLosesMacro
jobEnvironments:
- name: InnerEnv
script:
actions:
onEnter:
command: python
args: ["-c", "print('openjd_env: LOST_VAR=should-not-surface')"]
steps:
- name: Step1
script:
actions:
onRun:
command: echo
args: ["placeholder"]
environments:
- specificationVersion: environment-2023-09
extensions:
- WRAP_ACTIONS
- EXPR
environment:
name: WrapEnv
script:
actions:
# Run the wrapped onEnter but discard its stdout: its macro line
# must be lost.
onWrapEnvEnter:
command: python
args:
- "-c"
- "import subprocess,sys; sys.exit(subprocess.run([{{repr_py(WrappedAction.Command)}}]+{{repr_py(WrappedAction.Args)}}, stdout=subprocess.DEVNULL).returncode)"
onWrapTaskRun:
command: python
args:
- "-c"
- "for e in {{repr_py(WrappedAction.Environment)}}:\n print('WAENV='+e)\nprint('WAENV_DUMP_DONE')"
onWrapEnvExit:
command: python
args:
- "-c"
- "import subprocess,sys; sys.exit(subprocess.run([{{repr_py(WrappedAction.Command)}}]+{{repr_py(WrappedAction.Args)}}).returncode)"
expected:
output:
- WAENV_DUMP_DONE
# Only the WAENV= form is forbidden: the raw macro string
# "LOST_VAR=should-not-surface" also occurs inside the resolved wrap-hook
# command line (via repr_py(WrappedAction.Args)), so forbidding it would
# false-fail any runner that echoes resolved commands into the log.
forbidden:
- WAENV=LOST_VAR
Loading
Loading