Skip to content

review(#4213): every reader of a declaration fails closed, and the window is read off the purge #4223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
meshweaver-cloud merged 2 commits into from
Sep 13, 2026
Merged

review(#4213): every reader of a declaration fails closed, and the window is read off the purge #4223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a9393e0
fix(#3438): hold the recorded purge to the decided 30-day window, and…
rbuergi Sep 13, 2026
7d48482
review(#4213): every reader of a declaration fails closed, and the wi…
rbuergi Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions .github/acr-retention/purge-old-images.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,30 @@ version: v1.1.0
# in direct contradiction, and nothing but the lock reconciles them.
#
# 🚨 Raising --ago / --keep is NOT a fix: it moves the cliff. A pin left stable for a quarter walks
# off the new one just the same.
# off the new one just the same. The window below is NOT offered as pin protection — the lock is —
# it is the decided policy the other two stores already enforce, stated here so that re-enabling
# this task cannot silently apply a window nobody chose.
#
# 🚨 THE WINDOW IS 30 DAYS BY AGE WITH NO BUILD-COUNT QUOTA, AND THAT IS A DECISION, NOT A TUNING.
# #3842, quoted in #3438's body and in #3859's acceptance list: *retain unreferenced continuous
# artifacts for at least 30 days by age, without a build-count quota*. #3843 clamped
# `PrebuiltBundleRetention` to it, #3846 clamped `AssemblyCacheRetention` to it, and #3843's own
# body records that the registry was left out: *"The ACR task record and live cloud cleanup are
# unchanged."* This step was the third store and the one nobody asserted — it carried
# `--ago 7d --keep 10` over five continuously-republished repositories, `memex-portal-ai` among
# them, which is the image BOTH production portals run.
#
# `--keep` is gone rather than raised. It counts NEWER BUILDS, so a repository republished many
# times a day ages its own manifests out in hours regardless of `--ago` — which is #3438's root
# cause stated as a flag, not a second-order concern.
#
# 🚨 THE RECORD IS AHEAD OF THE REGISTRY HERE, ON PURPOSE, AND tasks.json DECLARES IT under
# `recordAheadOfRegistry`. The live task is DISABLED (paused, Memex#219) and still carries
# `--ago 7d --keep 10`; `acr-retention-tasks.sh apply` is what pushes this file onto it, so this
# is the window a re-enable would install. Do NOT "fix" the drift by running
# `acr-retention-tasks.sh record` — that overwrites this file FROM the live task and silently
# restores the 7-day quota window. `lock-pinned-digests.py --check-retention-record` then reds,
# on every pull request, which is how that mistake is caught.
#
# PROTECTION IS THE MANIFEST LOCK, not this policy. Live pinned digests carry
# `--delete-enabled false`; `acr purge` skips locked manifests by default and neither step passes
Expand All @@ -36,7 +59,7 @@ version: v1.1.0
# Doc: Doc/Architecture/PinnedImageRetention. Incidents: Memex#122, MeshWeaver#3438 (Education,
# Manufacturing and SocialMedia CI down simultaneously 2026-09-05T15:29Z on a purged tester digest).
steps:
- cmd: acr purge --filter 'mw-plugin-test:.*' --filter 'memex-migration:.*' --filter 'memex-portal-ai:.*' --filter 'memex-portal-next:.*' --filter 'memex-bake:.*' --ago 7d --keep 10 --untagged
- cmd: acr purge --filter 'mw-plugin-test:.*' --filter 'memex-migration:.*' --filter 'memex-portal-ai:.*' --filter 'memex-portal-next:.*' --filter 'memex-bake:.*' --ago 30d --untagged
disableWorkingDirectoryOverride: true
timeout: 3600
- cmd: acr purge --filter 'memex-portal:.*' --ago 30d --untagged
Expand Down
24 changes: 21 additions & 3 deletions .github/acr-retention/tasks.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,16 @@
"creationDate": "2026-08-31T20:33:48.969331+00:00",
"notes": [
"🚨 DISABLED 2026-09-12 — see the top-level `pause` block. This was `Enabled` at `0 3 * * *`; every note below describes the policy it runs WHEN RE-ENABLED.",
"ONE task, TWO steps: a 7d --keep 10 step over the CI image repositories, and a separate",
"30d step for memex-portal. The split is behaviour-preserving — merging the filter lists",
"under the stricter window would retune memex-portal without anyone deciding to.",
"ONE task, TWO steps: a 30d step over the CI image repositories, and a separate 30d step",
"for memex-portal. The filter lists stay SEPARATE even at one window — they are not",
"symmetric (mw-plugin-test is unique to the first, memex-portal to the second), and a",
"union-merge would silently retune one of them the next time either window moves.",
"🚨 The first step was `--ago 7d --keep 10` until 2026-09-13 (MeshWeaver#3438). #3842's",
"decided rule is at least 30 days by age with NO build-count quota; #3843 and #3846",
"clamped the bundle store and the assembly cache to it and left the registry out in as",
"many words. `lock-pinned-digests.py --check-retention-record` now asserts it on every",
"pull request. See `recordAheadOfRegistry` below: the live task still carries the old",
"window, and this record is what `apply` would push.",
"Both steps share ONE 3600 s task budget; the per-step `timeout` is a per-step ceiling.",
"🚨 NEITHER STEP PASSES `--include-locked`, and that is the whole of the pin protection:",
"`acr purge` skips locked manifests by default. Adding that flag would delete every",
Expand All @@ -81,6 +88,17 @@
]
}
],
"recordAheadOfRegistry": {
"inForce": true,
"since": "2026-09-13",
"tasks": [
"purge-old-images"
],
"what": "purge-old-images.yaml step 1 records `--ago 30d` with no `--keep`; the live (disabled) task still carries `--ago 7d --keep 10`.",
"why": "#3842's decided rule — at least 30 days by age, no build-count quota — was landed in the bundle store (#3843) and the assembly cache (#3846) and explicitly NOT in the registry. The record is what `acr-retention-tasks.sh apply` pushes, so stating the decided window here is what makes re-enabling the task install a window somebody chose. Nothing was applied: this is a record change, not a registry change.",
"clearedBy": "the next `acr-retention-tasks.sh apply` — after which `verify` finds no drift on these tasks and reds on THIS declaration being stale, which is how it expires rather than rotting.",
"issue": "MeshWeaver#3438"
},
"pause": {
"inForce": true,
"since": "2026-09-12",
Expand Down
174 changes: 167 additions & 7 deletions .github/scripts/acr-retention-tasks.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,50 @@ cmd_show() {
done
}

# The tasks `recordAheadOfRegistry` declares, space-separated; empty when nothing is declared.
# 🚨 NO `2>/dev/null` AND NO `|| true`. A tasks.json this cannot read must stop the caller, not
# answer "nothing is declared" — that is the swallow that turns "the call failed" into "the answer
# is empty", and here it would decide whether a drift is excused. Callers say `|| return 1`.
declared_ahead() {
if ! python3 -c "
import json, sys
d = json.load(open('$RECORD/tasks.json'))
a = d.get('recordAheadOfRegistry')
if a is None:
print(''); sys.exit(0)
Comment on lines +80 to +82
# FAIL CLOSED ON A MALFORMED SWITCH: an inForce of the STRING true is not the BOOLEAN true, and
# every reader asks 'is True'. A typed quote mark would read as 'nothing is declared' and skip the
# overwrite guard below, while looking to a human exactly like an armed declaration.
if not isinstance(a, dict) or not isinstance(a.get('inForce'), bool):
sys.exit('recordAheadOfRegistry.inForce is ' + repr(a.get('inForce') if isinstance(a, dict) else a)
+ ', not a JSON boolean')
print(' '.join(a.get('tasks') or []) if a['inForce'] else '')"; then
echo "::error::cannot read $RECORD/tasks.json, so it is unknown whether the record is" >&2
echo " deliberately ahead of the registry. Refusing to decide either way." >&2
return 1
fi
}

cmd_record() {
# 🚨 `record` OVERWRITES THE RECORD FROM LIVE, so it is the one command that can silently undo a
# policy the record carries and the registry does not. On 2026-09-13 that was the whole of
# MeshWeaver#3438's remaining window fix: re-recording would have restored `--ago 7d --keep 10`
# into a file whose comments explain at length why it must not say that.
local ahead
ahead=$(declared_ahead) || return 1
if [ -n "$ahead" ]; then
echo "🚨 tasks.json declares recordAheadOfRegistry for: $ahead"
echo " Recording OVERWRITES those files from the LIVE task, which discards the recorded"
echo " policy and restores whatever the registry currently carries."
echo " If that is what you want, say so in the same diff that deletes the declaration."
printf ' Type OVERWRITE to continue: '
local confirm
read -r confirm
if [ "$confirm" != "OVERWRITE" ]; then
echo "aborted."
return 1
fi
fi
# 🚨 CAPTURE THROUGH THE SAME READER `verify` COMPARES WITH. Recording with a hand-typed
# `az … | base64 -d > file` and verifying through `$(…)` differ by one trailing newline, and the
# verifier then reports permanent drift over a byte nobody wrote. Measured 2026-09-07: the first
Expand All @@ -83,7 +126,9 @@ cmd_record() {
}

cmd_verify() {
local task drift=0 checked=0
local task drift=0 checked=0 ahead
ahead=$(declared_ahead) || return 1
ahead=" $ahead "
for task in $TASKS; do
local recorded="$RECORD/$task.yaml"
if [ ! -f "$recorded" ]; then
Expand All @@ -99,14 +144,72 @@ cmd_verify() {
live_yaml "$task" > "$live" || { rm -f "$live"; return 1; }
checked=$((checked + 1))
if ! diff -u "$recorded" "$live" > "${live}.diff" 2>&1; then
echo "::error::task '$task' DRIFTED from .github/acr-retention/$task.yaml:"
cat "${live}.diff"
echo " Someone changed retention in the cloud without updating the record — or the record"
echo " was changed without applying it. Decide which is right, then re-run \`apply\` or"
echo " re-record with \`record\`. The reasoning in these comments is the only copy there is."
drift=1
# 🚨 A DECLARED drift is the record being deliberately AHEAD of the registry — the state a
# policy change lives in between "decided" and "applied". Reporting it as an incident would
# make `verify` permanently red, and a drift report that is always red is one nobody reads
# (this file learned that on 2026-09-07 over a trailing newline). It is still PRINTED.
if [ "$ahead" != " " ] && [ "${ahead#* $task }" != "$ahead" ]; then
echo " $task: drift is DECLARED (tasks.json → recordAheadOfRegistry) — the record is"
echo " deliberately ahead of the registry and \`apply\` is what closes it:"
sed 's/^/ /' "${live}.diff"
# 🚨 A DECLARATION EXCUSES THE WINDOW MOVING, NEVER THE TASK BECOMING SOMETHING ELSE.
# Keyed on the task NAME alone, this exemption would print — and pass — a live definition
# that had gained `--include-locked` (which deletes every manifest the lock protects),
# swapped its filters (purging a repository nobody decided to purge), or stopped being an
# `acr purge` at all. The record-side checks below read the RECORDED file and would never
# see it. So the LIVE steps of a declared task are held to the invariants that are not
# what the declaration is about.
local live_steps
live_steps=$(grep -E "^[[:space:]]*-[[:space:]]+cmd:" "$live" || true)
if [ -z "$live_steps" ]; then
echo "::error::the LIVE definition of '$task' has no \`cmd:\` step, so the declared drift"
echo " excused a task that no longer purges anything."
drift=1
fi
if printf '%s\n' "$live_steps" | grep -q -- "--include-locked"; then
echo "::error::the LIVE definition of '$task' passes --include-locked."
echo " That flag DELETES LOCKED MANIFESTS — the entire protection the lock job provides."
echo " A recordAheadOfRegistry declaration is about the retention WINDOW; it does not"
echo " excuse this, and without this line the drift would have printed as declared."
Comment on lines +169 to +173
drift=1
fi
if printf '%s\n' "$live_steps" | grep -qv "acr purge"; then
echo "::error::the LIVE definition of '$task' has a \`cmd:\` step that is not an acr purge."
drift=1
fi
# The declaration is about the window, so the FILTERS must still match the record. A live
# filter set the record does not carry is a repository nobody reviewed into the purge.
local live_filters recorded_filters
live_filters=$(printf '%s\n' "$live_steps" | grep -o -- "--filter '[^']*'" | sort | uniq)
recorded_filters=$(grep -E "^[[:space:]]*-[[:space:]]+cmd:" "$recorded" \
| grep -o -- "--filter '[^']*'" | sort | uniq)
if [ "$live_filters" != "$recorded_filters" ]; then
echo "::error::the LIVE definition of '$task' purges a different set of repositories than"
echo " the record does. A recordAheadOfRegistry declaration covers the WINDOW, not WHAT"
echo " is purged. live: $(printf '%s' "$live_filters" | tr '\n' ' ')"
echo " recorded: $(printf '%s' "$recorded_filters" | tr '\n' ' ')"
drift=1
fi
else
echo "::error::task '$task' DRIFTED from .github/acr-retention/$task.yaml:"
cat "${live}.diff"
echo " Someone changed retention in the cloud without updating the record — or the record"
echo " was changed without applying it. Decide which is right, then re-run \`apply\` or"
echo " re-record with \`record\`. The reasoning in these comments is the only copy there is."
drift=1
fi
else
echo " $task: matches the record byte for byte."
# 🚨 THE DECLARATION EXPIRES BY BEING CHECKED, never by being remembered. Once `apply` has
# run, a still-standing `recordAheadOfRegistry` is a stale exemption that would excuse the
# NEXT real drift on this task — the same failure the instance roster's stale-entry arm
# exists for.
if [ "$ahead" != " " ] && [ "${ahead#* $task }" != "$ahead" ]; then
echo "::error::tasks.json declares recordAheadOfRegistry for '$task' and there is NO drift."
echo " The record has been applied, so the declaration now excuses nothing and would"
echo " excuse the next real drift on this task. Delete it from tasks.json."
drift=1
fi
fi
rm -f "$live" "${live}.diff"

Expand Down Expand Up @@ -161,7 +264,64 @@ cmd_verify() {
return $drift
}

# 🚨 THE RE-ENABLE INTERLOCK (MeshWeaver#3859, acceptance criterion 1).
#
# `apply` is the ONLY thing in this repository that can turn a destructive schedule back on: it
# pushes `status` out of tasks.json with `az acr task update --status`. Until this function existed
# it did so with no reference to anything — so editing one word in tasks.json (`Disabled` →
# `Enabled`) and running `apply` restored the 03:00 purge with NO protection decision consulted,
# which is the criterion this issue states in as many words: *failed / unavailable / incomplete
# protection collection cannot be followed by deletion*.
#
# This does NOT close #3859. The two clocks are still independent — the lock is an Actions cron at
# 01:00, the purge an ACR timer task at 03:00, and nothing makes the second wait for the first, so
# a nightly deletion is still not downstream of that night's protection verdict. What it closes is
# the ACT of re-enabling, which is the one edge this repository owns. The nightly interlock needs a
# registry-side mechanism and an RBAC grant, both of which #3859 says to choose with the maintainer.
#
# 🚨 It refuses rather than warns, and it names the condition RATHER THAN asking for a flag. A
# prompt answered "yes" is not a decision anyone can audit; `pause.reEnableWhen` is, and lifting
# the pause is a reviewed diff against this record.
assert_pause_permits_enabling() {
local paused reenable since blocked
# 🚨 FAIL CLOSED, same reason as declared_ahead: a non-boolean `inForce` must stop the apply, not
# convert to `no`. `pause.inForce: "true"` would otherwise walk straight past this interlock.
paused=$(python3 -c "
import json, sys
d = json.load(open('$RECORD/tasks.json'))
p = d.get('pause')
if p is None:
print('no'); sys.exit(0)
if not isinstance(p, dict) or not isinstance(p.get('inForce'), bool):
sys.exit('pause.inForce is ' + repr(p.get('inForce') if isinstance(p, dict) else p)
+ ', not a JSON boolean — refusing to decide whether the purge is paused')
print('yes' if p['inForce'] else 'no')") || return 1
[ "$paused" = "yes" ] || return 0
# 🚨 NO `|| true` — a status this cannot read must stop the apply, not be treated as Disabled.
blocked=$(python3 -c "
import json
d = json.load(open('$RECORD/tasks.json'))
print(' '.join(t['name'] for t in d['tasks'] if str(t.get('status','')).lower() == 'enabled'))") || return 1
[ -n "$blocked" ] || return 0
since=$(python3 -c "import json;print((json.load(open('$RECORD/tasks.json')).get('pause') or {}).get('since',''))")
reenable=$(python3 -c "import json;print((json.load(open('$RECORD/tasks.json')).get('pause') or {}).get('reEnableWhen',''))")
echo "::error::REFUSING to apply: the record declares an in-force PAUSE (since $since) and asks" >&2
echo " to enable: $blocked" >&2
echo "" >&2
echo " re-enable when: $reenable" >&2
echo "" >&2
echo " A pause is the mitigation held in front of an incomplete protection decision, so pushing" >&2
echo " an Enabled status while it stands puts deletion back in front of the protection that is" >&2
echo " not finished — MeshWeaver#3859's first acceptance criterion. Lift the pause DELIBERATELY:" >&2
echo " delete the \`pause\` block from .github/acr-retention/tasks.json in a reviewed diff that" >&2
echo " says what satisfied \`reEnableWhen\`, then run this again." >&2
return 1
}

cmd_apply() {
# Before the prompt, not after: a refusal the operator reads only after typing the registry name
# teaches them the prompt is the gate.
assert_pause_permits_enabling || return 1
echo "🚨 This MUTATES shared registry infrastructure every deployment depends on."
echo " Registry: $REGISTRY Tasks: $TASKS"
echo " Re-read Doc/Architecture/PinnedImageRetention before continuing."
Expand Down
Loading
Loading