WIP(disk-space): enforce threshold ordering invariant (DC-059)

[UNJUDGED] Will run tests + GLM judge before final amend.

dashcaddy-api/routes/disk-space.js

@@ -1,5 +1,6 @@
 const express = require('express');
 const { success, error: errorResponse } = require('../src/utils/responses');
+const { ValidationError } = require('../src/utilities/errors');
 
 /**
  * Disk space management routes
@@ -10,6 +11,76 @@ const { success, error: errorResponse } = require('../src/utils/responses');
  * POST /disk/config          — update disk budget settings
  * POST /disk/cleanup         — trigger manual cleanup (standard|aggressive|logs-only)
  */
+
+// DC-059: monotonic-ordering invariant for the three threshold percentages.
+// DiskSpaceMonitor._getBudgetStatus() walks them in order
+// (cleanupAggressivePct → criticalThresholdPct → warningThresholdPct) and
+// returns at the FIRST threshold the usage crosses. If a caller writes
+// them out of order (e.g. warningThresholdPct=95, criticalThresholdPct=60),
+// the higher-priority branches become unreachable and the monitor silently
+// misclassifies budget state. Validate against the *effective* config
+// (current value + incoming update for each field) so partial updates can
+// be applied one field at a time without violating the invariant.
+//
+// Clamp values to the same ranges the previous inline Math.min/Math.max
+// chains enforced (warning 50..99, critical 60..99, aggressive 70..99)
+// so we don't loosen the original bounds while adding the new check.
+const THRESHOLD_BOUNDS = Object.freeze({
+  warning:    { min: 50, max: 99 },
+  critical:   { min: 60, max: 99 },
+  aggressive: { min: 70, max: 99 },
+});
+
+function clampThreshold(name, value) {
+  const { min, max } = THRESHOLD_BOUNDS[name];
+  return Math.min(Math.max(value, min), max);
+}
+
+/**
+ * Apply a candidate update to a baseline config, then verify the three
+ * threshold percentages still satisfy
+ *   warningThresholdPct < criticalThresholdPct < cleanupAggressivePct.
+ * The POST /config endpoint accepts partial updates (single field at a
+ * time), so we merge into the live diskSpaceMonitor config first, then test
+ * the merged value. Returns the merged candidate on success; throws
+ * ValidationError if the ordering invariant would be violated.
+ *
+ * @param {Object} baseline - current effective config from diskSpaceMonitor
+ * @param {Object} candidate - the partial update being applied this request
+ * @returns {Object} merged candidate with thresholds clamped to bounds
+ */
+function mergeAndCheckOrdering(baseline, candidate) {
+  const next = { ...baseline };
+  if (typeof candidate.warningThresholdPct === 'number') {
+    next.warningThresholdPct = clampThreshold('warning', candidate.warningThresholdPct);
+  }
+  if (typeof candidate.criticalThresholdPct === 'number') {
+    next.criticalThresholdPct = clampThreshold('critical', candidate.criticalThresholdPct);
+  }
+  if (typeof candidate.cleanupAggressivePct === 'number') {
+    next.cleanupAggressivePct = clampThreshold('aggressive', candidate.cleanupAggressivePct);
+  }
+  if (!(next.warningThresholdPct < next.criticalThresholdPct)) {
+    throw new ValidationError(
+      `warningThresholdPct (${next.warningThresholdPct}) must be strictly less than criticalThresholdPct (${next.criticalThresholdPct})`,
+      'warningThresholdPct'
+    );
+  }
+  if (!(next.criticalThresholdPct < next.cleanupAggressivePct)) {
+    throw new ValidationError(
+      `criticalThresholdPct (${next.criticalThresholdPct}) must be strictly less than cleanupAggressivePct (${next.cleanupAggressivePct})`,
+      'criticalThresholdPct'
+    );
+  }
+  // Return only the fields the caller asked to change (preserves partial-
+  // update semantics; diskSpaceMonitor.configure does its own merge).
+  const out = {};
+  if (typeof candidate.warningThresholdPct === 'number') out.warningThresholdPct = next.warningThresholdPct;
+  if (typeof candidate.criticalThresholdPct === 'number') out.criticalThresholdPct = next.criticalThresholdPct;
+  if (typeof candidate.cleanupAggressivePct === 'number') out.cleanupAggressivePct = next.cleanupAggressivePct;
+  return out;
+}
+
 module.exports = function({ diskSpaceMonitor, asyncHandler, log }) {
   const router = express.Router();
 
@@ -36,9 +107,16 @@ module.exports = function({ diskSpaceMonitor, asyncHandler, log }) {
 
     const updates = {};
     if (typeof diskBudgetGB === 'number' && diskBudgetGB > 0) updates.diskBudgetGB = Math.min(diskBudgetGB, 1000);
-    if (typeof warningThresholdPct === 'number') updates.warningThresholdPct = Math.min(Math.max(warningThresholdPct, 50), 99);
-    if (typeof criticalThresholdPct === 'number') updates.criticalThresholdPct = Math.min(Math.max(criticalThresholdPct, 60), 99);
-    if (typeof cleanupAggressivePct === 'number') updates.cleanupAggressivePct = Math.min(Math.max(cleanupAggressivePct, 70), 99);
+    // DC-059: threshold percentages must satisfy a strict monotonic order
+    // (warning < critical < aggressive) so _getBudgetStatus() reaches the
+    // correct branch. mergeAndCheckOrdering() validates against the live
+    // baseline, so partial updates that violate the invariant are rejected
+    // BEFORE we mutate diskSpaceMonitor.diskConfig.
+    const thresholdUpdates = mergeAndCheckOrdering(
+      diskSpaceMonitor.getConfig(),
+      { warningThresholdPct, criticalThresholdPct, cleanupAggressivePct }
+    );
+    Object.assign(updates, thresholdUpdates);
     if (typeof autoCleanup === 'boolean') updates.autoCleanup = autoCleanup;
     if (typeof enabled === 'boolean') updates.enabled = enabled;