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
16 changes: 16 additions & 0 deletions plugins/kbagent/skills/kbagent/references/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,22 @@ it, and the next deploy re-created it (field report: 18 phantom configs hiding
statements into one. Run `sync pull` for that project, then push again.
Genuine SQL edits are never blocked by this guard.

## `sync push` runs the same script-shape guard as `config update` (since vNEXT)

`sync push` now runs `normalize_blocks_codes_script` -- the runtime-safety guard
`config update` and `transformation edit/create` have always run -- on every
config body it sends (create, update, and the Phase C variables backfill). After
the #686 fix above it is a no-op on the GitOps path by construction; it still
catches a hand-authored `_config.yml` that carries `parameters.blocks` inline
with NO companion `transform.sql`, which code merging passes through verbatim
(a `script` string, or one element packing several `;`-separated statements,
passes the Storage API and fails the JOB).

Each fix is surfaced -- never silent -- as a push-envelope `warnings[]` entry with
`change_type: "script_normalization"` carrying `path` / `action` / `after_length`
(printed in human mode like any other push warning, structured under `warnings`
in `--json`). `config update` keeps its own dedicated `normalizations` key.

## `sync push` fresh-CREATE writeback now updates placeholders in place (since v0.47.0)

Before v0.47.0, `kbagent sync push` always **appended** new `ManifestConfiguration`
Expand Down
7 changes: 7 additions & 0 deletions plugins/kbagent/skills/kbagent/references/sync-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,13 @@ Stored in `.keboola/branch-mapping.json`:
read-back), never from the files on disk. Before vNEXT the two producers
disagreed and every pushed multi-statement SQL transformation showed
permanent phantom `REMOTE MODIFIED` drift
- **Push runs the same runtime-safety script normalization as `config update`
(since vNEXT)**: `parameters.blocks[].codes[].script` is normalized to the
runtime's shape (one element = one executable statement) before every write.
A no-op for a normally pulled tree; it catches a hand-authored `_config.yml`
with inline `parameters.blocks` and no `transform.sql`. Any fix is reported
in the push envelope's `warnings[]` as `change_type: "script_normalization"`
(human mode prints it; `--json` carries `path` / `action` / `after_length`)
- **Encrypted values**: nonce differences are ignored in diff (no false positives)
- **New configs**: push auto-assigns IDs from the API, updates manifest
- **Storage metadata is read-only**: not tracked in manifest, excluded from diff/push
Expand Down
13 changes: 10 additions & 3 deletions src/keboola_agent_cli/services/_sync_bindings.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
FlowBindingResult,
VariableBindingResult,
)
from ._sync_push_ops import guard_script_shape

if TYPE_CHECKING:
from .sync_service import SyncService
Expand Down Expand Up @@ -257,7 +258,7 @@ def _apply_variable_binding(
row_ulid: str | None,
manifest: Manifest,
branch_id: int | None,
warnings: list[dict[str, str]],
warnings: list[dict[str, Any]],
) -> None:
"""PUT the resolved variables link, rewrite local, refresh manifest hashes.

Expand All @@ -269,6 +270,12 @@ def _apply_variable_binding(
merged = copy.deepcopy(local_data)
merge_code_files(created.component_id, merged, created.config_dir)
_name, _description, configuration = local_config_to_api(merged)
# This backfill PUTs the WHOLE configuration again, so it is the LAST write
# a freshly-created transformation receives -- an unguarded body here would
# undo the normalization ``push_create`` just applied.
configuration = guard_script_shape(
created.component_id, configuration, warnings, config_id=created.config_id
)
configuration["variables_id"] = parent_ulid
if row_ulid:
configuration["variables_values_id"] = row_ulid
Expand Down Expand Up @@ -317,7 +324,7 @@ def _refresh_binding_hashes(
manifest: Manifest,
branch_id: int | None,
response: Any,
warnings: list[dict[str, str]],
warnings: list[dict[str, Any]],
) -> None:
"""Re-stamp a rebound config's manifest bookkeeping after the backfill PUT.

Expand Down Expand Up @@ -457,7 +464,7 @@ def _apply_flow_task_binding(
local_data: dict[str, Any],
manifest: Manifest,
branch_id: int | None,
warnings: list[dict[str, str]],
warnings: list[dict[str, Any]],
) -> None:
"""PUT a remapped flow, rewrite local ``_config.yml``, refresh hashes.

Expand Down
5 changes: 3 additions & 2 deletions src/keboola_agent_cli/services/_sync_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

from dataclasses import dataclass, field
from pathlib import Path
from typing import Any

from ..sync.manifest import ManifestConfiguration

Expand Down Expand Up @@ -65,7 +66,7 @@ class VariableBindingResult:
"""

errors: list[dict[str, str]] = field(default_factory=list)
warnings: list[dict[str, str]] = field(default_factory=list)
warnings: list[dict[str, Any]] = field(default_factory=list)
configs_rewritten: int = 0


Expand All @@ -81,7 +82,7 @@ class FlowBindingResult:
"""

errors: list[dict[str, str]] = field(default_factory=list)
warnings: list[dict[str, str]] = field(default_factory=list)
warnings: list[dict[str, Any]] = field(default_factory=list)
configs_rewritten: int = 0
tasks_remapped: int = 0

Expand Down
105 changes: 99 additions & 6 deletions src/keboola_agent_cli/services/_sync_push_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from ..constants import CONFIG_FILENAME
from ..errors import ErrorCode, KeboolaApiError
from ..sync.code_extraction import merge_code_files
from ..sync.code_extraction import merge_code_files, normalize_blocks_codes_script
from ..sync.config_format import local_config_to_api, local_row_to_api
from ..sync.manifest import Manifest, ManifestConfiguration
from ._encryption import encrypt_secrets_in_config
Expand All @@ -31,6 +31,81 @@
logger = logging.getLogger(__name__)


def guard_script_shape(
component_id: str,
configuration: dict[str, Any],
warnings: list[dict[str, Any]] | None,
*,
config_id: str = "",
config_path: str = "",
) -> dict[str, Any]:
"""Run the runtime-safety ``script[]`` guard on an outgoing push body.

``sync push`` is the one deploy route that used to reach the Storage API
without :func:`normalize_blocks_codes_script` -- ``config update`` and
``transformation edit/create`` have run it since 0.28.0 / 0.30.8. The
Storage API accepts a ``script`` string, or a list element packing several
``;``-separated statements; the Keboola runtime then fails the job
("Expected array, got string" / ``MULTI_STATEMENT_COUNT``, issues
#245/#274).

After issue #686 parts 2+3 this is a no-op on the GitOps path *by
construction*: ``merge_code_files`` rebuilds ``parameters.blocks`` from
``transform.sql`` through the single canonical producer
(``canonical_sql_script``). It is wired in as a REGRESSION BACKSTOP -- and
it still covers the shape that bypasses code extraction entirely: a
hand-authored ``_config.yml`` carrying ``parameters.blocks`` inline with no
companion code file, which ``merge_code_files`` passes through verbatim.

The semantics of the guard are untouched; only its records are re-shaped
into the push envelope's ``warnings[]`` entries (``change_type``
``script_normalization``) so both ``--json`` and human mode surface them
through the channel every other non-fatal push warning already uses.
``config update`` surfaces the same records under its own dedicated
``normalizations`` key -- a per-config envelope can afford one; a push
envelope spans many configs, so each record carries its own identity.
"""
configuration, records = normalize_blocks_codes_script(component_id, configuration)
if warnings is None:
return configuration
for record in records:
warnings.append(
_script_normalization_warning(
component_id=component_id,
config_id=config_id,
config_path=config_path,
record=record,
)
)
return configuration


def _script_normalization_warning(
*,
component_id: str,
config_id: str,
config_path: str,
record: dict[str, Any],
) -> dict[str, Any]:
"""Wrap one normalization record as a push-envelope warning."""
label = config_id or config_path or "(new config)"
message = (
f"Normalized {component_id}/{label} {record['path']} before the write "
f"({record['action']} -> {record['after_length']} element(s)): the local files held a "
f"script shape the Keboola runtime rejects. Run 'kbagent sync pull' to bring the "
f"local tree in line with what was sent."
)
logger.warning("%s", message)
return {
"change_type": "script_normalization",
"component_id": component_id,
"config_id": config_id,
"config_path": config_path,
"message": message,
**record,
}


def push_row_change(
service: SyncService,
client: Any,
Expand All @@ -44,7 +119,7 @@ def push_row_change(
manifest: Manifest,
branch_id: int | None,
allow_plaintext_fallback: bool = False,
warnings: list[dict[str, str]] | None = None,
warnings: list[dict[str, Any]] | None = None,
) -> str | None:
"""Dispatch a single row-level change (added/modified/deleted) to the API.

Expand Down Expand Up @@ -148,7 +223,7 @@ def _push_create_row(
branch_id: int | None,
project_id: int | None,
allow_plaintext_fallback: bool,
warnings: list[dict[str, str]] | None = None,
warnings: list[dict[str, Any]] | None = None,
) -> str:
"""POST a new row; record API-assigned id + hashes in the parent's row list.

Expand Down Expand Up @@ -220,7 +295,7 @@ def push_update_row(
branch_id: int | None,
project_id: int | None,
allow_plaintext_fallback: bool,
warnings: list[dict[str, str]] | None = None,
warnings: list[dict[str, Any]] | None = None,
) -> None:
"""PUT an existing row; refresh its hashes in the parent's row list.

Expand Down Expand Up @@ -309,8 +384,13 @@ def push_create(
branch_id: int | None,
*,
allow_plaintext_fallback: bool = False,
warnings: list[dict[str, Any]] | None = None,
) -> dict[str, Any] | None:
"""Create a new config from a local _config.yml file."""
"""Create a new config from a local _config.yml file.

``warnings`` accumulates the ``script[]`` normalization records of
:func:`guard_script_shape` for the push envelope.
"""
branch_path = service._resolve_source_branch_path(manifest, project_root, branch_id)
config_dir = project_root / branch_path / config_path_str
local_data = service._read_config_file(config_dir)
Expand All @@ -326,6 +406,11 @@ def push_create(

name, description, configuration = local_config_to_api(local_data)

# Runtime-safety backstop on the assembled API body (issues #245/#274).
configuration = guard_script_shape(
component_id, configuration, warnings, config_path=config_path_str
)

# Encrypt #-prefixed secrets before sending to API
project_id = manifest.project.id if manifest.project else None
configuration = encrypt_secrets_in_config(
Expand Down Expand Up @@ -365,11 +450,14 @@ def push_update(
branch_id: int | None,
*,
allow_plaintext_fallback: bool = False,
warnings: list[dict[str, Any]] | None = None,
) -> dict[str, Any]:
"""Update an existing config from a local _config.yml file.

Returns the API response so the caller can stamp the manifest baseline
from the remote's own view of the config (issue #686).
from the remote's own view of the config (issue #686). ``warnings``
accumulates the ``script[]`` normalization records of
:func:`guard_script_shape` for the push envelope.
"""
branch_path = service._resolve_source_branch_path(manifest, project_root, branch_id)
config_dir = project_root / branch_path / config_path_str
Expand All @@ -386,6 +474,11 @@ def push_update(

name, description, configuration = local_config_to_api(local_data)

# Runtime-safety backstop on the assembled API body (issues #245/#274).
configuration = guard_script_shape(
component_id, configuration, warnings, config_id=config_id, config_path=config_path_str
)

# Encrypt #-prefixed secrets before sending to API
project_id = manifest.project.id if manifest.project else None
configuration = encrypt_secrets_in_config(
Expand Down
10 changes: 7 additions & 3 deletions src/keboola_agent_cli/services/sync_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -1608,9 +1608,11 @@ def push(
updated = 0
deleted = 0
errors: list[dict[str, str]] = []
# Non-fatal push warnings: today only unstampable manifest baselines
# (issue #686), i.e. the API state could not be read back after a write.
warnings: list[dict[str, str]] = []
# Non-fatal push warnings: unstampable manifest baselines (issue #686,
# the API state could not be read back after a write) and ``script[]``
# runtime-safety normalizations (``change_type`` ``script_normalization``,
# whose records carry non-string values such as ``after_length``).
warnings: list[dict[str, Any]] = []
pushed_details: list[dict[str, str]] = []
manifest_dirty = False

Expand Down Expand Up @@ -1650,6 +1652,7 @@ def push(
manifest,
branch_id,
allow_plaintext_fallback=allow_plaintext_fallback,
warnings=warnings,
)
if result:
new_id = str(result.get("id", ""))
Expand Down Expand Up @@ -1718,6 +1721,7 @@ def push(
manifest,
branch_id,
allow_plaintext_fallback=allow_plaintext_fallback,
warnings=warnings,
)
# Update hashes so pull knows local == remote
if (config_dir / CONFIG_FILENAME).exists():
Expand Down
Loading
Loading