Address bot review comments for updateConfigObject

- Roll back the config file and its referenced files if the agent fails or
  its result doesn't validate, so no half-applied update is left behind
- Validate the rendered fast-path config in memory before writing to disk
- Only schedule a background sync after a successful update (manager.ts)
- Treat empty-object update values as leaves and skip undefined values so the
  agent prompt matches override() semantics
- Detect agent completion via "result" in message (matches the CLI)
- Accept the export { config } specifier form in the structural check
- Add importAttributes to parseStackConfigFileContent for parser consistency
- Drop the unsafe Record<string,string> cast in stripClaudeCodeEnv
- Add a bounded, configurable timeout to the agent run

Co-Authored-By: mantra <mantra@stack-auth.com>

apps/dashboard/src/lib/remote-development-environment/config-file.test.ts

@@ -305,4 +305,29 @@ describe("remote development environment config file", () => {
     await expect(updateConfigObject(configPath, { "auth.allowSignUp": false }))
       .rejects.toThrow(/validation failed/);
   });
+
+  it("rolls back the config and its referenced files when the agent's result fails validation", async () => {
+    const templatePath = writeTempFile("welcome-email.tsx", "export default <div>Old email</div>;\n");
+    const configSource = `import welcomeEmail from "./welcome-email.tsx" with { type: "text" };\n\nexport const config = {\n  emails: { templates: { welcome: welcomeEmail } },\n};\n`;
+    const configPath = writeTempConfig(configSource);
+
+    const { updateConfigObject } = await import("./config-file");
+
+    // The agent edits both files but then fails, so the partially-applied edits
+    // must be rolled back and the failure surfaced.
+    mockAgentImpl = () => {
+      writeFileSync(templatePath, "export default <div>Corrupted</div>;\n", "utf-8");
+      writeFileSync(configPath, `export const config = { auth: { allowSignUp: true } };\n`, "utf-8");
+      throw new Error("agent blew up");
+    };
+
+    await expect(updateConfigObject(configPath, {
+      "emails.templates.welcome": "export default <div>New email</div>;\n",
+    })).rejects.toThrow("agent blew up");
+
+    // Both the config file and the externally-referenced file are back to their
+    // original contents \u2014 no half-applied update is left behind.
+    expect(readFileSync(configPath, "utf-8")).toBe(configSource);
+    expect(readFileSync(templatePath, "utf-8")).toBe("export default <div>Old email</div>;\n");
+  });
 });

apps/dashboard/src/lib/remote-development-environment/config-file.ts

@@ -3,9 +3,9 @@ import "server-only";
 import { showOnboardingStackConfigValue } from "@hexclave/shared/dist/config-authoring";
 import { Config, ConfigValue, NormalizedConfig, isValidConfig, normalize, override } from "@hexclave/shared/dist/config/format";
 import { detectImportPackageFromDir, renderConfigFileContent } from "@hexclave/shared/dist/config-rendering";
-import { stackConfigFileExportsConfig, tryParseStackConfigFileContent } from "@hexclave/shared/dist/stack-config-file";
+import { getRelativeImportSpecifiers, stackConfigFileExportsConfig, tryParseStackConfigFileContent } from "@hexclave/shared/dist/stack-config-file";
 import { createHash } from "crypto";
-import { existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from "fs";
+import { existsSync, mkdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "fs";
 import { createJiti } from "jiti";
 import path from "path";
 import { runConfigUpdateAgent } from "./config-update-agent";
@@ -88,23 +88,31 @@ export async function readConfigFile(configFilePath: string): Promise<{ config:
 }
 
 /**
- * Deterministically renders a config object into the file, overwriting whatever
- * was there. This is the canonical, lossy representation (a single
- * `export const config = { ...JSON... }`); it does not preserve imports, helper
- * wrappers, comments, or external file references. Only use it when there is no
- * existing structure to preserve (a brand-new/empty file, or a file that is
- * already a plain static literal). Otherwise use {@link updateConfigObject}.
+ * Renders a config object to its canonical, lossy source text (a single
+ * `export const config = { ...JSON... }`). It does not preserve imports, helper
+ * wrappers, comments, or external file references, so only render this for a
+ * file that has no existing structure to preserve (a brand-new/empty file, or a
+ * plain static literal). Otherwise use {@link updateConfigObject}.
  */
-function renderConfigObjectToFile(configFilePath: string, config: Config): void {
+function renderConfigObjectToString(configFilePath: string, config: Config): string {
+  const importPackage = detectImportPackageFromDir(path.dirname(configFilePath));
+  return renderConfigFileContent(config, importPackage);
+}
+
+/** Writes `content` to `configFilePath` atomically (write to temp, then rename). */
+function writeFileAtomic(configFilePath: string, content: string): void {
   const dir = path.dirname(configFilePath);
   mkdirSync(dir, { recursive: true });
-  const importPackage = detectImportPackageFromDir(dir);
-  const content = renderConfigFileContent(config, importPackage);
   const tempPath = path.join(dir, `.stack.config.${Math.random().toString(36).slice(2)}.tmp`);
   writeFileSync(tempPath, content, "utf-8");
   renameSync(tempPath, configFilePath);
 }
 
+/** Renders `config` to its canonical source text and writes it to disk, overwriting the file. */
+function renderConfigObjectToFile(configFilePath: string, config: Config): void {
+  writeFileAtomic(configFilePath, renderConfigObjectToString(configFilePath, config));
+}
+
 /**
  * Applies a config update to the file at `configFilePath`, merging `configUpdate`
  * (a partial config, possibly using dot-notation keys) over the current config.
@@ -139,11 +147,14 @@ export async function updateConfigObject(configFilePath: string, configUpdate: C
       throw new Error(`Config in ${configFilePath} parsed to a static literal that is not a valid config object.`);
     }
     const target = override(current, configUpdate);
-    renderConfigObjectToFile(configFilePath, target);
-    const written = tryParseStackConfigFileContent(readFileSync(configFilePath, "utf-8"), configFilePath);
+    // Validate the exact bytes we are about to write *before* touching the file,
+    // so a bad render can never leave the user's config in a broken state.
+    const rendered = renderConfigObjectToString(configFilePath, target);
+    const written = tryParseStackConfigFileContent(rendered, configFilePath);
     if (written == null || written === showOnboardingStackConfigValue || !isValidConfig(written) || !configsEqual(canonicalizeConfig(written), canonicalizeConfig(target))) {
       throw new Error(`Config update validation failed for ${configFilePath}: the regenerated file does not match the expected configuration.`);
     }
+    writeFileAtomic(configFilePath, rendered);
     return;
   }
 
@@ -151,12 +162,50 @@ export async function updateConfigObject(configFilePath: string, configUpdate: C
   // full semantic check afterwards), then let the agent apply the change.
   const baselineConfig = await tryReadConfigForValidation(configFilePath);
 
-  await runConfigUpdateAgent({
-    prompt: buildConfigUpdatePrompt(path.basename(configFilePath), configUpdate),
-    cwd: path.dirname(configFilePath),
-  });
+  // Snapshot the config file and any externally-referenced files the agent might
+  // edit, so we can roll back to the original state if the agent fails or its
+  // result doesn't validate — never leaving a half-applied update behind.
+  const snapshots = snapshotConfigFiles(configFilePath, content);
+  try {
+    await runConfigUpdateAgent({
+      prompt: buildConfigUpdatePrompt(path.basename(configFilePath), configUpdate),
+      cwd: path.dirname(configFilePath),
+    });
+    await validateAgentUpdate(configFilePath, baselineConfig, configUpdate);
+  } catch (error) {
+    restoreConfigFiles(snapshots);
+    throw error;
+  }
+}
 
-  await validateAgentUpdate(configFilePath, baselineConfig, configUpdate);
+/** A captured file state used for rollback. `content` is `null` if the file did not exist. */
+type ConfigFileSnapshot = { path: string, content: string | null };
+
+/**
+ * Captures the config file plus every file it imports via a relative path (e.g.
+ * `import x from "./welcome-email.tsx" with { type: "text" }`), which are exactly
+ * the files an in-place update is expected to touch. Files are read as UTF-8
+ * text, which matches this feature's text-import use case.
+ */
+function snapshotConfigFiles(configFilePath: string, configContent: string): ConfigFileSnapshot[] {
+  const dir = path.dirname(configFilePath);
+  const snapshots: ConfigFileSnapshot[] = [{ path: configFilePath, content: configContent }];
+  for (const specifier of getRelativeImportSpecifiers(configContent)) {
+    const resolved = path.resolve(dir, specifier);
+    if (snapshots.some((snapshot) => snapshot.path === resolved)) continue;
+    snapshots.push({ path: resolved, content: existsSync(resolved) ? readFileSync(resolved, "utf-8") : null });
+  }
+  return snapshots;
+}
+
+function restoreConfigFiles(snapshots: ConfigFileSnapshot[]): void {
+  for (const { path: filePath, content } of snapshots) {
+    if (content === null) {
+      if (existsSync(filePath)) rmSync(filePath);
+    } else {
+      writeFileSync(filePath, content, "utf-8");
+    }
+  }
 }
 
 /**
@@ -196,20 +245,27 @@ async function validateAgentUpdate(configFilePath: string, baselineConfig: Confi
   }
 }
 
-type ConfigChange = { path: string, value: ConfigValue | undefined };
+type ConfigChange = { path: string, value: ConfigValue };
 
 /**
  * Flattens a (possibly dot-notation) config update into individual leaf changes,
- * so the agent gets an explicit list of which config paths to change. Arrays and
- * primitives are leaves; nested plain objects are walked. A `value` of
- * `undefined` denotes a field that should be removed.
+ * so the agent gets an explicit list of which config paths to set. Arrays,
+ * primitives, and empty objects are leaves; nested non-empty plain objects are
+ * walked.
+ *
+ * `undefined` values are skipped, matching `override` (which filters them out as
+ * a no-op rather than treating them as removals); emitting removals here would
+ * diverge from the configuration the update is validated against.
  */
 function flattenConfigUpdate(update: Config): ConfigChange[] {
   const changes: ConfigChange[] = [];
   const walk = (prefix: string, obj: Config): void => {
     for (const [key, value] of Object.entries(obj)) {
       const fullPath = prefix === "" ? key : `${prefix}.${key}`;
-      if (value !== null && typeof value === "object" && !Array.isArray(value)) {
+      if (value === undefined) continue;
+      // An empty object is a leaf value (an explicit `{}`); only recurse into
+      // objects that actually have keys.
+      if (value !== null && typeof value === "object" && !Array.isArray(value) && Object.keys(value).length > 0) {
         walk(fullPath, value);
       } else {
         changes.push({ path: fullPath, value });
@@ -223,9 +279,6 @@ function flattenConfigUpdate(update: Config): ConfigChange[] {
 function buildConfigUpdatePrompt(configFileName: string, configUpdate: Config): string {
   const changes = flattenConfigUpdate(configUpdate);
   const changeLines = changes.map(({ path: configPath, value }) => {
-    if (value === undefined) {
-      return `- \`${configPath}\`: (remove this field)`;
-    }
     return `- \`${configPath}\`: set to ${JSON.stringify(value)}`;
   }).join("\n");
 
@@ -246,7 +299,6 @@ Rules:
 - Change ONLY the config paths listed above. Leave every other part of the file byte-for-byte unchanged: imports, comments, formatting, helper wrappers, and any config fields not listed.
 - If a listed path's value is currently provided by an imported external file (like the \`import ... with { type: "text" }\` example above), DO NOT inline the new value into the config file. Instead, overwrite that external file with the new value and keep the import statement intact.
 - If a listed path's value is a plain inline literal, edit it inline.
-- For a path marked "(remove this field)", delete that field from the config.
 - Keep the file valid: it must still export a \`config\` that, once evaluated, reflects the new values exactly.
 - Do not run any shell commands and do not create files other than what is required to apply these changes.`;
 }

apps/dashboard/src/lib/remote-development-environment/config-update-agent.ts

@@ -19,13 +19,27 @@ const ANTHROPIC_PROXY_BASE_URL: string = process.env.STACK_CLAUDE_PROXY_URL ?? D
 
 const LOG_PREFIX = "[Stack RDE]";
 
-function stripClaudeCodeEnv(): Record<string, string> {
+// Upper bound on how long the agent may run before we abort it, so a stuck or
+// runaway agent can't hang the dashboard's config-update request forever.
+// Overridable for slow environments via STACK_CONFIG_UPDATE_AGENT_TIMEOUT_MS.
+const DEFAULT_AGENT_TIMEOUT_MS = 120_000;
+const AGENT_TIMEOUT_MS: number = (() => {
+  const raw = process.env.STACK_CONFIG_UPDATE_AGENT_TIMEOUT_MS;
+  if (raw == null || raw.trim() === "") return DEFAULT_AGENT_TIMEOUT_MS;
+  const parsed = Number(raw);
+  if (!Number.isFinite(parsed) || parsed <= 0) {
+    throw new Error(`Invalid STACK_CONFIG_UPDATE_AGENT_TIMEOUT_MS: ${JSON.stringify(raw)}. Expected a positive number of milliseconds.`);
+  }
+  return parsed;
+})();
+
+function stripClaudeCodeEnv(): Record<string, string | undefined> {
   const env = { ...process.env };
   // Removing CLAUDECODE prevents the SDK from detecting a nested agent. The
   // ANTHROPIC_API_KEY must be non-empty or users without Claude Code installed
   // hit a login error (it is ignored by the proxy).
   delete env.CLAUDECODE;
-  return env as Record<string, string>;
+  return env;
 }
 
 /**
@@ -38,25 +52,49 @@ export async function runConfigUpdateAgent(options: {
   prompt: string,
   cwd: string,
 }): Promise<void> {
-  for await (const message of query({
-    prompt: options.prompt,
-    options: {
-      // Bash is intentionally omitted: applying a config delta only needs file
-      // inspection and editing, and withholding shell access reduces the blast
-      // radius of running an agent against the user's project.
-      allowedTools: ["Read", "Write", "Edit", "Glob", "Grep"],
-      permissionMode: "dontAsk",
-      cwd: options.cwd,
-      env: {
-        ...stripClaudeCodeEnv(),
-        ANTHROPIC_BASE_URL: ANTHROPIC_PROXY_BASE_URL,
-        ANTHROPIC_API_KEY: "stack-auth-proxy",
+  const abortController = new AbortController();
+  const timeout = setTimeout(() => abortController.abort(), AGENT_TIMEOUT_MS);
+  // Marks whether we observed a terminal "result" message from the SDK. The
+  // stream can otherwise end without one (e.g. the process dies), which we must
+  // treat as a failure rather than silently succeeding.
+  let sawResult = false;
+  try {
+    for await (const message of query({
+      prompt: options.prompt,
+      options: {
+        // Bash is intentionally omitted: applying a config delta only needs file
+        // inspection and editing, and withholding shell access reduces the blast
+        // radius of running an agent against the user's project.
+        allowedTools: ["Read", "Write", "Edit", "Glob", "Grep"],
+        permissionMode: "dontAsk",
+        cwd: options.cwd,
+        abortController,
+        env: {
+          ...stripClaudeCodeEnv(),
+          ANTHROPIC_BASE_URL: ANTHROPIC_PROXY_BASE_URL,
+          ANTHROPIC_API_KEY: "stack-auth-proxy",
+        },
+        stderr: (data: string) => { console.warn(`${LOG_PREFIX} [agent] ${data}`); },
       },
-      stderr: (data: string) => { console.warn(`${LOG_PREFIX} [agent] ${data}`); },
-    },
-  })) {
-    if (message.type === "result" && (message.is_error || message.subtype !== "success")) {
-      throw new Error(`Config update agent failed (${message.subtype}). It was unable to apply the config changes to the file.`);
+    })) {
+      // Detect the terminal message via `"result" in message` to match the CLI
+      // (`runClaudeAgent`); only a successful result carries a `result` field,
+      // so any other terminal subtype is reported as a failure.
+      if ("result" in message) {
+        sawResult = true;
+      } else if (message.type === "result") {
+        throw new Error(`Config update agent failed (${message.subtype}). It was unable to apply the config changes to the file.`);
+      }
     }
+  } catch (error) {
+    if (abortController.signal.aborted) {
+      throw new Error(`Config update agent timed out after ${AGENT_TIMEOUT_MS}ms. It was unable to apply the config changes to the file.`);
+    }
+    throw error;
+  } finally {
+    clearTimeout(timeout);
+  }
+  if (!sawResult) {
+    throw new Error("Config update agent ended without reporting a result. It was unable to apply the config changes to the file.");
   }
 }

apps/dashboard/src/lib/remote-development-environment/manager.ts

@@ -651,16 +651,19 @@ export async function applyRemoteDevelopmentEnvironmentConfigUpdate(options: {
     // The membership is held until the update resolves and then cleared after a
     // debounce so the file-change events our own edits produce are ignored too.
     state.synchronouslyUpdatingConfigFiles.add(configFilePath);
+    let updateSucceeded = false;
     try {
       await updateConfigObject(configFilePath, options.configUpdate);
+      updateSucceeded = true;
     } finally {
       setTimeout(() => {
         state.synchronouslyUpdatingConfigFiles.delete(configFilePath);
         // For fire-and-forget updates the sync is scheduled only after the
         // suppression window clears, otherwise scheduleSync would be swallowed
         // by its own guard while the path is still marked as synchronously
-        // updating.
-        if (options.waitForSync === false) {
+        // updating. Only sync a successful update: a failed one is rolled back,
+        // so there is nothing new to push.
+        if (options.waitForSync === false && updateSucceeded) {
           scheduleSync(configFilePath);
         }
       }, SYNC_DEBOUNCE_MS).unref();

packages/stack-shared/src/stack-config-file.ts

@@ -126,23 +126,66 @@ export function stackConfigFileExportsConfig(content: string, filePath: string):
     return false;
   }
   for (const statement of ast.program.body) {
-    if (!t.isExportNamedDeclaration(statement) || !t.isVariableDeclaration(statement.declaration)) {
+    if (!t.isExportNamedDeclaration(statement)) {
       continue;
     }
-    for (const declaration of statement.declaration.declarations) {
-      if (t.isIdentifier(declaration.id) && declaration.id.name === "config" && declaration.init != null) {
-        return true;
+    // `export const config = ...`
+    if (t.isVariableDeclaration(statement.declaration)) {
+      for (const declaration of statement.declaration.declarations) {
+        if (t.isIdentifier(declaration.id) && declaration.id.name === "config" && declaration.init != null) {
+          return true;
+        }
+      }
+    }
+    // `export { config }` / `export { somethingElse as config }`
+    for (const specifier of statement.specifiers) {
+      if (t.isExportSpecifier(specifier)) {
+        const exportedName = t.isIdentifier(specifier.exported) ? specifier.exported.name : specifier.exported.value;
+        if (exportedName === "config") {
+          return true;
+        }
       }
     }
   }
   return false;
 }
 
+/**
+ * Returns the relative import sources (those starting with `./` or `../`)
+ * declared in `content`. Used to discover the external files a config update may
+ * touch — e.g. `import x from "./welcome-email.tsx" with { type: "text" }` — so
+ * they can be snapshotted and rolled back if an in-place update fails. Returns
+ * an empty array if the file can't be parsed.
+ */
+export function getRelativeImportSpecifiers(content: string): string[] {
+  let ast: parser.ParseResult<t.File>;
+  try {
+    ast = parser.parse(content, {
+      sourceType: "module",
+      plugins: ["typescript", "importAttributes"],
+    });
+  } catch {
+    return [];
+  }
+  const sources: string[] = [];
+  for (const statement of ast.program.body) {
+    if (t.isImportDeclaration(statement)) {
+      const source = statement.source.value;
+      if (source.startsWith("./") || source.startsWith("../")) {
+        sources.push(source);
+      }
+    }
+  }
+  return sources;
+}
+
 export function parseStackConfigFileContent(content: string, filePath: string): ParsedStackConfig {
   if (content.trim() === "") return {};
   const ast = parser.parse(content, {
     sourceType: "module",
-    plugins: ["typescript"],
+    // `importAttributes` matches `stackConfigFileExportsConfig` so both parsers
+    // accept the same files (e.g. `import x from "./f.txt" with { type: "text" }`).
+    plugins: ["typescript", "importAttributes"],
   });
 
   for (const statement of ast.program.body) {