Skip to content

Close three gates that render a tick without checking anything (#2210, #1786, #1709) - #2278

Merged
rbuergi merged 12 commits into
mainfrom
fix/gates-that-never-fired
Aug 26, 2026
Merged

rbuergi merged 12 commits into
mainfrom
fix/gates-that-never-fired

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Three members of the "the gate is there and it has never fired" cluster. Every gate here is verified by making it go RED on the real failure case — a gate you cannot show failing is not a gate, and that is precisely this cluster's subject.


#2210 — a mis-nested values key renders "" and DELETES live config

The incident. After the 08-24 near-miss the GitHub App identity was committed to the per-env overlay — one section off, under config.memex_migration, which the portal ConfigMap never reads. The overlay looked safe (key present, reassuring comment beside it) while every deploy rendered the portal keys "" and helm's three-way merge applied the blanking as a deletion. GitSync and plugin-registry pulls were down fleet-wide, 11:13–11:24Z.

The deploy-time blank-guard in Systemorph/Memex's helm-release.yml catches that shape at the last possible moment, and needs a live cluster to do it. This is the same invariant asserted at AUTHORING time, from the files alone — so the mis-nesting is a red pull request instead of a production incident with a rollback.

deploy/aks/scripts/check-values-are-read.{sh,py}: every config.<component>.<KEY> a values file sets must be read by that component's templates. Wired into chart-gate.yml (PR + push to main). No secret, no cluster, no network ⇒ no condition under which it may decline to run. It takes values paths, so the private deployment repo can run the identical check on its overlays — where the mis-nesting actually happened.

It is the general form of a rule config.yaml states in a comment and that has been broken four times: SelfUpdate__PollInterval (#1778), SelfUpdate__MinRollInterval (#1780/#1925), Modules__Root (#1925), and #2210's identity trio. A rule stated in a comment is not a gate; this is the gate.

The mechanism, rendered rather than argued

Per the constraint that a chart claim must be verified by rendering and diffing, not by reading the template. helm template over values.yaml + values.aks.yaml, correct nesting vs. the #2210 mis-nesting:

--- render-correct.yaml
+++ render-misnested.yaml
@@ -533,9 +533,9 @@
-  GitHub__App__ClientId: "Iv23liBtPCOxYjqyK3Yx"
-  GitHub__App__InstallationOwner: "Systemorph"
-  GitHub__App__InstallationId: "144517285"
+  GitHub__App__ClientId: ""
+  GitHub__App__InstallationOwner: ""
+  GitHub__App__InstallationId: ""

Shown going red — six ways

input result
the repo's own three values files ✅ All 114 config key(s) … are read by a template
the exact #2210 mis-nesting ❌ 3 errors, each naming config.memex_portal as the fix
an #1925-shaped orphan (Modules__NoSuchKey) ❌ "read by NO template, under any component"
an unknown config: section ❌ names the three sections the chart does read
a values file with no config keys at all ❌ "A run with no evidence must never read as a pass"
a values file that does not exist ❌ refuses
a chart that stops naming keys explicitly ❌ names the offending range line — the premise this check rests on, asserted rather than assumed

#1786 — the last two NodeTypes, and the guard that silently stopped covering them

Both remaining CompileError NodeTypes are fixed. In-mesh source is never compiled by CI, so each was reproduced locally against a faithful copy of the runtime compile's import scope (DynamicMeshNodeAttributeGenerator.StandardUsings ∪ the union of the files' own usings) — red before, green after:

  • Northwind/AnalyticsCatalog — DashboardLayoutAreas named four classes by their old *Views names; the real classes are *LayoutAreas. Control run reproduces the issue's quoted CS0103 OrderViews / SalesViews exactly: 4 errors → 0.
  • Northwind/Product — ProductContent's [Dimension(typeof(Supplier))] / typeof(Category) name records that live in Northwind/AnalyticsCatalog/Source, which the default source query cannot reach. Control reproduces CS0246 Supplier, Category; Product.json now declares exactly those two files as shared=@… sources: 2 errors → 0.

The issue's scoping note said Product ships no Source/ at all — it does (ProductContent.cs, ProductNodeLayoutAreas.cs); what it cannot reach is the two dimension records.

.github/samples-gate.allow is emptied. The two entries above would now be STALE (a listed check that starts passing fails the run — the ratchet only shrinks), and the six FutuRe/* entries name nodes b63fbf057 retired to Markdown, so they read as Unverifiable — warned and never failed, i.e. six lines that could sit there for ever describing debt that no longer exists.

The trapdoor this fix would have opened, closed instead

ShippedNodeTypeSourceResolutionTest used to continue past any NodeType declaring sources — so adding one to Product.json would have removed it from the guard entirely. That is this cluster's own defect, one level in: a skip renders identically to a pass. It now resolves the two shapes anyone authors (namespace:Source scope:subtree, [name=]@Some/Node/Path) and skips only an entry it genuinely cannot decide.

A/B on identical broken input (a sources entry pointing at a namespace that does not exist, so ProductContent resolves nowhere):

origin/main's guard   Passed!  - Failed: 0, Passed: 2     ← the trapdoor
this PR's guard       Failed!  - Failed: 2, Passed: 0
  samples/Graph/Data/Northwind/Product.json: ProductContent

A new pin in TheScansAreNotVacuous keeps Northwind/Product as the live witness that a declared sources list is still checked, so the coverage cannot vanish again silently.


#1709 — Chart Drift's header was wrong about why it is red

The issue text and the workflow header are both stale: the secret IS provisioned. MEMEX_DEPLOY_REPO_TOKEN was created 2026-08-17T11:12:42Z (created_at == updated_at — never re-set), and every run since has died in the same place:

##[error]secrets.MEMEX_DEPLOY_REPO_TOKEN is set but GitHub rejects it (HTTP 401, Bad credentials).

The dispatch eleven seconds after it was stored, then nine consecutive schedules through 2026-08-25. So the remaining action is RE-MINT, not provision — and 401 rules out the two adjacent explanations by construction: GitHub answers 404 for a repo a valid token may not read and 403 for a valid token pending org approval. Item 3 now states this, and warns against reading the daily red as "items 1 and 2 are outstanding too" — they are done; preflight never gets far enough to exercise them.

The preflight itself was already correct and is untouched in shape. But its 401 branch fired daily for nine days with a generic message, so it now reports what is actually stored: a length, a prefix class, and a whitespace check. All derived — the value is never printed and never narrowed. A trailing newline from gh secret set NAME < token.txt produces exactly this 401 and is invisible in the Actions UI, which renders every secret as ***; that branch is verified firing (38 characters vs 37, plus the named advice).

⚠️ Chart Drift stays RED after this PR, by design — it is one manual re-mint away, and it will not go green on a workflow edit.


Not in this PR

🤖 Generated with Claude Code

…2210, #1786, #1709)

Three members of the "the gate is there and it has never fired" cluster. Each is verified
by making it go RED on the real failure case, not by reading it.

## #2210 — a mis-nested values key renders "" and DELETES live config

The 2026-08-25 incident: after the 08-24 near-miss the GitHub App identity WAS committed
to the per-env overlay — one section off, under `config.memex_migration`, which the portal
ConfigMap never reads. The overlay looked safe while every deploy rendered the portal keys
"" and helm's three-way merge applied the blanking as a deletion. GitSync and plugin-registry
pulls were down fleet-wide 11:13–11:24Z. The deploy-time blank-guard in Systemorph/Memex
catches that shape at the last possible moment and needs a cluster to do it.

`deploy/aks/scripts/check-values-are-read.{sh,py}` asserts the same invariant at AUTHORING
time, from the files alone: every `config.<component>.<KEY>` a values file sets must be read
by that component's templates. Wired into chart-gate.yml (PR + push to main; no secret, no
cluster, so no skip-trapdoor), and it takes values paths so the private deployment repo can
run the identical check on its overlays — where the mis-nesting actually happened.

It is the general form of a rule config.yaml states in a comment and that has been broken
four times: SelfUpdate__PollInterval (#1778), SelfUpdate__MinRollInterval (#1780/#1925),
Modules__Root (#1925), and #2210's identity trio.

Verified — 114 keys across the repo's three values files pass; and it goes red, naming the
fix, on: the exact #2210 mis-nesting (3 keys), an #1925-shaped orphan, an unknown config
section, a values file with no keys at all ("checked nothing" is a failure, not a pass), a
values file that does not exist, and a chart that stops naming keys explicitly (the premise
the check rests on). The mechanism itself is shown by rendering, not argued:
`helm template` with the mis-nested overlay emits GitHub__App__{ClientId,InstallationOwner,
InstallationId} as "" where the correctly-nested one emits the values.

## #1786 — the last two NodeTypes, and the guard that silently stopped covering them

Both remaining CompileError NodeTypes fixed, each reproduced locally against a faithful
copy of the runtime compile's import scope (StandardUsings ∪ the union of the files' usings)
— red before, green after:

  * Northwind/AnalyticsCatalog — DashboardLayoutAreas named four classes by their old
    `*Views` names; the real classes are `*LayoutAreas`. 4 errors → 0.
  * Northwind/Product — ProductContent's `[Dimension(typeof(Supplier))]`/`typeof(Category)`
    name records living in Northwind/AnalyticsCatalog/Source, which the default source query
    cannot reach. Product.json now declares exactly those two files as `shared=@…` sources.

`.github/samples-gate.allow` is emptied: the two entries above would now be STALE (a listed
check that starts passing fails the run), and the six FutuRe entries named nodes that
b63fbf0 retired to Markdown — they read as `Unverifiable`, which is warned and never
failed, so they could have sat there for ever describing debt that no longer exists.

ShippedNodeTypeSourceResolutionTest used to `continue` past ANY NodeType declaring `sources`
— so the fix above would have removed Product from the guard entirely. That is this cluster's
own defect one level in. It now resolves the two shapes anyone authors and skips only an entry
it cannot decide. A/B measured on identical broken input (a sources entry pointing at a
namespace that does not exist): the old guard passes 2/2, the new one fails naming the file
and the unresolvable type.

## #1709 — Chart Drift's header was wrong about why it is red

MEMEX_DEPLOY_REPO_TOKEN is NOT unprovisioned. It was created 2026-08-17T11:12:42Z (created_at
== updated_at: never re-set) and every run since — the dispatch eleven seconds later, then
nine consecutive schedules through 2026-08-25 — dies at "HTTP 401, Bad credentials". The
remaining action is RE-MINT, not provision; 401 rules out both scope (404) and org approval
(403). Item 3 now says so.

The 401 branch fired daily for nine days with a generic message, so it now reports what is
actually stored — a length, a prefix CLASS, and a whitespace check (all derived; the value
itself is never printed, never narrowed). A trailing newline from `gh secret set NAME < file`
produces exactly this 401 and is invisible in the Actions UI, which renders every secret
as ***.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens several “gate looks green while checking nothing” failure modes, and fixes the last two Northwind sample NodeType compile errors by correcting layout-area references and explicitly declaring shared source roots.

Changes:

  • Fix Northwind sample NodeTypes by (1) updating dashboard layout-area type names and (2) declaring shared sources for Northwind/Product so its configuration lambda can resolve Supplier/Category.
  • Strengthen ShippedNodeTypeSourceResolutionTest so NodeTypes with explicit sources are no longer automatically skipped (resolving the two authored shapes that are filesystem-resolvable).
  • Add a new “values keys are actually read by templates” guard (check-values-are-read.{sh,py}) and wire it into chart-gate.yml; improve chart-drift.yml operator guidance for the known 401-token condition.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/MeshWeaver.Graph.Test/ShippedNodeTypeSourceResolutionTest.cs Extends the guard to resolve and scan explicit NodeType sources instead of skipping them.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-25-northwind-dashboard-renders-again.md Adds a What’s New entry describing the Northwind fixes and the new guard behavior.
samples/Graph/Data/Northwind/Product.json Declares explicit sources so ProductContent can resolve sibling shared dimension types.
samples/Graph/Data/Northwind/AnalyticsCatalog/Source/DashboardLayoutAreas.cs Fixes stale *Views references to the current *LayoutAreas types.
deploy/aks/scripts/check-values-are-read.sh Adds a script to ensure config.<component>.* keys set in values files are referenced by chart templates.
deploy/aks/scripts/check-values-are-read.py Implements the config-key “read by template” check (YAML parse + template reference scan).
.github/workflows/chart-gate.yml Runs the new values-read guard as part of the Chart Gate workflow.
.github/workflows/chart-drift.yml Updates workflow guidance and improves the 401-token diagnostics output (without printing secrets).
.github/samples-gate.allow Clears the samples known-debt ratchet list and documents why it is now empty.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/MeshWeaver.Graph.Test/ShippedNodeTypeSourceResolutionTest.cs Outdated
rbuergi and others added 3 commits August 25, 2026 23:20
The chart also writes `(.Values.config.memex_portal).Deployment__Orleans__Clustering`
(memex-portal/secrets.yaml, memex-migration/secrets.yaml). The read-set regex required a bare
dot, so a key appearing ONLY in that form would have been reported as orphaned — a FALSE ALARM
on a key that is read perfectly well. No key sits in that position today, which is exactly why
it would have surfaced later, on someone else's unrelated change.

A gate that cries wolf gets switched off, so it has to match every shape the chart actually uses
rather than the tidiest one. Re-verified: green baseline unchanged (114 keys examined, 125
readable), and still red on the #2210 mis-nesting, an #1925 orphan, an unknown config section, a
no-evidence run, a missing file, and a chart that stops naming keys explicitly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…annot resolve

Copilot review on #2278, and it found the right thing: the `name=` stripping did not mirror
`CodeQueryResolver.ParseName`. Two gaps, both silent —

  * no `TrimEnd()` on the candidate label, so `shared =@Northwind/…` (a space before the `=`)
    was not recognised as a named entry, and
  * no "the body must be non-empty" check, so a bare `shared=` was mis-parsed.

Either one dropped the entry, which under the previous code dropped the WHOLE NodeType out of
coverage — the exact silent-coverage-drop this change exists to close, reintroduced one level
down. `$self` substitution and Expand's `TrimStart('@').TrimStart()` are mirrored too.

The deeper half: "return null = skip this NodeType" is gone. Narrowing a silent skip from
"declares sources" to "declares an entry I cannot decide" is the same failure with a smaller
blast radius, and it fails in the direction that hides things. An entry this cannot resolve on
disk is now REPORTED — it names the entry, says why, and asks to be authored differently or to
teach ResolveSourceRoots the shape. Nothing about a `sources` list can remove a NodeType from
this guard any more.

Verified both ways on the real tree: `shared =@…` (Copilot's case) resolves and stays green,
and an entry shaped as a genuine mesh query fails naming itself:

    samples/Graph/Data/Northwind/Product.json: unresolvable `sources` entry
      'nodeType:Code namespace:Northwind/* scope:subtree' — not a relative `namespace:<name> …`
      and not `@<node path>`, so it is a mesh query only a running mesh can answer

`TheScansAreNotVacuous` now also asserts no shipped NodeType carries an unresolvable entry, so
the reporting path itself cannot rot back into a skip.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Off-by-one in my own correction, which would be a poor way to correct someone else's. The run
list since the secret landed is 9 runs total: the 11:12:53Z workflow_dispatch, then eight daily
schedules 08-18 → 08-25. All nine failed; `gh run list --workflow chart-drift.yml` confirms
`all(.conclusion == "failure")` over that window.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi

rbuergi commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

The #1786 fix, verified against a real mesh rather than a scratch compile

Ran the actual samples gate locally, exactly as dotnet-test.yml's doc-gate job runs it — stage-samples-gate.sh then mw-plugin-test with this PR's emptied allowlist:

known-debt allowlist: 0 entr(ies)
Discovered 8 package(s): ACME → Cornerstone → FutuRe → MathDemo → Northwind → PensionFund → PythonDemo → Systemorph
…
[PASS] Northwind (276 node(s), 8 type(s))
    ok  Northwind/AnalyticsCatalog: compile=Ok render=ok tests=skipped
    ok  Northwind/Product:           compile=Ok render=ok tests=skipped
    …
ALL GREEN.

That is stronger than the scratch type-check in the PR body, and it closes the one thing that check could not reach: Product's shared=@Northwind/AnalyticsCatalog/Source/{Supplier,Category} entries resolve against a real mesh, not just against a folder I pointed a csproj at. Both NodeTypes compile and render.

known-debt allowlist: 0 entr(ies) is the other half — with the ratchet empty, nothing is being tolerated. Every one of the 8 Northwind types, and all 8 packages, pass on their own merit.


One pre-existing finding, out of scope here — flagging rather than fixing

The same run logs a warning that is not mine and is not new:

Content type discriminator 'AnalysisContent' is claimed by two different declarations
(nodeType:FutuRe/GroupAnalysis and nodeType:FutuRe/LocalAnalysis) — it can no longer be resolved
by name, and content carrying it stays an untyped JsonElement on any hub that has not registered
the type itself.

This is the duplicate-discriminator defect: two NodeTypes claiming one content-type name are two distinct CLR types, the registry refuses the discriminator, and the polymorphic converter degrades content to a raw JsonElement instead of throwing — so the symptom is views rendering empty, with a green build. The gate passes because the warning is not a failure, which is exactly the camouflage that makes this class survive.

It is unrelated to this PR's diff and the fix is a scope call (which of the two NodeTypes owns AnalysisContent — and per the precedent, drop the surplus claim rather than renaming the record, since a rename orphans persisted "$type":"AnalysisContent" content). Leaving it for a separate change; noting it here so it is not lost, since nothing currently ticketed covers it.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

616 tests   612 ✅  17m 37s ⏱️
  7 suites    4 💤
  7 files      0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

2 878 tests   2 775 ✅  7m 53s ⏱️
    9 suites    103 💤
    9 files        0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    9 files      9 suites   7m 5s ⏱️
1 609 tests 1 608 ✅ 1 💤 0 ❌
1 610 runs  1 609 ✅ 1 💤 0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

    9 files      9 suites   5m 5s ⏱️
1 952 tests 1 950 ✅ 2 💤 0 ❌
2 524 runs  2 522 ✅ 2 💤 0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

2 195 tests   2 193 ✅  15m 8s ⏱️
   10 suites      2 💤
   10 files        0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

2 036 tests   1 843 ✅  9m 36s ⏱️
    9 suites    193 💤
    9 files        0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

    53 files      53 suites   1h 2m 26s ⏱️
11 284 tests 10 979 ✅ 305 💤 0 ❌
11 859 runs  11 554 ✅ 305 💤 0 ❌

Results for commit a14edca.

♻️ This comment has been updated with latest results.

rbuergi and others added 5 commits August 26, 2026 17:18
…fired

# Conflicts:
#	.github/samples-gate.allow
#	.github/workflows/chart-drift.yml
…y this branch pays off

Two conflicts, resolved on evidence rather than by preferring a side wholesale.

`.github/workflows/chart-drift.yml` — took MAIN. This branch still carried the
PAT-based item 3 and its re-mint diagnostics; main replaced that path entirely
with a GitHub App installation token minted per run. The branch's diagnostics
describe a mechanism that no longer exists, so keeping them would have restored
a dead instruction to the operator-facing header.

`.github/samples-gate.allow` — took MAIN, then removed exactly one line.
Main is the current truth about which debt exists, and emptying the file is
#1786's subject, not this branch's. But this branch DOES pay off one entry:
Product.json gains `shared=@Northwind/AnalyticsCatalog/Source/{Supplier,Category}`,
which is what `Northwind/Product compile` was allowing for. The ratchet fails on
a STALE entry as well as a new failure, so carrying that line forward would have
turned this branch red for having fixed something.

Left the six FutuRe entries: they name NodeTypes retired to Markdown, so the gate
reports them Unverifiable, which is warned and never failed.

Revert-check run after resolving (`git diff origin/main...HEAD --stat`, three
dots): exactly six files, all this branch's own subject. No foreign hunk revived.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rbuergi
rbuergi enabled auto-merge August 26, 2026 19:02
@rbuergi
rbuergi merged commit 953c7ce into main Aug 26, 2026
26 checks passed
@rbuergi
rbuergi deleted the fix/gates-that-never-fired branch August 28, 2026 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants