From 33958d78aedf385a001817c7037458a3187df825 Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 19 Aug 2026 20:09:08 +0200 Subject: [PATCH] refactor(#3255): a constant, not a module with an essay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review call: 46 lines of documentation around a single constant, part of it already stale. The worst paragraph explained why the earlier per-hive shape had been justified wrongly — history of a design that never shipped, written into the file within an hour of that design being dropped. A file is not a changelog; why it was wrong belongs in the PR. The constant moves to lib.rs beside the status bucket name, keeping only the rationale that stays true: three crates must agree on the string, and the one that agrees hardest speaks neither jetstream nor kv, which is why it cannot sit behind a feature gate. status earns a module of its own because it holds a bucket name AND the functions that open it. This held a constant. --- hive-c0re/src/swarm_status.rs | 2 +- swarm-controller/src/webhook.rs | 2 +- swarm-nats-auth/src/policy.rs | 6 ++-- swarm-queue-client/src/knowledge.rs | 46 ----------------------------- swarm-queue-client/src/lib.rs | 18 +++++------ 5 files changed, 12 insertions(+), 62 deletions(-) delete mode 100644 swarm-queue-client/src/knowledge.rs diff --git a/hive-c0re/src/swarm_status.rs b/hive-c0re/src/swarm_status.rs index 072b5fdd..d4310b91 100644 --- a/hive-c0re/src/swarm_status.rs +++ b/hive-c0re/src/swarm_status.rs @@ -212,7 +212,7 @@ async fn drain_swarm_events( // One subject for the whole swarm, so this hive's own name never enters // it: the controller publishes once and core NATS fans out to whoever is // subscribed. - let subject = swarm_queue_client::knowledge::SUBJECT; + let subject = swarm_queue_client::KNOWLEDGE_SUBJECT; let mut sub = match client.subscribe(subject).await { Ok(sub) => sub, Err(e) => { diff --git a/swarm-controller/src/webhook.rs b/swarm-controller/src/webhook.rs index 740cf358..61417e22 100644 --- a/swarm-controller/src/webhook.rs +++ b/swarm-controller/src/webhook.rs @@ -416,7 +416,7 @@ async fn announce_knowledge_change(state: &AppState) { return; }; let client = status.queue_client(); - let subject = swarm_queue_client::knowledge::SUBJECT; + let subject = swarm_queue_client::KNOWLEDGE_SUBJECT; // One publish, not one per hive: every subscriber gets the same empty // event, so the roster is not consulted at all. The controller does not diff --git a/swarm-nats-auth/src/policy.rs b/swarm-nats-auth/src/policy.rs index 23e21911..1eb3e083 100644 --- a/swarm-nats-auth/src/policy.rs +++ b/swarm-nats-auth/src/policy.rs @@ -246,7 +246,7 @@ impl Policy { // a **timeout**, so the visible failure is a hive that never hears // about a change, with nothing in the controller's log to say a // permission was the reason. - swarm_queue_client::knowledge::SUBJECT.to_owned(), + swarm_queue_client::KNOWLEDGE_SUBJECT.to_owned(), ]); subjects } @@ -313,7 +313,7 @@ mod tests { let p = policy().permissions("swarm-controller").expect("a reader"); assert!( p.publish - .contains(&swarm_queue_client::knowledge::SUBJECT.to_owned()), + .contains(&swarm_queue_client::KNOWLEDGE_SUBJECT.to_owned()), "the controller is the only publisher of this event; without the \ grant its publish is refused, and a refusal arrives as a timeout" ); @@ -336,7 +336,7 @@ mod tests { assert!( !p.publish .iter() - .any(|s| s == swarm_queue_client::knowledge::SUBJECT), + .any(|s| s == swarm_queue_client::KNOWLEDGE_SUBJECT), "a hive must not publish the knowledge event: {:?}", p.publish ); diff --git a/swarm-queue-client/src/knowledge.rs b/swarm-queue-client/src/knowledge.rs deleted file mode 100644 index a62e2977..00000000 --- a/swarm-queue-client/src/knowledge.rs +++ /dev/null @@ -1,46 +0,0 @@ -//! The subject carrying *the knowledge repository changed*. -//! -//! One writer, many readers: the swarm controller publishes, every hive -//! subscribes. The controller does not need to know who the hives are to tell -//! them the repository moved — core NATS fans one publish out to whoever is -//! listening. -//! -//! # Why one subject rather than one per hive -//! -//! The payload is empty and means the same thing to every hive, so per-hive -//! addressing delivers exactly what one subject does, having first made the -//! publisher enumerate the roster and the grant carry a wildcard. Delivery is -//! at-most-once either way, and a hive that was down reconciles on its next -//! periodic pull — a missed event costs latency, not correctness. -//! -//! ⚠️ The per-hive shape was justified by the callout policy's rule that an -//! extra subject must contain `{hive}`. That rule governs what a *hive* may -//! publish; this subject lives in the controller's reader grant, which it does -//! not constrain. The argument came from the wrong half of the permission model. -//! -//! # Why the constant lives here -//! -//! Three consumers name it: the controller publishing, the callout responder -//! granting, and hive-c0re subscribing. A copied literal in the third gives the -//! worst failure of the set — a grant that looks right, a publish that is -//! refused, and, because a NATS denial reaches the client as a timeout, nothing -//! saying so. Unconditional and NATS-type-free for the same reason -//! [`crate::status`]'s bucket name is: the responder speaks neither `jetstream` -//! nor `kv`. - -/// The subject the swarm controller publishes on when the hive-wide knowledge -/// repository has changed. -/// -/// `$SWARM` rather than a bare name: the `$` prefix is NATS' convention for -/// system-ish subjects, keeping it clear of anything an application might -/// choose for itself. -/// -/// Semantic, not transport-shaped — it says what happened, not that a forge -/// webhook arrived. The controller interprets a delivery and decides this is -/// what it meant; a hive receiving it does not need to know a forge exists. -/// -/// No unit test here: a single constant has no structure to assert, and a test -/// comparing it to a second spelling is the failure this module exists to -/// prevent. The property worth testing is *who may publish it*, which lives -/// with the policy that decides — see `swarm-nats-auth`. -pub const SUBJECT: &str = "$SWARM.knowledge"; diff --git a/swarm-queue-client/src/lib.rs b/swarm-queue-client/src/lib.rs index 0213373e..c48f981b 100644 --- a/swarm-queue-client/src/lib.rs +++ b/swarm-queue-client/src/lib.rs @@ -156,18 +156,14 @@ pub fn chain(error: &dyn std::error::Error) -> String { /// which is the disagreement this module exists to prevent. pub mod status; -/// The *knowledge repository changed* subject — one writer (the controller), -/// many readers (the hives). +/// The subject the swarm controller publishes on when the hive-wide knowledge +/// repository has changed. One writer, many readers — every hive subscribes. /// -/// Its own category rather than a leaf under a general event namespace: a -/// namespace built for events that do not exist yet is a shape decided before -/// there is anything to decide it from. -/// -/// Unconditional and NATS-type-free for the same reason the bucket name above -/// is: three crates must agree on this string, and the one that agrees hardest -/// — the auth-callout responder, which decides whether a publish is even -/// permitted — speaks neither `jetstream` nor `kv`. -pub mod knowledge; +/// Here rather than in a module of its own for the same reason as the bucket +/// name above: three crates must agree on the string, and the one that agrees +/// hardest — the auth-callout responder, which decides whether a publish is +/// permitted at all — speaks neither `jetstream` nor `kv`. +pub const KNOWLEDGE_SUBJECT: &str = "$SWARM.knowledge"; /// Only the fields this needs; authelia returns several. #[derive(serde::Deserialize)]