Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .claude-plugin/marketplace.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"plugins": [
{
"name": "kbagent",
"version": "0.85.0",
"version": "0.85.1",
"source": "./plugins/kbagent",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, sync configs as files, manage dev branches, and debug SQL in workspaces",
"category": "development"
Expand Down
2 changes: 1 addition & 1 deletion plugins/kbagent/.claude-plugin/plugin.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "kbagent",
"version": "0.85.0",
"version": "0.85.1",
"description": "AI-friendly interface to Keboola Connection projects — explore configs, jobs, lineage, sync configs as files, manage dev branches, and debug SQL in workspaces",
"author": {
"name": "Keboola",
Expand Down
16 changes: 11 additions & 5 deletions plugins/kbagent/skills/kbagent/references/gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -1160,11 +1160,17 @@ events and emits a final `done` SSE frame mirroring the same record.
`<root>:` error on a body you know is correct, the config is fine and the
validator is wrong -- upgrade rather than reaching for `--no-validate`,
which switches off the checking that still works.
- **A body with no `parameters` key is validated whole** -- that is the
keboola.flow shape (`phases` / `tasks` sit at the configuration root).
Consequence: a parameters-level body posted by mistake as the whole
configuration still validates `ok`, because it is indistinguishable from a
flow-style config. Always POST the full object.
- **A body with no `parameters` key is validated as an EMPTY `parameters`
section (since v0.85.1, issue #605).** So a body that forgot the wrapper --
the component's own fields sitting at the configuration root -- now fails
with the schema's required-field errors plus a `hint:` line naming the
missing wrapper. **Before v0.85.1 it validated `ok` and was POSTed
verbatim**, producing a live configuration with no `parameters` key, which
the UI and the component runtime both read as empty (blank boilerplate) even
though `--push` reported success. The whole-body exemption is now keyed on
the component (`keboola.flow` / `keboola.orchestrator`, whose `phases` /
`tasks` genuinely ARE the configuration root), not on the body's shape.
Always POST the full object: `{"storage": ..., "parameters": {...}}`.

## `config clone` duplicates a config whole; cross-project cannot carry secrets (since v0.84.2)

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "keboola-cli"
version = "0.85.0"
version = "0.85.1"
description = "AI-friendly CLI for managing Keboola projects"
readme = "README.md"
requires-python = ">=3.12"
Expand Down
19 changes: 19 additions & 0 deletions src/keboola_agent_cli/changelog.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,25 @@

# Ordered newest-first. Each value is a list of brief one-line descriptions.
CHANGELOG: dict[str, list[str]] = {
"0.85.1": [
"Fix: `kbagent config new --push` no longer creates a broken configuration from a "
"body that forgot the `parameters` wrapper (#605). A component's "
"`configurationSchema` describes the CONTENTS of `configuration.parameters`, so a "
"flattened body -- the component's own fields sitting at the configuration root -- "
"matched that schema, validated `ok`, and was POSTed verbatim. The result was a "
"live configuration with no `parameters` key at all, which the Keboola UI and the "
"component runtime both read as empty (blank boilerplate) while `--push` reported "
"success and returned a config id. A body with no `parameters` key is now validated "
"as an EMPTY parameters section, so the schema's required fields fail it, and the "
"error list carries a `hint:` line naming the missing wrapper -- otherwise the "
"report reads \"'db' is a required property\" to a caller who did supply `db`, one "
"level too high. The whole-body exemption that this used to ride on is now keyed on "
"the component (`keboola.flow` / `keboola.orchestrator`, whose `phases` / `tasks` "
"genuinely are the configuration root) instead of on the body's shape -- deciding it "
"from the body meant the one thing under test was also the thing granting the "
"exemption. Pass `--no-validate` if a component legitimately takes a root-level "
"body. Completes the half of #587 that #589 left standing.",
],
"0.85.0": [
"BREAKING: the MCP passthrough is removed (epic #390 phase 3, deprecated since "
"0.74.0). `kbagent tool list` and `kbagent tool call` are gone, `kbagent agent "
Expand Down
15 changes: 15 additions & 0 deletions src/keboola_agent_cli/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,3 +811,18 @@ def _resolve_app_name() -> str:
# Token value prefixes, used only for masking/validation -- never for auth logic.
AUTH_ACCESS_TOKEN_PREFIX: str = "kbc_at_"
AUTH_REFRESH_TOKEN_PREFIX: str = "kbc_rt_"

# --- Components whose configurationSchema describes the configuration ROOT ---
# For almost every component the AI Service `configurationSchema` describes the
# CONTENTS of `configuration.parameters` (issue #587). Conditional flows are the
# exception: `phases` / `tasks` ARE the configuration root and the schema
# describes that root, so their body must be validated whole. Keying that
# exemption on the component -- rather than on "the body happens to carry no
# `parameters` key" -- is what stops a body that merely FORGOT the wrapper from
# validating clean and being created broken (issue #605).
ROOT_LEVEL_CONFIG_COMPONENTS: frozenset[str] = frozenset(
{
"keboola.flow",
"keboola.orchestrator", # legacy flows; kbagent cannot write them, but reads share this path
}
)
Comment thread
padak marked this conversation as resolved.
46 changes: 34 additions & 12 deletions src/keboola_agent_cli/services/config_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from ..config_store import ConfigStore
from ..constants import (
CONFIG_STATE_MAX_BYTES,
ROOT_LEVEL_CONFIG_COMPONENTS,
)
from ..errors import ConfigError, ErrorCode, KeboolaApiError
from ..json_utils import compute_diff, deep_merge, set_nested_value
Expand Down Expand Up @@ -2000,10 +2001,14 @@ def _validate_config_body(
The schema describes the contents of the body's ``parameters`` key, so
that section is what gets validated; sibling keys (``storage``,
``runtime``, ``authorization``) are not covered by it and are left
alone. A body with no ``parameters`` key is validated as a whole
(keboola.flow-style configurations). Reported error paths are prefixed
with ``parameters.`` so they point at the section the caller must fix.
Unwrapping affects validation only -- the POSTed body is never altered.
alone. A body with no ``parameters`` key is validated as an EMPTY
parameters section, which is what makes a forgotten wrapper fail loudly
instead of creating a broken configuration. Components whose schema
describes the configuration root (``ROOT_LEVEL_CONFIG_COMPONENTS``,
i.e. flows) are validated whole instead. Reported error paths are
prefixed with ``parameters.`` so they point at the section the caller
must fix. Unwrapping affects validation only -- the POSTed body is
never altered.

Returns:
``("ok", [])`` when the body matches the schema.
Expand Down Expand Up @@ -2043,12 +2048,13 @@ def _validate_config_body(
# ("<root>: 'db' is a required property") while a body missing the
# ``parameters`` wrapper validated clean.
#
# Two shapes are validated WHOLE instead, and both are decided before
# unwrapping:
# Two shapes are validated WHOLE instead, and both are decided on the
# COMPONENT and the SCHEMA -- never on the body, which is the very thing
# under test (issue #605):
#
# 1. The body carries no ``parameters`` key. For keboola.flow,
# ``phases`` / ``tasks`` ARE the configuration root and the schema
# describes that root, so there is nothing to unwrap.
# 1. The component's configuration root is what the schema describes.
# For keboola.flow, ``phases`` / ``tasks`` ARE that root, so there is
# nothing to unwrap.
# 2. The schema itself declares a top-level ``parameters`` property,
# which means it describes the whole configuration object. Deciding
# on the body alone would regress such a component: every body used
Expand All @@ -2057,9 +2063,17 @@ def _validate_config_body(
# include one named ``parameters`` -- is misvalidated today too, so
# preferring the schema signal here never trades a working case for
# a broken one.
schema_is_whole_body = "parameters" in (schema.get("properties") or {})
unwrapped = "parameters" in body and not schema_is_whole_body
target = body["parameters"] if unwrapped else body
schema_is_whole_body = component_id in ROOT_LEVEL_CONFIG_COMPONENTS or "parameters" in (
schema.get("properties") or {}
)
unwrapped = not schema_is_whole_body
# A body with no ``parameters`` key has an EMPTY parameters section --
# it is not a licence to validate the body whole. Reading it the other
# way let a flattened body (the component's own fields sitting at the
# configuration root) match the parameters-level schema, pass, and be
# POSTed verbatim into a configuration the UI and the runtime both read
# as empty, with ``--push`` reporting success (issue #605).
target = body.get("parameters", {}) if unwrapped else body

try:
validator = jsonschema.Draft7Validator(schema)
Expand All @@ -2085,6 +2099,14 @@ def _validate_config_body(
return ("skipped", [])

if errors:
if unwrapped and "parameters" not in body:
# Otherwise the report reads "'db' is a required property" to a
# caller who DID supply ``db`` -- one level too high.
errors.append(
"hint: the body has no 'parameters' key -- this component's schema "
"describes the CONTENTS of configuration.parameters, so wrap it as "
'{"parameters": {...}} (or pass --no-validate to skip this check)'
)
return ("failed", errors)
return ("ok", [])

Expand Down
99 changes: 99 additions & 0 deletions tests/test_config_create_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,3 +561,102 @@ def test_config_without_parameters_key_still_reports_its_errors(
configuration={"phases": []}, # 'tasks' missing
)
storage.create_config.assert_not_called()


# ---------------------------------------------------------------------------
# Missing `parameters` wrapper (issue #605)
# ---------------------------------------------------------------------------

# A parameters-level schema with NO required fields: an empty ``parameters``
# section is legitimate for such a component, so a body that carries only
# sibling keys must not be rejected.
OPTIONAL_TABLE_SCHEMA = {
"type": "object",
"properties": {"table": {"type": "string"}},
}


class TestMissingParametersWrapper:
"""A body missing the ``parameters`` wrapper must fail, not create silently.

Issue #587 fixed one half of the mismatch (a correctly nested body was
rejected). The other half survived: a FLATTENED body -- the component's
parameters sitting at the configuration root -- was validated whole against
the parameters-level schema, matched it, and was POSTed verbatim. The
result was a live configuration with no ``parameters`` key at all, which
the UI and the component runtime both read as empty, while ``--push``
reported success (issue #605).
"""

def test_flattened_body_fails_instead_of_creating_a_broken_config(
self, tmp_config_dir: Path
) -> None:
"""The parameters section of a flattened body is empty, so a schema with
required fields must reject it -- nothing reaches the Storage API.
"""
service, storage, _ = _make_service(tmp_config_dir, schema=TABLE_SCHEMA)

with pytest.raises(ConfigError, match="failed schema validation"):
service.create_config(
alias="prod",
component_id="keboola.ex-db-snowflake",
name="My Config",
configuration={"table": "orders"}, # missing the parameters wrapper
)
storage.create_config.assert_not_called()

def test_flattened_body_error_names_the_missing_wrapper(self, tmp_config_dir: Path) -> None:
""" "'table' is a required property" alone is baffling when the caller DID
supply ``table`` -- at the wrong level. The errors must say so.
"""
service, _, _ = _make_service(tmp_config_dir, schema=TABLE_SCHEMA)

result = service.create_config(
alias="prod",
component_id="keboola.ex-db-snowflake",
name="My Config",
configuration={"table": "orders"},
dry_run=True,
)

assert result["validation_status"] == "failed"
assert result["validation_errors"][0] == "parameters: 'table' is a required property"
assert any("no 'parameters' key" in err for err in result["validation_errors"]), result[
"validation_errors"
]

def test_body_without_parameters_passes_when_the_schema_requires_nothing(
self, tmp_config_dir: Path
) -> None:
"""Treating a missing wrapper as an empty ``parameters`` section must not
invent a failure: a config that is legitimately storage-only still creates.
"""
service, storage, _ = _make_service(tmp_config_dir, schema=OPTIONAL_TABLE_SCHEMA)

result = service.create_config(
alias="prod",
component_id="keboola.ex-db-snowflake",
name="My Config",
configuration={"storage": {"input": {"tables": []}}},
)

storage.create_config.assert_called_once()
assert result["validation_status"] == "ok", result

def test_whole_body_carve_out_is_keyed_on_the_component_not_the_body_shape(
self, tmp_config_dir: Path
) -> None:
"""``keboola.flow`` keeps whole-body validation because ITS configuration
root is what the schema describes -- an ordinary component with a
flow-shaped body does not inherit that exemption.
"""
service, storage, _ = _make_service(tmp_config_dir, schema=FLOW_SCHEMA)

with pytest.raises(ConfigError, match="failed schema validation"):
service.create_config(
alias="prod",
component_id="keboola.ex-db-snowflake",
name="My Config",
configuration={"phases": [], "tasks": []},
)
storage.create_config.assert_not_called()
25 changes: 25 additions & 0 deletions tests/test_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -2259,6 +2259,31 @@ def _test_config_new_push_schema_validation(self) -> None:
# which pointed the reader at the wrong level of their own config.
assert all(e.startswith("parameters") for e in invalid["validation_errors"]), invalid

# A body that FORGOT the `parameters` wrapper must fail too (issue
# #605). It used to validate clean -- the flattened body matched the
# parameters-level schema, so `--push` created a configuration with no
# `parameters` key at all, which the UI and the runtime read as empty
# while reporting success. Same `db` payload as `valid_body`, one level
# too high.
flattened = self._run_ok(
"config",
"new",
"--component-id",
"keboola.ex-db-mysql",
"--project",
self.alias,
"--name",
f"{RUN_ID} validation-flat (dry)",
"--push",
"--no-files",
"--configuration",
json.dumps({"db": {"host": "mysql.example.com", "database": "e2e"}}),
"--dry-run",
)["data"]

assert flattened["validation_status"] == "failed", flattened
assert any("no 'parameters' key" in e for e in flattened["validation_errors"]), flattened

def _test_config_clone(self) -> None:
"""Test ``config clone`` -- whole-configuration duplicate (0.84.2+, #587).

Expand Down
2 changes: 1 addition & 1 deletion uv.lock

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

Loading