From 4b49eb5bff92603071cae871fe2e68285d874180 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 25 Aug 2026 14:38:05 +0200 Subject: [PATCH] feat(sync): run the runtime-safety script guard on the push path `normalize_blocks_codes_script` -- the #245/#274 guard that turns a `parameters.blocks[].codes[].script` string into an array and re-splits an element packing several `;`-separated statements -- ran on `config update` (0.28.0) and `transformation edit/create` (0.30.8) but never on `sync push`, the one remaining deploy route to the Storage API. After #686 parts 2+3 the guard is a no-op on the GitOps path by construction (`merge_code_files` rebuilds the blocks through the single canonical producer), so this is a regression backstop rather than a correctness fix. It does still cover the shape that bypasses code extraction entirely: a hand-authored `_config.yml` carrying `parameters.blocks` inline with no companion `transform.sql`, which `merge_code_files` passes through verbatim -- a shape the Storage API accepts and the job runtime rejects. - new `guard_script_shape()` in `_sync_push_ops.py` wraps the helper unchanged and shapes its records into push-envelope warnings; - called in `push_create` / `push_update` after `merge_code_files` + `local_config_to_api`, before encryption and send; - called in the Phase C variables backfill too: it re-PUTs the WHOLE body, so it is the last write a freshly-created transformation receives; - rows are deliberately NOT guarded: code extraction is config-level only (`merge_code_files` is never called for a row) and the sibling `config row-create` / `row-update` path is likewise unguarded; - records surface as `warnings[]` entries with `change_type: "script_normalization"` (`path` / `action` / `after_length` kept), so human mode prints them through the existing push-warning loop and `--json` carries them structurally. `config update` keeps its dedicated `normalizations` key; a push envelope spans many configs, so each record carries its own identity. `warnings[]` element type widened to `dict[str, Any]` -- the records carry a non-string `after_length`. Docs: `(since vNEXT)` notes in gotchas.md and sync-workflow.md. --- .../skills/kbagent/references/gotchas.md | 16 + .../kbagent/references/sync-workflow.md | 7 + .../services/_sync_bindings.py | 13 +- .../services/_sync_models.py | 5 +- .../services/_sync_push_ops.py | 105 ++++- .../services/sync_service.py | 10 +- tests/test_sync_push_script_guard.py | 371 ++++++++++++++++++ 7 files changed, 513 insertions(+), 14 deletions(-) create mode 100644 tests/test_sync_push_script_guard.py diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 8bf0818b..834e4606 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -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` diff --git a/plugins/kbagent/skills/kbagent/references/sync-workflow.md b/plugins/kbagent/skills/kbagent/references/sync-workflow.md index 26d40c61..4867ba62 100644 --- a/plugins/kbagent/skills/kbagent/references/sync-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/sync-workflow.md @@ -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 diff --git a/src/keboola_agent_cli/services/_sync_bindings.py b/src/keboola_agent_cli/services/_sync_bindings.py index e5417972..c86b9184 100644 --- a/src/keboola_agent_cli/services/_sync_bindings.py +++ b/src/keboola_agent_cli/services/_sync_bindings.py @@ -34,6 +34,7 @@ FlowBindingResult, VariableBindingResult, ) +from ._sync_push_ops import guard_script_shape if TYPE_CHECKING: from .sync_service import SyncService @@ -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. @@ -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 @@ -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. @@ -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. diff --git a/src/keboola_agent_cli/services/_sync_models.py b/src/keboola_agent_cli/services/_sync_models.py index 2307d322..89df5a54 100644 --- a/src/keboola_agent_cli/services/_sync_models.py +++ b/src/keboola_agent_cli/services/_sync_models.py @@ -11,6 +11,7 @@ from dataclasses import dataclass, field from pathlib import Path +from typing import Any from ..sync.manifest import ManifestConfiguration @@ -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 @@ -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 diff --git a/src/keboola_agent_cli/services/_sync_push_ops.py b/src/keboola_agent_cli/services/_sync_push_ops.py index 4ffa8d9e..0e8f2cc2 100644 --- a/src/keboola_agent_cli/services/_sync_push_ops.py +++ b/src/keboola_agent_cli/services/_sync_push_ops.py @@ -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 @@ -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, @@ -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. @@ -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. @@ -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. @@ -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) @@ -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( @@ -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 @@ -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( diff --git a/src/keboola_agent_cli/services/sync_service.py b/src/keboola_agent_cli/services/sync_service.py index f54ed01e..746a3ecc 100644 --- a/src/keboola_agent_cli/services/sync_service.py +++ b/src/keboola_agent_cli/services/sync_service.py @@ -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 @@ -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", "")) @@ -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(): diff --git a/tests/test_sync_push_script_guard.py b/tests/test_sync_push_script_guard.py new file mode 100644 index 00000000..587e9cc2 --- /dev/null +++ b/tests/test_sync_push_script_guard.py @@ -0,0 +1,371 @@ +"""``sync push`` runs the #274 runtime-safety script guard (follow-up to #686). + +``normalize_blocks_codes_script`` closes the gap between the Storage API's lax +shape validator and the Keboola runtime's strict one: a ``script`` that is a +string, or a list element packing several ``;``-separated statements, is +accepted by the API and crashes the job later ("Expected array, got string" / +``MULTI_STATEMENT_COUNT``). ``config update`` and ``transformation +edit/create`` have run it since 0.28.0 / 0.30.8; the GitOps deploy route did +not. + +After #686 parts 2+3 the guard is a no-op on this path *by construction* -- +``merge_code_files`` rebuilds ``parameters.blocks`` from ``transform.sql`` +through the single canonical producer. It is wired in as a REGRESSION +BACKSTOP, and it still catches the one shape that bypasses code extraction +entirely: a hand-authored ``_config.yml`` that carries ``parameters.blocks`` +inline with no companion code file (``_merge_sql_transformation`` returns +early when ``transform.sql`` is absent, so those parameters reach the API +verbatim). +""" + +import json +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import yaml +from typer.testing import CliRunner + +from keboola_agent_cli.cli import app +from keboola_agent_cli.constants import CONFIG_FILENAME +from keboola_agent_cli.services._sync_push_ops import guard_script_shape +from keboola_agent_cli.services.project_service import ProjectService +from test_sync_baseline_stamping import ( + SQL_COMPONENT, + FakeApi, + _config_file, + _init_and_pull, + _service, + _sql_components, + _sql_file, +) +from test_sync_cli import TEST_TOKEN, _setup_config + +runner = CliRunner() + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _sent_script(api: FakeApi) -> Any: + """The ``script`` value of the last configuration written to the API.""" + configuration = api.update_calls[-1]["configuration"] + return configuration["parameters"]["blocks"][0]["codes"][0]["script"] + + +def _normalization_warnings(result: dict[str, Any]) -> list[dict[str, Any]]: + return [w for w in result.get("warnings", []) if w.get("change_type") == "script_normalization"] + + +def _inline_blocks(project_root: Path, script: Any) -> None: + """Rewrite the pulled config into a code-file-less, inline-blocks tree. + + Deleting ``transform.sql`` is what makes ``merge_code_files`` a no-op, so + whatever ``parameters.blocks`` the YAML holds is exactly what push sends. + """ + _sql_file(project_root).unlink() + config_file = _config_file(project_root) + data = yaml.safe_load(config_file.read_text(encoding="utf-8")) + data["parameters"] = { + "blocks": [{"name": "Block 1", "codes": [{"name": "Code 1", "script": script}]}] + } + config_file.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") + + +# =================================================================== +# The guard fires on a body that would reach the API in a crashing shape +# =================================================================== + + +def test_push_update_normalizes_string_script(tmp_config_dir: Path, tmp_path: Path) -> None: + """A string ``script`` in _config.yml is split into an array before send.""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;", "SELECT 2;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + + _inline_blocks(project_root, "SELECT 1;\nSELECT 2;") + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert _sent_script(api) == ["SELECT 1;", "SELECT 2;"] + + records = _normalization_warnings(result) + assert len(records) == 1 + assert records[0]["action"] == "sql_split" + assert records[0]["after_length"] == 2 + assert records[0]["component_id"] == SQL_COMPONENT + assert records[0]["config_id"] == "cfg-sql" + assert records[0]["path"] == "parameters.blocks[0].codes[0].script" + assert "runtime" in records[0]["message"].lower() + + +def test_push_update_resplits_packed_list_element(tmp_config_dir: Path, tmp_path: Path) -> None: + """One list element packing two statements is re-split (#274 crash shape).""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;", "SELECT 2;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + + _inline_blocks(project_root, ["SELECT 1;\nSELECT 2;"]) + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert _sent_script(api) == ["SELECT 1;", "SELECT 2;"] + + records = _normalization_warnings(result) + assert len(records) == 1 + assert records[0]["action"] == "sql_resplit" + assert records[0]["after_length"] == 2 + + +def test_push_create_normalizes_string_script(tmp_config_dir: Path, tmp_path: Path) -> None: + """The CREATE path is guarded too (a hand-authored, never-pulled config).""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + + new_dir = _config_file(project_root).parent.parent / "new-transformation" + new_dir.mkdir(parents=True) + (new_dir / CONFIG_FILENAME).write_text( + yaml.safe_dump( + { + "name": "New transformation", + "description": "", + "_keboola": {"component_id": SQL_COMPONENT}, + "parameters": { + "blocks": [ + { + "name": "Block 1", + "codes": [{"name": "Code 1", "script": "SELECT 9;\nSELECT 8;"}], + } + ] + }, + }, + sort_keys=False, + ), + encoding="utf-8", + ) + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert result["created"] == 1 + created = next( + c + for component in api.components + if component["id"] == SQL_COMPONENT + for c in component["configurations"] + if c["name"] == "New transformation" + ) + assert created["configuration"]["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "SELECT 9;", + "SELECT 8;", + ] + + records = _normalization_warnings(result) + assert len(records) == 1 + assert records[0]["action"] == "sql_split" + assert records[0]["config_path"].endswith("new-transformation") + + +# =================================================================== +# No-op path: a canonical body produces NO records +# =================================================================== + + +def test_canonical_push_produces_no_normalization_records( + tmp_config_dir: Path, tmp_path: Path +) -> None: + """The ordinary pull -> edit transform.sql -> push flow stays silent.""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1;", "SELECT 2;", "SELECT 3;"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + + sql_file = _sql_file(project_root) + sql_file.write_text( + sql_file.read_text(encoding="utf-8").replace("SELECT 3;", "SELECT 4;"), encoding="utf-8" + ) + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert result["updated"] == 1 + assert _normalization_warnings(result) == [] + assert _sent_script(api) == ["SELECT 1;", "SELECT 2;", "SELECT 4;"] + # The guard must not perturb the baseline it was wired in beside. + diff_result = _service(store, api).diff(alias="prod", project_root=project_root) + assert diff_result["summary"]["remote_modified"] == 0 + assert diff_result["summary"]["modified"] == 0 + + +def test_canonical_push_without_semicolons_produces_no_records( + tmp_config_dir: Path, tmp_path: Path +) -> None: + """Semicolon-less canonical elements are already one-statement-per-element.""" + project_root = tmp_path / "project" + api = FakeApi(_sql_components(["SELECT 1", "SELECT 2"])) + store = _init_and_pull(tmp_config_dir, project_root, api) + + sql_file = _sql_file(project_root) + sql_file.write_text( + sql_file.read_text(encoding="utf-8").replace("SELECT 2", "SELECT 20"), encoding="utf-8" + ) + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert _normalization_warnings(result) == [] + assert _sent_script(api) == ["SELECT 1", "SELECT 20"] + + +def test_non_transformation_push_is_untouched(tmp_config_dir: Path, tmp_path: Path) -> None: + """A component without blocks/codes never grows a normalization record.""" + project_root = tmp_path / "project" + api = FakeApi( + [ + { + "id": "keboola.ex-http", + "type": "extractor", + "configurations": [ + { + "id": "cfg-001", + "name": "My HTTP Extractor", + "description": "", + "configuration": {"parameters": {"baseUrl": "https://api.example.com"}}, + "rows": [], + } + ], + } + ] + ) + store = _init_and_pull(tmp_config_dir, project_root, api) + + config_file = _config_file(project_root) + data = yaml.safe_load(config_file.read_text(encoding="utf-8")) + data["parameters"]["baseUrl"] = "https://api.example.org" + config_file.write_text(yaml.safe_dump(data, sort_keys=False), encoding="utf-8") + + result = _service(store, api).push(alias="prod", project_root=project_root) + + assert result["errors"] == [] + assert result["updated"] == 1 + assert _normalization_warnings(result) == [] + + +# =================================================================== +# The shared helper -- also used by the Phase C variables backfill, which +# re-PUTs the WHOLE body and would otherwise undo push_create's fix +# =================================================================== + + +def test_guard_script_shape_records_identity_and_normalizes() -> None: + """The helper normalizes in place and shapes one warning per record.""" + configuration = { + "parameters": { + "blocks": [{"name": "B", "codes": [{"name": "C", "script": "SELECT 1;\nSELECT 2;"}]}] + } + } + warnings: list[dict[str, Any]] = [] + + out = guard_script_shape(SQL_COMPONENT, configuration, warnings, config_id="cfg-sql") + + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == ["SELECT 1;", "SELECT 2;"] + assert len(warnings) == 1 + assert warnings[0]["change_type"] == "script_normalization" + assert warnings[0]["config_id"] == "cfg-sql" + assert warnings[0]["action"] == "sql_split" + + +def test_guard_script_shape_is_silent_on_canonical_bodies() -> None: + """A canonical body produces no records and is returned untouched.""" + configuration = { + "parameters": {"blocks": [{"name": "B", "codes": [{"name": "C", "script": ["SELECT 1;"]}]}]} + } + warnings: list[dict[str, Any]] = [] + + out = guard_script_shape(SQL_COMPONENT, configuration, warnings, config_id="cfg-sql") + + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == ["SELECT 1;"] + assert warnings == [] + + +def test_guard_script_shape_still_normalizes_without_a_warning_sink() -> None: + """``warnings=None`` drops the records but never the fix itself.""" + configuration = { + "parameters": { + "blocks": [{"name": "B", "codes": [{"name": "C", "script": "SELECT 1;\nSELECT 2;"}]}] + } + } + + out = guard_script_shape(SQL_COMPONENT, configuration, None) + + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == ["SELECT 1;", "SELECT 2;"] + + +# =================================================================== +# CLI surfacing (human + JSON), the same channel push warnings use +# =================================================================== + + +def _push_envelope() -> dict[str, Any]: + return { + "status": "pushed", + "created": 0, + "updated": 1, + "deleted": 0, + "errors": [], + "warnings": [ + { + "change_type": "script_normalization", + "component_id": SQL_COMPONENT, + "config_id": "cfg-sql", + "config_path": "", + "path": "parameters.blocks[0].codes[0].script", + "action": "sql_split", + "before_type": "str", + "after_type": "list", + "after_length": 2, + "message": "Normalized keboola.snowflake-transformation/cfg-sql script", + } + ], + } + + +def _invoke_push(tmp_path: Path, *, json_mode: bool) -> Any: + config_dir = tmp_path / "config" + config_dir.mkdir() + store = _setup_config(config_dir, {"prod": {"token": TEST_TOKEN}}) + + mock_sync = MagicMock() + mock_sync.push.return_value = _push_envelope() + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.SyncService") as MockSyncService, + ): + MockStore.return_value = store + MockProjService.return_value = ProjectService(config_store=store) + MockSyncService.return_value = mock_sync + argv = ["sync", "push", "--project", "prod", "--directory", str(tmp_path)] + return runner.invoke(app, (["--json"] if json_mode else []) + argv) + + +def test_cli_push_human_mode_prints_normalization(tmp_path: Path) -> None: + """Human mode surfaces the record through the shared warnings channel.""" + result = _invoke_push(tmp_path, json_mode=False) + assert result.exit_code == 0, result.output + assert "Normalized keboola.snowflake-transformation/cfg-sql script" in result.output + + +def test_cli_push_json_mode_carries_normalization(tmp_path: Path) -> None: + """JSON mode carries the structured record on the push envelope.""" + result = _invoke_push(tmp_path, json_mode=True) + assert result.exit_code == 0, result.output + data = json.loads(result.output)["data"] + record = next(w for w in data["warnings"] if w["change_type"] == "script_normalization") + assert record["action"] == "sql_split" + assert record["after_length"] == 2