fix(config): validate the parameters section, not the whole config body (#587)

[padak]

Closes #587.

The bug

A component's configurationSchema (AI Service) describes the contents of the parameters key, not the whole configuration object. _validate_config_body validated the whole body against it, so it was off by one level — which inverted every outcome.

Confirmed live against the AI Service (keboola.ex-db-mysql, --dry-run, nothing created):

body sent	what it is	before	after
{"parameters": {"db": {…}}, "runtime": {…}}	valid Keboola config	❌ failed — <root>: 'db' is a required property	✅ ok
{"parameters": {"nonsense": 1}}	invalid parameters	✅ failed	✅ failed — parameters: 'db' is a required property
{"db": {…}}	malformed (no wrapper)	✅ ok (wrong)	unchanged — see Known limit

Not platform-specific despite the Windows environment in the report — reproduced on macOS.

Why it mattered more than a bad error message

Faced with a bogus failure on a body they knew was correct, the reporter passed --no-validate to get past it, and thereby also switched off the checking that did work. What slipped through was a dropped runtime.parallelism. Keboola treats a missing runtime as parallelism: 1, so a 65-row Snowflake writer ran strictly sequentially — 140 minutes instead of the expected 60–90 — with nothing in any tool output pointing at it. It was caught by hand-comparing per-row job timestamps after the fact.

The fix

services/config_service.py — validation targets body["parameters"], and error paths are prefixed parameters. so they name the section to fix. Sibling keys (storage, runtime, authorization), which the schema does not describe, no longer fail validation.

Unwrapping affects validation only — the POSTed body still carries every sibling key. That has its own regression test, since dropping siblings is the very failure this issue is about.

A body with no parameters key is still validated whole: that is the keboola.flow shape, where phases / tasks are the configuration root (cf. resources/flow/conditional-flow-schema.json, whose required is ["phases","tasks"]).

Known limit: a parameters-level body posted by mistake as the whole configuration is indistinguishable from a flow-style config and still validates ok. That is unchanged from today's behavior, not a new gap — the two shapes cannot be told apart from the body alone.

Scope: validation runs only in create_config. config update and config row-create do not validate against a schema and are untouched.

Why CI never caught it

The mock schema in tests/test_config_create_service.py was itself the reason this shipped: it wrapped everything in a parameters property — a shape no real component returns — so the off-by-one-level validation matched it and the suite stayed green. It is now the real parameters-level shape.

Six tests cover the new contract, of which three are true regression tests — verified to fail against the pre-fix code:

• siblings next to parameters do not fail validation
• the POSTed body keeps its siblings
• error paths are prefixed parameters.

The other three pass against both old and new code by design, and are guards rather than regression proofs: the two flow-style fallback cases (which must keep behaving exactly as before) and the empty-parameters case (which raises under both implementations, for different reported reasons).

Because unit-test mocking is what failed here in the first place, tests/test_e2e.py gains step 19c: a valid wrapped body posted against a real keboola.ex-db-mysql schema (dry-run only, nothing created), asserting the invariant that holds on any stack — a valid configuration must never come back failed — plus, when the stack does serve a schema, that junk parameters are still rejected.

make check: 5495 passed, 12 skipped.

Not addressed here

Issue #587 raises two further asks, both worth their own issues:

1. A real config clone command — copy an entire existing configuration and override only named fields, removing the "forgot a sibling key" class of bug at the source.
2. config examples / config new --help should say that a configuration may carry runtime / storage / authorization next to parameters. Documented in gotchas.md in this PR, but not yet surfaced in the CLI itself.

Review also flagged a third, in component_service._resolve_parameters (the scaffold generator): it still probes schema["properties"]["parameters"]["properties"] — the same wrong mental model — before falling back to the correct flat read. Harmless today because the fallback catches it, but it is a different code path with its own tests, so it is a follow-up rather than scope creep here.

[devin-ai-integration]

Devin Review found 3 potential issues.

[padak]

Review of #589 — fix(config): validate the parameters section, not the whole config body (#587)

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR fixes a real off-by-one-level bug in ConfigService._validate_config_body (src/keboola_agent_cli/services/config_service.py): a component's AI Service configurationSchema describes the contents of a configuration's parameters key, not the whole config body, so the body is now unwrapped (body["parameters"]) before Draft7 validation and error paths are prefixed parameters.. A body with no parameters key (the keboola.flow shape) is still validated whole. The mock schema in the test suite — the actual reason this shipped broken — was rewritten to match the real AI Service shape, and 6 new regression tests were added. I live-verified the fix against a real project (kbagent config new --push --dry-run against keboola.ex-db-mysql) and it reproduces exactly the before/after table in the PR description, including the documented "known limit". No layer violations, no security issues, make check is green (5495 passed), and the changelog/version/plugin-sync files are consistent. Verdict: COMMENT — no blocking findings, a few small non-blocking follow-ups.

Verdict

• Verdict: COMMENT
• Blocking findings: 0
• Non-blocking findings: 4
• Nits: 1

Blocking findings

(none)

Non-blocking findings

[NB-1] plugins/kbagent/skills/kbagent/references/gotchas.md:1180 — new heading has no (since vX.Y.Z) tag

The new section ## Cloning a config by hand: copy the WHOLE object, not just parameters is the only topical gotcha heading added in this diff that carries no version tag, while its sibling bullets two paragraphs above (under the config new --push heading) correctly say (since v0.84.1). Per CONTRIBUTING.md "every new gotcha MUST be tagged with (since vX.Y.Z)", and per the review rubric this is BLOCKING for behavior changes — but this heading documents a persistent fact (there has never been a config clone command, config examples has always shown only parameters) rather than a version-gated behavior change, so I'm not treating it as blocking. Still, tagging it (e.g. (documented since v0.84.1, issue #587)) would keep it consistent with the rest of the file's convention and let an agent anchor "when was this written down" the same way every other heading does.

[NB-2] src/keboola_agent_cli/services/component_service.py:130-146 — _resolve_parameters still carries the old (wrong) off-by-one mental model as dead code

Focus item (3) asked whether any other configurationSchema consumer shares the fixed off-by-one assumption. I checked all three other call sites:

• component_service.py:403 (get_component_detail schema summary) only counts properties/required — schema-shape-agnostic, no bug.
• flow_service.py:293 / flow_validation.py:33 validate the whole {"phases", "tasks"} document directly against the live CF schema — correct, because keboola.flow genuinely has no parameters wrapper (confirmed via flow_service.py:506, configuration = {"phases": phases, "tasks": tasks}).
• component_service.py:140-146 (_resolve_parameters, used by config new's scaffold generator) has a first branch that looks for schema["properties"]["parameters"]["properties"] (the OLD wrong nested-schema model) before falling back to _generate_from_schema(schema) (the correct flat model). Since real AI Service schemas never nest under parameters (confirmed live and via tests/test_component_service.py's fixtures, which already use the flat shape), the first branch is dead in production and untested — it just happens to be harmless because the fallback saves it. Not a live bug, out of scope for this diff, but worth a follow-up cleanup so the stale mental model doesn't confuse the next reader or get copy-pasted into a new call site.

[NB-3] tests/test_e2e.py:2084 (_test_config_new_push) — no E2E test exercises schema validation with an explicit --configuration body against a real component

The existing E2E test for config new --push only exercises the empty-shell path (configuration == {}, validation_status ends up "skipped"/untested for "ok"/"failed"). Issue #587 was specifically about validation being wrong when a real body is provided — the exact scenario the author manually reproduced live against keboola.ex-db-mysql in the PR description's before/after table — but that repro was never codified into test_e2e.py. Per CONTRIBUTING.md this is not strictly mandatory (the E2E requirement in the checklist targets new commands, and this is a fix to existing command behavior), so this is non-blocking, but given the regression already escaped once because the mock schema was unrealistic, an E2E case posting a real --configuration body against a real writer component (dry-run is enough) would close the loop that unit-test mocking alone cannot.

[NB-4] tests/test_config_create_service.py:415 — test_empty_parameters_still_fails_a_schema_that_requires_fields does not discriminate old vs. new code

Per focus item (4) I ran the new TestParametersLevelSchemaValidation class against the pre-fix _validate_config_body (checked out from main, same new fixtures) in an isolated worktree. 3 of 6 tests genuinely fail against the old code (test_sibling_keys_next_to_parameters_do_not_fail_validation, test_whole_body_is_posted_even_though_only_parameters_is_validated, test_validation_error_path_points_inside_parameters), confirming they are honest regression tests, not tests that merely re-describe the new implementation. The other 3 pass against both old and new code by design (the flow-fallback pair behaves identically before/after since the old code always validated whole-body; the test_empty_parameters_... case also raises ConfigError under both implementations, just for a different reported reason). This isn't a defect — it's still a legitimate edge-case test for the new code path — but the PR description's "six regression tests covering the contract" reads as if all six catch the regression, which isn't quite accurate; worth a one-word tweak in the PR body (not the test file) if the author wants precision.

Nits

• [NIT-1] plugins/kbagent/skills/kbagent/references/gotchas.md:1180 — the new ## Cloning a config by hand... heading has no blank line before it (runs directly off the previous bullet list). Pre-existing style drift exists elsewhere in this file (e.g. the ## data-app JSON output heading a few lines below has the same issue on main), so this isn't a new pattern, but since it's new content it would be an easy one-line fix while in the area.

Verification log

• Read CONTRIBUTING.md (Checklist: Adding a New CLI Command, Plugin synchronization map, Releasing a new version) and CLAUDE.md (convention #17, All CLI Commands) in full.
• Read plugins/kbagent/agents/keboola-expert.md §1 (non-negotiable rules) and §3 (inline gotchas).
• gh auth status → authenticated as padak, repo/workflow scopes ✓
• gh pr view 589 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → OPEN, fix(config): ... conventional prefix matches (bug fix) ✓, +245/-16 across 8 files
• git rev-parse --abbrev-ref HEAD → fix/config-new-parameters-schema-587, matches <branch> ✓ (no checkout needed)
• gh pr diff 589 → 368 lines; git diff main...HEAD --stat confirms only config_service.py + tests + docs/version-sync files changed; no command surface touched (git diff main...HEAD -- src/keboola_agent_cli/commands/ src/keboola_agent_cli/permissions.py src/keboola_agent_cli/server/routers/ → empty), so context.py/CLAUDE.md/permissions.py/commands-reference.md/keboola-expert.md §2 matrix updates are correctly not required
• Layer-violation greps (typer/click/formatter in services; httpx/requests in commands) → all empty ✓
• Convention greps (magic numbers, raw error-code strings, bare except:, print(), token leaks, new tuple[...] returns) → all empty ✓
• Simulated jsonschema.Draft7Validator.iter_errors() against None/[]/'a string'/{"db": None} targets → all produce clean, non-crashing type errors with sensible messages ("parameters: None is not of type 'object'" after prefixing) — confirms focus item (2), no crash on non-dict parameters
• Traced all 4 configuration_schema consumers (config_service.py, component_service.py x2, flow_service.py/flow_validation.py) — confirms focus item (3), only _validate_config_body had the live bug; see NB-2 for the one dead-code loose end
• Created an isolated detached git worktree at merge-base(HEAD, main) (does not touch this working tree), copied the PR's new tests/test_config_create_service.py onto pre-fix config_service.py, ran uv run pytest -k ParametersLevelSchemaValidation → 3 of 6 tests FAIL against the old code (confirms focus item (4), test honesty) — worktree removed after
• make check (fresh uv sync --extra server first) → exit 0, 5495 passed, 12 skipped, lint/format/typecheck/skill-freshness/version-sync/command-sync/changelog-check/error-codes/sentinel-guards all green
• pyproject.toml/changelog.py/.claude-plugin/marketplace.json/plugins/kbagent/.claude-plugin/plugin.json/uv.lock all bumped to 0.84.1 consistently ✓
• Live behavior verification (--config-dir /Users/padak/kbagent/demo/.kbagent, project shop, config new --push --no-files --dry-run against real keboola.ex-db-mysql, no API calls made):
  • Valid wrapped body ({"parameters": {"db": {...}}, "runtime": {...}}) → validation_status: "ok" ✓ matches PR's claimed fix
  • Invalid parameters ({"parameters": {"nonsense": 1}}) → validation_status: "failed", ["parameters: 'db' is a required property"] ✓ exact match to PR's claimed error text
  • Malformed / no wrapper ({"db": {...}}) → validation_status: "ok" ✓ reproduces the documented "Known limit"
  • Dry-run envelope's configuration field kept ["parameters", "runtime"] — siblings survive validation-only unwrapping ✓
  • Human-mode output (no --json) on the invalid-parameters case correctly prints parameters: 'db' is a required property — both JSON and human surfaces agree ✓

Open questions for the author

(none)

[padak]

Review addressed in 6384242.

NB-1 / NIT-1 — fixed. The Cloning a config by hand heading now carries (documented since v0.84.1, issue #587) and the missing blank line before it.

NB-3 — fixed. tests/test_e2e.py gains step 19c (_test_config_new_push_schema_validation), posting a valid wrapped body against a real keboola.ex-db-mysql schema, dry-run only. It asserts the one-directional invariant that holds regardless of what the stack serves — a valid configuration must never come back failed — plus that the planned POST keeps its runtime sibling. When the stack does return a schema it additionally asserts junk parameters are still rejected with parameters.-prefixed paths, so the unwrap cannot silently become a no-op. Worth doing exactly for the reason you gave: mocking is what failed here, so a mock cannot be what proves it fixed.

NB-4 — correct, and the PR description was the inaccurate part, not the tests. Rewritten: three tests are true regression tests (verified to fail against pre-fix code), the other three are guards that pass against both implementations by design. Your isolated-worktree check matches what I saw in the RED run.

NB-2 — confirmed and not fixed here. component_service._resolve_parameters:142-144 does probe schema["properties"]["parameters"]["properties"] before falling back to the correct flat read. One refinement to your note: it is not strictly dead, it is latent — it would fire for a component whose parameters legitimately contain a field named parameters, and would then scaffold from the wrong level. Unlikely, but not impossible. It is a different code path (scaffold generation, not validation) with its own test surface, so fixing it here would mean shipping an untested change on the back of an unrelated diff. Tracked as a follow-up instead, and noted in the PR description so it is not lost.