Skip to content

Second install rewrites default-valued nodes: default-suppression makes the stored node differ from the authored file, so the unchanged-skip misses #2271

Closed

Description

@rbuergi

What happens

A second install of the same package rewrites nodes whose content is default-valued — observed on PensionFund/Currency/{CHF,EUR,USD} (3 nodes rewritten, expected 0). It is the deploy-flicker the idempotence pin exists to catch.

Mechanism

Currency.DecimalPlaces carries a C# initializer = 2 (PensionFund/Currency/Source/Currency.cs), and all three instance files author "decimalPlaces": 2.

  1. Once the Currency NodeType compiles, its content type is registered.
  2. The installer therefore materialises the instance as a typed Currency rather than an untyped JsonElement, and re-serialises it.
  3. Default-suppression drops the property whose value equals the type's default.
  4. The stored node now differs from the authored file — so the unchanged-skip misses, and the next install rewrites it.

Year/2024 and every Position/* round-trip clean. The three that churn are exactly the three whose content is default-valued.

Why it is intermittent — and why that matters

It is invisible until the NodeType compiles. That is why the gate reported it inconsistently within one hour tonight:

  • one run: STALE allow entry (now passing — remove it): PensionFund idempotence
  • the very next: GATE FAILED — idempotence: PensionFund — 1 new failure(s)

Both reports are correct. I removed the allow entry on the first and had to restore it on the second — recorded at the entry so the next person does not repeat it. A single green run is not evidence this is gone.

Why it is surfacing now

#2063's discovery fix means the gate now finds PensionFund's 5 NodeTypes where it previously found 0 (the BOM hid them). So the compile happens, step 1 above becomes reachable, and a defect that was always present becomes intermittently visible.

Suggested direction

The write path should compare against what was authored, not against a re-serialisation of the materialised value — or default-suppression must not apply on the install round-trip. Related and possibly the same root: #2266, where the installer logs "the file's authored content could not be re-read — installing the materialised value, which may carry another package's defaults." Both are the installer preferring a materialised value over the authored one.

Currently carried as known debt in .github/samples-gate.allow (PensionFund idempotence).

Activity

  1. added a commit that references this issue on Aug 26, 2026
  2. rbuergi commented on Aug 26, 2026

    @rbuergi
    ContributorAuthor

    Fixed by #2318, merged as 187f91048 — as a duplicate root cause of #2266, and this issue's stated mechanism was wrong.

    The issue (and the matching comment in .github/samples-gate.allow) explained the rewrite like this:

    Currency.DecimalPlaces carries a C# initializer = 2 … default-suppression drops the property whose value equals the type's default.

    That is not how it works. DefaultIgnoreCondition = WhenWritingDefault suppresses at default(T) — for int that is 0, never a property's = 2 initializer. Serialization.md states it, and MeshNodeVersioning.md uses the same fact as its worked example (Version = 0 would be omitted, which is why versions start at 1). So "decimalPlaces": 2 is never dropped, and that explanation — which I wrote into the allow file earlier — was incorrect.

    The real cause is #2266's BOM defect. The Currency instance files are BOM'd, so pre-fix AsAuthored installed the materialised value on first install; the second install's unchanged-skip then compared the authored file against that wrongly-installed value and saw a difference. Fixing the re-read removes the wrong write at its source, so there is no second bug here.

    .github/samples-gate.allow was deliberately left untouched: its PensionFund idempotence entry is documented as intermittent and must not be dropped on a single green run. Note #2324 has since made intermittent a machine-readable token, so that entry can now be marked rather than silently re-litigated — and if this fix makes the check deterministic, the entry should be removed rather than marked.

    (Closing by hand — #2318's body omitted the closing keyword.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions