Merge pull request #37 from closedloop-ai/FEAT-138

FEAT-133: Fix approval dialog labels and add none tier

Author: shafty023

apps/desktop/package.json

@@ -1,6 +1,6 @@
 {
   "name": "desktop",
-  "version": "0.7.0",
+  "version": "0.7.1",
   "description": "ClosedLoop Desktop",
   "author": "ClosedLoop AI <support@closedloop.ai>",
   "private": true,

apps/desktop/src/main/app.ts

@@ -29,7 +29,7 @@ import {
 } from "../server/operations/symphony-utils.js";
 import { seedReposConfig } from "./seed-repos-config.js";
 import { SUPPORTED_OPERATION_IDS, resolveOperationId } from "./approval-operations.js";
-import { shouldAutoApprove } from "./approval-policy.js";
+import { shouldAutoApprove, OPERATION_RISK_TIERS } from "./approval-policy.js";
 import { ActivityLogStore } from "./activity-log-store.js";
 import { ApprovalStore } from "./approval-store.js";
 import { JobStore, isTerminalJobStatus } from "./job-store.js";
@@ -553,20 +553,17 @@ export class DesktopApplication {
 
     const configuredTier = (settings.autoApprovalRules[operationId] ??
       settings.defaultApprovalTier) as RiskTier;
-    if (configuredTier === "auto" && !request.forceApproval) {
-      return { allow: true };
-    }
-    const manualTier: Exclude<RiskTier, "auto"> = configuredTier === "auto" ? "high" : configuredTier;
-    if (shouldAutoApprove(operationId, manualTier, request.forceApproval ?? false)) {
+    if (shouldAutoApprove(operationId, configuredTier, request.forceApproval ?? false)) {
       return { allow: true };
     }
 
+    const operationRisk = (OPERATION_RISK_TIERS as Record<string, Exclude<RiskTier, "none">>)[operationId] ?? "high";
     const reason =
       request.approvalReason?.trim() ||
-      `Manual approval required for ${operationId} (${manualTier})`;
+      `${operationId} is ${operationRisk}-risk, but your auto-approve threshold is ${configuredTier}`;
     const pending = this.approvalStore.enqueue({
       operationId,
-      riskTier: manualTier,
+      riskTier: operationRisk,
       method: request.method,
       path: request.path,
       body: request.body,
@@ -750,11 +747,20 @@ export class DesktopApplication {
         relayOrigin?: string;
         apiOrigin?: string;
         webAppOrigin?: string;
-        defaultApprovalTier?: "auto" | "low" | "medium" | "high";
-        autoApprovalRules?: Record<string, "auto" | "low" | "medium" | "high">;
+        defaultApprovalTier?: "auto" | "none" | "low" | "medium" | "high";
+        autoApprovalRules?: Record<string, "auto" | "none" | "low" | "medium" | "high">;
       }) => {
         const currentSettings = this.settingsStore.getAll();
         const nextPartial = { ...partial };
+        // Normalize legacy "auto" tier to "high" (they behave identically)
+        if (nextPartial.defaultApprovalTier === "auto") {
+          nextPartial.defaultApprovalTier = "high";
+        }
+        if (nextPartial.autoApprovalRules) {
+          for (const [key, val] of Object.entries(nextPartial.autoApprovalRules)) {
+            if (val === "auto") nextPartial.autoApprovalRules[key] = "high";
+          }
+        }
         if (typeof partial.relayOrigin === "string") {
           nextPartial.relayOrigin = normalizeAndValidateOrigin(partial.relayOrigin);
         }
@@ -782,7 +788,7 @@ export class DesktopApplication {
           throw new Error("Complete onboarding requires a sandbox base directory");
         }
 
-        const updated = this.settingsStore.update(nextPartial);
+        const updated = this.settingsStore.update(nextPartial as Partial<DesktopSettings>);
 
         if (
           typeof partial.sandboxBaseDirectory === "string" &&

apps/desktop/src/main/approval-policy.ts

@@ -5,7 +5,7 @@ import type { OperationId } from "./approval-operations.js";
  * Per-operation inherent risk tiers. The risk assigned reflects the
  * highest-risk HTTP method that each approval ID handles.
  */
-export const OPERATION_RISK_TIERS: Record<OperationId, Exclude<RiskTier, "auto">> = {
+export const OPERATION_RISK_TIERS: Record<OperationId, Exclude<RiskTier, "none">> = {
   health_check:            "low",
   repos_config:            "medium",
   filesystem:              "medium",
@@ -34,9 +34,10 @@ export const OPERATION_RISK_TIERS: Record<OperationId, Exclude<RiskTier, "auto">
   learnings:               "medium"
 };
 
-/** Converts a non-auto RiskTier to a numeric value for threshold comparison. */
-export function riskTierOrder(tier: Exclude<RiskTier, "auto">): number {
+/** Converts a RiskTier to a numeric value for threshold comparison. */
+export function riskTierOrder(tier: RiskTier): number {
   switch (tier) {
+    case "none": return 0;
     case "low": return 1;
     case "medium": return 2;
     case "high": return 3;
@@ -49,10 +50,10 @@ export function riskTierOrder(tier: Exclude<RiskTier, "auto">): number {
  */
 export function shouldAutoApprove(
   operationId: string,
-  configuredTier: Exclude<RiskTier, "auto">,
+  configuredTier: RiskTier,
   forceApproval: boolean
 ): boolean {
   if (forceApproval) return false;
-  const operationRisk = (OPERATION_RISK_TIERS as Record<string, Exclude<RiskTier, "auto">>)[operationId] ?? "high";
+  const operationRisk = (OPERATION_RISK_TIERS as Record<string, Exclude<RiskTier, "none">>)[operationId] ?? "high";
   return riskTierOrder(operationRisk) <= riskTierOrder(configuredTier);
 }

apps/desktop/src/main/approval-store.ts

@@ -8,7 +8,7 @@ export type PendingApproval = {
   id: string;
   createdAt: string;
   operationId: string;
-  riskTier: Exclude<RiskTier, "auto">;
+  riskTier: Exclude<RiskTier, "none">;
   method: string;
   path: string;
   scopePath?: string;
@@ -74,7 +74,7 @@ export class ApprovalStore {
 
   enqueue(input: {
     operationId: string;
-    riskTier: Exclude<RiskTier, "auto">;
+    riskTier: Exclude<RiskTier, "none">;
     method: string;
     path: string;
     body: string;

apps/desktop/src/main/settings-store.ts

@@ -66,6 +66,24 @@ export class SettingsStore {
     if (hadAuthApiOrigin) {
       this.store.delete("authApiOrigin" as keyof DesktopSettings);
     }
+
+    // Migration: replace legacy "auto" tier with "high" (identical behavior).
+    if (raw.defaultApprovalTier === "auto") {
+      this.store.set("defaultApprovalTier", "high" as RiskTier);
+    }
+    const rules = raw.autoApprovalRules as Record<string, string> | undefined;
+    if (rules) {
+      let rulesChanged = false;
+      for (const [key, val] of Object.entries(rules)) {
+        if (val === "auto") {
+          rules[key] = "high";
+          rulesChanged = true;
+        }
+      }
+      if (rulesChanged) {
+        this.store.set("autoApprovalRules", rules as unknown as Record<string, RiskTier>);
+      }
+    }
   }
 
   getAll(): DesktopSettings {

apps/desktop/src/renderer/index.html

@@ -514,6 +514,48 @@
         line-height: 1.4;
       }
 
+      .approval-risk-tier {
+        display: inline-flex;
+        align-items: center;
+        border-radius: 4px;
+        padding: 1px 7px;
+        font-size: 10px;
+        font-weight: 700;
+        letter-spacing: 0.04em;
+        text-transform: uppercase;
+        flex-shrink: 0;
+      }
+
+      .approval-risk-tier.risk-low {
+        background: rgba(22, 163, 74, 0.12);
+        color: #16a34a;
+      }
+
+      .approval-risk-tier.risk-medium {
+        background: rgba(245, 158, 11, 0.12);
+        color: #d97706;
+      }
+
+      .approval-risk-tier.risk-high {
+        background: rgba(220, 38, 38, 0.12);
+        color: #dc2626;
+      }
+
+      @media (prefers-color-scheme: dark) {
+        .approval-risk-tier.risk-low {
+          background: rgba(22, 163, 74, 0.18);
+          color: #4ade80;
+        }
+        .approval-risk-tier.risk-medium {
+          background: rgba(245, 158, 11, 0.18);
+          color: #fbbf24;
+        }
+        .approval-risk-tier.risk-high {
+          background: rgba(220, 38, 38, 0.18);
+          color: #f87171;
+        }
+      }
+
       .approval-reason {
         color: var(--ink);
         font-size: 13px;
@@ -1427,10 +1469,10 @@ <h3 class="settings-group-title">Approval Policy</h3>
             <div class="row">
               <label for="defaultApprovalTier">Default Approval Tier</label>
               <select id="defaultApprovalTier">
-                <option value="high">high -- approve everything</option>
-                <option value="medium">medium -- approve risky operations</option>
-                <option value="low">low -- approve only destructive operations</option>
-                <option value="auto">auto -- never prompt</option>
+                <option value="high">high -- auto-approve all operations</option>
+                <option value="medium">medium -- prompt only for high-risk (e.g. deploy)</option>
+                <option value="low">low -- prompt for medium and high-risk operations</option>
+                <option value="none">none -- prompt for all operations</option>
               </select>
             </div>
             <div class="row">
@@ -1445,7 +1487,7 @@ <h3 class="settings-group-title">Approval Policy</h3>
                   <option value="high">high</option>
                   <option value="medium">medium</option>
                   <option value="low">low</option>
-                  <option value="auto">auto</option>
+                  <option value="none">none</option>
                 </select>
                 <button class="secondary" id="tierOverrideAddBtn" type="button">Add</button>
               </div>
@@ -2114,6 +2156,13 @@ <h3 class="settings-group-title">Always-Allow Rules</h3>
         title.textContent = operationLabel(approval.operationId);
         header.appendChild(title);
 
+        if (approval.riskTier) {
+          const riskBadge = document.createElement("span");
+          riskBadge.className = `approval-risk-tier risk-${approval.riskTier}`;
+          riskBadge.textContent = approval.riskTier;
+          header.appendChild(riskBadge);
+        }
+
         const badge = document.createElement("span");
         badge.className = `approval-badge ${statusKey}`;
         badge.textContent = resolved
@@ -2232,7 +2281,7 @@ <h3 class="settings-group-title">Always-Allow Rules</h3>
         apiOrigin.value = settings.apiOrigin || "";
         webAppOrigin.value = settings.webAppOrigin || "";
         sandboxBaseDirectory.value = settings.sandboxBaseDirectory || "";
-        defaultApprovalTier.value = settings.defaultApprovalTier || "high";
+        defaultApprovalTier.value = settings.defaultApprovalTier === "auto" ? "high" : (settings.defaultApprovalTier || "high");
         renderTierOverrides(settings.autoApprovalRules || {});
         renderAlwaysAllowRules(settings.alwaysAllowRules || []);
         updateSandboxBaseWarning();
@@ -2247,7 +2296,7 @@ <h3 class="settings-group-title">Always-Allow Rules</h3>
         "health_check", "repos_config", "deploy", "filesystem"
       ];
 
-      const TIER_OPTIONS = ["high", "medium", "low", "auto"];
+      const TIER_OPTIONS = ["high", "medium", "low", "none"];
 
       function renderTierOverrides(rules) {
         // Sync hidden input for save
@@ -2272,7 +2321,8 @@ <h3 class="settings-group-title">Always-Allow Rules</h3>
             const opt = document.createElement("option");
             opt.value = t;
             opt.textContent = t;
-            if (t === tier) opt.selected = true;
+            const normalizedTier = tier === "auto" ? "high" : tier;
+            if (t === normalizedTier) opt.selected = true;
             sel.appendChild(opt);
           }
           sel.addEventListener("change", () => {

apps/desktop/src/shared/contracts.ts

@@ -35,7 +35,7 @@ export interface HealthResponse {
   port: number;
 }
 
-export type RiskTier = "auto" | "low" | "medium" | "high";
+export type RiskTier = "none" | "low" | "medium" | "high";
 
 export interface AlwaysAllowRule {
   id: string;

apps/desktop/test/approval-policy.test.ts

@@ -6,6 +6,7 @@ import { SUPPORTED_OPERATION_IDS, resolveOperationId } from "../src/main/approva
 // --- riskTierOrder ---
 
 test("riskTierOrder returns correct numeric ordering", () => {
+  assert.ok(riskTierOrder("none") < riskTierOrder("low"));
   assert.ok(riskTierOrder("low") < riskTierOrder("medium"));
   assert.ok(riskTierOrder("medium") < riskTierOrder("high"));
 });
@@ -30,6 +31,13 @@ test("policy high: auto-approves all mapped operations", () => {
   assert.equal(shouldAutoApprove("deploy", "high", false), true);
 });
 
+test("policy none: blocks all operations including low-risk", () => {
+  assert.equal(shouldAutoApprove("health_check", "none", false), false);
+  assert.equal(shouldAutoApprove("symphony_loop", "none", false), false);
+  assert.equal(shouldAutoApprove("deploy", "none", false), false);
+  assert.equal(shouldAutoApprove("unknown_op", "none", false), false);
+});
+
 test("forceApproval overrides threshold", () => {
   assert.equal(shouldAutoApprove("health_check", "low", true), false);
 });

apps/desktop/test/settings-migration.test.ts

@@ -108,6 +108,42 @@ test("migration: fresh install applies defaults", () => {
   assert.equal("authApiOrigin" in all, false, "no stale authApiOrigin key should be present");
 });
 
+// --- Approval tier "auto" → "high" migration ---
+
+test("migration: defaultApprovalTier 'auto' is rewritten to 'high'", () => {
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "settings-migration-auto-tier-"));
+  tempDirs.push(tmpDir);
+
+  const storeName = "test-auto-tier";
+  fs.writeFileSync(
+    path.join(tmpDir, `${storeName}.json`),
+    JSON.stringify({
+      defaultApprovalTier: "auto",
+      autoApprovalRules: { deploy: "auto", health_check: "low" }
+    })
+  );
+
+  const store = new SettingsStore({ cwd: tmpDir, name: storeName });
+  const all = store.getAll();
+
+  assert.equal(all.defaultApprovalTier, "high", "defaultApprovalTier should be migrated to 'high'");
+  assert.equal(
+    (all.autoApprovalRules as Record<string, string>).deploy,
+    "high",
+    "autoApprovalRules 'auto' entries should be migrated to 'high'"
+  );
+  assert.equal(
+    (all.autoApprovalRules as Record<string, string>).health_check,
+    "low",
+    "non-auto autoApprovalRules entries should be preserved"
+  );
+
+  // Verify persisted JSON no longer contains "auto"
+  const persisted = JSON.parse(fs.readFileSync(path.join(tmpDir, `${storeName}.json`), "utf-8"));
+  assert.equal(persisted.defaultApprovalTier, "high", "persisted defaultApprovalTier should be 'high'");
+  assert.equal(persisted.autoApprovalRules?.deploy, "high", "persisted autoApprovalRules.deploy should be 'high'");
+});
+
 test("migration: already migrated install is a no-op — both values preserved", () => {
   const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "settings-migration-noop-"));
   tempDirs.push(tmpDir);