fix(agent-editor): stop dropping behavior_policy.supports_team

packages/desktop/src/common/types/platform/acpTypes.ts

@@ -15,7 +15,17 @@ import type { TConversationRuntimeSummary } from '@/common/config/storage';
 export interface CustomAgentAdvancedOverrides {
   yolo_id?: string;
   native_skills_dirs?: string[];
-  behavior_policy?: { supports_side_question?: boolean };
+  behavior_policy?: {
+    supports_side_question?: boolean;
+    /**
+     * Gates team-mode eligibility for this agent. Seeded `false` for custom
+     * agents and read by the backend, so a custom ACP agent that genuinely
+     * speaks MCP stdio cannot join a team unless this is set. The backend
+     * accepts it on `/api/agents/custom`; it was simply unreachable from the
+     * editor, which dropped the key while parsing the JSON panel.
+     */
+    supports_team?: boolean;
+  };
   description?: string;
 }
 

packages/desktop/src/renderer/pages/settings/AgentSettings/InlineAgentEditor.tsx

@@ -5,6 +5,7 @@
  */
 
 import type { CustomAgentAdvancedOverrides } from '@/common/types/platform/acpTypes';
+import { buildAdvancedJson, parseAdvancedOverrides } from './advancedOverrides';
 import type { AgentMetadata, ManagedAgent } from '@/renderer/utils/model/agentTypes';
 import { acpConversation, dialog, fs } from '@/common/adapter/ipcBridge';
 import { useAssistantList } from '@/renderer/hooks/assistant';
@@ -258,23 +259,15 @@ const InlineAgentEditor: React.FC<InlineAgentEditorProps> = ({ agent, onSave, on
     }
   }, [t]);
 
-  // Canonical empty shape shown when the user has not filled anything yet.
-  // Keep keys in sync with CustomAgentAdvancedOverrides.
-  const buildJsonFromAdvanced = useCallback((advancedVal: CustomAgentAdvancedOverrides) => {
-    const skeleton: CustomAgentAdvancedOverrides = {
-      yolo_id: advancedVal.yolo_id ?? '',
-      native_skills_dirs: advancedVal.native_skills_dirs ?? [],
-      behavior_policy: advancedVal.behavior_policy ?? { supports_side_question: false },
-      description: advancedVal.description ?? '',
-    };
-    return JSON.stringify(skeleton, null, 2);
-  }, []);
-
+  // Serialization lives in `advancedOverrides.ts` so the round-trip is unit-testable — a key
+  // silently dropped while parsing is indistinguishable, for the user, from one that was saved.
+  // Called directly rather than wrapped in `useCallback`: it is a module-level pure function, so it
+  // is already stable across renders.
   useEffect(() => {
     if (!isJsonEditingRef.current) {
-      setJsonInput(buildJsonFromAdvanced(advanced));
+      setJsonInput(buildAdvancedJson(advanced));
     }
-  }, [advanced, buildJsonFromAdvanced]);
+  }, [advanced]);
 
   useEffect(() => {
     setTestStatus('idle');
@@ -305,28 +298,14 @@ const InlineAgentEditor: React.FC<InlineAgentEditorProps> = ({ agent, onSave, on
     isJsonEditingRef.current = true;
     if (jsonEditTimerRef.current) clearTimeout(jsonEditTimerRef.current);
     setJsonInput(value);
-    try {
-      const parsed: unknown = JSON.parse(value);
-      setJsonError('');
-      if (parsed && typeof parsed === 'object') {
-        const next: CustomAgentAdvancedOverrides = {};
-        const p = parsed as Record<string, unknown>;
-        if (typeof p.yolo_id === 'string' && p.yolo_id.trim()) next.yolo_id = p.yolo_id;
-        if (Array.isArray(p.native_skills_dirs)) {
-          const dirs = p.native_skills_dirs.filter((x): x is string => typeof x === 'string');
-          if (dirs.length > 0) next.native_skills_dirs = dirs;
-        }
-        if (p.behavior_policy && typeof p.behavior_policy === 'object') {
-          const bp = p.behavior_policy as Record<string, unknown>;
-          if (typeof bp.supports_side_question === 'boolean') {
-            next.behavior_policy = { supports_side_question: bp.supports_side_question };
-          }
-        }
-        if (typeof p.description === 'string' && p.description.trim()) next.description = p.description;
-        setAdvanced(next);
-      }
-    } catch {
+    const result = parseAdvancedOverrides(value);
+    // `ignored` (valid JSON but not an object) clears the error and keeps the previous bag —
+    // behaviour preserved verbatim from the inline version this replaced.
+    if (result.kind === 'invalid') {
       setJsonError('Invalid JSON');
+    } else {
+      setJsonError('');
+      if (result.kind === 'ok') setAdvanced(result.value);
     }
     jsonEditTimerRef.current = setTimeout(() => {
       isJsonEditingRef.current = false;

packages/desktop/src/renderer/pages/settings/AgentSettings/advancedOverrides.ts

@@ -0,0 +1,93 @@
+/**
+ * @license
+ * Copyright 2025 AionUi (aionui.com)
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Serialization for the custom agent editor's advanced JSON panel.
+ *
+ * Extracted from `InlineAgentEditor` so the round-trip can be unit-tested: the panel accepts
+ * free-form JSON but only keeps a known set of keys, and a key silently dropped here is
+ * indistinguishable — to the user — from one that was saved.
+ */
+import type { CustomAgentAdvancedOverrides } from '@/common/types/platform/acpTypes';
+
+type BehaviorPolicy = NonNullable<CustomAgentAdvancedOverrides['behavior_policy']>;
+
+/**
+ * Canonical shape shown when the user has not filled anything yet.
+ *
+ * Every supported key appears, including the booleans at `false`: the panel is the only place these
+ * are documented, so an absent key reads as "unsupported" rather than "unset".
+ */
+export function buildAdvancedJson(advanced: CustomAgentAdvancedOverrides): string {
+  const skeleton: CustomAgentAdvancedOverrides = {
+    yolo_id: advanced.yolo_id ?? '',
+    native_skills_dirs: advanced.native_skills_dirs ?? [],
+    behavior_policy: advanced.behavior_policy ?? { supports_side_question: false, supports_team: false },
+    description: advanced.description ?? '',
+  };
+  return JSON.stringify(skeleton, null, 2);
+}
+
+/**
+ * Outcome of parsing the panel. The three cases are distinct on purpose:
+ *   - `invalid`  — not JSON at all; the editor shows "Invalid JSON";
+ *   - `ignored`  — valid JSON but not an object (`"s"`, `null`, a number); the editor clears the
+ *                  error and keeps the previous bag. Preserved verbatim from the original inline
+ *                  logic. Note an array is NOT ignored: `typeof [] === 'object'`, so it has always
+ *                  parsed as an object with no known keys, i.e. it empties the bag;
+ *   - `ok`       — a usable override bag.
+ */
+export type AdvancedParseResult =
+  | { kind: 'ignored' }
+  | { kind: 'invalid' }
+  | { kind: 'ok'; value: CustomAgentAdvancedOverrides };
+
+/**
+ * Parse the panel's contents into the override bag.
+ *
+ * Unknown keys are ignored on purpose (the bag maps onto specific backend columns), and empty
+ * values are omitted so an untouched panel does not send a payload of blanks.
+ */
+export function parseAdvancedOverrides(value: string): AdvancedParseResult {
+  let parsed: unknown;
+  try {
+    parsed = JSON.parse(value);
+  } catch {
+    return { kind: 'invalid' };
+  }
+  if (!parsed || typeof parsed !== 'object') return { kind: 'ignored' };
+
+  const p = parsed as Record<string, unknown>;
+  const next: CustomAgentAdvancedOverrides = {};
+
+  if (typeof p.yolo_id === 'string' && p.yolo_id.trim()) next.yolo_id = p.yolo_id;
+
+  if (Array.isArray(p.native_skills_dirs)) {
+    const dirs = p.native_skills_dirs.filter((x): x is string => typeof x === 'string');
+    if (dirs.length > 0) next.native_skills_dirs = dirs;
+  }
+
+  if (p.behavior_policy && typeof p.behavior_policy === 'object') {
+    const policy = parseBehaviorPolicy(p.behavior_policy as Record<string, unknown>);
+    if (policy) next.behavior_policy = policy;
+  }
+
+  if (typeof p.description === 'string' && p.description.trim()) next.description = p.description;
+
+  return { kind: 'ok', value: next };
+}
+
+/**
+ * Known policy flags, accumulated rather than assigned as a fresh object per key: the previous
+ * inline shape kept only `supports_side_question`, so `supports_team` was discarded even though the
+ * backend reads it and `/api/agents/custom` accepts it.
+ *
+ * `undefined` when no known flag is present, so an unrecognised policy does not send an empty object.
+ */
+function parseBehaviorPolicy(bp: Record<string, unknown>): BehaviorPolicy | undefined {
+  const policy: BehaviorPolicy = {};
+  if (typeof bp.supports_side_question === 'boolean') policy.supports_side_question = bp.supports_side_question;
+  if (typeof bp.supports_team === 'boolean') policy.supports_team = bp.supports_team;
+  return Object.keys(policy).length > 0 ? policy : undefined;
+}

packages/desktop/src/renderer/utils/model/agentTypes.ts

@@ -65,6 +65,13 @@ export type AgentEnvEntry = {
  */
 export type BehaviorPolicy = {
   supports_side_question?: boolean;
+  /**
+   * Gates team-mode eligibility. Persisted by the backend and seeded `false`
+   * for custom agents, so a custom ACP agent stays unselectable in a team
+   * until it is set — even when the agent advertises
+   * `agentCapabilities.mcpCapabilities.stdio: true` at init.
+   */
+  supports_team?: boolean;
 };
 
 /**

tests/unit/settings/advancedOverrides.test.ts

@@ -0,0 +1,99 @@
+import type { CustomAgentAdvancedOverrides } from '@/common/types/platform/acpTypes';
+import { buildAdvancedJson, parseAdvancedOverrides } from '@/renderer/pages/settings/AgentSettings/advancedOverrides';
+import { describe, expect, it } from 'vitest';
+
+/** The override bag for a parse that succeeded, or the outcome kind otherwise. */
+function bagOf(json: string): CustomAgentAdvancedOverrides | 'ignored' | 'invalid' {
+  const result = parseAdvancedOverrides(json);
+  return result.kind === 'ok' ? result.value : result.kind;
+}
+
+describe('parseAdvancedOverrides', () => {
+  // Regression: the panel accepted this JSON, showed no error, and dropped the key on save — so a
+  // custom ACP agent could never be made team-selectable from the UI.
+  it('keeps behavior_policy.supports_team', () => {
+    expect(bagOf('{"behavior_policy":{"supports_team":true}}')).toEqual({
+      behavior_policy: { supports_team: true },
+    });
+  });
+
+  it('keeps both policy flags without either clobbering the other', () => {
+    expect(bagOf('{"behavior_policy":{"supports_side_question":true,"supports_team":true}}')).toEqual({
+      behavior_policy: { supports_side_question: true, supports_team: true },
+    });
+  });
+
+  it('keeps supports_side_question alone (unchanged behaviour)', () => {
+    expect(bagOf('{"behavior_policy":{"supports_side_question":true}}')).toEqual({
+      behavior_policy: { supports_side_question: true },
+    });
+  });
+
+  it('preserves false, which is a meaningful value and not an absence', () => {
+    expect(bagOf('{"behavior_policy":{"supports_team":false}}')).toEqual({
+      behavior_policy: { supports_team: false },
+    });
+  });
+
+  it('ignores non-boolean flags rather than coercing them', () => {
+    expect(bagOf('{"behavior_policy":{"supports_team":"yes"}}')).toEqual({});
+  });
+
+  it('omits behavior_policy entirely when no known flag is present', () => {
+    expect(bagOf('{"behavior_policy":{"unknown_flag":true}}')).toEqual({});
+  });
+
+  it('keeps the other override keys', () => {
+    expect(bagOf('{"yolo_id":"y","native_skills_dirs":["/a"],"description":"d"}')).toEqual({
+      description: 'd',
+      native_skills_dirs: ['/a'],
+      yolo_id: 'y',
+    });
+  });
+
+  it('drops blank strings and empty arrays so an untouched panel sends nothing', () => {
+    expect(bagOf('{"yolo_id":"  ","native_skills_dirs":[],"description":""}')).toEqual({});
+  });
+
+  it('filters non-string entries out of native_skills_dirs', () => {
+    expect(bagOf('{"native_skills_dirs":["/a",1,null]}')).toEqual({ native_skills_dirs: ['/a'] });
+  });
+
+  it('reports invalid JSON so the editor can surface it', () => {
+    expect(bagOf('{oops')).toBe('invalid');
+  });
+
+  // Valid JSON that is neither object nor array clears the error and keeps the previous bag —
+  // behaviour preserved verbatim from the inline logic this replaced, hence a kind of its own.
+  it('reports non-object JSON as ignored, not invalid', () => {
+    expect(bagOf('"str"')).toBe('ignored');
+    expect(bagOf('null')).toBe('ignored');
+  });
+
+  // An array passes `typeof x === 'object'`, so it has always been read as an object with no known
+  // keys — i.e. it clears the bag. Documented rather than changed: this PR fixes a dropped key, not
+  // the panel's tolerance for odd input.
+  it('treats an array as an empty bag, as before', () => {
+    expect(bagOf('[]')).toEqual({});
+  });
+});
+
+describe('buildAdvancedJson', () => {
+  it('advertises supports_team in the skeleton — the panel is its only documentation', () => {
+    const skeleton = JSON.parse(buildAdvancedJson({})) as CustomAgentAdvancedOverrides;
+    expect(skeleton.behavior_policy).toEqual({ supports_side_question: false, supports_team: false });
+  });
+
+  // The panel is a serialize → edit → parse loop; a key surviving the skeleton but not the parse
+  // (the bug this suite guards) is precisely what the user cannot see.
+  it('round-trips a policy through build then parse', () => {
+    const advanced = { behavior_policy: { supports_side_question: true, supports_team: true } };
+    expect(bagOf(buildAdvancedJson(advanced))).toEqual(advanced);
+  });
+
+  it('round-trips the empty skeleton to the default policy', () => {
+    expect(bagOf(buildAdvancedJson({}))).toEqual({
+      behavior_policy: { supports_side_question: false, supports_team: false },
+    });
+  });
+});