src/lib/server/agent-config/agent-config.test.ts

@@ -3,6 +3,7 @@ import { mkdtemp } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { afterEach, describe, expect, test } from "vitest";
+import type { DashboardDocument } from "$lib/model";
 import { genericDashboardFixture } from "$lib/model/fixtures/generic";
 import { createDashboardStore, type DashboardStore } from "$lib/server/db/dashboard-store";
 import {
@@ -13,6 +14,7 @@ import {
   publishDashboardChanges,
   rollbackDashboardRevision,
   type AgentDashboardOperation,
+  type JsonPatchOperation,
 } from ".";
 
 const stores: DashboardStore[] = [];
@@ -125,6 +127,82 @@ describe("agent dashboard configuration API", () => {
     expect(result.patch.map((operation) => operation.op)).toContain("remove");
   });
 
+  test("returns RFC 6902-applicable patches for multiple array removals", () => {
+    const result = previewDashboardChanges(genericDashboardFixture, [
+      {
+        type: "remove_item",
+        target: { kind: "statusItem", stripId: "runtime", id: "status" },
+      },
+      {
+        type: "remove_item",
+        target: { kind: "statusItem", stripId: "runtime", id: "sync" },
+      },
+    ]);
+
+    expect(result.ok).toBe(true);
+    if (!result.ok) throw new Error("expected remove success");
+    expect(
+      result.patch
+        .filter((operation) => operation.op === "remove")
+        .map((operation) => operation.path),
+    ).toEqual(["/statusStrips/0/items/1", "/statusStrips/0/items/0"]);
+    expect(applyJsonPatch(genericDashboardFixture, result.patch)).toMatchObject({
+      statusStrips: [{ items: [] }],
+    });
+  });
+
+  test("rejects ambiguous service and status targets", () => {
+    const duplicateDocument = documentWithDuplicateNestedIds();
+
+    const connect = previewDashboardChanges(duplicateDocument, [
+      {
+        type: "connect_datasource",
+        target: { kind: "service", id: "identity" },
+        datasource: {
+          type: "external",
+          adapter: "http-status",
+          reference: "GET https://identity.example.test/health",
+        },
+      },
+    ]);
+    expect(connect.ok).toBe(false);
+    if (connect.ok) throw new Error("expected ambiguous service failure");
+    expect(connect.errors[0]).toMatchObject({
+      code: "ambiguous_target",
+      operationIndex: 0,
+      path: "/serviceGroups",
+    });
+
+    const remove = previewDashboardChanges(duplicateDocument, [
+      {
+        type: "remove_item",
+        target: { kind: "service", id: "identity" },
+      },
+    ]);
+    expect(remove.ok).toBe(false);
+    if (remove.ok) throw new Error("expected ambiguous removal failure");
+    expect(remove.errors[0]).toMatchObject({
+      code: "ambiguous_target",
+      operationIndex: 0,
+      path: "/serviceGroups",
+    });
+
+    const status = previewDashboardChanges(duplicateDocument, [
+      {
+        type: "set_status_rule",
+        target: { kind: "statusItem", id: "status" },
+        value: "Healthy",
+      },
+    ]);
+    expect(status.ok).toBe(false);
+    if (status.ok) throw new Error("expected ambiguous status failure");
+    expect(status.errors[0]).toMatchObject({
+      code: "ambiguous_target",
+      operationIndex: 0,
+      path: "/statusStrips",
+    });
+  });
+
   test("rejects create_dashboard when it is not the first operation", () => {
     const result = previewDashboardChanges(genericDashboardFixture, [
       {
@@ -434,6 +512,77 @@ function exampleService() {
   };
 }
 
+function documentWithDuplicateNestedIds(): DashboardDocument {
+  const document = structuredClone(genericDashboardFixture);
+  document.layout.serviceGroups.push("secondary-services");
+  document.serviceGroups.push({
+    id: "secondary-services",
+    title: "Secondary Services",
+    layout: "list",
+    services: [
+      {
+        ...document.serviceGroups[0].services[0],
+        label: "Shadow Identity",
+      },
+    ],
+  });
+  document.layout.statusStrips.push("secondary-runtime");
+  document.statusStrips.push({
+    id: "secondary-runtime",
+    items: [
+      {
+        ...document.statusStrips[0].items[0],
+        value: "Healthy",
+      },
+    ],
+  });
+  return document;
+}
+
+function applyJsonPatch<T>(value: T, patch: JsonPatchOperation[]): T {
+  const next = structuredClone(value);
+  for (const operation of patch) {
+    const { parent, key } = jsonPointerTarget(next, operation.path);
+    if (operation.op === "remove") {
+      if (Array.isArray(parent)) {
+        parent.splice(Number(key), 1);
+      } else {
+        delete parent[key];
+      }
+    } else if (operation.op === "add") {
+      if (Array.isArray(parent)) {
+        parent.splice(Number(key), 0, operation.value);
+      } else {
+        parent[key] = operation.value;
+      }
+    } else if (operation.op === "replace") {
+      if (Array.isArray(parent)) {
+        parent[Number(key)] = operation.value;
+      } else {
+        parent[key] = operation.value;
+      }
+    }
+  }
+  return next;
+}
+
+function jsonPointerTarget(value: unknown, path: string) {
+  const segments = path
+    .split("/")
+    .slice(1)
+    .map((segment) => segment.replace(/~1/g, "/").replace(/~0/g, "~"));
+  const key = segments.pop();
+  if (key === undefined) throw new Error(`Invalid JSON pointer: ${path}`);
+
+  let parent = value as Record<string, unknown> | unknown[];
+  for (const segment of segments) {
+    parent = Array.isArray(parent)
+      ? (parent[Number(segment)] as Record<string, unknown> | unknown[])
+      : (parent[segment] as Record<string, unknown> | unknown[]);
+  }
+  return { parent, key };
+}
+
 function jsonRequest(body: unknown) {
   return new Request("https://dimensionlab.test/api/agent/dashboard", {
     body: JSON.stringify(body),

src/lib/server/agent-config/index.ts

@@ -746,16 +746,12 @@ function connectDatasource(
     );
   }
 
-  const target = resolveTarget(document, operation.target);
-  if (!target) {
-    return failure(
-      "item_not_found",
-      `Target not found: ${operation.target.id}`,
-      operationIndex,
-      targetPath(operation.target),
-    );
+  const targetResolution = resolveTarget(document, operation.target, operationIndex);
+  if (!targetResolution.ok) {
+    return { ok: false, errors: targetResolution.errors };
   }
 
+  const target = targetResolution.target;
   (target as DatasourceAgentTarget).datasource = operation.datasource;
   return { ok: true, document };
 }
@@ -798,16 +794,12 @@ function setStatusRule(
     );
   }
 
-  const target = resolveTarget(document, operation.target);
-  if (!target) {
-    return failure(
-      "item_not_found",
-      `Target not found: ${operation.target.id}`,
-      operationIndex,
-      targetPath(operation.target),
-    );
+  const targetResolution = resolveTarget(document, operation.target, operationIndex);
+  if (!targetResolution.ok) {
+    return { ok: false, errors: targetResolution.errors };
   }
 
+  const target = targetResolution.target;
   if (operation.thresholds && operation.target.kind === "telemetry") {
     (target as TelemetryCard).thresholds = operation.thresholds;
   }
@@ -881,8 +873,8 @@ function removeItem(
   }
 
   if (operation.target.kind === "service") {
-    const location = findService(document, operation.target.id, operation.target.groupId);
-    if (!location) {
+    const matches = findServices(document, operation.target.id, operation.target.groupId);
+    if (!matches.length) {
       return failure(
         "item_not_found",
         `Service not found: ${operation.target.id}`,
@@ -890,7 +882,17 @@ function removeItem(
         "/serviceGroups",
       );
     }
-    location.group.services = location.group.services.filter((service) => service.id !== operation.target.id);
+    if (!operation.target.groupId && matches.length > 1) {
+      return failure(
+        "ambiguous_target",
+        `Service exists in multiple groups: ${operation.target.id}`,
+        operationIndex,
+        "/serviceGroups",
+      );
+    }
+
+    const match = matches[0];
+    match.group.services = match.group.services.filter((service) => service.id !== operation.target.id);
     return { ok: true, document };
   }
 
@@ -938,25 +940,83 @@ function removeItem(
 function resolveTarget(
   document: DashboardDocument,
   target: ConnectDatasourceOperation["target"],
-): ResolvedAgentTarget | null {
+  operationIndex: number,
+): ResolveTargetResult {
   if (target.kind === "telemetry") {
-    return document.telemetry.find((card) => card.id === target.id) || null;
+    const card = document.telemetry.find((item) => item.id === target.id);
+    if (!card) {
+      return resolutionFailure(
+        "item_not_found",
+        `Telemetry card not found: ${target.id}`,
+        operationIndex,
+        "/telemetry",
+      );
+    }
+    return { ok: true, target: card };
   }
   if (target.kind === "service") {
-    return findService(document, target.id, target.groupId)?.service || null;
+    const matches = findServices(document, target.id, target.groupId);
+    if (!matches.length) {
+      return resolutionFailure(
+        "item_not_found",
+        `Service not found: ${target.id}`,
+        operationIndex,
+        "/serviceGroups",
+      );
+    }
+    if (!target.groupId && matches.length > 1) {
+      return resolutionFailure(
+        "ambiguous_target",
+        `Service exists in multiple groups: ${target.id}`,
+        operationIndex,
+        "/serviceGroups",
+      );
+    }
+    return { ok: true, target: matches[0].service };
   }
   if (target.kind === "statusItem") {
-    const strips = target.stripId
-      ? document.statusStrips.filter((strip) => strip.id === target.stripId)
-      : document.statusStrips;
-    return strips.flatMap((strip) => strip.items).find((item) => item.id === target.id) || null;
+    const matches = findStatusItems(document, target.id, target.stripId);
+    if (!matches.length) {
+      return resolutionFailure(
+        "item_not_found",
+        `Status item not found: ${target.id}`,
+        operationIndex,
+        "/statusStrips",
+      );
+    }
+    if (!target.stripId && matches.length > 1) {
+      return resolutionFailure(
+        "ambiguous_target",
+        `Status item exists in multiple strips: ${target.id}`,
+        operationIndex,
+        "/statusStrips",
+      );
+    }
+    return { ok: true, target: matches[0].item };
   }
   if (target.kind === "module") {
-    return (document.modules || []).find((module) => module.id === target.id) || null;
+    const module = (document.modules || []).find((item) => item.id === target.id);
+    if (!module) {
+      return resolutionFailure(
+        "item_not_found",
+        `Module not found: ${target.id}`,
+        operationIndex,
+        "/modules",
+      );
+    }
+    return { ok: true, target: module };
   }
-  return null;
+  return resolutionFailure(
+    "item_not_found",
+    `Target not found: ${target.id}`,
+    operationIndex,
+    targetPath(target),
+  );
 }
 
+type ResolveTargetResult =
+  | { ok: true; target: ResolvedAgentTarget }
+  | { ok: false; errors: AgentConfigError[] };
 type ResolvedAgentTarget = TelemetryCard | ServiceEntry | StatusItem | DashboardModule;
 type DatasourceAgentTarget = TelemetryCard | ServiceEntry | DashboardModule;
 type DetailAgentTarget = TelemetryCard | ServiceEntry | DashboardModule;
@@ -995,15 +1055,15 @@ function supportsStringValue(
   return kind === "statusItem" || kind === "module";
 }
 
-function findService(document: DashboardDocument, id: string, groupId?: string) {
+function findServices(document: DashboardDocument, id: string, groupId?: string) {
   const groups = groupId
     ? document.serviceGroups.filter((group) => group.id === groupId)
     : document.serviceGroups;
-  for (const group of groups) {
-    const service = group.services.find((item) => item.id === id);
-    if (service) return { group, service };
-  }
-  return null;
+  return groups.flatMap((group) =>
+    group.services.flatMap((service) =>
+      service.id === id ? [{ group, service }] : [],
+    ),
+  );
 }
 
 function findStatusItems(document: DashboardDocument, id: string, stripId?: string) {
@@ -1011,10 +1071,29 @@ function findStatusItems(document: DashboardDocument, id: string, stripId?: stri
     ? document.statusStrips.filter((strip) => strip.id === stripId)
     : document.statusStrips;
   return strips.flatMap((strip) =>
-    strip.items.some((item) => item.id === id) ? [{ strip }] : [],
+    strip.items.flatMap((item) => (item.id === id ? [{ strip, item }] : [])),
   );
 }
 
+function resolutionFailure(
+  code: string,
+  message: string,
+  operationIndex: number,
+  path: string,
+): ResolveTargetResult {
+  return {
+    ok: false,
+    errors: [
+      {
+        code,
+        message,
+        operationIndex,
+        path,
+      },
+    ],
+  };
+}
+
 function targetPath(target: ConnectDatasourceOperation["target"]): string {
   if (target.kind === "telemetry") return "/telemetry";
   if (target.kind === "service") return "/serviceGroups";
@@ -1140,12 +1219,18 @@ function createJsonPatch(
       return [{ op: "replace", path: path || "/", value: after }];
     }
     const operations: JsonPatchOperation[] = [];
-    const max = Math.max(before.length, after.length);
-    for (let index = 0; index < max; index += 1) {
+    const sharedLength = Math.min(before.length, after.length);
+    for (let index = 0; index < sharedLength; index += 1) {
       operations.push(
         ...createJsonPatch(before[index], after[index], `${path}/${index}`),
       );
     }
+    for (let index = before.length - 1; index >= after.length; index -= 1) {
+      operations.push({ op: "remove", path: `${path}/${index}` });
+    }
+    for (let index = before.length; index < after.length; index += 1) {
+      operations.push({ op: "add", path: `${path}/${index}`, value: after[index] });
+    }
     return operations;
   }