From 548fa250511cd2ebdc3f48544b7f85c881ad06b8 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 18 Aug 2026 15:34:26 -0400 Subject: [PATCH 1/2] fix(client): require configuration/is_disabled on rebase, keyword-only MR fields `/rebase` replaces a configuration rather than patching it, so an omitted key is not "leave unchanged" but "take the server-side default". The RFC documents those defaults: `diff.configuration` -> `{}` and `diff.isDisabled` -> `false`. With both parameters optional, a caller resolving a conflict on a disabled config and passing only the two fields the signature marks as required (`name`, `rows`) would wipe the configuration body and re-enable the config -- and then merge that into production. Both are now required, for the same reason `name` and `rows` are. `description` and `change_description` stay optional: the backend maps an absent key to null / a default change message rather than substituting content. Also makes `_optional_mr_fields` keyword-only. The helper exists so the create and update bodies cannot drift, but four of its five parameters are `str | None`, so a positional transposition was the one drift mode it could not catch -- it type-checks cleanly and surfaces only as a backend 422. Tests pin that both fields always reach the wire, that omitting either is a TypeError at the call site rather than silent loss on the wire, and that every `_optional_mr_fields` parameter is KEYWORD_ONLY. --- docs/merge-requests-layer3-rfc.md | 27 ++++-- src/keboola_agent_cli/client/configs.py | 46 ++++++---- .../client/merge_requests.py | 18 +++- tests/test_merge_request_client.py | 91 ++++++++++++++++++- 4 files changed, 150 insertions(+), 32 deletions(-) diff --git a/docs/merge-requests-layer3-rfc.md b/docs/merge-requests-layer3-rfc.md index 5ec0649a..52bc7fec 100644 --- a/docs/merge-requests-layer3-rfc.md +++ b/docs/merge-requests-layer3-rfc.md @@ -135,9 +135,24 @@ per source branch, ever; both guards are **404**, not 400 (`MergeRequestCreateAc **D1 — Explicit typed parameters, not payload dicts.** House style is named parameters with presence detection inside the method — see `update_config` (`client/configs.py:352`, *"Only -provided (non-None) fields are sent"*). `is_disabled: bool | None` stays tri-state for -consistency with `update_config` / `update_config_row`, not because the API forces it (inside a -non-empty `diff` envelope, omitting it defaults to `false` server-side). +provided (non-None) fields are sent"*). + +Presence detection is the right idiom for `update_config`, which **patches**, and the wrong one +for `rebase_config`, which **replaces**: there, an omitted key is not "leave unchanged" but +"take the server-side default". `diff.configuration` defaults to `{}` and `diff.isDisabled` to +`false` (see *Rules* above), so a tri-state `is_disabled: bool | None` and an optional +`configuration` would make silent data loss the signature's default — a caller resolving a +conflict on a disabled config and passing only `name` / `rows` would wipe the configuration body +and re-enable the config, then merge that into production. Both are therefore **required** +parameters, for the same reason `name` and `rows` are: on a replacement endpoint every field the +backend fills in must be supplied deliberately. `description` and `change_description` stay +optional — the backend maps an absent key to null / a default change message rather than +substituting content. + +The same reasoning applies to `_optional_mr_fields` in `client/merge_requests.py`, where +presence detection *is* correct (create and update genuinely patch): the helper is keyword-only, +because four of its five parameters are `str | None` and a positional transposition would +type-check cleanly and surface only as a backend 422. **D2 — JSON bodies throughout, deviating from `configs.py`.** Per *Request bodies are JSON*: `json=`, real nested objects, no `json.dumps`, no `"1"` / `"0"` booleans. Every method gets a @@ -329,7 +344,7 @@ Branch-scoped; `branch_id` required (D5). | Method | Endpoint | |---|---| | `get_config_diff(component_id, config_id, branch_id) -> dict` | `GET …/branch/{branch_id}/components/{c}/configs/{cfg}/diff` | -| `rebase_config(component_id, config_id, branch_id, version, name, rows, configuration=None, description=None, change_description=None, is_disabled=None) -> dict` | `POST …/rebase` (keep) | +| `rebase_config(component_id, config_id, branch_id, version, name, rows, configuration, is_disabled, description=None, change_description=None) -> dict` | `POST …/rebase` (keep) | | `rebase_config_delete(component_id, config_id, branch_id, version) -> dict` | `POST …/rebase` (delete) | `get_config_diff` returns the three-way diff (`base` = dev branch v1, `ours` = dev head, @@ -337,8 +352,8 @@ Branch-scoped; `branch_id` required (D5). Flattening the nested `diff` payload is Layer 2's job. The Python signatures are flat; only the body construction knows about the envelope. -`rebase_config` sends `version` at the top level and puts `name` and `rows` (always) plus any -non-`None` optional inside `diff`; `is_disabled=None` is omitted, `is_disabled=False` is sent. +`rebase_config` sends `version` at the top level and puts the four required content fields +(`name`, `rows`, `configuration`, `is_disabled`) plus any non-`None` optional inside `diff`. `rebase_config_delete` sends exactly `{"version": N, "diff": {}}`. Component and configuration ids are `quote()`d, as everywhere in `configs.py`. diff --git a/src/keboola_agent_cli/client/configs.py b/src/keboola_agent_cli/client/configs.py index a5109887..7652f10d 100644 --- a/src/keboola_agent_cli/client/configs.py +++ b/src/keboola_agent_cli/client/configs.py @@ -685,10 +685,10 @@ def rebase_config( version: int, name: str, rows: list[dict[str, Any]], - configuration: dict[str, Any] | None = None, + configuration: dict[str, Any], + is_disabled: bool, description: str | None = None, change_description: str | None = None, - is_disabled: bool | None = None, ) -> dict[str, Any]: """Rebase a dev-branch configuration onto a newer default-branch version. @@ -698,11 +698,17 @@ def rebase_config( The resolved content travels in a ``diff`` envelope mirroring the shape ``get_config_diff`` returns each side in, so a resolved diff - side posts back nearly 1:1. ``name`` and ``rows`` are required by the - backend for a keep rebase (``rows=[]`` legitimately deletes all - rows); to resolve a conflict by DELETING the config, use - ``rebase_config_delete`` -- the two rebase kinds are separate methods - on purpose, so no illegal combination is expressible (RFC, D6). + side posts back nearly 1:1. Rebase REPLACES the configuration rather + than patching it, so every field the backend fills in on a missing + key is required here, not optional: ``name`` and ``rows`` because + the backend rejects the request without them (``rows=[]`` + legitimately deletes all rows), ``configuration`` and ``is_disabled`` + because it substitutes ``{}`` and ``false`` -- omitting those two + wipes the configuration body and re-enables a disabled config, which + a caller reading "optional" would not expect. To resolve a conflict + by DELETING the config, use ``rebase_config_delete`` -- the two + rebase kinds are separate methods on purpose, so no illegal + combination is expressible (RFC, D6). ``branch_id`` is required with no production fallback (see ``get_config_diff``); the endpoint also requires the @@ -722,28 +728,32 @@ def rebase_config( (``{id?, name?, description?, isDisabled?, configuration?}``); missing/null ``id`` means a new row, duplicates are rejected, array order becomes sort order. - configuration: Resolved configuration body (backend default: {}). + configuration: Resolved configuration body. Required: on a + missing key the backend substitutes ``{}``, wiping it. + is_disabled: Resolved disabled flag. Required: on a missing key + the backend substitutes ``false``, re-enabling a config that + was disabled. Not the tri-state of ``update_config`` + (RFC, D1) -- that method patches, this one replaces. description: Resolved description. ``None`` omits the key -- - which loses nothing: server-side an explicit JSON null and - an absent key are indistinguishable (``isset`` mapping). + which costs no expressiveness: server-side an explicit JSON + null and an absent key are indistinguishable (``isset`` + mapping). change_description: Change log message; when ``None``/omitted the backend uses a default rebase message. - is_disabled: When None, omitted (backend defaults to False); - False is sent explicitly -- tri-state for consistency with - ``update_config`` (RFC, D1). Returns: The rebased configuration dict. """ - diff: dict[str, Any] = {"name": name, "rows": rows} - if configuration is not None: - diff["configuration"] = configuration + diff: dict[str, Any] = { + "name": name, + "rows": rows, + "configuration": configuration, + "isDisabled": is_disabled, + } if description is not None: diff["description"] = description if change_description is not None: diff["changeDescription"] = change_description - if is_disabled is not None: - diff["isDisabled"] = is_disabled return self._rebase_request(component_id, config_id, branch_id, version, diff) def rebase_config_delete( diff --git a/src/keboola_agent_cli/client/merge_requests.py b/src/keboola_agent_cli/client/merge_requests.py index 6e17acdb..ed328f85 100644 --- a/src/keboola_agent_cli/client/merge_requests.py +++ b/src/keboola_agent_cli/client/merge_requests.py @@ -79,6 +79,7 @@ def wait_for_storage_job( def _optional_mr_fields( + *, description: str | None, reviewer_ids: _IntList | None, auto_merge_strategy: str | None, @@ -88,7 +89,10 @@ def _optional_mr_fields( """Build the optional-field part of a create/update body. Shared so the two bodies cannot drift: only provided (non-None) fields - are included, under their wire (camelCase) names. + are included, under their wire (camelCase) names. Keyword-only by + signature: four of the five parameters are ``str | None``, so a + positional transposition would type-check cleanly and only surface as a + backend 422 -- the one drift mode this helper otherwise cannot catch. """ body: dict[str, Any] = {} if description is not None: @@ -194,7 +198,11 @@ def create( } body.update( _optional_mr_fields( - description, reviewer_ids, auto_merge_strategy, auto_merge_at, external_id + description=description, + reviewer_ids=reviewer_ids, + auto_merge_strategy=auto_merge_strategy, + auto_merge_at=auto_merge_at, + external_id=external_id, ) ) return self._requester.request("POST", _BASE, json=body).json() @@ -222,7 +230,11 @@ def update( PUTs ``{}``, which the backend treats as a no-op returning the MR. """ body = _optional_mr_fields( - description, reviewer_ids, auto_merge_strategy, auto_merge_at, external_id + description=description, + reviewer_ids=reviewer_ids, + auto_merge_strategy=auto_merge_strategy, + auto_merge_at=auto_merge_at, + external_id=external_id, ) if title is not None: body["title"] = title diff --git a/tests/test_merge_request_client.py b/tests/test_merge_request_client.py index 77712360..633e96b0 100644 --- a/tests/test_merge_request_client.py +++ b/tests/test_merge_request_client.py @@ -8,6 +8,7 @@ the namespace-never-touches-the-client seam. """ +import inspect import json from typing import Any @@ -15,7 +16,7 @@ import pytest from keboola_agent_cli.client import KeboolaClient -from keboola_agent_cli.client.merge_requests import MergeRequests +from keboola_agent_cli.client.merge_requests import MergeRequests, _optional_mr_fields from keboola_agent_cli.errors import ErrorCode, KeboolaApiError STACK_URL = "https://connection.keboola.com" @@ -325,9 +326,9 @@ def test_keep_rebase_builds_the_envelope(self, client, httpx_mock) -> None: name="My config", rows=[{"id": "r1", "name": "Row 1"}], configuration={"parameters": {"baseUrl": "https://example.com"}}, + is_disabled=False, description="resolved", change_description="rebase onto v7", - is_disabled=False, ) body = _sent_json(httpx_mock.get_requests()[0]) @@ -346,15 +347,61 @@ def test_keep_rebase_builds_the_envelope(self, client, httpx_mock) -> None: def test_keep_rebase_omits_unset_optionals_and_sends_empty_rows( self, client, httpx_mock ) -> None: - """is_disabled=None is omitted; rows=[] is sent (it deletes all rows).""" + """description/change_description are omitted; rows=[] is sent (it deletes all rows).""" httpx_mock.add_response(url=self.REBASE_URL, json={}) client.rebase_config( - "keboola.ex-http", "cfg-1", branch_id=123, version=7, name="My config", rows=[] + "keboola.ex-http", + "cfg-1", + branch_id=123, + version=7, + name="My config", + rows=[], + configuration={}, + is_disabled=False, ) body = _sent_json(httpx_mock.get_requests()[0]) - assert body["diff"] == {"name": "My config", "rows": []} + assert body["diff"] == { + "name": "My config", + "rows": [], + "configuration": {}, + "isDisabled": False, + } + + def test_keep_rebase_always_sends_configuration_and_is_disabled( + self, client, httpx_mock + ) -> None: + """Rebase REPLACES, so neither field may be left to a server-side default. + + A missing ``diff.configuration`` is substituted with ``{}`` and a missing + ``diff.isDisabled`` with ``false``, so omitting either would wipe the + configuration body and re-enable a disabled config. Both are required + parameters; this pins that they always reach the wire. + """ + httpx_mock.add_response(url=self.REBASE_URL, json={}) + + client.rebase_config( + "keboola.ex-http", + "cfg-1", + branch_id=123, + version=7, + name="My config", + rows=[], + configuration={"parameters": {"keep": "me"}}, + is_disabled=True, + ) + + diff = _sent_json(httpx_mock.get_requests()[0])["diff"] + assert diff["configuration"] == {"parameters": {"keep": "me"}} + assert diff["isDisabled"] is True + + def test_keep_rebase_requires_configuration_and_is_disabled(self, client) -> None: + """Omitting either is a TypeError at the call, not silent data loss on the wire.""" + with pytest.raises(TypeError): + client.rebase_config( + "keboola.ex-http", "cfg-1", branch_id=123, version=7, name="N", rows=[] + ) def test_delete_rebase_sends_empty_diff_object(self, client, httpx_mock) -> None: """Delete resolution is exactly {"version": N, "diff": {}} -- diff a JSON object.""" @@ -389,6 +436,40 @@ def wait_for_storage_job( assert namespace.list() == [SAMPLE_MR] assert calls == [("GET", "/v2/storage/merge-request")] + +class TestOptionalFieldHelper: + """_optional_mr_fields is keyword-only, so create/update cannot transpose.""" + + def test_every_field_is_keyword_only(self) -> None: + """No parameter may be positional -- a transposition would type-check cleanly. + + Asserted on the signature rather than by making a deliberately wrong + call: a positional call is a static error too (which is the point), so + writing one would just mean fighting ``ty`` to prove ``ty`` is right. + """ + kinds = { + name: param.kind + for name, param in inspect.signature(_optional_mr_fields).parameters.items() + } + assert set(kinds) == { + "description", + "reviewer_ids", + "auto_merge_strategy", + "auto_merge_at", + "external_id", + } + assert all(kind is inspect.Parameter.KEYWORD_ONLY for kind in kinds.values()), kinds + + def test_helper_omits_unset_fields(self) -> None: + """Only non-None fields are included, under their camelCase wire names.""" + assert _optional_mr_fields( + description=None, + reviewer_ids=[7], + auto_merge_strategy=None, + auto_merge_at=None, + external_id="DMD-1701", + ) == {"reviewerIds": [7], "externalId": "DMD-1701"} + def test_namespace_is_cached_on_the_client(self) -> None: """client.merge_requests returns the same namespace instance every time.""" client = KeboolaClient(stack_url=STACK_URL, token=TOKEN) From 5920cc60a58f5e638a7464360509abf9d3edf0d1 Mon Sep 17 00:00:00 2001 From: Petr Date: Tue, 18 Aug 2026 15:41:06 -0400 Subject: [PATCH 2/2] fix(client): require description on rebase too -- it is part of the replaced body Resolves the open question the PR body left: an absent `diff.description` does NOT preserve the previous description. `RebaseRequest::mapValidatedData` maps a missing key to null, and `ConfigurationRebaseService` documents `$name` / `$description` / `$configuration` / `$isDisabled` as "the complete 3-way diff result" that "fully replaces" the resolved version's body. So an omitted description is written as null -- the same silent-loss failure mode this branch already fixed for `configuration` and `is_disabled`. `description` is now required but stays `str | None`: None is a legitimate resolved value ("the config ends up with no description") and still omits the key rather than sending an explicit null, which costs no expressiveness because the two are indistinguishable server-side. `change_description` stays optional -- it is not part of the replaced body tuple, and null selects a default rebase message rather than clearing anything. The required-field test now iterates every replaced body field rather than naming two, so a future optional-with-default reintroduced on any of them fails. --- docs/merge-requests-layer3-rfc.md | 31 ++++++++++++++-------- src/keboola_agent_cli/client/configs.py | 35 +++++++++++++++---------- tests/test_merge_request_client.py | 33 +++++++++++++++++------ 3 files changed, 66 insertions(+), 33 deletions(-) diff --git a/docs/merge-requests-layer3-rfc.md b/docs/merge-requests-layer3-rfc.md index 52bc7fec..984da67e 100644 --- a/docs/merge-requests-layer3-rfc.md +++ b/docs/merge-requests-layer3-rfc.md @@ -140,14 +140,21 @@ provided (non-None) fields are sent"*). Presence detection is the right idiom for `update_config`, which **patches**, and the wrong one for `rebase_config`, which **replaces**: there, an omitted key is not "leave unchanged" but "take the server-side default". `diff.configuration` defaults to `{}` and `diff.isDisabled` to -`false` (see *Rules* above), so a tri-state `is_disabled: bool | None` and an optional -`configuration` would make silent data loss the signature's default — a caller resolving a -conflict on a disabled config and passing only `name` / `rows` would wipe the configuration body -and re-enable the config, then merge that into production. Both are therefore **required** -parameters, for the same reason `name` and `rows` are: on a replacement endpoint every field the -backend fills in must be supplied deliberately. `description` and `change_description` stay -optional — the backend maps an absent key to null / a default change message rather than -substituting content. +`false`, and an absent `diff.description` to null (`RebaseRequest::mapValidatedData`). So a +tri-state `is_disabled: bool | None` and optional `configuration` / `description` would make +silent data loss the signature's default — a caller resolving a conflict on a disabled config +and passing only `name` / `rows` would wipe the configuration body, drop the description and +re-enable the config, then merge that into production. + +`ConfigurationRebaseService` settles which fields that covers: `$name` / `$description` / +`$configuration` / `$isDisabled` are *"the complete 3-way diff result"* and *"fully replace"* the +resolved version's body. All four are therefore **required** parameters, alongside `rows` (which +the backend rejects the request without). `description` is required-but-nullable — `None` is a +legitimate resolved value meaning "no description", and it omits the key rather than sending an +explicit null, which costs no expressiveness because the two are indistinguishable server-side. + +`change_description` is the one genuine optional: it is not part of the replaced body, and null +selects a default rebase message rather than clearing anything. The same reasoning applies to `_optional_mr_fields` in `client/merge_requests.py`, where presence detection *is* correct (create and update genuinely patch): the helper is keyword-only, @@ -344,7 +351,7 @@ Branch-scoped; `branch_id` required (D5). | Method | Endpoint | |---|---| | `get_config_diff(component_id, config_id, branch_id) -> dict` | `GET …/branch/{branch_id}/components/{c}/configs/{cfg}/diff` | -| `rebase_config(component_id, config_id, branch_id, version, name, rows, configuration, is_disabled, description=None, change_description=None) -> dict` | `POST …/rebase` (keep) | +| `rebase_config(component_id, config_id, branch_id, version, name, rows, configuration, is_disabled, description, change_description=None) -> dict` | `POST …/rebase` (keep) | | `rebase_config_delete(component_id, config_id, branch_id, version) -> dict` | `POST …/rebase` (delete) | `get_config_diff` returns the three-way diff (`base` = dev branch v1, `ours` = dev head, @@ -352,8 +359,10 @@ Branch-scoped; `branch_id` required (D5). Flattening the nested `diff` payload is Layer 2's job. The Python signatures are flat; only the body construction knows about the envelope. -`rebase_config` sends `version` at the top level and puts the four required content fields -(`name`, `rows`, `configuration`, `is_disabled`) plus any non-`None` optional inside `diff`. +`rebase_config` sends `version` at the top level and puts the five required content fields +(`name`, `rows`, `configuration`, `is_disabled`, `description`) plus `change_description` when +set inside `diff`. `description=None` is required-but-nullable: it omits the key, which is how +"the resolved config has no description" is expressed. `rebase_config_delete` sends exactly `{"version": N, "diff": {}}`. Component and configuration ids are `quote()`d, as everywhere in `configs.py`. diff --git a/src/keboola_agent_cli/client/configs.py b/src/keboola_agent_cli/client/configs.py index 7652f10d..5e8de9d1 100644 --- a/src/keboola_agent_cli/client/configs.py +++ b/src/keboola_agent_cli/client/configs.py @@ -687,7 +687,7 @@ def rebase_config( rows: list[dict[str, Any]], configuration: dict[str, Any], is_disabled: bool, - description: str | None = None, + description: str | None, change_description: str | None = None, ) -> dict[str, Any]: """Rebase a dev-branch configuration onto a newer default-branch version. @@ -699,15 +699,19 @@ def rebase_config( The resolved content travels in a ``diff`` envelope mirroring the shape ``get_config_diff`` returns each side in, so a resolved diff side posts back nearly 1:1. Rebase REPLACES the configuration rather - than patching it, so every field the backend fills in on a missing - key is required here, not optional: ``name`` and ``rows`` because - the backend rejects the request without them (``rows=[]`` - legitimately deletes all rows), ``configuration`` and ``is_disabled`` - because it substitutes ``{}`` and ``false`` -- omitting those two - wipes the configuration body and re-enables a disabled config, which - a caller reading "optional" would not expect. To resolve a conflict - by DELETING the config, use ``rebase_config_delete`` -- the two - rebase kinds are separate methods on purpose, so no illegal + than patching it: server-side, ``name`` / ``description`` / + ``configuration`` / ``isDisabled`` are "the complete 3-way diff + result" and "fully replace" the resolved version's body + (``ConfigurationRebaseService``). So all four are required here, not + optional -- an omitted key is not "leave unchanged" but "take the + server-side default", which for ``configuration`` is ``{}``, for + ``isDisabled`` is ``false`` and for ``description`` is null. ``rows`` + is required too, because the backend rejects a keep rebase without + it (``rows=[]`` legitimately deletes all rows). Only + ``change_description`` is genuinely optional: it is not part of the + replaced body, and null selects a default rebase message. To resolve + a conflict by DELETING the config, use ``rebase_config_delete`` -- + the two rebase kinds are separate methods on purpose, so no illegal combination is expressible (RFC, D6). ``branch_id`` is required with no production fallback (see @@ -734,10 +738,13 @@ def rebase_config( the backend substitutes ``false``, re-enabling a config that was disabled. Not the tri-state of ``update_config`` (RFC, D1) -- that method patches, this one replaces. - description: Resolved description. ``None`` omits the key -- - which costs no expressiveness: server-side an explicit JSON - null and an absent key are indistinguishable (``isset`` - mapping). + description: Resolved description. Required, but ``None`` is a + legitimate resolved value -- it means the rebased config + ends up with no description. ``None`` omits the key rather + than sending an explicit null, which costs no expressiveness + because server-side the two are indistinguishable (``isset`` + mapping). It has no default precisely because that default + would silently drop an existing description. change_description: Change log message; when ``None``/omitted the backend uses a default rebase message. diff --git a/tests/test_merge_request_client.py b/tests/test_merge_request_client.py index 633e96b0..2ef9a03b 100644 --- a/tests/test_merge_request_client.py +++ b/tests/test_merge_request_client.py @@ -347,7 +347,7 @@ def test_keep_rebase_builds_the_envelope(self, client, httpx_mock) -> None: def test_keep_rebase_omits_unset_optionals_and_sends_empty_rows( self, client, httpx_mock ) -> None: - """description/change_description are omitted; rows=[] is sent (it deletes all rows).""" + """description=None and an unset change_description are omitted; rows=[] is sent.""" httpx_mock.add_response(url=self.REBASE_URL, json={}) client.rebase_config( @@ -359,6 +359,7 @@ def test_keep_rebase_omits_unset_optionals_and_sends_empty_rows( rows=[], configuration={}, is_disabled=False, + description=None, ) body = _sent_json(httpx_mock.get_requests()[0]) @@ -372,7 +373,7 @@ def test_keep_rebase_omits_unset_optionals_and_sends_empty_rows( def test_keep_rebase_always_sends_configuration_and_is_disabled( self, client, httpx_mock ) -> None: - """Rebase REPLACES, so neither field may be left to a server-side default. + """Rebase REPLACES, so no body field may be left to a server-side default. A missing ``diff.configuration`` is substituted with ``{}`` and a missing ``diff.isDisabled`` with ``false``, so omitting either would wipe the @@ -390,18 +391,34 @@ def test_keep_rebase_always_sends_configuration_and_is_disabled( rows=[], configuration={"parameters": {"keep": "me"}}, is_disabled=True, + description="kept", ) diff = _sent_json(httpx_mock.get_requests()[0])["diff"] assert diff["configuration"] == {"parameters": {"keep": "me"}} assert diff["isDisabled"] is True + assert diff["description"] == "kept" + + def test_keep_rebase_requires_every_replaced_body_field(self, client) -> None: + """Omitting any replaced body field is a TypeError, not silent loss on the wire. - def test_keep_rebase_requires_configuration_and_is_disabled(self, client) -> None: - """Omitting either is a TypeError at the call, not silent data loss on the wire.""" - with pytest.raises(TypeError): - client.rebase_config( - "keboola.ex-http", "cfg-1", branch_id=123, version=7, name="N", rows=[] - ) + ``name`` / ``description`` / ``configuration`` / ``isDisabled`` are the + tuple the backend calls "the complete 3-way diff result" and fully + replaces the resolved version's body with, so none of them may carry a + default here. ``change_description`` is not part of that tuple and stays + optional -- omitting it selects a default rebase message. + """ + required = { + "name": "N", + "rows": [], + "configuration": {}, + "is_disabled": False, + "description": None, + } + for omitted in required: + kwargs = {k: v for k, v in required.items() if k != omitted} + with pytest.raises(TypeError): + client.rebase_config("keboola.ex-http", "cfg-1", branch_id=123, version=7, **kwargs) def test_delete_rebase_sends_empty_diff_object(self, client, httpx_mock) -> None: """Delete resolution is exactly {"version": N, "diff": {}} -- diff a JSON object."""