fix(subagents): keep agent profile fields this app does not own

AGENTS.md

@@ -190,6 +190,8 @@ Newer pi emits `compaction_start` / `compaction_end`; older versions emitted `au
 - When enabled, only a recognized legacy `pi-subagents` extension that registers any reserved tool (`Agent`, `get_subagent_result`, or `steer_subagent`) is removed. Unrelated extensions remain loaded, and resolved conflict diagnostics are discarded.
 - Runtime `Agent` dispatch checks the setting again so a stale tool call cannot start a subagent after the feature is switched off.
 - See `docs/adr/0003-built-in-subagent-toggle.md` for the precedence and persistence rationale.
+- Agent profile files (`~/.pi/agent/agents/*.md`, project `.pi/agents/*.md`) are shared with other runtimes, so a save round-trips the frontmatter keys this app does not own (`name`, `allowed_subagents`, `exclude_extensions`, `disallowed_tools`, …) and carries foreign `ext:` tool selectors through. Managed keys are exactly `description`, `display_name`, `tools`, `load_skills`, `load_extensions`, `enabled`, `inherit_context`, `run_in_background`, `model`, `thinking`, `max_turns`.
+- The `skills` / `extensions` spellings pi-subagents reads are seeded on first save and kept in step while they are booleans; a hand-authored whitelist such as `extensions: pi-advisor-flow` is never rewritten, and the two flags fall back to those aliases when `load_skills` / `load_extensions` are absent.
 
 ### Auth and model config
 - `ModelsConfig` combines models from `~/.pi/agent/models.json` with provider auth status from pi's `AuthStorage`/`ModelRegistry`.

lib/subagents.test.mjs

@@ -493,3 +493,109 @@ test("project profile directories cannot escape cwd through symbolic links", asy
   );
   assert.match(await readFile(join(outside, "secret.md"), "utf8"), /private/);
 });
+
+test("a save keeps frontmatter keys this app does not manage", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "pi-web-subagents-"));
+  try {
+    await mkdir(join(cwd, ".pi", "agents"), { recursive: true });
+    const file = join(cwd, ".pi", "agents", "orchestrator.md");
+    await writeFile(
+      file,
+      [
+        "---",
+        "name: orchestrator",
+        "description: Hands out work",
+        "display_name: orchestrator",
+        "tools: read, bash, edit, write, grep, find, ls, ext:pi-advisor-flow/ask_advisor",
+        "skills: false",
+        "extensions: pi-advisor-flow",
+        "exclude_extensions: pi-advisor-flow",
+        "allowed_subagents: thinker, executor",
+        "disallowed_tools: write",
+        "enabled: true",
+        "inherit_context: false",
+        "run_in_background: false",
+        "---",
+        "Dispatch the work.",
+      ].join("\n"),
+    );
+
+    saveProjectSubagentProfile(cwd, profile({ name: "orchestrator", tools: ["read", "bash"] }));
+    const source = await readFile(file, "utf8");
+
+    assert.match(source, /^name: orchestrator$/m);
+    assert.match(source, /allowed_subagents: thinker, executor/);
+    assert.match(source, /exclude_extensions: pi-advisor-flow/);
+    assert.match(source, /disallowed_tools: write/);
+    assert.match(source, /skills: false/);
+    assert.match(source, /extensions: pi-advisor-flow/);
+    assert.match(source, /tools: read, bash, ext:pi-advisor-flow\/ask_advisor/);
+    assert.match(source, /Test prompt\./);
+
+    const loaded = listSubagentProfiles(cwd).find((item) => item.name === "orchestrator");
+    assert.deepEqual(loaded.tools, ["read", "bash"]);
+  } finally {
+    await rm(cwd, { recursive: true, force: true });
+  }
+});
+
+test("a save refuses to overwrite malformed existing frontmatter", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "pi-web-subagents-"));
+  try {
+    await mkdir(join(cwd, ".pi", "agents"), { recursive: true });
+    const file = join(cwd, ".pi", "agents", "malformed.md");
+    const source = "---\nallowed_subagents: [executor\n---\nKeep this file intact.\n";
+    await writeFile(file, source);
+
+    assert.throws(
+      () => saveProjectSubagentProfile(cwd, profile({ name: "malformed" })),
+      /existing frontmatter is invalid/,
+    );
+    assert.equal(await readFile(file, "utf8"), source);
+  } finally {
+    await rm(cwd, { recursive: true, force: true });
+  }
+});
+
+test("the pi-subagents flag aliases are seeded, kept in step, and never overwrite a whitelist", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "pi-web-subagents-"));
+  try {
+    const file = join(cwd, ".pi", "agents", "fresh.md");
+    saveProjectSubagentProfile(cwd, profile({ name: "fresh", loadSkills: true, loadExtensions: true }));
+    const seeded = await readFile(file, "utf8");
+    assert.match(seeded, /load_skills: true/);
+    assert.match(seeded, /skills: true/);
+    assert.match(seeded, /load_extensions: true/);
+    assert.match(seeded, /extensions: true/);
+
+    saveProjectSubagentProfile(cwd, profile({ name: "fresh", loadSkills: false, loadExtensions: false }));
+    const flipped = await readFile(file, "utf8");
+    assert.match(flipped, /skills: false/);
+    assert.match(flipped, /extensions: false/);
+
+    await mkdir(join(cwd, ".pi", "agents"), { recursive: true });
+    const scoped = join(cwd, ".pi", "agents", "scoped.md");
+    await writeFile(scoped, "---\ndescription: Scoped\nextensions: pi-advisor-flow\n---\nOnly the advisor.\n");
+    saveProjectSubagentProfile(cwd, profile({ name: "scoped", loadExtensions: true }));
+    assert.match(await readFile(scoped, "utf8"), /extensions: pi-advisor-flow/);
+  } finally {
+    await rm(cwd, { recursive: true, force: true });
+  }
+});
+
+test("profile flags fall back to the pi-subagents spellings", async () => {
+  const cwd = await mkdtemp(join(tmpdir(), "pi-web-subagents-"));
+  try {
+    await mkdir(join(cwd, ".pi", "agents"), { recursive: true });
+    await writeFile(
+      join(cwd, ".pi", "agents", "legacy-flags.md"),
+      "---\ndescription: Legacy flags\nskills: false\nextensions: pi-advisor-flow\n---\nScoped.\n",
+    );
+
+    const loaded = listSubagentProfiles(cwd).find((item) => item.name === "legacy-flags");
+    assert.equal(loaded.loadSkills, false);
+    assert.equal(loaded.loadExtensions, true);
+  } finally {
+    await rm(cwd, { recursive: true, force: true });
+  }
+});

lib/subagents.ts

@@ -111,6 +111,40 @@ const BUILTIN_TOOLS = new Set(DEFAULT_TOOLS);
 const SUBAGENT_CONTROL_TOOLS = new Set<string>(SUBAGENT_CONTROL_TOOL_NAMES);
 const THINKING_LEVELS = new Set<ThinkingLevel>(["off", "minimal", "low", "medium", "high", "xhigh", "max"]);
 
+/**
+ * Frontmatter keys the web UI owns. Everything else in a profile file belongs to
+ * whichever runtime reads it (pi-subagents and friends), so a save from this app must
+ * carry those keys through untouched. Dropping them silently changed behaviour:
+ * `allowed_subagents` was lost and an orchestrator could no longer spawn anything,
+ * `exclude_extensions` was lost and an opt-out became an opt-in.
+ */
+const MANAGED_FRONTMATTER_KEYS = new Set([
+  "description",
+  "display_name",
+  "tools",
+  "load_skills",
+  "load_extensions",
+  "enabled",
+  "inherit_context",
+  "run_in_background",
+  "model",
+  "thinking",
+  "max_turns",
+  "prompt_mode",
+  "color",
+  "isolation",
+  "persist_session",
+]);
+
+const FRONTMATTER_OPEN_RE = /^(?:\uFEFF)?---[ \t]*(?:\r\n|\n|\r)/;
+
+/**
+ * The UI exposes two booleans (`load_skills` / `load_extensions`); pi-subagents reads
+ * the aliases `skills` / `extensions`, which also accept a whitelist. Aliases are
+ * carried through by `unmanagedFrontmatter` and only rewritten once we own them.
+ */
+const OWNED_ALIAS_VALUES = new Set(["none", "all", "true", "false"]);
+
 const BUILTIN_PROFILES: SubagentProfile[] = [
   {
     name: "general-purpose",
@@ -169,28 +203,78 @@ function resourceBoolean(value: unknown, fallback: boolean): boolean {
   return Array.isArray(value) || typeof value === "string" ? true : fallback;
 }
 
-function parseTools(value: unknown, fallback: string[]): string[] {
+function stringList(value: unknown): string[] {
   const values = Array.isArray(value)
     ? value
     : typeof value === "string"
       ? value.split(",")
       : [];
-  const tools = values.map((item) => String(item).trim()).filter(Boolean);
+  return values.map((item) => String(item).trim()).filter(Boolean);
+}
+
+function parseTools(value: unknown, fallback: string[]): string[] {
+  const tools = stringList(value);
   if (tools.includes("none")) return [];
   if (tools.includes("all") || tools.includes("*")) return [...DEFAULT_TOOLS];
   if (tools.length === 0) return [...fallback];
   return [...new Set(tools.filter((tool) => BUILTIN_TOOLS.has(tool)))];
 }
 
 function rawToolValues(value: unknown): string[] {
-  const values = Array.isArray(value) ? value : typeof value === "string" ? value.split(",") : [];
-  return values.map((item) => String(item).trim()).filter(Boolean);
+  return stringList(value);
 }
 
 function parseExtensionToolSelectors(value: unknown): string[] {
   return [...new Set(rawToolValues(value).filter((tool) => tool.toLowerCase().startsWith("ext:")))];
 }
 
+/** Read existing frontmatter without allowing malformed metadata to be overwritten. */
+function readStoredFrontmatter(filePath: string): Record<string, unknown> {
+  if (!existsSync(filePath)) return {};
+  const source = readFileSync(filePath, "utf8");
+  const { data } = parseFrontmatter(source);
+  if (data) return data;
+  if (FRONTMATTER_OPEN_RE.test(source)) {
+    throw new Error("Cannot save agent profile: existing frontmatter is invalid");
+  }
+  return {};
+}
+
+/** Keys another runtime owns, in file order, so a save round-trips them. */
+function unmanagedFrontmatter(stored: Record<string, unknown>): Record<string, unknown> {
+  const preserved: Record<string, unknown> = {};
+  for (const [key, value] of Object.entries(stored)) {
+    if (!MANAGED_FRONTMATTER_KEYS.has(key)) preserved[key] = value;
+  }
+  return preserved;
+}
+
+/**
+ * pi-web filters `tools` down to the built-ins it can dispatch, which would drop
+ * another runtime's `ext:<name>` selectors on every save — carry them through.
+ */
+function composeToolsField(tools: string[], storedTools: unknown): string {
+  const selectors = stringList(storedTools).filter((tool) => tool.startsWith("ext:"));
+  const combined = [...tools, ...selectors.filter((selector) => !tools.includes(selector))];
+  return combined.length > 0 ? combined.join(", ") : "none";
+}
+
+/**
+ * Keep the alias in step with the boolean the UI owns. A boolean (or a "none" /
+ * "all" spelling) is ours to rewrite; a whitelist such as `extensions:
+ * pi-advisor-flow` expresses scoping the UI cannot show, so it stays as authored.
+ */
+function syncFlagAlias(
+  frontmatter: Record<string, unknown>,
+  alias: string,
+  storedValue: unknown,
+  flag: boolean,
+): void {
+  const owned = storedValue === undefined
+    || typeof storedValue === "boolean"
+    || (typeof storedValue === "string" && OWNED_ALIAS_VALUES.has(storedValue.trim().toLowerCase()));
+  if (owned) frontmatter[alias] = flag;
+}
 function parseProfileFile(filePath: string, scope: SubagentScope): SubagentProfile | null {
   try {
     const source = readFileSync(filePath, "utf8");
@@ -333,23 +417,31 @@ export function saveSubagentProfile(
     throw new Error("Agent profile directory is outside the project root");
   }
   const filePath = join(dir, `${name}.md`);
-  const frontmatter: Record<string, unknown> = {
+  const stored = readStoredFrontmatter(filePath);
+  const managed: Record<string, unknown> = {
     description,
     display_name: displayName,
-    tools: [...tools, ...extensionTools].length > 0 ? [...tools, ...extensionTools].join(", ") : "none",
+    tools: composeToolsField([...tools, ...extensionTools], stored.tools),
     load_skills: loadSkills,
     load_extensions: loadExtensions,
     enabled: profile.enabled,
     inherit_context: profile.inheritContext,
     run_in_background: profile.runInBackground,
     prompt_mode: promptMode,
   };
-  if (model) frontmatter.model = model;
-  if (profile.thinking) frontmatter.thinking = profile.thinking;
-  if (maxTurns) frontmatter.max_turns = maxTurns;
-  if (profile.color?.trim()) frontmatter.color = profile.color.trim();
-  if (profile.isolation) frontmatter.isolation = profile.isolation;
-  if (profile.persistSession !== undefined) frontmatter.persist_session = profile.persistSession;
+  syncFlagAlias(managed, "skills", stored.skills, loadSkills);
+  syncFlagAlias(managed, "extensions", stored.extensions, loadExtensions);
+  if (model) managed.model = model;
+  if (profile.thinking) managed.thinking = profile.thinking;
+  if (maxTurns) managed.max_turns = maxTurns;
+  if (profile.color?.trim()) managed.color = profile.color.trim();
+  if (profile.isolation) managed.isolation = profile.isolation;
+  if (profile.persistSession !== undefined) managed.persist_session = profile.persistSession;
+  // Managed keys win; keys this app does not own follow in their original order.
+  const frontmatter: Record<string, unknown> = { ...managed };
+  for (const [key, value] of Object.entries(unmanagedFrontmatter(stored))) {
+    if (!(key in frontmatter)) frontmatter[key] = value;
+  }
   const yaml = stringifyYaml(frontmatter, { noRefs: true, lineWidth: 1000 }).trimEnd();
   writePrivateFileAtomicSync(filePath, `---\n${yaml}\n---\n\n${systemPrompt}\n`);
   return {