refactor(agents): improve writeAgents with file-level tracking

src/cli/commands/agents/pull.ts

@@ -24,18 +24,14 @@ async function pullAgentsAction(): Promise<RunCommandResult> {
     },
   );
 
-  if (remoteAgents.items.length === 0) {
-    return { outroMessage: "No agents found on Base44" };
-  }
-
   const { written, deleted } = await runTask(
-    "Writing agent files",
+    "Syncing agent files",
     async () => {
       return await writeAgents(agentsDir, remoteAgents.items);
     },
     {
-      successMessage: "Agent files written successfully",
-      errorMessage: "Failed to write agent files",
+      successMessage: "Agent files synced successfully",
+      errorMessage: "Failed to sync agent files",
     },
   );
 
@@ -45,6 +41,9 @@ async function pullAgentsAction(): Promise<RunCommandResult> {
   if (deleted.length > 0) {
     log.warn(`Deleted: ${deleted.join(", ")}`);
   }
+  if (written.length === 0 && deleted.length === 0) {
+    log.info("All agents are already up to date");
+  }
 
   return {
     outroMessage: `Pulled ${remoteAgents.total} agents to ${agentsDir}`,

src/core/resources/agent/config.ts

@@ -1,6 +1,7 @@
 import { join } from "node:path";
+import { isDeepStrictEqual } from "node:util";
 import { globby } from "globby";
-import { SchemaValidationError } from "@/core/errors.js";
+import { InvalidInputError, SchemaValidationError } from "@/core/errors.js";
 import {
   CONFIG_FILE_EXTENSION,
   CONFIG_FILE_EXTENSION_GLOB,
@@ -14,22 +15,11 @@ import {
 import type { AgentConfig, AgentConfigApiResponse } from "./schema.js";
 import { AgentConfigSchema } from "./schema.js";
 
-/**
- * Convert an agent name to a filesystem-safe filename slug.
- * Lowercases, replaces non-alphanumeric characters with underscores,
- * and collapses consecutive underscores.
- */
-function toFileSlug(name: string): string {
-  return name
-    .toLowerCase()
-    .replace(/[^a-z0-9_]/g, "_")
-    .replace(/_+/g, "_")
-    .replace(/^_|_$/g, "");
-}
-
-async function readAgentFile(agentPath: string): Promise<AgentConfig> {
-  const parsed = await readJsonFile(agentPath);
-  const result = AgentConfigSchema.safeParse(parsed);
+async function readAgentFile(
+  agentPath: string,
+): Promise<{ data: AgentConfig; raw: unknown }> {
+  const raw = await readJsonFile(agentPath);
+  const result = AgentConfigSchema.safeParse(raw);
 
   if (!result.success) {
     throw new SchemaValidationError(
@@ -39,10 +29,16 @@ async function readAgentFile(agentPath: string): Promise<AgentConfig> {
     );
   }
 
-  return result.data;
+  return { data: result.data, raw };
 }
 
-export async function readAllAgents(agentsDir: string): Promise<AgentConfig[]> {
+interface AgentFileEntry {
+  data: AgentConfig;
+  raw: unknown;
+  filePath: string;
+}
+
+async function readAgentFiles(agentsDir: string): Promise<AgentFileEntry[]> {
   if (!(await pathExists(agentsDir))) {
     return [];
   }
@@ -52,48 +48,96 @@ export async function readAllAgents(agentsDir: string): Promise<AgentConfig[]> {
     absolute: true,
   });
 
-  const agents = await Promise.all(
-    files.map((filePath) => readAgentFile(filePath)),
+  return await Promise.all(
+    files.map(async (filePath) => {
+      const { data, raw } = await readAgentFile(filePath);
+      return { data, raw, filePath };
+    }),
   );
+}
 
-  const names = new Set<string>();
-  for (const agent of agents) {
-    if (names.has(agent.name)) {
-      throw new Error(`Duplicate agent name "${agent.name}"`);
+function buildNameToEntryMap(
+  entries: AgentFileEntry[],
+): Map<string, AgentFileEntry> {
+  const nameToEntry = new Map<string, AgentFileEntry>();
+  for (const entry of entries) {
+    if (nameToEntry.has(entry.data.name)) {
+      throw new InvalidInputError(`Duplicate agent name "${entry.data.name}"`, {
+        hints: [
+          {
+            message: `Remove duplicate agents with name "${entry.data.name}" - only one agent per name is allowed`,
+          },
+        ],
+      });
     }
-    names.add(agent.name);
+    nameToEntry.set(entry.data.name, entry);
   }
+  return nameToEntry;
+}
 
-  return agents;
+export async function readAllAgents(agentsDir: string): Promise<AgentConfig[]> {
+  const entries = await readAgentFiles(agentsDir);
+  const nameToEntry = buildNameToEntryMap(entries);
+  return [...nameToEntry.values()].map((e) => e.data);
+}
+
+function findAvailablePath(
+  agentsDir: string,
+  name: string,
+  claimedPaths: Set<string>,
+): string {
+  const base = join(agentsDir, `${name}.${CONFIG_FILE_EXTENSION}`);
+  if (!claimedPaths.has(base)) {
+    return base;
+  }
+  for (let i = 1; ; i++) {
+    const candidate = join(agentsDir, `${name}_${i}.${CONFIG_FILE_EXTENSION}`);
+    if (!claimedPaths.has(candidate)) {
+      return candidate;
+    }
+  }
 }
 
 export async function writeAgents(
   agentsDir: string,
   remoteAgents: AgentConfigApiResponse[],
 ): Promise<{ written: string[]; deleted: string[] }> {
-  const existingAgents = await readAllAgents(agentsDir);
+  const entries = await readAgentFiles(agentsDir);
+  const nameToEntry = buildNameToEntryMap(entries);
+
   const newNames = new Set(remoteAgents.map((a) => a.name));
 
-  const toDelete = existingAgents.filter((a) => !newNames.has(a.name));
-  for (const agent of toDelete) {
-    const slug = toFileSlug(agent.name);
-    const files = await globby(`${slug}.${CONFIG_FILE_EXTENSION_GLOB}`, {
-      cwd: agentsDir,
-      absolute: true,
-    });
-    for (const filePath of files) {
-      await deleteFile(filePath);
+  const deleted: string[] = [];
+  for (const [name, entry] of nameToEntry) {
+    if (!newNames.has(name)) {
+      await deleteFile(entry.filePath);
+      deleted.push(name);
     }
   }
 
+  // Track all paths that are in use (existing files that weren't deleted)
+  const claimedPaths = new Set<string>();
+  for (const [name, entry] of nameToEntry) {
+    if (newNames.has(name)) {
+      claimedPaths.add(entry.filePath);
+    }
+  }
+
+  const written: string[] = [];
   for (const agent of remoteAgents) {
-    const slug = toFileSlug(agent.name);
-    const filePath = join(agentsDir, `${slug}.${CONFIG_FILE_EXTENSION}`);
+    const existing = nameToEntry.get(agent.name);
+
+    if (existing && isDeepStrictEqual(existing.raw, agent)) {
+      continue;
+    }
+
+    const filePath =
+      existing?.filePath ??
+      findAvailablePath(agentsDir, agent.name, claimedPaths);
+    claimedPaths.add(filePath);
     await writeJsonFile(filePath, agent);
+    written.push(agent.name);
   }
 
-  const written = remoteAgents.map((a) => a.name);
-  const deleted = toDelete.map((a) => a.name);
-
   return { written, deleted };
 }

tests/cli/agents_pull.spec.ts

@@ -1,4 +1,4 @@
-import { describe, it } from "vitest";
+import { describe, expect, it } from "vitest";
 import { fixture, setupCLITests } from "./testkit/index.js";
 
 describe("agents pull command", () => {
@@ -11,7 +11,7 @@ describe("agents pull command", () => {
     const result = await t.run("agents", "pull");
 
     t.expectResult(result).toSucceed();
-    t.expectResult(result).toContain("No agents found on Base44");
+    t.expectResult(result).toContain("All agents are already up to date");
   });
 
   it("fails when not in a project directory", async () => {
@@ -37,7 +37,7 @@ describe("agents pull command", () => {
 
     t.expectResult(result).toSucceed();
     t.expectResult(result).toContain("Agents fetched successfully");
-    t.expectResult(result).toContain("Agent files written successfully");
+    t.expectResult(result).toContain("Agent files synced successfully");
     t.expectResult(result).toContain("Pulled 2 agents");
   });
 
@@ -52,4 +52,111 @@ describe("agents pull command", () => {
 
     t.expectResult(result).toFail();
   });
+
+  it("writes agent files to disk after pull", async () => {
+    await t.givenLoggedInWithProject(fixture("basic"));
+    t.api.mockAgentsFetch({
+      items: [
+        {
+          name: "support_agent",
+          description: "Helps users",
+          instructions: "Be helpful",
+        },
+      ],
+      total: 1,
+    });
+
+    const result = await t.run("agents", "pull");
+
+    t.expectResult(result).toSucceed();
+
+    expect(await t.fileExists("base44/agents/support_agent.jsonc")).toBe(true);
+
+    const fileContent = await t.readProjectFile(
+      "base44/agents/support_agent.jsonc",
+    );
+    const parsed = JSON.parse(fileContent!);
+    expect(parsed.name).toBe("support_agent");
+    expect(parsed.description).toBe("Helps users");
+    expect(parsed.instructions).toBe("Be helpful");
+  });
+
+  it("skips unchanged agents and preserves file content", async () => {
+    await t.givenLoggedInWithProject(fixture("with-agents-for-pull"));
+    t.api.mockAgentsFetch({
+      items: [
+        {
+          name: "support_agent",
+          description: "Helps users",
+          instructions: "Be helpful",
+        },
+      ],
+      total: 1,
+    });
+
+    const result = await t.run("agents", "pull");
+
+    t.expectResult(result).toSucceed();
+    t.expectResult(result).toContain("All agents are already up to date");
+
+    // The file should not have been rewritten — JSONC comment must still be present
+    const fileContent = await t.readProjectFile(
+      "base44/agents/support_agent.jsonc",
+    );
+    expect(fileContent).toContain("// My support agent");
+  });
+
+  it("succeeds when a new remote agent's default filename clashes with an existing custom-named file", async () => {
+    // Local: custom_name.jsonc has name "support_agent" (custom filename)
+    // Remote returns support_agent (maps to custom_name.jsonc) AND custom_name
+    // (new agent whose default path custom_name.jsonc is already taken — overwrites)
+    await t.givenLoggedInWithProject(fixture("with-agents-naming-clash"));
+    t.api.mockAgentsFetch({
+      items: [
+        {
+          name: "support_agent",
+          description: "Helps users",
+          instructions: "Be helpful",
+        },
+        {
+          name: "custom_name",
+          description: "A new agent",
+          instructions: "Do new things",
+        },
+      ],
+      total: 2,
+    });
+
+    const result = await t.run("agents", "pull");
+
+    t.expectResult(result).toSucceed();
+  });
+
+  it("updates agent file in-place when remote data changes", async () => {
+    await t.givenLoggedInWithProject(fixture("with-agents-for-pull"));
+    t.api.mockAgentsFetch({
+      items: [
+        {
+          name: "support_agent",
+          description: "Updated description",
+          instructions: "Updated instructions",
+        },
+      ],
+      total: 1,
+    });
+
+    const result = await t.run("agents", "pull");
+
+    t.expectResult(result).toSucceed();
+
+    // File should be updated at the same path (in-place)
+    expect(await t.fileExists("base44/agents/support_agent.jsonc")).toBe(true);
+
+    const fileContent = await t.readProjectFile(
+      "base44/agents/support_agent.jsonc",
+    );
+    const parsed = JSON.parse(fileContent!);
+    expect(parsed.description).toBe("Updated description");
+    expect(parsed.instructions).toBe("Updated instructions");
+  });
 });