diff --git a/changelog/+store-merge-behaviour.changed.md b/changelog/+store-merge-behaviour.changed.md new file mode 100644 index 000000000..ecdd6318f --- /dev/null +++ b/changelog/+store-merge-behaviour.changed.md @@ -0,0 +1 @@ +The client store merge fix comes with four observable behaviour changes. First, the object returned by `get`, `filters` or `all` is a per-query snapshot and is no longer the same Python object as the store entry: `client.get(id) is client.store.get(id)` was previously true and now is false, although the two still compare equal (`==`). Second, the store hands out living objects: `store.get()` returns the same object across calls, and a later query that re-fetches the node updates that object in place. Third, the store is now timestamp-coherent per branch: the first population stamps the branch cache as live or as one `at` point in time, queries at the same timestamp use the store normally (a fully historical script gets complete store functionality), and a query at a mismatching timestamp skips the store with a warning instead of silently blending or overwriting data from a different point in time (which is what pre-1.23.0 versions did). Fourth, a successful `save()`, `create()` or `update()` now resets the node's in-memory mutation tracking: the persisted values count as server state, so later fetches of the same node refresh those fields in the store instead of treating the long-saved edit as a pending local change forever. diff --git a/changelog/413.fixed.md b/changelog/413.fixed.md new file mode 100644 index 000000000..c3f2bc18f --- /dev/null +++ b/changelog/413.fixed.md @@ -0,0 +1 @@ +Fixed the client store silently losing data when the same node was fetched more than once. Previously the latest query fully replaced the stored node, so a shallow re-fetch (for example a node returned as a related node of another query) dropped attributes and relationships that an earlier, deeper query had loaded, and left duplicate entries in the store. The store now keeps one object per node UUID and merges each fetch into it field by field: fields carried by the new fetch overwrite the stored ones (even to empty or `None`), fields the fetch did not request keep their stored value, and local unsaved edits always win over a re-fetch. The previous replace behaviour remains available per query and per `store.set()` call with `merge=False`, or globally with the new `store_merge` configuration option (`INFRAHUB_STORE_MERGE`). diff --git a/dev/specs/ihs-138-store-merge/decisions.md b/dev/specs/ihs-138-store-merge/decisions.md new file mode 100644 index 000000000..46afc93f3 --- /dev/null +++ b/dev/specs/ihs-138-store-merge/decisions.md @@ -0,0 +1,311 @@ +# IHS-138 - Decision sign-off sheet + +One row per open decision from [`plan.md`](./plan.md) section 11. Each has a +recommendation and the reasoning; mark **Accept** / **Override** and note any +change. Nothing in Stage 1 (presence flags) depends on these - they gate Stage 2 +onward. + +Already settled (recorded in the plan, listed here for completeness): + +- **Version:** SDK 1.23.0, semver minor, shipped as a bug fix with a required + opt-out and a required behaviour-change note. +- **Where the option lives:** `Config` default -> store default -> per-call + `merge=` override. + +--- + +## D1 - Returned object vs stored object (C2) + +**Question.** After a re-fetch, the store keeps the merged canonical object. What +does a query method (`get` / `all` / `filters`) return? + +- **(a) Return the freshly built per-query object** (diverges from the store copy). +- **(b) Return the canonical merged store object.** + +**Recommendation: (a).** + +**Why.** + +- Minor-version safe: what `client.get()` returns is byte-for-byte what it returns + today (exactly the fields this query asked for). Only `store.get()` gets richer. + Option (b) changes every query's return value to carry data the caller did not + request - much harder to call non-breaking. +- It shrinks the D2 blast radius: the only "living" object is the internal store + copy; returned objects are stable per-query snapshots. +- Less code: (b) needs the query path to map incoming -> canonical and swap + returned references. +- Clean mental model: "the return value is this query's result; the store + accumulates across queries." + +**Cost / what to verify.** `client.get(id) is client.store.get(id)` stops being +true (it holds today). Value equality `==` still holds (id-based, `node.py:720`). +Add a `save()` test: `save()` calls `store.set(self)`, so `self` merges into the +canonical copy while the caller keeps `self` - confirm no field is revived that +the save did not touch. + +**Decision: ACCEPTED (a).** Query methods return the per-query object; the store +holds the merged canonical copy. Guiding rule agreed with the reporter: *"queries +hand you what you asked for; the store remembers the union of everything it has +seen."* + +--- + +## D2 - In-place mutation / living objects (C3) + +**Question.** Merge mutates the existing stored object, so a `store.get()` +reference can change under the caller when an unrelated query re-fetches that node. +Is that the intended model? + +**Recommendation: accept it, documented, with local-edit protection.** + +**Why.** + +- It is the correct property for a cache: every store reference points at the one + canonical object, which is always current. The alternative (merge into a new + object, replace the entry) leaves previously handed-out `store.get()` references + stale and diverging from the store - worse. +- Under D1(a) the surprise is confined to code that deliberately holds a + `store.get()` result across queries; plain query return values never mutate. +- Protect unsaved local edits: merge must skip fields flagged + `value_has_been_mutated` (`attribute.py:100`) / `_peer_has_been_mutated` + (`related_node.py:74`), so re-querying never discards in-memory changes. + +**Cost.** New, documented behaviour. Covered by the section 6 "behaviour change +must be clear" requirement. + +**Decision: ACCEPTED.** The store keeps one always-current, merged object per node; +`store.get()` references reflect later fetches. Unsaved local edits are protected. + +--- + +## D3 - Default for the public `store.set()` (C5) + +**Question.** The query-population path defaults to merge (the fix). What should the +public `store.set(node=...)` default to? + +- **(a) `merge=False` (replace) by default**, opt into merge. +- **(b) `merge=True` (merge) by default**, same as queries. + +**Recommendation: (a) - public `set()` defaults to replace.** + +**Why.** + +- "set" reads as an imperative "make the store hold this," like `dict[k] = v`. + Merge is the enrichment behaviour the *query* path needs, not what an explicit + `set` implies. +- Preserves the documented contract ("store this object", `store.mdx:147`). That + example sets a fresh object under a custom key, usually with no prior entry, so + replace and merge are identical there - no example breaks. +- The IHS-138 bug lives in the query path, not in manual `set()`. Keep the default + change where the bug is; leave the explicit call predictable. +- A user who wants enrichment passes `merge=True`. + +**Note.** This means the default differs by entry point: queries follow +`Config.store_merge` (default merge); `set()` defaults to replace regardless. That +is intentional and should be stated in the `set()` docstring. + +**Decision: OVERRIDE -> (b) merge by default, uniformly.** The recommendation above +was not taken. Agreed model: *anything that lands in the store gets merged*, with no +special-casing by entry point - `store.set()` merges just like the query path. This +is the simpler, single-rule mental model. Consequence: **replace is opt-in only** - +the sole way to store a node verbatim / drop previously cached data is +`store.set(node, merge=False)` or the `store_merge=False` config opt-out. The public +`store.set()` docstring must state that it merges by default and how to force +replace. + +--- + +## D4 - Config option name and type + +**Question.** How is the global default expressed on `Config`? + +- **(a) Bool `store_merge` with a full `description`** (drafted in plan section 3). +- **(b) A clearer bool name** (e.g. `merge_store_results`). +- **(c) Enum `store_update_mode: StoreUpdateMode` (`MERGE` / `REPLACE`)**, matching + existing `Config` enums (`InfrahubClientMode`, `RecorderType`). + +**Recommendation: (a) - bool `store_merge` carried by its description.** + +**Why.** + +- Type-consistency with the per-call `merge: bool` argument. An enum on `Config` + plus a bool per call is a mismatch; keeping both bool is simplest to reason about. +- The clarity the user asked for is delivered by the `description` (which states + both behaviours and the default), per the section 6 hard requirement - so the + terse name is acceptable. +- Enum is the fallback if call-site self-documentation is valued over type + consistency, and we are willing to accept the enum/bool split (or make the + per-call arg accept the enum too). + +**Decision: ACCEPTED (a).** Bool `store_merge` with the full description. Revisit only +if the description proves insufficient in review. + +--- + +## D5 - Internal presence-flag naming + +**Question.** Name for the new "present in this response" flag on `Attribute` and +`RelatedNode`. + +**Recommendation: `is_fetched`, with a uniform accessor across all three field +types.** + +**Why.** + +- `RelatedNode` already has `initialized` meaning "has a peer" + (`related_node.py:189`) - a *different* concept - so the new flag there cannot be + called `initialized` without a confusing collision. +- `RelationshipManager.initialized` already means "was fetched." Expose + `is_fetched` on it as a thin alias of `initialized`, add a real `is_fetched` + flag to `Attribute` and `RelatedNode`, and the merge code can branch uniformly + on `field.is_fetched` regardless of field type. +- Internal only (not public API), so low stakes - but the uniform accessor removes + per-type special-casing in the merge. + +**Decision: ACCEPTED.** `is_fetched` on `Attribute` and `RelatedNode`; uniform +`is_fetched` accessor on `RelationshipManager` aliasing its existing `initialized`. + +--- + +## D6 - Scope of `merge=False` (replace) + +**Question.** When a node is stored with `merge=False`, does it also drop that +node's relationship peers from the store, or only replace the node's own entry? + +**Recommendation: only replace the node's own entry.** + +**Why.** + +- Peers are independent store entries that other nodes may reference; cascading + eviction could break unrelated references. +- Peers fetched in the same query go through their own `set()` calls and follow the + same `merge` flag independently. No special cascade needed. + +**Decision: ACCEPTED.** `merge=False` replaces the node's own entry only; peers are +handled independently. + +--- + +## D7 - Mutation-flag lifecycle (added during implementation, 2026-07-04) + +**Question.** D2 protects "unsaved local edits" via the mutation flags +(`value_has_been_mutated`, `_peer_has_been_mutated`, `_has_update`). The code review +showed those flags were sticky - never reset after a successful save - so a saved +edit would block merge refreshes of that field forever, and the store would serve +the stale saved value even after the server changed. What is the flag lifecycle? + +**Decision: reset on save, propagate on merge.** + +- A successful `create()`/`update()`/`save()` resets all three flag types + (`_reset_mutation_tracking()` at the end of `_process_mutation_result`): the + persisted values count as server state from then on. +- The merge propagates the markers from an unsaved incoming copy instead of + clearing them, so an unsaved edit merged into the store (manual `store.set`) + keeps its will-be-saved status and is still sent by the store copy's next save. + +**Why.** The two halves depend on each other: propagation is only safe because +saving resets the flags (a freshly saved copy no longer reads as "pending"), and +resetting is what makes "local edits win" mean *unsaved* edits, which is what D2 +intended. Consequence, listed in the changelog: mutation tracking now resets after +a successful mutation, so a second `update()` no longer re-sends relationship edits +that were already persisted. + +--- + +## D8 - Same-peer identity fields gate on presence (refines the merge rule) + +**Question.** The plan said cardinality-one peer identity "always refreshes" when +the relationship was fetched. A payload carrying only `node { id }` for the *same* +peer would then null previously fetched `hfid`/`display_label`/`typename`/`kind` +and drop the cached `_peer` - the silent-loss class this feature exists to prevent +(reachable via `from_graphql` on custom payloads + `store.set`, not via +SDK-generated queries). + +**Decision: split by peer change.** A changed peer (different id, including cleared +to none) takes the full incoming identity, so moves and move-to-root behave as +specified. The same peer only refreshes the descriptive identity fields the +incoming payload actually carried - consistent with how attribute properties and +edge properties already merge. + +--- + +## D9 - Timestamp-coherent store (supersedes grill item 3's mechanism, 2026-07-04) + +**Question.** Grill item 3 decided historical (`at`) reads must not blend into the +live cache. The first implementation made `at` queries skip the store by default, +with an explicit `populate_store=True` opt-in that forced replace. That required +widening `populate_store` from `bool = True` to `bool | None = None` (to detect an +explicit `True`), left `at` + `prefetch_relationships` without `.peer` resolution, +and - because the opt-in forced replace - quietly revived the IHS-138 bug for +fully-historical scripts. Was the signature change actually needed? + +**Decision: no - make the store timestamp-coherent instead.** The store holds one +timestamp context per branch: live data, or one `at` instant, stamped by the first +population. Same-context queries use the store normally, so a script running all +its queries at one `at` gets full store functionality (merge, `.peer`, hfid +lookups). A mismatching population (live vs historical, or two different instants) +emits a warning and skips the store for that query; the query itself still returns +its results. + +**Why.** + +- The incoherence grill item 3 feared comes only from *mixing* timestamps; a + consistent-`at` session is exactly as coherent as a live one and deserves the + same cache behaviour, including the merge semantics this whole feature adds. +- `populate_store: bool = True` is restored - no public signature change, no `None` + sentinel in a boolean parameter. +- Strictly safer than both alternatives considered: pre-1.23 silently overwrote + live entries with historical data; skip-by-default silently degraded historical + scripts. Warn-and-skip makes the mismatch visible without breaking mixed scripts + (which are safer than they were on 1.22, not worse). +- Mismatch policy is warn + skip rather than raise: raising would break scripts + that mix `at` and live queries, which worked (subtly wrong) before, and this + ships as a semver-minor bug fix. +- Documented footgun: recompute a relative timestamp per call and every query + after the first trips the warning - compute `at` once, or use one + `client.clone()` per timestamp (the context is per client store, per branch). + +--- + +## Summary table + +| ID | Decision | Outcome | +| -- | -------- | ------- | +| D1 | Returned vs stored object | Return per-query object; store holds the merged canonical copy | +| D2 | In-place mutation model | Accept; one always-current merged object per node; protect local edits | +| D3 | `store.set()` default | **Merge by default, uniformly** (override); replace is opt-in via `merge=False` | +| D4 | Config option name/type | Bool `store_merge` + full description | +| D5 | Presence-flag name | `is_fetched`, uniform accessor on all three field types | +| D6 | `merge=False` scope | Node entry only; peers handled independently | +| D7 | Mutation-flag lifecycle | Reset on successful save; merge propagates pending markers (2026-07-04) | +| D8 | Same-peer identity fields | Presence-gated for the same peer; full refresh on peer change (2026-07-04) | +| D9 | `at` and the store | Timestamp-coherent per branch; mismatching populations warn + skip (2026-07-04) | + +**Guiding rule:** queries hand you what you asked for; the store remembers the union +of everything it has seen. Replace happens only when explicitly requested +(`merge=False` / `store_merge=False`). + +All decisions are settled and implemented (D1-D6 as planned; D7/D8/D9 added during +the 2026-07-04 code-review hardening - see plan section 13 for the full list of +implementation outcomes, including the performance constraints on `store.set()`). +The remaining external gate is the 1.23.0 pre-release run of the Ansible collection +and `infrahubctl` integration suites. + +## Grill refinements (2026-07-02) + +Stress-testing the plan (see plan section 12) added these, all accepted: + +- **Merge scope** also covers node-level scalars (`display_label`, `typename`) and + merges attributes/properties field-by-field (not object-swap). +- **Kind change is a documented exception to D2:** if `typename` differs for the same + uuid (a `ConvertObjectType` migration), the store entry is *replaced* wholesale, not + mutated in place - merging across schemas is incoherent. +- **`at` (time-travel) queries skip store population by default** (behaviour change + from today; must be in the migration note). Explicit `populate_store=True` + `at` + replaces rather than blends. *Superseded by D9:* the shipped mechanism is a + timestamp-coherent store (one `at` context per branch, warn + skip on mismatch), + which keeps the same goal - never blend timestamps - without the `populate_store` + signature change. +- **Release gate:** Ansible collection + `infrahubctl` integration suites run against + the 1.23.0 pre-release; migration note enumerates the D1 identity change and the + `at` change with before/after. diff --git a/dev/specs/ihs-138-store-merge/plan.md b/dev/specs/ihs-138-store-merge/plan.md new file mode 100644 index 000000000..a8af1d976 --- /dev/null +++ b/dev/specs/ihs-138-store-merge/plan.md @@ -0,0 +1,676 @@ +# IHS-138 - Merge nodes in the client store instead of overwriting + +- **Ticket:** [IHS-138](https://opsmill.atlassian.net/browse/IHS-138) (GitHub [#413](https://github.com/opsmill/infrahub-sdk-python/issues/413)) +- **Priority:** High +- **Affected versions:** observed on SDK 1.12.1 +- **Target version:** SDK 1.23.0 (ships alongside Infrahub 1.11.0) +- **Status:** implemented (2026-07-02); hardened after code review (2026-07-04, section 13) + +> The SDK is versioned independently of Infrahub core. 1.23.0 is a semver *minor* +> bump even though it rides the Infrahub 1.11.0 release. Because it changes +> default store behaviour (see sections 3 and 10), shipping it as a minor depends +> on two things being true: the change is framed as a bug fix, and an explicit +> opt-out exists. Both are required deliverables, not nice-to-haves. + +## 1. Problem + +Querying the same Infrahub node more than once can silently drop information +that was previously held in the client store. + +Reproduction from the ticket: + +```python +client = InfrahubClientSync(address="https://demo.infrahub.app/") + +interface = client.get("InfraInterface", device__name__value="jfk1-edge1", + name__value="Ethernet6", prefetch_relationships=True) +client.store.get(interface.id).device.id # works + +client.all("InfraCircuitEndpoint", prefetch_relationships=True) +client.store.get(interface.id).device.id # AttributeError: 'NoneType' has no attribute 'id' +``` + +The first query stores the interface with its `device` relationship. The second +query re-fetches the same interface as a *related node* of each circuit endpoint, +at a depth that does not include the interface's `device`. After the second query +the device link is gone, and the interface is present in the store twice. + +## 2. Root cause + +The store keys objects by a random per-object id, not by their stable UUID. + +- `infrahub_sdk/node/node.py:110` - every node object gets a fresh + `self._internal_id = generate_short_id()` on construction. +- `infrahub_sdk/store.py:47-60` - `NodeStoreBranch.set()` stores under + `self._objs[node._internal_id]` and points `self._uuids[node.id]` / + `self._hfids[...]` at that random id. +- `infrahub_sdk/client.py:1161-1168` - the query paths unconditionally call + `self.store.set(node=node)` for every node and related node, with no check for + whether the UUID is already present. + +So a second fetch of the same UUID: + +1. creates a new Python object with a new `_internal_id`, +2. leaves a duplicate entry in `_objs` (unbounded growth), +3. overwrites `_uuids[node.id]` to point at the newest, possibly poorer copy. + +`store.get(uuid)` then resolves to the poorer copy, whose relationship was never +initialized -> `AttributeError`. + +## 3. Design + +Replace "last write wins, keyed by a random id" with "merge into the existing +object, keyed by UUID, field by field." A field is only overwritten when the new +fetch actually carried it; fields the new fetch did not request are left intact. + +The decision of whether the new fetch carried a field requires a per-field +"was this present in the response" signal. The state of that signal across the +field types is **not uniform today**, and getting it uniform is the heart of the +fix: + +- **Cardinality-many relationships** (`RelationshipManager`, including the + hierarchical `children` / `ancestors` / `descendants`) already have a correct + signal: `initialized = (data is not None)` (`relationship.py:202`). This genuinely + means "was fetched." Usable as-is. +- **Cardinality-one relationships** (`RelatedNode`, including the hierarchical + `parent` and every normal one-relationship) have a *misleading* signal: + `initialized = bool(self.id) or bool(self.hfid)` (`related_node.py:189`) means + "has a peer," NOT "was fetched." A fetched-but-empty one-relationship (node moved + to root, optional relationship cleared) reports `initialized == False` and is + indistinguishable from "not fetched." Using it as the merge gate would keep a + stale parent after a move-to-root. **We must add a real presence flag to + `RelatedNode`** (set from key-presence in `_init_relationships`) and gate the + merge on that, not on `initialized`. +- **Attributes** have no signal at all. `_init_attributes` (`node.py:269`) builds an + `Attribute` for every schema attribute regardless of the query, and a field absent + from the response collapses to `_value = None` (`attribute.py:88-99`), + indistinguishable from "fetched and genuinely null." We add the flag. + +**Unifying principle:** every field type - attribute, cardinality-one, +cardinality-many, hierarchical - must carry a "present in this response" flag, and +the merge replaces present fields (even to empty / None) and keeps absent ones. +This is what makes moves, clears, and partial fetches all behave the way a user +reads them: *the store reflects the latest server state for whatever you actually +re-fetched, and leaves everything else untouched.* + +### Merge rule (per field) + +```text +for each field on the incoming node: + if field was present in this response (initialized): + if the stored field was locally mutated by the user (unsaved): + keep the local value # local edits win + else: + take the incoming value # refresh, even to None + else: + keep the stored value # not queried -> no fresher info +``` + +- Within a fetched cardinality-many relationship, the member list is *replaced*, + never unioned, so a peer removed on the server is correctly dropped. +- For attributes, merge **field-by-field into the existing `Attribute`**, not by + swapping the whole object (grill 4, 2026-07-02). An attribute carries more than a + value - properties (`source`, `owner`, `is_protected`, `is_visible`) and metadata + (`is_default`, `is_from_profile`). A query can fetch the value without the + properties; a blind object-swap would then null previously-fetched properties - + the same silent-loss bug one level down. Rule: overwrite the value when the + attribute is fetched; overwrite each property/metadata sub-field only when the + re-fetch actually included it, otherwise keep the stored one. The same applies to + relationship properties and `node_metadata`. + +### "Field" means all three relationship buckets *and* node-level scalars + +A node holds relationships in **three** separate containers, and the merge must +iterate all of them or it reintroduces the bug on the ones it skips: + +- `_relationship_cardinality_one_data` (`RelatedNode`) +- `_relationship_cardinality_many_data` (`RelationshipManager`) +- `_hierarchical_data` (parent / children / ancestors / descendants) + +Note `self._relationships` (`node.py:117`) lists **only** `schema.relationships`, +so it does *not* include the hierarchical bucket. Do not drive the merge off +`self._relationships` alone. + +It must **also** cover node-level scalars set directly in `__init__` - `display_label` +and `typename` (`node.py:113-114`) - which live on neither `_attribute_data` nor a +relationship bucket (grill 1, 2026-07-02). They are gated on presence like any other +field: refresh `display_label` when the response carried it, keep it otherwise. +`display_label` is user-visible (it drives `__repr__`, `node.py:298`), so leaving it +stale after a refresh would visibly violate the guiding rule. `id` is exempt (it is +the merge key). `hfid` is derived from attributes, so it refreshes once attributes +merge. + +### Kind change -> full replace, not merge + +If the incoming node's concrete kind (`get_kind()`, == GraphQL `__typename`) differs +from the stored node's kind for the same uuid, the node was migrated to another kind +(the `ConvertObjectType` mutation, `convert_object_type.py`, converts in place and +preserves the uuid). Merging field-by-field across two schemas is incoherent - it +would leave phantom attributes/relationships from the old kind. In that case +**discard the stored entry and store the incoming node wholesale**, regardless of the +`merge` setting (grill 1b, 2026-07-02). This is the one case where the store object +identity is replaced even in merge mode - a deliberate exception to "mutate in place" +(D2). Purge the old kind's `_hfids` entries on replace so the index does not point at +the discarded object. Misfire risk is low: GraphQL `__typename` is always the concrete +kind, so a peer fetched via a generic relationship still reports its concrete kind. + +### Object identity + +Merge mutates the *existing* stored object and keeps its `_internal_id`. This +removes the duplicate `_objs` entries and keeps `store.get(id)` returning a stable +object across repeated fetches. Equality is unaffected: `InfrahubNode.__eq__` / +`__hash__` are id-based (`node.py:717-723`), so `node == client.store.get(node.id)` +remains true. + +### Decisions deliberately taken + +1. **Local edits win over a re-fetch.** Honour `value_has_been_mutated` + (`attribute.py:100`) and `_peer_has_been_mutated` so re-collecting the same + info never clobbers unsaved in-memory changes. Least surprising. +2. **Merge is the default.** Re-collecting the same information is therefore + idempotent and only ever refreshes or adds knowledge. +3. **`fetched-None` counts as fetched.** The presence flag means "present in the + response," not "value is non-null," so a genuine server-side clear still + overwrites a stale cached value. + +### Known limitation -> escape hatch + +Merge can never *forget* a field that was cached earlier but not re-fetched (for +example, to observe that a relationship no longer exists when your refresh query +did not select it). This is the one case where a full replace is wanted. Provide +an explicit opt-out rather than guessing. It lives in three layers, mirroring how +`populate_store` / `pagination_size` already work (config default -> per-call +override): + +1. **Implementation:** `NodeStoreBranch.set()` / `NodeStoreBase._set()` in + `store.py` - the only place that writes `_objs`. +2. **Per-call control:** a `merge` argument on `NodeStore.set()` / + `NodeStoreSync.set()` (`store.py:344`/`432`) and on the query methods `get` / + `all` / `filters` (and sync variants), placed next to the existing + `populate_store` argument. It threads into the `store.set(...)` calls at + `client.py:1164`/`1168`/`2903`/`2907`. +3. **Global default / opt-out:** a field on `Config` (`config.py`), passed into the + store at construction (`client.py:358`, currently + `NodeStore(default_branch=self.default_branch)`). Defaults to merge. + `InfrahubClient(config=Config(