Skip to content
Merged
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
51 changes: 37 additions & 14 deletions app/sep/apps/atw/batch.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@
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

__all__ = [
"MAX_BATCH_SNIPPETS",
"NON_SHAREABLE_FIELD_NAMES",
"ATWBatchExecuteItemResponse",
"ATWBatchExecuteItemWrite",
"ATWBatchExecuteResponse",
Expand Down Expand Up @@ -97,6 +99,14 @@
_SYNTHETIC_FIELD_NAMES = frozenset(
{EXECUTOR_HOST_FIELD_NAME, SUDO_FIELD_NAME, SCRIPT_PREVIEW_FIELD_NAME}
)
NON_SHAREABLE_FIELD_NAMES = frozenset({EXTRA_ARGS_FIELD_NAME})
"""Keep these fields per-snippet even when several snippets declare them identically.

Unlike ``_SYNTHETIC_FIELD_NAMES`` (fields excluded entirely because a caller
re-synthesises them), these are ordinary per-snippet fields that must never be
promoted to the shared section: extra args are snippet-specific CLI flags, so
merging them would silently apply one snippet's flags to another's command.
"""


async def resolve_snippets(filenames: Sequence[str]) -> dict[str, SnippetScript]:
Expand Down Expand Up @@ -138,7 +148,9 @@ class ATWMergedSchemaResponse(BaseModel):
``AppSchema`` section carries only a display title.

:param shared: The batch-level execution fields followed by every parameter
the selection declares identically.
the selection declares identically, excluding ``NON_SHAREABLE_FIELD_NAMES``
(e.g. Extra Args), which stay per-snippet even when every item declares
them.
:param per_snippet: The remaining per-snippet fields, in request order.
"""

Expand All @@ -165,7 +177,9 @@ class ATWBatchExecuteWrite(BaseModel):
:param sudo: The sudo choice applied to every item; snippets whose sudo
option is not optional ignore it.
:param shared_args: Arguments offered to every item, filtered per snippet to
the parameters that snippet declares.
the parameters that snippet declares. ``NON_SHAREABLE_FIELD_NAMES`` (e.g.
Extra Args) are never applied from ``shared_args``, even for a snippet
that declares a field with that name.
:param items: The snippets to execute, at least one and at most
``MAX_BATCH_SNIPPETS``.
"""
Expand Down Expand Up @@ -248,13 +262,15 @@ class ATWIncidentExecutionResponse(BaseModel):
def parameter_fields(script: SnippetScript) -> list[AnyField]:
"""Return a snippet's parameter fields, without the synthetic execution ones.

The per-snippet schema appends an executor-host selector, a sudo toggle, and a
script-preview pane to the frontmatter parameters. Those are batch-level or
presentational, so a merged batch form owns them once (or not at all) rather
than repeating them per snippet.
The per-snippet schema appends an executor-host selector, a sudo toggle, a
script-preview pane, and — for a snippet that opts in — an Extra Args input to
the frontmatter parameters. The first three are batch-level or presentational,
so a merged batch form owns them once (or not at all) rather than repeating
them per snippet; Extra Args stays because it is per-snippet by nature.

:param script: The resolved snippet whose form schema is flattened.
:return: Every parameter field the snippet declares, in schema order.
:return: Every parameter field the snippet declares, in schema order, plus the
synthesized Extra Args field when the snippet opts into it.
"""
return [
field
Expand Down Expand Up @@ -316,14 +332,15 @@ def shared_field_names(declarations: dict[str, list[AnyField]]) -> set[str]:
every declaration serialises identically — the wire form is what the renderer
consumes, so byte-identity there is the sharing contract. Cosmetically similar
but differing declarations (a per-product default, a required-vs-optional
divergence) stay per-snippet, where they mean different things.
divergence) stay per-snippet, where they mean different things. A name in
``NON_SHAREABLE_FIELD_NAMES`` never merges, however unanimous its declarations.

:param declarations: Every declaration of each parameter name, keyed by name.
:return: The names whose declarations are unanimous across two or more snippets.
"""
shared = set()
for name, fields in declarations.items():
if len(fields) < _MIN_SHARED_DECLARERS:
if name in NON_SHAREABLE_FIELD_NAMES or len(fields) < _MIN_SHARED_DECLARERS:
continue
dumps = [field.model_dump(by_alias=True) for field in fields]
if all(dump == dumps[0] for dump in dumps[1:]):
Expand All @@ -339,10 +356,12 @@ async def dispatch_batch_item(
) -> ScriptExecutionResponse:
"""Narrow the shared args to one already-resolved batch item and dispatch it.

Shared arguments are filtered to the parameters the snippet actually declares,
so a batch may offer a value no single snippet accepts, and the item's own
``args`` then override what remains. The snippet is resolved once for the whole
batch by the caller and handed in, so a repeated filename costs one lookup.
Shared arguments are filtered to the parameters the snippet actually declares
and excludes ``NON_SHAREABLE_FIELD_NAMES``, so a batch may offer a value no
single snippet accepts (or one no snippet may share, like Extra Args), and the
item's own ``args`` then override what remains. The snippet is resolved once
for the whole batch by the caller and handed in, so a repeated filename costs
one lookup.

:param body: The batch payload supplying the executor host, sudo choice, and
shared arguments.
Expand All @@ -356,7 +375,11 @@ async def dispatch_batch_item(
:raises OSError: Propagated from ``execute_script`` when the Tasks API
transport itself fails.
"""
declared = {field.name for field in parameter_fields(script)}
declared = {
field.name
for field in parameter_fields(script)
if field.name not in NON_SHAREABLE_FIELD_NAMES
}
args = {name: value for name, value in body.shared_args.items() if name in declared}
args.update(item.args)
return await execute_script(
Expand Down
26 changes: 26 additions & 0 deletions app/sep/snippets/models/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# 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 <https://www.gnu.org/licenses/>.

"""Define wire-name constants shared across snippet and framework schema modules."""

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.
"""
18 changes: 18 additions & 0 deletions app/sep/snippets/models/meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
TextInputElement,
TextInputHTMLElement,
)
from app.sep.snippets.models.constants import EXTRA_ARGS_FIELD_NAME

ParameterType = str | int | float | bool | datetime | None

Expand Down Expand Up @@ -278,6 +279,7 @@ class SnippetMetaParameter(BaseModel):
name: NonEmptyStr = Field(
..., pattern=r"^\w(?:[\w-]*\w)?$", serialization_alias="title"
)

py_type: SnippetMetaParameterType = Field(
SnippetMetaParameterType.STR, validation_alias="type"
)
Expand Down Expand Up @@ -310,6 +312,22 @@ class SnippetMetaParameter(BaseModel):
hidden: bool = False
sensitive: bool = False

@field_validator("name")
@classmethod
def _reject_reserved_name(cls, value: str) -> str:
"""Reject a parameter name reserved for a synthesized execution field.

: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:
raise ValueError(
f"parameter name {value!r} is reserved for the synthesized "
"Extra Args field"
)
return value

@field_validator(
"visible_when",
"visible_when_not",
Expand Down
12 changes: 10 additions & 2 deletions app/sep/snippets/models/snippet.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
from aiofiles.ospath import getsize
from async_lru import alru_cache
from pydantic import (
AliasChoices,
BaseModel,
BeforeValidator,
computed_field,
Expand Down Expand Up @@ -83,6 +84,7 @@
SubmitButtonElement,
TextInputElement,
)
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,
Expand Down Expand Up @@ -334,7 +336,7 @@ class BaseSnippetArgs(BaseModel):
:type executor_host: NonEmptyStr
"""

extra_args_field: ClassVar[str] = "extra_args"
extra_args_field: ClassVar[str] = EXTRA_ARGS_FIELD_NAME
sudo_field: ClassVar[str] = "sudo"
executor_host: NonEmptyStr = Field(
validation_alias=EXECUTOR_HOSTS_INPUT_NAME, exclude=True
Expand Down Expand Up @@ -955,7 +957,13 @@ def _get_execution_model(
if add_extra_args_field:
fields[BaseSnippetArgs.extra_args_field] = (
ExtraArgsField,
Field(default_factory=list, alias=EXTRA_ARGS_INPUT_NAME),
Field(
default_factory=list,
validation_alias=AliasChoices(
EXTRA_ARGS_INPUT_NAME, EXTRA_ARGS_FIELD_NAME
),
serialization_alias=EXTRA_ARGS_FIELD_NAME,
),
Comment thread
marcuscruz-percona marked this conversation as resolved.
)

if add_sudo_field:
Expand Down
13 changes: 13 additions & 0 deletions app/sep/snippets/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
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,
Expand Down Expand Up @@ -375,6 +376,18 @@ def build_snippet_schema(snippet: BaseSnippet) -> AppSchema:
),
),
)
if snippet.allow_extra_args:
execution_fields.append(
cast(
AnyField,
StringField(
name=EXTRA_ARGS_FIELD_NAME,
label="Extra Args",
placeholder="e.g. --verbose",
description="Any extra args to pass to the snippet execution command",
),
),
)
forms.append(FormSection(title="Execution", fields=execution_fields))
forms.append(
FormSection(
Expand Down
2 changes: 2 additions & 0 deletions changelog.d/SEP-1664.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Schema-driven snippet forms now render the Extra Args field for snippets that opt into it, matching the legacy Jinja form.
Reject a frontmatter snippet parameter named "extra_args", which collided with the synthesized Extra Args execution field. A snippet declaring that parameter name now fails parameter validation and cannot be executed until it is renamed.
4 changes: 2 additions & 2 deletions frontend/packages/api/specs/sep.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions frontend/packages/api/src/generated/sep.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading