diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 703a3762..36d079a5 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -10,7 +10,7 @@ "plugins": [ { "name": "kbagent", - "version": "0.27.0", + "version": "0.28.0", "source": "./plugins/kbagent", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "category": "development" diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index cf6d1764..2d06ad7c 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.27.0", + "version": "0.28.0", "description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, call MCP tools, manage dev branches, and debug SQL in workspaces", "author": { "name": "Keboola", diff --git a/plugins/kbagent/agents/keboola-expert.md b/plugins/kbagent/agents/keboola-expert.md index c6cc4fa7..7fdbafd1 100644 --- a/plugins/kbagent/agents/keboola-expert.md +++ b/plugins/kbagent/agents/keboola-expert.md @@ -64,11 +64,12 @@ a critical failure. needed for the current task (e.g. `flow update` needs 0.22.0+, `schedule find` needs 0.23.0+, `config set-default-bucket` needs 0.26.0+, `data-app create / deploy / start / stop / delete / password` - need 0.27.0+, `storage retype` is a future composite), you MUST refuse - the task and return a handoff message to the parent: `"Cannot proceed - safely on kbagent . Missing: . Ask user to run - kbagent update, then re-invoke me."` Do not attempt the task with - workarounds that use MCP strip-bug-prone tools. + need 0.27.0+, `config update` script[] auto-normalize against #245 + trap needs 0.28.0+, `storage retype` is a future composite), you + MUST refuse the task and return a handoff message to the parent: + `"Cannot proceed safely on kbagent . Missing: . + Ask user to run kbagent update, then re-invoke me."` Do not attempt + the task with workarounds that use MCP strip-bug-prone tools. 7. **ALWAYS USE `--json`**. Every `kbagent` invocation MUST have `--json` as the first flag after `kbagent`. This makes output @@ -87,6 +88,7 @@ a critical failure. | Update flow (rename, description, phases) | `kbagent flow update` (partial, no `--file`) | `--file` after fetching current phases, merging locally, passing full YAML | `tool call update_flow` (strips `behavior.onError` pre-MCP v1.60); partial `--file` that drops fields | | Schedule flow | `kbagent flow schedule --cron ... [--timezone]` | `tool call create_flow_schedule` | raw REST to `/storage/configurations/keboola.scheduler` | | Create Snowflake transformation | `kbagent config new --component-id keboola.snowflake-transformation` + `config update --set ...` | `tool call create_sql_transformation` (lower schema, avoids the component refusal) | `tool call create_config` (refuses keboola.snowflake-transformation) | +| Update SQL transformation body (script[]) | `kbagent config update --project P --component-id keboola.snowflake-transformation --config-id K --configuration @body.json` (0.28.0+ auto-normalizes string `script` to array; SQL gets statement-level split, Python/R gets `[script]` wrap; envelope's `normalizations: [...]` records every change) | `kbagent --hint client config update ...` if you need to bypass the auto-normalize for some reason | `tool call update_sql_transformation` -- still vulnerable to the #245 string-vs-array runtime crash because it pushes raw to Storage API; raw `PUT /v2/storage/components/.../configs/...` -- same trap | | Run a job (and wait) | `kbagent job run --project P --component-id C --config-id K --wait` | `tool call run_component` | `job run` without `--wait` when user expects the result | | Browse configs (exploration) | `kbagent config list` / `kbagent config search --query Q` | `tool call list_configs` | full-project pull via MCP just to grep locally | | Fetch a specific config | `kbagent config detail --project P --component-id C --config-id K --json` | `tool call get_config` | re-using an earlier JSON dump | @@ -127,6 +129,22 @@ success, not a failure. for the local scaffold, then `kbagent config update` for the body. Or MCP `create_sql_transformation` which uses a lower-level schema. +- **`script[]` string-vs-array runtime crash** (0.28.0+ auto-fix; #245): + the Storage API silently accepts `parameters.blocks[].codes[].script` + as a string, but the runtime validator rejects it (`Expected array, + got string`) -- the broken push lands silently and the job crashes + hours later (often via the scheduler). `kbagent config update` + auto-normalizes string -> array before pushing: SQL transformations + get statement-level split via the existing `split_statements()` state + machine; Python / R / `kds-team.app-custom-python` get a single-element + `[script]` wrap. Inspect the result envelope's `normalizations: [...]` + to see what was changed (empty list means already-valid input). + **Caveat**: the trap STILL FIRES if you bypass kbagent. `tool call + update_sql_transformation` / `create_sql_transformation` and raw + `PUT /v2/storage/components/.../configs/...` calls do NOT inherit the + normalization (as of MCP v1.59.x). For SQL transformation body + updates, prefer `kbagent config update` over MCP/REST. + - **Primary keys on new output tables**: Keboola creates columns as nullable by default on first insert. A PK on a nullable column crashes the first run. Pattern: strip PKs before first run, run, diff --git a/plugins/kbagent/skills/kbagent/references/commands-reference.md b/plugins/kbagent/skills/kbagent/references/commands-reference.md index 2976b66c..3b2f6ae4 100644 --- a/plugins/kbagent/skills/kbagent/references/commands-reference.md +++ b/plugins/kbagent/skills/kbagent/references/commands-reference.md @@ -38,7 +38,7 @@ All commands support `--json` for structured output. Multi-project flags (`--pro - `config list [--project NAME] [--component-type TYPE] [--component-id ID] [--branch ID] [--include-rows]` -- list configs across projects (branch-aware). With `--include-rows` each row extends to include the full `configuration` and `rows` body (noticeably larger payload -- use only when the bodies are needed; the summary default covers name/description/component/last_modified/folder) - `config detail --project NAME [--project ...] --component-id ID [--config-id ID] [--branch ID] [--with-state]` -- **two modes.** **Single** (with `--config-id`): full config dict, shape unchanged from previous releases (callers depending on `.id`, `.configuration`, `.rows` etc. are unaffected). **Bulk** (omit `--config-id`): returns `{"configs": [...], "errors": [...]}` with every configuration of `--component-id` across one or many projects -- each row tagged with `project_alias`/`branch_id`. One HTTP request per project via `list_components_with_configs` (not one per config; a project with 100 Snowflake writers returns in a single round-trip). `--project` is repeatable in bulk mode; `--config-id` with multiple `--project` is rejected (exit 2) because a single config lives in one project. `--with-state` attaches the runtime `state` dict: single-mode triggers an extra `get_config_state` call, bulk-mode adds `include=state` to the listing call (no N+1) - `config search --query PATTERN [--project NAME] [-i] [-r] [--branch ID]` -- search config bodies for string/regex (branch-aware) -- `config update --project NAME --component-id ID --config-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--configuration-file PATH] [--set PATH=VALUE ...] [--merge] [--dry-run] [--branch ID]` -- update metadata and/or configuration content. `--set` targets a nested key (e.g. `parameters.db.host=new-host`). `--merge` deep-merges into existing config (preserves sibling keys). `--dry-run` previews changes without applying. Paths are relative to the configuration root (unlike MCP's `update_config` which uses paths relative to `parameters`) +- `config update --project NAME --component-id ID --config-id ID [--name N] [--description D] [--configuration JSON|@file|-] [--configuration-file PATH] [--set PATH=VALUE ...] [--merge] [--dry-run] [--branch ID]` -- update metadata and/or configuration content. `--set` targets a nested key (e.g. `parameters.db.host=new-host`). `--merge` deep-merges into existing config (preserves sibling keys). `--dry-run` previews changes without applying. Paths are relative to the configuration root (unlike MCP's `update_config` which uses paths relative to `parameters`). **Auto-normalize (0.28.0+; #245)**: `parameters.blocks[].codes[].script` strings are silently rewritten to arrays before pushing to Storage API -- SQL transformations get statement-level split (respects `'...'` / `"..."` / `$$..$$` / `--` / `#` / `//` / `/* ... */`); Python / R / `kds-team.app-custom-python` get `[script]` wrap. The result envelope's `normalizations: [{path, action: "sql_split"|"wrap_array", before_type, after_type, after_length}]` records every change (empty when nothing was malformed). Closes the runtime "Expected array, got string" trap that the lax Storage API silently lets through. Bypassing kbagent (raw REST, MCP `update_sql_transformation`) does NOT inherit this -- prefer `kbagent config update` for SQL transformation body changes. - `config set-default-bucket --project NAME --component-id ID --config-id ID (--bucket BUCKET_ID | --clear) [--dry-run] [--branch ID]` -- set or clear `configuration.storage.output.default_bucket` on a configuration. Discoverable shortcut for the raw-mode workaround at https://keboola.atlassian.net/wiki/spaces/SUP/pages/3770155030/. Read-modify-write that preserves sibling keys; returns `{"changed": false}` when the value already matches the requested state. Honored by output tables that don't pin their own `destination`. - `config rename --project NAME --component-id ID --config-id ID --name "New Name" [--branch ID] [--directory DIR]` -- rename a configuration (API update + local sync directory rename with git mv support) - `config delete --project NAME --component-id ID --config-id ID [--branch ID]` -- delete a configuration diff --git a/plugins/kbagent/skills/kbagent/references/gotchas.md b/plugins/kbagent/skills/kbagent/references/gotchas.md index 82321578..c0ad563b 100644 --- a/plugins/kbagent/skills/kbagent/references/gotchas.md +++ b/plugins/kbagent/skills/kbagent/references/gotchas.md @@ -526,6 +526,67 @@ CREATE TABLE foo AS See `scaffold-workflow.md` for the complete file structure reference. +## `config update` auto-normalizes `script[]` from string to array (since v0.28.0) + +The Storage API silently accepts a string for `parameters.blocks[].codes[].script`, +but the Keboola runtime validator rejects it with: + +``` +Invalid type for path "root.parameters.blocks.0.codes.X.script". +Expected "array", but got "string" +``` + +The trap: the failed PUT returns 200, the version increments, the UI looks +fine -- the crash happens only when the job runs (often hours later, e.g. by +the scheduler), with no attribution back to the offending write. Reported in +issue #245 after a programmatic refactor of 3 production Snowflake +transformations. + +`kbagent config update` (and any wrapper that takes a full configuration -- +`--configuration`, `--configuration-file`, `--set parameters.blocks.0.codes.0.script=...`, +and dry-run preview) now closes the gap on the write side **before** the +Storage API touch: + +- **SQL transformations** (Snowflake / Synapse / Oracle / Redshift / + BigQuery / DuckDB, plus fragment fallback for `*-exasol-transformation`, + `*-teradata-transformation`, etc.): the string is split on statement + boundaries via the existing `split_statements()` state machine that + already powers `kbagent sync push`. The splitter respects `'...'` / + `"..."` / `$$...$$` / `--` / `#` / `//` / `/* ... */`, so semicolons + inside string literals and block comments do NOT cause splits. +- **Python / R / `kds-team.app-custom-python`** and any other component + sharing the `parameters.blocks[].codes[].script` shape: the string is + wrapped as a single-element array `[script]`. Statement-level split + does not apply -- the runtime treats the script as one code chunk. +- **Already-array `script` values pass through unchanged.** + +Observability: every normalization is surfaced. +- JSON mode: the result envelope gains + `"normalizations": [{"path": "parameters.blocks[0].codes[0].script", + "action": "sql_split", "before_type": "str", "after_type": "list", + "after_length": 3}]`. Empty list when nothing was normalized. +- Human mode: a yellow `Auto-normalized N script field(s) to array + (string -> list). See --json for details.` warning followed by a + per-element trace. +- `--dry-run`: the `new_configuration` field already reflects the + post-normalize shape, so the preview matches what would actually land. + +**The trap still exists when bypassing kbagent.** Direct +`PUT /v2/storage/components/{component}/configs/{config}` calls (curl, +custom Python, the MCP `update_sql_transformation` / `create_sql_transformation` +tools as of MCP v1.59.x) do NOT inherit this normalization. If an LLM agent +is composing the configuration JSON itself, prefer +`kbagent config update --configuration ...` over raw REST or MCP tool calls +for SQL transformations -- that way the normalization fires regardless of +upstream client behaviour. + +Bonus fix in 0.28.0: `kbagent sync push` previously did NOT split semicolons +in BigQuery / DuckDB transformations because those component IDs were +missing from `SQL_TRANSFORMATION_COMPONENTS`. Push collapsed multiple +statements into one `script` element, mirroring closed issue #119 on a +different backend. The 0.28.0 registry now covers BQ / DuckDB explicitly, +plus fragment-based fallback for future / self-hosted SQL backends. + ## Snowflake: MULTI_STATEMENT_COUNT Keboola sends each code block to Snowflake as a single query batch via the ODBC diff --git a/plugins/kbagent/skills/kbagent/references/sql-migration-workflow.md b/plugins/kbagent/skills/kbagent/references/sql-migration-workflow.md index 6b3fb38e..5621fa3d 100644 --- a/plugins/kbagent/skills/kbagent/references/sql-migration-workflow.md +++ b/plugins/kbagent/skills/kbagent/references/sql-migration-workflow.md @@ -123,6 +123,32 @@ failures: | `Actual statement count N did not match desired count 1` | Missing `ALTER SESSION SET MULTI_STATEMENT_COUNT = 0` | | `Database 'SAPI_226' does not exist` | Unquoted database name (Step 4d) | | `invalid identifier '"column"'` | Workspace table conflict (Step 5) | +| `Expected "array", but got "string"` for `script` (runtime, not Storage write) | Pushed `parameters.blocks[].codes[].script` as a string. Storage API silently accepted it; the runtime validator rejects it. **0.28.0+ fix**: `kbagent config update` auto-normalizes string -> array before push (SQL split / Python wrap). MCP `update_sql_transformation` and raw REST do NOT auto-normalize -- prefer `kbagent config update` for SQL transformation body edits. See `gotchas.md` `script[]` entry. | + +## Auto-normalization of `script[]` (since 0.28.0, #245) + +When pushing a SQL transformation body via `kbagent config update --configuration`, +`--configuration-file`, or `--set parameters.blocks.0.codes.0.script=...`, the +CLI normalizes any string `script` value to an array using the same +state-machine splitter that powers `kbagent sync push`: + +- SQL transformations (Snowflake / BigQuery / Synapse / Oracle / Redshift / + DuckDB; fragment fallback for Exasol / Teradata / `*-transformation-v2` + variants): each statement becomes one element. The splitter respects + `'...'`, `"..."`, `$$..$$`, `--`, `#`, `//`, and `/* ... */`, so + semicolons inside literals or comments do not split. +- Python / R transformations and `kds-team.app-custom-python` apps: the + string is wrapped as a single-element array `[script]`. Each runtime + treats the array as one code chunk. + +The result envelope exposes a `normalizations: [...]` field listing every +element that was rewritten (empty list when input was already valid). Human +mode prints a yellow `Auto-normalized N script field(s)` warning followed by +a per-element trace; `--dry-run`'s `new_configuration` already reflects the +post-normalize shape. **The trap still fires when bypassing kbagent** -- if +an LLM agent pushes via `tool call update_sql_transformation` / +`create_sql_transformation` or raw `PUT /v2/storage/components/.../configs/...`, +the string lands as-is and the job crashes at runtime. ## Anti-patterns to avoid @@ -134,3 +160,7 @@ failures: Local files like `/tmp/current_config.json` go stale after every version bump. - **Applying the same replacement twice**: Guard against re-replacing already-migrated paths. Check if the string already contains `sapi_` before replacing. +- **Pushing `script` as a single multi-statement string**: The Storage API + accepts it (200 OK) but the runtime validator crashes (`Expected array, + got string`). Use `kbagent config update` (0.28.0+ auto-normalizes) or + build the array yourself if going via MCP / raw REST. diff --git a/pyproject.toml b/pyproject.toml index 8884aef6..dfca5d5a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.27.0" +version = "0.28.0" description = "AI-friendly CLI for managing Keboola projects" readme = "README.md" requires-python = ">=3.12" diff --git a/src/keboola_agent_cli/changelog.py b/src/keboola_agent_cli/changelog.py index c0ee1a56..611c7b7f 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,13 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.28.0": [ + 'Fix: `kbagent config update` now auto-normalizes `parameters.blocks[].codes[].script` from string to array before pushing to the Storage API. Closes #245. The Storage API silently accepts a string for `script` while the runtime schema validator requires an array (`Invalid type for path "root.parameters.blocks.0.codes.X.script". Expected "array", but got "string"`); the broken push lands silently and crashes only at job-run time, often hours later, with no attribution back to the offending write. The CLI now closes the gap on the write side: SQL transformations (`keboola.snowflake-transformation`, `keboola.synapse-transformation`, `keboola.oracle-transformation`, `keboola.redshift-sql-transformation`, `keboola.google-bigquery-transformation`, `keboola.duckdb-transformation`, plus fragment-fallback for self-hosted variants like `*-exasol-transformation` / `*-teradata-transformation`) get statement-level split via the existing `split_statements()` state-machine (respects `\'...\'` / `"..."` / `$$...$$` / `--` / `#` / `//` / `/* ... */`); Python / R / `kds-team.app-custom-python` and any other component sharing the schema get a single-element array wrap. Already-array `script` values pass through unchanged.', + 'Observability: every normalization is surfaced -- the JSON envelope gains a `normalizations: [{path, action: "sql_split"|"wrap_array", before_type, after_type, after_length}]` field per write (and on `--dry-run` the `new_configuration` reflects the post-normalize shape). Human mode prints a yellow `Auto-normalized N script field(s) to array (string -> list). See --json for details.` warning followed by a per-element trace, so the silent fix is observable to operators and AI agents alike. Default behaviour is silent normalize -- the issue\'s preferred design -- because the Keboola UI splitter and `keboola-as-code` produce the same array shape kbagent now writes; the audit fields exist precisely so callers who want to detect "my agent produced a string" can.', + "Fix (silent gap): `SQL_TRANSFORMATION_COMPONENTS` in `src/keboola_agent_cli/sync/code_extraction.py` was missing `keboola.google-bigquery-transformation` and `keboola.duckdb-transformation`, so `kbagent sync push` previously did NOT split semicolons in BigQuery / DuckDB transformations -- it joined every statement into a single `script` element. Same failure shape as #119 (closed for Snowflake / Synapse / Oracle / Redshift), just on different backends. The fragment-based `is_sql_transformation_component()` helper now also matches `*-bigquery-transformation`, `*-duckdb-transformation`, `*-exasol-transformation`, `*-teradata-transformation`, so newer or self-hosted SQL backends do not require an edit to the exact set.", + "Plumbing: new `normalize_blocks_codes_script(component_id, config) -> (config, normalizations)` helper in `src/keboola_agent_cli/sync/code_extraction.py`, called from `ConfigService.update_config` immediately after `_resolve_configuration` (before the Storage API write). 35 new unit tests in `tests/test_normalize_script.py` covering the registry detection (exact + fragment fallback), splitter edge cases (semicolons inside block comments and string literals), per-component dispatch (SQL split vs Python wrap vs already-array passthrough), `ConfigService` integration (write path, dry-run path, `--set` path), and CLI surfacing in both JSON and human modes. New E2E test class `TestE2EConfigUpdateNormalization` in `tests/test_e2e.py` exercising the full path against a real Snowflake transformation: push string-script -> Storage API stores array -> job runs to `success`. Live-validated against project 901 (`padak`).", + "Plugin: new `(since v0.28.0)` gotcha in `gotchas.md`; `keboola-expert.md` Rule 6 VERSION GATE updated; `commands-reference.md` `config update` bullet annotated; `sql-migration-workflow.md` cross-references the new normalize behaviour next to the `MULTI_STATEMENT_COUNT` section. Upstream `update_sql_transformation` / `create_sql_transformation` MCP tools still need a parallel fix in `keboola-mcp-server` -- a separate issue is recommended.", + ], "0.27.0": [ "New: `kbagent data-app` command group — first-class lifecycle for Keboola data apps (`keboola.data-apps` Storage component + Data Science API `/apps`). Eight subcommands: `list`, `detail`, `create`, `deploy`, `start`, `stop`, `delete`, `password`. The CLI encapsulates the **§9 redeploy contract** (always sends the `{desiredState=running, configVersion, restartIfRunning=true}` trio together; without it, `PATCH /apps {desiredState:running}` silently pins to the empty-shell v2 and the runner errors `dataApp.git.repository is required in /data/config.json`), per-project KMS encryption of git PATs (refuses to write plaintext if the Encryption API does not return a project-scoped ciphertext), cleanup-in-finally on initial-deploy failure (orphan shell deleted by default; `--keep-on-failure` opts out), and a poll loop that respects pitfall #1 — `state == stopped` is NOT terminal while `desiredState == running` (the platform transitions `created → stopped → starting → running` during initial deploy). `data-app create` accepts `--git-pat-env VAR` (recommended; no argv leak), `--git-pat-file PATH`, or `--git-pat-encrypted KBC::Project...` (must be encrypted under THIS project's KMS — ciphertext does not cross projects).", "New: `DataScienceClient` (`src/keboola_agent_cli/data_science_client.py`) — third HTTP client class alongside `KeboolaClient` and `AiServiceClient`. Auth via `X-StorageApi-Token`; URL derived as `data-science.{stack-suffix}` from the connection URL; inherits `BaseHttpClient` for retry/backoff/token-masking. `get_app_password()` accepts the Manage token per-call so it never lives on the persistent client.", diff --git a/src/keboola_agent_cli/commands/config.py b/src/keboola_agent_cli/commands/config.py index 5001e4e6..17e06a0c 100644 --- a/src/keboola_agent_cli/commands/config.py +++ b/src/keboola_agent_cli/commands/config.py @@ -703,6 +703,8 @@ def config_update( raise typer.Exit(code=map_error_to_exit_code(exc)) from None # --- Output --------------------------------------------------------------- + normalizations = result.get("normalizations") or [] + if result.get("dry_run"): changes = result.get("changes", []) if formatter.json_mode: @@ -715,6 +717,7 @@ def config_update( for change in changes: formatter.console.print(f" {change}") formatter.console.print() + _emit_normalizations_warning(formatter, normalizations) return if formatter.json_mode: @@ -729,6 +732,29 @@ def config_update( f"({result.get('component_id', component_id)}/{config_id})" f"{branch_info}" ) + _emit_normalizations_warning(formatter, normalizations) + + +def _emit_normalizations_warning(formatter: Any, normalizations: list[dict[str, Any]]) -> None: + """Surface ``parameters.blocks[].codes[].script`` normalizations in human mode. + + Storage API silently accepts a string for ``script``; the runtime + validator rejects it later. When kbagent auto-fixes the shape, the + operator must see what was changed -- otherwise the fix is invisible + and a downstream consumer might rely on the original (broken) input + shape having been written verbatim. JSON mode already exposes the + same data via the ``normalizations`` field on the envelope. + """ + if not normalizations: + return + formatter.console.print( + f"[yellow]Auto-normalized {len(normalizations)} script field(s) " + f"to array (string -> list). See --json for details.[/yellow]" + ) + for entry in normalizations: + formatter.console.print( + f" [dim]{entry['path']}: {entry['action']} -> {entry['after_length']} element(s)[/dim]" + ) @config_app.command("set-default-bucket") diff --git a/src/keboola_agent_cli/commands/context.py b/src/keboola_agent_cli/commands/context.py index ea00b6d5..18d23104 100644 --- a/src/keboola_agent_cli/commands/context.py +++ b/src/keboola_agent_cli/commands/context.py @@ -124,6 +124,15 @@ nested key (e.g. parameters.db.host=new-host). --merge deep-merges into existing config (preserves sibling keys). --dry-run previews changes. Paths are always relative to the configuration root. + Auto-normalize (0.28.0+; #245): parameters.blocks[].codes[].script + strings are silently rewritten to arrays before pushing to Storage -- + SQL transformations split on statement boundaries (state machine + respects 'string' / "ident" / $$..$$ / -- / # / // / /* */); Python / + R / kds-team.app-custom-python wrap as [script]. The result envelope + gains a normalizations: [{{path, action, before_type, after_type, + after_length}}] field listing every change (empty when input was + already valid). Closes the runtime "Expected array, got string" trap + that the lax Storage API silently lets through. kbagent config set-default-bucket --project NAME --component-id ID --config-id ID (--bucket BUCKET_ID | --clear) [--dry-run] [--branch ID] Set or clear configuration.storage.output.default_bucket on a config without diff --git a/src/keboola_agent_cli/services/config_service.py b/src/keboola_agent_cli/services/config_service.py index 01708c6e..6ab7d4ad 100644 --- a/src/keboola_agent_cli/services/config_service.py +++ b/src/keboola_agent_cli/services/config_service.py @@ -16,6 +16,7 @@ from ..errors import ConfigError, ErrorCode, KeboolaApiError from ..json_utils import compute_diff, deep_merge, set_nested_value from ..models import ProjectConfig +from ..sync.code_extraction import normalize_blocks_codes_script from ..sync.manifest import Manifest, load_manifest, save_manifest from ..sync.naming import sanitize_name from .base import BaseService, sanitize_unexpected_error @@ -567,6 +568,7 @@ def update_config( client = self._client_factory(project.stack_url, project.token) try: final_config: dict[str, Any] | None = None + normalizations: list[dict[str, Any]] = [] if has_content: final_config = self._resolve_configuration( @@ -578,6 +580,14 @@ def update_config( merge=merge, branch_id=effective_branch_id, ) + # Defense-in-depth: Storage API silently accepts a string + # for parameters.blocks[].codes[].script but the runtime + # validator rejects it ("Expected array, got string"), + # turning the broken push into a delayed, hard-to-attribute + # job-time crash. See issue #245. + final_config, normalizations = normalize_blocks_codes_script( + component_id, final_config + ) if dry_run: current = client.get_config_detail( @@ -595,6 +605,7 @@ def update_config( "changes": changes, "old_configuration": old_cfg, "new_configuration": new_cfg, + "normalizations": normalizations, } change_parts = [] @@ -618,6 +629,7 @@ def update_config( result["project_alias"] = alias result["branch_id"] = effective_branch_id + result["normalizations"] = normalizations return result def _resolve_configuration( diff --git a/src/keboola_agent_cli/sync/code_extraction.py b/src/keboola_agent_cli/sync/code_extraction.py index 5f899891..e99df964 100644 --- a/src/keboola_agent_cli/sync/code_extraction.py +++ b/src/keboola_agent_cli/sync/code_extraction.py @@ -36,12 +36,19 @@ def _lines_to_script(lines: list[str], *, is_sql: bool = False) -> list[str]: return [content] -# Component patterns that contain SQL transformations +# Component patterns that contain SQL transformations. +# Used for exact-match dispatch in pull/push code-file extraction. +# For runtime-shape detection (i.e. "does this component's script[] +# need to be split on statement boundaries?") prefer +# :func:`is_sql_transformation_component` -- it adds fragment-based +# fallback so newer/variant SQL backends are covered without code edits. SQL_TRANSFORMATION_COMPONENTS: set[str] = { "keboola.snowflake-transformation", "keboola.synapse-transformation", "keboola.oracle-transformation", "keboola.redshift-sql-transformation", + "keboola.google-bigquery-transformation", + "keboola.duckdb-transformation", } # Component patterns that contain Python transformations @@ -54,6 +61,24 @@ def _lines_to_script(lines: list[str], *, is_sql: bool = False) -> list[str]: "kds-team.app-custom-python", } +# Fragment-based detection for SQL transformations. Keeps +# is_sql_transformation_component robust against newer/variant +# component IDs (e.g. ``keboola.snowflake-transformation-v2``, +# self-hosted ``keboola.exasol-transformation``) without requiring +# an edit to SQL_TRANSFORMATION_COMPONENTS for every new backend. +_SQL_TRANSFORMATION_FRAGMENTS: tuple[str, ...] = ( + "snowflake-transformation", + "synapse-transformation", + "oracle-transformation", + "redshift-sql-transformation", + "redshift-transformation", + "google-bigquery-transformation", + "bigquery-transformation", + "duckdb-transformation", + "exasol-transformation", + "teradata-transformation", +) + SQL_BLOCK_MARKER = "/* ===== BLOCK: {name} ===== */" SQL_CODE_MARKER = "/* ===== CODE: {name} ===== */" PYTHON_BLOCK_MARKER = "# ===== BLOCK: {name} =====" @@ -61,6 +86,116 @@ def _lines_to_script(lines: list[str], *, is_sql: bool = False) -> list[str]: DESCRIPTION_FILENAME = "_description.md" +def is_sql_transformation_component(component_id: str) -> bool: + """Return True if the component's script[] elements are SQL statements. + + The Keboola runtime treats each ``parameters.blocks[].codes[].script`` + element of a SQL transformation as ONE logical statement. Pushing a + string instead of an array passes the Storage API (lax validator) but + crashes at job runtime ("Expected array, got string"). + + Combines exact-match (``SQL_TRANSFORMATION_COMPONENTS``) with a + fragment fallback so newer/variant SQL backends -- including ones + not yet enumerated in the exact set -- still get correct treatment. + """ + if component_id in SQL_TRANSFORMATION_COMPONENTS: + return True + return any(fragment in component_id for fragment in _SQL_TRANSFORMATION_FRAGMENTS) + + +def normalize_blocks_codes_script( + component_id: str, + config: dict[str, Any], +) -> tuple[dict[str, Any], list[dict[str, Any]]]: + """Normalize ``parameters.blocks[].codes[].script`` string -> array. + + The Keboola runtime schema validator requires ``script`` to be an + array. The Storage API silently accepts a string and the runtime + crashes at job execution with:: + + Invalid type for path "root.parameters.blocks.0.codes.X.script". + Expected "array", but got "string" + + This helper closes the gap on the kbagent write side. For SQL + transformations the string is split on statement boundaries via + :func:`split_statements` (state machine respecting comments and + string literals); for Python / R / custom-Python apps and other + components with the same shape, the string is wrapped as a + single-element array. Components that already have ``script`` as a + list are passed through unchanged. + + Args: + component_id: The configuration's component ID. Used to choose + split-vs-wrap and to passthrough components that don't have + this schema at all. + config: The configuration dict to normalize. Mutated in place. + + Returns: + ``(config, normalizations)``. ``normalizations`` is a list of + per-element change records of shape:: + + { + "path": "parameters.blocks[0].codes[1].script", + "action": "sql_split" | "wrap_array", + "before_type": "str", + "after_type": "list", + "after_length": 3, + } + + Empty when nothing was normalized (already-valid input). + + The caller is responsible for surfacing the normalization records to + the user (stderr in human mode, JSON envelope in JSON mode) so the + silent fix is observable. + """ + normalizations: list[dict[str, Any]] = [] + if not isinstance(config, dict): + return config, normalizations + parameters = config.get("parameters") + if not isinstance(parameters, dict): + return config, normalizations + blocks = parameters.get("blocks") + if not isinstance(blocks, list): + return config, normalizations + + is_sql = is_sql_transformation_component(component_id) + + for block_idx, block in enumerate(blocks): + if not isinstance(block, dict): + continue + codes = block.get("codes") + if not isinstance(codes, list): + continue + for code_idx, code in enumerate(codes): + if not isinstance(code, dict): + continue + script = code.get("script") + if not isinstance(script, str): + continue + if is_sql: + new_script = split_statements(script) + action = "sql_split" + else: + # Single-element wrap for Python / R / custom-Python apps + # and any unknown component sharing the schema. Empty + # strings collapse to ``[]`` -- the runtime treats both + # as no-op. + new_script = [script] if script.strip() else [] + action = "wrap_array" + code["script"] = new_script + normalizations.append( + { + "path": f"parameters.blocks[{block_idx}].codes[{code_idx}].script", + "action": action, + "before_type": "str", + "after_type": "list", + "after_length": len(new_script), + } + ) + + return config, normalizations + + def _extract_description(config_data: dict[str, Any], config_dir: Path) -> None: """Extract description field into _description.md.""" description = config_data.get("description", "") diff --git a/tests/test_e2e.py b/tests/test_e2e.py index 096b7ed8..c1c24e7f 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -6082,3 +6082,336 @@ def test_data_app_lifecycle_private_and_redeploy(self) -> None: ) )["data"] assert deploy["config_version"], "deploy must pin a configVersion" + + +# --------------------------------------------------------------------------- +# Issue #245: parameters.blocks[].codes[].script auto-normalize on config update +# --------------------------------------------------------------------------- + + +@skip_without_credentials +class TestE2EConfigUpdateNormalization: + """End-to-end coverage for the v0.28.0 ``script[]`` auto-normalize fix. + + The Storage API silently accepts a string for + ``parameters.blocks[].codes[].script`` while the runtime validator + requires an array. ``kbagent config update`` closes the gap by + splitting / wrapping before pushing to Storage. + + The test creates a Snowflake transformation in an isolated dev branch, + pushes a multi-statement string ``script`` via three different code + paths -- ``--configuration-file``, ``--set`` on a nested path, and + ``--dry-run`` preview -- and asserts each one writes (or previews) + an array, exposes the change record on the result envelope, and + leaves the API config in a state the runtime can parse. + + The job is then run on the normalized config to confirm Snowflake + accepts the multi-statement form (with ``MULTI_STATEMENT_COUNT = 0`` + set as the first statement, per the workflow doc). + + Branch + config are torn down even on failure. + """ + + @pytest.fixture(autouse=True) + def setup(self, tmp_path: Path) -> Any: + self.token = os.environ[ENV_TOKEN] + raw_url = os.environ.get(ENV_URL, "connection.keboola.com") + self.url = raw_url if raw_url.startswith("https://") else f"https://{raw_url}" + self.alias = f"{RUN_ID}-norm" + self.config_dir = tmp_path / "config" + self.config_dir.mkdir() + self.client = KeboolaClient(stack_url=self.url, token=self.token) + + self._created_branch_ids: list[int] = [] + self._created_config_ids: list[tuple[str, str, int | None]] = [] + + result = _invoke( + self.config_dir, + [ + "--json", + "project", + "add", + "--project", + self.alias, + "--url", + self.url, + "--token", + self.token, + ], + ) + assert result.exit_code == 0, f"project add failed: {result.output}" + + yield + + # Teardown: configs first, then branches (branch delete cascades but + # configs in the default branch must be cleaned up explicitly). + for component_id, config_id, branch_id in self._created_config_ids: + with contextlib.suppress(Exception): + self.client.delete_config( + component_id=component_id, + config_id=config_id, + branch_id=branch_id, + ) + for branch_id in self._created_branch_ids: + with contextlib.suppress(Exception): + self.client.delete_dev_branch(branch_id) + self.client.close() + + def _run_ok(self, *args: str) -> dict[str, Any]: + return _json_ok(_invoke(self.config_dir, ["--json", *args])) + + def test_config_update_auto_normalizes_script_array(self, tmp_path: Path) -> None: + """Full path: create transformation, push string-script, verify + every observable surface (envelope, API state, runtime job) reflects + the post-normalize array shape.""" + + _step(1, "branch create", "isolate the test in a short-lived dev branch") + branch_data = self._run_ok( + "branch", + "create", + "--project", + self.alias, + "--name", + f"{RUN_ID}-norm-branch", + )["data"] + branch_id = int(branch_data["branch_id"]) + self._created_branch_ids.append(branch_id) + + _step(2, "create Snowflake transformation in the dev branch") + # Minimum-viable Snowflake transformation: one block, one code, + # initial script as an already-valid array (so create succeeds). + # We will then test config update by REPLACING the script with a + # malformed string -- the bug scenario. + cfg_body = self.client.create_config( + component_id="keboola.snowflake-transformation", + name=f"{RUN_ID} normalize-test", + configuration={ + "parameters": { + "blocks": [ + { + "name": "Block 1", + "codes": [ + { + "name": "init", + "script": ["ALTER SESSION SET MULTI_STATEMENT_COUNT = 0;"], + } + ], + } + ] + } + }, + description=f"E2E #245 normalize check ({RUN_ID})", + branch_id=branch_id, + ) + config_id = str(cfg_body["id"]) + self._created_config_ids.append(("keboola.snowflake-transformation", config_id, branch_id)) + + _step( + 3, + "config update --dry-run with string script", + "preview must show the post-normalize array shape; no API write", + ) + # Multi-statement string with comments and string literals to + # exercise the splitter's state machine (semicolons inside `'...'` + # and `/* ... */` must NOT split). + bad_payload_path = tmp_path / "bad_string_script.json" + bad_payload_path.write_text( + json.dumps( + { + "parameters": { + "blocks": [ + { + "name": "Block 1", + "codes": [ + { + "name": "multi", + "script": ( + "ALTER SESSION SET MULTI_STATEMENT_COUNT = 0;" + " /* trailing block comment with ; semicolons */" + " SELECT 'a;b;c' AS literal_with_semicolons;" + " SELECT 1 AS one;" + " -- trailing line comment" + ), + } + ], + } + ] + } + } + ) + ) + + dry_run = self._run_ok( + "config", + "update", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + "--configuration-file", + str(bad_payload_path), + "--dry-run", + )["data"] + assert dry_run.get("dry_run") is True, "dry-run flag must be set" + norms = dry_run.get("normalizations") or [] + assert len(norms) == 1, f"expected exactly 1 normalization, got {norms}" + assert norms[0]["action"] == "sql_split" + assert norms[0]["before_type"] == "str" + assert norms[0]["after_type"] == "list" + assert norms[0]["after_length"] >= 3, ( + f"splitter should produce >=3 elements (ALTER + 2 SELECTs at minimum); " + f"got after_length={norms[0]['after_length']}" + ) + # The dry-run preview must reflect the post-normalize shape so the + # operator sees what would actually land on Storage. + new_cfg = dry_run["new_configuration"] + new_script = new_cfg["parameters"]["blocks"][0]["codes"][0]["script"] + assert isinstance(new_script, list), ( + f"new_configuration.script must be list after normalize; got {type(new_script).__name__}" + ) + + _step(4, "config update real push", "Storage API must end up with array, not string") + write_envelope = self._run_ok( + "config", + "update", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + "--configuration-file", + str(bad_payload_path), + )["data"] + write_norms = write_envelope.get("normalizations") or [] + assert len(write_norms) == 1, ( + f"expected 1 normalization in write envelope, got {write_norms}" + ) + assert write_norms[0]["action"] == "sql_split" + + _step(5, "fetch from API and assert script[] is array") + detail = self._run_ok( + "config", + "detail", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + )["data"] + stored_script = detail["configuration"]["parameters"]["blocks"][0]["codes"][0]["script"] + assert isinstance(stored_script, list), ( + f"Storage API stored script as {type(stored_script).__name__}, " + f"not list -- normalization did not fire. Value: {stored_script!r}" + ) + # Spot-check splitter correctness: block comment must not have caused + # an extra split, string literal `'a;b;c'` must not have caused one + # either. After normalize we expect ALTER + literal SELECT + numeric + # SELECT + trailing comment as separate elements. + joined = "\n".join(stored_script) + assert "MULTI_STATEMENT_COUNT" in joined, "ALTER SESSION line must survive splitter intact" + assert "'a;b;c'" in joined, ( + "string literal with embedded semicolons must NOT have been split mid-literal" + ) + # Find the literal element and verify it contains the full quoted text + literal_elem = next((s for s in stored_script if "'a;b;c'" in s), None) + assert literal_elem is not None and literal_elem.count("'") >= 2, ( + f"literal element must keep both quotes: {literal_elem!r}" + ) + + _step( + 6, + "--set on a nested path also normalizes", + "even when the user pushes a string at a deep --set path", + ) + # Reset to a known starting point with a single-element array, then + # push a string via --set and assert the same normalize behaviour. + self._run_ok( + "config", + "update", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + "--set", + ( + "parameters.blocks.0.codes.0.script=" + "ALTER SESSION SET MULTI_STATEMENT_COUNT = 0;" + " SELECT 1 AS one;" + " SELECT 2 AS two;" + ), + ) + after_set = self._run_ok( + "config", + "detail", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + )["data"] + after_set_script = after_set["configuration"]["parameters"]["blocks"][0]["codes"][0][ + "script" + ] + assert isinstance(after_set_script, list), ( + f"--set path must also normalize; got {type(after_set_script).__name__}" + ) + assert len(after_set_script) >= 3, ( + f"three statements separated by ; must split into >=3 elements; " + f"got {after_set_script!r}" + ) + + _step( + 7, + "run job on the normalized config", + "Snowflake runtime must accept the array shape (no 'Expected array, got string')", + ) + job_result = self._run_ok( + "job", + "run", + "--project", + self.alias, + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + config_id, + "--branch", + str(branch_id), + "--wait", + "--timeout", + "180", + )["data"] + # We only need a successful schema validation pass + Snowflake parse. + # The transformation has no input/output mappings so it may report + # `success` (no rows moved) or `warning` (no work done); both are + # acceptable -- what we are asserting is the ABSENCE of the runtime + # validator's "Expected array, got string" failure mode. + assert job_result.get("status") in ("success", "warning"), ( + f"Job ended in unexpected state: {job_result.get('status')!r} " + f"(error: {job_result.get('error_message')!r}). " + "If status is 'error' with the schema-validator message, the " + "v0.28.0 normalize fix regressed -- script[] reached the runtime " + "as a string." + ) + # Belt-and-braces: explicitly assert the failure-mode string is NOT + # present anywhere on the envelope, even on a non-error status. + rendered = json.dumps(job_result) + assert "Expected" not in rendered or "script" not in rendered, ( + f"job envelope still mentions the script type-mismatch failure: {rendered}" + ) diff --git a/tests/test_normalize_script.py b/tests/test_normalize_script.py new file mode 100644 index 00000000..4044746f --- /dev/null +++ b/tests/test_normalize_script.py @@ -0,0 +1,577 @@ +"""Tests for ``parameters.blocks[].codes[].script`` normalization (issue #245). + +The Storage API silently accepts a string for ``script`` while the runtime +validator requires an array. Issue #245 closes the gap on the kbagent +write side: SQL transformations get statement-level split via the existing +:func:`split_statements` state machine; Python / R / custom-Python apps +get a single-element wrap. + +Covers: +- helper function ``normalize_blocks_codes_script`` directly +- ``ConfigService.update_config`` writes normalized arrays and exposes + the change record on the result envelope +- CLI human mode emits a yellow warning + per-element trace +""" + +from __future__ import annotations + +import copy +import json +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +from typer.testing import CliRunner + +from helpers import setup_single_project +from keboola_agent_cli.cli import app +from keboola_agent_cli.services.config_service import ConfigService +from keboola_agent_cli.sync.code_extraction import ( + SQL_TRANSFORMATION_COMPONENTS, + is_sql_transformation_component, + normalize_blocks_codes_script, +) + +runner = CliRunner() + + +# --------------------------------------------------------------------------- +# is_sql_transformation_component +# --------------------------------------------------------------------------- + + +class TestIsSqlTransformationComponent: + @pytest.mark.parametrize( + "component_id", + [ + "keboola.snowflake-transformation", + "keboola.synapse-transformation", + "keboola.oracle-transformation", + "keboola.redshift-sql-transformation", + "keboola.google-bigquery-transformation", + "keboola.duckdb-transformation", + ], + ) + def test_known_sql_components(self, component_id: str) -> None: + assert is_sql_transformation_component(component_id) is True + assert component_id in SQL_TRANSFORMATION_COMPONENTS + + @pytest.mark.parametrize( + "component_id", + [ + # variant naming covered by fragment fallback + "keboola.snowflake-transformation-v2", + "keboola.bigquery-transformation", + "custom.exasol-transformation", + "self-hosted.teradata-transformation", + ], + ) + def test_fragment_fallback(self, component_id: str) -> None: + assert is_sql_transformation_component(component_id) is True + + @pytest.mark.parametrize( + "component_id", + [ + "keboola.python-transformation-v2", + "kds-team.app-custom-python", + "keboola.ex-db-mysql", + "keboola.wr-google-bigquery-v2", + "keboola.orchestrator", + "", + ], + ) + def test_non_sql_components(self, component_id: str) -> None: + assert is_sql_transformation_component(component_id) is False + + +# --------------------------------------------------------------------------- +# normalize_blocks_codes_script +# --------------------------------------------------------------------------- + + +class TestNormalizeBlocksCodesScript: + def test_sql_string_split_into_statements(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B1", + "codes": [ + { + "name": "c1", + "script": "CREATE TABLE x AS SELECT 1; INSERT INTO x VALUES (2);", + } + ], + } + ] + } + } + out, norms = normalize_blocks_codes_script( + "keboola.snowflake-transformation", copy.deepcopy(cfg) + ) + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "CREATE TABLE x AS SELECT 1;", + "INSERT INTO x VALUES (2);", + ] + assert len(norms) == 1 + assert norms[0]["action"] == "sql_split" + assert norms[0]["after_length"] == 2 + assert norms[0]["path"] == "parameters.blocks[0].codes[0].script" + + def test_sql_split_respects_block_comments(self) -> None: + """Semicolons inside ``/* ... */`` must not split the statement.""" + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [ + { + "name": "c", + "script": "/* note; with; semicolons */ SELECT 1;", + } + ], + } + ] + } + } + out, _ = normalize_blocks_codes_script( + "keboola.snowflake-transformation", copy.deepcopy(cfg) + ) + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "/* note; with; semicolons */ SELECT 1;" + ] + + def test_sql_split_respects_string_literals(self) -> None: + """Semicolons inside ``'...'`` must not split the statement.""" + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [ + { + "name": "c", + "script": "SELECT 'a;b;c' AS literal; SELECT 2;", + } + ], + } + ] + } + } + out, _ = normalize_blocks_codes_script( + "keboola.snowflake-transformation", copy.deepcopy(cfg) + ) + scripts = out["parameters"]["blocks"][0]["codes"][0]["script"] + assert scripts == ["SELECT 'a;b;c' AS literal;", "SELECT 2;"] + + def test_python_string_wraps_to_single_element(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [ + { + "name": "c", + "script": "import os\nprint('hi')", + } + ], + } + ] + } + } + out, norms = normalize_blocks_codes_script( + "keboola.python-transformation-v2", copy.deepcopy(cfg) + ) + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == ["import os\nprint('hi')"] + assert norms[0]["action"] == "wrap_array" + assert norms[0]["after_length"] == 1 + + def test_custom_python_app_wraps(self) -> None: + cfg = { + "parameters": {"blocks": [{"name": "B", "codes": [{"name": "c", "script": "x = 1"}]}]} + } + out, norms = normalize_blocks_codes_script("kds-team.app-custom-python", copy.deepcopy(cfg)) + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == ["x = 1"] + assert norms[0]["action"] == "wrap_array" + + def test_already_array_passthrough(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": ["SELECT 1;", "SELECT 2;"]}], + } + ] + } + } + before = copy.deepcopy(cfg) + out, norms = normalize_blocks_codes_script("keboola.snowflake-transformation", cfg) + assert out == before + assert norms == [] + + def test_no_blocks_passthrough(self) -> None: + cfg = {"parameters": {"db": {"host": "x"}}} + before = copy.deepcopy(cfg) + out, norms = normalize_blocks_codes_script("keboola.snowflake-transformation", cfg) + assert out == before + assert norms == [] + + def test_no_parameters_passthrough(self) -> None: + cfg: dict = {"storage": {}} + _, norms = normalize_blocks_codes_script( + "keboola.snowflake-transformation", copy.deepcopy(cfg) + ) + assert norms == [] + + def test_empty_string_collapses_to_empty_list(self) -> None: + """A whitespace-only / empty string yields ``[]`` (runtime no-op).""" + cfg = { + "parameters": {"blocks": [{"name": "B", "codes": [{"name": "c", "script": " \n "}]}]} + } + out, _ = normalize_blocks_codes_script( + "keboola.python-transformation-v2", copy.deepcopy(cfg) + ) + assert out["parameters"]["blocks"][0]["codes"][0]["script"] == [] + + def test_multi_block_multi_code_normalizes_each(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B1", + "codes": [ + {"name": "c1", "script": "SELECT 1;"}, + {"name": "c2", "script": ["already array"]}, + ], + }, + { + "name": "B2", + "codes": [{"name": "c3", "script": "SELECT 2; SELECT 3;"}], + }, + ] + } + } + out, norms = normalize_blocks_codes_script( + "keboola.snowflake-transformation", copy.deepcopy(cfg) + ) + assert len(norms) == 2 # c1 and c3, not c2 (already array) + paths = {n["path"] for n in norms} + assert paths == { + "parameters.blocks[0].codes[0].script", + "parameters.blocks[1].codes[0].script", + } + assert out["parameters"]["blocks"][0]["codes"][1]["script"] == ["already array"] + + def test_bigquery_uses_sql_split(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + out, norms = normalize_blocks_codes_script( + "keboola.google-bigquery-transformation", copy.deepcopy(cfg) + ) + assert len(out["parameters"]["blocks"][0]["codes"][0]["script"]) == 2 + assert norms[0]["action"] == "sql_split" + + def test_duckdb_uses_sql_split(self) -> None: + cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + out, norms = normalize_blocks_codes_script( + "keboola.duckdb-transformation", copy.deepcopy(cfg) + ) + assert len(out["parameters"]["blocks"][0]["codes"][0]["script"]) == 2 + assert norms[0]["action"] == "sql_split" + + +# --------------------------------------------------------------------------- +# ConfigService integration +# --------------------------------------------------------------------------- + + +def _make_service(tmp_config_dir: Path, current_cfg: dict) -> tuple[ConfigService, MagicMock]: + store = setup_single_project(tmp_config_dir) + mock_client = MagicMock() + mock_client.get_config_detail.return_value = { + "id": "cfg-001", + "name": "T", + "description": "", + "configuration": current_cfg, + } + mock_client.update_config.return_value = { + "id": "cfg-001", + "name": "T", + "componentId": "keboola.snowflake-transformation", + } + service = ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ) + return service, mock_client + + +class TestConfigServiceUpdateNormalizes: + def test_sql_string_normalized_before_push(self, tmp_config_dir: Path) -> None: + service, client = _make_service(tmp_config_dir, current_cfg={}) + new_cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + + result = service.update_config( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="cfg-001", + configuration=new_cfg, + ) + + sent = client.update_config.call_args.kwargs["configuration"] + # The script that reached the API is an ARRAY, not the original string. + assert sent["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "SELECT 1;", + "SELECT 2;", + ] + # The result envelope exposes what was changed. + assert len(result["normalizations"]) == 1 + assert result["normalizations"][0]["action"] == "sql_split" + + def test_passthrough_when_already_array(self, tmp_config_dir: Path) -> None: + service, client = _make_service(tmp_config_dir, current_cfg={}) + new_cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": ["SELECT 1;", "SELECT 2;"]}], + } + ] + } + } + + result = service.update_config( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="cfg-001", + configuration=new_cfg, + ) + + sent = client.update_config.call_args.kwargs["configuration"] + assert sent["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "SELECT 1;", + "SELECT 2;", + ] + assert result["normalizations"] == [] + + def test_dry_run_shows_normalized_new_configuration(self, tmp_config_dir: Path) -> None: + """Dry-run output reflects what WOULD be pushed (post-normalize).""" + service, client = _make_service(tmp_config_dir, current_cfg={"parameters": {}}) + new_cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + + result = service.update_config( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="cfg-001", + configuration=new_cfg, + dry_run=True, + ) + + assert result["dry_run"] is True + assert result["new_configuration"]["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "SELECT 1;", + "SELECT 2;", + ] + assert len(result["normalizations"]) == 1 + client.update_config.assert_not_called() + + def test_set_path_writing_string_script_is_normalized(self, tmp_config_dir: Path) -> None: + """``--set parameters.blocks.0.codes.0.script="..."`` also normalizes.""" + current_cfg = { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": ["original;"]}], + } + ] + } + } + service, client = _make_service(tmp_config_dir, current_cfg=current_cfg) + + service.update_config( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="cfg-001", + set_paths=[("parameters.blocks.0.codes.0.script", "SELECT 1; SELECT 2;")], + ) + + sent = client.update_config.call_args.kwargs["configuration"] + # --set bypassed the array, but the post-write normalization caught it. + assert sent["parameters"]["blocks"][0]["codes"][0]["script"] == [ + "SELECT 1;", + "SELECT 2;", + ] + + def test_non_transformation_component_passthrough(self, tmp_config_dir: Path) -> None: + """Configs without blocks/codes/script schema are untouched.""" + service, client = _make_service(tmp_config_dir, current_cfg={}) + new_cfg = {"parameters": {"db": {"host": "x"}}} + + result = service.update_config( + alias="prod", + component_id="keboola.ex-db-mysql", + config_id="cfg-001", + configuration=new_cfg, + ) + + sent = client.update_config.call_args.kwargs["configuration"] + assert sent == new_cfg + assert result["normalizations"] == [] + + +# --------------------------------------------------------------------------- +# CLI integration +# --------------------------------------------------------------------------- + + +class TestConfigUpdateCliNormalization: + def _invoke_json(self, tmp_config_dir: Path, args: list[str]) -> object: + return runner.invoke( + app, + ["--json", "--config-dir", str(tmp_config_dir), "config", "update", *args], + ) + + def _invoke_human(self, tmp_config_dir: Path, args: list[str]) -> object: + return runner.invoke( + app, + ["--config-dir", str(tmp_config_dir), "config", "update", *args], + ) + + def test_json_envelope_carries_normalizations(self, tmp_config_dir: Path) -> None: + store = setup_single_project(tmp_config_dir) + mock_client = MagicMock() + mock_client.update_config.return_value = { + "id": "cfg-001", + "name": "T", + "componentId": "keboola.snowflake-transformation", + } + + cfg_payload = json.dumps( + { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + ) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands.config.get_service", + lambda ctx, name: ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ), + ) + result = self._invoke_json( + tmp_config_dir, + [ + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "cfg-001", + "--configuration", + cfg_payload, + ], + ) + + assert result.exit_code == 0, result.output + envelope = json.loads(result.stdout) + normalizations = envelope["data"]["normalizations"] + assert len(normalizations) == 1 + assert normalizations[0]["action"] == "sql_split" + assert normalizations[0]["after_length"] == 2 + + def test_human_mode_emits_warning(self, tmp_config_dir: Path) -> None: + store = setup_single_project(tmp_config_dir) + mock_client = MagicMock() + mock_client.update_config.return_value = { + "id": "cfg-001", + "name": "T", + "componentId": "keboola.snowflake-transformation", + } + + cfg_payload = json.dumps( + { + "parameters": { + "blocks": [ + { + "name": "B", + "codes": [{"name": "c", "script": "SELECT 1; SELECT 2;"}], + } + ] + } + } + ) + + with pytest.MonkeyPatch.context() as mp: + mp.setattr( + "keboola_agent_cli.commands.config.get_service", + lambda ctx, name: ConfigService( + config_store=store, + client_factory=lambda url, token: mock_client, + ), + ) + result = self._invoke_human( + tmp_config_dir, + [ + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "cfg-001", + "--configuration", + cfg_payload, + ], + ) + + assert result.exit_code == 0, result.output + assert "Auto-normalized" in result.output + assert "sql_split" in result.output diff --git a/uv.lock b/uv.lock index 0363ff09..d39aa76e 100644 --- a/uv.lock +++ b/uv.lock @@ -439,7 +439,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.27.0" +version = "0.28.0" source = { editable = "." } dependencies = [ { name = "httpx" },