fix(chart-drift): compute the pending-deletion half of CLUSTER-ONLY by rbuergi · Pull Request #3320 · Systemorph/MeshWeaver

rbuergi · 2026-09-04T22:13:57Z

Three sides, not two

Chart drift has three sides, and the gate only ever looked at two of them:

side	what it is	how it is read
D	the chart as CI renders it	`helm template`
L	the live cluster objects	`kubectl get …`
M	the last-deployed release manifest	`helm get manifest <release> -n <ns>`

check-chart-drift.sh + chart-drift-compare.py computed D vs L.
deploy/aks/operator/bin/hosting-audit computes M vs L. Nobody computed D vs M — and
that is where the only real deletion hazard lives. M is the original side of helm's three-way
merge, i.e. exactly what helm previously owned, so a key that is in L and in M but no
longer in D is a chart retirement that has not landed: the next helm upgrade will remove
it. Everything else survives a deploy — measured 2026-09-03 on helm v3.21.1 and v4.2.4 with a
positive control, and recorded in ChartDriftSemantics.md; this PR does not re-derive it.

Until now both halves were reported identically, as a plain CLUSTER-ONLY sorted in among dozens
of inert ones. The split was measured by hand on 2026-09-04 — 13 owned-but-retired out of 36
findings (memex 7, memex-cloud 6) — and nothing recomputed it on the next run.
Systemorph/Memex#152 asked for exactly this check.

What changed

check-chart-drift.sh gains fetch_manifest() beside fetch() — helm get manifest on the
kubectl transport, the same through az aks command invoke on aks-invoke — and passes it to
the comparator as a new positional argument.
chart-drift-compare.py parses M exactly the way desired.yaml is parsed and splits both
CLUSTER-ONLY branches (ConfigMap keys, inline env) on manifest membership. The class name is
unchanged — the class is "live and not rendered" and both halves are that — so order, the
legend and every existing assertion still line up; what changes is the finding's verdict, plus a
PENDING DELETION count on the summary line.
The finding reports the VALUE's emptiness, never the value. A pending deletion of a
zero-length value is a no-op; of a non-empty value it is a real removal. All 13 found on
2026-09-04 were zero-length, so a key-name-only report would have raised a false alarm. Only the
length/emptiness is printed — inline env entries hold tokens.
The never-owned half now says helm never owned it (it is NOT in the release manifest) rather
than leaving "a helm upgrade PRESERVES it" to be taken on trust.

The input fails closed

An unreadable or absent manifest is a hard FAILURE on both layers, naming helm get manifest.
This is not defensiveness: with M missing, every live-only key reads as never-owned — the
harmless half — so the report would answer "nothing here is about to be deleted" at exactly the
moment it could not tell. That is AGENTS.md's skip-trapdoor shape expressed as a default value
rather than as an if:, which is harder to see and worse than silence.

Mutation proof

deploy/aks/scripts/test-chart-drift-compare.sh (run by chart-gate.yml, job invariants) now
carries 49 assertions and passes. It was proven by mutation, not by watching it go green:

Mutation 1 — manifest membership always false (m_data = {}, m_env = set()). The suite went
RED with 8 failing assertions; the first was:

::error::the finding matching 'CLUSTER-ONLY *ConfigMap LogWatch__DefaultRepository' does not say
'PENDING DELETION' — helm owns this key and the chart no longer renders it, so the next upgrade
removes it. Actual:
      ::error::CLUSTER-ONLY ConfigMap LogWatch__DefaultRepository
                       live 'Systemorph/MeshWeaver' — edited on the cluster; helm never owned it …

Mutation 2 — ignore the value's emptiness (if value == "": → if False:). RED on exactly the
two zero-length cases:

::error::the finding matching 'CLUSTER-ONLY *ConfigMap Authentication__DevAdminUsers' does not say
'NO-OP' — an empty value means the removal changes nothing the pod reads, and saying so is the point.
::error::the finding matching 'CLUSTER-ONLY *inline env Legacy__Flag' does not say 'NO-OP' — an
empty inline env deletes to nothing.

Both mutations were reverted and the suite re-run: EXIT=0, 49 ok lines, no MUTATION marker
left in the tree.

What the fixtures cover

release-manifest.yaml added under both testdata/chart-drift/clean/ and drifted/ (the
clean one agrees with desired.yaml, or the control breaks):

case	shape	asserted
positive	`ConfigMap LogWatch__DefaultRepository` / `inline env Features__Ai__Clis__Copilot` — live, in `M`, not rendered, non-empty	`PENDING DELETION` + `NON-EMPTY`
zero-length	`ConfigMap Authentication__DevAdminUsers` / `inline env Legacy__Flag` — same shape, `""`	`PENDING DELETION` + `NO-OP`
never-owned control	`ConfigMap Ops__Contact` / `inline env Tok` — live, not rendered, not in `M`	must NOT say `PENDING DELETION`; must say `PRESERVES`
🚨 negative control	`Retention__Days` — live and in `M` and still rendered	must not be reported at all — without it the check could pass by flagging everything the manifest carries
🚨 fail-closed control	empty argument / missing path / empty file	each must exit non-zero and name `helm get manifest`

The fail-closed control lives in test-chart-drift-compare.sh rather than in a shell-level test:
the comparator itself hard-requires the manifest, so the refusal is directly exercisable there with
no cluster. check-chart-drift.sh's own fetch failure path is the same rule one layer up and is
not covered by an automated test (see Not verified below).

Docs

ChartDriftSemantics.md — the section "CLUSTER-ONLY splits in two" was a hand measurement
with a "go and run helm get manifest yourself" instruction. Rewritten to describe the automated
check, keeping the 2026-09-04 numbers explicitly as a baseline observation ("the last time
anyone had to"), and the stale "the manifest probe above is that check, and running it before a
deploy is what turns a silent deletion into a reviewed one" framing replaced.
DeploymentAKS.md — the five-class summary and the fail-RED list updated in the same commit.

No What's New entry: a CI drift gate is not something a portal user experiences, and the two closest
prior PRs touching these files (#3275, #3166) carried none either.

Verification

deploy/aks/scripts/test-chart-drift-compare.sh → EXIT=0, all 49 assertions pass; RED under
both mutations above.
shellcheck -S warning on both shell scripts → clean.
dotnet build src/MeshWeaver.Documentation/MeshWeaver.Documentation.csproj -c Release -warnaserror
→ Build succeeded. 0 Warning(s) 0 Error(s); the built assembly contains the edited text
(strings … | grep -c 'PENDING DELETION' → 5), so the embed is not stale.
DocumentationLinkIntegrityTest → Passed! - Failed: 0, Passed: 1, linkintegrity.trx written.

The new required input, measured on the live cluster before shipping it

A gate whose new required input turns out to be unreadable in production goes red for a
missing-input reason on its first scheduled run — indistinguishable from finding drift. So it was
measured read-only (2026-09-04), not assumed:

helm version --short                          → v3.21.1+gc56dd00   (inside az aks command invoke)
helm get manifest memex      -n memex         → exit 0, 76 386 bytes
helm get manifest memexcloud -n memex-cloud   → exit 0, 79 884 bytes

Every object the comparator hard-requires — the memex-portal-config ConfigMap, the
memex-portal-deployment Deployment, the memex-portal container inside it — is present in both
release manifests, so none of the three "the manifest parses but lacks an object" sys.exit(1) paths
can fire on either production namespace. Recorded in the doc along with the trap that goes with it:
helm get manifest emits quoted scalars (kind: "ConfigMap"), so grep '^kind: ConfigMap'
returns nothing and reads as "the object is absent" — yaml.safe_load_all unquotes them, which is
why the comparator's comparison is right and a shell spot-check is not.

Review follow-up

The automatic Copilot review raised two points, both addressed in the second commit:

Quoting in the remote --command. The aks-invoke transport builds a string that a shell on
the far side re-parses; $NS/$RELEASE/the resource arg are expanded locally, so an odd value
would be re-split there. Quoted in all three builders (fetch, fetch_data_keys,
fetch_manifest), not only the new one — it is one defect in one function family, and
fetch_data_keys already quoted its go-template expansion, so this makes the file consistent.
Behaviour is unchanged for well-formed values.
Dates. 2026-09-05 → 2026-09-04, the UTC date this lands on and the day of the hand
measurement it automates.

Still not verified: check-chart-drift.sh's own manifest fetch-failure branch is exercised
by no automated test (it needs a cluster); the fail-closed control covers the comparator's refusal,
one layer down. The nightly Chart Drift workflow remains the first thing to run the fetch
end-to-end.

Refs #2355
Refs Systemorph/Memex#152

🤖 Generated with Claude Code

Chart drift has THREE sides, not two: D = the chart as CI renders it (`helm template`), L = the live cluster objects, M = the last-deployed release manifest (`helm get manifest`), which is the `original` side of helm's three-way merge — exactly what helm previously owned. check-chart-drift.sh computed D vs L only; hosting-audit computes M vs L only. Nobody computed D vs M, and that is where the sole deletion hazard lives: a key that is in L and in M but no longer in D is a chart RETIREMENT that has not landed, and the next `helm upgrade` WILL remove it, because the merge removes exactly what helm previously owned. Everything else survives a deploy (measured 2026-09-03, helm v3.21.1 and v4.2.4, with a positive control). The script now fetches M on both transports and the comparator classifies against it. Such a finding stays in the CLUSTER-ONLY class — the class is "live and not rendered" and both halves are that — but reads PENDING DELETION, carries a count in the summary, and states whether the LIVE VALUE is empty (the removal is a no-op) or not (a real removal). The value decides that, not the key name: all 13 owned-but-retired keys found by hand on 2026-09-04 were zero-length, so a name-only report would have raised a false alarm. No value is ever printed, only its emptiness and, for a ConfigMap key, its length. The manifest is a REQUIRED input on both layers. With M missing every live-only key reads as never-owned — the harmless half — so the report would answer "nothing is about to be deleted" precisely when it could not tell. That is a skip-trapdoor expressed as a default value instead of an `if:`, so the script fails RED naming `helm get manifest` and the comparator refuses to run at all. test-chart-drift-compare.sh drives all four shapes off committed fixtures, plus the two controls: a key that is live, helm-owned AND still rendered must not be reported at all, and three ways of withholding the manifest must each fail red naming the command. Refs #2355 Refs Systemorph/Memex#152 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

It includes future-dated statements (2026-09-05) and a small hardening fix is needed to safely quote variables in the remote aks-invoke manifest command.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR extends the AKS “chart drift” gate to incorporate Helm’s last-deployed release manifest, so the report can distinguish harmless cluster-only drift from cluster-only keys that Helm previously owned and will delete on the next upgrade (“pending deletion”), and documents the updated semantics.

Changes:

Add manifest fetching (helm get manifest) to check-chart-drift.sh and require it as a fail-closed input.
Update chart-drift-compare.py to parse the release manifest and mark the owned-but-retired subset of CLUSTER-ONLY as PENDING DELETION, including “empty vs non-empty” impact reporting without printing secret-bearing values.
Expand the comparator self-test fixtures and assertions to cover D vs M semantics (including negative and fail-closed controls) and update docs accordingly.

File summaries

File	Description
src/MeshWeaver.Documentation/Data/Architecture/DeploymentAKS.md	Updates the AKS deployment documentation to reflect manifest-aware drift semantics and fail-closed behavior.
src/MeshWeaver.Documentation/Data/Architecture/ChartDriftSemantics.md	Documents the “three sides” model (D/L/M) and explains the new `PENDING DELETION` reporting within `CLUSTER-ONLY`.
deploy/aks/scripts/check-chart-drift.sh	Fetches the release manifest and passes it to the comparator; fails closed if the manifest cannot be read.
deploy/aks/scripts/chart-drift-compare.py	Adds manifest parsing and splits `CLUSTER-ONLY` into never-owned vs pending-deletion, plus summary counting.
deploy/aks/scripts/test-chart-drift-compare.sh	Adds assertions for pending-deletion reporting, negative controls, and manifest-required fail-closed checks.
deploy/aks/scripts/testdata/chart-drift/desired.yaml	Adds a negative-control key (`Retention__Days`) to ensure manifest membership alone doesn’t generate findings.
deploy/aks/scripts/testdata/chart-drift/clean/release-manifest.yaml	Adds a “clean” manifest fixture aligned with desired output (control case).
deploy/aks/scripts/testdata/chart-drift/drifted/release-manifest.yaml	Adds a “drifted” manifest fixture to drive pending-deletion cases (ConfigMap + inline env).
deploy/aks/scripts/testdata/chart-drift/clean/live-configmap.json	Updates clean live fixture to include the negative-control key.
deploy/aks/scripts/testdata/chart-drift/drifted/live-configmap.json	Updates drifted live fixture to include pending-deletion and never-owned examples.
deploy/aks/scripts/testdata/chart-drift/drifted/live-deployment.json	Adds an empty inline env entry used to validate NO-OP pending deletion reporting.

Review details

Files reviewed: 11/11 changed files
Comments generated: 1
Review effort level: Lite

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

+  case "$VIA" in
+    kubectl)    helm get manifest "$RELEASE" -n "$NS" 2>"$WORK/helm-manifest.err" ;;
+    aks-invoke) az aks command invoke -g "$RG" -n "$AKS" -o tsv --query logs \
+                   --command "helm get manifest $RELEASE -n $NS" 2>"$WORK/helm-manifest.err" ;;
+  esac


…surement Two review findings from the automatic Copilot review, plus the evidence that closes this change's largest unverified claim. 1. The `aks-invoke` transport builds the remote `--command` as a STRING that a shell on the other side re-parses. `$NS`, `$RELEASE` and the resource argument are expanded LOCALLY, so the remote receives the literal — unquoted, a value carrying a space or a `;` would be split or executed there. Quoted in all three builders, not just the new one: it is one defect in one function family, and fixing a third of it reads as an oversight later. `fetch_data_keys` already quoted its go-template expansion; this makes the file consistent with itself. Behaviour is unchanged for every well-formed value. 2. Dated comments moved 2026-09-05 to 2026-09-04, the UTC date this lands on — and the same day the hand measurement it automates was taken, which is the more useful pairing anyway. 3. helm's availability on the `aks-invoke` transport was asserted from the script's own header rather than measured. Now measured, read-only, on the live cluster: `helm version --short` inside `az aks command invoke` answers v3.21.1+gc56dd00, and `helm get manifest` returns a manifest for both production releases (memex 76386 bytes, memexcloud 79884 bytes) carrying every object the comparator hard-requires — the ConfigMap, the Deployment and the memex-portal container. So none of the three "parses but lacks an object" failure paths can fire on either namespace. A new REQUIRED input that turns out to be unreadable in production reddens the gate for a reason indistinguishable from finding drift, which is exactly what this measurement rules out. Recorded with the trap that goes with it: `helm get manifest` emits QUOTED scalars (kind: "ConfigMap"), so grep '^kind: ConfigMap' returns nothing and reads as "the object is absent". safe_load_all unquotes them, which is why the comparator's comparison is right and a shell spot-check is not. Refs #2355 Refs Systemorph/Memex#152 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-04T22:32:44Z

Test Results (shard 1)

475 tests 475 ✅ 37s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:33:03Z

Test Results (shard 0)

251 tests 251 ✅ 2m 27s ⏱️
1 suites 0 💤
1 files 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:33:34Z

Test Results (shard 3)

368 tests 366 ✅ 1m 3s ⏱️
3 suites 2 💤
3 files 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:34:37Z

Test Results (shard 5)

4 files 4 suites 1m 28s ⏱️
1 487 tests 1 487 ✅ 0 💤 0 ❌
1 488 runs 1 488 ✅ 0 💤 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:35:41Z

Test Results (shard 2)

588 tests 396 ✅ 2m 56s ⏱️
3 suites 192 💤
3 files 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:37:02Z

Test Results (shard 4)

2 291 tests 2 291 ✅ 2m 35s ⏱️
3 suites 0 💤
3 files 0 ❌

Results for commit daaa961.

github-actions · 2026-09-04T22:43:57Z

Test Results

15 files 15 suites 11m 8s ⏱️
5 460 tests 5 266 ✅ 194 💤 0 ❌
5 461 runs 5 267 ✅ 194 💤 0 ❌

Results for commit daaa961.

Copilot AI lite review requested due to automatic review settings September 4, 2026 22:13

rbuergi enabled auto-merge September 4, 2026 22:14

Copilot started reviewing on behalf of rbuergi September 4, 2026 22:16

View session

rbuergi mentioned this pull request Sep 4, 2026

Chart Drift's first completing run found 91 divergences on memex/memex-cloud — 8 DIFFERS a deploy would overwrite, and nobody owns the backlog #2355

Closed

Copilot AI reviewed Sep 4, 2026

View reviewed changes

rbuergi added this pull request to the merge queue Sep 4, 2026

rbuergi mentioned this pull request Sep 4, 2026

test(ComboGate): wait until a recorded verdict is READABLE — the gate read before the write was visible #3324

Merged

Merged via the queue into main with commit 94d668a Sep 4, 2026
27 checks passed

rbuergi deleted the fix/2355-chart-drift-pending-deletion branch September 13, 2026 10:23

Navigation Menu

Repository navigation

fix(chart-drift): compute the pending-deletion half of CLUSTER-ONLY - #3320

fix(chart-drift): compute the pending-deletion half of CLUSTER-ONLY#3320
rbuergi merged 2 commits into
mainfrom
fix/2355-chart-drift-pending-deletion

rbuergi commented Sep 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

rbuergi commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Three sides, not two

What changed

The input fails closed

Mutation proof

What the fixtures cover

Docs

Verification

The new required input, measured on the live cluster before shipping it

Review follow-up

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 1)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 0)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 3)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 5)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 2)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results (shard 4)

Uh oh!

github-actions Bot commented Sep 4, 2026

Test Results

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rbuergi commented Sep 4, 2026 •

edited

Loading