From 60652cc6b62afee2adb1ff7f00bbe002b4db1b4b Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 14:19:15 +0200 Subject: [PATCH 1/8] feat(appkit): appkit plugin add with --auth-mode and --set resource config Adds a plugin from appkit.plugins.json to an existing app: registers it in the server, appends .env values, and patches app.yaml and databricks.yml. Flags and semantics follow databricks apps init: --auth-mode obo|sp default, --set plugin.resourceKey.authMode=obo|sp|both per resource, --set for values (keys validated against the manifest's declared resources), prompts when interactive. OBO adds user_api_scopes and a literal app.yaml value; SP binds the resource with its permission. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../shared/src/cli/commands/plugin/add/add.ts | 332 ++++++++++++++ .../src/cli/commands/plugin/add/plan.test.ts | 194 ++++++++ .../src/cli/commands/plugin/add/plan.ts | 417 ++++++++++++++++++ .../shared/src/cli/commands/plugin/index.ts | 4 + .../src/cli/commands/registry/config-plan.ts | 5 +- .../commands/registry/config-writer.test.ts | 29 ++ .../cli/commands/registry/config-writer.ts | 23 +- .../src/cli/commands/registry/env-writer.ts | 4 +- .../cli/commands/registry/server-register.ts | 7 +- packages/shared/src/cli/deploy-config.ts | 13 +- 10 files changed, 1016 insertions(+), 12 deletions(-) create mode 100644 packages/shared/src/cli/commands/plugin/add/add.ts create mode 100644 packages/shared/src/cli/commands/plugin/add/plan.test.ts create mode 100644 packages/shared/src/cli/commands/plugin/add/plan.ts diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts new file mode 100644 index 000000000..bab01fbc2 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -0,0 +1,332 @@ +import fs from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { Command } from "commander"; +import pc from "picocolors"; + +import { profileFromEnv } from "../../registry/add"; +import { + planHasContent, + reportConfigWrite, + validateBundle, + writeConfig, +} from "../../registry/config-writer"; +import { serializeEnvAppend } from "../../registry/env-reconcile"; +import { + AUTH_MODE_BOTH, + AUTH_MODE_FIELD, + AUTH_MODE_OBO, + AUTH_MODE_SP, + type AuthMode, + authKey, + buildAddPlan, + findMissingValues, + type ManifestPlugin, + type ManifestResource, + oboCapable, + parseSetAuthModes, + parseSetValues, + resolveAuthModes, + resourceConfigured, + selectResources, +} from "./plan"; + +const MANIFEST_FILE = "appkit.plugins.json"; +const SERVER_SUBDIRS = ["server", "api", "backend"]; + +interface AddOptions { + set: string[]; + authMode?: string; + cwd?: string; + yes?: boolean; + register?: boolean; + profile?: string; +} + +function loadPlugin(cwd: string, name: string): ManifestPlugin { + const file = path.join(cwd, MANIFEST_FILE); + if (!fs.existsSync(file)) { + throw new Error( + `${MANIFEST_FILE} not found in ${cwd}; run \`appkit plugin sync --write\` first`, + ); + } + const manifest = JSON.parse(fs.readFileSync(file, "utf-8")) as { + plugins?: Record; + }; + const plugins = manifest.plugins ?? {}; + const plugin = plugins[name]; + if (!plugin) { + throw new Error( + `unknown plugin "${name}"; available: ${Object.keys(plugins).sort().join(", ")}`, + ); + } + return { + ...plugin, + name, + resources: { + required: plugin.resources?.required ?? [], + optional: plugin.resources?.optional ?? [], + }, + }; +} + +function authModeLabel(mode: AuthMode): string { + if (mode === AUTH_MODE_OBO) return "on behalf of user"; + if (mode === AUTH_MODE_BOTH) return "service principal and on behalf of user"; + return "service principal"; +} + +/** Bails out of the command on Ctrl+C in a prompt. */ +async function answered(value: T | symbol): Promise { + const { cancel, isCancel } = await import("@clack/prompts"); + if (isCancel(value)) { + cancel("Cancelled."); + process.exit(1); + } + return value as T; +} + +/** Asks which optional resources to configure (they're only added with values). */ +async function promptOptional( + plugin: ManifestPlugin, + values: Record, +): Promise { + const { confirm } = await import("@clack/prompts"); + const picked: ManifestResource[] = []; + for (const r of plugin.resources.optional) { + if (resourceConfigured(r, values)) continue; + const yes = await answered( + await confirm({ + message: `Configure ${r.alias}? ${pc.dim(`${r.description ?? ""} (optional)`)}`, + initialValue: false, + }), + ); + if (yes) picked.push(r); + } + return picked; +} + +/** + * As `apps init`: one question for the app, and for "Mixed" one per + * OBO-capable resource. Resources without --set authMode only. + */ +async function promptAuthModes( + resources: ManifestResource[], +): Promise> { + const capable = resources.filter(oboCapable); + if (capable.length === 0) return {}; + const { log, select } = await import("@clack/prompts"); + const appMode = await answered( + await select({ + message: "How should the app access resources?", + options: [ + { value: AUTH_MODE_SP, label: "Service principal" }, + { value: AUTH_MODE_OBO, label: "On behalf of user" }, + { value: "mixed", label: "Mixed (choose per resource)" }, + ], + }), + ); + const modes: Record = {}; + for (const r of capable) { + modes[authKey(r)] = + appMode === "mixed" + ? await answered( + await select({ + message: `How should the app access ${r.alias}?`, + options: [ + { value: AUTH_MODE_OBO, label: "On behalf of user" }, + { value: AUTH_MODE_SP, label: "Service principal" }, + { value: AUTH_MODE_BOTH, label: "Both" }, + ], + }), + ) + : (appMode as AuthMode); + log.info(`${r.alias}: ${authModeLabel(modes[authKey(r)])}`); + } + for (const r of resources) { + if (!oboCapable(r)) { + log.info(`${r.alias}: ${authModeLabel(AUTH_MODE_SP)} (only option)`); + } + } + return modes; +} + +async function runPluginAdd(name: string, opts: AddOptions): Promise { + const cwd = opts.cwd ? path.resolve(opts.cwd) : process.cwd(); + if ( + opts.authMode !== undefined && + opts.authMode !== AUTH_MODE_SP && + opts.authMode !== AUTH_MODE_OBO + ) { + throw new Error( + `invalid --auth-mode value: "${opts.authMode}" (must be obo or sp; use --set ..${AUTH_MODE_FIELD}=both for a single resource)`, + ); + } + const plugin = loadPlugin(cwd, name); + const values = parseSetValues(opts.set, plugin); + const setModes = parseSetAuthModes(opts.set, plugin); + const interactive = !opts.yes && Boolean(process.stdin.isTTY); + + let resources = selectResources(plugin, values); + if (interactive) resources.push(...(await promptOptional(plugin, values))); + + const promptedModes = + interactive && opts.authMode === undefined + ? await promptAuthModes( + resources.filter((r) => !(authKey(r) in setModes)), + ) + : {}; + const { modes, keptSP } = resolveAuthModes( + resources, + setModes, + promptedModes, + opts.authMode, + ); + if (keptSP.length > 0) { + console.log( + `Note: these resources cannot be accessed on behalf of the user and use the service principal: ${keptSP.join(", ")}`, + ); + } + + const missing = findMissingValues(plugin.name, resources, values, modes); + if (missing.length > 0 && !interactive) { + const m = missing[0]; + throw new Error( + `missing value for ${m.setKey}; use --set ${m.setKey}=value`, + ); + } + if (missing.length > 0) { + const { text } = await import("@clack/prompts"); + for (const m of missing) { + const field = m.resource.fields?.[m.field]; + const value = await answered( + await text({ + message: `${m.resource.alias}: ${m.field}`, + placeholder: field?.description, + validate: (v) => + !v?.trim() || /[\r\n]/.test(v) ? "Enter a value" : undefined, + }), + ); + values[`${m.resource.resourceKey}.${m.field}`] = value.trim(); + } + } + resources = selectResources(plugin, values); + + const plan = buildAddPlan(plugin, resources, values, modes); + + if (opts.register !== false) await registerInServer(cwd, plugin); + + const { appendToFile, readEnvFile } = + await import("../../registry/env-writer.js"); + const envPath = path.join(cwd, ".env"); + const existing = readEnvFile(envPath); + const newEnv = plan.dotEnv.filter( + (e, i) => + !(e.env in existing) && + plan.dotEnv.findIndex((x) => x.env === e.env) === i, + ); + appendToFile(envPath, serializeEnvAppend(newEnv)); + if (newEnv.length > 0) { + console.log( + `${pc.green("Updated .env:")} ${newEnv.map((e) => e.env).join(", ")}`, + ); + } + + if (planHasContent(plan)) { + const result = writeConfig(cwd, plan); + reportConfigWrite(result); + if (result.databricksYmlChanged) { + validateBundle(cwd, opts.profile ?? profileFromEnv(cwd)); + } + } +} + +async function registerInServer( + cwd: string, + plugin: ManifestPlugin, +): Promise { + const serverRoot = + SERVER_SUBDIRS.map((s) => path.join(cwd, s)).find((p) => + fs.existsSync(p), + ) ?? cwd; + const importPath = + plugin.stability === "beta" ? `${plugin.package}/beta` : plugin.package; + // Lazy: server-register pulls in the @ast-grep/napi native addon. + const { registerPluginInServer } = + await import("../../registry/server-register.js"); + { + { + const result = registerPluginInServer( + serverRoot, + importPath, + plugin.name, + ); + const shown = + result.file && path.relative(cwd, path.join(serverRoot, result.file)); + if (result.status === "wired") { + console.log(`${pc.green("Registered")} ${plugin.name} in ${shown}`); + } else if (result.status === "already") { + console.log(pc.dim(`${plugin.name} is already registered in ${shown}`)); + } else { + console.log( + pc.yellow( + `Couldn't auto-register ${plugin.name}: ${result.reason}.`, + ) + + `\nAdd it to your server's createApp call:\n` + + pc.dim( + ` import { ${plugin.name} } from "${importPath}";\n` + + ` createApp({ plugins: [${plugin.name}(), /* ... */] });`, + ), + ); + } + } + } +} + +export const pluginAddCommand = new Command("add") + .description( + "Add a plugin from appkit.plugins.json to the app: server registration, .env, app.yaml and databricks.yml", + ) + .argument("", "Plugin name, as in appkit.plugins.json") + .option( + "--set ", + "Set resource values (format: plugin.resourceKey.field=value, can specify multiple)", + (v: string, acc: string[]) => [...acc, v], + [] as string[], + ) + .option( + "--auth-mode ", + "Default resource auth mode: obo (on behalf of the user) or sp (service principal). With obo, resources that cannot be accessed on behalf of the user use sp. Prompts if not provided in interactive mode, otherwise sp.", + ) + .option("-C, --cwd ", "Run as if started in ") + .option("-y, --yes", "Don't prompt; fail on missing required values") + .option("--no-register", "Don't edit the server entry to register the plugin") + .option( + "-p, --profile ", + "Databricks profile for bundle validate (defaults to the app's DATABRICKS_CONFIG_PROFILE)", + ) + .addHelpText( + "after", + ` +Resource configuration (--set), as in \`databricks apps init\`: + Set resource values using --set plugin.resourceKey.field=value + Keys are defined in the app's appkit.plugins.json manifest. + Multi-field resources (e.g., database, secret) require all fields to be set together. + Use --set plugin.resourceKey.authMode=obo|sp|both to choose how the app accesses a resource: + on behalf of the user (obo), as the app's service principal (sp), or both. + obo/both add the resource's user_api_scopes; sp/both bind it with its permission. + +Existing entries in .env, app.yaml and databricks.yml are never overwritten. + +Examples: + $ appkit plugin add analytics --set analytics.sql-warehouse.id=abc123 + $ appkit plugin add analytics --auth-mode obo --set analytics.sql-warehouse.id=abc123 + $ appkit plugin add jobs --auth-mode obo --set jobs.job.authMode=sp --set jobs.job.id=42`, + ) + .action((name: string, opts: AddOptions) => + runPluginAdd(name, opts).catch((err: Error) => { + console.error(pc.red(err.message)); + process.exit(1); + }), + ); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.test.ts b/packages/shared/src/cli/commands/plugin/add/plan.test.ts new file mode 100644 index 000000000..f2ae70319 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/plan.test.ts @@ -0,0 +1,194 @@ +import { describe, expect, test } from "vitest"; + +import { + buildAddPlan, + findMissingValues, + type ManifestPlugin, + parseSetAuthModes, + parseSetValues, + resolveAuthModes, + selectResources, + validSetKeys, +} from "./plan"; + +const warehouse = { + type: "sql_warehouse", + alias: "SQL Warehouse", + resourceKey: "sql-warehouse", + description: "Warehouse", + permission: "CAN_USE", + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID", description: "id" } }, + scope: "sql", + binding: { yamlKey: "sql_warehouse", varFields: [["id", "id"]] }, +} satisfies ManifestPlugin["resources"]["required"][number]; + +const secret = { + type: "secret", + alias: "Secret", + resourceKey: "secret", + permission: "READ", + fields: { key: { env: "SECRET_KEY" }, scope: { env: "SECRET_SCOPE" } }, + appOnly: true, + binding: { + yamlKey: "secret", + varFields: [ + ["scope", "scope"], + ["key", "key"], + ], + }, +} satisfies ManifestPlugin["resources"]["optional"][number]; + +const job = { + type: "job", + alias: "Job", + resourceKey: "job", + permission: "CAN_MANAGE_RUN", + fields: { id: { env: "DATABRICKS_JOB_ID" } }, + binding: { yamlKey: "job", varFields: [["id", "id"]] }, +} satisfies ManifestPlugin["resources"]["optional"][number]; + +const plugin: ManifestPlugin = { + name: "analytics", + package: "@databricks/appkit", + scopes: ["ai-gateway"], + resources: { required: [warehouse], optional: [secret, job] }, +}; + +describe("parseSetValues", () => { + test("keys values by resourceKey.field and skips authMode", () => { + expect( + parseSetValues( + [ + "analytics.sql-warehouse.id=wh", + "analytics.sql-warehouse.authMode=obo", + ], + plugin, + ), + ).toEqual({ "sql-warehouse.id": "wh" }); + }); + + test.each([ + ["other.sql-warehouse.id=x", "unknown plugin"], + ["analytics.nope.id=x", "does not match a resource declared"], + ["analytics.sql-warehouse=x", "invalid --set key"], + ["analytics.secret.scope=s", "incomplete resource"], + ])("%s → %s", (sv, err) => { + expect(() => parseSetValues([sv], plugin)).toThrow(err); + }); +}); + +test("validSetKeys derives keys and values from the manifest", () => { + const keys = validSetKeys(plugin); + expect([...keys.keys()]).toEqual([ + "analytics.sql-warehouse.id", + "analytics.sql-warehouse.authMode", + "analytics.secret.key", + "analytics.secret.scope", + "analytics.job.id", + "analytics.job.authMode", + ]); + expect(keys.get("analytics.job.authMode")).toEqual(["sp"]); + expect(() => + parseSetValues(["analytics.sql-warehouse.name=x"], plugin), + ).toThrow("valid keys: analytics.sql-warehouse.id"); + expect(() => + parseSetAuthModes(["analytics.nope.authMode=sp"], plugin), + ).toThrow("does not match a resource declared"); +}); + +describe("parseSetAuthModes", () => { + test.each([ + ["analytics.sql-warehouse.authMode=user", "invalid auth mode"], + ["analytics.secret.authMode=sp", "always accessed by the app's service"], + ["analytics.job.authMode=obo", "cannot be accessed on behalf of the user"], + ])("%s → %s", (sv, err) => { + expect(() => parseSetAuthModes([sv], plugin)).toThrow(err); + }); + + test("accepts both and sp without scope", () => { + expect( + parseSetAuthModes( + ["analytics.sql-warehouse.authMode=both", "analytics.job.authMode=sp"], + plugin, + ), + ).toEqual({ "sql_warehouse:sql-warehouse": "both", "job:job": "sp" }); + }); +}); + +test("resolveAuthModes: --set wins, obo default skips non-capable", () => { + const { modes, keptSP } = resolveAuthModes( + [warehouse, secret, job], + {}, + {}, + "obo", + ); + expect(modes).toEqual({ "sql_warehouse:sql-warehouse": "obo" }); + expect(keptSP).toEqual(["secret", "job"]); + expect( + resolveAuthModes( + [warehouse], + { "sql_warehouse:sql-warehouse": "sp" }, + {}, + "obo", + ).modes, + ).toEqual({}); +}); + +describe("buildAddPlan", () => { + const values = { "sql-warehouse.id": "wh", "job.id": "42" }; + const resources = selectResources(plugin, values); + + test("sp binds the resource with its permission and no scopes", () => { + const plan = buildAddPlan(plugin, resources, values, {}); + expect(resources.map((r) => r.resourceKey)).toEqual([ + "sql-warehouse", + "job", + ]); + expect(plan.userApiScopes).toEqual([]); + expect(plan.appYamlEnv).toEqual([ + { name: "DATABRICKS_WAREHOUSE_ID", valueFrom: "sql-warehouse" }, + { name: "DATABRICKS_JOB_ID", valueFrom: "job" }, + ]); + expect(plan.resourceBindings[0]).toEqual({ + name: "sql-warehouse", + type: "sql_warehouse", + permission: "CAN_USE", + fields: { id: "${var.sql_warehouse_id}" }, + }); + expect(plan.bundleVariables[0]).toMatchObject({ + name: "sql_warehouse_id", + value: "wh", + }); + expect(plan.dotEnv).toContainEqual({ + env: "DATABRICKS_WAREHOUSE_ID", + value: "wh", + }); + }); + + test("obo uses a literal value, no binding, and adds scopes", () => { + const modes = { "sql_warehouse:sql-warehouse": "obo" } as const; + const plan = buildAddPlan(plugin, resources, values, modes); + expect(plan.appYamlEnv[0]).toEqual({ + name: "DATABRICKS_WAREHOUSE_ID", + value: "wh", + }); + expect(plan.resourceBindings.map((b) => b.name)).toEqual(["job"]); + expect(plan.userApiScopes).toEqual(["sql", "ai-gateway"]); + }); + + test("both binds and adds the scope", () => { + const modes = { "sql_warehouse:sql-warehouse": "both" } as const; + const plan = buildAddPlan(plugin, resources, values, modes); + expect(plan.resourceBindings.map((b) => b.name)).toEqual([ + "sql-warehouse", + "job", + ]); + expect(plan.userApiScopes).toEqual(["sql", "ai-gateway"]); + }); +}); + +test("findMissingValues names the --set key", () => { + expect( + findMissingValues("analytics", [warehouse], {}, {}).map((m) => m.setKey), + ).toEqual(["analytics.sql-warehouse.id"]); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.ts b/packages/shared/src/cli/commands/plugin/add/plan.ts new file mode 100644 index 000000000..2f3204443 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/plan.ts @@ -0,0 +1,417 @@ +import type { AppYamlEnvEntry, ResourceBinding } from "../../../deploy-config"; +import type { BundleVariable, ConfigPlan } from "../../registry/config-plan"; +import { isValidEnvName } from "../../registry/requirements"; + +/** + * Pure planning for `appkit plugin add`: a port of the Databricks CLI's + * `apps init` resource handling (cmd/apps/init.go + libs/apps/generator), so a + * plugin added later gets the same .env, app.yaml and databricks.yml entries + * (and the same --set / --auth-mode semantics) it would have had at init. + */ + +export const AUTH_MODE_SP = "sp"; +export const AUTH_MODE_OBO = "obo"; +export const AUTH_MODE_BOTH = "both"; +export type AuthMode = + | typeof AUTH_MODE_SP + | typeof AUTH_MODE_OBO + | typeof AUTH_MODE_BOTH; + +/** Reserved --set field that selects a resource's auth mode. */ +export const AUTH_MODE_FIELD = "authMode"; + +export interface ManifestField { + env?: string; + description?: string; + value?: string; + localOnly?: boolean; + bundleIgnore?: boolean; +} + +export interface ManifestResource { + type: string; + alias: string; + resourceKey: string; + description?: string; + permission?: string; + fields?: Record; + scope?: string; + appOnly?: boolean; + binding?: { + yamlKey: string; + varFields: [string, string][]; + staticFields?: [string, string][]; + }; +} + +/** A plugin entry of the app's appkit.plugins.json (the fields used here). */ +export interface ManifestPlugin { + name: string; + package: string; + stability?: string; + scopes?: string[]; + resources: { required: ManifestResource[]; optional: ManifestResource[] }; +} + +/** Same identity as resource de-duplication: type + key. */ +export function authKey(r: ManifestResource): string { + return `${r.type}:${r.resourceKey}`; +} + +/** Keys `resourceKey.field`, as the CLI's resourceValues map. */ +function valueKey(r: ManifestResource, field: string): string { + return `${r.resourceKey}.${field}`; +} + +function fieldNames(r: ManifestResource): string[] { + return Object.keys(r.fields ?? {}).sort(); +} + +function allResources(p: ManifestPlugin): ManifestResource[] { + return [...p.resources.required, ...p.resources.optional]; +} + +/** Manifest fields the resource's databricks.yml binding references. */ +function bindingVarFields(r: ManifestResource): string[] { + return (r.binding?.varFields ?? []).map(([f]) => f); +} + +/** + * The --set keys the plugin manifest declares, each with its valid values: + * `plugin.resourceKey.field` for every declared field and binding var field + * (any value), plus `plugin.resourceKey.authMode` for resources that are not + * app-only (obo/both only when the resource declares a user scope). + */ +export function validSetKeys( + plugin: ManifestPlugin, +): Map { + const keys = new Map(); + for (const r of allResources(plugin)) { + const prefix = `${plugin.name}.${r.resourceKey}.`; + for (const f of new Set([...fieldNames(r), ...bindingVarFields(r)])) { + keys.set(prefix + f, null); + } + if (!r.appOnly) { + keys.set( + prefix + AUTH_MODE_FIELD, + r.scope + ? [AUTH_MODE_OBO, AUTH_MODE_SP, AUTH_MODE_BOTH] + : [AUTH_MODE_SP], + ); + } + } + return keys; +} + +/** Throws unless the key is one the plugin manifest declares. */ +function assertDeclared(plugin: ManifestPlugin, sv: string, key: string): void { + const keys = validSetKeys(plugin); + if (keys.has(key)) return; + throw new Error( + `--set "${sv}" does not match a resource declared by plugin "${plugin.name}"; valid keys: ${[...keys.keys()].join(", ") || "(none, the plugin declares no resources)"}`, + ); +} + +function splitSetKey(sv: string): { + pluginName: string; + resourceKey: string; + field: string; + value: string; +} { + const eq = sv.indexOf("="); + const key = eq === -1 ? "" : sv.slice(0, eq); + if (!key) { + throw new Error( + `invalid --set format "${sv}", expected plugin.resourceKey.field=value`, + ); + } + const parts = key.split("."); + if (parts.length < 3) { + throw new Error( + `invalid --set key "${key}", expected plugin.resourceKey.field`, + ); + } + return { + pluginName: parts[0], + resourceKey: parts[1], + field: parts.slice(2).join("."), + value: sv.slice(eq + 1), + }; +} + +/** + * Parses `--set plugin.resourceKey.field=value` into `resourceKey.field` → value, + * validating plugin, resource and field against the manifest and that + * multi-field resources are set together. `authMode` pairs are skipped (see + * {@link parseSetAuthModes}). + */ +export function parseSetValues( + setValues: string[], + plugin: ManifestPlugin, +): Record { + const rv: Record = {}; + for (const sv of setValues) { + const { pluginName, resourceKey, field, value } = splitSetKey(sv); + if (field === AUTH_MODE_FIELD) continue; + if (pluginName !== plugin.name) { + throw new Error( + `unknown plugin "${pluginName}" in --set "${sv}"; this command adds "${plugin.name}"`, + ); + } + assertDeclared(plugin, sv, `${pluginName}.${resourceKey}.${field}`); + rv[`${resourceKey}.${field}`] = value; + } + + for (const r of allResources(plugin)) { + const checkable = fieldNames(r).filter( + (f) => !r.fields?.[f].bundleIgnore && !r.fields?.[f].localOnly, + ); + if (checkable.length <= 1) continue; + const missing = checkable.filter((f) => !rv[valueKey(r, f)]); + if (missing.length > 0 && missing.length < checkable.length) { + throw new Error( + `incomplete resource "${r.resourceKey}": missing fields [${missing.map((f) => valueKey(r, f)).join(" ")}] (all fields must be set together)`, + ); + } + } + return rv; +} + +/** Parses `--set plugin.resourceKey.authMode=obo|sp|both` into authKey → mode. */ +export function parseSetAuthModes( + setValues: string[], + plugin: ManifestPlugin, +): Record { + const modes: Record = {}; + for (const sv of setValues) { + const { pluginName, resourceKey, field, value } = splitSetKey(sv); + if (field !== AUTH_MODE_FIELD) continue; + if (pluginName !== plugin.name) { + throw new Error( + `unknown plugin "${pluginName}" in --set "${sv}"; this command adds "${plugin.name}"`, + ); + } + const r = allResources(plugin).find((x) => x.resourceKey === resourceKey); + if (!r) + throw assertDeclared(plugin, sv, `${pluginName}.${resourceKey}.${field}`); + if ( + value !== AUTH_MODE_SP && + value !== AUTH_MODE_OBO && + value !== AUTH_MODE_BOTH + ) { + throw new Error( + `invalid auth mode "${value}" in --set "${sv}" (must be obo, sp, or both)`, + ); + } + if (r.appOnly) { + throw new Error( + `resource "${resourceKey}" of plugin "${plugin.name}" is always accessed by the app's service principal; remove --set ${sv}`, + ); + } + if (value !== AUTH_MODE_SP && !r.scope) { + throw new Error( + `resource "${resourceKey}" of plugin "${plugin.name}" cannot be accessed on behalf of the user; use ${plugin.name}.${resourceKey}.${AUTH_MODE_FIELD}=sp`, + ); + } + modes[authKey(r)] = value; + } + return modes; +} + +/** True when the resource can be accessed on behalf of the user. */ +export function oboCapable(r: ManifestResource): boolean { + return Boolean(r.scope) && !r.appOnly; +} + +/** + * Auth mode per resource. Precedence: --set authMode, the prompted choice, the + * --auth-mode default, then sp. The obo default only applies to OBO-capable + * resources; the others are returned in `keptSP`. Only obo/both are stored. + */ +export function resolveAuthModes( + resources: ManifestResource[], + setModes: Record, + promptedModes: Record, + defaultMode: string | undefined, +): { modes: Record; keptSP: string[] } { + const modes: Record = {}; + const keptSP: string[] = []; + for (const r of resources) { + let mode: AuthMode | undefined = + setModes[authKey(r)] ?? promptedModes[authKey(r)]; + if (mode === undefined && defaultMode === AUTH_MODE_OBO) { + if (!oboCapable(r)) { + keptSP.push(r.resourceKey); + continue; + } + mode = AUTH_MODE_OBO; + } + if (mode === AUTH_MODE_OBO || mode === AUTH_MODE_BOTH) { + modes[authKey(r)] = mode; + } + } + return { modes, keptSP }; +} + +/** True when any field value of the resource is set. */ +export function resourceConfigured( + r: ManifestResource, + values: Record, +): boolean { + return [...fieldNames(r), ...bindingVarFields(r)].some( + (f) => values[valueKey(r, f)], + ); +} + +/** + * The resources that end up in the app: every required one, plus optional + * ones that have a value. + */ +export function selectResources( + plugin: ManifestPlugin, + values: Record, +): ManifestResource[] { + return [ + ...plugin.resources.required, + ...plugin.resources.optional.filter((r) => resourceConfigured(r, values)), + ]; +} + +export interface MissingValue { + resource: ManifestResource; + field: string; + /** The --set key that supplies it. */ + setKey: string; +} + +/** + * Values still needed, in prompt order: for each resource bound to the service + * principal (sp/both), every field its binding references; for resources + * accessed only on behalf of the user, every field with an env var (its + * app.yaml entry carries the literal value). + */ +export function findMissingValues( + pluginName: string, + resources: ManifestResource[], + values: Record, + modes: Record, +): MissingValue[] { + const missing: MissingValue[] = []; + for (const r of resources) { + const fields = + modes[authKey(r)] === AUTH_MODE_OBO + ? fieldNames(r).filter( + (f) => r.fields?.[f].env && !r.fields[f].localOnly, + ) + : variableFields(r); + for (const field of fields) { + if (values[valueKey(r, field)] || r.fields?.[field]?.value) continue; + missing.push({ + resource: r, + field, + setKey: `${pluginName}.${r.resourceKey}.${field}`, + }); + } + } + return missing; +} + +/** Fields that become bundle variables: manifest fields plus binding var fields. */ +function variableFields(r: ManifestResource): string[] { + const out = fieldNames(r).filter( + (f) => !r.fields?.[f].bundleIgnore && !r.fields?.[f].localOnly, + ); + const covered = new Set(fieldNames(r)); + for (const f of bindingVarFields(r)) { + if (!covered.has(f)) out.push(f); + } + return out; +} + +function varName(r: ManifestResource, field: string): string { + return `${r.resourceKey.replaceAll("-", "_")}_${field}`; +} + +export interface AddPlan extends ConfigPlan { + /** .env lines to add (skipped later when the name is already present). */ + dotEnv: { env: string; value: string }[]; +} + +/** + * Builds the file changes for a plugin's resources, mirroring the CLI + * generator: OBO-only resources get a literal app.yaml `value` and no bundle + * binding; sp/both resources get variables, a binding and `valueFrom`; + * obo/both resources add their scope (plus the plugin's own scopes) to + * user_api_scopes. + */ +export function buildAddPlan( + plugin: ManifestPlugin, + resources: ManifestResource[], + values: Record, + modes: Record, +): AddPlan { + const dotEnv: AddPlan["dotEnv"] = []; + const appYamlEnv: AppYamlEnvEntry[] = []; + const bundleVariables: BundleVariable[] = []; + const resourceBindings: ResourceBinding[] = []; + const unverifiedTypes: string[] = []; + const scopes: string[] = []; + + for (const r of resources) { + const mode = modes[authKey(r)]; + const oboOnly = mode === AUTH_MODE_OBO; + if ((mode === AUTH_MODE_OBO || mode === AUTH_MODE_BOTH) && r.scope) { + scopes.push(r.scope); + } + + for (const f of fieldNames(r)) { + const field = r.fields?.[f] ?? {}; + if (!field.env || !isValidEnvName(field.env)) continue; + const value = (values[valueKey(r, f)] || field.value || "").replace( + /[\r\n]/g, + "", + ); + dotEnv.push({ env: field.env, value }); + if (field.localOnly) continue; + appYamlEnv.push( + oboOnly + ? { name: field.env, value } + : { name: field.env, valueFrom: r.resourceKey }, + ); + } + + if (oboOnly) continue; + if (!r.binding) { + unverifiedTypes.push(r.type); + continue; + } + for (const f of variableFields(r)) { + bundleVariables.push({ + name: varName(r, f), + description: r.fields?.[f]?.description || r.description, + value: values[valueKey(r, f)] || undefined, + }); + } + const fields: Record = {}; + for (const [mf, yf] of r.binding.varFields) { + fields[yf] = `\${var.${varName(r, mf)}}`; + } + for (const [yf, v] of r.binding.staticFields ?? []) fields[yf] = v; + resourceBindings.push({ + name: r.resourceKey, + type: r.binding.yamlKey, + permission: r.permission, + fields, + }); + } + + if (scopes.length > 0) scopes.push(...(plugin.scopes ?? [])); + return { + dotEnv, + appYamlEnv, + bundleVariables, + resourceBindings, + unverifiedTypes, + userApiScopes: [...new Set(scopes)], + }; +} diff --git a/packages/shared/src/cli/commands/plugin/index.ts b/packages/shared/src/cli/commands/plugin/index.ts index 6612d205a..810ae673b 100644 --- a/packages/shared/src/cli/commands/plugin/index.ts +++ b/packages/shared/src/cli/commands/plugin/index.ts @@ -1,6 +1,7 @@ import { Command } from "commander"; import { pluginAddResourceCommand } from "./add-resource/add-resource"; +import { pluginAddCommand } from "./add/add"; import { pluginCreateCommand } from "./create/create"; import { pluginListCommand } from "./list/list"; import { pluginPromoteCommand } from "./promote/promote"; @@ -14,6 +15,7 @@ import { pluginValidateCommand } from "./validate/validate"; * - create: Scaffold a new plugin (interactive) * - validate: Validate manifest(s) against the JSON schema * - list: List plugins from appkit.plugins.json or a directory + * - add: Add a plugin to the app (server, .env, app.yaml, databricks.yml) * - add-resource: Add a resource requirement to a plugin manifest (interactive) * - promote: Promote a plugin to a higher stability tier */ @@ -23,6 +25,7 @@ export const pluginCommand = new Command("plugin") .addCommand(pluginCreateCommand) .addCommand(pluginValidateCommand) .addCommand(pluginListCommand) + .addCommand(pluginAddCommand) .addCommand(pluginAddResourceCommand) .addCommand(pluginPromoteCommand) .addHelpText( @@ -33,6 +36,7 @@ Examples: $ appkit plugin create --placement in-repo --path plugins/my-plugin --name my-plugin --description "Does X" $ appkit plugin validate . $ appkit plugin list --json + $ appkit plugin add analytics --auth-mode obo --set analytics.sql-warehouse.id=abc123 $ appkit plugin add-resource --path plugins/my-plugin --type sql_warehouse $ appkit plugin promote my-plugin --to ga`, ); diff --git a/packages/shared/src/cli/commands/registry/config-plan.ts b/packages/shared/src/cli/commands/registry/config-plan.ts index 01260cf8a..168bd19f1 100644 --- a/packages/shared/src/cli/commands/registry/config-plan.ts +++ b/packages/shared/src/cli/commands/registry/config-plan.ts @@ -26,6 +26,8 @@ export interface ConfigPlan { appYamlEnv: AppYamlEnvEntry[]; bundleVariables: BundleVariable[]; resourceBindings: ResourceBinding[]; + /** Entries for resources.apps.app.user_api_scopes (added if absent). */ + userApiScopes?: string[]; /** Resource types encountered that have no verified binding spec. */ unverifiedTypes: string[]; } @@ -178,6 +180,7 @@ export function planHasContent(plan: ConfigPlan): boolean { return ( plan.appYamlEnv.length > 0 || plan.bundleVariables.length > 0 || - plan.resourceBindings.length > 0 + plan.resourceBindings.length > 0 || + (plan.userApiScopes?.length ?? 0) > 0 ); } diff --git a/packages/shared/src/cli/commands/registry/config-writer.test.ts b/packages/shared/src/cli/commands/registry/config-writer.test.ts index 9bcf7f04e..53c5152d7 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.test.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.test.ts @@ -195,3 +195,32 @@ describe("writeConfig — additive patching", () => { expect(sameYaml(db, db)).toBe(true); }); }); + +describe("writeConfig — user_api_scopes and literal env values", () => { + it("adds missing scopes once and writes literal app.yaml values", () => { + const cwd = makeTempDir(); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n app:\n user_api_scopes:\n - sql\n", + ); + const result = writeConfig(cwd, { + appYamlEnv: [{ name: "DATABRICKS_WAREHOUSE_ID", value: "wh" }], + bundleVariables: [], + resourceBindings: [], + userApiScopes: ["sql", "files.files"], + unverifiedTypes: [], + }); + const db = parseDocument( + fs.readFileSync(path.join(cwd, "databricks.yml"), "utf-8"), + ).toJSON(); + expect(db.resources.apps.app.user_api_scopes).toEqual([ + "sql", + "files.files", + ]); + expect(result.databricksYmlChanged).toBe(true); + const app = parseDocument( + fs.readFileSync(path.join(cwd, "app.yaml"), "utf-8"), + ).toJSON(); + expect(app.env).toEqual([{ name: "DATABRICKS_WAREHOUSE_ID", value: "wh" }]); + }); +}); diff --git a/packages/shared/src/cli/commands/registry/config-writer.ts b/packages/shared/src/cli/commands/registry/config-writer.ts index 479d30ed7..065d15f8d 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.ts @@ -52,7 +52,7 @@ function patchAppYaml(file: string, entries: AppYamlEnvEntry[]): string[] { const added: string[] = []; for (const entry of entries) { if (existingNames.has(entry.name)) continue; - seq.add(doc.createNode({ name: entry.name, valueFrom: entry.valueFrom })); + seq.add(doc.createNode(entry)); added.push(entry.name); } @@ -89,7 +89,11 @@ function patchDatabricksYml( file: string, plan: ConfigPlan, ): { added: string[]; changed: boolean } { - if (plan.bundleVariables.length === 0 && plan.resourceBindings.length === 0) { + if ( + plan.bundleVariables.length === 0 && + plan.resourceBindings.length === 0 && + !plan.userApiScopes?.length + ) { return { added: [], changed: false }; } const doc = loadDoc(file); @@ -127,6 +131,21 @@ function patchDatabricksYml( } } + if (plan.userApiScopes?.length) { + const app = ensureMap(doc, ["resources", "apps", "app"]); + let scopes = app.get("user_api_scopes") as YAMLSeq | undefined; + if (!scopes || typeof (scopes as YAMLSeq).add !== "function") { + app.set("user_api_scopes", doc.createNode([])); + scopes = app.get("user_api_scopes") as YAMLSeq; + } + const existing = new Set(scopes.toJSON() as unknown[]); + for (const scope of plan.userApiScopes) { + if (existing.has(scope)) continue; + scopes.add(scope); + added.push(scope); + } + } + // Target-level variable values. Tracked separately from `added` because the // binding/top-level var may already exist (e.g. from scaffold) while the // target VALUE is still missing — in that case nothing is in `added` yet the diff --git a/packages/shared/src/cli/commands/registry/env-writer.ts b/packages/shared/src/cli/commands/registry/env-writer.ts index 4f06b66e8..7f136a578 100644 --- a/packages/shared/src/cli/commands/registry/env-writer.ts +++ b/packages/shared/src/cli/commands/registry/env-writer.ts @@ -46,13 +46,13 @@ const MANUAL = "__manual__"; const PICKER_LIMIT = 25; /** Reads a `.env`-style file into a map; empty when the file is absent. */ -function readEnvFile(file: string): Record { +export function readEnvFile(file: string): Record { if (!fs.existsSync(file)) return {}; return parseEnv(fs.readFileSync(file, "utf-8")); } /** Appends text to a file, creating it (with a trailing newline) if needed. */ -function appendToFile(file: string, text: string): void { +export function appendToFile(file: string, text: string): void { if (text === "") return; if (fs.existsSync(file)) { const existing = fs.readFileSync(file, "utf-8"); diff --git a/packages/shared/src/cli/commands/registry/server-register.ts b/packages/shared/src/cli/commands/registry/server-register.ts index 327b0f4ee..eab825269 100644 --- a/packages/shared/src/cli/commands/registry/server-register.ts +++ b/packages/shared/src/cli/commands/registry/server-register.ts @@ -91,11 +91,14 @@ export function registerPluginInServer( // exportName and importPath are interpolated into the user's server source. // Registry items are untrusted, so refuse anything that isn't a plain JS // identifier / clean relative module path — prevents code injection via a - // crafted export name or import path. + // crafted export name or import path. Core plugins import from the AppKit package. if (!JS_IDENTIFIER.test(exportName)) { return { status: "skipped", reason: "invalid plugin export name" }; } - if (!/^[.][./A-Za-z0-9_-]*$/.test(importPath)) { + if ( + !/^[.][./A-Za-z0-9_-]*$/.test(importPath) && + !/^@databricks\/appkit(\/beta)?$/.test(importPath) + ) { return { status: "skipped", reason: "invalid plugin import path" }; } diff --git a/packages/shared/src/cli/deploy-config.ts b/packages/shared/src/cli/deploy-config.ts index 9da8c0df4..b0bdec65d 100644 --- a/packages/shared/src/cli/deploy-config.ts +++ b/packages/shared/src/cli/deploy-config.ts @@ -10,11 +10,14 @@ export const DATABRICKS_YML_FILE = "databricks.yml"; /** Canonical Databricks Apps runtime config file name. */ export const APP_YAML_FILE = "app.yaml"; -/** An `app.yaml` env entry: `- name: `, `valueFrom: `. */ -export interface AppYamlEnvEntry { - name: string; - valueFrom: string; -} +/** + * An `app.yaml` env entry: `- name: ` with `valueFrom: `, + * or a literal `value` (resources accessed only on behalf of the user have no + * binding to reference). + */ +export type AppYamlEnvEntry = + | { name: string; valueFrom: string } + | { name: string; value: string }; /** * A `databricks.yml` app-resource binding under `resources.apps..resources[]`, From 1232c23f6cb81932e072e9c2767377b9e2efa507 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 14:34:20 +0200 Subject: [PATCH 2/8] fix(appkit): plugin add OBO files auth, genie scopes, lakebase derived values, local imports - files/aiSearch with only OBO resources are registered with { auth: "on-behalf-of-user" } so the runtime matches the unbound plan - plugin-level scopes (genie) are always written, as the init template does - fields with a resolver (lakebase PGHOST, PGDATABASE, LAKEBASE_ENDPOINT) are resolved from the branch/database like apps init, else prompted or rejected; never written empty - local plugin packages are rebased from the app root to the server file Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../shared/src/cli/commands/plugin/add/add.ts | 176 +++++++++++++----- .../src/cli/commands/plugin/add/plan.test.ts | 25 ++- .../src/cli/commands/plugin/add/plan.ts | 14 +- .../cli/commands/plugin/add/resolve.test.ts | 63 +++++++ .../src/cli/commands/plugin/add/resolve.ts | 49 +++++ .../commands/registry/server-register.test.ts | 15 ++ .../cli/commands/registry/server-register.ts | 6 +- 7 files changed, 292 insertions(+), 56 deletions(-) create mode 100644 packages/shared/src/cli/commands/plugin/add/resolve.test.ts create mode 100644 packages/shared/src/cli/commands/plugin/add/resolve.ts diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts index bab01fbc2..add3a941a 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -24,6 +24,7 @@ import { findMissingValues, type ManifestPlugin, type ManifestResource, + type MissingValue, oboCapable, parseSetAuthModes, parseSetValues, @@ -31,6 +32,7 @@ import { resourceConfigured, selectResources, } from "./plan"; +import { resolveDerivedValues } from "./resolve"; const MANIFEST_FILE = "appkit.plugins.json"; const SERVER_SUBDIRS = ["server", "api", "backend"]; @@ -152,6 +154,34 @@ async function promptAuthModes( return modes; } +/** Prompts for each missing value, or fails on the first one when not interactive. */ +async function fillMissing( + missing: MissingValue[], + values: Record, + interactive: boolean, +): Promise { + if (missing.length > 0 && !interactive) { + const m = missing[0]; + throw new Error( + `missing value for ${m.setKey}; use --set ${m.setKey}=value`, + ); + } + if (missing.length === 0) return; + const { text } = await import("@clack/prompts"); + for (const m of missing) { + const field = m.resource.fields?.[m.field]; + const value = await answered( + await text({ + message: `${m.resource.alias}: ${m.field}`, + placeholder: field?.description, + validate: (v) => + !v?.trim() || /[\r\n]/.test(v) ? "Enter a value" : undefined, + }), + ); + values[`${m.resource.resourceKey}.${m.field}`] = value.trim(); + } +} + async function runPluginAdd(name: string, opts: AddOptions): Promise { const cwd = opts.cwd ? path.resolve(opts.cwd) : process.cwd(); if ( @@ -189,33 +219,42 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { ); } - const missing = findMissingValues(plugin.name, resources, values, modes); - if (missing.length > 0 && !interactive) { - const m = missing[0]; - throw new Error( - `missing value for ${m.setKey}; use --set ${m.setKey}=value`, - ); - } - if (missing.length > 0) { - const { text } = await import("@clack/prompts"); - for (const m of missing) { - const field = m.resource.fields?.[m.field]; - const value = await answered( - await text({ - message: `${m.resource.alias}: ${m.field}`, - placeholder: field?.description, - validate: (v) => - !v?.trim() || /[\r\n]/.test(v) ? "Enter a value" : undefined, - }), + const missingNow = () => + findMissingValues(plugin.name, resources, values, modes); + // Values the user supplies come first: the derived ones need them. + await fillMissing( + missingNow().filter((m) => !m.resource.fields?.[m.field]?.resolve), + values, + interactive, + ); + const profile = opts.profile ?? profileFromEnv(cwd); + for (const r of resources) { + try { + for (const [k, v] of Object.entries( + await resolveDerivedValues(r, values, profile), + )) { + values[k] ||= v; + } + } catch (err) { + console.warn( + pc.yellow( + `Could not resolve derived values for ${r.alias}: ${(err as Error).message}`, + ), ); - values[`${m.resource.resourceKey}.${m.field}`] = value.trim(); } } + await fillMissing(missingNow(), values, interactive); resources = selectResources(plugin, values); const plan = buildAddPlan(plugin, resources, values, modes); - if (opts.register !== false) await registerInServer(cwd, plugin); + if (opts.register !== false) { + await registerInServer( + cwd, + plugin, + pluginCallArgs(plugin, resources, modes), + ); + } const { appendToFile, readEnvFile } = await import("../../registry/env-writer.js"); @@ -237,50 +276,91 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { const result = writeConfig(cwd, plan); reportConfigWrite(result); if (result.databricksYmlChanged) { - validateBundle(cwd, opts.profile ?? profileFromEnv(cwd)); + validateBundle(cwd, profile); } } } +/** + * Plugins whose identity is a plugin-level `auth` option (default service + * principal) rather than per-request: OBO-only resources need it set, or the + * runtime uses the service principal that the OBO plan leaves unbound. + */ +const AUTH_CONFIG_PLUGINS = new Set(["files", "aiSearch"]); + +/** The `createApp` call arguments for the plugin: the auth option, if any. */ +function pluginCallArgs( + plugin: ManifestPlugin, + resources: ManifestResource[], + modes: Record, +): string { + if (!AUTH_CONFIG_PLUGINS.has(plugin.name)) return ""; + const obo = resources.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); + if (obo.length === 0) return ""; + if (obo.length < resources.length) { + console.warn( + pc.yellow( + `${plugin.name} has one auth option for all its resources; set auth per volume in its config to mix on-behalf-of-user (${obo.map((r) => r.resourceKey).join(", ")}) with service principal access.`, + ), + ); + return ""; + } + return `{ auth: "on-behalf-of-user" }`; +} + async function registerInServer( cwd: string, plugin: ManifestPlugin, + callArgs: string, ): Promise { const serverRoot = SERVER_SUBDIRS.map((s) => path.join(cwd, s)).find((p) => fs.existsSync(p), ) ?? cwd; - const importPath = - plugin.stability === "beta" ? `${plugin.package}/beta` : plugin.package; // Lazy: server-register pulls in the @ast-grep/napi native addon. - const { registerPluginInServer } = + const { findServerFile, registerPluginInServer } = await import("../../registry/server-register.js"); - { - { - const result = registerPluginInServer( - serverRoot, - importPath, - plugin.name, + let importPath = + plugin.stability === "beta" ? `${plugin.package}/beta` : plugin.package; + // Local plugins are recorded relative to the app root; imports resolve + // relative to the server file. + if (plugin.package.startsWith(".")) { + const from = path.dirname(findServerFile(serverRoot) ?? serverRoot); + const rel = path + .relative(from, path.resolve(cwd, plugin.package)) + .split(path.sep) + .join("/"); + importPath = rel.startsWith(".") ? rel : `./${rel}`; + } + const call = `${plugin.name}(${callArgs})`; + const result = registerPluginInServer( + serverRoot, + importPath, + plugin.name, + callArgs, + ); + const shown = + result.file && path.relative(cwd, path.join(serverRoot, result.file)); + if (result.status === "wired") { + console.log(`${pc.green("Registered")} ${call} in ${shown}`); + } else if (result.status === "already") { + console.log(pc.dim(`${plugin.name} is already registered in ${shown}`)); + if (callArgs) { + console.warn( + pc.yellow( + ` Make sure it is configured as ${call} for the chosen auth mode.`, + ), ); - const shown = - result.file && path.relative(cwd, path.join(serverRoot, result.file)); - if (result.status === "wired") { - console.log(`${pc.green("Registered")} ${plugin.name} in ${shown}`); - } else if (result.status === "already") { - console.log(pc.dim(`${plugin.name} is already registered in ${shown}`)); - } else { - console.log( - pc.yellow( - `Couldn't auto-register ${plugin.name}: ${result.reason}.`, - ) + - `\nAdd it to your server's createApp call:\n` + - pc.dim( - ` import { ${plugin.name} } from "${importPath}";\n` + - ` createApp({ plugins: [${plugin.name}(), /* ... */] });`, - ), - ); - } } + } else { + console.log( + pc.yellow(`Couldn't auto-register ${plugin.name}: ${result.reason}.`) + + "\nAdd it to your server's createApp call:\n" + + pc.dim( + ` import { ${plugin.name} } from "${importPath}";\n` + + ` createApp({ plugins: [${call}, /* ... */] });`, + ), + ); } } diff --git a/packages/shared/src/cli/commands/plugin/add/plan.test.ts b/packages/shared/src/cli/commands/plugin/add/plan.test.ts index f2ae70319..5fb75a28c 100644 --- a/packages/shared/src/cli/commands/plugin/add/plan.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/plan.test.ts @@ -138,13 +138,14 @@ describe("buildAddPlan", () => { const values = { "sql-warehouse.id": "wh", "job.id": "42" }; const resources = selectResources(plugin, values); - test("sp binds the resource with its permission and no scopes", () => { + test("sp binds the resource with its permission; only plugin scopes", () => { const plan = buildAddPlan(plugin, resources, values, {}); expect(resources.map((r) => r.resourceKey)).toEqual([ "sql-warehouse", "job", ]); - expect(plan.userApiScopes).toEqual([]); + // Plugin scopes (genie-style always-OBO routes) apply in sp mode too. + expect(plan.userApiScopes).toEqual(["ai-gateway"]); expect(plan.appYamlEnv).toEqual([ { name: "DATABRICKS_WAREHOUSE_ID", valueFrom: "sql-warehouse" }, { name: "DATABRICKS_JOB_ID", valueFrom: "job" }, @@ -192,3 +193,23 @@ test("findMissingValues names the --set key", () => { findMissingValues("analytics", [warehouse], {}, {}).map((m) => m.setKey), ).toEqual(["analytics.sql-warehouse.id"]); }); + +test("fields with a resolver are required even when localOnly", () => { + const pg = { + type: "postgres", + alias: "Postgres", + resourceKey: "postgres", + appOnly: true, + fields: { + branch: {}, + host: { env: "PGHOST", localOnly: true, resolve: "postgres:host" }, + port: { env: "PGPORT", localOnly: true, value: "5432" }, + }, + binding: { yamlKey: "postgres", varFields: [["branch", "branch"]] }, + } satisfies ManifestPlugin["resources"]["required"][number]; + expect( + findMissingValues("lakebase", [pg], { "postgres.branch": "b" }, {}).map( + (m) => m.setKey, + ), + ).toEqual(["lakebase.postgres.host"]); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.ts b/packages/shared/src/cli/commands/plugin/add/plan.ts index 2f3204443..e812edc99 100644 --- a/packages/shared/src/cli/commands/plugin/add/plan.ts +++ b/packages/shared/src/cli/commands/plugin/add/plan.ts @@ -26,6 +26,8 @@ export interface ManifestField { value?: string; localOnly?: boolean; bundleIgnore?: boolean; + /** Derived-value resolver key, e.g. "postgres:host" (see resolve.ts). */ + resolve?: string; } export interface ManifestResource { @@ -288,7 +290,8 @@ export interface MissingValue { * Values still needed, in prompt order: for each resource bound to the service * principal (sp/both), every field its binding references; for resources * accessed only on behalf of the user, every field with an env var (its - * app.yaml entry carries the literal value). + * app.yaml entry carries the literal value). Fields with a resolver (e.g. + * Lakebase PGHOST) are always needed: local .env must carry them to connect. */ export function findMissingValues( pluginName: string, @@ -304,7 +307,8 @@ export function findMissingValues( (f) => r.fields?.[f].env && !r.fields[f].localOnly, ) : variableFields(r); - for (const field of fields) { + const resolved = fieldNames(r).filter((f) => r.fields?.[f].resolve); + for (const field of new Set([...fields, ...resolved])) { if (values[valueKey(r, field)] || r.fields?.[field]?.value) continue; missing.push({ resource: r, @@ -341,7 +345,7 @@ export interface AddPlan extends ConfigPlan { * Builds the file changes for a plugin's resources, mirroring the CLI * generator: OBO-only resources get a literal app.yaml `value` and no bundle * binding; sp/both resources get variables, a binding and `valueFrom`; - * obo/both resources add their scope (plus the plugin's own scopes) to + * obo/both resources add their scope, and the plugin always adds its own, to * user_api_scopes. */ export function buildAddPlan( @@ -405,7 +409,9 @@ export function buildAddPlan( }); } - if (scopes.length > 0) scopes.push(...(plugin.scopes ?? [])); + // Plugin scopes (e.g. genie) cover routes that always run on behalf of the + // user, so they apply whatever the resources' auth mode is. + scopes.push(...(plugin.scopes ?? [])); return { dotEnv, appYamlEnv, diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts new file mode 100644 index 000000000..7cb3b0d60 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts @@ -0,0 +1,63 @@ +import { expect, test, vi } from "vitest"; + +import type { ManifestResource } from "./plan"; +import { resolveDerivedValues } from "./resolve"; + +const listEndpointsIter = vi.fn(async function* () { + yield { name: "ro", status: { endpointType: "ENDPOINT_TYPE_READ_ONLY" } }; + yield { + name: "b/endpoints/rw", + status: { endpointType: "ENDPOINT_TYPE_READ_WRITE", hosts: { host: "h" } }, + }; +}); +const listDatabasesIter = vi.fn(async function* () { + yield { name: "b/databases/db", status: { postgresDatabase: "appdb" } }; +}); +vi.mock("../../registry/workspace-picker.js", () => ({ + makeWorkspaceClient: () => ({ + postgres: { listEndpointsIter, listDatabasesIter }, + }), +})); + +const pg: ManifestResource = { + type: "postgres", + alias: "Postgres", + resourceKey: "postgres", + fields: { + branch: {}, + database: {}, + host: { env: "PGHOST", resolve: "postgres:host" }, + databaseName: { env: "PGDATABASE", resolve: "postgres:databaseName" }, + endpointPath: { + env: "LAKEBASE_ENDPOINT", + resolve: "postgres:endpointPath", + }, + }, +}; + +test("maps the read-write endpoint and database to resolver fields", async () => { + const values = { + "postgres.branch": "b", + "postgres.database": "b/databases/db", + }; + expect(await resolveDerivedValues(pg, values, undefined)).toEqual({ + "postgres.host": "h", + "postgres.databaseName": "appdb", + "postgres.endpointPath": "b/endpoints/rw", + }); +}); + +test("skips the API when every derived field is set", async () => { + listEndpointsIter.mockClear(); + await resolveDerivedValues( + pg, + { + "postgres.branch": "b", + "postgres.host": "x", + "postgres.databaseName": "x", + "postgres.endpointPath": "x", + }, + undefined, + ); + expect(listEndpointsIter).not.toHaveBeenCalled(); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.ts b/packages/shared/src/cli/commands/plugin/add/resolve.ts new file mode 100644 index 000000000..94d4d69e2 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/resolve.ts @@ -0,0 +1,49 @@ +import type { ManifestResource } from "./plan"; + +/** + * Derived values for fields with a `resolve` key, a port of the Databricks + * CLI's ResolvePostgresValues (libs/apps/prompt): the branch's read-write + * endpoint gives `postgres:host` and `postgres:endpointPath`, the database + * gives `postgres:databaseName`. Returns `resourceKey.field` → value for the + * fields it could resolve; throws on API errors. + */ +export async function resolveDerivedValues( + r: ManifestResource, + values: Record, + profile: string | undefined, +): Promise> { + const branch = values[`${r.resourceKey}.branch`]; + const database = values[`${r.resourceKey}.database`]; + const needs = Object.entries(r.fields ?? {}).some( + ([name, f]) => + f.resolve?.startsWith("postgres:") && !values[`${r.resourceKey}.${name}`], + ); + if (r.type !== "postgres" || !needs || !branch) return {}; + + const { makeWorkspaceClient } = + await import("../../registry/workspace-picker.js"); + const pg = makeWorkspaceClient(profile).postgres; + const resolved: Record = {}; + for await (const ep of pg.listEndpointsIter({ parent: branch })) { + if (ep.status?.endpointType !== "ENDPOINT_TYPE_READ_WRITE") continue; + if (ep.name) resolved["postgres:endpointPath"] = ep.name; + if (ep.status.hosts?.host) resolved["postgres:host"] = ep.status.hosts.host; + break; + } + if (database) { + for await (const db of pg.listDatabasesIter({ parent: branch })) { + if (db.name !== database) continue; + if (db.status?.postgresDatabase) { + resolved["postgres:databaseName"] = db.status.postgresDatabase; + } + break; + } + } + + const out: Record = {}; + for (const [name, field] of Object.entries(r.fields ?? {})) { + const v = field.resolve && resolved[field.resolve]; + if (v) out[`${r.resourceKey}.${name}`] = v; + } + return out; +} diff --git a/packages/shared/src/cli/commands/registry/server-register.test.ts b/packages/shared/src/cli/commands/registry/server-register.test.ts index 0298545c7..fb96744ad 100644 --- a/packages/shared/src/cli/commands/registry/server-register.test.ts +++ b/packages/shared/src/cli/commands/registry/server-register.test.ts @@ -38,6 +38,21 @@ afterEach(() => { }); describe("registerPluginInServer", () => { + it("passes call args and accepts the AppKit package import path", () => { + const dir = makeTempDir(); + const file = writeServer(dir, "server.ts", "server()"); + const result = registerPluginInServer( + dir, + "@databricks/appkit", + "files", + `{ auth: "on-behalf-of-user" }`, + ); + expect(result.status).toBe("wired"); + const out = fs.readFileSync(file, "utf-8"); + expect(out).toContain(`files({ auth: "on-behalf-of-user" }), server()`); + expect(out).toContain(`import { files } from "@databricks/appkit";`); + }); + it("wires a plugin into a server root that is a subdir (e.g. api/)", () => { // The server root is the resolved subdir, not the repo root — entry files // are looked up within it, so an app laid out under api/ still gets wired. diff --git a/packages/shared/src/cli/commands/registry/server-register.ts b/packages/shared/src/cli/commands/registry/server-register.ts index eab825269..96442541c 100644 --- a/packages/shared/src/cli/commands/registry/server-register.ts +++ b/packages/shared/src/cli/commands/registry/server-register.ts @@ -20,7 +20,7 @@ export interface RegisterResult { reason?: string; } -function findServerFile(serverRoot: string): string | null { +export function findServerFile(serverRoot: string): string | null { for (const candidate of SERVER_FILE_CANDIDATES) { const p = path.join(serverRoot, candidate); if (fs.existsSync(p)) return p; @@ -87,6 +87,8 @@ export function registerPluginInServer( serverRoot: string, importPath: string, exportName: string, + /** Call arguments, e.g. `{ auth: "on-behalf-of-user" }` (CLI-built, never registry data). */ + callArgs = "", ): RegisterResult { // exportName and importPath are interpolated into the user's server source. // Registry items are untrusted, so refuse anything that isn't a plain JS @@ -127,7 +129,7 @@ export function registerPluginInServer( const edits = []; // toPlugin exports are factories, registered as a call: `hello()`. - const newElem = `${exportName}()`; + const newElem = `${exportName}(${callArgs})`; // Insert before the first element, matching its indentation so the array // formatting is preserved (or inline for a single-line array). From 6f1cb430f60e647b318d6f3b50c1d161b368eb22 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 14:44:39 +0200 Subject: [PATCH 3/8] fix(appkit): plugin add sets aiSearch OBO auth on its default index aiSearch routes read auth per index (indexes..auth), not from the plugin config, so an OBO add now registers aiSearch({ indexes: { default: { auth: "on-behalf-of-user" } } }). Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/cli/commands/plugin/add/add.test.ts | 34 +++++++++++++++++++ .../shared/src/cli/commands/plugin/add/add.ts | 21 +++++++----- 2 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 packages/shared/src/cli/commands/plugin/add/add.test.ts diff --git a/packages/shared/src/cli/commands/plugin/add/add.test.ts b/packages/shared/src/cli/commands/plugin/add/add.test.ts new file mode 100644 index 000000000..c3bac7389 --- /dev/null +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -0,0 +1,34 @@ +import { expect, test } from "vitest"; + +import { pluginCallArgs } from "./add"; +import type { ManifestPlugin, ManifestResource } from "./plan"; + +const index: ManifestResource = { + type: "vector_search_index", + alias: "Index", + resourceKey: "vector-search-index", + scope: "vector-search", +}; +const plugin = (name: string): ManifestPlugin => ({ + name, + package: "@databricks/appkit", + resources: { required: [index], optional: [] }, +}); +const obo = { "vector_search_index:vector-search-index": "obo" } as const; + +test("aiSearch OBO sets auth on the default index, where the routes read it", () => { + expect(pluginCallArgs(plugin("aiSearch"), [index], obo)).toBe( + `{ indexes: { default: { auth: "on-behalf-of-user" } } }`, + ); +}); + +test("files OBO sets the plugin-level auth", () => { + expect(pluginCallArgs(plugin("files"), [index], obo)).toBe( + `{ auth: "on-behalf-of-user" }`, + ); +}); + +test("no args for sp/both or plugins without an auth option", () => { + expect(pluginCallArgs(plugin("aiSearch"), [index], {})).toBe(""); + expect(pluginCallArgs(plugin("analytics"), [index], obo)).toBe(""); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts index add3a941a..781906fa1 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -282,30 +282,35 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { } /** - * Plugins whose identity is a plugin-level `auth` option (default service - * principal) rather than per-request: OBO-only resources need it set, or the - * runtime uses the service principal that the OBO plan leaves unbound. + * `createApp` arguments that make a plugin access its resources on behalf of + * the user, for plugins that otherwise default to the service principal (which + * an OBO plan leaves unbound). files reads a plugin-level `auth`; aiSearch reads + * it per index, and its `default` index takes DATABRICKS_VS_INDEX_NAME. */ -const AUTH_CONFIG_PLUGINS = new Set(["files", "aiSearch"]); +const OBO_CALL_ARGS: Record = { + files: `{ auth: "on-behalf-of-user" }`, + aiSearch: `{ indexes: { default: { auth: "on-behalf-of-user" } } }`, +}; /** The `createApp` call arguments for the plugin: the auth option, if any. */ -function pluginCallArgs( +export function pluginCallArgs( plugin: ManifestPlugin, resources: ManifestResource[], modes: Record, ): string { - if (!AUTH_CONFIG_PLUGINS.has(plugin.name)) return ""; + const oboArgs = OBO_CALL_ARGS[plugin.name]; + if (!oboArgs) return ""; const obo = resources.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); if (obo.length === 0) return ""; if (obo.length < resources.length) { console.warn( pc.yellow( - `${plugin.name} has one auth option for all its resources; set auth per volume in its config to mix on-behalf-of-user (${obo.map((r) => r.resourceKey).join(", ")}) with service principal access.`, + `${plugin.name} is registered with one auth setting for all its resources; set auth per volume or index in its config to mix on-behalf-of-user (${obo.map((r) => r.resourceKey).join(", ")}) with service principal access.`, ), ); return ""; } - return `{ auth: "on-behalf-of-user" }`; + return oboArgs; } async function registerInServer( From aafef48a242524cc812bb365123d76efc7025ba0 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 14:53:09 +0200 Subject: [PATCH 4/8] fix(appkit): plugin add writes aiSearch index columns so production starts aiSearch refuses to start outside development without columns. plugin add now discovers them like the plugin's dev mode (Delta Sync source table minus embedding columns), prompts when that fails, and without a terminal fails before writing anything. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/cli/commands/plugin/add/add.test.ts | 30 +++-- .../shared/src/cli/commands/plugin/add/add.ts | 127 ++++++++++++++---- .../cli/commands/plugin/add/resolve.test.ts | 20 ++- .../src/cli/commands/plugin/add/resolve.ts | 28 ++++ 4 files changed, 165 insertions(+), 40 deletions(-) diff --git a/packages/shared/src/cli/commands/plugin/add/add.test.ts b/packages/shared/src/cli/commands/plugin/add/add.test.ts index c3bac7389..d8c7e540a 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -1,6 +1,6 @@ import { expect, test } from "vitest"; -import { pluginCallArgs } from "./add"; +import { pluginConfig, toSource } from "./add"; import type { ManifestPlugin, ManifestResource } from "./plan"; const index: ManifestResource = { @@ -16,19 +16,29 @@ const plugin = (name: string): ManifestPlugin => ({ }); const obo = { "vector_search_index:vector-search-index": "obo" } as const; -test("aiSearch OBO sets auth on the default index, where the routes read it", () => { - expect(pluginCallArgs(plugin("aiSearch"), [index], obo)).toBe( - `{ indexes: { default: { auth: "on-behalf-of-user" } } }`, +test("aiSearch sets columns and OBO auth on the default index", () => { + const config = pluginConfig(plugin("aiSearch"), [index], obo, ["id", "t"]); + expect(toSource(config)).toBe( + `{ indexes: { default: { columns: ["id", "t"], auth: "on-behalf-of-user" } } }`, ); + expect(pluginConfig(plugin("aiSearch"), [index], {}, ["id"])).toEqual({ + indexes: { default: { columns: ["id"] } }, + }); }); test("files OBO sets the plugin-level auth", () => { - expect(pluginCallArgs(plugin("files"), [index], obo)).toBe( - `{ auth: "on-behalf-of-user" }`, - ); + expect(pluginConfig(plugin("files"), [index], obo)).toEqual({ + auth: "on-behalf-of-user", + }); +}); + +test("no config for sp or plugins without an auth option", () => { + expect(pluginConfig(plugin("files"), [index], {})).toBeUndefined(); + expect(pluginConfig(plugin("analytics"), [index], obo)).toBeUndefined(); }); -test("no args for sp/both or plugins without an auth option", () => { - expect(pluginCallArgs(plugin("aiSearch"), [index], {})).toBe(""); - expect(pluginCallArgs(plugin("analytics"), [index], obo)).toBe(""); +test("toSource escapes strings and quotes non-identifier keys", () => { + expect(toSource({ "a-b": ['x"); evil()'] })).toBe( + `{ "a-b": ["x\\"); evil()"] }`, + ); }); diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts index 781906fa1..3e0691699 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -32,7 +32,7 @@ import { resourceConfigured, selectResources, } from "./plan"; -import { resolveDerivedValues } from "./resolve"; +import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; const MANIFEST_FILE = "appkit.plugins.json"; const SERVER_SUBDIRS = ["server", "api", "backend"]; @@ -182,6 +182,51 @@ async function fillMissing( } } +/** + * Columns for the aiSearch default index: discovered from the workspace, else + * prompted. Without a terminal it fails before anything is written, since the + * app would not start in production without them. + */ +async function aiSearchColumns( + values: Record, + profile: string | undefined, + interactive: boolean, +): Promise { + const indexName = values["vector-search-index.id"]; + try { + const columns = await discoverIndexColumns(indexName, profile); + if (columns.length > 0) { + console.log(pc.dim(`Index columns: ${columns.join(", ")}`)); + return columns; + } + } catch (err) { + console.warn( + pc.yellow( + `Could not discover the columns of ${indexName}: ${(err as Error).message}`, + ), + ); + } + if (!interactive) { + throw new Error( + `aiSearch needs the columns of ${indexName} to start in production; run with workspace access (--profile) or interactively to enter them`, + ); + } + const { text } = await import("@clack/prompts"); + const answer = await answered( + await text({ + message: `Columns to return from ${indexName} (comma-separated)`, + validate: (v) => + v?.split(",").some((c) => c.trim()) + ? undefined + : "Enter at least one column", + }), + ); + return answer + .split(",") + .map((c) => c.trim()) + .filter(Boolean); +} + async function runPluginAdd(name: string, opts: AddOptions): Promise { const cwd = opts.cwd ? path.resolve(opts.cwd) : process.cwd(); if ( @@ -246,14 +291,19 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { await fillMissing(missingNow(), values, interactive); resources = selectResources(plugin, values); + const config = pluginConfig( + plugin, + resources, + modes, + plugin.name === "aiSearch" && opts.register !== false + ? await aiSearchColumns(values, profile, interactive) + : undefined, + ); + const plan = buildAddPlan(plugin, resources, values, modes); if (opts.register !== false) { - await registerInServer( - cwd, - plugin, - pluginCallArgs(plugin, resources, modes), - ); + await registerInServer(cwd, plugin, config ? toSource(config) : ""); } const { appendToFile, readEnvFile } = @@ -281,36 +331,55 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { } } +/** Plugins that default to the service principal unless told otherwise. */ +const AUTH_OPTION_PLUGINS = new Set(["files", "aiSearch"]); +const OBO = "on-behalf-of-user"; + /** - * `createApp` arguments that make a plugin access its resources on behalf of - * the user, for plugins that otherwise default to the service principal (which - * an OBO plan leaves unbound). files reads a plugin-level `auth`; aiSearch reads - * it per index, and its `default` index takes DATABRICKS_VS_INDEX_NAME. + * The `createApp` config for the plugin, or undefined for none. An OBO plan + * leaves the service principal unbound, so plugins that default to it must be + * told: files reads a plugin-level `auth`, aiSearch reads it per index. The + * aiSearch `default` index takes DATABRICKS_VS_INDEX_NAME, and production + * refuses to start without its `columns`. */ -const OBO_CALL_ARGS: Record = { - files: `{ auth: "on-behalf-of-user" }`, - aiSearch: `{ indexes: { default: { auth: "on-behalf-of-user" } } }`, -}; - -/** The `createApp` call arguments for the plugin: the auth option, if any. */ -export function pluginCallArgs( +export function pluginConfig( plugin: ManifestPlugin, resources: ManifestResource[], modes: Record, -): string { - const oboArgs = OBO_CALL_ARGS[plugin.name]; - if (!oboArgs) return ""; - const obo = resources.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); - if (obo.length === 0) return ""; - if (obo.length < resources.length) { - console.warn( - pc.yellow( - `${plugin.name} is registered with one auth setting for all its resources; set auth per volume or index in its config to mix on-behalf-of-user (${obo.map((r) => r.resourceKey).join(", ")}) with service principal access.`, - ), + columns?: string[], +): Record | undefined { + let obo = false; + if (AUTH_OPTION_PLUGINS.has(plugin.name)) { + const oboRes = resources.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); + obo = oboRes.length > 0 && oboRes.length === resources.length; + if (oboRes.length > 0 && !obo) { + console.warn( + pc.yellow( + `${plugin.name} is registered with one auth setting for all its resources; set auth per volume or index in its config to mix on-behalf-of-user (${oboRes.map((r) => r.resourceKey).join(", ")}) with service principal access.`, + ), + ); + } + } + if (plugin.name === "aiSearch") { + const index = { ...(columns && { columns }), ...(obo && { auth: OBO }) }; + return Object.keys(index).length > 0 + ? { indexes: { default: index } } + : undefined; + } + return obo ? { auth: OBO } : undefined; +} + +/** Source text for a config literal: `{ auth: "x", columns: ["a"] }`. */ +export function toSource(v: unknown): string { + if (Array.isArray(v)) return `[${v.map(toSource).join(", ")}]`; + if (v && typeof v === "object") { + const props = Object.entries(v).map( + ([k, x]) => + `${/^[A-Za-z_$][\w$]*$/.test(k) ? k : JSON.stringify(k)}: ${toSource(x)}`, ); - return ""; + return `{ ${props.join(", ")} }`; } - return oboArgs; + return JSON.stringify(v); } async function registerInServer( diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts index 7cb3b0d60..5338f681a 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts @@ -1,7 +1,7 @@ import { expect, test, vi } from "vitest"; import type { ManifestResource } from "./plan"; -import { resolveDerivedValues } from "./resolve"; +import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; const listEndpointsIter = vi.fn(async function* () { yield { name: "ro", status: { endpointType: "ENDPOINT_TYPE_READ_ONLY" } }; @@ -16,6 +16,20 @@ const listDatabasesIter = vi.fn(async function* () { vi.mock("../../registry/workspace-picker.js", () => ({ makeWorkspaceClient: () => ({ postgres: { listEndpointsIter, listDatabasesIter }, + vectorSearch: { + getVectorIndex: async () => ({ + indexSpec: { + $case: "deltaSyncIndexSpec", + deltaSyncIndexSpec: { + sourceTable: "c.s.t", + embeddingVectorColumns: [{ name: "vec" }], + }, + }, + }), + }, + tables: { + getTable: async () => ({ columns: [{ name: "id" }, { name: "vec" }] }), + }, }), })); @@ -61,3 +75,7 @@ test("skips the API when every derived field is set", async () => { ); expect(listEndpointsIter).not.toHaveBeenCalled(); }); + +test("discoverIndexColumns drops embedding columns", async () => { + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual(["id"]); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.ts b/packages/shared/src/cli/commands/plugin/add/resolve.ts index 94d4d69e2..9e326e2c2 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.ts @@ -47,3 +47,31 @@ export async function resolveDerivedValues( } return out; } + +/** + * Columns for an AI Search index, as the plugin's development-mode discovery + * finds them: the Delta Sync source table's columns minus embedding vectors. + * Production refuses to start without `columns`, so they are written into the + * plugin config at add time. Throws when the index has no source table. + */ +export async function discoverIndexColumns( + indexName: string, + profile: string | undefined, +): Promise { + const { makeWorkspaceClient } = + await import("../../registry/workspace-picker.js"); + const client = makeWorkspaceClient(profile); + const index = await client.vectorSearch.getVectorIndex({ name: indexName }); + const spec = + index.indexSpec?.$case === "deltaSyncIndexSpec" + ? index.indexSpec.deltaSyncIndexSpec + : undefined; + if (!spec?.sourceTable) { + throw new Error(`index ${indexName} has no Delta Sync source table`); + } + const excluded = new Set(spec.embeddingVectorColumns?.map((c) => c.name)); + const table = await client.tables.getTable({ fullNameArg: spec.sourceTable }); + return (table.columns ?? []).flatMap((c) => + c.name && !excluded.has(c.name) ? [c.name] : [], + ); +} From f4050589302661312262e5c6aef1eaa01acfc461 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 16:01:20 +0200 Subject: [PATCH 5/8] fix(appkit): plugin add agents OBO, .env placeholders, defaults, app key, packages, bindings - agents (like files) is registered with auth: on-behalf-of-user when its OBO-capable resources are OBO; resources that can't be OBO don't block it - empty .env placeholders are filled in place; a conflicting set value warns - manifest field defaults fill the bundle variable value - databricks.yml edits go to the bundle's only app, whatever its key - plugins from external npm packages are registered (package specifier) - resources without binding metadata use the built-in binding for the type Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/cli/commands/plugin/add/add.test.ts | 11 ++++ .../shared/src/cli/commands/plugin/add/add.ts | 52 ++++++++++++++----- .../src/cli/commands/plugin/add/plan.test.ts | 36 +++++++++++++ .../src/cli/commands/plugin/add/plan.ts | 39 ++++++++++---- .../commands/registry/config-writer.test.ts | 30 +++++++++++ .../cli/commands/registry/config-writer.ts | 18 ++++++- .../commands/registry/server-register.test.ts | 11 ++++ .../cli/commands/registry/server-register.ts | 5 +- 8 files changed, 174 insertions(+), 28 deletions(-) diff --git a/packages/shared/src/cli/commands/plugin/add/add.test.ts b/packages/shared/src/cli/commands/plugin/add/add.test.ts index d8c7e540a..d2e2144c2 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -42,3 +42,14 @@ test("toSource escapes strings and quotes non-identifier keys", () => { `{ "a-b": ["x\\"); evil()"] }`, ); }); + +test("agents OBO sets plugin auth; resources that can't be OBO don't block it", () => { + const experiment: ManifestResource = { + type: "experiment", + alias: "Experiment", + resourceKey: "agents-mlflow-experiment", + }; + expect(pluginConfig(plugin("agents"), [index, experiment], obo)).toEqual({ + auth: "on-behalf-of-user", + }); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts index 3e0691699..4cdcd25d0 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -310,16 +310,36 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { await import("../../registry/env-writer.js"); const envPath = path.join(cwd, ".env"); const existing = readEnvFile(envPath); - const newEnv = plan.dotEnv.filter( - (e, i) => - !(e.env in existing) && - plan.dotEnv.findIndex((x) => x.env === e.env) === i, + const dotEnv = plan.dotEnv.filter( + (e, i) => plan.dotEnv.findIndex((x) => x.env === e.env) === i, ); + // Empty placeholders (`KEY=`) are filled in place; set values are kept. + const filled = dotEnv.filter((e) => existing[e.env] === "" && e.value); + if (filled.length > 0) { + let content = fs.readFileSync(envPath, "utf-8"); + for (const e of filled) { + // e.env is a validated identifier, so it is safe in the pattern. + content = content.replace( + new RegExp(`^(\\s*(?:export\\s+)?${e.env}\\s*=).*$`, "m"), + (_, key: string) => key + e.value, + ); + } + fs.writeFileSync(envPath, content); + } + const newEnv = dotEnv.filter((e) => !(e.env in existing)); appendToFile(envPath, serializeEnvAppend(newEnv)); - if (newEnv.length > 0) { - console.log( - `${pc.green("Updated .env:")} ${newEnv.map((e) => e.env).join(", ")}`, - ); + const updated = [...filled, ...newEnv].map((e) => e.env); + if (updated.length > 0) { + console.log(`${pc.green("Updated .env:")} ${updated.join(", ")}`); + } + for (const e of dotEnv) { + if (existing[e.env] && e.value && existing[e.env] !== e.value) { + console.warn( + pc.yellow( + `.env already sets ${e.env}; kept it (the app.yaml and databricks.yml entries use the new value).`, + ), + ); + } } if (planHasContent(plan)) { @@ -332,13 +352,13 @@ async function runPluginAdd(name: string, opts: AddOptions): Promise { } /** Plugins that default to the service principal unless told otherwise. */ -const AUTH_OPTION_PLUGINS = new Set(["files", "aiSearch"]); +const AUTH_OPTION_PLUGINS = new Set(["files", "aiSearch", "agents"]); const OBO = "on-behalf-of-user"; /** * The `createApp` config for the plugin, or undefined for none. An OBO plan * leaves the service principal unbound, so plugins that default to it must be - * told: files reads a plugin-level `auth`, aiSearch reads it per index. The + * told: files and agents read a plugin-level `auth`, aiSearch reads it per index. The * aiSearch `default` index takes DATABRICKS_VS_INDEX_NAME, and production * refuses to start without its `columns`. */ @@ -350,8 +370,11 @@ export function pluginConfig( ): Record | undefined { let obo = false; if (AUTH_OPTION_PLUGINS.has(plugin.name)) { - const oboRes = resources.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); - obo = oboRes.length > 0 && oboRes.length === resources.length; + // Resources that can't be accessed on behalf of the user (e.g. an agents + // tracing experiment) stay on the service principal and don't count. + const capable = resources.filter(oboCapable); + const oboRes = capable.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); + obo = oboRes.length > 0 && oboRes.length === capable.length; if (oboRes.length > 0 && !obo) { console.warn( pc.yellow( @@ -394,8 +417,11 @@ async function registerInServer( // Lazy: server-register pulls in the @ast-grep/napi native addon. const { findServerFile, registerPluginInServer } = await import("../../registry/server-register.js"); + // The /beta subpath is AppKit's; other packages export their plugins as is. let importPath = - plugin.stability === "beta" ? `${plugin.package}/beta` : plugin.package; + plugin.stability === "beta" && plugin.package === "@databricks/appkit" + ? `${plugin.package}/beta` + : plugin.package; // Local plugins are recorded relative to the app root; imports resolve // relative to the server file. if (plugin.package.startsWith(".")) { diff --git a/packages/shared/src/cli/commands/plugin/add/plan.test.ts b/packages/shared/src/cli/commands/plugin/add/plan.test.ts index 5fb75a28c..e7be3da51 100644 --- a/packages/shared/src/cli/commands/plugin/add/plan.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/plan.test.ts @@ -213,3 +213,39 @@ test("fields with a resolver are required even when localOnly", () => { ), ).toEqual(["lakebase.postgres.host"]); }); + +test("a manifest default fills the bundle variable", () => { + const p: ManifestPlugin = { + ...plugin, + resources: { + required: [ + { + ...warehouse, + fields: { id: { env: "DATABRICKS_WAREHOUSE_ID", value: "dflt" } }, + }, + ], + optional: [], + }, + }; + const plan = buildAddPlan(p, p.resources.required, {}, {}); + expect(plan.bundleVariables[0]).toMatchObject({ + name: "sql_warehouse_id", + value: "dflt", + }); +}); + +test("a resource without binding metadata uses the built-in binding", () => { + const { binding: _, ...noBinding } = warehouse; + const plan = buildAddPlan( + plugin, + [noBinding], + { "sql-warehouse.id": "wh" }, + {}, + ); + expect(plan.unverifiedTypes).toEqual([]); + expect(plan.resourceBindings[0]).toMatchObject({ + name: "sql-warehouse", + type: "sql_warehouse", + fields: { id: "${var.sql_warehouse_id}" }, + }); +}); diff --git a/packages/shared/src/cli/commands/plugin/add/plan.ts b/packages/shared/src/cli/commands/plugin/add/plan.ts index e812edc99..8a5a7bf19 100644 --- a/packages/shared/src/cli/commands/plugin/add/plan.ts +++ b/packages/shared/src/cli/commands/plugin/add/plan.ts @@ -1,3 +1,4 @@ +import { DABS_BINDING_BY_TYPE } from "../../../../schemas/manifest"; import type { AppYamlEnvEntry, ResourceBinding } from "../../../deploy-config"; import type { BundleVariable, ConfigPlan } from "../../registry/config-plan"; import { isValidEnvName } from "../../registry/requirements"; @@ -39,11 +40,26 @@ export interface ManifestResource { fields?: Record; scope?: string; appOnly?: boolean; - binding?: { - yamlKey: string; - varFields: [string, string][]; - staticFields?: [string, string][]; - }; + binding?: Binding; +} + +type FieldPair = readonly [string, string]; +interface Binding { + yamlKey: string; + varFields: readonly FieldPair[]; + staticFields?: readonly FieldPair[]; +} + +/** + * The resource's databricks.yml binding: the manifest's, else the built-in + * one for its type (`binding` is optional in the manifest schema; the + * Databricks CLI generator falls back the same way). + */ +function bindingOf(r: ManifestResource): Binding | undefined { + return ( + r.binding ?? + (DABS_BINDING_BY_TYPE as Record)[r.type] + ); } /** A plugin entry of the app's appkit.plugins.json (the fields used here). */ @@ -75,7 +91,7 @@ function allResources(p: ManifestPlugin): ManifestResource[] { /** Manifest fields the resource's databricks.yml binding references. */ function bindingVarFields(r: ManifestResource): string[] { - return (r.binding?.varFields ?? []).map(([f]) => f); + return (bindingOf(r)?.varFields ?? []).map(([f]) => f); } /** @@ -385,7 +401,8 @@ export function buildAddPlan( } if (oboOnly) continue; - if (!r.binding) { + const binding = bindingOf(r); + if (!binding) { unverifiedTypes.push(r.type); continue; } @@ -393,17 +410,17 @@ export function buildAddPlan( bundleVariables.push({ name: varName(r, f), description: r.fields?.[f]?.description || r.description, - value: values[valueKey(r, f)] || undefined, + value: values[valueKey(r, f)] || r.fields?.[f]?.value || undefined, }); } const fields: Record = {}; - for (const [mf, yf] of r.binding.varFields) { + for (const [mf, yf] of binding.varFields) { fields[yf] = `\${var.${varName(r, mf)}}`; } - for (const [yf, v] of r.binding.staticFields ?? []) fields[yf] = v; + for (const [yf, v] of binding.staticFields ?? []) fields[yf] = v; resourceBindings.push({ name: r.resourceKey, - type: r.binding.yamlKey, + type: binding.yamlKey, permission: r.permission, fields, }); diff --git a/packages/shared/src/cli/commands/registry/config-writer.test.ts b/packages/shared/src/cli/commands/registry/config-writer.test.ts index 53c5152d7..51a13cb7c 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.test.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.test.ts @@ -224,3 +224,33 @@ describe("writeConfig — user_api_scopes and literal env values", () => { expect(app.env).toEqual([{ name: "DATABRICKS_WAREHOUSE_ID", value: "wh" }]); }); }); + +describe("writeConfig — app key", () => { + it("writes scopes and bindings to the bundle's only app, whatever its key", () => { + const cwd = makeTempDir(); + fs.writeFileSync( + path.join(cwd, "databricks.yml"), + "resources:\n apps:\n dashboard:\n name: dash\n", + ); + writeConfig(cwd, { + appYamlEnv: [], + bundleVariables: [], + resourceBindings: [ + { + name: "wh", + type: "sql_warehouse", + permission: "CAN_USE", + fields: {}, + }, + ], + userApiScopes: ["sql"], + unverifiedTypes: [], + }); + const apps = parseDocument( + fs.readFileSync(path.join(cwd, "databricks.yml"), "utf-8"), + ).toJSON().resources.apps; + expect(Object.keys(apps)).toEqual(["dashboard"]); + expect(apps.dashboard.user_api_scopes).toEqual(["sql"]); + expect(apps.dashboard.resources[0].name).toBe("wh"); + }); +}); diff --git a/packages/shared/src/cli/commands/registry/config-writer.ts b/packages/shared/src/cli/commands/registry/config-writer.ts index 065d15f8d..d2cd1e9da 100644 --- a/packages/shared/src/cli/commands/registry/config-writer.ts +++ b/packages/shared/src/cli/commands/registry/config-writer.ts @@ -79,6 +79,20 @@ function ensureMap( return node; } +/** + * Key of the bundle's app under `resources.apps`: the only one there (apps + * are often named after the project, not `app`), else `app`. Throws when + * several apps exist and none is `app`, rather than guess or add a new one. + */ +function appKey(doc: ReturnType): string { + const keys = Object.keys(doc.toJSON()?.resources?.apps ?? {}); + if (keys.length === 1) return keys[0]; + if (keys.length === 0 || keys.includes("app")) return "app"; + throw new Error( + `databricks.yml defines several apps (${keys.join(", ")}); add the resources to the right one by hand`, + ); +} + /** * Additively patches `databricks.yml`: adds bundle `variables`, the app * `resources` bindings, and the target-level variable values — each only if @@ -113,7 +127,7 @@ function patchDatabricksYml( // App resource bindings. if (plan.resourceBindings.length > 0) { - const app = ensureMap(doc, ["resources", "apps", "app"]); + const app = ensureMap(doc, ["resources", "apps", appKey(doc)]); let bindings = app.get("resources") as YAMLSeq | undefined; if (!bindings || typeof (bindings as YAMLSeq).add !== "function") { app.set("resources", doc.createNode([])); @@ -132,7 +146,7 @@ function patchDatabricksYml( } if (plan.userApiScopes?.length) { - const app = ensureMap(doc, ["resources", "apps", "app"]); + const app = ensureMap(doc, ["resources", "apps", appKey(doc)]); let scopes = app.get("user_api_scopes") as YAMLSeq | undefined; if (!scopes || typeof (scopes as YAMLSeq).add !== "function") { app.set("user_api_scopes", doc.createNode([])); diff --git a/packages/shared/src/cli/commands/registry/server-register.test.ts b/packages/shared/src/cli/commands/registry/server-register.test.ts index fb96744ad..34d089a22 100644 --- a/packages/shared/src/cli/commands/registry/server-register.test.ts +++ b/packages/shared/src/cli/commands/registry/server-register.test.ts @@ -38,6 +38,17 @@ afterEach(() => { }); describe("registerPluginInServer", () => { + it("accepts an external npm package and rejects code in the path", () => { + const dir = makeTempDir(); + writeServer(dir, "server.ts", "server()"); + expect( + registerPluginInServer(dir, "@example/analytics", "analytics").status, + ).toBe("wired"); + expect(registerPluginInServer(dir, `x"; evil(); "`, "other").status).toBe( + "skipped", + ); + }); + it("passes call args and accepts the AppKit package import path", () => { const dir = makeTempDir(); const file = writeServer(dir, "server.ts", "server()"); diff --git a/packages/shared/src/cli/commands/registry/server-register.ts b/packages/shared/src/cli/commands/registry/server-register.ts index 96442541c..26b10eb33 100644 --- a/packages/shared/src/cli/commands/registry/server-register.ts +++ b/packages/shared/src/cli/commands/registry/server-register.ts @@ -93,13 +93,14 @@ export function registerPluginInServer( // exportName and importPath are interpolated into the user's server source. // Registry items are untrusted, so refuse anything that isn't a plain JS // identifier / clean relative module path — prevents code injection via a - // crafted export name or import path. Core plugins import from the AppKit package. + // crafted export name or import path. Plugins declared by an npm package + // (AppKit's or another) import from a plain package specifier. if (!JS_IDENTIFIER.test(exportName)) { return { status: "skipped", reason: "invalid plugin export name" }; } if ( !/^[.][./A-Za-z0-9_-]*$/.test(importPath) && - !/^@databricks\/appkit(\/beta)?$/.test(importPath) + !/^(@[a-z0-9][\w.-]*\/)?[a-z0-9][\w.-]*(\/[\w.-]+)*$/i.test(importPath) ) { return { status: "skipped", reason: "invalid plugin import path" }; } From ee2aba3564de7e23a938d8cf6147ccd8b46c7bb4 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 16:11:03 +0200 Subject: [PATCH 6/8] fix(appkit): plugin add agents OBO follows the model endpoint only The skills volume is read by the service principal at boot whatever the agent auth is, so an SP skills volume no longer keeps an OBO model on the service principal. An OBO skills volume now warns that it needs sp/both. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/cli/commands/plugin/add/add.test.ts | 38 ++++++++++++++++++- .../shared/src/cli/commands/plugin/add/add.ts | 27 ++++++++++--- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/packages/shared/src/cli/commands/plugin/add/add.test.ts b/packages/shared/src/cli/commands/plugin/add/add.test.ts index d2e2144c2..59d27286f 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.test.ts @@ -49,7 +49,43 @@ test("agents OBO sets plugin auth; resources that can't be OBO don't block it", alias: "Experiment", resourceKey: "agents-mlflow-experiment", }; - expect(pluginConfig(plugin("agents"), [index, experiment], obo)).toEqual({ + const endpoint: ManifestResource = { + type: "serving_endpoint", + alias: "LLM", + resourceKey: "agents-serving-endpoint", + scope: "model-serving", + }; + expect( + pluginConfig(plugin("agents"), [endpoint, experiment], { + "serving_endpoint:agents-serving-endpoint": "obo", + }), + ).toEqual({ + auth: "on-behalf-of-user", + }); +}); + +test("agents auth follows the model endpoint, not the skills volume", () => { + const endpoint: ManifestResource = { + type: "serving_endpoint", + alias: "LLM", + resourceKey: "agents-serving-endpoint", + scope: "model-serving", + }; + const skills: ManifestResource = { + type: "volume", + alias: "Skills", + resourceKey: "agents-skills", + scope: "files", + }; + const modelObo = { + "serving_endpoint:agents-serving-endpoint": "obo", + } as const; + expect(pluginConfig(plugin("agents"), [endpoint, skills], modelObo)).toEqual({ auth: "on-behalf-of-user", }); + expect( + pluginConfig(plugin("agents"), [endpoint, skills], { + "volume:agents-skills": "obo", + }), + ).toBeUndefined(); }); diff --git a/packages/shared/src/cli/commands/plugin/add/add.ts b/packages/shared/src/cli/commands/plugin/add/add.ts index 4cdcd25d0..a7b44b302 100644 --- a/packages/shared/src/cli/commands/plugin/add/add.ts +++ b/packages/shared/src/cli/commands/plugin/add/add.ts @@ -370,11 +370,28 @@ export function pluginConfig( ): Record | undefined { let obo = false; if (AUTH_OPTION_PLUGINS.has(plugin.name)) { - // Resources that can't be accessed on behalf of the user (e.g. an agents - // tracing experiment) stay on the service principal and don't count. - const capable = resources.filter(oboCapable); - const oboRes = capable.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); - obo = oboRes.length > 0 && oboRes.length === capable.length; + // The resources whose access the option governs. For agents that is the + // model endpoint: skills load from their volume as the service principal + // at boot whatever `auth` is, and the tracing experiment can't be OBO. + // Elsewhere, every resource that can be accessed on behalf of the user. + const governed = resources.filter( + (r) => + oboCapable(r) && + (plugin.name !== "agents" || r.type === "serving_endpoint"), + ); + const oboRes = governed.filter((r) => modes[authKey(r)] === AUTH_MODE_OBO); + obo = oboRes.length > 0 && oboRes.length === governed.length; + if (plugin.name === "agents") { + for (const r of resources) { + if (r.type === "volume" && modes[authKey(r)] === AUTH_MODE_OBO) { + console.warn( + pc.yellow( + `agents loads skills from ${r.resourceKey} as the service principal at boot, which gets no access in obo mode; use --set agents.${r.resourceKey}.authMode=sp (or both).`, + ), + ); + } + } + } if (oboRes.length > 0 && !obo) { console.warn( pc.yellow( From b28a02a40980b8ea587f792c26b904fd4e08ed84 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 16:50:31 +0200 Subject: [PATCH 7/8] fix(appkit): plugin add limits aiSearch columns to the index's synced columns Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../cli/commands/plugin/add/resolve.test.ts | 26 +++++++++++++++++-- .../src/cli/commands/plugin/add/resolve.ts | 12 ++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts index 5338f681a..936862b03 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts @@ -3,6 +3,7 @@ import { expect, test, vi } from "vitest"; import type { ManifestResource } from "./plan"; import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; +let columnsToSync: string[] | undefined; const listEndpointsIter = vi.fn(async function* () { yield { name: "ro", status: { endpointType: "ENDPOINT_TYPE_READ_ONLY" } }; yield { @@ -23,12 +24,20 @@ vi.mock("../../registry/workspace-picker.js", () => ({ deltaSyncIndexSpec: { sourceTable: "c.s.t", embeddingVectorColumns: [{ name: "vec" }], + columnsToSync, }, }, }), }, tables: { - getTable: async () => ({ columns: [{ name: "id" }, { name: "vec" }] }), + getTable: async () => ({ + columns: [ + { name: "id" }, + { name: "text" }, + { name: "vec" }, + { name: "notes" }, + ], + }), }, }), })); @@ -77,5 +86,18 @@ test("skips the API when every derived field is set", async () => { }); test("discoverIndexColumns drops embedding columns", async () => { - expect(await discoverIndexColumns("c.s.i", undefined)).toEqual(["id"]); + columnsToSync = undefined; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + "notes", + ]); +}); + +test("discoverIndexColumns keeps only synced columns", async () => { + columnsToSync = ["id", "text", "vec"]; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + ]); }); diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.ts b/packages/shared/src/cli/commands/plugin/add/resolve.ts index 9e326e2c2..e27a14ef4 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.ts @@ -49,8 +49,9 @@ export async function resolveDerivedValues( } /** - * Columns for an AI Search index, as the plugin's development-mode discovery - * finds them: the Delta Sync source table's columns minus embedding vectors. + * Columns for an AI Search index: the Delta Sync source table's columns minus + * embedding vectors, limited to `columnsToSync` when the index syncs a subset + * (other source columns aren't in the index, and querying them fails). * Production refuses to start without `columns`, so they are written into the * plugin config at add time. Throws when the index has no source table. */ @@ -71,7 +72,12 @@ export async function discoverIndexColumns( } const excluded = new Set(spec.embeddingVectorColumns?.map((c) => c.name)); const table = await client.tables.getTable({ fullNameArg: spec.sourceTable }); + const synced = spec.columnsToSync?.length + ? new Set(spec.columnsToSync) + : undefined; return (table.columns ?? []).flatMap((c) => - c.name && !excluded.has(c.name) ? [c.name] : [], + c.name && !excluded.has(c.name) && (!synced || synced.has(c.name)) + ? [c.name] + : [], ); } From 2959afcac3988d6215926226ae2296ae814b76ec Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Tue, 20 Oct 2026 16:56:52 +0200 Subject: [PATCH 8/8] fix(appkit): plugin add honors the columnsToIndex alias for aiSearch columns Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .../src/cli/commands/plugin/add/resolve.test.ts | 12 ++++++++++++ .../shared/src/cli/commands/plugin/add/resolve.ts | 10 ++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts index 936862b03..0e756aadc 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.test.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.test.ts @@ -4,6 +4,7 @@ import type { ManifestResource } from "./plan"; import { discoverIndexColumns, resolveDerivedValues } from "./resolve"; let columnsToSync: string[] | undefined; +let columnsToIndex: string[] | undefined; const listEndpointsIter = vi.fn(async function* () { yield { name: "ro", status: { endpointType: "ENDPOINT_TYPE_READ_ONLY" } }; yield { @@ -25,6 +26,7 @@ vi.mock("../../registry/workspace-picker.js", () => ({ sourceTable: "c.s.t", embeddingVectorColumns: [{ name: "vec" }], columnsToSync, + columnsToIndex, }, }, }), @@ -101,3 +103,13 @@ test("discoverIndexColumns keeps only synced columns", async () => { "text", ]); }); + +test("discoverIndexColumns honors the columnsToIndex alias", async () => { + columnsToSync = undefined; + columnsToIndex = ["id", "text", "vec"]; + expect(await discoverIndexColumns("c.s.i", undefined)).toEqual([ + "id", + "text", + ]); + columnsToIndex = undefined; +}); diff --git a/packages/shared/src/cli/commands/plugin/add/resolve.ts b/packages/shared/src/cli/commands/plugin/add/resolve.ts index e27a14ef4..58d47da7b 100644 --- a/packages/shared/src/cli/commands/plugin/add/resolve.ts +++ b/packages/shared/src/cli/commands/plugin/add/resolve.ts @@ -50,7 +50,7 @@ export async function resolveDerivedValues( /** * Columns for an AI Search index: the Delta Sync source table's columns minus - * embedding vectors, limited to `columnsToSync` when the index syncs a subset + * embedding vectors, limited to `columnsToSync` (or its alias `columnsToIndex`) when the index syncs a subset * (other source columns aren't in the index, and querying them fails). * Production refuses to start without `columns`, so they are written into the * plugin config at add time. Throws when the index has no source table. @@ -72,9 +72,11 @@ export async function discoverIndexColumns( } const excluded = new Set(spec.embeddingVectorColumns?.map((c) => c.name)); const table = await client.tables.getTable({ fullNameArg: spec.sourceTable }); - const synced = spec.columnsToSync?.length - ? new Set(spec.columnsToSync) - : undefined; + // columnsToIndex is the response's alias of columnsToSync. + const subset = spec.columnsToSync?.length + ? spec.columnsToSync + : spec.columnsToIndex; + const synced = subset?.length ? new Set(subset) : undefined; return (table.columns ?? []).flatMap((c) => c.name && !excluded.has(c.name) && (!synced || synced.has(c.name)) ? [c.name]