From c7636c53e8a409c095c2bd6dceec16faa5167a91 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 22 Jul 2026 11:31:00 +0200 Subject: [PATCH 1/2] fix(appkit): report only the actually-missing env vars in resource errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResourceRegistry.formatMissingResources and formatDevWarningBanner printed every field's env var for a missing resource via Object.values(entry.fields).map(f => f.env) — including vars already set and env-less discovery-only fields (which mapped to undefined, rendering as stray empty entries like 'set , , , PGHOST, ...'). With a multi-field resource such as Lakebase Postgres, a single unset var (e.g. PGPORT) produced a confusing message listing all seven fields. Add a private unsetEnvVars() helper that returns only the env vars that are actually unset/empty, skipping fields with no env, mirroring validate()'s own resolution check. Both formatters now list exactly what the caller must set. Add tests: only-unset-reported, and env-less fields omitted (a mutation reverting the fix fails both). Signed-off-by: MarioCadenas --- .../appkit/src/registry/resource-registry.ts | 29 ++++++-- .../registry/tests/resource-registry.test.ts | 71 +++++++++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) diff --git a/packages/appkit/src/registry/resource-registry.ts b/packages/appkit/src/registry/resource-registry.ts index fd8c7dfce..87397c2e4 100644 --- a/packages/appkit/src/registry/resource-registry.ts +++ b/packages/appkit/src/registry/resource-registry.ts @@ -416,14 +416,33 @@ export class ResourceRegistry { * @param missing - Array of missing resource entries * @returns Formatted error message string */ + /** + * Env var names for an entry's fields that are actually unset (undefined or + * empty), skipping fields that declare no env var (e.g. discovery-only + * fields resolved at deploy time). Mirrors the resolution check in + * {@link validate} so error output lists exactly what the caller must set — + * not every field on the resource. + */ + private static unsetEnvVars(entry: ResourceEntry): string[] { + const unset: string[] = []; + for (const field of Object.values(entry.fields)) { + if (!field.env) continue; + const val = process.env[field.env]; + if (val === undefined || val === "") { + unset.push(field.env); + } + } + return unset; + } + public static formatMissingResources(missing: ResourceEntry[]): string { if (missing.length === 0) { return "No missing resources"; } const lines = missing.map((entry) => { - const envVars = Object.values(entry.fields).map((f) => f.env); - const envHint = ` (set ${envVars.join(", ")})`; + const envVars = ResourceRegistry.unsetEnvVars(entry); + const envHint = envVars.length > 0 ? ` (set ${envVars.join(", ")})` : ""; return ` - ${entry.type}:${entry.alias} [${entry.plugin}]${envHint}`; }); @@ -444,11 +463,13 @@ export class ResourceRegistry { ]; for (const entry of missing) { - const envVars = Object.values(entry.fields).map((f) => f.env); + const envVars = ResourceRegistry.unsetEnvVars(entry); contentLines.push( ` ${entry.type}:${entry.alias} (plugin: ${entry.plugin})`, ); - contentLines.push(` Set: ${envVars.join(", ")}`); + if (envVars.length > 0) { + contentLines.push(` Set: ${envVars.join(", ")}`); + } } contentLines.push(""); diff --git a/packages/appkit/src/registry/tests/resource-registry.test.ts b/packages/appkit/src/registry/tests/resource-registry.test.ts index 7d5598d56..a7a1dfd4c 100644 --- a/packages/appkit/src/registry/tests/resource-registry.test.ts +++ b/packages/appkit/src/registry/tests/resource-registry.test.ts @@ -528,6 +528,77 @@ describe("ResourceRegistry", () => { else delete process.env.DATABRICKS_WAREHOUSE_ID; } }); + + it("should list only the unset env vars, not the ones already set", () => { + const prevScope = process.env.SECRET_SCOPE; + const prevKey = process.env.SECRET_KEY; + process.env.SECRET_SCOPE = "my-scope"; + delete process.env.SECRET_KEY; + try { + const registry = new ResourceRegistry(); + registry.register("analytics", { + type: ResourceType.SECRET, + alias: "creds", + resourceKey: "creds", + description: "Credentials", + permission: "READ", + required: true, + fields: { + scope: { env: "SECRET_SCOPE" }, + key: { env: "SECRET_KEY" }, + }, + }); + + const result = registry.validate(); + expect(result.valid).toBe(false); + + const formatted = ResourceRegistry.formatMissingResources( + result.missing, + ); + // Only the genuinely-missing var is reported; the one already set is not. + expect(formatted).toContain("SECRET_KEY"); + expect(formatted).not.toContain("SECRET_SCOPE"); + } finally { + if (prevScope !== undefined) process.env.SECRET_SCOPE = prevScope; + else delete process.env.SECRET_SCOPE; + if (prevKey !== undefined) process.env.SECRET_KEY = prevKey; + else delete process.env.SECRET_KEY; + } + }); + + it("should omit fields that declare no env var (e.g. discovery-only fields)", () => { + const prevHost = process.env.PGHOST_TEST; + delete process.env.PGHOST_TEST; + try { + const registry = new ResourceRegistry(); + registry.register("lakebase", { + type: ResourceType.POSTGRES, + alias: "Postgres", + resourceKey: "postgres", + description: "Lakebase Postgres", + permission: "CAN_CONNECT_AND_CREATE", + required: true, + fields: { + // Discovery-only field: no env var, resolved at deploy time. + project: { description: "Project resource name" }, + host: { env: "PGHOST_TEST" }, + }, + }); + + const result = registry.validate(); + const formatted = ResourceRegistry.formatMissingResources( + result.missing, + ); + // The one real missing env var is present… + expect(formatted).toContain("PGHOST_TEST"); + // …and there are no stray empty entries from the env-less field. + expect(formatted).not.toContain("(set , "); + expect(formatted).not.toContain(", )"); + } finally { + if (prevHost !== undefined) process.env.PGHOST_TEST = prevHost; + else delete process.env.PGHOST_TEST; + } + }); }); describe("collectResources with getResourceRequirements", () => { From bb6fe65faef95a0c21707fe3b73f297634bc08fe Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 22 Jul 2026 11:50:26 +0200 Subject: [PATCH 2/2] docs(appkit): align resource-registry docs/context with only-unset reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only-missing-env-var fix left three spots still using the old Object.values(fields).map(f => f.env) pattern: - validate() JSDoc @example printed every field's env var (incl. undefined for env-less fields) — now uses formatMissingResources(). - enforceValidation()'s ConfigurationError context built envVars the old way, so the structured error listed all fields while the message listed only unset ones — now uses unsetEnvVars(). - unsetEnvVars was inserted between formatMissingResources's doc comment and the method, orphaning the doc; reordered so each method keeps its own. Regenerated Class.ResourceRegistry.md. Signed-off-by: MarioCadenas --- docs/docs/api/appkit/Class.ResourceRegistry.md | 2 +- .../appkit/src/registry/resource-registry.ts | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/docs/api/appkit/Class.ResourceRegistry.md b/docs/docs/api/appkit/Class.ResourceRegistry.md index e0291fb68..4a10e838a 100644 --- a/docs/docs/api/appkit/Class.ResourceRegistry.md +++ b/docs/docs/api/appkit/Class.ResourceRegistry.md @@ -244,7 +244,7 @@ const registry = ResourceRegistry.getInstance(); const result = registry.validate(); if (!result.valid) { - console.error("Missing resources:", result.missing.map(r => Object.values(r.fields).map(f => f.env))); + console.error(ResourceRegistry.formatMissingResources(result.missing)); } ``` diff --git a/packages/appkit/src/registry/resource-registry.ts b/packages/appkit/src/registry/resource-registry.ts index 87397c2e4..7b2025b21 100644 --- a/packages/appkit/src/registry/resource-registry.ts +++ b/packages/appkit/src/registry/resource-registry.ts @@ -304,7 +304,7 @@ export class ResourceRegistry { * const result = registry.validate(); * * if (!result.valid) { - * console.error("Missing resources:", result.missing.map(r => Object.values(r.fields).map(f => f.env))); + * console.error(ResourceRegistry.formatMissingResources(result.missing)); * } * ``` */ @@ -392,7 +392,7 @@ export class ResourceRegistry { type: r.type, alias: r.alias, plugin: r.plugin, - envVars: Object.values(r.fields).map((f) => f.env), + envVars: ResourceRegistry.unsetEnvVars(r), })), }, }); @@ -410,12 +410,6 @@ export class ResourceRegistry { return validation; } - /** - * Formats missing resources into a human-readable error message. - * - * @param missing - Array of missing resource entries - * @returns Formatted error message string - */ /** * Env var names for an entry's fields that are actually unset (undefined or * empty), skipping fields that declare no env var (e.g. discovery-only @@ -435,6 +429,12 @@ export class ResourceRegistry { return unset; } + /** + * Formats missing resources into a human-readable error message. + * + * @param missing - Array of missing resource entries + * @returns Formatted error message string + */ public static formatMissingResources(missing: ResourceEntry[]): string { if (missing.length === 0) { return "No missing resources";