Fix diamond dependency double-import handling (#553)

* Fix diamond dependency double-import causing 'already initialized' error

When the same schema directory is imported via multiple paths (diamond
dependency), the schema file was being loaded twice. This caused plugin
init decorators (like @initAws) to run twice, throwing "Instance with id
'_default' already initialized".

Fix: Add _loadedImportPaths Set to EnvGraph to track already-loaded
import paths. Before adding a new DirectoryDataSource or
DotEnvFileDataSource for an import, check if the path was already
imported. If so, skip the duplicate import.

Also adds a test plugin (test-plugin-with-init) and three new tests in
import.test.ts covering the diamond dependency pattern for directory
imports, directory imports with plugins, and file imports.

Agent-Logs-Url: https://github.com/dmno-dev/varlock/sessions/a36fdb26-ef5a-41af-a057-cfa6d4d04f7c

Co-authored-by: philmillman <3722211+philmillman@users.noreply.github.com>

* Refactor: extract checkAndRecordImportPath helper to reduce duplication

Agent-Logs-Url: https://github.com/dmno-dev/varlock/sessions/a36fdb26-ef5a-41af-a057-cfa6d4d04f7c

Co-authored-by: philmillman <3722211+philmillman@users.noreply.github.com>

* Fix diamond dependency: use ImportAliasSource for correct precedence and importKeys handling

The previous approach skipped duplicate imports entirely, which lost importKeys
from the second import and placed definitions at the wrong precedence position.

Now, duplicate imports create lightweight ImportAliasSource nodes in the tree.
These alias nodes share the original source's definitions but carry their own
importMeta/parent, giving them the correct position in the precedence chain.

A new sortedDefinitionSources list (derived from sortedDataSources by expanding
aliases) is used by ConfigItem.defs for item resolution, while sortedDataSources
remains unchanged for plugin init, error collection, and serialization.

Also adds type field to serialized source entries and updates the Next.js
integration to filter by type instead of label prefix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix typecheck: add 'import-alias' to DataSourceType union

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix smoke-test CI: use env vars to avoid shell injection from PR title

PR titles containing quotes broke the bash conditional. Using env vars
instead of inline interpolation prevents shell parsing issues.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix plugin typecheck: add node types to plugin tsconfig

The plugin tsconfig was missing node type definitions, causing typecheck
failures for any plugin importing node built-ins.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: philmillman <3722211+philmillman@users.noreply.github.com>
Co-authored-by: Theo Ephraim <theo@dmno.dev>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 4 people

.changeset/fix-diamond-dependency-double-import.md

@@ -0,0 +1,6 @@
+---
+"varlock": patch
+"@varlock/nextjs-integration": patch
+---
+
+Fix diamond dependency handling when the same schema is imported via multiple paths. Previously, duplicate imports caused plugin init decorators to run twice ("Instance already initialized" error). Now, duplicate imports create lightweight `ImportAliasSource` nodes that appear at the correct precedence position without re-initializing the source. This correctly handles different importKeys subsets across import sites and preserves override semantics matching non-deduplicated behavior. Also adds `type` field to serialized source entries for easier filtering.

.github/workflows/smoke-test.yaml

@@ -20,8 +20,12 @@ jobs:
       is-release-pr: ${{ steps.check.outputs.is-release-pr }}
     steps:
       - id: check
+        env:
+          PR_TITLE: ${{ github.event.pull_request.title }}
+          HAS_LABEL: ${{ contains(github.event.pull_request.labels.*.name, 'smoke-tests') }}
+          EVENT_NAME: ${{ github.event_name }}
         run: |
-          if [[ "${{ github.event.pull_request.title }}" == *"[Changesets]"* ]] || [[ "${{ contains(github.event.pull_request.labels.*.name, 'smoke-tests') }}" == "true" ]] || [[ "${{ github.event_name }}" == "workflow_dispatch" ]] || [[ "${{ github.event_name }}" == "push" ]]; then
+          if [[ "$PR_TITLE" == *"[Changesets]"* ]] || [[ "$HAS_LABEL" == "true" ]] || [[ "$EVENT_NAME" == "workflow_dispatch" ]] || [[ "$EVENT_NAME" == "push" ]]; then
             echo "is-release-pr=true" >> $GITHUB_OUTPUT
           else
             echo "is-release-pr=false" >> $GITHUB_OUTPUT

bun.lock

@@ -35,9 +35,7 @@ let rootDir: string | undefined;
 // a list of filenames loaded, for example: `Environments: .env, .env.development`
 function getVarlockSourcesAsLoadedEnvFiles(): LoadedEnvFiles {
   const envFilesLabels = varlockLoadedEnv.sources
-    // TODO expose more info so we can filter out disabled sources
-    // and maybe show relative paths
-    .filter((s) => s.enabled && !s.label.startsWith('directory -'))
+    .filter((s) => s.enabled && s.type !== 'container' && s.type !== 'import-alias')
     .map((s) => s.label);
   if (envFilesLabels.length) {
     // this adds an additional line, below the list of files

packages/integrations/nextjs/src/next-env-compat.ts

@@ -1,3 +1,6 @@
 {
-  "extends": "./base.tsconfig.json"
+  "extends": "./base.tsconfig.json",
+  "compilerOptions": {
+    "types": ["node"]
+  }
 }

packages/tsconfig/plugin.tsconfig.json

@@ -45,20 +45,19 @@ export class ConfigItem {
   }
 
   /**
-   * fetch ordered list of definitions for this item, by following up sorted data sources list
+   * fetch ordered list of definitions for this item, by following the sorted definition sources list
    * internal defs (builtins) are appended last (lowest priority)
    */
   get defs() {
     // TODO: this is somewhat inneficient, because some of the checks on the data source follow up the parent chain
     // we may want to cache the definition list at some point when loading is complete
     // although we need it to be dynamic during the loading process when doing any early resolution of the envFlag
     const defs: Array<ConfigItemDefAndSource> = [];
-    for (const source of this.envGraph.sortedDataSources) {
+    for (const { source, importKeys } of this.envGraph.sortedDefinitionSources) {
       if (!source.configItemDefs[this.key]) continue;
       if (source.disabled) continue;
-      if (source.importKeys && !source.importKeys.includes(this.key)) continue;
-      const itemDef = source.configItemDefs[this.key];
-      if (itemDef) defs.push({ itemDef, source });
+      if (importKeys && !importKeys.includes(this.key)) continue;
+      defs.push({ itemDef: source.configItemDefs[this.key], source });
     }
     defs.push(...this._internalDefs);
     return defs;

packages/varlock/src/env-graph/lib/config-item.ts

@@ -35,6 +35,8 @@ const DATA_SOURCE_TYPES = Object.freeze({
   },
   container: {
   },
+  'import-alias': {
+  },
 });
 type DataSourceType = keyof typeof DATA_SOURCE_TYPES;
 
@@ -386,6 +388,18 @@ export abstract class EnvGraphDataSource {
             const fileName = path.basename(fullImportPath);
 
             // TODO: might be nice to move this logic somewhere else
+            // Check for diamond dependency: if this path was already loaded,
+            // add an alias instead of re-loading (prevents double plugin init)
+            const existingSource = this.graph.getLoadedImportSource(fullImportPath);
+            if (existingSource) {
+              // eslint-disable-next-line no-use-before-define
+              await this.addChild(new ImportAliasSource(existingSource), {
+                isImport: true, importKeys, isConditionallyEnabled,
+              });
+              this.graph.registerItemsForImport(existingSource, this, importKeys);
+              continue;
+            }
+
             if (this.graph.virtualImports) {
               if (importPath.endsWith('/')) {
                 const dirExists = Object.keys(this.graph.virtualImports).some((p) => p.startsWith(fullImportPath));
@@ -395,9 +409,11 @@ export abstract class EnvGraphDataSource {
                   return;
                 }
                 // eslint-disable-next-line no-use-before-define
-                await this.addChild(new DirectoryDataSource(fullImportPath), {
+                const dirChild = new DirectoryDataSource(fullImportPath);
+                await this.addChild(dirChild, {
                   isImport: true, importKeys, isConditionallyEnabled,
                 });
+                this.graph.recordLoadedImportPath(fullImportPath, dirChild);
               } else {
                 const fileExists = this.graph.virtualImports[fullImportPath];
                 if (!fileExists && allowMissing) continue;
@@ -406,10 +422,11 @@ export abstract class EnvGraphDataSource {
                   return;
                 }
                 // eslint-disable-next-line no-use-before-define
-                const source = new DotEnvFileDataSource(fullImportPath, {
+                const fileChild = new DotEnvFileDataSource(fullImportPath, {
                   overrideContents: this.graph.virtualImports[fullImportPath],
                 });
-                await this.addChild(source, { isImport: true, importKeys, isConditionallyEnabled });
+                await this.addChild(fileChild, { isImport: true, importKeys, isConditionallyEnabled });
+                this.graph.recordLoadedImportPath(fullImportPath, fileChild);
               }
             } else {
               const fsStat = await tryCatch(async () => fs.stat(fullImportPath), (_err) => {
@@ -426,9 +443,11 @@ export abstract class EnvGraphDataSource {
               if (importPath.endsWith('/')) {
                 if (fsStat.isDirectory()) {
                   // eslint-disable-next-line no-use-before-define
-                  await this.addChild(new DirectoryDataSource(fullImportPath), {
+                  const dirChild = new DirectoryDataSource(fullImportPath);
+                  await this.addChild(dirChild, {
                     isImport: true, importKeys, isConditionallyEnabled,
                   });
+                  this.graph.recordLoadedImportPath(fullImportPath, dirChild);
                 } else {
                   this._loadingError = new Error(`Imported path ending with "/" is not a directory: ${fullImportPath}`);
                   return;
@@ -444,9 +463,11 @@ export abstract class EnvGraphDataSource {
                 }
                 // TODO: once we have more file types, here we would detect the type and import it correctly
                 // eslint-disable-next-line no-use-before-define
-                await this.addChild(new DotEnvFileDataSource(fullImportPath), {
+                const fileChild = new DotEnvFileDataSource(fullImportPath);
+                await this.addChild(fileChild, {
                   isImport: true, importKeys, isConditionallyEnabled,
                 });
+                this.graph.recordLoadedImportPath(fullImportPath, fileChild);
               }
             }
           } else if (importPath.startsWith('http://') || importPath.startsWith('https://')) {
@@ -546,6 +567,32 @@ export abstract class EnvGraphDataSource {
   }
 }
 
+/**
+ * Lightweight alias created when the same path is imported from multiple locations.
+ * Lives in the tree like any other child (has its own importMeta/parent) but delegates
+ * definitions to the original source. Has no rootDecorators or configItemDefs of its own,
+ * so sortedDataSources consumers (plugin init, error collection, etc.) naturally skip it.
+ */
+export class ImportAliasSource extends EnvGraphDataSource {
+  type = 'import-alias' as const;
+  typeLabel = 'import-alias';
+
+  constructor(
+    /** the real data source this alias points to */
+    readonly original: EnvGraphDataSource,
+  ) {
+    super();
+  }
+
+  get label() { return `re-import of ${this.original.label}`; }
+
+  // no-op — the original was already fully initialized
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  async _finishInit() {}
+  // eslint-disable-next-line @typescript-eslint/no-empty-function
+  async _processImports() {}
+}
+
 export abstract class FileBasedDataSource extends EnvGraphDataSource {
   fullPath: string;
   fileName: string;

packages/varlock/src/env-graph/lib/data-source.ts

@@ -1,7 +1,7 @@
 import _ from '@env-spec/utils/my-dash';
 import path from 'node:path';
 import { ConfigItem } from './config-item';
-import { EnvGraphDataSource, FileBasedDataSource } from './data-source';
+import { EnvGraphDataSource, FileBasedDataSource, ImportAliasSource } from './data-source';
 
 import { BaseResolvers, createResolver, type ResolverChildClass } from './resolver';
 import { BaseDataTypes, type EnvGraphDataTypeFactory } from './data-types';
@@ -27,9 +27,18 @@ export type SerializedEnvGraphErrors = {
   root?: Array<string>;
 };
 
+/** Entry in the sorted definition sources list — pairs a data source with the importKeys
+ * filter that applies at that specific position in the precedence chain */
+export type DefinitionSourceEntry = {
+  source: EnvGraphDataSource;
+  /** importKeys filter for this position (undefined = all keys visible) */
+  importKeys?: Array<string>;
+};
+
 export type SerializedEnvGraph = {
   basePath?: string;
   sources: Array<{
+    type: string;
     label: string;
     enabled: boolean;
     path?: string;
@@ -68,6 +77,63 @@ export class EnvGraph {
   configSchema: Record<string, ConfigItem> = {};
 
 
+  /**
+   * Tracks directory/file paths that have already been loaded as imports.
+   * Maps each import path to the data source that was created for it.
+   * Used to prevent diamond-dependency re-imports (same schema imported via multiple paths),
+   * which would otherwise cause plugin init decorators to run multiple times.
+   */
+  private _loadedImportPaths = new Map<string, EnvGraphDataSource>();
+
+  /** Returns the existing source for a path if already loaded, or undefined */
+  getLoadedImportSource(importPath: string): EnvGraphDataSource | undefined {
+    return this._loadedImportPaths.get(importPath);
+  }
+
+  /** Records the data source that was created for an import path */
+  recordLoadedImportPath(importPath: string, dataSource: EnvGraphDataSource) {
+    this._loadedImportPaths.set(importPath, dataSource);
+  }
+
+  /**
+   * Register ConfigItems for keys visible through an import
+   * that may not have been registered during the original source's finishInit.
+   */
+  registerItemsForImport(
+    source: EnvGraphDataSource,
+    importSite: EnvGraphDataSource,
+    importKeys?: Array<string>,
+  ) {
+    // Compute effective importKeys: intersection of import filter and importSite's parent chain
+    const siteKeys = importSite.importKeys;
+    let effectiveKeys: Array<string> | undefined;
+    const hasFilter = importKeys && importKeys.length > 0;
+    if (hasFilter && siteKeys?.length) {
+      effectiveKeys = importKeys.filter((k) => siteKeys.includes(k));
+    } else if (hasFilter) {
+      effectiveKeys = importKeys;
+    } else {
+      effectiveKeys = siteKeys;
+    }
+
+    for (const s of this._getDescendants(source)) {
+      const keys = effectiveKeys || _.keys(s.configItemDefs);
+      for (const itemKey of keys) {
+        if (!s.configItemDefs[itemKey]) continue;
+        this.configSchema[itemKey] ??= new ConfigItem(this, itemKey);
+      }
+    }
+  }
+
+  /** Get a data source and all its descendants (DFS) */
+  private _getDescendants(source: EnvGraphDataSource): Array<EnvGraphDataSource> {
+    const result: Array<EnvGraphDataSource> = [source];
+    for (const child of source.children) {
+      result.push(...this._getDescendants(child));
+    }
+    return result;
+  }
+
   /** virtual imports for testing */
   virtualImports?: Record<string, string>;
   setVirtualImports(basePath: string, files: Record<string, string>) {
@@ -85,6 +151,36 @@ export class EnvGraph {
     return this.rootDataSource ? getSourceAndChildren(this.rootDataSource) : [];
   }
 
+  /**
+   * Precedence-ordered list of definition sources, used by ConfigItem.defs.
+   *
+   * Unlike `sortedDataSources` (which contains each real source exactly once),
+   * this list can contain the same source multiple times at different positions
+   * when it's imported from multiple locations (diamond dependency). Each entry
+   * carries its own `importKeys` filter for that specific import context.
+   *
+   * Built from `sortedDataSources` by expanding `ImportAliasSource` nodes into
+   * the original source's full subtree at the alias's precedence position.
+   */
+  get sortedDefinitionSources(): Array<DefinitionSourceEntry> {
+    const result: Array<DefinitionSourceEntry> = [];
+
+    for (const source of this.sortedDataSources) {
+      if (source instanceof ImportAliasSource) {
+        // Alias: expand to the original source's subtree at this position,
+        // using the alias's importKeys (derived from its own parent chain)
+        const importKeys = source.importKeys;
+        for (const descendant of this._getDescendants(source.original)) {
+          result.push({ source: descendant, importKeys });
+        }
+      } else {
+        result.push({ source, importKeys: source.importKeys });
+      }
+    }
+
+    return result;
+  }
+
   registeredResolverFunctions: Record<string, ResolverChildClass> = {};
   registerResolver(resolverClass: ResolverChildClass) {
     // because its a class, we can't use `name`
@@ -439,6 +535,7 @@ export class EnvGraph {
     };
     for (const source of this.sortedDataSources) {
       serializedGraph.sources.push({
+        type: source.type,
         label: source.label,
         enabled: !source.disabled,
         path: source instanceof FileBasedDataSource ? path.relative(this.basePath ?? '', source.fullPath) : undefined,