1pass plugin improvements

.changeset/big-swans-own.md

@@ -0,0 +1,7 @@
+---
+"@dmno/encrypted-vault-plugin": patch
+"@dmno/1password-plugin": patch
+"dmno": patch
+---
+
+1password plugin improvements, related refactoring

example-repo/.dmno/config.mts

@@ -7,12 +7,13 @@ import { EncryptedVaultDmnoPlugin, EncryptedVaultTypes } from '@dmno/encrypted-v
 const OnePassSecretsProd = new OnePasswordDmnoPlugin('1pass/prod', {
   token: configPath('OP_TOKEN'),
   envItemLink: 'https://start.1password.com/open/i?a=I3GUA2KU6BD3FBHA47QNBIVEV4&v=ut2dftalm3ugmxc6klavms6tfq&i=n4wmgfq77mydg5lebtroa3ykvm&h=dmnoinc.1password.com',
+
   // token: InjectPluginInputByType,
   // token: 'asdf',
 });
 const OnePassSecretsDev = new OnePasswordDmnoPlugin('1pass', {
   token: configPath('OP_TOKEN'),
-  envItemLink: 'https://start.1password.com/open/i?a=I3GUA2KU6BD3FBHA47QNBIVEV4&v=ut2dftalm3ugmxc6klavms6tfq&i=n4wmgfq77mydg5lebtroa3ykvm&h=dmnoinc.1password.com',
+  envItemLink: 'https://start.1password.com/open/i?a=I3GUA2KU6BD3FBHA47QNBIVEV4&v=ut2dftalm3ugmxc6klavms6tfq&i=4u4klfhpldobgdxrcjwb2bqsta&h=dmnoinc.1password.com',
   // token: InjectPluginInputByType,
   // token: 'asdf',
 });
@@ -47,8 +48,21 @@ export default defineDmnoService({
     OP_TOKEN: {
       extends: OnePasswordTypes.serviceAccountToken,
     },
-    OP_TOKEN_PROD: {
-      extends: OnePasswordTypes.serviceAccountToken,
+    // OP_TOKEN_PROD: {
+    //   extends: OnePasswordTypes.serviceAccountToken,
+    // },
+
+    OP_ITEM_1: {
+      value: OnePassSecretsDev.item(),
+    },
+    OP_ITEM_BY_ID: {
+      value: OnePassSecretsDev.itemById("ut2dftalm3ugmxc6klavms6tfq", "bphvvrqjegfmd5yoz4buw2aequ", "username"),
+    },
+    OP_ITEM_BY_LINK: {
+      value: OnePassSecretsDev.itemByLink("https://start.1password.com/open/i?a=I3GUA2KU6BD3FBHA47QNBIVEV4&v=ut2dftalm3ugmxc6klavms6tfq&i=bphvvrqjegfmd5yoz4buw2aequXXX&h=dmnoinc.1password.com", "helturjryuy73yjbnaovlce5fu"),
+    },
+    OP_ITEM_BY_REFERENCE: {
+      value: OnePassSecretsDev.itemByReference("op://dev test/example/username"),
     },
 
     SOME_API_KEY: {

example-repo/packages/astro-web/.dmno/config.mts

@@ -16,6 +16,7 @@ export default defineDmnoService({
       source: 'api',
       key: 'API_URL',
     },
+    'SOME_API_KEY',
   ],
   schema: {
     OP_TOKEN: { extends: OnePasswordTypes.serviceAccountToken },

example-repo/packages/astro-web/astro.config.ts

@@ -14,8 +14,8 @@ console.log('> secret value =', DMNO_CONFIG.SECRET_FOO);
 console.log('> secret value in obj', { secret: DMNO_CONFIG.SECRET_FOO });
 console.log('> secret value in array', ['secret', DMNO_CONFIG.SECRET_FOO ]);
 
-console.log('\nthe secret on the next line should not');
-console.log('> secret value =', unredact(DMNO_CONFIG.SECRET_FOO));
+// console.log('\nthe secret on the next line should not');
+// console.log('> secret value =', unredact(DMNO_CONFIG.SECRET_FOO));
 
 // https://astro.build/config
 export default defineConfig({

packages/core/src/config-engine/config-engine.ts

@@ -269,7 +269,7 @@ class CacheEntry {
     this.encryptedValue = more?.encryptedValue;
   }
   async getEncryptedValue() {
-    return encrypt(CacheEntry.encryptionKey, this.value, CacheEntry.encryptionKeyName);
+    return encrypt(CacheEntry.encryptionKey, JSON.stringify(this.value), CacheEntry.encryptionKeyName);
   }
   // have to make this async because of the encryption call
   async getJSON(): Promise<SerializedCacheEntry> {
@@ -283,9 +283,9 @@ class CacheEntry {
   static async fromSerialized(itemKey: string, raw: SerializedCacheEntry) {
     // currently this setup means the encryptedValue changes on every run...
     // we could instead store the encryptedValue and reuse it if it has not changed
-    const value = await decrypt(CacheEntry.encryptionKey, raw.encryptedValue, CacheEntry.encryptionKeyName);
+    const valueStr = await decrypt(CacheEntry.encryptionKey, raw.encryptedValue, CacheEntry.encryptionKeyName);
     // we are also tossing out the saved "usedBy" entries since we'll have new ones after this config run
-    return new CacheEntry(itemKey, value, {
+    return new CacheEntry(itemKey, JSON.parse(valueStr), {
       updatedAt: new Date(raw.updatedAt),
       encryptedValue: raw.encryptedValue,
     });
@@ -628,7 +628,7 @@ export class DmnoWorkspace {
       return this.valueCache[key].value;
     }
   }
-  async setCacheItem(key: string, value: string, usedBy?: string) {
+  async setCacheItem(key: string, value: any, usedBy?: string) {
     if (this.cacheMode === 'skip') return undefined;
     this.valueCache[key] = new CacheEntry(key, value, { usedBy });
   }
@@ -952,9 +952,9 @@ export class ResolverContext {
   async setCacheItem(key: string, value: ConfigValue) {
     if (process.env.DISABLE_DMNO_CACHE) return;
     if (value === undefined || value === null) return;
-    return this.service?.workspace.setCacheItem(key, value.toString(), this.itemFullPath);
+    return this.service?.workspace.setCacheItem(key, value, this.itemFullPath);
   }
-  async getOrSetCacheItem(key: string, getValToWrite: () => Promise<string>) {
+  async getOrSetCacheItem(key: string, getValToWrite: () => Promise<ConfigValue>) {
     if (!process.env.DISABLE_DMNO_CACHE) {
       const cachedValue = await this.getCacheItem(key);
       if (cachedValue) return cachedValue;
@@ -991,6 +991,8 @@ export abstract class DmnoConfigItemBase {
   /** error encountered during resolution */
   get resolutionError(): ResolutionError | undefined {
     return this.valueResolver?.resolutionError;
+    // TODO: look at resolver child branches
+      // || this.valueResolver?.def
   }
 
   /** resolved value _after_ coercion logic applied */

packages/core/src/config-engine/errors.ts

@@ -39,27 +39,36 @@ export class DmnoError extends Error {
 
   icon = '❌';
 
-  constructor(err: string | Error, readonly more?: {
-    tip?: string,
+  constructor(errOrMessage: string | Error, readonly more?: {
+    tip?: string | Array<string>,
+    err?: Error,
   }) {
-    if (_.isError(err)) {
-      super(err.message);
-      this.originalError = err;
+    if (_.isError(errOrMessage)) {
+      super(errOrMessage.message);
+      this.originalError = errOrMessage;
       this.icon = '💥';
-    } else {
-      super(err);
+    } else { // string
+      super(errOrMessage);
+      this.originalError = more?.err;
     }
+    if (Array.isArray(more?.tip)) more.tip = more.tip.join('\n');
     this.name = this.constructor.name;
   }
 
+  get tip() {
+    if (!this.more?.tip) return undefined;
+    if (Array.isArray(this.more.tip)) return this.more.tip.join('\n');
+    return this.more.tip;
+  }
+
   toJSON() {
     return {
       icon: this.icon,
       type: this.type,
       name: this.name,
       message: this.message,
       isUnexpected: this.isUnexpected,
-      ...this.more,
+      ...this.tip && { tip: this.tip },
     };
   }
 }

packages/core/src/config-engine/plugins.ts

@@ -216,16 +216,24 @@ export abstract class DmnoPlugin<
     initializedPluginInstanceNames.push(instanceName);
 
     // const callStack = new Error('').stack!.split('\n');
+    // console.log(callStack);
     // const pluginDefinitionPath = callStack[2]
     //   .replace(/.*\(/, '')
     //   .replace(/:.*\)/, '');
     // // special case for local dev when we have the plugins symlinked by pnpm
-    // if (pluginDefinitionPath.includes('/core/packages/plugins/')) {
-    //   const pluginPackageName
-    // } else {
+    // // if (pluginDefinitionPath.includes('/core/packages/plugins/')) {
+    // //   const pluginPackageName
+    // // } else {
 
-    // }
+    // // }
     // console.log(pluginDefinitionPath);
+
+    // ideally we would detect the current package name and version automatically here but I dont think it's possible
+    // instead we made static properties, which really should be abstract, but that is not supported
+    // so here we have some runtime checks to ensure they have been set
+    // see https://github.com/microsoft/TypeScript/issues/34516
+    if (!this.pluginPackageName) throw new Error('DmnoPlugin class must set `static pluginPackageName` prop');
+    if (!this.pluginPackageVersion) throw new Error('DmnoPlugin class must set `static pluginPackageVersion` prop');   
   }
 
   /** name of the plugin itself - which is the name of the class */
@@ -234,10 +242,16 @@ export abstract class DmnoPlugin<
   icon?: string;
 
   static cliPath?: string;
-  get cliPath() {
+  // these 2 should be required, but TS currently does not support static abstract
+  static pluginPackageName: string;
+  static pluginPackageVersion: string;
+  private getStaticProp(key: 'cliPath' | 'pluginPackageName' | 'pluginPackageVersion')  {
     const PluginClass = this.constructor as typeof DmnoPlugin;
-    return PluginClass.cliPath;
+    return PluginClass[key];
   }
+  get cliPath() { return this.getStaticProp('cliPath') }
+  get pluginPackageName() { return this.getStaticProp('pluginPackageName')!; }
+  get pluginPackageVersion() { return this.getStaticProp('pluginPackageVersion')!; }
 
   /**
    * reference back to the service this plugin was initialized in

packages/core/src/config-engine/resolvers/resolvers.ts

@@ -60,7 +60,7 @@ type ResolverDefinition = {
 ({
   resolve: (ctx: ResolverContext) => MaybePromise<ValueResolverResult>,
 } | {
-  resolveBranches: Array<ResolverBranch>
+  resolveBranches: Array<ResolverBranchDefinition>
 });
 
 export function createResolver(def: ResolverDefinition) {
@@ -70,13 +70,12 @@ export function createResolver(def: ResolverDefinition) {
 
 
 
-type ResolverBranch = {
+type ResolverBranchDefinition = {
   id: string,
   label: string;
   resolver: ConfigValueResolver;
   condition: (ctx: ResolverContext) => boolean;
   isDefault: boolean;
-  isActive?: boolean;
 };
 
 export class ConfigValueResolver {
@@ -90,13 +89,14 @@ export class ConfigValueResolver {
     // and to this parent resolver
     // so they can access the branch path if needed
     if ('resolveBranches' in this.def) {
-      _.each(this.def.resolveBranches, (branchDef) => {
-        branchDef.resolver.branchDef = branchDef;
-        branchDef.resolver.parentResolver = this;
+      this.branches = this.def.resolveBranches.map((branchDef) => {
+        return new ConfigValueResolverBranch(branchDef, this);
       });
     }
   }
 
+  linkedBranch?: ConfigValueResolverBranch;
+  branches: Array<ConfigValueResolverBranch> | undefined;
   isResolved = false;
   resolvedValue?: ConfigValue;
   isUsingCache = false;
@@ -109,28 +109,28 @@ export class ConfigValueResolver {
   private _configItem?: DmnoConfigItemBase;
   set configItem(configItem: DmnoConfigItemBase | undefined) {
     this._configItem = configItem;
-    if ('resolveBranches' in this.def) {
-      _.each(this.def.resolveBranches, (branch) => {
-        branch.resolver.configItem = configItem;
-      });
-    }
+    this.branches?.forEach((branch) => {
+      branch.def.resolver.configItem = configItem;
+    });
   }
   get configItem() {
     return this._configItem;
   }
 
-  parentResolver?: ConfigValueResolver;
-  branchDef?: ResolverBranch;
 
+  get parentResolver() {
+    return this.linkedBranch?.parentResolver;
+  }
   get branchIdPath(): string | undefined {
-    if (!this.branchDef) return undefined;
+    if (!this.linkedBranch) return undefined;
+    const thisBranchId = this.linkedBranch.def.id;
     if (this.parentResolver) {
       const parentBranchIdPath = this.parentResolver.branchIdPath;
       if (parentBranchIdPath) {
-        return `${this.parentResolver.branchIdPath}/${this.branchDef.id}`;
+        return `${this.parentResolver.branchIdPath}/${thisBranchId}`;
       }
     }
-    return this.branchDef?.id;
+    return thisBranchId;
   }
 
   getFullPath() {
@@ -140,7 +140,6 @@ export class ConfigValueResolver {
     ]).join('#');
   }
 
-
   async resolve(ctx: ResolverContext) {
     if (_.isFunction(this.def.icon)) this.icon = this.def.icon(ctx);
     if (_.isFunction(this.def.label)) this.label = this.def.label(ctx);
@@ -168,28 +167,44 @@ export class ConfigValueResolver {
     let resolutionResult: ConfigValueResolver | ValueResolverResult;
 
     // deal with branched case (ex: switch / if-else)
-    if ('resolveBranches' in this.def) {
+    if (this.branches) {
       // find first branch that passes
-      let matchingBranch = _.find(this.def.resolveBranches, (branch) => {
-        if (branch.isDefault) return false;
-        return branch.condition(ctx);
+      let matchingBranch = _.find(this.branches, (branch) => {
+        if (branch.def.isDefault) return false;
+        try {
+          return branch.def.condition(ctx);
+        } catch (err) {
+          this.resolutionError = new ResolutionError(`Error in resolver branch condition (${branch.def.label})`, { err: err as Error });
+        }
+        return false;
       });
+      // bail early if we failed evaluating resolver conditions
+      if (this.resolutionError) {
+        this.isResolved = false;
+        return;
+      }
+
       if (!matchingBranch) {
-        matchingBranch = _.find(this.def.resolveBranches, (branch) => branch.isDefault);
+        matchingBranch = _.find(this.branches, (branch) => branch.def.isDefault);
       }
 
-      _.each(this.def.resolveBranches, (branch) => {
+      _.each(this.branches, (branch) => {
         branch.isActive = branch === matchingBranch;
       });
 
       // TODO: might be able to force a default to be defined?
       if (!matchingBranch) {
-        throw new Error('no matching resolver branch found and no default');
+        throw new ResolutionError('no matching resolver branch found and no default');
       }
-      resolutionResult = matchingBranch.resolver || undefined;
+      resolutionResult = matchingBranch.def.resolver || undefined;
 
     // deal with normal case
     } else {
+      // should always be the case, since resolvers must have branches or a resolve fn
+      if (!('resolve' in this.def)) {
+        throw new Error('expected `resolve` fn in resolver definition');
+      }
+
       // actually call the resolver
       try {
         resolutionResult = await this.def.resolve(ctx);
@@ -234,20 +249,34 @@ export class ConfigValueResolver {
       createdByPluginInstanceName: this.def.createdByPlugin?.instanceName,
       // itemPath: this.configItem?.getFullPath(),
       // branchIdPath: this.branchIdPath,
-      ...'resolveBranches' in this.def && {
-        branches: _.map(this.def.resolveBranches, (b) => ({
-          id: b.id,
-          label: b.label,
-          isDefault: b.isDefault,
-          isActive: b.isActive,
-          resolver: b.resolver.toJSON(),
-        })),
+      ...this.branches && {
+        branches: this.branches.map((b) => b.toJSON()),
       },
       resolvedValue: this.resolvedValue,
       resolutionError: this.resolutionError?.toJSON(),
     };
   }
 }
+export class ConfigValueResolverBranch {
+  isActive?: boolean;
+  constructor(
+    readonly def: ResolverBranchDefinition,
+    readonly parentResolver: ConfigValueResolver,
+  ) {
+    // link the branch definition resolver to this object
+    this.def.resolver.linkedBranch = this;
+  }
+
+  toJSON() {
+    return {
+      id: this.def.id,
+      label: this.def.label,
+      isDefault: this.def.isDefault,
+      isActive: this.isActive,
+      resolver: this.def.resolver.toJSON(),
+    };
+  }
+}
 
 export function processInlineResolverDef(resolverDef: InlineValueResolverDef) {
   // set up value resolver