fix(cli): resolve lockgen imports through modules a push leaves alone (#11160)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

cli/src/commands/sync/sync.ts

@@ -128,6 +128,8 @@ import {
 } from "../../utils/metadata.ts";
 import {
   DoubleLinkedDependencyTree,
+  LocalScripts,
+  resolvePlaceholdersFromLocal,
   uploadScripts,
 } from "../../utils/dependency_tree.ts";
 import {
@@ -176,6 +178,7 @@ import {
   isDbtModulePath,
   isDbtGeneratedPath,
   isModuleEntryPoint,
+  scriptPathToRemotePath,
   getScriptBasePathFromModulePath,
   hasWrongFormatSuffix,
   DBT_DESCRIPTOR_NAME,
@@ -3385,6 +3388,37 @@ async function addToChangedIfNotExists(p: string, tracker: ChangeTracker) {
   }
 }
 
+/**
+ * Index the checkout's standalone scripts by the remote path a relative import
+ * resolves to, reusing the content the local/remote diff already read.
+ *
+ * Same classification as `addToChangedIfNotExists`: a flow or app inline script
+ * is not addressable as an import target, and a module bundle is addressed by
+ * its entry point.
+ */
+function localScriptsByRemotePath(
+  localMap: Record<string, string>,
+): LocalScripts {
+  const byRemotePath: LocalScripts = new Map();
+  for (const [p, content] of Object.entries(localMap)) {
+    if (isScriptModulePath(p)) {
+      if (!isModuleEntryPoint(p)) continue;
+    } else if (
+      !hasScriptExt(p) ||
+      isDatatableMigrationPath(p) ||
+      isFileResource(p) ||
+      isFilesetResource(p) ||
+      isFlowPath(p) ||
+      isAppPath(p) ||
+      isRawAppPath(p)
+    ) {
+      continue;
+    }
+    byRemotePath.set(scriptPathToRemotePath(p), { localPath: p, content });
+  }
+  return byRemotePath;
+}
+
 export async function buildTracker(changes: Change[]) {
   const tracker: ChangeTracker = {
     scripts: [],
@@ -5074,6 +5108,14 @@ export async function push(
   }
 
   if (autoRegenerate && tree) {
+    // Pass 1 only ever walks the change set, so anything imported through a
+    // module the push leaves alone is still a dead end here.
+    await resolvePlaceholdersFromLocal(
+      tree,
+      localScriptsByRemotePath(localMap),
+      opts.defaultTs,
+    );
+
     // Propagate staleness through imports + upload script content to
     // raw_script_temp so the dep job can resolve cross-folder relative imports
     // via temp_script_refs (instead of hitting 404s for not-yet-deployed

cli/src/utils/dependency_tree.ts

@@ -5,7 +5,7 @@
 import { Workspace } from "../commands/workspace/workspace.ts";
 import * as wmill from "../../gen/services.gen.ts";
 import type { ScriptLang } from "../../gen/types.gen.ts";
-import { ScriptLanguage } from "./script_common.ts";
+import { ScriptLanguage, inferContentTypeFromFilePath } from "./script_common.ts";
 import {
   filterWorkspaceDependencies,
   generateScriptHash,
@@ -14,6 +14,65 @@ import {
   updateMetadataGlobalLock,
 } from "./metadata.ts";
 import { generateHash } from "./utils.ts";
+import { extractRelativeImports } from "./relative_imports.ts";
+
+/** A local script file, keyed in `LocalScripts` by its Windmill remote path. */
+export interface LocalScriptSource {
+  localPath: string;
+  content: string;
+}
+export type LocalScripts = Map<string, LocalScriptSource>;
+
+/**
+ * Give every import target that is only a placeholder its local content and its
+ * own imports, so the graph continues through it. A tree seeded from a subset of
+ * the checkout otherwise dead-ends at any module outside that subset — a
+ * re-export barrel needing no edit, typically — hiding what it re-exports from
+ * `getTempScriptRefs`, which then resolves it against the deployed copy.
+ */
+export async function resolvePlaceholdersFromLocal(
+  tree: DoubleLinkedDependencyTree,
+  localScripts: LocalScripts,
+  defaultTs: "bun" | "deno" | undefined
+): Promise<void> {
+  // A module resolved here can expose placeholders of its own (a barrel behind
+  // a barrel), so keep going until a round resolves nothing.
+  for (;;) {
+    let resolved = false;
+    for (const remotePath of tree.placeholderPaths()) {
+      const local = localScripts.get(remotePath);
+      if (!local) continue;
+      let language: ScriptLanguage;
+      try {
+        language = inferContentTypeFromFilePath(local.localPath, defaultTs);
+      } catch {
+        // A bare `.sql` names no dialect, so its imports cannot be read here.
+        continue;
+      }
+      const imports = await extractRelativeImports(
+        local.content,
+        remotePath,
+        language
+      );
+      // Never directly stale: it is outside the change set, so nothing relocks
+      // it. It is here to carry edges, and to be uploaded if it differs from
+      // what is deployed.
+      await tree.addNode(
+        remotePath,
+        local.content,
+        language,
+        "",
+        imports,
+        "script",
+        remotePath,
+        local.localPath,
+        false
+      );
+      resolved = true;
+    }
+    if (!resolved) break;
+  }
+}
 
 /**
  * Diff local scripts against deployed versions, upload only those that differ.
@@ -97,6 +156,9 @@ interface DependencyNode {
   originalPath: string;  // Original path passed to handler (with extension for scripts)
   isRawApp?: boolean;    // Only set for apps
   isDirectlyStale: boolean;  // True if this item's content changed (vs transitively stale)
+  // True while the node exists only because something imports it, so it carries
+  // no content and no imports of its own.
+  isPlaceholder: boolean;
 }
 
 export class DoubleLinkedDependencyTree {
@@ -130,9 +192,11 @@ export class DoubleLinkedDependencyTree {
         content: "", stalenessHash: "", language: "deno", metadata: "",
         imports: new Set(), importedBy: new Set(),
         itemType: "script", folder: "", originalPath: "", isDirectlyStale: false,
+        isPlaceholder: true,
       });
     }
     const node = this.nodes.get(path)!;
+    node.isPlaceholder = false;
     node.content = content;
     node.stalenessHash = stalenessHash;
     node.language = language;
@@ -155,7 +219,7 @@ export class DoubleLinkedDependencyTree {
           stalenessHash: "", language: depsInfo?.language ?? "deno", metadata: "",
           imports: new Set(), importedBy: new Set(),
           itemType: "dependencies", folder: "", originalPath: depsPath,
-          isDirectlyStale: !isUpToDate,
+          isDirectlyStale: !isUpToDate, isPlaceholder: false,
         });
       }
     }
@@ -169,6 +233,7 @@ export class DoubleLinkedDependencyTree {
           content: "", stalenessHash: "", language: "deno", metadata: "",
           imports: new Set(), importedBy: new Set(),
           itemType: "script", folder: "", originalPath: "", isDirectlyStale: false,
+          isPlaceholder: true,
         });
       }
       this.nodes.get(importPath)!.importedBy.add(path);
@@ -309,6 +374,18 @@ export class DoubleLinkedDependencyTree {
     return this.nodes.keys();
   }
 
+  /**
+   * Paths that exist only as somebody's import target, so the traversal stops
+   * at them instead of continuing into what they themselves import.
+   */
+  placeholderPaths(): string[] {
+    const result: string[] = [];
+    for (const [path, node] of this.nodes.entries()) {
+      if (node.isPlaceholder) result.push(path);
+    }
+    return result;
+  }
+
   /**
    * Returns paths of all stale nodes (those with a staleReason).
    */

cli/test/dependency_tree_unit.test.ts

@@ -2,7 +2,10 @@ import { expect, test } from "bun:test";
 import { mkdtemp, rm } from "node:fs/promises";
 import os from "node:os";
 import * as path from "node:path";
-import { DoubleLinkedDependencyTree } from "../src/utils/dependency_tree.ts";
+import {
+  DoubleLinkedDependencyTree,
+  resolvePlaceholdersFromLocal,
+} from "../src/utils/dependency_tree.ts";
 
 // addNode consults wmill-lock.yaml from cwd for workspace deps; run inside a
 // temp dir so the test never reads/writes the repo's own lock file.
@@ -104,3 +107,62 @@ test("getAllTempScriptRefs is a superset of getTempScriptRefs for any node", asy
     });
   });
 });
+
+// Two barrels deep so the fixpoint matters: resolving the first one is what
+// puts the second in the tree, and only a further round reaches the leaf.
+test("resolvePlaceholdersFromLocal walks the graph through unresolved barrels", async () => {
+  await withTempDir(async () => {
+    const tree = new DoubleLinkedDependencyTree();
+    await tree.addNode(
+      "f/app/consumer",
+      `import { subtract } from "../barrel/index.ts"`,
+      "bun",
+      "",
+      ["f/barrel/index"],
+      "script",
+      "f/app/consumer",
+      "f/app/consumer.ts",
+      true,
+    );
+    await tree.addNode(
+      "f/barrel/helper",
+      "export function subtract(a: number, b: number) { return a - b }",
+      "bun",
+      "",
+      [],
+      "script",
+      "f/barrel/helper",
+      "f/barrel/helper.ts",
+      true,
+    );
+    // Neither barrel is in the change set, so both are bare import targets.
+    expect(tree.getTempScriptRefs("f/app/consumer")).toEqual({});
+
+    await resolvePlaceholdersFromLocal(
+      tree,
+      new Map([
+        [
+          "f/barrel/index",
+          {
+            localPath: "f/barrel/index.ts",
+            content: `export * from "./mid.ts"`,
+          },
+        ],
+        [
+          "f/barrel/mid",
+          {
+            localPath: "f/barrel/mid.ts",
+            content: `export * from "./helper.ts"`,
+          },
+        ],
+      ]),
+      "bun",
+    );
+
+    // Only the leaf diverged from deployed, so only it was uploaded.
+    tree.setContentHash("f/barrel/helper", "hash_helper");
+    expect(tree.getTempScriptRefs("f/app/consumer")).toEqual({
+      "f/barrel/helper": "hash_helper",
+    });
+  });
+});

cli/test/sync_push_auto_metadata_repro.test.ts

@@ -268,3 +268,78 @@ test(
     });
   },
 );
+
+// The importer and the leaf change; the barrel between them does not, so it is
+// absent from the push's change set. See `resolvePlaceholdersFromLocal`.
+test(
+  "sync push --auto-metadata succeeds when a changed leaf sits behind an unchanged barrel",
+  { timeout: 180000 },
+  async () => {
+    await withTestBackend(async (backend, tempDir) => {
+      await writeFile(`${tempDir}/wmill.yaml`, wmillYaml);
+
+      await createLocalScript(
+        tempDir,
+        "f/barrel",
+        "helper",
+        "bun",
+        `export function add(a: number, b: number) { return a + b; }\n`,
+      );
+      await createLocalScript(
+        tempDir,
+        "f/barrel",
+        "index",
+        "bun",
+        `export * from "./helper.ts";\n`,
+      );
+      await createLocalScript(
+        tempDir,
+        "f/app",
+        "consumer",
+        "bun",
+        `import { add } from "../barrel/index.ts";
+export async function main() { return add(1, 2); }
+`,
+      );
+
+      const deploy = await backend.runCLICommand(
+        ["sync", "push", "--yes", "--auto-metadata"],
+        tempDir,
+      );
+      if (deploy.code !== 0) {
+        console.log("STDOUT:", deploy.stdout);
+        console.log("STDERR:", deploy.stderr);
+      }
+      expect(deploy.code).toBe(0);
+
+      // Add an export to the leaf and use it from the importer. The barrel
+      // re-exports it already, so it stays byte-identical and out of the push.
+      await writeFile(
+        `${tempDir}/f/barrel/helper.ts`,
+        `export function add(a: number, b: number) { return a + b; }
+export function subtract(a: number, b: number) { return a - b; }
+`,
+      );
+      await writeFile(
+        `${tempDir}/f/app/consumer.ts`,
+        `import { subtract } from "../barrel/index.ts";
+export async function main() { return subtract(3, 1); }
+`,
+      );
+
+      const result = await backend.runCLICommand(
+        ["sync", "push", "--yes", "--auto-metadata"],
+        tempDir,
+      );
+      if (result.code !== 0) {
+        console.log("STDOUT:", result.stdout);
+        console.log("STDERR:", result.stderr);
+      }
+      expect(result.code).toBe(0);
+
+      const combined = result.stdout + result.stderr;
+      expect(combined).not.toContain("No matching export");
+      expect(combined).not.toContain("Failed to generate lockfile");
+    });
+  },
+);