Skip to content

fix(config): reject a config body that forgot the parameters wrapper (#605) - #614

Merged
padak merged 1 commit into
mainfrom
claude/elastic-sammet-0b3555
Aug 19, 2026
Merged

fix(config): reject a config body that forgot the parameters wrapper (#605)#614
padak merged 1 commit into
mainfrom
claude/elastic-sammet-0b3555

Conversation

@padak

@padak padak commented Aug 19, 2026

Copy link
Copy Markdown
Member

Closes #605.

The half of #587 that #589 left standing

A component's configurationSchema describes the CONTENTS of configuration.parameters, while the Storage API POST takes the whole configuration object. #589 (v0.84.1) fixed one direction of that mismatch: a correctly nested body is no longer rejected. The other direction survived and is what #605 reports.

A flattened body -- the component's own fields sitting at the configuration root, no parameters wrapper -- matched the parameters-level 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. Silent shape corruption: nothing errors, and config detail looks superficially fine.

Reproduced on main before touching anything:

validation_status: ok
POSTed configuration: {'table': 'orders'}

Root cause

The whole-body exemption was decided from the body itself:

unwrapped = "parameters" in body and not schema_is_whole_body
target = body["parameters"] if unwrapped else body

"Body carries no parameters key" was read as "this is a flow-style config whose root IS what the schema describes". But that condition is also exactly what a forgotten wrapper looks like, so the one thing under test was also the thing granting itself the exemption -- the two cases are indistinguishable from the body alone.

Fix

Decide the exemption from the COMPONENT and the SCHEMA, never from the 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
target = body.get("parameters", {}) if unwrapped else body
  • A body with no parameters key is validated as an empty parameters section, so the schema's required fields fail it.
  • ROOT_LEVEL_CONFIG_COMPONENTS (new, in constants.py) carries the genuine root-level cases: keboola.flow / keboola.orchestrator, whose phases / tasks really are the configuration root.
  • The error list gains a hint: line when the wrapper is missing -- otherwise the report reads 'db' is a required property to a caller who DID supply db, one level too high.
  • Escape hatch unchanged: --no-validate for a component that legitimately takes a root-level body.

The POSTed body is still never altered -- unwrapping affects validation only, so runtime / storage / authorization siblings survive (the data loss that prompted #587).

Tests

New TestMissingParametersWrapper (4 cases), written failing-first -- 3 of the 4 failed before the fix:

  • flattened body raises ConfigError and nothing reaches the Storage API
  • the error list names the missing wrapper, not just the required field
  • a body with no parameters against a schema with no required fields still creates (guards against inventing a failure for a legitimately storage-only config)
  • the whole-body carve-out is keyed on the component: a non-flow component with a flow-shaped body is not exempt

tests/test_e2e.py::_test_config_new_push_schema_validation extended with the flattened-body case, mirroring how #589 covered its own fix.

Live verification

Against a real stack with a real AI Service schema (project conditional-flow-testing, keboola.ex-db-mysql), everything --dry-run -- no configuration created:

body before after
{"parameters": {"db": ...}, "runtime": ...} ok ok
{"db": ...} (no wrapper) ok, created broken failed + hint
keboola.flow {"phases": [], "tasks": []} ok ok (carve-out holds)
status: failed
  - parameters: 'db' is a required property
  - 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)

Docs / release

  • gotchas.md: the existing bullet documented this hole as expected behaviour ("a parameters-level body posted by mistake ... still validates ok, because it is indistinguishable from a flow-style config") -- rewritten, tagged since v0.85.1.
  • Version bumped to 0.85.1 with a changelog entry; make version-sync run.
  • make check green (lint, format, typecheck, skill, version, command-sync, changelog, error-codes, sentinel-guards, file-size, 5575 tests).

Note for the reporter

The first symptom in #605 -- a correctly nested body failing validation with <root>: 'venv' is a required property -- was #587, fixed in v0.84.1. The report was filed from v0.79.0, so upgrading resolves that half on its own; this PR covers the second half.


Open in Devin Review

…605)

A component's configurationSchema describes the CONTENTS of
configuration.parameters, so a flattened body -- the component's own fields at
the configuration root -- matched it, validated ok, and was POSTed verbatim,
creating a live configuration with no parameters key while --push reported
success. A body with no parameters key is now validated as an EMPTY parameters
section, and the whole-body exemption is keyed on the component (flows) rather
than on the body's shape.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread src/keboola_agent_cli/constants.py
@padak
padak merged commit d6c6ef8 into main Aug 19, 2026
5 checks passed
@padak
padak deleted the claude/elastic-sammet-0b3555 branch August 19, 2026 22:12
padak added a commit that referenced this pull request Aug 20, 2026
…85.1

The version bump to 0.86.0 already landed on main (#615), but the release
notes it produces were incomplete in two ways.

Missing entries. PR #616 (`token list`, plus the retry-policy and
exceptionId changes) carried no changelog note at all -- its commit message
says "No version bump: this lands in a stack of PRs released as one version.
The (since v0.86.0) doc tags assume 0.86.0 and the bump PR must confirm
that", and the bump PR did not. `make changelog-check` cannot catch this: it
verifies every published GitHub release has an entry, not that every merged
PR has a note. #556/#606 (merge-request endpoints, Layer 3) and #610
(winget job disabled) were likewise unannounced. All four are added.

Phantom 0.85.1. pyproject went 0.85.0 -> 0.85.1 (#614) -> 0.86.0 (#615)
without a tag in between, so 0.85.1 exists only as a changelog bucket -- no
release, no artifact, nobody running it. `format_whats_new` shows the notes
of the *target* version only, so every user upgrading 0.85.0 -> 0.86.0 would
have silently missed those four fixes (Azure ciphertext prefix, the
`parameters` wrapper, GCP/Azure sync ciphertext, the encrypt-values docs).
The bucket is folded into 0.86.0 verbatim.

The same phantom leaked into the agent-facing version gates, which is the
worse half: `keboola-expert.md` told users to "upgrade to 0.85.1+" and four
gotchas.md entries were tagged `(since v0.85.1)` -- a version nobody can
install. Retagged to 0.86.0, along with two source comments.

Three of the new notes had to lead with a shorter sentence to satisfy
`test_newest_release_notes_are_not_truncated` (the headline is the note's
first sentence, capped at 160 chars).

No behaviour change; documentation and release metadata only.
padak added a commit that referenced this pull request Aug 20, 2026
…85.1 (#619)

* chore(release): complete the 0.86.0 changelog and drop the phantom 0.85.1

The version bump to 0.86.0 already landed on main (#615), but the release
notes it produces were incomplete in two ways.

Missing entries. PR #616 (`token list`, plus the retry-policy and
exceptionId changes) carried no changelog note at all -- its commit message
says "No version bump: this lands in a stack of PRs released as one version.
The (since v0.86.0) doc tags assume 0.86.0 and the bump PR must confirm
that", and the bump PR did not. `make changelog-check` cannot catch this: it
verifies every published GitHub release has an entry, not that every merged
PR has a note. #556/#606 (merge-request endpoints, Layer 3) and #610
(winget job disabled) were likewise unannounced. All four are added.

Phantom 0.85.1. pyproject went 0.85.0 -> 0.85.1 (#614) -> 0.86.0 (#615)
without a tag in between, so 0.85.1 exists only as a changelog bucket -- no
release, no artifact, nobody running it. `format_whats_new` shows the notes
of the *target* version only, so every user upgrading 0.85.0 -> 0.86.0 would
have silently missed those four fixes (Azure ciphertext prefix, the
`parameters` wrapper, GCP/Azure sync ciphertext, the encrypt-values docs).
The bucket is folded into 0.86.0 verbatim.

The same phantom leaked into the agent-facing version gates, which is the
worse half: `keboola-expert.md` told users to "upgrade to 0.85.1+" and four
gotchas.md entries were tagged `(since v0.85.1)` -- a version nobody can
install. Retagged to 0.86.0, along with two source comments.

Three of the new notes had to lead with a shorter sentence to satisfy
`test_newest_release_notes_are_not_truncated` (the headline is the note's
first sentence, capped at 160 chars).

No behaviour change; documentation and release metadata only.

* fix(changelog): correct the serve route and give every 0.86.0 note a recognised prefix

Two findings from Devin's review of this PR, plus one they could not see.

The serve route for `token list` was cited as `GET /tokens/{project}`. Both
halves are wrong: the router carries `prefix="/token"` (singular) and the
operation is registered at `/{project}/list`, so the real path is
`GET /token/{project}/list` -- confirmed against the runtime OpenAPI schema,
not the source, because that is what a caller actually hits. Worth noting the
review's proposed correction (`/tokens/{project}/list`) is itself wrong on the
prefix; taking it verbatim would have swapped one 404 for another.

`CI:` is not a recognised note prefix. `_PREFIX_STYLES` / `_PREFIX_RE` in
commands/changelog.py define the set, the module docstring states the contract,
and an unrecognised label renders unhighlighted. Retitled to `Note:`, which
also reads better: the winget job being disabled has a user-facing consequence
(WinGet users stay on the last published version), so burying it under a dim
`Internal:` would understate it.

The finding Devin could not report: the four notification notes carried by
#615/#618 have no prefix at all. They were outside this PR's diff, so no
reviewer looking at the diff would flag them -- but they ship in the same
release block and break the same contract, leaving half of v0.86.0 rendering
flat. Prefixed `New:` / `Note:` with no change of meaning. Every 0.86.0 note
now matches `_PREFIX_RE`, verified by asserting over the live CHANGELOG rather
than by reading.

Each replacement is written to disk on its own. Running several in one script
means a later failed assert discards the earlier successful writes, which is
precisely how #618's stale "server-side ?event=" claim survived its own fix
pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

config new --push: schema validation checks parameters shape but POST sends body unwrapped as configuration

1 participant