Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 123 additions & 15 deletions src/web-ui/src/infrastructure/config/components/AcpAgentsConfig.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,6 @@ vi.mock('@openbitfun/ui', async importOriginal => ({
onChange?: React.ChangeEventHandler<HTMLInputElement>;
placeholder?: string;
}) => <input value={value} onChange={onChange} placeholder={placeholder} />,
Select: ({
value,
onChange,
options,
}: {
value?: string;
onChange?: (value: string) => void;
options?: Array<{ value: string; label: string }>;
}) => (
<select value={value} onChange={(event) => onChange?.(event.target.value)}>
{(options ?? []).map((option) => (
<option key={option.value} value={option.value}>{option.label}</option>
))}
</select>
),
TabGroup: ({
items,
onValueChange,
Expand Down Expand Up @@ -244,6 +229,19 @@ async function openView(container: HTMLElement, label: string): Promise<void> {
});
}

async function selectPermission(container: HTMLElement, value: string): Promise<void> {
const trigger = container.querySelector<HTMLButtonElement>(
'[data-openbitfun-part="confirmation"] button[role="combobox"]',
);
expect(trigger).not.toBeNull();
await act(async () => trigger!.click());
const options = Array.from(document.querySelectorAll<HTMLElement>('[role="option"]'));
expect(options.map(option => option.dataset.value)).toEqual(['ask', 'allow_once']);
const selected = options.find(option => option.dataset.value === value);
expect(selected).toBeTruthy();
await act(async () => selected!.click());
}

describe('AcpAgentsConfig', () => {
let container: HTMLDivElement;
let root: Root;
Expand Down Expand Up @@ -300,6 +298,116 @@ describe('AcpAgentsConfig', () => {
vi.clearAllMocks();
});

it.each([
['ask', 'ask'],
['allow_once', 'allow_once'],
])('loads and saves permission mode %s as %s with only supported choices', async (storedMode, expectedMode) => {
loadJsonConfigMock.mockResolvedValue(JSON.stringify({
acpClients: {
opencode: { command: 'opencode', args: ['acp'], permissionMode: storedMode },
},
}));

await act(async () => {
root.render(<AcpAgentsConfig />);
});

expect(container.textContent).not.toContain('permissionMode.legacyRejectWarning');
await selectPermission(container, expectedMode);
expect(saveJsonConfigMock).not.toHaveBeenCalled();

await openView(container, 'views.json');
const editor = container.querySelector<HTMLTextAreaElement>('textarea')!;
await act(async () => {
Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, 'value')?.set
?.call(editor, `${editor.value}\n`);
editor.dispatchEvent(new Event('input', { bubbles: true }));
});
const saveButton = Array.from(container.querySelectorAll('button'))
.find(button => button.textContent === 'actions.saveJson');
expect(saveButton?.disabled).toBe(false);
await act(async () => {
saveButton!.click();
});

const savedConfig = JSON.parse(saveJsonConfigMock.mock.calls[0][0]);
expect(savedConfig.acpClients.opencode).toMatchObject({
command: 'opencode', args: ['acp'], permissionMode: expectedMode,
});
});

it.each([
['local', true],
['ssh', true],
['json', true],
['local', false],
])('explicitly applies a legacy permission from %s (wrapped config: %s)', async (view, wrapped) => {
const legacyClient = {
command: 'opencode', args: ['acp'], env: { ACP_TEST: 'preserved' }, permissionMode: 'reject_once',
};
const otherClient = { command: 'custom-agent', args: [], permissionMode: 'allow_once' };
const acpClients = { opencode: legacyClient, custom: otherClient };
loadJsonConfigMock.mockResolvedValue(JSON.stringify(wrapped ? { acpClients } : acpClients));
saveJsonConfigMock.mockImplementation(async (rawConfig: string) => {
loadJsonConfigMock.mockResolvedValue(rawConfig);
window.dispatchEvent(new Event('openbitfun:acp-clients-changed'));
});

await act(async () => root.render(<AcpAgentsConfig />));
expect(container.querySelector('[data-openbitfun-part="confirmation"]')?.textContent)
.toContain('permissionMode.ask');
expect(container.querySelector('[role="alert"]')?.textContent)
.toContain('permissionMode.legacyRejectWarning');
expect(saveJsonConfigMock).not.toHaveBeenCalled();

// The real Select does not emit a change when Ask is already selected.
await selectPermission(container, 'ask');
expect(Array.from(container.querySelectorAll('button'))
.find(button => button.textContent === 'actions.save')).toBeUndefined();
if (view !== 'local') await openView(container, `views.${view}`);
expect(saveJsonConfigMock).not.toHaveBeenCalled();

const applyButton = Array.from(container.querySelectorAll('button'))
.find(button => button.textContent === 'permissionMode.saveAndApply');
expect(applyButton?.disabled).toBe(false);
await act(async () => applyButton!.click());

expect(saveJsonConfigMock).toHaveBeenCalledTimes(1);
const savedConfig = JSON.parse(saveJsonConfigMock.mock.calls[0][0]);
expect(savedConfig.acpClients.opencode).toMatchObject({ ...legacyClient, permissionMode: 'ask' });
expect(savedConfig.acpClients.custom).toMatchObject(otherClient);
expect(container.textContent).not.toContain('permissionMode.legacyRejectWarning');
expect(container.textContent).not.toContain('permissionMode.saveAndApply');

await act(async () => {
window.dispatchEvent(new Event('openbitfun:acp-clients-changed'));
});
expect(container.textContent).not.toContain('permissionMode.legacyRejectWarning');
expect(saveJsonConfigMock).toHaveBeenCalledTimes(1);
});

it('keeps the migration action after a failed save and applies the current selection on retry', async () => {
loadJsonConfigMock.mockResolvedValue(JSON.stringify({
acpClients: { opencode: { command: 'opencode', permissionMode: 'reject_once' } },
}));
saveJsonConfigMock.mockRejectedValueOnce(new Error('Save failed'));
await act(async () => root.render(<AcpAgentsConfig />));
await selectPermission(container, 'allow_once');
const applyButton = () => Array.from(container.querySelectorAll('button'))
.find(button => button.textContent === 'permissionMode.saveAndApply');

await act(async () => applyButton()!.click());
expect(notifyErrorMock).toHaveBeenCalledWith('Save failed', expect.anything());
expect(container.textContent).toContain('permissionMode.legacyRejectWarning');
expect(applyButton()?.disabled).toBe(false);

await act(async () => applyButton()!.click());
expect(saveJsonConfigMock).toHaveBeenCalledTimes(2);
expect(JSON.parse(saveJsonConfigMock.mock.calls[1][0]).acpClients.opencode.permissionMode)
.toBe('allow_once');
expect(container.textContent).not.toContain('permissionMode.legacyRejectWarning');
});

it('probes requirements when opened and does not treat missing probe data as invalid config', async () => {
await act(async () => {
root.render(<AcpAgentsConfig />);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { OverflowText,
Alert,
Button,
ConfirmDialog,
Icon,
Expand Down Expand Up @@ -210,7 +211,10 @@ function defaultConfigForPreset(preset: AcpClientPreset): AcpClientConfig {
};
}

function normalizeConfigValue(value: unknown): AcpClientConfigFile {
function normalizeConfigValue(value: unknown): {
config: AcpClientConfigFile;
hasLegacyPermissionModes: boolean;
} {
const candidate = value && typeof value === 'object' ? value as Record<string, unknown> : {};
const rawClients = (
candidate.acpClients && typeof candidate.acpClients === 'object' && !Array.isArray(candidate.acpClients)
Expand All @@ -219,6 +223,7 @@ function normalizeConfigValue(value: unknown): AcpClientConfigFile {
: candidate;

const acpClients: Record<string, AcpClientConfig> = {};
let hasLegacyPermissionModes = false;
for (const [id, rawConfig] of Object.entries(rawClients)) {
if (!rawConfig || typeof rawConfig !== 'object' || Array.isArray(rawConfig)) {
continue;
Expand All @@ -230,6 +235,7 @@ function normalizeConfigValue(value: unknown): AcpClientConfigFile {
continue;
}

hasLegacyPermissionModes ||= item.permissionMode === 'reject_once';
acpClients[id] = {
name: typeof item.name === 'string' ? item.name : undefined,
command,
Expand All @@ -242,7 +248,7 @@ function normalizeConfigValue(value: unknown): AcpClientConfigFile {
};
}

return { acpClients };
return { config: { acpClients }, hasLegacyPermissionModes };
}

function normalizeEnvObject(value: unknown): Record<string, string> {
Expand All @@ -253,7 +259,7 @@ function normalizeEnvObject(value: unknown): Record<string, string> {
}

function normalizePermissionMode(value: unknown): AcpClientPermissionMode {
return value === 'allow_once' || value === 'reject_once' ? value : 'ask';
return value === 'allow_once' ? value : 'ask';
Comment thread
wgqqqqq marked this conversation as resolved.
}

function normalizeSubagentConfig(value: unknown): AcpClientSubagentConfig {
Expand Down Expand Up @@ -483,6 +489,7 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({
const [loadFailed, setLoadFailed] = useState(false);
const [saving, setSaving] = useState(false);
const [dirty, setDirty] = useState(false);
const [pendingPermissionMigration, setPendingPermissionMigration] = useState(false);
const [jsonConfig, setJsonConfig] = useState('');
const [jsonBaseline, setJsonBaseline] = useState(formatConfig({ acpClients: {} }));
const [jsonDirty, setJsonDirty] = useState(false);
Expand Down Expand Up @@ -713,8 +720,9 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({
log.warn('Failed to load saved SSH connections for ACP remote overrides', error);
return [] as SavedConnection[];
});
const parsed = normalizeConfigValue(JSON.parse(rawConfig || '{}'));
const { config: parsed, hasLegacyPermissionModes } = normalizeConfigValue(JSON.parse(rawConfig || '{}'));
setConfig(parsed);
setPendingPermissionMigration(hasLegacyPermissionModes);
const formattedConfig = formatConfig(parsed);
setJsonConfig(formattedConfig);
setJsonBaseline(formattedConfig);
Expand Down Expand Up @@ -959,6 +967,7 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({
setJsonBaseline(formattedConfig);
setDirty(false);
setJsonDirty(false);
setPendingPermissionMigration(false);
await refreshRequirementProbes({ force: true, notifyOnError: false });
loadedRemoteProbeIdsRef.current.clear();
setRemoteProbeRefreshNonce(prev => prev + 1);
Expand Down Expand Up @@ -1010,7 +1019,7 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({

const saveJsonConfig = async (): Promise<boolean> => {
try {
const parsed = normalizeConfigValue(JSON.parse(jsonConfig));
const { config: parsed } = normalizeConfigValue(JSON.parse(jsonConfig));
const saved = await saveConfig(parsed, { mergeEnvDrafts: false });
if (!saved) return false;
setConfig(parsed);
Expand Down Expand Up @@ -1056,7 +1065,6 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({
const permissionOptions = useMemo(() => [
{ value: 'ask', label: t('permissionMode.ask') },
{ value: 'allow_once', label: t('permissionMode.allowOnce') },
{ value: 'reject_once', label: t('permissionMode.rejectOnce') },
], [t]);

const registryFilterOptions = useMemo(() => [
Expand Down Expand Up @@ -1348,6 +1356,23 @@ const AcpAgentsConfig: React.FC<AcpAgentsConfigProps> = ({
onValueChange={handleViewChange}
value={activeView}
/>
{pendingPermissionMigration && (
<Alert
tone="warning"
message={t('permissionMode.legacyRejectWarning')}
description={(
<Button
variant="fill"
size="sm"
disabled={saving}
loading={saving}
onClick={() => { void (activeView === 'json' ? saveJsonConfig() : saveConfig()); }}
>
{t('permissionMode.saveAndApply')}
</Button>
)}
/>
)}
{activeView === 'json' && (
<ConfigMessage message={{ type: 'warning', text: t('security.secretWarning') }} />
)}
Expand Down
3 changes: 2 additions & 1 deletion src/web-ui/src/locales/en-US/settings/acp-agents.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@
"permissionMode": {
"ask": "Ask",
"allowOnce": "Auto approve",
"rejectOnce": "Auto reject"
"legacyRejectWarning": "Some agents still have Auto reject saved. The editor replaces it with Ask. Save and apply your current choices to make them take effect; until then, those agents will continue to reject requests automatically.",
"saveAndApply": "Save and apply"
},
"requirements": {
"tool": "CLI",
Expand Down
3 changes: 2 additions & 1 deletion src/web-ui/src/locales/zh-CN/settings/acp-agents.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@
"permissionMode": {
"ask": "询问",
"allowOnce": "自动通过",
"rejectOnce": "自动拒绝"
"legacyRejectWarning": "部分 Agent 的已保存设置仍为“自动拒绝”,编辑器已将其替换为“询问”。请保存并应用当前选择;保存前,这些 Agent 仍会自动拒绝请求。",
"saveAndApply": "保存并应用"
},
"requirements": {
"tool": "CLI",
Expand Down
3 changes: 2 additions & 1 deletion src/web-ui/src/locales/zh-TW/settings/acp-agents.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@
"permissionMode": {
"ask": "詢問",
"allowOnce": "自動通過",
"rejectOnce": "自動拒絕"
"legacyRejectWarning": "部分 Agent 的已儲存設定仍為「自動拒絕」,編輯器已將其替換為「詢問」。請儲存並套用目前選擇;儲存前,這些 Agent 仍會自動拒絕請求。",
"saveAndApply": "儲存並套用"
},
"requirements": {
"tool": "CLI",
Expand Down
Loading