Skip to content

Commit b47cb58

Browse files
kingpanther13claude
andcommitted
fix(flows): keep unnamed fields when editing an existing config entry
An options, reconfigure or subentry-reconfigure step arrives pre-filled by Home Assistant's add_suggested_values_to_schema, and saving the UI form posts every box back. The flow walker submitted only the keys the caller named, so voluptuous filled each omitted vol.Optional(k, default=STATIC) with its static default and dropped every no-default optional outright: a one-field patch through ha_set_integration(entry_id=..., config=...) silently reset the rest of the entry. Repro on core workday, where config={"days_offset": 3} reset the workday/exclude lists and wiped province, returning success with no warnings. Thread keep_current_values through the two flow walkers and the form-step consumption. Under it, a declared field the caller named no key for is submitted with the value the step itself carries (suggestion, else a constant's only legal value), including leaves inside sections the caller never named, since the section is a box on the same form. A bare "default" still means omission, exactly as it does for the UI's own form. An explicit null is the opposite request and is honoured as a clear: consumed, then left out of the payload. Backfilled values are the step's data, so they neither count towards the "consumed at least one caller key" guard nor satisfy reconfigure's "consumed EVERY key" one, and a value carrying a redaction sentinel is never written back. The flag is set by update_config_entry_options (ha_set_integration options mode and helper updates), the official reconfigure flow, and the subentry reconfigure branch. Create flows - add integration, create helper, create subentry - are unchanged: there is no stored value to preserve and materializing a field would invent data. Fixes #2254 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012d29UJTiH4Uy2Pm37SBPtr
1 parent 858d09e commit b47cb58

7 files changed

Lines changed: 817 additions & 20 deletions

src/ha_mcp/tools/config_entry_flow.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,10 @@ async def set_config_subentry(
168168
169169
The reconfigure branch fails when the flow leaves any supplied config key
170170
unconsumed, where it previously returned success plus a warning — see
171-
:func:`_handle_config_subentry_flow_steps` for why. The create branch is
172-
unchanged.
171+
:func:`_handle_config_subentry_flow_steps` for why. It also walks with
172+
``keep_current_values`` (issue #2254), so a partial patch keeps the
173+
subentry fields it does not name instead of resetting them. The create
174+
branch is unchanged on both counts.
173175
"""
174176
_reject_redaction_sentinels(config_dict)
175177
flow_result = await client.start_config_subentry_flow(
@@ -205,6 +207,7 @@ async def set_config_subentry(
205207
flow_result,
206208
config_dict,
207209
is_reconfigure=subentry_id is not None,
210+
keep_current_values=subentry_id is not None,
208211
)
209212
except asyncio.CancelledError:
210213
await _abort_subentry_flow_best_effort(client, flow_id)
@@ -289,6 +292,15 @@ async def update_config_entry_options(
289292
``ha_set_integration`` path passes ``None`` to accept any domain). Starts
290293
an options flow, walks the flow steps, and returns the result. Aborts the
291294
flow on error. ``noun`` only affects response wording.
295+
296+
This edits an existing entry, so the walk runs with
297+
``keep_current_values``: every field an options step declares that
298+
``config_dict`` does not name is submitted with the value the step itself
299+
carries, exactly as the HA UI's "Configure" dialog posts back the boxes
300+
nobody touched. Before issue #2254 those keys were dropped and voluptuous
301+
substituted each field's static default, so a one-key patch silently reset
302+
the rest of the entry's options. A key the caller sets to ``None`` is the
303+
opposite request and is honoured as a clear.
292304
"""
293305
_reject_redaction_sentinels(config_dict)
294306
config_entry = await client.get_config_entry(entry_id)
@@ -332,6 +344,7 @@ async def update_config_entry_options(
332344
config_dict,
333345
submit_fn=client.submit_options_flow_step,
334346
helper_type=expected_domain,
347+
keep_current_values=True,
335348
)
336349
except Exception:
337350
try:

src/ha_mcp/tools/config_entry_flow_form.py

Lines changed: 93 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
entry points) when that module crossed the ~1000-line split threshold. Turns a
55
step's serialized ``data_schema`` plus the caller's config dict into the payload
66
to submit, and tracks what was consumed so a later step redeclaring a field can
7-
be filled. Imports the menu selection keys from ``config_entry_flow_menu``
8-
(never submitted as form data); ``config_entry_flow_walker`` imports from here.
7+
be filled. Flows that edit an existing object pass ``keep_current_values``,
8+
so a field the caller left out goes back as the step presented it instead of
9+
being dropped (issue #2254). Imports the menu selection keys from
10+
``config_entry_flow_menu`` (never submitted as form data);
11+
``config_entry_flow_walker`` imports from here.
912
"""
1013

1114
import copy
@@ -15,6 +18,7 @@
1518
from typing import Any
1619

1720
from ..errors import ErrorCode, create_error_response
21+
from ..redaction import carries_sentinel
1822
from .config_entry_flow_menu import _MENU_SELECTION_KEY_ORDER
1923
from .helpers import raise_tool_error
2024

@@ -245,6 +249,7 @@ def _consume_section_schema(
245249
*,
246250
allow_reuse: bool = True,
247251
explicit_source: bool = False,
252+
keep_current_values: bool = False,
248253
) -> dict[str, Any]:
249254
"""Consume config values for a nested flow section.
250255
@@ -272,6 +277,7 @@ def _consume_section_schema(
272277
reuse_state,
273278
allow_reuse=allow_reuse,
274279
explicit_source=True,
280+
keep_current_values=keep_current_values,
275281
)
276282
)
277283
_record_ignored_section_keys(
@@ -290,6 +296,7 @@ def _consume_section_schema(
290296
reuse_state,
291297
allow_reuse=allow_reuse,
292298
explicit_source=explicit_source,
299+
keep_current_values=keep_current_values,
293300
)
294301
)
295302
return nested_data
@@ -363,12 +370,45 @@ def _step_owned_submission_value(field: dict[str, Any]) -> Any:
363370
return _MISSING_DEFAULT
364371

365372

373+
def _is_redacted_value(value: Any) -> bool:
374+
"""True when a step-owned value is, or contains, a redaction sentinel.
375+
376+
``redact_secrets`` rewrites a deep copy of a schema for error contexts, so
377+
the live step a walk submits against should never carry a sentinel.
378+
Backfill is the one place that turns schema data back into submitted data,
379+
though, so the check is made anyway: writing ``<redacted: set>`` into a
380+
password field would replace a working secret with a placeholder.
381+
"""
382+
if isinstance(value, list):
383+
return any(carries_sentinel(item) for item in value)
384+
return carries_sentinel(value)
385+
386+
387+
def _current_value_backfill(field: dict[str, Any]) -> tuple[Any, bool]:
388+
"""Resubmit what an edit-mode step carries for a field the caller left out.
389+
390+
Options, reconfigure and subentry-reconfigure steps arrive pre-filled by
391+
Home Assistant's ``add_suggested_values_to_schema``, and the UI's save
392+
posts every box back, so a field nobody named has to be submitted as the
393+
step presented it or the save rewrites it (issue #2254). Returns
394+
``_NO_SUBMISSION`` when the step supplies no value of its own: a bare
395+
``"default"`` is the static schema value, which voluptuous fills in for an
396+
omitted key exactly as it does for the UI's own form.
397+
"""
398+
step_owned = _step_owned_submission_value(field)
399+
if step_owned is _MISSING_DEFAULT or _is_redacted_value(step_owned):
400+
return _NO_SUBMISSION
401+
return step_owned, False
402+
403+
366404
def _redeclared_field_submission(
367405
field: dict[str, Any],
368406
name: str,
369407
path_prefix: str,
370408
reuse_state: _ReuseState | None,
371409
allow_reuse: bool,
410+
*,
411+
keep_current_values: bool = False,
372412
) -> tuple[Any, bool]:
373413
"""Decide what to submit for a declared field the caller named no key for here.
374414
@@ -378,9 +418,13 @@ def _redeclared_field_submission(
378418
and the caller's value for this step outranks anything injected. Otherwise,
379419
in order:
380420
381-
1. The field is not required: omit it. Nothing is ever injected into an
382-
optional field, or into a section that is neither required nor named by
383-
the caller (``allow_reuse``) — materializing either would invent data.
421+
1. The field is not required, or reuse is barred for this site because the
422+
caller named neither it nor the section holding it (``allow_reuse``).
423+
Under ``keep_current_values`` the step's own value still goes back, per
424+
:func:`_current_value_backfill` — it is the step's data, not the
425+
caller's, so barring reuse does not bar it, and a section the backfill
426+
itself materializes has to carry what it requires. Otherwise omit:
427+
injecting into either on a create flow would invent data.
384428
2. The step's own schema supplies a value — a suggestion or a constant's
385429
only legal value, per :func:`_step_owned_submission_value`: submit that.
386430
It is schema data rather than a caller key, so it is neither marked
@@ -397,17 +441,20 @@ def _redeclared_field_submission(
397441
spend the one write allowed per (step, path).
398442
399443
Mutates ``reuse_state`` on the fourth branch. Menu selection keys never
400-
reach here. Motivating regression (issue #2057): an options flow —
401-
LocalTuya's — that declares the same field on an early step and again on a
402-
later one.
444+
reach here. Motivating regressions: issue #2057, an options flow
445+
(LocalTuya's) that declares the same field on an early step and again on a
446+
later one; issue #2254, a one-field options patch that reset every field
447+
the caller did not name back to its static schema default.
403448
"""
404-
if reuse_state is None or not allow_reuse:
449+
if reuse_state is None:
405450
return _NO_SUBMISSION
406451
dotted = _section_path(path_prefix, name)
407452
if dotted in reuse_state.filled:
408453
return _NO_SUBMISSION
409-
if not field.get("required"):
410-
return _NO_SUBMISSION
454+
if not allow_reuse or not field.get("required"):
455+
if not keep_current_values:
456+
return _NO_SUBMISSION
457+
return _current_value_backfill(field)
411458
step_owned = _step_owned_submission_value(field)
412459
if step_owned is not _MISSING_DEFAULT:
413460
return step_owned, False
@@ -432,6 +479,7 @@ def _consume_leaf_field(
432479
*,
433480
allow_reuse: bool = True,
434481
explicit_source: bool = False,
482+
keep_current_values: bool = False,
435483
) -> None:
436484
"""Fill ``name`` in ``form_data`` from the caller's config or from the step itself.
437485
@@ -441,17 +489,30 @@ def _consume_leaf_field(
441489
the flat caller dict. With no key to pop,
442490
:func:`_redeclared_field_submission` chooses between omitting the field,
443491
submitting the step's own value, and resubmitting the recorded one.
492+
493+
Under ``keep_current_values`` an explicit ``None`` on a non-required field
494+
is a clear rather than a value: the key is consumed and recorded, but left
495+
out of the payload, because omission is how the UI's form clears a box
496+
(issue #2254). A required field keeps submitting ``None`` verbatim — Home
497+
Assistant, not this walker, decides what a required null means.
444498
"""
445499
if name in remaining_config:
446500
value = remaining_config.pop(name)
447-
form_data[name] = value
501+
clearing = keep_current_values and value is None and not field.get("required")
502+
if not clearing:
503+
form_data[name] = value
448504
_mark_consumed(consumed_config_keys, path_prefix, name)
449505
if reuse_state is not None:
450506
reuse_state.record(path_prefix, name, value, scoped_only=explicit_source)
451507
return
452508

453509
value, from_caller = _redeclared_field_submission(
454-
field, name, path_prefix, reuse_state, allow_reuse
510+
field,
511+
name,
512+
path_prefix,
513+
reuse_state,
514+
allow_reuse,
515+
keep_current_values=keep_current_values,
455516
)
456517
if value is _MISSING_DEFAULT:
457518
return
@@ -471,6 +532,7 @@ def _consume_declared_section(
471532
*,
472533
allow_reuse: bool,
473534
explicit_source: bool,
535+
keep_current_values: bool,
474536
) -> None:
475537
"""Merge one section field's data into ``form_data``.
476538
@@ -501,6 +563,7 @@ def _consume_declared_section(
501563
allow_reuse=allow_reuse
502564
and (bool(field.get("required")) or explicit_section is not None),
503565
explicit_source=explicit_source,
566+
keep_current_values=keep_current_values,
504567
)
505568
if not nested_data:
506569
return
@@ -520,6 +583,7 @@ def _consume_form_schema(
520583
*,
521584
allow_reuse: bool = True,
522585
explicit_source: bool = False,
586+
keep_current_values: bool = False,
523587
) -> dict[str, Any]:
524588
"""Consume matching config values and shape nested flow sections.
525589
@@ -528,7 +592,8 @@ def _consume_form_schema(
528592
Unknown keys inside explicit section dicts are added to
529593
``ignored_config_keys`` with their dotted section path. A declared field the
530594
caller named no key for is filled per
531-
:func:`_redeclared_field_submission`.
595+
:func:`_redeclared_field_submission`, which ``keep_current_values`` puts
596+
into the edit-mode contract described there.
532597
"""
533598
form_data: dict[str, Any] = {}
534599

@@ -548,6 +613,7 @@ def _consume_form_schema(
548613
reuse_state,
549614
allow_reuse=allow_reuse,
550615
explicit_source=explicit_source,
616+
keep_current_values=keep_current_values,
551617
)
552618
continue
553619

@@ -562,6 +628,7 @@ def _consume_form_schema(
562628
path_prefix,
563629
allow_reuse=allow_reuse,
564630
explicit_source=explicit_source,
631+
keep_current_values=keep_current_values,
565632
)
566633

567634
return form_data
@@ -636,6 +703,8 @@ def _handle_form_step(
636703
ignored_config_keys: set[str] | None = None,
637704
consumed_config_keys: set[str] | None = None,
638705
reuse_state: _ReuseState | None = None,
706+
*,
707+
keep_current_values: bool = False,
639708
) -> dict[str, Any]:
640709
"""Validate a form step and return form data to submit.
641710
@@ -654,6 +723,15 @@ def _handle_form_step(
654723
warning. Nothing is injected into an optional field, or into a section
655724
neither marked required nor named by the caller.
656725
726+
``keep_current_values`` switches that last sentence off for the flows that
727+
edit an existing object — options, reconfigure, subentry reconfigure. Their
728+
steps arrive pre-filled with the stored values and the UI's save posts all
729+
of them back, so a declared field the caller left out is submitted with the
730+
step's own value wherever the step supplies one (issue #2254); a field the
731+
caller set to ``None`` is instead consumed and omitted, which is the UI's
732+
clear gesture. It takes a ``reuse_state`` to work: the record is what keeps
733+
a backfill off a path the caller already filled in this same step.
734+
657735
When ``data_schema`` is absent (HA didn't tell us field names), falls
658736
back to legacy behaviour: submit all non-menu keys and clear them. This
659737
keeps single-step flows working when HA omits the schema.
@@ -691,4 +769,5 @@ def _handle_form_step(
691769
consumed_config_keys,
692770
"",
693771
reuse_state,
772+
keep_current_values=keep_current_values,
694773
)

src/ha_mcp/tools/config_entry_flow_walker.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,12 +685,14 @@ def _flow_form_payload(
685685
remaining_config: dict[str, Any],
686686
ignored_config_keys: set[str],
687687
reuse_state: _ReuseState,
688+
keep_current_values: bool,
688689
) -> tuple[dict[str, Any], bool]:
689690
"""Build one generic flow form payload.
690691
691692
Returns the payload and whether the step consumed at least one caller key
692693
— not which ones; ``remaining_config`` and ``ignored_config_keys`` carry
693-
that.
694+
that. A value the step backfills under ``keep_current_values`` is the
695+
step's own data, so it never counts as a consumed caller key.
694696
"""
695697
consumed_form_keys: set[str] = set()
696698
form_data = _auto_confirm_form_payload(current_step)
@@ -702,6 +704,7 @@ def _flow_form_payload(
702704
ignored_config_keys,
703705
consumed_form_keys,
704706
reuse_state,
707+
keep_current_values=keep_current_values,
705708
)
706709
return form_data, bool(consumed_form_keys)
707710

@@ -757,6 +760,7 @@ async def _handle_flow_steps(
757760
helper_type: str | None = None,
758761
*,
759762
is_reconfigure: bool = False,
763+
keep_current_values: bool = False,
760764
) -> dict[str, Any]:
761765
"""Walk a multi-step config flow handling menu and form steps.
762766
@@ -793,6 +797,18 @@ async def _handle_flow_steps(
793797
the caller can react. Under ``is_reconfigure`` no schema is
794798
fetched (the live step already carries the right one) and the
795799
value is used only to name the integration in error prose.
800+
keep_current_values: Whether this flow edits an existing object
801+
(options, reconfigure, subentry reconfigure) rather than creating
802+
one. Its steps arrive pre-filled with the stored values and the HA
803+
UI's save posts every box back, so a declared field the caller
804+
named no key for is submitted with the step's own value instead of
805+
being dropped — which is what stopped a one-field options patch
806+
from resetting everything it did not mention (issue #2254). A
807+
field the caller explicitly set to ``None`` is consumed and
808+
omitted instead: omission is the UI's clear gesture. Backfilled
809+
values are the step's data, so they neither count towards the
810+
"consumed at least one caller key" test below nor satisfy the
811+
reconfigure "consumed EVERY key" one.
796812
is_reconfigure: Whether this is the official reconfigure flow — the
797813
same mode HA uses for reauth, so both ``reconfigure_successful``
798814
and ``reauth_successful`` count as its success aborts. In this
@@ -893,6 +909,7 @@ async def _handle_flow_steps(
893909
remaining_config=remaining_config,
894910
ignored_config_keys=ignored_config_keys,
895911
reuse_state=reuse_state,
912+
keep_current_values=keep_current_values,
896913
)
897914
if consumed_any:
898915
any_form_key_consumed = True
@@ -1040,6 +1057,7 @@ async def _handle_config_subentry_flow_steps(
10401057
config: dict[str, Any],
10411058
*,
10421059
is_reconfigure: bool,
1060+
keep_current_values: bool = False,
10431061
) -> dict[str, Any]:
10441062
"""Walk a config subentry flow and accept HA's reconfigure-success abort.
10451063
@@ -1057,6 +1075,11 @@ async def _handle_config_subentry_flow_steps(
10571075
read as "done" to an agent. This is a behaviour change for
10581076
``ha_config_set_helper(helper_type="config_subentry", subentry_id=...)``
10591077
callers who previously relied on the warning.
1078+
1079+
``keep_current_values`` carries the same edit-mode contract as in
1080+
:func:`_handle_flow_steps` (issue #2254): reconfiguring a subentry
1081+
resubmits the step's own value for every declared field the caller named
1082+
no key for, so a partial patch stops wiping the rest of the subentry.
10601083
"""
10611084
remaining_config = dict(config)
10621085
current_step = initial_step
@@ -1124,6 +1147,7 @@ async def _handle_config_subentry_flow_steps(
11241147
remaining_config,
11251148
ignored_config_keys,
11261149
reuse_state=reuse_state,
1150+
keep_current_values=keep_current_values,
11271151
)
11281152
logger.debug(
11291153
"Config subentry flow step %s: form submit (step_id=%s, keys=%s)",

0 commit comments

Comments
 (0)