fix(cli): resolve transitive registryDependencies and surface optional resource env vars

packages/shared/src/cli/commands/registry/add.test.ts

@@ -0,0 +1,96 @@
+import { describe, expect, it, vi } from "vitest";
+import { declaredEnvVars, resolveItems } from "./add";
+import type { RegistryItem } from "./client";
+
+function item(name: string, extra: Partial<RegistryItem> = {}): RegistryItem {
+  return { name, ...extra };
+}
+
+describe("declaredEnvVars", () => {
+  it("collects env vars from required resources", () => {
+    const manifest = {
+      resources: {
+        required: [{ fields: { id: { env: "DATABRICKS_WAREHOUSE_ID" } } }],
+      },
+    };
+    expect(declaredEnvVars(manifest)).toEqual(["DATABRICKS_WAREHOUSE_ID"]);
+  });
+
+  // Bug #2: optional resources were dropped entirely.
+  it("also collects env vars from optional resources", () => {
+    const manifest = {
+      resources: {
+        required: [{ fields: { id: { env: "REQUIRED_ENV" } } }],
+        optional: [{ fields: { id: { env: "OPTIONAL_ENV" } } }],
+      },
+    };
+    expect(declaredEnvVars(manifest)).toEqual(["REQUIRED_ENV", "OPTIONAL_ENV"]);
+  });
+
+  it("skips fields without an env property", () => {
+    const manifest = {
+      resources: {
+        required: [{ fields: { host: { env: "PGHOST" }, note: {} } }],
+      },
+    };
+    expect(declaredEnvVars(manifest)).toEqual(["PGHOST"]);
+  });
+
+  it("returns empty for a manifest with no resources", () => {
+    expect(declaredEnvVars({})).toEqual([]);
+  });
+});
+
+describe("resolveItems", () => {
+  it("returns requested items in order", async () => {
+    const fetch = vi.fn(async (name: string) => item(name));
+    const result = await resolveItems(["a", "b"], null, fetch);
+    expect(result.map((i) => i.name)).toEqual(["a", "b"]);
+  });
+
+  // Bugs #1 + #3: registryDependencies were ignored on plugins and never
+  // resolved transitively.
+  it("resolves transitive registryDependencies", async () => {
+    const graph: Record<string, RegistryItem> = {
+      a: item("a", { registryDependencies: ["b"] }),
+      b: item("b", { registryDependencies: ["c"] }),
+      c: item("c"),
+    };
+    const fetch = vi.fn(async (name: string) => graph[name]);
+    const result = await resolveItems(["a"], null, fetch);
+    expect(result.map((i) => i.name)).toEqual(["a", "b", "c"]);
+  });
+
+  it("de-duplicates shared dependencies and fetches each once", async () => {
+    const graph: Record<string, RegistryItem> = {
+      a: item("a", { registryDependencies: ["shared"] }),
+      b: item("b", { registryDependencies: ["shared"] }),
+      shared: item("shared"),
+    };
+    const fetch = vi.fn(async (name: string) => graph[name]);
+    const result = await resolveItems(["a", "b"], null, fetch);
+    expect(result.map((i) => i.name)).toEqual(["a", "b", "shared"]);
+    expect(fetch).toHaveBeenCalledTimes(3);
+  });
+
+  it("does not loop on circular dependencies", async () => {
+    const graph: Record<string, RegistryItem> = {
+      a: item("a", { registryDependencies: ["b"] }),
+      b: item("b", { registryDependencies: ["a"] }),
+    };
+    const fetch = vi.fn(async (name: string) => graph[name]);
+    const result = await resolveItems(["a"], null, fetch);
+    expect(result.map((i) => i.name)).toEqual(["a", "b"]);
+    expect(fetch).toHaveBeenCalledTimes(2);
+  });
+
+  it("strips the namespace from dependency refs", async () => {
+    const graph: Record<string, RegistryItem> = {
+      a: item("a", { registryDependencies: ["@databricks-appkit/b"] }),
+      b: item("b"),
+    };
+    const fetch = vi.fn(async (name: string) => graph[name]);
+    const result = await resolveItems(["a"], null, fetch);
+    expect(result.map((i) => i.name)).toEqual(["a", "b"]);
+  });
+});

packages/shared/src/cli/commands/registry/add.ts

@@ -10,7 +10,7 @@ import {
   type RegistryItemFile,
   stripNamespace,
 } from "./client";
-import { REGISTRY_REPO, resolveToken } from "./constants";
+import { REGISTRY_REPO, type RegistryToken, resolveToken } from "./constants";
 import { registerPluginInServer } from "./server-register";
 
 /** Subdirectories that commonly hold the frontend / server in an AppKit app. */
@@ -25,7 +25,7 @@ interface ManifestResource {
 }
 interface PluginManifestShape {
   name?: string;
-  resources?: { required?: ManifestResource[] };
+  resources?: { required?: ManifestResource[]; optional?: ManifestResource[] };
 }
 
 function isDir(p: string): boolean {
@@ -86,9 +86,14 @@ function resolveUiTarget(base: string, file: RegistryItemFile): string {
   return path.join(base, target);
 }
 
-function requiredEnvVars(manifest: PluginManifestShape): string[] {
+/** Env var names declared by a manifest's resources (required and optional). */
+export function declaredEnvVars(manifest: PluginManifestShape): string[] {
   const envs: string[] = [];
-  for (const res of manifest.resources?.required ?? []) {
+  const resources = [
+    ...(manifest.resources?.required ?? []),
+    ...(manifest.resources?.optional ?? []),
+  ];
+  for (const res of resources) {
     for (const field of Object.values(res.fields ?? {})) {
       if (field.env) envs.push(field.env);
     }
@@ -184,6 +189,39 @@ interface PluginSummary {
   envs: string[];
 }
 
+/**
+ * Fetches the requested items plus their transitive registryDependencies.
+ * Dependencies are resolved breadth-first and de-duplicated by name, so a
+ * plugin that depends on another registry item pulls the whole graph in one
+ * `add`. Explicitly-requested items keep their request order and come first.
+ */
+export async function resolveItems(
+  names: string[],
+  token: RegistryToken | null,
+  fetchItem: (
+    name: string,
+    token: RegistryToken | null,
+  ) => Promise<RegistryItem> = fetchRegistryItem,
+): Promise<RegistryItem[]> {
+  const seen = new Set<string>();
+  const ordered: RegistryItem[] = [];
+  const queue = [...names];
+
+  while (queue.length > 0) {
+    const name = stripNamespace(queue.shift() as string);
+    if (seen.has(name)) continue;
+    seen.add(name);
+    const item = await fetchItem(name, token);
+    ordered.push(item);
+    for (const dep of item.registryDependencies ?? []) {
+      const depName = stripNamespace(dep);
+      if (!seen.has(depName)) queue.push(depName);
+    }
+  }
+
+  return ordered;
+}
+
 async function runAdd(
   refs: string[],
   opts: { force?: boolean; cwd?: string; register?: boolean },
@@ -196,11 +234,7 @@ async function runAdd(
     );
   }
 
-  const names = refs.map(stripNamespace);
-  const items: RegistryItem[] = [];
-  for (const name of names) {
-    items.push(await fetchRegistryItem(name, token));
-  }
+  const items = await resolveItems(refs, token);
 
   const hasUi = items.some((i) => !isPluginItem(i));
   const hasPlugin = items.some(isPluginItem);
@@ -241,16 +275,10 @@ async function runAdd(
       pluginSummaries.push({
         importPath: `./${pluginRel}`,
         exportName: pluginExportName(item),
-        envs: requiredEnvVars(manifest),
+        envs: declaredEnvVars(manifest),
       });
     } else {
       for (const file of item.files ?? []) {
-        // UI (Option A) items have no registry deps; warn on any a future item adds.
-        for (const rd of item.registryDependencies ?? []) {
-          console.warn(
-            `  Note: "${item.name}" declares registryDependency "${rd}" — add it separately if needed.`,
-          );
-        }
         writeItemFile(
           resolveUiTarget(frontendRoot, file),
           file.content,