diff --git a/app/sep/apps/atw/batch.py b/app/sep/apps/atw/batch.py
index 5299dbca1..351c57d93 100644
--- a/app/sep/apps/atw/batch.py
+++ b/app/sep/apps/atw/batch.py
@@ -37,13 +37,16 @@
NonEmptyStr,
UTCDatetime,
)
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ EXTRA_ARGS_FIELD_NAME,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
from app.sep.apps.framework.schema import (
AnyField,
BoolField,
- EXECUTOR_HOST_FIELD_NAME,
HostField,
- SCRIPT_PREVIEW_FIELD_NAME,
- SUDO_FIELD_NAME,
)
from app.sep.apps.framework.script_helpers import execute_script
from app.sep.apps.framework.script_source import (
@@ -52,7 +55,6 @@
ScriptExecutionResponse,
)
from app.sep.apps.labels import EXECUTION_HOST_LABEL
-from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.script_source import snippet_source, SnippetScript
from app.tasks.models import TaskHistoryStatusEnum
diff --git a/app/sep/apps/dipper/deps.py b/app/sep/apps/dipper/deps.py
index bd9424f2f..8660d65bc 100644
--- a/app/sep/apps/dipper/deps.py
+++ b/app/sep/apps/dipper/deps.py
@@ -297,10 +297,36 @@ def build_dipper_meta_from_args(
*,
sudo_default: bool = False,
) -> SnippetExecutionMeta:
- """Build shared execution metadata for legacy and JSON Dipper flows."""
+ """Build shared execution metadata for legacy and JSON Dipper flows.
+
+ Both flows assemble their meta here, so the guard against invalid frontmatter
+ parameters lives here rather than at either entry point. A parameter the
+ frontmatter declared but the parser rejected -- a reserved execution field
+ name, say -- is dropped from the form, so executing anyway would silently run
+ the script without an argument its author asked for.
+
+ The refusal enumerates the parser's own messages because this app does not
+ surface them on the form: without them the operator would see a rejection
+ naming no cause anywhere in the UI.
+
+ :param service: The inventory service the collector runs against.
+ :param script: The collector script being dispatched.
+ :param script_source: The signed URL the executor downloads the script from.
+ :param execution_args: The validated arguments for this execution.
+ :param sudo_default: Whether to run with ``sudo`` when the args request nothing.
+ :return: The execution metadata the framework posts to the Tasks API.
+ :raises HTTPBadRequestException: When the script declares no runnable
+ interpreter, or carries invalid frontmatter parameters.
+ """
interpreter = script.execution_interpreter
if interpreter is None:
raise HTTPBadRequestException(detail="No interpreter configured for script")
+ if not script.can_execute:
+ reasons = "; ".join(script.validated_parameters.errors)
+ raise HTTPBadRequestException(
+ detail=f"Script {script.filename!r} has invalid frontmatter parameters: "
+ f"{reasons}"
+ )
return build_execution_meta(
script,
execution_args,
diff --git a/app/sep/apps/dipper/schema.py b/app/sep/apps/dipper/schema.py
index 608e8d9dd..91e51b789 100644
--- a/app/sep/apps/dipper/schema.py
+++ b/app/sep/apps/dipper/schema.py
@@ -19,6 +19,11 @@
from urllib.parse import urlencode
from app.inventory.models import ServiceTypeEnum
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
from app.sep.apps.framework.schema import (
AnyField,
AppSchema,
@@ -32,14 +37,11 @@
DetailSection,
DetailView,
EXECUTION_HOST_LABEL,
- EXECUTOR_HOST_FIELD_NAME,
FormSection,
HostField,
ListView,
- SCRIPT_PREVIEW_FIELD_NAME,
ScriptPreviewField,
ServiceField,
- SUDO_FIELD_NAME,
)
from app.sep.snippets.config import SnippetSudoOption
from app.sep.snippets.models.snippet import BaseSnippet
diff --git a/app/sep/apps/field_names.py b/app/sep/apps/field_names.py
new file mode 100644
index 000000000..c55121b6f
--- /dev/null
+++ b/app/sep/apps/field_names.py
@@ -0,0 +1,68 @@
+# Copyright (C) 2026 Percona LLC
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see .
+
+"""Name the execution fields a script app's form can synthesize.
+
+Together these are the union of what the script apps append to a script's
+frontmatter parameters when building an execution form -- each app synthesizes
+its own subset, and some of those are conditional on the script's configuration.
+A consumer that merges or strips them needs the same spelling the producer used,
+whichever app produced them. :data:`RESERVED_EXECUTION_FIELD_NAMES` closes the loop
+back at the other producer of form fields, the script author: a frontmatter
+parameter declaring one of these names is rejected at parse time by
+:meth:`app.sep.snippets.models.meta.SnippetMetaParameter._reject_reserved_name`, so
+a synthesized field can never collide with an author-declared one.
+
+Lives alongside (not inside) :mod:`app.sep.apps.framework` because both ends of an
+existing one-way dependency need these spellings:
+:mod:`app.sep.apps.framework.script_helpers` imports
+:mod:`app.sep.snippets.models.snippet`, so nothing under ``app.sep.snippets.models``
+can import back from ``framework``. This module imports nothing, so the form
+builders under ``framework`` and the parameter model under ``snippets.models`` can
+both depend on it without closing that loop.
+"""
+
+EXECUTOR_HOST_FIELD_NAME = "executor_host"
+"""Name the synthesized execution-host field on the wire."""
+
+SUDO_FIELD_NAME = "sudo"
+"""Name the synthesized sudo toggle on the wire."""
+
+SCRIPT_PREVIEW_FIELD_NAME = "script_preview"
+"""Name the synthesized script-preview field on the wire."""
+
+EXTRA_ARGS_FIELD_NAME = "extra_args"
+"""Name the synthesized Extra Args field on the wire."""
+
+RESERVED_EXECUTION_FIELD_NAMES = frozenset(
+ {
+ EXECUTOR_HOST_FIELD_NAME,
+ SUDO_FIELD_NAME,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ EXTRA_ARGS_FIELD_NAME,
+ }
+)
+"""Reserve every synthesized field name against frontmatter parameter names.
+
+Reservation is unconditional and app-wide: a name is reserved even for a script
+whose app never synthesizes that particular field -- a ``sudo: never`` snippet, a
+disk-backed script app with no preview endpoint -- because the alternative is a
+per-app, per-configuration rule that the author of a frontmatter file cannot
+evaluate from the file in front of them.
+
+The members are the same constants the form builders synthesize from, so reserving
+a fifth name is a two-line edit in this module and cannot be forgotten at the
+validator.
+"""
diff --git a/app/sep/apps/framework/schema.py b/app/sep/apps/framework/schema.py
index 7878b252f..c9a2e452a 100644
--- a/app/sep/apps/framework/schema.py
+++ b/app/sep/apps/framework/schema.py
@@ -89,17 +89,6 @@
)
from app.sep.apps.labels import EXECUTION_HOST_LABEL
-EXECUTOR_HOST_FIELD_NAME = "executor_host"
-SUDO_FIELD_NAME = "sudo"
-SCRIPT_PREVIEW_FIELD_NAME = "script_preview"
-"""Name the execution fields every script app's form synthesises.
-
-Each script app appends these to its frontmatter parameters, and a consumer that
-merges or strips them needs the same spelling the producer used. They live here,
-next to the field types they name, so no app package owns the vocabulary its
-siblings depend on.
-"""
-
# Dots are permitted so nested one-of branch fields can use paths such as
# ``source.source_db_id`` (see :class:`OneOfGroup`).
_FIELD_NAME_PATTERN = r"^[A-Za-z_](?:[\w.-]*\w)?$"
diff --git a/app/sep/apps/shared/disk_script_source.py b/app/sep/apps/shared/disk_script_source.py
index 7f6e58f65..41e923449 100644
--- a/app/sep/apps/shared/disk_script_source.py
+++ b/app/sep/apps/shared/disk_script_source.py
@@ -46,17 +46,16 @@
HTTPNotFoundException,
HTTPUnprocessableEntityException,
)
+from app.sep.apps.field_names import EXECUTOR_HOST_FIELD_NAME, SUDO_FIELD_NAME
from app.sep.apps.framework.list_query import in_memory_list_scripts
from app.sep.apps.framework.schema import (
AppSchema,
BoolField,
Column,
EXECUTION_HOST_LABEL,
- EXECUTOR_HOST_FIELD_NAME,
FormSection,
HostField,
ListView,
- SUDO_FIELD_NAME,
)
from app.sep.apps.framework.script_helpers import (
build_artifact_download_url,
diff --git a/app/sep/snippets/models/constants.py b/app/sep/snippets/models/constants.py
index df4f2074b..390ef626d 100644
--- a/app/sep/snippets/models/constants.py
+++ b/app/sep/snippets/models/constants.py
@@ -13,7 +13,7 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see .
-"""Define wire-name constants shared across snippet and framework schema modules."""
+"""Define constants shared across snippet meta models."""
from enum import StrEnum
@@ -25,14 +25,3 @@ class TextInputHTMLElement(EnumFieldMixin, StrEnum):
INPUT = "input"
TEXTAREA = "textarea"
-
-
-EXTRA_ARGS_FIELD_NAME = "extra_args"
-"""Name the synthesized Extra Args execution field on the wire.
-
-Shared, cycle-free home for this spelling: ``app.sep.apps.framework.schema``
-and ``app.sep.snippets.models.snippet`` both need it, but ``framework``
-imports ``snippet`` (via ``script_helpers.py``), so ``snippet`` can't import
-back from ``framework``. This leaf module has no imports of its own, so both
-sides can depend on it without cycling.
-"""
diff --git a/app/sep/snippets/models/meta.py b/app/sep/snippets/models/meta.py
index c23eaa475..c4e2aed34 100644
--- a/app/sep/snippets/models/meta.py
+++ b/app/sep/snippets/models/meta.py
@@ -67,8 +67,10 @@
field_with_metadata,
loc_to_dot_sep,
)
+from app.sep.apps.field_names import (
+ RESERVED_EXECUTION_FIELD_NAMES,
+)
from app.sep.snippets.models.constants import (
- EXTRA_ARGS_FIELD_NAME,
TextInputHTMLElement,
)
@@ -310,14 +312,21 @@ class SnippetMetaParameter(BaseModel):
def _reject_reserved_name(cls, value: str) -> str:
"""Reject a parameter name reserved for a synthesized execution field.
+ Reservation is unconditional and case-sensitive. A name in
+ :data:`~app.sep.apps.field_names.RESERVED_EXECUTION_FIELD_NAMES` is
+ rejected even when the app rendering this script never synthesises that
+ particular field, so an author can apply the rule to the frontmatter in
+ front of them without knowing which app will render it.
+
:param value: The candidate parameter name.
:return: ``value`` unchanged, when it is not reserved.
:raises ValueError: When the name matches a reserved synthesized field name.
"""
- if value == EXTRA_ARGS_FIELD_NAME:
+ if value in RESERVED_EXECUTION_FIELD_NAMES:
+ reserved = ", ".join(sorted(RESERVED_EXECUTION_FIELD_NAMES))
raise ValueError(
- f"parameter name {value!r} is reserved for the synthesized "
- "Extra Args field"
+ f"parameter name {value!r} is reserved for a synthesized "
+ f"execution field; rename it (reserved names: {reserved})"
)
return value
diff --git a/app/sep/snippets/models/snippet.py b/app/sep/snippets/models/snippet.py
index 919d66aa6..a90f5b050 100644
--- a/app/sep/snippets/models/snippet.py
+++ b/app/sep/snippets/models/snippet.py
@@ -64,6 +64,7 @@
UTCDatetime,
)
from app.core.utils.pydantic import CustomFieldMetadata
+from app.sep.apps.field_names import EXTRA_ARGS_FIELD_NAME, SUDO_FIELD_NAME
from app.sep.snippets.config import (
DEFAULT_SNIPPETS_TASK,
SnippetFilterType,
@@ -74,7 +75,6 @@
if TYPE_CHECKING:
from aiofiles.threadpool.text import AsyncTextIOWrapper
-from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.models.meta import (
META_KEY_DESCRIPTION,
META_KEY_SERVICE_TYPE,
@@ -274,18 +274,17 @@ async def _from_path(
class BaseSnippetArgs(BaseModel):
- """Base model for validating snippet execution arguments.
+ """Validate the arguments a snippet execution was submitted with.
:cvar extra_args_field: The name of the field used to store extra arguments
- ("extra_args").
- :vartype extra_args_field: ClassVar[str]
+ (``extra_args``).
+ :cvar sudo_field: The name of the field used to store the sudo toggle (``sudo``).
:param executor_host: The hostname of the target system where the snippet will be
executed.
- :type executor_host: NonEmptyStr
"""
extra_args_field: ClassVar[str] = EXTRA_ARGS_FIELD_NAME
- sudo_field: ClassVar[str] = "sudo"
+ sudo_field: ClassVar[str] = SUDO_FIELD_NAME
executor_host: NonEmptyStr = Field(
validation_alias=EXECUTOR_HOSTS_INPUT_NAME, exclude=True
)
diff --git a/app/sep/snippets/schema.py b/app/sep/snippets/schema.py
index 2748eba19..ba42a1851 100644
--- a/app/sep/snippets/schema.py
+++ b/app/sep/snippets/schema.py
@@ -33,6 +33,12 @@
from typing import cast
from urllib.parse import urlencode
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ EXTRA_ARGS_FIELD_NAME,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
from app.sep.apps.framework.rules import (
evaluate_conditional_rules,
extract_forbidden_field_gate_plan,
@@ -54,19 +60,15 @@
ColumnFormat,
DateTimeField,
EXECUTION_HOST_LABEL,
- EXECUTOR_HOST_FIELD_NAME,
FloatField,
FormSection,
HostField,
IntegerField,
ListView,
- SCRIPT_PREVIEW_FIELD_NAME,
ScriptPreviewField,
StringField,
- SUDO_FIELD_NAME,
)
from app.sep.snippets.config import SnippetSudoOption
-from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
from app.sep.snippets.models.meta import (
SnippetMetaParameter,
SnippetMetaParameterType,
diff --git a/changelog.d/SEP-1715.fixed.md b/changelog.d/SEP-1715.fixed.md
new file mode 100644
index 000000000..ac0a97cff
--- /dev/null
+++ b/changelog.d/SEP-1715.fixed.md
@@ -0,0 +1 @@
+A snippet or script whose frontmatter declares a parameter named "executor_host", "sudo" or "script_preview" no longer fails to render its execution form with an opaque "duplicate field name(s) across form sections" error. Those three names are now reserved alongside "extra_args": a script declaring any of them fails parameter validation with a message naming the parameter, and cannot be executed until it is renamed unless invalid parameters are configured to be ignored. Reservation is unconditional and applies to every script app, so "sudo" is reserved even on a script configured never to use sudo. Dipper Data Collection now refuses to dispatch a collector script carrying invalid frontmatter parameters, matching the snippets and disk-backed script apps, instead of running it without the arguments its author declared; the refusal lists the validation messages, which that app does not show on the form.
diff --git a/pyproject.toml b/pyproject.toml
index 13c065302..718090991 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -287,6 +287,7 @@ order-by-type = false
"test_*.py" = ["S", "ARG001", "ARG002", "ANN", "SLF001"]
"conftest.py" = ["S", "ARG001", "ARG002"]
"tests/app/sep/apps/framework/contract_suite.py" = ["S"] # pytest contract-suite mixin uses bare assert
+"tests/app/sep/form_schema_utils.py" = ["S"] # shared form-schema assertion helper uses bare assert
"scripts/*.py" = [
"T201", # CLI scripts use print() for user-facing output
"S603", # CLI scripts coordinate git/gh/make subprocesses with literal arg lists
diff --git a/tests/app/sep/apps/dipper/test_deps.py b/tests/app/sep/apps/dipper/test_deps.py
index 4a95a4e86..149e111ef 100644
--- a/tests/app/sep/apps/dipper/test_deps.py
+++ b/tests/app/sep/apps/dipper/test_deps.py
@@ -31,9 +31,11 @@
get_dipper_script_filename,
has_pmm_script,
)
+from app.sep.apps.dipper.models import DipperScript
+from app.sep.apps.field_names import RESERVED_EXECUTION_FIELD_NAMES
from app.sep.clients.pmm import PMMRemoteAPI
from app.sep.inventory import CreatedService
-from app.sep.snippets.config import SnippetSudoOption
+from app.sep.snippets.config import snippets_settings, SnippetSudoOption
from tests.app.factories import CreatedNodeFactory, CreatedServiceFactory
@@ -137,6 +139,71 @@ def test_interpreter_does_not_become_sudo_none_string(self):
pass # correct — guard fired before producing the bad string
+def _script_with_parameter(name: str) -> DipperScript:
+ """Return a DB-free Dipper script declaring a single frontmatter parameter.
+
+ ``sudo: never`` keeps the script's own configuration out of the way, so a
+ failure here can only come from the parameter name.
+
+ :param name: The frontmatter parameter name the script declares.
+ :return: A collector script backed by no database row.
+ """
+ return DipperScript(
+ filename="collect.sh",
+ size=1,
+ md5_digest="a" * 32,
+ meta={
+ "title": "Collect",
+ "sudo": SnippetSudoOption.NEVER.value,
+ "parameters": [{"name": name, "type": "str"}],
+ },
+ )
+
+
+class TestInvalidFrontmatterBlocksExecution:
+ """Cover the execution block for a script carrying invalid frontmatter."""
+
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_parameter_name_blocks_the_shared_meta_builder(
+ self, reserved_name: str
+ ) -> None:
+ """Refuse to build execution meta for a script with a reserved parameter name.
+
+ Dropping the parameter keeps the form renderable, but the script is still
+ misconfigured -- the operator would silently run it without the argument
+ its author declared. Both Dipper execution flows assemble their meta here,
+ so guarding this one seam blocks both.
+
+ The parser's own messages have to reach the refusal: this app does not
+ surface them on the form, so a detail naming only the filename would leave
+ the cause invisible everywhere in the UI.
+ """
+ script = _script_with_parameter(reserved_name)
+ assert script.execution_interpreter is not None
+ assert script.can_execute is False
+
+ with pytest.raises(HTTPBadRequestException) as exc_info:
+ build_dipper_meta_from_args(
+ _make_service(), script, "src://test", _make_args()
+ )
+
+ assert script.filename in exc_info.value.detail
+ for error in script.validated_parameters.errors:
+ assert error in exc_info.value.detail
+
+ def test_execution_proceeds_when_invalid_parameters_are_ignored(
+ self, monkeypatch: pytest.MonkeyPatch
+ ) -> None:
+ """Honour the operator's opt-out rather than blocking unconditionally."""
+ monkeypatch.setattr(snippets_settings.META, "IGNORE_INVALID_PARAMETERS", True)
+
+ meta = build_dipper_meta_from_args(
+ _make_service(), _script_with_parameter("sudo"), "src://test", _make_args()
+ )
+
+ assert meta.target == "host1"
+
+
def _make_service_with_node(
*,
service_name: str = "svc",
diff --git a/tests/app/sep/apps/dipper/test_schema.py b/tests/app/sep/apps/dipper/test_schema.py
new file mode 100644
index 000000000..cd6fc0c36
--- /dev/null
+++ b/tests/app/sep/apps/dipper/test_schema.py
@@ -0,0 +1,94 @@
+# Copyright (C) 2026 Percona LLC
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see .
+
+"""Test the per-script Dipper execution form schema."""
+
+from typing import Any
+
+import pytest
+
+from app.sep.apps.dipper.schema import build_dipper_form_schema
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ RESERVED_EXECUTION_FIELD_NAMES,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
+from app.sep.apps.framework.schema import BoolField, HostField, ScriptPreviewField
+from app.sep.snippets.config import SnippetSudoOption
+from app.sep.snippets.models.snippet import BaseSnippet
+from tests.app.sep.form_schema_utils import (
+ assert_only_synthesized_fields,
+ form_field_names,
+ form_field_types,
+)
+
+SERVICE_ID = 1
+COLLECTOR_TYPE = "environment"
+
+#: The execution fields this builder synthesizes, and the widget rendering each.
+_SYNTHESIZED_FIELD_TYPES = {
+ EXECUTOR_HOST_FIELD_NAME: HostField,
+ SUDO_FIELD_NAME: BoolField,
+ SCRIPT_PREVIEW_FIELD_NAME: ScriptPreviewField,
+}
+
+
+def _script(**meta: Any) -> BaseSnippet:
+ """Return a DB-free script carrying the given frontmatter meta.
+
+ :param meta: Frontmatter keys merged over the title and sudo defaults.
+ :return: A script whose meta is the merge, backed by no database row.
+ """
+ return BaseSnippet(
+ filename="collect.py",
+ size=1,
+ md5_digest="a" * 32,
+ meta={
+ "title": "Collect",
+ "sudo": SnippetSudoOption.OPTIONAL.value,
+ **meta,
+ },
+ )
+
+
+class TestReservedParameterNames:
+ """Cover frontmatter parameters colliding with synthesized execution fields."""
+
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_named_parameter_is_dropped_not_raised(
+ self, reserved_name: str
+ ) -> None:
+ """Drop a reserved-name parameter instead of failing the schema build.
+
+ Reservation is unconditional, so this also holds for ``extra_args``,
+ which Dipper never synthesizes.
+ """
+ script = _script(parameters=[{"name": reserved_name, "type": "str"}])
+
+ schema = build_dipper_form_schema(
+ script, service_id=SERVICE_ID, collector_type=COLLECTOR_TYPE
+ )
+
+ assert_only_synthesized_fields(schema, _SYNTHESIZED_FIELD_TYPES)
+
+ def test_every_synthesized_field_name_is_reserved(self) -> None:
+ """Keep every field this builder synthesizes covered by the reserved set."""
+ schema = build_dipper_form_schema(
+ _script(), service_id=SERVICE_ID, collector_type=COLLECTOR_TYPE
+ )
+
+ assert form_field_types(schema) == _SYNTHESIZED_FIELD_TYPES
+ assert set(form_field_names(schema)) <= RESERVED_EXECUTION_FIELD_NAMES
diff --git a/tests/app/sep/apps/shared/test_disk_script_source.py b/tests/app/sep/apps/shared/test_disk_script_source.py
index f0e94ece9..6f008d880 100644
--- a/tests/app/sep/apps/shared/test_disk_script_source.py
+++ b/tests/app/sep/apps/shared/test_disk_script_source.py
@@ -35,6 +35,11 @@
HTTPUnprocessableEntityException,
)
from app.core.pagination import Pagination
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ RESERVED_EXECUTION_FIELD_NAMES,
+ SUDO_FIELD_NAME,
+)
from app.sep.apps.framework.list_query import InMemoryListQuery
from app.sep.apps.framework.schema import (
BoolField,
@@ -50,9 +55,21 @@
)
from app.sep.snippets.config import snippets_settings
from app.sep.snippets.models.snippet import BaseSnippet, SnippetExecutionMeta
+from tests.app.sep.form_schema_utils import (
+ assert_only_synthesized_fields,
+ form_field_names,
+ form_field_types,
+ form_fields_by_name,
+)
pytestmark = pytest.mark.asyncio
+#: The fields this app synthesizes for a sudo-optional script, and their widgets.
+_SYNTHESIZED_FIELD_TYPES = {
+ EXECUTOR_HOST_FIELD_NAME: HostField,
+ SUDO_FIELD_NAME: BoolField,
+}
+
_SHELL_NO_PARAMS = """#!/usr/bin/env bash
# ---
@@ -163,9 +180,23 @@ def _framework_processed_body(
return body.model_copy(update={"args": validated.model_dump()})
-def _fields(schema: object) -> dict[str, object]:
- """Return every form field across a schema's sections, keyed by field name."""
- return {field.name: field for section in schema.forms for field in section.fields}
+def _shell_with_parameter(name: str) -> str:
+ """Return an optional-sudo shell script declaring exactly one parameter.
+
+ :param name: The frontmatter parameter name the script declares.
+ :return: The script body, frontmatter included.
+ """
+ return (
+ "#!/usr/bin/env bash\n"
+ "# ---\n"
+ "# title: Reserved\n"
+ "# sudo: optional\n"
+ "# parameters:\n"
+ f"# - name: {name}\n"
+ "# label: Reserved\n"
+ "# ---\n"
+ 'echo "run"\n'
+ )
@pytest.fixture
@@ -383,7 +414,7 @@ async def test_typed_parameters_reuse_field_for_field_types(
"""Map choice/int/bool parameters onto their framework field counterparts."""
_write_script(script_dir, "typed.sh", _SHELL_TYPED_PARAMS)
script = await source.load_script("typed.sh")
- fields = _fields(source.build_form_schema(script))
+ fields = form_fields_by_name(source.build_form_schema(script))
assert isinstance(fields["mode"], ChoiceField)
assert isinstance(fields["retries"], IntegerField)
assert isinstance(fields["verbose"], BoolField)
@@ -394,7 +425,7 @@ async def test_execution_section_carries_host_and_optional_sudo(
"""Render the executor-host field plus a sudo toggle for an optional-sudo script."""
_write_script(script_dir, "sudo.sh", _SHELL_OPTIONAL_SUDO)
script = await source.load_script("sudo.sh")
- fields = _fields(source.build_form_schema(script))
+ fields = form_fields_by_name(source.build_form_schema(script))
assert isinstance(fields["executor_host"], HostField)
assert isinstance(fields["sudo"], BoolField)
@@ -404,7 +435,7 @@ async def test_script_without_parameters_has_only_execution_fields(
"""Render only the Execution section when the script declares no parameters."""
_write_script(script_dir, "bare.sh", _SHELL_NO_PARAMS)
script = await source.load_script("bare.sh")
- fields = _fields(source.build_form_schema(script))
+ fields = form_fields_by_name(source.build_form_schema(script))
assert set(fields) == {"executor_host"}
assert isinstance(fields["executor_host"], HostField)
@@ -414,9 +445,39 @@ async def test_string_parameter_maps_to_string_field(
"""Map a plain string parameter onto a ``StringField``."""
_write_script(script_dir, "msg.sh", _SHELL_MESSAGE_PARAM)
script = await source.load_script("msg.sh")
- fields = _fields(source.build_form_schema(script))
+ fields = form_fields_by_name(source.build_form_schema(script))
assert isinstance(fields["message"], StringField)
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ async def test_reserved_named_parameter_is_dropped_not_raised(
+ self, source: ScriptSource, script_dir: Path, reserved_name: str
+ ) -> None:
+ """Drop a reserved-name parameter instead of failing the schema build.
+
+ Reservation is unconditional, so this holds for the two names this app
+ never synthesizes (``script_preview``, ``extra_args``) as well as for
+ the two it does: an author cannot tell from their frontmatter which app
+ will render it.
+ """
+ _write_script(script_dir, "reserved.sh", _shell_with_parameter(reserved_name))
+ script = await source.load_script("reserved.sh")
+
+ schema = source.build_form_schema(script)
+
+ assert_only_synthesized_fields(schema, _SYNTHESIZED_FIELD_TYPES)
+
+ async def test_every_synthesized_field_name_is_reserved(
+ self, source: ScriptSource, script_dir: Path
+ ) -> None:
+ """Keep every field this builder synthesizes covered by the reserved set."""
+ _write_script(script_dir, "sudo.sh", _SHELL_OPTIONAL_SUDO)
+ script = await source.load_script("sudo.sh")
+
+ schema = source.build_form_schema(script)
+
+ assert form_field_types(schema) == _SYNTHESIZED_FIELD_TYPES
+ assert set(form_field_names(schema)) <= RESERVED_EXECUTION_FIELD_NAMES
+
class TestExecuteMeta:
"""Cover args validation, meta assembly, sudo, and gate enforcement."""
diff --git a/tests/app/sep/apps/test_field_names.py b/tests/app/sep/apps/test_field_names.py
new file mode 100644
index 000000000..6b19c1897
--- /dev/null
+++ b/tests/app/sep/apps/test_field_names.py
@@ -0,0 +1,87 @@
+# Copyright (C) 2026 Percona LLC
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see .
+
+"""Test the synthesized execution field-name vocabulary."""
+
+import ast
+from pathlib import Path
+
+from app.sep.apps import field_names
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ EXTRA_ARGS_FIELD_NAME,
+ RESERVED_EXECUTION_FIELD_NAMES,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
+
+
+class TestWireNames:
+ """Test the spelling of each synthesized execution field name."""
+
+ def test_each_constant_spells_its_wire_name(self) -> None:
+ """Pin the wire spelling of every synthesized execution field."""
+ assert EXECUTOR_HOST_FIELD_NAME == "executor_host"
+ assert SUDO_FIELD_NAME == "sudo"
+ assert SCRIPT_PREVIEW_FIELD_NAME == "script_preview"
+ assert EXTRA_ARGS_FIELD_NAME == "extra_args"
+
+
+class TestReservedExecutionFieldNames:
+ """Test the set reserved against frontmatter parameter names."""
+
+ def test_reserves_every_field_name_the_module_declares(self) -> None:
+ """Reserve every wire name declared here, and nothing else.
+
+ The expectation is read off the module's own constants rather than
+ retyped, so a fifth constant added without being added to the frozenset
+ fails here -- which is the whole point of a single definition site. A
+ retyped expectation would stay green in exactly that case; the wire
+ spellings are pinned once, in :class:`TestWireNames`.
+ """
+ declared = {
+ value
+ for name, value in vars(field_names).items()
+ if name.endswith("_FIELD_NAME")
+ }
+
+ assert declared == RESERVED_EXECUTION_FIELD_NAMES
+
+ def test_is_immutable(self) -> None:
+ """Keep the reserved set immutable so no consumer can widen it in place."""
+ assert isinstance(RESERVED_EXECUTION_FIELD_NAMES, frozenset)
+
+
+class TestLeafModule:
+ """Test that the vocabulary module stays importable from both directions."""
+
+ def test_declares_no_runtime_imports(self) -> None:
+ """Keep the module import-free so it can never close an import cycle.
+
+ ``app.sep.apps.framework`` and ``app.sep.snippets.models`` both depend
+ on these names, and ``framework.script_helpers`` already imports
+ ``snippets.models.snippet``. An import added here would close that loop,
+ so the absence of imports is the module's whole contract. ``__future__``
+ is exempt: it binds no module and so cannot cycle.
+ """
+ source = Path(field_names.__file__).read_text()
+ imported = [
+ alias.name
+ for node in ast.walk(ast.parse(source))
+ if isinstance(node, ast.Import | ast.ImportFrom)
+ for alias in node.names
+ if getattr(node, "module", None) != "__future__"
+ ]
+ assert imported == []
diff --git a/tests/app/sep/form_schema_utils.py b/tests/app/sep/form_schema_utils.py
new file mode 100644
index 000000000..a3c2a6b76
--- /dev/null
+++ b/tests/app/sep/form_schema_utils.py
@@ -0,0 +1,83 @@
+# Copyright (C) 2026 Percona LLC
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program. If not, see .
+
+"""Traverse and assert against the form sections of a built :class:`AppSchema`.
+
+Shared by the per-script schema suites -- snippets, Dipper and the disk-script
+source -- which each assert the same two things about their builder's output:
+which field names it emits, and which field type renders each of them.
+"""
+
+from app.sep.apps.framework.schema import AnyField, AppSchema
+
+
+def form_field_names(schema: AppSchema) -> list[str]:
+ """Return every form field name across a schema's sections, sorted.
+
+ A list rather than a set, so a duplicate wire name -- the failure the
+ reserved-name tests exist to catch -- stays visible instead of collapsing
+ into a single entry.
+
+ :param schema: The built schema whose form sections are traversed.
+ :return: Every field name, repeats included, in sorted order.
+ """
+ return sorted(field.name for section in schema.forms for field in section.fields)
+
+
+def form_fields_by_name(schema: AppSchema) -> dict[str, AnyField]:
+ """Return every form field across a schema's sections, keyed by field name.
+
+ Keying by name cannot mask a duplicate: ``AppSchema`` runs
+ ``_validate_unique_field_names_in_forms`` in a ``mode="after"`` validator, so a
+ builder emitting one wire name twice raises before any schema object exists for
+ this function to receive. The repeat surfaces out of the builder call itself.
+
+ :param schema: The built schema whose form sections are traversed.
+ :return: Each field keyed by its wire name.
+ """
+ return {field.name: field for section in schema.forms for field in section.fields}
+
+
+def form_field_types(schema: AppSchema) -> dict[str, type]:
+ """Map every form field name to the field class that renders it.
+
+ Pairs with :func:`form_field_names`: that one pins which names a builder
+ emits, this one pins that each name is still rendered by the widget its
+ users expect, so a field silently retyped fails as loudly as one dropped.
+
+ :param schema: The built schema whose form sections are traversed.
+ :return: Each field's class keyed by its wire name.
+ """
+ return {name: type(field) for name, field in form_fields_by_name(schema).items()}
+
+
+def assert_only_synthesized_fields(
+ schema: AppSchema, expected: dict[str, type]
+) -> None:
+ """Assert a schema carries the synthesized execution fields and nothing else.
+
+ Shared by the reserved-name suites, where the author's colliding parameter has
+ to be dropped. Asserting the whole field set, rather than only that the
+ reserved name appears at most once, keeps a build that dropped the
+ *synthesized* field as well from passing; asserting each field's type keeps one
+ silently retyped -- the author's ``str`` parameter shadowing the synthesized
+ widget -- from passing either.
+
+ :param schema: The built schema whose form sections are traversed.
+ :param expected: Each synthesized field's class keyed by its wire name.
+ """
+ assert not any(section.title == "Parameters" for section in schema.forms)
+ assert form_field_names(schema) == sorted(expected)
+ assert form_field_types(schema) == expected
diff --git a/tests/app/sep/snippets/models/test_meta.py b/tests/app/sep/snippets/models/test_meta.py
index 51013a7d2..8e459bfb6 100644
--- a/tests/app/sep/snippets/models/test_meta.py
+++ b/tests/app/sep/snippets/models/test_meta.py
@@ -22,7 +22,9 @@
from app.core.utils.fields import UTCDatetime
from app.core.utils.pydantic import CustomFieldMetadata
-from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME
+from app.sep.apps.field_names import (
+ RESERVED_EXECUTION_FIELD_NAMES,
+)
from app.sep.snippets.models.meta import (
serialize_cli_value,
SnippetMetaParameter,
@@ -84,19 +86,65 @@ def test_datetime_type_has_no_step(self):
class TestReservedParameterName:
"""Test rejection of parameter names reserved for synthesized fields."""
- def test_extra_args_name_raises(self):
- """Raise when a parameter is named after the synthesized Extra Args field."""
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_name_raises(self, reserved_name: str) -> None:
+ """Raise when a parameter is named after a synthesized execution field."""
+ with pytest.raises(ValidationError) as exc_info:
+ SnippetMetaParameter(name=reserved_name, type=SnippetMetaParameterType.STR)
+ message = str(exc_info.value)
+ assert "reserved" in message
+ assert reserved_name in message
+
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_name_is_rejected_when_hidden(self, reserved_name: str) -> None:
+ """Raise for a reserved name even when the parameter renders in no form.
+
+ ``hidden`` suppresses form rendering only. The parameter still reaches
+ the generated execution model, where its validation alias is its
+ declared name, so a hidden reserved name would collide there instead.
+ """
with pytest.raises(ValidationError, match="reserved"):
SnippetMetaParameter(
- name=EXTRA_ARGS_FIELD_NAME, type=SnippetMetaParameterType.STR
+ name=reserved_name, type=SnippetMetaParameterType.STR, hidden=True
)
- def test_other_names_are_unaffected(self):
- """Verify an unreserved name is still accepted."""
- param = SnippetMetaParameter(
- name="extra_args_suffix", type=SnippetMetaParameterType.STR
- )
- assert param.name == "extra_args_suffix"
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_name_is_rejected_when_positional(
+ self, reserved_name: str
+ ) -> None:
+ """Raise for a reserved name even when the parameter is positional."""
+ with pytest.raises(ValidationError, match="reserved"):
+ SnippetMetaParameter(
+ name=reserved_name, type=SnippetMetaParameterType.STR, positional=True
+ )
+
+ @pytest.mark.parametrize(
+ "name",
+ [
+ "extra_args_suffix",
+ "sudo_mode",
+ "executor_hosts",
+ "script_preview_url",
+ "preview",
+ "host",
+ ],
+ )
+ def test_near_miss_names_are_unaffected(self, name: str) -> None:
+ """Accept a name that merely resembles a reserved one."""
+ param = SnippetMetaParameter(name=name, type=SnippetMetaParameterType.STR)
+ assert param.name == name
+
+ @pytest.mark.parametrize("name", ["Sudo", "SUDO", "Extra_Args"])
+ def test_reservation_is_case_sensitive(self, name: str) -> None:
+ """Accept a re-cased reserved name, which cannot collide.
+
+ Duplicate form field names are detected by exact comparison, and every
+ downstream binding -- HTML input names, generated model field keys,
+ validation aliases -- is case-sensitive too, so ``Sudo`` is provably
+ distinct from the synthesized ``sudo`` field.
+ """
+ param = SnippetMetaParameter(name=name, type=SnippetMetaParameterType.STR)
+ assert param.name == name
class TestDatetimeTypeResolution:
diff --git a/tests/app/sep/snippets/models/test_snippet.py b/tests/app/sep/snippets/models/test_snippet.py
index 138c6d59a..9e2ff6eea 100644
--- a/tests/app/sep/snippets/models/test_snippet.py
+++ b/tests/app/sep/snippets/models/test_snippet.py
@@ -20,6 +20,7 @@
import pytest
+from app.sep.apps.field_names import RESERVED_EXECUTION_FIELD_NAMES
from app.sep.snippets.config import (
DEFAULT_SNIPPETS_TASK,
snippets_settings,
@@ -34,6 +35,7 @@
)
EXPECTED_PARAM_COUNT = 2
+EXPECTED_RESERVED_ERROR_COUNT = 2
UPDATED_SIZE = 200
MD5_DIGEST_LENGTH = 32
@@ -526,26 +528,90 @@ def test_with_invalid_parameters(self):
result = snippet.validated_parameters
assert len(result.errors) > 0
- def test_extra_args_named_parameter_is_rejected(self):
- """Drop a parameter reserved for the synthesized Extra Args field.
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_named_parameter_is_rejected(self, reserved_name: str) -> None:
+ """Drop a parameter reserved for a synthesized execution field.
Regression test for a wire-name collision: an ordinary parameter
- named ``extra_args`` would otherwise share its wire name with the
- synthesized Extra Args execution field, causing
- ``build_snippet_schema`` to raise a duplicate-field error, or a
- submitted value to silently double-bind in the execution model. It
- is now rejected like any other invalid parameter, before either path
- is reached.
+ sharing its wire name with a synthesized execution field would
+ otherwise make the schema builders raise a duplicate-field error, or a
+ submitted value silently double-bind in the execution model. It is now
+ rejected like any other invalid parameter, before either path is
+ reached.
"""
snippet = BaseSnippet(
filename="test.sh",
size=100,
md5_digest="a" * 32,
- meta={"parameters": [{"name": "extra_args", "type": "str"}]},
+ meta={"parameters": [{"name": reserved_name, "type": "str"}]},
)
result = snippet.validated_parameters
assert len(result.parameters) == 0
- assert any("reserved" in error for error in result.errors)
+ assert any(
+ "reserved" in error and reserved_name in error for error in result.errors
+ )
+
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ def test_reserved_named_parameter_blocks_execution(
+ self, reserved_name: str
+ ) -> None:
+ """Block execution of a snippet declaring a reserved parameter name."""
+ snippet = BaseSnippet(
+ filename="test.sh",
+ size=100,
+ md5_digest="a" * 32,
+ meta={"parameters": [{"name": reserved_name, "type": "str"}]},
+ )
+ assert snippet.can_execute is False
+
+ def test_every_reserved_named_parameter_is_reported(self) -> None:
+ """Report one error per reserved name and keep the valid siblings."""
+ snippet = BaseSnippet(
+ filename="test.sh",
+ size=100,
+ md5_digest="a" * 32,
+ meta={
+ "parameters": [
+ {"name": "sudo", "type": "str"},
+ {"name": "mode", "type": "str"},
+ {"name": "extra_args", "type": "str"},
+ ]
+ },
+ )
+ result = snippet.validated_parameters
+ assert [param.name for param in result.parameters] == ["mode"]
+ assert len(result.errors) == EXPECTED_RESERVED_ERROR_COUNT
+ assert any("'sudo'" in error for error in result.errors)
+ assert any("'extra_args'" in error for error in result.errors)
+
+ def test_reserved_named_parameter_is_not_reported_as_unknown_reference(
+ self,
+ ) -> None:
+ """Keep a sibling's reference to a rejected parameter unflagged.
+
+ Visibility references are checked against the names the frontmatter
+ *declared*, not the ones that survived validation, so a rejected
+ parameter is reported once as reserved rather than twice -- once as
+ reserved and again as an unknown reference.
+ """
+ snippet = BaseSnippet(
+ filename="test.sh",
+ size=100,
+ md5_digest="a" * 32,
+ meta={
+ "parameters": [
+ {"name": "sudo", "type": "str"},
+ {
+ "name": "mode",
+ "type": "str",
+ "visible_when": {"parameter": "sudo", "equals": "1"},
+ },
+ ]
+ },
+ )
+ errors = snippet.validated_parameters.errors
+ assert len(errors) == 1
+ assert "reserved" in errors[0]
def test_no_parameters(self):
"""Verify empty parameters produce no errors."""
diff --git a/tests/app/sep/snippets/test_schema.py b/tests/app/sep/snippets/test_schema.py
index 4e2a0f536..0b330e5d2 100644
--- a/tests/app/sep/snippets/test_schema.py
+++ b/tests/app/sep/snippets/test_schema.py
@@ -15,11 +15,21 @@
"""Tests for the snippets plugin schema synthesiser."""
+from collections.abc import Awaitable, Callable
+from functools import cached_property
+from typing import Any
from urllib.parse import urlencode
import pytest
from app.sep.apps.dipper.models import DipperScript
+from app.sep.apps.field_names import (
+ EXECUTOR_HOST_FIELD_NAME,
+ EXTRA_ARGS_FIELD_NAME,
+ RESERVED_EXECUTION_FIELD_NAMES,
+ SCRIPT_PREVIEW_FIELD_NAME,
+ SUDO_FIELD_NAME,
+)
from app.sep.apps.framework.schema import (
BoolField,
ChoiceField,
@@ -29,6 +39,7 @@
ScriptPreviewField,
StringField,
)
+from app.sep.snippets.config import snippets_settings, SnippetSudoOption
from app.sep.snippets.models.meta import (
SnippetMetaParameter,
SnippetMetaParameterType,
@@ -43,6 +54,11 @@
field_for,
SNIPPETS_PLUGIN_SCHEMA,
)
+from tests.app.sep.form_schema_utils import (
+ assert_only_synthesized_fields,
+ form_field_names,
+ form_field_types,
+)
def test_static_schema_has_no_forms_and_keyed_columns():
@@ -190,37 +206,116 @@ async def test_per_snippet_schema_omits_extra_args_field_by_default(create_snipp
assert not any(field.name == "extra_args" for field in all_fields)
-@pytest.mark.asyncio
-async def test_per_snippet_schema_drops_extra_args_named_parameter(create_snippet):
- """Drop a frontmatter parameter reserved for the synthesized Extra Args field.
-
- Regression test: previously this reached ``build_snippet_schema`` and
- raised an opaque ``duplicate field name(s)`` error when
- ``allow_extra_args`` was set, or silently double-bound submitted values
- in the execution model when it was not. The reserved-name parameter is
- now caught earlier, at parameter parse time (see ``validated_parameters``
- on the snippet), so it never reaches either path -- ``build_snippet_schema``
- just omits it like any other invalid parameter.
+#: Every execution field this builder can synthesize, and the widget rendering it.
+#: Snippets is the maximal builder, so these are the whole reserved set.
+_SYNTHESIZED_FIELD_TYPES = {
+ EXECUTOR_HOST_FIELD_NAME: HostField,
+ SUDO_FIELD_NAME: BoolField,
+ SCRIPT_PREVIEW_FIELD_NAME: ScriptPreviewField,
+ EXTRA_ARGS_FIELD_NAME: StringField,
+}
+
+
+def _reset_cached_meta(snippet: Snippet, **meta: Any) -> None:
+ """Re-apply snippet meta and drop every cached property derived from it.
+
+ Every ``cached_property`` on the class is dropped rather than a named few,
+ so a property added later cannot leave a test asserting against a stale
+ value computed from the pre-mutation meta.
+
+ :param snippet: The already-created snippet whose meta is being replaced.
+ :param meta: Frontmatter keys merged over the snippet's existing meta.
"""
- snippet = await create_snippet("hello.sh", approved=True)
- snippet.__dict__.pop("validated_parameters", None)
- snippet.meta = {
- **snippet.meta,
- "allow_extra_args": True,
- "parameters": [{"name": "extra_args", "type": "str"}],
- }
- snippet.__dict__.pop("validated_parameters", None)
- snippet.__dict__.pop("allow_extra_args", None)
+ snippet.meta = {**snippet.meta, **meta}
+ for klass in type(snippet).__mro__:
+ for name, attribute in vars(klass).items():
+ if isinstance(attribute, cached_property):
+ snippet.__dict__.pop(name, None)
+
+
+class TestReservedParameterNames:
+ """Cover frontmatter parameters colliding with synthesized execution fields."""
+
+ @pytest.mark.parametrize("reserved_name", sorted(RESERVED_EXECUTION_FIELD_NAMES))
+ @pytest.mark.asyncio
+ async def test_per_snippet_schema_drops_reserved_named_parameter(
+ self,
+ create_snippet: Callable[..., Awaitable[Snippet]],
+ reserved_name: str,
+ ) -> None:
+ """Drop a frontmatter parameter reserved for a synthesized execution field.
+
+ Regression test: previously this reached ``build_snippet_schema`` and
+ raised an opaque ``duplicate field name(s)`` error whenever the snippet
+ configuration made the builder synthesize the colliding field, or silently
+ double-bound submitted values in the execution model when it did not. The
+ reserved-name parameter is now caught earlier, at parameter parse time (see
+ ``validated_parameters`` on the snippet), so it never reaches either path --
+ ``build_snippet_schema`` just omits it like any other invalid parameter.
+
+ Both ``allow_extra_args`` and ``sudo`` are enabled so that every one of the
+ four reserved names is actually synthesized, making the collision reachable.
+ """
+ snippet = await create_snippet("hello.sh", approved=True)
+ _reset_cached_meta(
+ snippet,
+ allow_extra_args=True,
+ sudo=SnippetSudoOption.OPTIONAL.value,
+ parameters=[{"name": reserved_name, "type": "str"}],
+ )
- assert any("reserved" in error for error in snippet.validated_parameters.errors)
+ assert any("reserved" in error for error in snippet.validated_parameters.errors)
- schema = build_snippet_schema(snippet)
+ schema = build_snippet_schema(snippet)
- assert not any(s.title == "Parameters" for s in schema.forms)
- section = _execution_section(schema)
- extra_args_fields = [f for f in section.fields if f.name == "extra_args"]
- assert len(extra_args_fields) == 1
- assert isinstance(extra_args_fields[0], StringField)
+ assert_only_synthesized_fields(schema, _SYNTHESIZED_FIELD_TYPES)
+
+ @pytest.mark.asyncio
+ async def test_per_snippet_schema_builds_when_invalid_parameters_are_ignored(
+ self,
+ create_snippet: Callable[..., Awaitable[Snippet]],
+ monkeypatch: pytest.MonkeyPatch,
+ ) -> None:
+ """Build a collision-free form even when invalid parameters are ignored.
+
+ ``IGNORE_INVALID_PARAMETERS`` relaxes ``can_execute`` only; the reserved
+ parameter is still dropped at parse time, so the form the operator opted
+ into still carries exactly one ``executor_host`` field -- still the
+ synthesized host widget, not the author's string -- rather than the
+ duplicate-field error this setting would otherwise expose them to.
+ """
+ monkeypatch.setattr(snippets_settings.META, "IGNORE_INVALID_PARAMETERS", True)
+ snippet = await create_snippet("hello.sh", approved=True)
+ _reset_cached_meta(
+ snippet, parameters=[{"name": "executor_host", "type": "str"}]
+ )
+
+ assert snippet.can_execute is True
+
+ schema = build_snippet_schema(snippet)
+
+ assert form_field_names(schema).count(EXECUTOR_HOST_FIELD_NAME) == 1
+ assert form_field_types(schema)[EXECUTOR_HOST_FIELD_NAME] is HostField
+
+ @pytest.mark.asyncio
+ async def test_every_synthesized_field_name_is_reserved(
+ self, create_snippet: Callable[..., Awaitable[Snippet]]
+ ) -> None:
+ """Keep every field this builder synthesizes covered by the reserved set.
+
+ A parameterless snippet yields a schema whose fields are all synthesized,
+ so a synthesized field added later without being reserved fails here rather
+ than only when an author happens to pick its name.
+ """
+ snippet = await create_snippet("hello.sh", approved=True)
+ _reset_cached_meta(
+ snippet, allow_extra_args=True, sudo=SnippetSudoOption.OPTIONAL.value
+ )
+
+ schema = build_snippet_schema(snippet)
+
+ assert set(form_field_names(schema)) == RESERVED_EXECUTION_FIELD_NAMES
+ assert form_field_types(schema) == _SYNTHESIZED_FIELD_TYPES
@pytest.mark.asyncio