From b5852856a5a53eb5709cd87a5171ea559258a03d Mon Sep 17 00:00:00 2001 From: Maxmilian Ottomansky Date: Wed, 22 Apr 2026 09:51:42 +0200 Subject: [PATCH 1/3] fix(0.21.2): job run banner reads resolved id, hint threads branch_id, whitespace strip - Rich-mode "Bound variable values row" banner now reads resolvedVariableValuesId from the service response instead of echoing the raw --variable-values-id flag; shows the auto-resolved row even when the flag was omitted. - --variable-values-id input is stripped of surrounding whitespace before the empty-string guard; prevents a padded value from passing the guard and reaching the service unstripped. - --hint client job run --branch ID now threads branch_id through all three client calls (get_config_detail, list_config_rows, create_job); previously the branch arg was silently dropped, causing the rendered hint to target production even when a dev branch was specified. - rich.markup.escape import hoisted to module level in commands/job.py. - New tests: rich-mode resolved-id banner, whitespace strip forwarding, --no-variables JSON output excludes resolvedVariableValuesId, wait=True preserves resolvedVariableValuesId on the waited result, hint branch_id threading contract. --- .gitignore | 2 + plugins/kbagent/.claude-plugin/plugin.json | 2 +- pyproject.toml | 2 +- src/keboola_agent_cli/changelog.py | 6 ++ src/keboola_agent_cli/commands/job.py | 34 +++---- .../hints/definitions/job.py | 3 + src/keboola_agent_cli/services/job_service.py | 8 +- tests/test_cli.py | 92 ++++++++++++++++++- tests/test_e2e.py | 2 +- tests/test_hints.py | 31 +++++++ tests/test_services.py | 42 ++++++++- uv.lock | 2 +- 12 files changed, 194 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index c4b6be52..e5c51e03 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,8 @@ ENV/ # Environment variables .env .env.* +!.env.example +!.env.template # IDE .idea/ diff --git a/plugins/kbagent/.claude-plugin/plugin.json b/plugins/kbagent/.claude-plugin/plugin.json index 8ec26950..ef33fbf1 100644 --- a/plugins/kbagent/.claude-plugin/plugin.json +++ b/plugins/kbagent/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "kbagent", - "version": "0.21.1", + "version": "0.21.2", "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/pyproject.toml b/pyproject.toml index 0eee9ad7..e37cb260 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "keboola-agent-cli" -version = "0.21.1" +version = "0.21.2" 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 29f65c04..568697d7 100644 --- a/src/keboola_agent_cli/changelog.py +++ b/src/keboola_agent_cli/changelog.py @@ -8,6 +8,12 @@ # Ordered newest-first. Each value is a list of brief one-line descriptions. CHANGELOG: dict[str, list[str]] = { + "0.21.2": [ + "Fix: `kbagent job run` rich-mode banner now reads `resolvedVariableValuesId` from the service response instead of echoing the raw `--variable-values-id` flag -- shows the auto-resolved row even when the flag was omitted", + "Fix: `--variable-values-id` value is stripped of surrounding whitespace before reaching the service -- prevents a padded input from bypassing the empty-string guard", + "Fix: `--hint client job run --branch ID` now threads `branch_id` through all three client calls (get_config_detail, list_config_rows, create_job) -- previously the branch arg was silently dropped, causing the hint to target production", + "Chore: `rich.markup.escape` import hoisted to module level in commands/job.py", + ], "0.21.1": [ "Fix: sync pull on a newly created dev branch now writes config rows (#193) -- idempotent skip guard for rows was missing a file-existence check, causing rows to be silently skipped when the branch directory was new (hash matched main because the branch is a clone)", ], diff --git a/src/keboola_agent_cli/commands/job.py b/src/keboola_agent_cli/commands/job.py index 15450f46..a7306d0d 100644 --- a/src/keboola_agent_cli/commands/job.py +++ b/src/keboola_agent_cli/commands/job.py @@ -5,6 +5,7 @@ """ import typer +from rich.markup import escape from ..config_store import ConfigStore from ..constants import ( @@ -244,15 +245,17 @@ def job_run( service = get_service(ctx, "job_service") config_store: ConfigStore = ctx.obj["config_store"] - if variable_values_id is not None and not variable_values_id.strip(): - formatter.error( - message=( - "--variable-values-id cannot be empty or whitespace. " - "Pass a row id, or omit the flag to auto-resolve the default row." - ), - error_code="INVALID_ARGUMENT", - ) - raise typer.Exit(code=2) + if variable_values_id is not None: + variable_values_id = variable_values_id.strip() + if not variable_values_id: + formatter.error( + message=( + "--variable-values-id cannot be empty or whitespace. " + "Pass a row id, or omit the flag to auto-resolve the default row." + ), + error_code="INVALID_ARGUMENT", + ) + raise typer.Exit(code=2) if variable_values_id and no_variables: formatter.error( @@ -278,13 +281,7 @@ def job_run( msg += f" [dim](waiting up to {timeout:.0f}s)[/dim]" msg += "..." formatter.console.print(msg) - if variable_values_id: - from rich.markup import escape - - formatter.console.print( - f"[dim]Using variable values row: {escape(variable_values_id)}[/dim]" - ) - elif no_variables: + if no_variables: formatter.console.print("[dim]Skipping variable-values resolution.[/dim]") try: @@ -314,6 +311,11 @@ def job_run( if formatter.json_mode: formatter.output(result) else: + resolved_id = result.get("resolvedVariableValuesId") + if resolved_id: + formatter.console.print( + f"[dim]Bound variable values row: {escape(str(resolved_id))}[/dim]" + ) job_id = result.get("id", "?") status = result.get("status", "unknown") if status in ("success", "terminated"): diff --git a/src/keboola_agent_cli/hints/definitions/job.py b/src/keboola_agent_cli/hints/definitions/job.py index 1da64866..fa11f8ae 100644 --- a/src/keboola_agent_cli/hints/definitions/job.py +++ b/src/keboola_agent_cli/hints/definitions/job.py @@ -85,6 +85,7 @@ args={ "component_id": "{component_id}", "config_id": "{config_id}", + "branch_id": "{branch}", }, result_var="detail", result_hint="dict", @@ -97,6 +98,7 @@ args={ "component_id": '"keboola.variables"', "config_id": 'detail["configuration"]["variables_id"]', + "branch_id": "{branch}", }, result_var="var_rows", result_hint="list", @@ -110,6 +112,7 @@ "component_id": "{component_id}", "config_id": "{config_id}", "config_row_ids": "{row_id}", + "branch_id": "{branch}", "variable_values_id": 'var_rows[0]["id"] if var_rows else None', }, result_var="job", diff --git a/src/keboola_agent_cli/services/job_service.py b/src/keboola_agent_cli/services/job_service.py index 8ef781e3..295ec17a 100644 --- a/src/keboola_agent_cli/services/job_service.py +++ b/src/keboola_agent_cli/services/job_service.py @@ -281,11 +281,9 @@ def resolve_variable_values_id( error_code="NO_VARIABLE_ROWS", ) - # Defense against a malformed Storage API response: a row without a - # usable `id` would otherwise be returned as `""`, which the client - # treats as "omit from Queue body" -- silently running the job with - # empty variable bindings. Fail loud instead (same silent-skip class - # as the PR1 encryption asymmetry bug). + # A row without a usable `id` would otherwise coerce to `""`, which + # the HTTP client treats as "omit from Queue body" -- silently + # submitting a job with empty variable bindings. Fail loud instead. first_row = rows[0] if isinstance(rows[0], dict) else {} first_row_id = first_row.get("id") if not first_row_id: diff --git a/tests/test_cli.py b/tests/test_cli.py index c3d84fb9..86055d81 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2331,6 +2331,9 @@ def test_job_run_no_variables_flag_forwarded(self, tmp_path: Path) -> None: kwargs = job_service.run_job.call_args.kwargs assert kwargs["no_variables"] is True assert kwargs["variable_values_id"] is None + # resolvedVariableValuesId must be absent when resolution was skipped. + payload = json.loads(result.output).get("data", {}) + assert "resolvedVariableValuesId" not in payload def test_job_run_mutually_exclusive_flags_rejected(self, tmp_path: Path) -> None: """--variable-values-id + --no-variables is an invalid combination (exit 2).""" @@ -2380,8 +2383,7 @@ def test_job_run_rejects_empty_variable_values_id(self, tmp_path: Path) -> None: Locks the fail-loud contract: an empty string must not fall through to create_job(variable_values_id="") where it would silently be - omitted from the Queue body (same silent-skip class as the PR1 - encryption asymmetry). + omitted from the Queue body. """ config_dir = tmp_path / "config" config_dir.mkdir() @@ -2511,6 +2513,92 @@ def test_job_run_no_variable_rows_error(self, tmp_path: Path) -> None: assert result.exit_code != 0 assert "NO_VARIABLE_ROWS" in result.output + def test_job_run_rich_mode_echoes_resolved_values_id(self, tmp_path: Path) -> None: + """Rich (non-JSON) output echoes ``resolvedVariableValuesId`` so auto-resolve is visible.""" + config_dir = tmp_path / "config" + config_dir.mkdir() + store = _setup_config_test( + config_dir, + {"prod": {"token": "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k"}}, + ) + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.ConfigService") as MockCfgService, + patch("keboola_agent_cli.cli.JobService") as MockJobService, + ): + MockStore.return_value = store + job_service = MagicMock() + job_service.run_job.return_value = { + "id": 800, + "status": "waiting", + "resolvedVariableValuesId": "row-auto-resolved", + } + MockJobService.return_value = job_service + MockProjService.return_value = ProjectService(config_store=store) + MockCfgService.return_value = ConfigService(config_store=store) + + result = runner.invoke( + app, + [ + "job", + "run", + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "100", + ], + ) + + assert result.exit_code == 0, result.output + assert "row-auto-resolved" in result.output + assert "Bound variable values row" in result.output + + def test_job_run_strips_whitespace_around_variable_values_id(self, tmp_path: Path) -> None: + """`--variable-values-id ' row-1 '` is trimmed before reaching the service.""" + config_dir = tmp_path / "config" + config_dir.mkdir() + store = _setup_config_test( + config_dir, + {"prod": {"token": "901-10493007-VDtlEDWDF6Tx5V8jjE8FshFlqM0Hl0c08KHqpt0k"}}, + ) + + with ( + patch("keboola_agent_cli.cli.ConfigStore") as MockStore, + patch("keboola_agent_cli.cli.ProjectService") as MockProjService, + patch("keboola_agent_cli.cli.ConfigService") as MockCfgService, + patch("keboola_agent_cli.cli.JobService") as MockJobService, + ): + MockStore.return_value = store + job_service = MagicMock() + job_service.run_job.return_value = {"id": 801, "status": "waiting"} + MockJobService.return_value = job_service + MockProjService.return_value = ProjectService(config_store=store) + MockCfgService.return_value = ConfigService(config_store=store) + + result = runner.invoke( + app, + [ + "--json", + "job", + "run", + "--project", + "prod", + "--component-id", + "keboola.snowflake-transformation", + "--config-id", + "100", + "--variable-values-id", + " row-trimmed ", + ], + ) + + assert result.exit_code == 0, result.output + assert job_service.run_job.call_args.kwargs["variable_values_id"] == "row-trimmed" + class TestJobTerminate: """Tests for `kbagent job terminate` command.""" diff --git a/tests/test_e2e.py b/tests/test_e2e.py index b8b8e778..c18e0336 100644 --- a/tests/test_e2e.py +++ b/tests/test_e2e.py @@ -3069,7 +3069,7 @@ def test_tool_call_get_buckets(self) -> None: # --------------------------------------------------------------------------- -# Job run variable values resolution (PR2 / P0-2) +# Job run variable values resolution # --------------------------------------------------------------------------- diff --git a/tests/test_hints.py b/tests/test_hints.py index d1111da9..d4a80157 100644 --- a/tests/test_hints.py +++ b/tests/test_hints.py @@ -157,6 +157,37 @@ def test_manage_client_import(self) -> None: assert "KBC_MANAGE_API_TOKEN" in code assert "KeboolaClient" not in code + def test_job_run_hint_threads_branch_id_through_all_steps(self) -> None: + """Branch is passed to all three client calls in the job.run hint. + + Locks the PR3 contract: if branch_id is dropped from any of the + get_config_detail / list_config_rows / create_job calls, the rendered + hint would silently hit the production branch even when --branch is + supplied. + """ + import keboola_agent_cli.hints.definitions.job # noqa: F401 — trigger registration + + hint = HintRegistry.get("job.run") + code = ClientRenderer.render( + hint, + params={ + "component_id": '"keboola.snowflake-transformation"', + "config_id": '"123"', + "row_id": None, + "branch": 42, + }, + stack_url=STACK_URL, + branch_id=42, + ) + # The renderer may also inject branch_id into the poll-loop steps, so + # assert >= 3 (one per resolver/create step) rather than an exact count. + assert code.count("branch_id=42") >= 3 + # Confirm each of the three key methods is actually present. + assert "get_config_detail" in code + assert "list_config_rows" in code + assert "create_job" in code + compile(code, "", "exec") + def test_poll_loop_rendering(self) -> None: """Poll loop steps generate a while loop with sleep.""" hint = CommandHint( diff --git a/tests/test_services.py b/tests/test_services.py index 1badbfba..3bd7f8f1 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -1860,8 +1860,8 @@ def test_run_job_with_branch_and_wait(self, tmp_config_dir: Path) -> None: class TestJobServiceVariableValuesResolution: """Tests for `resolve_variable_values_id` + auto-resolution in `run_job`. - Locks the P0-2 contract: transformations with linked variables must - run against the deployed values row, not empty strings. + Locks the contract: transformations with linked variables must run + against the deployed values row, not empty strings. """ def _store(self, tmp_config_dir: Path) -> ConfigStore: @@ -1963,8 +1963,7 @@ def test_resolve_raises_when_first_row_has_no_id(self) -> None: Locks the fail-loud contract: if the Storage API ever returns a row without a usable ``id``, the resolver must refuse rather than returning ``""`` and letting the Queue body quietly omit - ``variableValuesId`` -- same silent-skip class as the PR1 - encryption-asymmetry bug. + ``variableValuesId``. """ mock_client = MagicMock() mock_client.get_config_detail.return_value = { @@ -2045,7 +2044,7 @@ def test_run_job_no_variables_skips_resolution(self, tmp_config_dir: Path) -> No mock_client = MagicMock() mock_client.create_job.return_value = {"id": 702, "status": "waiting"} - self._service(store, mock_client).run_job( + result = self._service(store, mock_client).run_job( alias="prod", component_id="keboola.snowflake-transformation", config_id="100", @@ -2054,6 +2053,39 @@ def test_run_job_no_variables_skips_resolution(self, tmp_config_dir: Path) -> No mock_client.get_config_detail.assert_not_called() assert mock_client.create_job.call_args.kwargs["variable_values_id"] is None + assert "resolvedVariableValuesId" not in result + + def test_run_job_wait_preserves_resolved_variable_values_id(self, tmp_config_dir: Path) -> None: + """resolvedVariableValuesId is stamped on the waited job, not the initial create result. + + Locks the ordering: `job = wait_for_queue_job(...)` replaces the dict + returned by `create_job`; the stamp must happen AFTER the wait so the + final returned dict carries it. + """ + store = self._store(tmp_config_dir) + mock_client = MagicMock() + mock_client.get_config_detail.return_value = { + "configuration": {"variables_id": "vars-cfg-99"} + } + mock_client.list_config_rows.return_value = [{"id": "row-waited"}] + mock_client.create_job.return_value = {"id": 750, "status": "waiting"} + mock_client.wait_for_queue_job.return_value = { + "id": 750, + "status": "success", + "isFinished": True, + } + + result = self._service(store, mock_client).run_job( + alias="prod", + component_id="keboola.snowflake-transformation", + config_id="100", + wait=True, + timeout=30.0, + ) + + assert result["status"] == "success" + assert result["resolvedVariableValuesId"] == "row-waited" + mock_client.wait_for_queue_job.assert_called_once_with("750", max_wait=30.0) def test_run_job_closes_client_when_resolver_raises(self, tmp_config_dir: Path) -> None: """NO_VARIABLE_ROWS raised by the resolver inside run_job still closes the client. diff --git a/uv.lock b/uv.lock index 400d03e1..e51a9b94 100644 --- a/uv.lock +++ b/uv.lock @@ -439,7 +439,7 @@ wheels = [ [[package]] name = "keboola-agent-cli" -version = "0.21.1" +version = "0.21.2" source = { editable = "." } dependencies = [ { name = "httpx" }, From e4018c997597d85e84cd09288846ede16e78747e Mon Sep 17 00:00:00 2001 From: Maxmilian Ottomansky Date: Wed, 22 Apr 2026 14:21:07 +0200 Subject: [PATCH 2/3] review: drop redundant str() in banner, remove PR reference from test docstring --- src/keboola_agent_cli/commands/job.py | 4 +--- tests/test_hints.py | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/keboola_agent_cli/commands/job.py b/src/keboola_agent_cli/commands/job.py index a7306d0d..f855cc33 100644 --- a/src/keboola_agent_cli/commands/job.py +++ b/src/keboola_agent_cli/commands/job.py @@ -313,9 +313,7 @@ def job_run( else: resolved_id = result.get("resolvedVariableValuesId") if resolved_id: - formatter.console.print( - f"[dim]Bound variable values row: {escape(str(resolved_id))}[/dim]" - ) + formatter.console.print(f"[dim]Bound variable values row: {escape(resolved_id)}[/dim]") job_id = result.get("id", "?") status = result.get("status", "unknown") if status in ("success", "terminated"): diff --git a/tests/test_hints.py b/tests/test_hints.py index d4a80157..da66842d 100644 --- a/tests/test_hints.py +++ b/tests/test_hints.py @@ -160,7 +160,7 @@ def test_manage_client_import(self) -> None: def test_job_run_hint_threads_branch_id_through_all_steps(self) -> None: """Branch is passed to all three client calls in the job.run hint. - Locks the PR3 contract: if branch_id is dropped from any of the + Locks the contract: if branch_id is dropped from any of the get_config_detail / list_config_rows / create_job calls, the rendered hint would silently hit the production branch even when --branch is supplied. From 902bbf3b5f4bab56dae5d7f95094b4309958efb6 Mon Sep 17 00:00:00 2001 From: ottomansky Date: Thu, 23 Apr 2026 09:09:53 +0200 Subject: [PATCH 3/3] chore: drop out-of-scope .gitignore whitelist (move to separate PR) --- .gitignore | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitignore b/.gitignore index e5c51e03..c4b6be52 100644 --- a/.gitignore +++ b/.gitignore @@ -18,8 +18,6 @@ ENV/ # Environment variables .env .env.* -!.env.example -!.env.template # IDE .idea/