Skip to content

The gate never checks a NodeType whose .json carries a BOM — it installs, compiles, and is silently ungated #2063

Closed

Description

@rbuergi

Found while building the bake/gate split (#1763, PR #2059) and deliberately not fixed there.

The finding

PluginGateRunner.DiscoverNodeTypes decides which NodeTypes a gate run CHECKS by parsing each *.json package file with a bare JsonDocument.Parse(file.Content) and continue-ing on JsonException:

catch (JsonException)
{
    continue; // malformed json is surfaced by the install itself
}

A UTF-8 BOM makes JsonDocument.Parse throw, so a BOM'd node file is silently dropped from the gate's type list. The IMPORTER does not drop it — FileFormatParserRegistry strips the BOM since #1767 — so the node installs, its hub activates, its type compiles, and nothing ever renders it, runs its Tests area, or judges its compile.

The comment is the trap: "malformed json is surfaced by the install itself" was true when the importer also rejected BOMs. Since #1767 the two disagree, and the disagreement is one-directional and silent.

Measured

samples/Graph/Data/PensionFund ships BOM'd .json (head -c 3 PensionFund/BalanceSheet.json → efbbbf). The gate reports:

[PASS] PensionFund (72 node(s), 0 type(s))

72 nodes installed, zero types gated — while the compiler-driven bake over the same tree finds PensionFund/BalanceSheet and compiles it. So the samples gate has been reporting PASS on a package whose only NodeType it never looked at.

Why it matters beyond the missed check

It is the reason PR #2059 does not switch the PR lane (dotnet-test.yml's doc/samples gate) to the compiler-driven bake, even though that lane's --bake-output is now a smoke test of a code path CD no longer uses. The tree baker discovers MORE types than the gate does, so switching would newly gate types that are absent from .github/samples-gate.allow — a red PR lane whose cause is this gap rather than the change.

The fix

Strip the BOM in DiscoverNodeTypes (or route it through the same FileFormatParserRegistry the importer uses, which is the version that cannot drift again). Expect the ratchet to need entries for whatever that newly reveals — those are pre-existing failures becoming visible, not new ones, and each should be judged on its own.

Then the PR lane can move to compile + --seed, matching CD.

Activity

  1. added a commit that references this issue on Aug 25, 2026
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