Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,16 @@ Both are filed: MeshWeaver.Plugins#1275 and #3213.
`CreatedBy` — which also makes the migration in §6 derivable.
`NotificationTriageService.cs:104` derives its recipient the same way and escalates into
`{space}/_Triage`, with the same defect. **MeshWeaver.Plugins#1275.**
- **Plugin-update notifications are unbounded.** `PackageUpdateReconciler.Notify` raises a fresh
`Notification` node on every reconcile that still sees an update — measured at four rows for the
same package in one day, 124 of the newest 200 rows overall. There is no "already told you"
suppression. This is the bell's row count, independent of where the rows live. **#3213.**
- ~~**Plugin-update notifications are unbounded.**~~ **FIXED (#3213).** `PackageUpdateReconciler.Notify`
used to raise a fresh `Notification` node on every reconcile that still saw an update — measured
at four rows for the same package in one day, 124 of the newest 200 rows overall. It had no
"already told you" suppression, and it could not have had one: the content-identity gate asks
*is the candidate installed?*, which the reminder path can never satisfy because it installs
nothing. The reminder path now carries its own marker
(`PackageManifest.NotifiedModuleVersion`), and `NotificationService.CreateNotification` accepts a
caller-supplied `identity` that derives a deterministic node id, so a repeat upserts the SAME row
instead of adding one. This was the bell's row count, and it was independent of where the rows
live — the **addressing** half of this emitter (§7 step 1: re-address to `Admin`) is still open.

## 6. The migration

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,52 @@ present:
prune, exactly as the static-repo importer skips it. Claiming is the deliberate act that
decouples one node from its package; an unclaimed local edit is overwritten, by design.

## 🚨 A reminder is told ONCE — the gate the content-identity check cannot be (#3213)

The content-identity gate above is the whole story for the **unattended** path and no part of it
for the **reminder** path, and conflating the two shipped a real defect: on memex.meshweaver.cloud,
**124 of the newest 200 notification rows** were this one emitter, four rows for the same package
in a single day, two packages accounting for over half of everything the mesh had written in four
days.

The gate asks *is the candidate installed?*:

- On the `AutoUpdate` path that question is **self-silencing**. The apply advances the record's
`ModuleVersion`, so the next reconcile compares equal and says nothing.
- On the reminder path it is **unsatisfiable**. Nothing is installed — by design, the user has not
acted — so `ModuleVersion` never advances and the comparison evaluates false on every subsequent
reconcile, forever. And the reconcile runs often: at boot, whenever a feed read drains a deferred
registry, and on every build webhook.

The notify path needs the *other* question — *have I already told them about this candidate?* — so
it carries its own answer, and the fix is two independent halves. **Either one alone still
duplicates:**

1. **A satisfiable silence gate.** `PackageManifest.NotifiedModuleVersion` records the candidate
the user was last reminded about, on the durable install record. Writing it is what makes the
gate *become true* — which is exactly what the version comparison could never do here. It lives
on the node rather than in a cache because a cache loses its memory on every pod start and
re-notifies, which is the symptom itself.
2. **A deterministic notification identity.** `NotificationService.CreateNotification` takes an
optional `identity`; when given, the node id is derived from *(main node, identity)* with the
platform's content-addressing helper and the write becomes an atomic upsert. The reconciler
keys it on *(record, kind, candidate version)*, so a repeat that does reach the notify path —
two reconcile entry points overlapping, a marker write that failed — lands on the **same** node
instead of adding a row.

The gate sits inside `Notify`, deliberately **not** in front of `Apply`: an unattended apply must
keep retrying every reconcile so a restored admin grant heals itself. It is only the *telling* that
is once per candidate. A refusal notification ("Update needs a Global Admin") is gated the same way
and by the same marker.

Order matters within the notify path: the bell is written **first**, the marker second. A marker
written first whose notification then failed would silence the reminder permanently; a bell written
first whose marker then failed costs one repeat, which the deterministic identity absorbs into the
same node.

A genuinely **new** candidate version changes both the marker comparison and the derived id, so it
gets its own unread bell — the suppression is per candidate, never per package.

## Configuration

### 1. Register the webhook on the plugin repo
Expand Down Expand Up @@ -264,7 +310,10 @@ install-time seed.
common case and it is supposed to be quiet.
- **A module changed** → an "Update available" notification on the install record, naming how many
files changed and removed, and the short sha it was built from. Nothing installs — this is the
platform default.
platform default. **Exactly one** notification per candidate version, however many times the
reconcile runs (see *A reminder is told once* below).
- **The SAME module change, seen again** → nothing at all. No new row, no re-raised bell, not one
file fetched.
- **A module changed on an opted-in record** → the delta install runs, touching only the changed
files — claimed (non-`Include` `SyncBehavior`) nodes excepted.

Expand All @@ -276,6 +325,7 @@ install-time seed.
| A module never updates, and the log says it "has no module content identity" | The module's `manifest.lock` is missing or unparseable, so there is no `ModuleVersion` to compare and "has it changed" is unanswerable. A missing hash is the **absence of evidence**, not evidence of a change: treating it as changed would re-install the module on every green build of the repo *and* on every pod start, which is acting on the event rather than the content. It is refused, loudly, and the catalog card's manual **Update** stays available. Fix the module's CI to emit the sidecar. |
| Nothing happens on a green build, **and the log says the delivery "matched NONE of the N sync config(s)"** | No `_GitSync` targets that repository — usually because the repository was **renamed** and the configs still store its old name. The matcher falls back to GitHub's canonical `full_name` (which follows the rename redirect) and repoints the config when it finds one, so this line surviving means the lookup could not be made either: the repository is unreachable with the config creator's credential, or the hook really is installed on a repository this mesh does not sync. The Warning names both sides — the incoming repository and everything it was compared against. |
| Build node updates but no installation reacts | The package is not installed on that instance. A catalog lists far more packages than any instance installs; only packages with an install record are considered. |
| The same "Update available" reminder keeps reappearing, or the bell re-lights on one you dismissed | Before #3213 this was the norm — a new row per reconcile, forever. The reminder is now told once per candidate version: the install record carries `NotifiedModuleVersion`, and the notification's id is derived from *(record, kind, candidate)*. Seeing it again means the candidate genuinely moved (`ModuleVersion` differs from the one on the record's marker) — read the record and compare the two, rather than assuming a duplicate. |
| The boot log says the feed of a registry could not be read after N attempts and was **recorded as PENDING**, and admins got a bell notification pointing at `Plugins/_RegistryReconcileLedger` | The registry stayed unavailable for longer than the boot's retry budget (#2888). Nothing is lost: the ledger entry for that registry reads `Pending: true`, and the reconcile runs on the next successful feed read — open the catalog page (or install anything from that registry) once the registry is back, then check the entry reads `LastReconciledVia: feed-read`. If the registry answers a *definite* refusal (401/403) instead, the entry is pending too, but no catalog open will drain it until the key or grant is fixed — the `LastFault` names which. |

The webhook **never throws** on a write failure: GitHub retries a non-2xx delivery, so an unhandled
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
Name: An update reminder is told once, not every hour
Category: Fix
Description: A plugin with an update waiting added a fresh "Update available" notification to your bell every time your installation checked — several a day, per package, for as long as you did not act. You now get one reminder per version, it stays dismissed once you dismiss it, and a genuinely newer build still gets its own.
Icon: Alert
Order: -20260903
---

# An update reminder is told once, not every hour

When a plugin you have installed publishes a new build, your installation tells you so with an
**Update available** notification on the bell, and leaves the decision to you. That is the intended
behaviour: nothing installs itself unless you asked it to.

What was not intended is that it told you again on every check. Your installation re-examines what
the plugin catalogue is serving whenever it starts, whenever it reads a catalogue, and whenever a
plugin repository publishes — and each of those checks added a *new* notification, because the
reminder had no memory of having been given. A package you had simply not got round to updating
produced several identical rows a day, for as long as you left it. On our own portal, **124 of the
newest 200 notifications** were this one message about two packages.

The effect was worse than clutter. Those rows are what the bell has to read every time you open it,
so a reminder you had already seen made the bell slower for everyone who could see it — and a
notification you had dismissed came straight back.

**A reminder is now a statement about a version, not an event.** The first time a new build of a
package is available, you get one notification. Every later check that still sees the same build
says nothing at all: no new row, no re-lit bell, and nothing fetched. If you dismiss it, it stays
dismissed.

When the plugin publishes a genuinely newer build, that is a different thing to be told about, and
you get a fresh reminder for it — which is the whole point of the message.
45 changes: 42 additions & 3 deletions src/MeshWeaver.Graph/NotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,30 @@ public static class NotificationService
/// Returns an IObservable that emits the created node and completes —
/// subscribe to drive the write. Safe to compose inside hub handlers /
/// click actions via Subscribe.
///
/// <para><b>An EVENT gets a fresh id; a STATE gets an <paramref name="identity"/>.</b> Left
/// null, every call mints a new GUID, which is right for a notification that reports something
/// that happened once ("your response is ready"). It is WRONG for one that reports a standing
/// condition an emitter re-evaluates on a schedule — "package X has version Y available" — and
/// that difference is the whole of Systemorph/MeshWeaver#3213: a reminder with no identity has
/// nothing for a repeat to collide with, so the reconciler minted a new bell row per poll,
/// forever (124 of the newest 200 rows on memex-cloud).</para>
/// </summary>
/// <param name="identity">
/// A caller-supplied identity for the CONDITION this notification reports (e.g.
/// <c>"package-update|update-available|{moduleVersion}"</c>). When given, the node id is
/// derived deterministically from it plus <paramref name="mainNodePath"/>, and the write is an
/// atomic upsert — so a repeat for the same condition lands on the SAME node and refreshes it
/// instead of adding a row. Change what the condition IS (a newer version) and the id changes
/// with it, so the new state gets its own unread bell. Null = the historic
/// fresh-GUID-per-call behaviour.
/// <para>🚨 The identity must capture everything that makes two reminders genuinely different.
/// Too coarse and a real change is swallowed into an existing row; the emitter, not this
/// method, owns that judgement. A repeat also refreshes <see cref="Notification.CreatedAt"/>
/// and clears <see cref="Notification.IsRead"/> — correct for "this is still true", which is
/// why the emitter should ALSO carry a marker that stops re-raising an unchanged condition
/// rather than relying on the upsert alone.</para>
/// </param>
public static IObservable<MeshNode> CreateNotification(
IMeshService nodeFactory,
string mainNodePath,
Expand All @@ -40,9 +63,18 @@ public static IObservable<MeshNode> CreateNotification(
NotificationType type,
string? targetNodePath = null,
string? createdBy = null,
string? icon = null)
string? icon = null,
string? identity = null)
{
var notificationId = Guid.NewGuid().AsString();
var deterministic = !string.IsNullOrEmpty(identity);
// Reuses the platform's ONE content-addressing helper rather than growing a second hash:
// it is exactly the (path, token) → stable-id shape the content-addressed import marker
// (`{Partition}/_Activity/import-{fingerprint}`) already uses, and its output is 16 lower
// hex chars — always a legal node-id segment, whatever characters the caller's identity
// string happens to contain.
var notificationId = deterministic
? PartitionSourceFingerprint.Compute([(mainNodePath, identity!)])
: Guid.NewGuid().AsString();
var parentPath = $"{mainNodePath}/{SatelliteSegment}";

var notification = new Notification
Expand All @@ -67,7 +99,14 @@ public static IObservable<MeshNode> CreateNotification(
Content = notification
};

return nodeFactory.CreateNode(node);
// 🚨 The upsert is the OWNER's single verb, not a client-side
// CreateNode().Catch(exists → UpdateNode()): two reconcile passes racing on the same
// deterministic path are exactly what this id makes possible, and the hand-rolled split
// races (the create's exists-check lags the concurrent create → the update patches a
// not-yet-materialised node → "NotFound … for patch apply").
return deterministic
? nodeFactory.CreateOrUpdateNode(node)
: nodeFactory.CreateNode(node);
}

private static readonly TimeSpan LookupTimeout = TimeSpan.FromSeconds(10);
Expand Down
28 changes: 28 additions & 0 deletions src/MeshWeaver.PluginCatalog/Package.cs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,34 @@ public record PackageManifest
/// </summary>
public bool AutoUpdate { get; init; }

/// <summary>
/// The candidate <see cref="ModuleVersion"/> this installation has ALREADY told the user
/// about — the reminder path's own silence gate, and the second half of
/// Systemorph/MeshWeaver#3213.
///
/// <para><b>Why <see cref="ModuleVersion"/> alone cannot do this job.</b> The content-identity
/// gate in <c>PackageUpdateReconciler.Decide</c> asks <em>is the candidate INSTALLED?</em>.
/// On the <see cref="AutoUpdate"/> path that is self-silencing: the apply advances
/// <see cref="ModuleVersion"/>, so the next poll compares equal. On the reminder path nothing
/// is installed — by design, the user has not acted — so the same comparison is
/// <b>unsatisfiable</b> and evaluated false on every subsequent poll, forever. The notify path
/// needs the other question, <em>have I already told them about this candidate?</em>, and this
/// is where its answer lives.</para>
///
/// <para>It is deliberately a fact on the DURABLE record rather than an in-memory de-dup
/// cache: a cache loses its memory on every pod start and re-notifies, which is the symptom
/// itself, and process-wide static state is forbidden here anyway.</para>
///
/// <para>Written by the reconciler AFTER the reminder is raised (so a failed write costs a
/// harmless repeat, which the notification's deterministic identity then absorbs, rather than
/// a lost reminder). Null on every record that has never been reminded, which is every record
/// written before this field existed — so the first poll after an upgrade reminds once and
/// then goes quiet. A record whose <see cref="ModuleVersion"/> catches up (the user clicked
/// Update) is silenced by the content-identity gate before this one is ever consulted, so a
/// re-stamp that drops this field cannot resurrect the storm.</para>
/// </summary>
public string? NotifiedModuleVersion { get; init; }

/// <summary>
/// The module manifest's per-file hash map as it stands AT THE CATALOG'S REF
/// (<see cref="ModuleManifest.Files"/>) — the candidate side of the diff, where
Expand Down
Loading
Loading