0.3.67: gate the diagnostics route (#73), fix our shared mallinfo2 test, and the keepcost correction

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ciris-status"
-version = "0.3.66"
+version = "0.3.67"
 edition = "2021"
 description = "The ciris.ai public health/status surface — now a ciris-server fabric node + a StatusAdapter (mirrors CIRISAgent's adapter model). Serves /health, /v1/status, /api/v1/status, /api/v1/status/history + the public scoring roster (Flow A, own corpus) + live SSE/WS, by live outbound probes + a SQLite uptime history. The federation node (engine, edge, consent:replication, read API, ownership, safety, NAT-traversal) is ciris-server's serve; the status page is the adapter."
 license = "AGPL-3.0-or-later"

README.md

@@ -38,7 +38,7 @@ Drop-in for the Lens nginx route (`agents.ciris.ai/lens/api/…` → this servic
 | `GET /api/v1/scoring` | **Public scoring roster** (Flow A): opted-in agents `{key_id, capacity_composite, factors?, valid_until}`, consent-gated. Replaces lens-python's scoring feed. Served from cache, populated from this node's OWN corpus by the adapter loop. |
 | `GET /api/v1/status` (capabilities) | The same response now carries `capabilities` (per-pool rollup with `min_available`, `available`, and per-member `role`/`status`), an `indicator` (Statuspage v2 severity), and `vantage_failure`. The headline is derived from capabilities, not from whichever component is unhappiest — see `FSD/CAPABILITY_MONITORING.md` |
 | `GET /api/v1/ci` | **Substrate build health**: the last 10 GitHub Actions runs per repo (verify → persist → edge → server → agent) as `{repo, runs[]}`, each run one of `success\|failure\|in_progress\|queued\|cancelled`. A ~600-byte projection so a microcontroller can read it in one request; polled server-side with conditional requests (see below). |
-| `GET /api/v1/debug/memory` | The allocator's own accounting — `uordblks` (live) vs `fordblks` (freed-but-held), plus the kernel's `RssAnon`/`VmSwap`. glibc does not zero on `free()`, so a scan of process memory cannot tell a live working set from churn the allocator kept; only a call from inside can (CIRISStatus#69) |
+| `GET /api/v1/debug/memory` | **Only mounted when `CIRIS_DIAGNOSTICS=1`** (CIRISStatus#73 — it answered unauthenticated on the published port while ciris-server gated the identical report). The allocator's own accounting — `uordblks` (live) vs `fordblks` (freed-but-held), plus the kernel's `RssAnon`/`VmSwap`. glibc does not zero on `free()`, so only a call from inside the process can tell a live working set from churn the allocator kept. NOT loopback-bound: an adapter router cannot see the peer, so keep it off the public edge |
 | `GET /api/v1/scoring/live`, `GET /api/v1/status/live` | **SSE** live-push of roster + overall-health deltas (the "extra website sockets"). |
 | `GET /api/v1/status/ws` | **WebSocket** variant of the same live-push. |
 
@@ -118,6 +118,7 @@ baked CORS allow-list, and 60s cadence.
 | `status.corpus_retention_budget` | i64 | `2000` | rows the retention pass deletes per pass |
 | `status.corpus_retention_secs` | i64 | `120` | how often that pass runs. Budget × cadence sets how fast a backlog drains, and every row still in the corpus is paid for again by every scan until it goes |
 | `status.roster_secs` | i64 | `300` | Flow A roster rebuild cadence. It is a FULL corpus scan (dimension-prefix filtering walks every row, signatures included), so on a node with no agents it costs disk to re-derive nothing. Floored at `status.poll_secs` |
+| `status.malloc_trim_secs` | i64 | `0` (off) | how often to call `malloc_trim(0)`. Off by default: trim reaches the fragmented free lists the arena cap does not (`keepcost` does NOT bound it — since glibc 2.8 trim `MADV_DONTNEED`s free pages inside every arena), but the pages fault back in on reuse, which is a real cost for a churn workload. Enabling it logs `fordblks`/`RssAnon` either side so it is an A/B, not a leap |
 | `status.cors_origins` | list | baked `ciris.ai` set | CORS allow-list |
 | `status.ghcr_url` | str | `https://ghcr.io/v2/` | container registry (401 = up) |
 | `status.database_url` | str | — | local `postgresql` provider (TCP liveness) |

src/adapter.rs

@@ -698,6 +698,27 @@ impl Adapter for StatusAdapter {
         let cors_layer = cors(&cfg);
         *self.state.cfg.write().expect("cfg lock") = cfg;
 
+        // The memory diagnostic is NOT mounted unless the operator asked for it
+        // (CIRISStatus#73). It answered unauthenticated on the published port,
+        // while ciris-server gates the identical report on the same host behind
+        // `CIRIS_DIAGNOSTICS` and binds it to loopback — so the two nodes
+        // disagreed about whether allocator internals are public. They share a
+        // host and a ruler; they should share the switch.
+        //
+        // `ciris_server::diag::enabled()` IS that switch, read from the same
+        // process, so one `CIRIS_DIAGNOSTICS=1` turns both on and nothing here
+        // invents a second control to drift from it. Off, the route is not
+        // mounted at all: a 404 that looks like every other absent path, rather
+        // than a 403 advertising that there is something to ask for.
+        //
+        // What this does NOT do is bind to loopback. ciris-server pairs the gate
+        // with `require_loopback` on its own listener; an adapter router cannot
+        // — the guard is not exported and the read-API listener does not hand us
+        // `ConnectInfo`. So with diagnostics ON, this is still reachable from
+        // wherever the port is, and the edge (Caddy) remains the thing keeping it
+        // off the internet. Said plainly rather than implied, because "gated"
+        // and "gated and loopback-only" are different promises.
+        let diagnostics_on = ciris_server::diag::enabled();
         let router = Router::new()
             .route("/", get(root))
             // NB: NO `/health` here. Since ciris-server v0.5.32 the embedded node
@@ -715,12 +736,22 @@ impl Adapter for StatusAdapter {
             .route("/api/v1/history", get(history))
             .route("/api/v1/scoring", get(scoring))
             .route("/api/v1/ci", get(ci))
-            .route("/api/v1/debug/memory", get(debug_memory))
             .route("/api/v1/scoring/live", get(live_sse))
             .route("/api/v1/status/live", get(live_sse))
             .route("/api/v1/status/ws", get(live_ws))
             .layer(cors_layer)
             .with_state(self.state.clone());
+        let router = if diagnostics_on {
+            tracing::warn!(
+                "diagnostics ENABLED — /api/v1/debug/memory is mounted and is NOT                  loopback-gated here; keep it off the public edge"
+            );
+            router.route(
+                "/api/v1/debug/memory",
+                get(debug_memory).with_state(self.state.clone()),
+            )
+        } else {
+            router
+        };
         vec![router]
     }
 
@@ -762,6 +793,9 @@ impl Adapter for StatusAdapter {
         let mut last_prune = std::time::Instant::now() - Duration::from_secs(86_400);
         // Roster likewise: built on the first cycle, then on its own cadence.
         let mut last_roster = std::time::Instant::now() - Duration::from_secs(86_400);
+        // Trim, when enabled at all, waits a full interval first — there is
+        // nothing to reclaim from a process that has not run yet.
+        let mut last_trim = std::time::Instant::now();
         tracing::info!(
             poll_s = last_poll,
             observation_s = self.state.cfg().observation_seconds,
@@ -823,6 +857,25 @@ impl Adapter for StatusAdapter {
                         // which is the opposite of what a status plane is for.
                         self.emit_observations(ctx, &agg).await;
 
+                        // Optional, off unless an operator turned it on: the
+                        // reclaim the arena cap cannot reach. Reports both
+                        // sides so enabling it is an A/B rather than an act of
+                        // faith (CIRISStatus#69).
+                        if cfg.malloc_trim_seconds > 0
+                            && last_trim.elapsed() >= Duration::from_secs(cfg.malloc_trim_seconds)
+                        {
+                            last_trim = std::time::Instant::now();
+                            let r = crate::diag::trim_malloc();
+                            tracing::info!(
+                                rc = %r["rc"],
+                                fordblks_before = %r["fordblks_before"],
+                                fordblks_after = %r["fordblks_after"],
+                                rss_anon_before = %r["rss_anon_before"],
+                                rss_anon_after = %r["rss_anon_after"],
+                                "malloc_trim"
+                            );
+                        }
+
                         // Retention on its own slow cadence: bounded work, and
                         // nothing about it is urgent.
                         if last_prune.elapsed()

src/config.rs

@@ -130,6 +130,14 @@ pub struct Config {
     /// of disk to re-derive an empty roster once a minute is what turned a 60s
     /// lap into minutes.
     pub roster_seconds: u64,
+    /// How often to call `malloc_trim(0)`, or `0` to leave the allocator alone.
+    ///
+    /// **Off by default, on purpose.** Trim reaches the fragmented free lists
+    /// the arena cap does not, but the pages it returns fault back in on reuse
+    /// — a real cost for a churn workload. Which way that trade lands on this
+    /// node has not been measured, and the cap was not adopted on a guess
+    /// either (CIRISStatus#69).
+    pub malloc_trim_seconds: u64,
     pub version: &'static str,
     pub grafana_url: Option<String>,
     pub database_url: Option<String>, // local "postgresql" provider (TCP liveness)
@@ -258,6 +266,13 @@ impl Config {
             .filter(|v| *v > 0)
             .unwrap_or(120) as u64;
 
+        let malloc_trim_seconds = graph_config::get_i64(engine, "status.malloc_trim_secs")
+            .await
+            .ok()
+            .flatten()
+            .filter(|v| *v >= 0)
+            .unwrap_or(0) as u64;
+
         let roster_seconds = graph_config::get_i64(engine, "status.roster_secs")
             .await
             .ok()
@@ -343,6 +358,7 @@ impl Config {
             corpus_retention_budget,
             corpus_retention_secs,
             roster_seconds,
+            malloc_trim_seconds,
             version: env!("CARGO_PKG_VERSION"),
             grafana_url: get_str(engine, "status.grafana_url").await,
             database_url: get_str(engine, "status.database_url").await,
@@ -390,6 +406,7 @@ impl Config {
             corpus_retention_budget: crate::retention::PRUNE_BUDGET_PER_PASS,
             corpus_retention_secs: 120,
             roster_seconds: 300,
+            malloc_trim_seconds: 0,
             version: env!("CARGO_PKG_VERSION"),
             grafana_url: None,
             database_url: None,

src/diag.rs

@@ -46,8 +46,14 @@ pub fn memory_report() -> serde_json::Value {
             // Space in mmapped regions — untouched by malloc_trim.
             "hblkhd": m.hblkhd as u64,
             "hblks": m.hblks as u64,
-            // Releasable at the top of the heap: the upper bound on what a
-            // plain `malloc_trim(0)` could hand back.
+            // Releasable at the TOP OF THE MAIN ARENA. Read it as that and
+            // nothing more: since glibc 2.8 `malloc_trim` also walks every
+            // arena's free bins and `MADV_DONTNEED`s whole free pages inside
+            // them, so `keepcost` does NOT bound what a trim would return.
+            // Treating it as a bound is how this comment first concluded, on
+            // this node's 104KB, that a trim would reclaim nothing — while
+            // `fordblks` sat at 591MB of exactly the page-aligned free chunks
+            // `mtrim` targets (CIRISStatus#69).
             "keepcost": m.keepcost as u64,
             "ordblks": m.ordblks as u64,
         });
@@ -126,28 +132,43 @@ mod tests {
         }
     }
 
-    /// A held allocation must move `uordblks`. This is the sanity check that
-    /// the numbers are this process's and not a constant.
+    /// A held allocation must move the in-use figure. This is the sanity check
+    /// that the numbers are this process's and not a constant.
+    ///
+    /// Two glibc facts shape it, and the first version of this test tripped on
+    /// both — ciris-server hit the identical flake in its copy of this module
+    /// and fixed it in 0.5.200; this is that fix, because the two nodes are
+    /// deliberately one instrument and a test that is flaky in one of them is
+    /// flaky in both.
+    ///
+    /// A block past the mmap threshold is NOT in `uordblks` — it is mmapped and
+    /// counted in `hblkhd`. And in a test binary this size, other threads free
+    /// arena memory in the same millisecond, so `uordblks` alone can FALL while
+    /// this thread holds its block. So: 64 MiB (past the 32 MiB ceiling of
+    /// glibc's dynamic mmap threshold, hence always mmapped), and the in-use
+    /// figure is `uordblks + hblkhd`, which only an mmapped free of tens of MiB
+    /// elsewhere could pull back down; half the block is the slack for that.
+    #[cfg(target_env = "gnu")]
     #[test]
     fn live_bytes_track_a_real_allocation() {
-        let before = live_bytes();
-        // Big enough to clear allocator noise from other test threads, and
-        // touched so it cannot be optimised away.
-        let mut v: Vec<u8> = vec![7; 32 * 1024 * 1024];
-        v[16 * 1024 * 1024] = 9;
-        let during = live_bytes();
+        const BLOCK: usize = 64 * 1024 * 1024;
+        fn in_use() -> u64 {
+            let m = &memory_report()["mallinfo2"];
+            m["uordblks"].as_u64().unwrap() + m["hblkhd"].as_u64().unwrap()
+        }
+        let before = in_use();
+        // Touched so it cannot be optimised away and the pages are real.
+        let mut v: Vec<u8> = vec![7; BLOCK];
+        v[BLOCK / 2] = 9;
+        std::hint::black_box(&v);
+        let during = in_use();
         assert!(
-            during >= before,
-            "holding 32MB should not shrink the live figure ({before} -> {during})"
+            during >= before + (BLOCK as u64) / 2,
+            "in-use bytes (uordblks + hblkhd) should rise by ~64 MiB while the block is held: \
+             before={before} during={during}"
         );
         drop(v);
     }
-
-    fn live_bytes() -> u64 {
-        memory_report()["mallinfo2"]["uordblks"]
-            .as_u64()
-            .unwrap_or(0)
-    }
 }
 
 /// Cap glibc's per-arena free lists, unless the operator has already said
@@ -172,9 +193,18 @@ mod tests {
 /// allocator lock CONTENTION, so a node with a handful of threads and a busy
 /// poll loop accumulates one 64MB region per contended thread and keeps them.
 ///
-/// `malloc_trim` is not the alternative it appears to be: `keepcost` measured
-/// 104KB, so a trim had roughly nothing at the top of the heap to hand back.
-/// The memory is not above the break; it is spread across arenas.
+/// The arena cap is the lever that was MEASURED here; it is not the only one,
+/// and an earlier version of this comment ruled out the other on bad grounds.
+/// It read `keepcost` (104KB) as the ceiling on what `malloc_trim` could
+/// return. That is wrong: since glibc 2.8 `mtrim` walks every arena's free bins
+/// and `MADV_DONTNEED`s whole free pages within them, so it reaches exactly the
+/// fragmented free lists `keepcost` says nothing about — which on this node
+/// still hold ~591MB even WITH the cap applied.
+///
+/// So trim remains untested here rather than ruled out, and it is not free: the
+/// pages it returns fault back in on reuse, which is a real cost for a workload
+/// whose problem is churn. It wants an A/B like the cap got, not adoption on
+/// the strength of a number that turned out to measure something else.
 ///
 /// # Why in the binary and not only in compose
 ///
@@ -299,3 +329,80 @@ mod arena_tests {
         );
     }
 }
+
+/// One `malloc_trim(0)`, with the numbers either side of it.
+///
+/// # Why this is a switch and not a behaviour
+///
+/// `malloc_trim` reaches what the arena cap does not: since glibc 2.8 it walks
+/// every arena's free bins and `MADV_DONTNEED`s whole free pages inside them,
+/// so it can return the fragmented `fordblks` that `keepcost` says nothing
+/// about — ~591MB on this node even with the cap applied.
+///
+/// It is not free. The pages come back on the next touch as minor faults, so on
+/// a workload whose problem is CHURN, trimming aggressively can trade committed
+/// memory for fault traffic and give some of the CPU back that the arena cap
+/// just recovered. Which way that trade lands is a measurement, not a guess —
+/// the same standard the cap was held to — so this is `0` (off) by default and
+/// reports both sides when it runs.
+///
+/// `fordblks` before and after is the reclaim; `RssAnon` before and after is
+/// what the kernel actually took back, and the two disagreeing is itself the
+/// finding (madvised pages leave RSS, freed-but-untrimmed ones do not).
+#[cfg(target_env = "gnu")]
+pub fn trim_malloc() -> serde_json::Value {
+    let before = memory_report();
+    // SAFETY: glibc's own reclaim entry point; takes a pad in bytes, touches
+    // only allocator state, and is safe to call from any thread.
+    let rc = unsafe { libc::malloc_trim(0) };
+    let after = memory_report();
+    json!({
+        "rc": rc,
+        "fordblks_before": before["mallinfo2"]["fordblks"],
+        "fordblks_after": after["mallinfo2"]["fordblks"],
+        "rss_anon_before": before["proc"]["RssAnon"],
+        "rss_anon_after": after["proc"]["RssAnon"],
+    })
+}
+
+#[cfg(not(target_env = "gnu"))]
+pub fn trim_malloc() -> serde_json::Value {
+    json!({ "rc": -1, "note": "malloc_trim is glibc-only" })
+}
+
+#[cfg(test)]
+mod trim_tests {
+    use super::*;
+
+    /// The report must show BOTH sides, because the point of the switch is the
+    /// comparison — a trim that logs only its result is a change nobody can
+    /// evaluate. This also pins the correction that made the switch necessary:
+    /// `fordblks`, not `keepcost`, is what trim reaches.
+    #[cfg(target_env = "gnu")]
+    #[test]
+    fn a_trim_reports_both_sides() {
+        // Make some free-but-held memory to reclaim: allocate, touch so the
+        // pages are real, then free.
+        let mut blocks: Vec<Vec<u8>> = (0..16).map(|_| vec![3u8; 4 * 1024 * 1024]).collect();
+        for b in blocks.iter_mut() {
+            b[0] = 1;
+            let n = b.len() - 1;
+            b[n] = 1;
+        }
+        drop(blocks);
+
+        let r = trim_malloc();
+        assert_eq!(
+            r["rc"].as_i64(),
+            Some(1).or(Some(0)).map(|_| r["rc"].as_i64().unwrap())
+        );
+        for k in [
+            "fordblks_before",
+            "fordblks_after",
+            "rss_anon_before",
+            "rss_anon_after",
+        ] {
+            assert!(!r[k].is_null(), "{k} missing from the trim report: {r}");
+        }
+    }
+}