Policies: specificity-aware default position on the server

.changeset/policy-position-server-default.md

@@ -0,0 +1,5 @@
+---
+"executor": patch
+---
+
+Policy create now defaults a new rule's position below any more-specific existing rule on the server, so a broad rule written without an explicit position (stale UI, API, agent tool) cannot shadow an existing narrower rule.

packages/core/sdk/src/executor.ts

@@ -5,8 +5,6 @@ import { memoryAdapter } from "@executor-js/fumadb/adapters/memory";
 import { withQueryContext, type Condition, type ConditionBuilder } from "@executor-js/fumadb/query";
 import { schema as fumaSchema, type RelationsMap } from "@executor-js/fumadb/schema";
 import type { AnyColumn } from "@executor-js/fumadb/schema";
-import { generateKeyBetween } from "fractional-indexing";
-
 import {
   StorageError,
   isStorageFailure,
@@ -99,6 +97,7 @@ import {
   comparePolicyRow,
   isValidPattern,
   matchPattern,
+  positionForNewPattern,
   resolveEffectivePolicy,
   rowToToolPolicy,
   type CreateToolPolicyInput,
@@ -3407,11 +3406,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         const existing = yield* core.findMany("tool_policy", {
           where: byOwner(input.owner),
         });
-        const minPosition = existing
-          .map((row) => row.position)
-          .sort()
-          .at(0);
-        const position = input.position ?? generateKeyBetween(null, minPosition ?? null);
+        // Default placement is specificity-aware (below any more-specific
+        // rule), not top-of-list: a client that omits position — the UI when
+        // its policy list is stale, the API, an agent tool — must not have its
+        // broad rule silently shadow an existing narrow one.
+        const position = input.position ?? positionForNewPattern(input.pattern, existing);
         const id = PolicyId.make(
           `pol_${Math.random().toString(36).slice(2)}${Date.now().toString(36)}`,
         );

packages/core/sdk/src/policies.test.ts

@@ -367,7 +367,7 @@ describe("executor.policies", () => {
     }),
   );
 
-  it.effect("create defaults new rules to the top of the list", () =>
+  it.effect("create defaults new rules above equally-or-less-specific ones", () =>
     Effect.gen(function* () {
       const executor = yield* setupExecutor();
       const first = yield* executor.policies.create({
@@ -387,6 +387,47 @@ describe("executor.policies", () => {
     }),
   );
 
+  it.effect("create places a broad rule below an existing narrower one", () =>
+    Effect.gen(function* () {
+      const executor = yield* setupExecutor();
+      // Narrow leaf rule first, then a broad category rule WITHOUT an explicit
+      // position — the category rule must not shadow the leaf rule.
+      yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.*.*.records.create",
+        action: "block",
+      });
+      yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.*.*.records.*",
+        action: "require_approval",
+      });
+
+      const rules = yield* executor.policies.list();
+      expect(rules.map((r) => r.pattern)).toEqual([
+        "vercel.*.*.records.create",
+        "vercel.*.*.records.*",
+      ]);
+
+      const rows = rules.map(
+        (r) =>
+          ({
+            id: String(r.id),
+            owner: r.owner,
+            subject: "",
+            pattern: r.pattern,
+            action: r.action,
+            position: r.position,
+            created_at: r.createdAt,
+            updated_at: r.updatedAt,
+          }) as ToolPolicyRow,
+      );
+      const match = resolveToolPolicy("vercel.org.acct.records.create", rows, () => 0);
+      expect(match?.action).toBe("block");
+      expect(match?.pattern).toBe("vercel.*.*.records.create");
+    }),
+  );
+
   it.effect("create stores rules at the requested owner", () =>
     Effect.gen(function* () {
       const executor = yield* setupExecutor();

packages/core/sdk/src/policies.ts

@@ -10,6 +10,7 @@
 // ---------------------------------------------------------------------------
 
 import { Match, Schema } from "effect";
+import { generateKeyBetween } from "fractional-indexing";
 
 import type { ToolPolicyAction, ToolPolicyRow } from "./core-schema";
 import { Owner, PolicyId } from "./ids";
@@ -139,6 +140,46 @@ export const comparePolicyRow = (
   return ia < ib ? -1 : ia > ib ? 1 : 0;
 };
 
+// Specificity score for ordering. Higher = more specific = should sit at a
+// lower position-key (higher precedence). New rules are auto-placed below
+// any more-specific existing rules so a freshly-added group rule never
+// silently shadows an existing leaf rule.
+//   `*`            → 0
+//   `vercel.*`     → 2  (1 literal segment, wildcard)
+//   `vercel.dns.*` → 4  (2 literal segments, wildcard)
+//   `vercel.dns`   → 5  (2 literal segments, exact — beats same-prefix wildcard)
+//   `vercel.dns.create` → 7  (3 literal segments, exact)
+export const patternSpecificity = (pattern: string): number => {
+  if (pattern === "*") return 0;
+  if (pattern.endsWith(".*")) {
+    const prefix = pattern.slice(0, -2);
+    return prefix.split(".").length * 2;
+  }
+  return pattern.split(".").length * 2 + 1;
+};
+
+/**
+ * Position key for a new rule among an owner's existing rules, placed just
+ * below every existing rule that is MORE specific (and above everything
+ * equally or less specific). Rows must be the owner's committed rules; order
+ * doesn't matter, they're sorted here. This is the authoritative default —
+ * the server applies it when `create` gets no explicit position, so a rule
+ * written by any client (UI, API, agent tool) cannot shadow a more-specific
+ * existing rule by racing to the top of the list.
+ */
+export const positionForNewPattern = (
+  pattern: string,
+  rows: ReadonlyArray<Pick<ToolPolicyRow, "pattern" | "position" | "id">>,
+): string => {
+  const committed = [...rows].sort(comparePolicyRow);
+  const newScore = patternSpecificity(pattern);
+  let idx = committed.findIndex((r) => patternSpecificity(r.pattern) <= newScore);
+  if (idx === -1) idx = committed.length; // below every more-specific rule
+  const prev = idx === 0 ? null : committed[idx - 1]!.position;
+  const next = idx === committed.length ? null : committed[idx]!.position;
+  return generateKeyBetween(prev, next);
+};
+
 const actionRestrictionRank = (action: ToolPolicyAction): number =>
   Match.value(action).pipe(
     Match.when("block", () => 3),

packages/core/sdk/src/shared.ts

@@ -88,6 +88,8 @@ export {
   isValidPattern,
   effectivePolicyFromSorted,
   comparePolicyRow,
+  patternSpecificity,
+  positionForNewPattern,
   ToolPolicyActionSchema,
   type ToolPolicy,
   type CreateToolPolicyInput,

packages/react/src/components/tool-detail.tsx

@@ -171,7 +171,7 @@ export function ToolDetail(props: {
   /** When provided, the policy badge becomes a dropdown trigger that
    *  applies a user rule to this tool's exact id. */
   onSetPolicy?: (pattern: string, action: ToolPolicyAction) => void;
-  onClearPolicy?: (pattern: string) => void;
+  onClearPolicy?: (pattern: string, policyId?: string) => void;
   patternForDisplay?: (displayPattern: string) => string;
   /** Run-tab wiring. When `integration` + `runToolName` are provided, a third
    *  "Run" tab hosts the per-connection tool tester. */
@@ -413,7 +413,7 @@ function PolicyBadgeMenu(props: {
   staticTool?: boolean;
   policy?: EffectivePolicy;
   onSetPolicy?: (pattern: string, action: ToolPolicyAction) => void;
-  onClearPolicy?: (pattern: string) => void;
+  onClearPolicy?: (pattern: string, policyId?: string) => void;
   patternForDisplay?: (displayPattern: string) => string;
 }) {
   const interactive = !!props.onSetPolicy;
@@ -487,7 +487,10 @@ function PolicyBadgeMenu(props: {
           <>
             <DropdownMenuSeparator />
             <DropdownMenuItem
-              onSelect={() => props.onClearPolicy?.(pattern)}
+              // Pass the resolved rule's id: right after this menu authored
+              // the rule, the policies list may still be mid-refresh, and a
+              // pattern-only lookup there would silently no-op the clear.
+              onSelect={() => props.onClearPolicy?.(pattern, props.policy?.policyId)}
               className="text-muted-foreground"
             >
               Clear

packages/react/src/hooks/use-policy-actions.ts

@@ -1,8 +1,12 @@
-import { useCallback, useMemo } from "react";
+import { useCallback, useMemo, useRef } from "react";
 import { useAtomSet, useAtomValue } from "@effect/atom-react";
 import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
-import { generateKeyBetween } from "fractional-indexing";
-import { PolicyId, type Owner, type ToolPolicyAction } from "@executor-js/sdk/shared";
+import {
+  PolicyId,
+  positionForNewPattern,
+  type Owner,
+  type ToolPolicyAction,
+} from "@executor-js/sdk/shared";
 
 import {
   createPolicyOptimistic,
@@ -13,31 +17,17 @@ import {
 import { policyWriteKeys } from "../api/reactivity-keys";
 import { trackEvent } from "../api/analytics";
 
-// Specificity score for ordering. Higher = more specific = should sit at a
-// lower position-key (higher precedence). New rules are auto-placed below
-// any more-specific existing rules so a freshly-added group rule never
-// silently shadows an existing leaf rule.
-//   `*`            → 0
-//   `vercel.*`     → 2  (1 literal segment, wildcard)
-//   `vercel.dns.*` → 4  (2 literal segments, wildcard)
-//   `vercel.dns`   → 5  (2 literal segments, exact — beats same-prefix wildcard)
-//   `vercel.dns.create` → 7  (3 literal segments, exact)
-const specificity = (pattern: string): number => {
-  if (pattern === "*") return 0;
-  if (pattern.endsWith(".*")) {
-    const prefix = pattern.slice(0, -2);
-    return prefix.split(".").length * 2;
-  }
-  return pattern.split(".").length * 2 + 1;
-};
-
 export interface PolicyAction {
   /** Set the action on a pattern. If a user rule with this exact pattern
    *  already exists, update it. Otherwise create with auto-placed
    *  position so more-specific rules keep precedence. */
   readonly set: (pattern: string, action: ToolPolicyAction) => Promise<void>;
-  /** Remove the user rule with this exact pattern, if any. No-op if none. */
-  readonly clear: (pattern: string) => Promise<void>;
+  /** Remove the user rule with this exact pattern, if any. `policyId` is the
+   *  id of the rule the caller believes is active (e.g. from the tool's
+   *  resolved EffectivePolicy); it is the fallback when the local policies
+   *  list is mid-refresh and doesn't contain the rule yet — without it a
+   *  clear in that window silently no-ops. */
+  readonly clear: (pattern: string, policyId?: string) => Promise<void>;
   /** True while a write is in flight. */
   readonly busy: boolean;
 }
@@ -83,20 +73,23 @@ export const usePolicyActions = (owner: Owner = "org"): PolicyAction => {
 
   const busy = policies.waiting;
 
+  // Server-assigned ids of rules this hook created, by pattern. The local
+  // policies list is optimistic: right after a create it holds a placeholder
+  // row with a fake `pending-*` id (and the resolved EffectivePolicy a caller
+  // hands back can carry that same fake id), so neither is safe to DELETE
+  // with. The create response is the one authoritative id source in that
+  // window.
+  const createdIdByPattern = useRef(new Map<string, string>());
+
+  // Specificity-aware placement (below any more-specific rule) via the shared
+  // sdk helper — the same computation the server applies when no position is
+  // sent. Computing it here too keeps the optimistic UI's final order stable;
+  // if this client's list is stale, the server default is the backstop.
   const computePosition = useCallback(
     (newPattern: string): string | undefined => {
       const committed = sorted.filter((r) => r.position !== "");
       if (committed.length === 0) return undefined;
-      const newScore = specificity(newPattern);
-      // Walk down the list (most-precedent first); place the new rule
-      // just before the first existing rule whose specificity is <= the
-      // new one. That way more-specific rules stay above us, and we win
-      // against everything equally or less specific.
-      let idx = committed.findIndex((r) => specificity(r.pattern) <= newScore);
-      if (idx === -1) idx = committed.length; // append at bottom
-      const prev = idx === 0 ? null : committed[idx - 1]!.position;
-      const next = idx === committed.length ? null : committed[idx]!.position;
-      return generateKeyBetween(prev, next);
+      return positionForNewPattern(newPattern, committed);
     },
     [sorted],
   );
@@ -121,24 +114,33 @@ export const usePolicyActions = (owner: Owner = "org"): PolicyAction => {
         return;
       }
       const position = computePosition(pattern);
-      await doCreate({
+      const created = await doCreate({
         payload:
           position === undefined
             ? { owner, pattern, action }
             : { owner, pattern, action, position },
         reactivityKeys: policyWriteKeys,
       });
+      createdIdByPattern.current.set(pattern, String(created.id));
       trackEvent("tool_policy_set", { action, pattern_kind: patternKind, owner });
     },
     [owner, doCreate, doUpdate, findExact, computePosition],
   );
 
   const clear = useCallback(
-    async (pattern: string) => {
+    async (pattern: string, policyId?: string) => {
+      // Resolve the id to delete, most-authoritative first. The local list
+      // and the caller's resolved policy can both hold an optimistic
+      // `pending-*` placeholder id right after a create (before the
+      // post-commit refetch lands); the create response we recorded is real,
+      // and a placeholder id must never reach the server.
       const existing = findExact(pattern);
-      if (!existing) return;
+      const isReal = (id: string | undefined): id is string => !!id && !id.startsWith("pending-");
+      const id = [existing?.id, createdIdByPattern.current.get(pattern), policyId].find(isReal);
+      if (!id) return;
+      createdIdByPattern.current.delete(pattern);
       await doRemove({
-        params: { policyId: PolicyId.make(existing.id) },
+        params: { policyId: PolicyId.make(id) },
         payload: { owner },
         reactivityKeys: policyWriteKeys,
       });

packages/react/src/pages/integration-detail.tsx

@@ -526,7 +526,9 @@ export function IntegrationDetailPage(props: {
                         staticTool={selection?.static}
                         policy={selectedTool.policy}
                         onSetPolicy={(pattern, action) => void policyActions.set(pattern, action)}
-                        onClearPolicy={(pattern) => void policyActions.clear(pattern)}
+                        onClearPolicy={(pattern, policyId) =>
+                          void policyActions.clear(pattern, policyId)
+                        }
                         {...(!selection?.static && selectedBareName
                           ? {
                               integration: slug,

packages/react/src/pages/tools.tsx

@@ -157,7 +157,9 @@ export function ToolsPage() {
                       staticTool={selection?.static}
                       policy={selectedTool.policy}
                       onSetPolicy={(pattern, action) => void policyActions.set(pattern, action)}
-                      onClearPolicy={(pattern) => void policyActions.clear(pattern)}
+                      onClearPolicy={(pattern, policyId) =>
+                        void policyActions.clear(pattern, policyId)
+                      }
                     />
                   ) : (
                     <ToolDetailEmpty hasTools={summaries.length > 0} />