fix(cli): attach the right job path to preview runs (#10606)

* fix(cli): attach the right job path to preview runs

* fix(cli): keep a deliberately-absent file in the directory it was named in

* fix(cli): resolve links for a path naming a file that is not there

cli/src/commands/flow/flow.ts

@@ -13,7 +13,12 @@ import { mkdirSync, writeFileSync } from "node:fs";
 import { buildFolderPath, getMetadataFileName, loadNonDottedPathsSetting } from "../../utils/resource_folders.ts";
 
 import { requireLogin } from "../../core/auth.ts";
-import { resolveWorkspace, validatePath } from "../../core/context.ts";
+import {
+  assertRemotePath,
+  resolveWorkspace,
+  toSyncRootRelativePath,
+  validatePath,
+} from "../../core/context.ts";
 import { resolve, track_job, pollForJobResult } from "../script/script.ts";
 import { defaultFlowDefinition } from "../../../bootstrap/flow_bootstrap.ts";
 import { SyncOptions, mergeConfigWithConfigFile } from "../../core/conf.ts";
@@ -605,6 +610,8 @@ async function preview(
     log.setSilent(true);
   }
   const useLocalPathScripts = !opts.remote;
+  // Captured before the config read, which chdirs to the wmill.yaml root.
+  const cwdBeforeConfig = process.cwd();
   if (useLocalPathScripts) {
     opts = await mergeConfigWithConfigFile(opts);
   }
@@ -612,6 +619,9 @@ async function preview(
   await requireLogin(opts);
   const codebases = useLocalPathScripts ? listSyncCodebases(opts) : [];
 
+  const argPath = flowPath;
+  flowPath = toSyncRootRelativePath(flowPath, cwdBeforeConfig);
+
   // Normalize path - ensure it's a directory path to a .flow or __flow folder
   const isFlowDir = flowPath.endsWith(".flow") || flowPath.endsWith(".flow" + SEP)
     || flowPath.endsWith("__flow") || flowPath.endsWith("__flow" + SEP);
@@ -633,6 +643,14 @@ async function preview(
     flowPath += SEP;
   }
 
+  // The flow's windmill path (e.g. "f/cli_smoke/myrelflow"). It is what the
+  // preview job runs under, and the anchor for relative-import resolution:
+  // inline scripts in this flow are treated as living at
+  // "<flow_wm_path>/<step_id>", so "./util" resolves to
+  // "<flow_wm_path_parent>/util" — matching the keys in temp_script_refs.
+  const flowWmPath = stripFlowSuffix(flowPath).replaceAll(SEP, "/");
+  assertRemotePath(flowWmPath, argPath);
+
   // Read and parse the flow definition
   const localFlow = (await yamlParseFile(flowPath + "flow.yaml")) as FlowFile;
 
@@ -703,12 +721,6 @@ async function preview(
   // too — PathScript modules have already been rewritten to inline rawscript
   // when `useLocalPathScripts` is set, and tempScriptRefs covers relative
   // imports in inline scripts.
-  // Compute the flow's windmill path (e.g. "f/cli_smoke/myrelflow"). Used as
-  // the anchor for relative-import resolution: inline scripts in this flow are
-  // treated as living at "<flow_wm_path>/<step_id>", so "./util" resolves to
-  // "<flow_wm_path_parent>/util" — matching the keys in temp_script_refs.
-  const flowWmPath = stripFlowSuffix(flowPath).replaceAll(SEP, "/");
-
   if (opts.step) {
     await previewStep(opts.step, localFlow, flowWmPath, workspace, input, tempScriptRefs, opts.silent, opts.tag);
     return;

cli/src/commands/script/script.ts

@@ -1,6 +1,11 @@
 import { GlobalOptions } from "../../types.ts";
 import { requireLogin } from "../../core/auth.ts";
-import { resolveWorkspace, validatePath } from "../../core/context.ts";
+import {
+  assertRemotePath,
+  resolveWorkspace,
+  toSyncRootRelativePath,
+  validatePath,
+} from "../../core/context.ts";
 import type { PermissionedAsContext } from "../../core/permissioned_as.ts";
 import { applyExtraPermsDiff } from "../../core/extra_perms.ts";
 import { writeFile, stat, mkdir } from "node:fs/promises";
@@ -1808,13 +1813,16 @@ async function preview(
   if (opts.silent) {
     log.setSilent(true);
   }
+  // Captured before the config read, which chdirs to the wmill.yaml root.
+  const cwdBeforeConfig = process.cwd();
   opts = await mergeConfigWithConfigFile(opts);
   const workspace = await resolveWorkspace(opts);
   await requireLogin(opts);
 
-  if (!validatePath(filePath)) {
-    return;
-  }
+  const argPath = filePath;
+  filePath = toSyncRootRelativePath(filePath, cwdBeforeConfig);
+  const remotePath = scriptPathToRemotePath(filePath);
+  assertRemotePath(remotePath, argPath);
 
   // Same as push: a descriptor-less dbt project's content path is deliberately
   // absent, and the project beside it is what says the script is real.
@@ -1869,11 +1877,7 @@ async function preview(
   const { extractRelativeImports } = await import(
     "../../utils/relative_imports.ts"
   );
-  const relImports = await extractRelativeImports(
-    content,
-    scriptPathToRemotePath(filePath),
-    language
-  );
+  const relImports = await extractRelativeImports(content, remotePath, language);
   if (relImports.length > 0) {
     const { buildPreviewTempScriptRefs } = await import(
       "../generate-metadata/generate-metadata.ts"
@@ -1976,7 +1980,7 @@ async function preview(
     const form = new FormData();
     const previewPayload = {
       content: content, // Pass the original content (frontend does this too)
-      path: filePath.substring(0, filePath.indexOf(".")).replaceAll(SEP, "/"),
+      path: remotePath,
       args: input,
       language: language,
       tag: opts.tag,
@@ -2046,7 +2050,7 @@ async function preview(
       workspace: workspace.workspaceId,
       requestBody: {
         content,
-        path: filePath.substring(0, filePath.indexOf(".")).replaceAll(SEP, "/"),
+        path: remotePath,
         args: input,
         language: language as any,
         tag: opts.tag,

cli/src/core/context.ts

@@ -22,8 +22,11 @@ import {
   readConfigFile,
   findWorkspaceByGitBranch,
   getEffectiveWorkspaceId,
+  getWmillYamlPath,
   WorkspaceEntryConfig,
 } from "./conf.ts";
+import { existsSync, realpathSync } from "node:fs";
+import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path";
 import {
   getCurrentGitBranch,
   getOriginalBranchForWorkspaceForks,
@@ -743,6 +746,92 @@ export async function tryResolveVersion(
   }
 }
 
+/**
+ * Directory the local tree mirrors the workspace from: the one holding
+ * wmill.yaml. Not `process.cwd()` — that only lands there once a config read
+ * has chdir'd into it, which `--remote` and fully-flagged invocations skip.
+ */
+function syncRoot(): string {
+  const wmillYaml = getWmillYamlPath();
+  return wmillYaml ? dirname(wmillYaml) : process.cwd();
+}
+
+/**
+ * Re-express a user-supplied file or folder argument as a path relative to the
+ * sync root, so the Windmill path derived from it is the same whatever shape
+ * the argument had (`./f/a/b.ts`, `/abs/repo/f/a/b.ts`, `b.ts` from inside
+ * `f/a`).
+ *
+ * `cwdBeforeConfig` must be the working directory as it was *before* the
+ * command read wmill.yaml: reading it chdirs into the directory holding it, and
+ * a relative argument was written against the directory the user was in.
+ * Arguments are resolved against that directory first and the sync root second,
+ * so both readings work.
+ *
+ * Resolving the sync root leaves the process in it, which is what makes the
+ * returned path readable by the caller — keep that in step if this ever stops
+ * going through `getWmillYamlPath`.
+ */
+export function toSyncRootRelativePath(
+  arg: string,
+  cwdBeforeConfig: string
+): string {
+  const root = syncRoot();
+  const candidates = isAbsolute(arg)
+    ? [arg]
+    : [resolve(cwdBeforeConfig, arg), resolve(root, arg)];
+  // A descriptor-less dbt project is named by a file that is deliberately not
+  // there, so an argument existing under neither reading is still a real one if
+  // the directory holding it is; only a path whose directory is missing too
+  // falls through to the sync-root reading for the caller to reject.
+  const abs =
+    candidates.find((c) => existsSync(c)) ??
+    candidates.find((c) => existsSync(dirname(c))) ??
+    candidates.at(-1)!;
+  const rel = relative(root, abs);
+  if (rel === "") return ".";
+  if (!rel.startsWith("..")) return rel;
+  // `relative` is purely lexical, so a root reached through a symlink (macOS'
+  // /var -> /private/var, a symlinked checkout) makes an absolute argument look
+  // like it escapes the tree. Resolve links only then, so a symlinked file
+  // *inside* the tree keeps the path it is filed under.
+  try {
+    const resolved = relative(realpathSync(root), realpathOfNamed(abs));
+    return resolved === "" ? "." : resolved;
+  } catch {
+    return rel;
+  }
+}
+
+/**
+ * `realpathSync` needs its target to exist, and a dbt descriptor deliberately
+ * does not. The directory naming it does, so resolve that and reattach.
+ */
+function realpathOfNamed(p: string): string {
+  return existsSync(p)
+    ? realpathSync(p)
+    : join(realpathSync(dirname(p)), basename(p));
+}
+
+/** Windmill workspace path: `u|f|g` followed by at least a folder and a name. */
+const REMOTE_PATH_RE = /^[ufg](\/[^/]+){2,}$/;
+
+/**
+ * Guard the Windmill path a preview run is pushed under. A preview job carries
+ * no runnable of its own, so this path is the only identity it has: it is what
+ * `WM_JOB_PATH` reports, what the runs page links to, and what relative imports
+ * inside the previewed code resolve against.
+ */
+export function assertRemotePath(remotePath: string, arg: string): void {
+  if (REMOTE_PATH_RE.test(remotePath)) return;
+  throw new Error(
+    `Cannot derive a Windmill path from '${arg}'` +
+      (remotePath ? ` (it maps to '${remotePath}')` : "") +
+      `: a preview runs under the path of the file it previews, which must sit inside the ` +
+      `wmill.yaml root and be of the form <u|g|f>/<username|group|folder>/<name>.`
+  );
+}
+
 export function validatePath(path: string): boolean {
   if (!(path.startsWith("g") || path.startsWith("u") || path.startsWith("f"))) {
     log.infoStderr(

cli/test/preview.test.ts

@@ -185,6 +185,60 @@ test("script preview: regular script (non-codebase)", async () => {
   });
 });
 
+test("script preview: job path is the script's Windmill path for every argument shape", async () => {
+  await withTestBackend(async (backend, tempDir) => {
+    await createWmillConfig(tempDir, { defaultTs: "bun" });
+    await createScript(
+      tempDir,
+      "f/test/job_path_script.ts",
+      `export function main() {
+  return process.env.WM_JOB_PATH;
+}`
+    );
+
+    const invocations: Array<[string, string]> = [
+      ["f/test/job_path_script.ts", tempDir],
+      ["./f/test/job_path_script.ts", tempDir],
+      [`${tempDir}/f/test/job_path_script.ts`, tempDir],
+      ["job_path_script.ts", `${tempDir}/f/test`],
+    ];
+
+    for (const [arg, workingDir] of invocations) {
+      const result = await backend.runCLICommand(
+        ["script", "preview", arg, "--silent"],
+        workingDir
+      );
+
+      expect(result.code).toEqual(0);
+      expect(result.stdout.trim()).toEqual(`"f/test/job_path_script"`);
+    }
+
+    // Folder layout: the job runs under the script's path, not the entry file's.
+    await createScript(
+      tempDir,
+      "f/test/job_path_module__mod/script.ts",
+      `export function main() {
+  return process.env.WM_JOB_PATH;
+}`
+    );
+    const moduleResult = await backend.runCLICommand(
+      ["script", "preview", "f/test/job_path_module__mod/script.ts", "--silent"],
+      tempDir
+    );
+    expect(moduleResult.code).toEqual(0);
+    expect(moduleResult.stdout.trim()).toEqual(`"f/test/job_path_module"`);
+
+    // A file outside the workspace tree has no Windmill path to run under, so
+    // the run is refused rather than pushed with a made-up one.
+    await writeFile(`${tempDir}/stray.ts`, `export function main() {}`, "utf-8");
+    const strayResult = await backend.runCLICommand(
+      ["script", "preview", "stray.ts", "--silent"],
+      tempDir
+    );
+    expect(strayResult.code).toEqual(1);
+  });
+});
+
 test("script preview: codebase script (CJS)", async () => {
   await withTestBackend(async (backend, tempDir) => {
     await createWmillConfig(tempDir, {
@@ -529,6 +583,37 @@ test("flow preview: simple flow", async () => {
   });
 });
 
+test("flow preview: step job path is anchored on the flow's Windmill path", async () => {
+  await withTestBackend(async (backend, tempDir) => {
+    await createWmillConfig(tempDir, { defaultTs: "bun" });
+    await createFlow(tempDir, "f/test/job_path_flow.flow", {
+      summary: "Test flow",
+      scriptContent: `export function main() { return process.env.WM_JOB_PATH; }`,
+    });
+
+    // The last one is `--remote` from a subdirectory: that combination reads
+    // no config of its own, so it is the one shape where nothing but the path
+    // resolution can put the process in the sync root.
+    const invocations: Array<[string[], string]> = [
+      [["f/test/job_path_flow.flow"], tempDir],
+      [["./f/test/job_path_flow.flow"], tempDir],
+      [[`${tempDir}/f/test/job_path_flow.flow`], tempDir],
+      [["job_path_flow.flow"], `${tempDir}/f/test`],
+      [["--remote", "job_path_flow.flow"], `${tempDir}/f/test`],
+    ];
+
+    for (const [args, workingDir] of invocations) {
+      const result = await backend.runCLICommand(
+        ["flow", "preview", ...args, "--silent"],
+        workingDir
+      );
+
+      expect(result.code).toEqual(0);
+      expect(result.stdout.trim()).toEqual(`"f/test/job_path_flow/a"`);
+    }
+  });
+});
+
 test("flow preview: uses local PathScript by default and remote PathScript with --remote", async () => {
   await withTestBackend(async (backend, tempDir) => {
     await createWmillConfig(tempDir, { defaultTs: "bun" });

cli/test/preview_path_unit.test.ts

@@ -0,0 +1,112 @@
+/**
+ * A preview job carries no runnable, so the path derived from the file
+ * argument is the whole of its identity — it has to come out the same
+ * whatever shape the argument had, and be refused rather than guessed when
+ * the file has no place in the workspace tree.
+ */
+import { expect, test, describe, beforeEach, afterEach } from "bun:test";
+import * as fs from "node:fs";
+import * as path from "node:path";
+import * as os from "node:os";
+import {
+  assertRemotePath,
+  toSyncRootRelativePath,
+} from "../src/core/context.ts";
+
+describe("toSyncRootRelativePath", () => {
+  let root: string;
+  let previousCwd: string;
+  let temps: string[];
+
+  beforeEach(() => {
+    previousCwd = process.cwd();
+    temps = [];
+    // realpath: macOS' tmpdir is /var -> /private/var, and the assertions
+    // compare against what the process reports as its own directory.
+    root = fs.realpathSync(
+      fs.mkdtempSync(path.join(os.tmpdir(), "wmill_preview_path_")),
+    );
+    temps.push(root);
+    fs.writeFileSync(path.join(root, "wmill.yaml"), "defaultTs: bun\n");
+    fs.mkdirSync(path.join(root, "f", "test"), { recursive: true });
+    fs.writeFileSync(path.join(root, "f", "test", "script.ts"), "");
+    process.chdir(root);
+  });
+
+  afterEach(() => {
+    process.chdir(previousCwd);
+    for (const dir of temps) fs.rmSync(dir, { recursive: true, force: true });
+  });
+
+  const normalized = (p: string) => p.replaceAll("\\", "/");
+
+  /** A dbt project whose descriptor is deliberately not written. */
+  function dbtProject(): string {
+    const project = path.join(root, "f", "test", "proj__dbt");
+    fs.mkdirSync(project, { recursive: true });
+    fs.writeFileSync(path.join(project, "dbt_project.yml"), "name: proj\n");
+    return project;
+  }
+
+  test("every spelling of the same file lands on the same path", () => {
+    const fromRoot = ["f/test/script.ts", "./f/test/script.ts"].map((arg) =>
+      normalized(toSyncRootRelativePath(arg, root)),
+    );
+    const absolute = normalized(
+      toSyncRootRelativePath(path.join(root, "f", "test", "script.ts"), root),
+    );
+    // As typed from the directory the file is in: the config read has already
+    // moved the process to the root by the time the argument is resolved.
+    const fromSubdir = normalized(
+      toSyncRootRelativePath("script.ts", path.join(root, "f", "test")),
+    );
+
+    expect(fromRoot).toEqual(["f/test/script.ts", "f/test/script.ts"]);
+    expect(absolute).toEqual("f/test/script.ts");
+    expect(fromSubdir).toEqual("f/test/script.ts");
+  });
+
+  test("a file that is deliberately absent keeps the directory it was named in", () => {
+    // A dbt project's descriptor is optional; `wmill script preview
+    // wm_dbt.yaml` from inside the project must still resolve to the project.
+    const project = dbtProject();
+
+    expect(normalized(toSyncRootRelativePath("wm_dbt.yaml", project))).toEqual(
+      "f/test/proj__dbt/wm_dbt.yaml",
+    );
+  });
+
+  // Windows only creates symlinks for a privileged process.
+  test.skipIf(process.platform === "win32")(
+    "an absent file reached through a symlinked root still lands in the tree",
+    () => {
+      dbtProject();
+      const aliasDir = fs.mkdtempSync(path.join(os.tmpdir(), "wmill_alias_"));
+      temps.push(aliasDir);
+      const alias = path.join(aliasDir, "link");
+      fs.symlinkSync(root, alias, "dir");
+
+      const arg = path.join(alias, "f", "test", "proj__dbt", "wm_dbt.yaml");
+      expect(normalized(toSyncRootRelativePath(arg, root))).toEqual(
+        "f/test/proj__dbt/wm_dbt.yaml",
+      );
+    },
+  );
+
+  test("a file outside the tree stays outside it", () => {
+    const outside = path.join(root, "..", "elsewhere.ts");
+    expect(toSyncRootRelativePath(outside, root).startsWith("..")).toBe(true);
+  });
+});
+
+describe("assertRemotePath", () => {
+  test("accepts a workspace path and refuses anything else", () => {
+    expect(() => assertRemotePath("f/test/script", "f/test/script.ts")).not.toThrow();
+    expect(() => assertRemotePath("u/admin/script", "script.ts")).not.toThrow();
+    for (const bad of ["", "script", "f/script", "../elsewhere"]) {
+      expect(() => assertRemotePath(bad, "arg.ts")).toThrow(
+        /Cannot derive a Windmill path/,
+      );
+    }
+  });
+});