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
7 changes: 7 additions & 0 deletions api/server/controllers/agents/__tests__/resume.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,13 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => {
await settled;

expect(capturedInit.isScheduledFire).toBe(true);
expect(mockInitializeClient.mock.calls[0][0].scheduledTokenContext).toEqual({
scheduleId: 'schedule-1',
ownerId: USER_ID,
tenantId: TENANT_ID,
agentId: AGENT_ID,
invocationMode: 'delegated',
});

expect(mockClaimScheduleResume).toHaveBeenCalledWith('schedule-1', scheduledFor, {
expectedConfigRevision: 4,
Expand Down
50 changes: 50 additions & 0 deletions api/server/controllers/agents/client.codeDecision.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
const AgentClient = require('./client');

describe('AgentClient code environment save options', () => {
const mac = { environmentId: 'code-mac', workspaceId: 'primary' };
const vm = { environmentId: 'code-vm', workspaceId: 'primary' };
const conversationId = 'conversation-1';

const buildClient = (resolvedConversation) => {
const client = Object.create(AgentClient.prototype);
client.options = {
req: {
body: { conversationId },
config: {},
resolvedConversation,
_codeEnvironmentDecision: { mode: 'attached', codeWorkspaces: [mac] },
},
endpoint: 'agents',
agent: { id: 'agent_1', provider: 'openai' },
};
return client;
};

it('recognizes an existing conversation before the client initializes its ID', () => {
const client = buildClient({
conversationId,
codeEnvironmentMode: 'attached',
codeWorkspaces: [vm],
});

expect(client.conversationId).toBeUndefined();
const initial = client.getSaveOptions();
expect(initial).not.toHaveProperty('codeEnvironmentMode');
expect(initial).not.toHaveProperty('codeWorkspaces');

client.conversationId = conversationId;
expect(client.getSaveOptions()).toEqual(initial);
});

it('seeds a new conversation and omits its decision after the row becomes available', () => {
const client = buildClient(null);
const initial = client.getSaveOptions();
expect(initial).toMatchObject({ codeEnvironmentMode: 'attached', codeWorkspaces: [mac] });

client.conversationId = conversationId;
client.options.req.resolvedConversation = { conversationId, ...initial };
const paused = client.getSaveOptions();
expect(paused).not.toHaveProperty('codeEnvironmentMode');
expect(paused).not.toHaveProperty('codeWorkspaces');
});
});
2 changes: 1 addition & 1 deletion api/server/controllers/agents/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1943,7 +1943,7 @@ class AgentClient extends BaseClient {
agentsEConfig?.toolApproval?.enabled !== false,
);
const persistedCodeEnvironmentDecision = resolvePersistableCodeEnvironmentDecision({
conversationId: this.conversationId,
conversationId: this.options.req.body.conversationId,
decision: this.options.req._codeEnvironmentDecision,
conversation: this.options.req.resolvedConversation,
requested: this.options.req.body,
Expand Down
4 changes: 2 additions & 2 deletions api/server/controllers/agents/client.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ describe('AgentClient code approval persistence', () => {
endpoint: EModelEndpoint.agents,
agent: { id: 'attached-agent' },
req: {
body: {},
body: { conversationId: 'convo-1' },
_codeEnvironmentDecision: {
mode: 'attached',
codeWorkspaces: [{ environmentId: 'mac', workspaceId: 'primary' }],
Expand Down Expand Up @@ -135,7 +135,7 @@ describe('AgentClient code approval persistence', () => {
endpoint: EModelEndpoint.agents,
agent: { id: 'attached-agent' },
req: {
body: {},
body: { conversationId: 'convo-1' },
_codeEnvironmentDecision: { mode: 'attached', codeWorkspaces },
resolvedConversation: { conversationId: 'convo-1', codeWorkspaces },
config: { endpoints: { [EModelEndpoint.agents]: {} } },
Expand Down
2 changes: 2 additions & 0 deletions api/server/controllers/agents/resume.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const {
findAgentEventAppliedAction,
assertCodeExecutionApprovalBinding,
collectReachableAgents,
restoreScheduledTokenContext,
} = require('@librechat/api');
const { disposeClient } = require('~/server/cleanup');
const { decryptMetadata } = require('~/server/services/ActionService');
Expand Down Expand Up @@ -1812,6 +1813,7 @@ const ResumeAgentController = async (req, res, next, initializeClient, addTitle)
parentMessageId: job.metadata.userMessage?.messageId ?? Constants.NO_PARENT,
});
const result = await initializeClient({
scheduledTokenContext: restoreScheduledTokenContext(req, job.metadata),
req,
res,
endpointOption: req.body.endpointOption,
Expand Down
3 changes: 2 additions & 1 deletion api/server/services/Endpoints/agents/initialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -1873,14 +1873,15 @@ const initializeClientWithProvider = async ({
* token material so refresh remains owned by the host integration.
*
* @param {object} [dependencies]
* @param {(user: import('@librechat/data-schemas').IUser, options: { signal?: AbortSignal }) => import('@librechat/api').UpstreamTokenProvider | undefined | Promise<import('@librechat/api').UpstreamTokenProvider | undefined>} [dependencies.resolveUpstreamTokenProvider]
* @param {import('@librechat/api').HostUpstreamTokenProviderResolver} [dependencies.resolveUpstreamTokenProvider]
*/
function createInitializeClient(dependencies = {}) {
return async (params) => {
const upstreamTokenProviderResolver = createScheduleUpstreamTokenProviderResolver(
params.req,
dependencies.resolveUpstreamTokenProvider,
params.signal,
params.scheduledTokenContext,
);
return initializeClientWithProvider({ ...params, upstreamTokenProviderResolver });
};
Expand Down
30 changes: 28 additions & 2 deletions api/server/services/Endpoints/agents/initialize.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,22 @@ describe('initializeClient — processAgent ACL gate', () => {
},
});

it('defers host credential resolution during scheduled agent initialization', async () => {
it.each([false, true])('defers host credential resolution with restored=%s', async (restored) => {
const upstreamTokenProvider = jest.fn();
const resolveUpstreamTokenProvider = jest.fn().mockResolvedValue(upstreamTokenProvider);
const hostInitializeClient = createInitializeClient({ resolveUpstreamTokenProvider });
const req = makeReq();
req._isScheduledFire = true;
req._isAgentTrigger = !restored;
req.body.agent_id = PRIMARY_ID;
req.body.agentTrigger = {
version: 1,
event: {
type: 'schedule.occurrence',
occurredAt: 0,
source: { type: 'schedule', id: 'sched-1' },
},
};
const signal = new AbortController().signal;
mockInitializeAgent.mockImplementationOnce(async ({ loadTools, agent }) => {
await loadTools({
Expand All @@ -203,6 +213,14 @@ describe('initializeClient — processAgent ACL gate', () => {
req,
res: {},
signal,
scheduledTokenContext: restored
? {
scheduleId: 'sched-1',
ownerId: req.user.id,
agentId: PRIMARY_ID,
invocationMode: 'delegated',
}
: undefined,
endpointOption: makeEndpointOption(),
});

Expand All @@ -211,7 +229,15 @@ describe('initializeClient — processAgent ACL gate', () => {
expect(toolLoadParams.upstreamTokenProvider).toBeUndefined();
const resolver = toolLoadParams.upstreamTokenProviderResolver;
await expect(resolver({ signal })).resolves.toBe(upstreamTokenProvider);
expect(resolveUpstreamTokenProvider).toHaveBeenCalledWith(req.user, { signal });
expect(resolveUpstreamTokenProvider).toHaveBeenCalledWith(req.user, {
signal,
context: {
scheduleId: 'sched-1',
ownerId: req.user.id,
agentId: PRIMARY_ID,
invocationMode: 'delegated',
},
});
});

it('keeps interactive agent initialization independent of the host resolver', async () => {
Expand Down
2 changes: 1 addition & 1 deletion api/server/services/Schedules/mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const methods = require('~/models');
* credential source and therefore remains fail-closed for unattended OBO.
*
* @param {object} [options]
* @param {(user: import('@librechat/data-schemas').IUser, options: { signal?: AbortSignal }) => import('@librechat/api').UpstreamTokenProvider | undefined | Promise<import('@librechat/api').UpstreamTokenProvider | undefined>} [options.resolveUpstreamTokenProvider]
* @param {import('@librechat/api').HostUpstreamTokenProviderResolver} [options.resolveUpstreamTokenProvider]
*/
function createMCPPreflight(options = {}) {
return createScheduleMCPPreflight({
Expand Down
7 changes: 6 additions & 1 deletion client/src/components/Chat/Input/CodeApprovalMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { CodeApprovalMode, TConversation } from 'librechat-data-provider';
import type { LucideIcon } from 'lucide-react';
import type { SetterOrUpdater } from 'recoil';
import type { TranslationKeys } from '~/hooks';
import { useCodeApprovalModePreference } from '~/hooks/Agents/codeApprovalPreference';
import { useCodeApprovalMode, useLocalize } from '~/hooks';
import { cn } from '~/utils';

Expand Down Expand Up @@ -51,6 +52,7 @@ export default function CodeApprovalMenu({
const localize = useLocalize();
const queryClient = useQueryClient();
const { available, modes, selected } = useCodeApprovalMode(conversation, addedConversation);
const preference = useCodeApprovalModePreference();
const menuStore = Ariakit.useMenuStore({ focusLoop: true, placement: 'top-start' });
const isOpen = menuStore.useState('open');

Expand All @@ -62,11 +64,14 @@ export default function CodeApprovalMenu({
* cache, so the pick lands there too, seeding the record from the live
* conversation when none exists yet (the resumable transport seeds the same
* key optimistically). A chat that has no id yet keeps the pick in
* conversation state alone until the run assigns one. */
* conversation state alone until the run assigns one. The pick is also this
* browser's remembered default, so the next chat opens on it rather than back
* at `ask`; policy is re-checked before it is ever shown or submitted. */
const selectMode = (mode: CodeApprovalMode) => {
if (!modes.includes(mode)) {
return;
}
preference.remember(mode);
setConversation((current) =>
current == null ? current : { ...current, codeApprovalMode: mode },
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import userEvent from '@testing-library/user-event';
import { QueryKeys, Constants } from 'librechat-data-provider';
import { render, screen, within } from '@testing-library/react';
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
import { QueryKeys, Constants, LocalStorageKeys } from 'librechat-data-provider';
import type { TConversation } from 'librechat-data-provider';
import CodeApprovalMenu from '../CodeApprovalMenu';

Expand Down Expand Up @@ -163,3 +163,28 @@ describe('CodeApprovalMenu', () => {
});
});
});

test("remembers the pick as this browser's default for the next chat", async () => {
localStorage.clear();
mockUseCodeApprovalMode.mockReturnValue({
available: true,
modes: ['ask', 'acceptEdits'],
selected: 'ask',
});
render(
<QueryClientProvider client={new QueryClient()}>
<CodeApprovalMenu
conversation={conversation}
setConversation={mockSetConversation}
disabled={false}
/>
</QueryClientProvider>,
);

await userEvent.click(screen.getByTestId('code-approval-mode'));
await userEvent.click(await screen.findByText('com_ui_code_approval_accept_edits'));

expect(localStorage.getItem(LocalStorageKeys.LAST_CODE_APPROVAL_MODE)).toBe(
JSON.stringify('acceptEdits'),
);
});
69 changes: 69 additions & 0 deletions client/src/hooks/Agents/__tests__/useCodeApprovalMode.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Provider } from 'jotai';
import { renderHook } from '@testing-library/react';
import { LocalStorageKeys } from 'librechat-data-provider';
import type { TConversation } from 'librechat-data-provider';
import useCodeApprovalMode from '../useCodeApprovalMode';

Expand Down Expand Up @@ -371,3 +373,70 @@ describe('useCodeApprovalMode', () => {
expect(result.current.selected).toBe('ask');
});
});

describe('useCodeApprovalMode remembered pick', () => {
const withoutMode = { ...conversation, codeApprovalMode: undefined } as TConversation;

beforeEach(() => {
jest.clearAllMocks();
localStorage.clear();
mockUseAgentsMapContext.mockReturnValue({});
mockUseAgentToolPermissions.mockReturnValue({
agent: {
id: 'agent_1',
tools: ['execute_code'],
stateful_code_sessions: true,
code_environment_id: 'mac',
},
});
mockUseGetAgentsConfig.mockReturnValue({
agentsConfig: {
statefulCodeSessions: {
approvalsEnabled: true,
approvalModes: ['ask', 'acceptEdits'],
environments: [
{
id: 'mac',
name: 'Mac',
type: 'attached',
configSchema: {
permissions: {
fileWrite: { allowed: ['ask', 'allow'], default: 'ask' },
commandExecution: { allowed: ['ask'], default: 'ask' },
},
},
},
],
},
},
});
});

test('opens a conversation with no stored mode on the last pick', () => {
localStorage.setItem(LocalStorageKeys.LAST_CODE_APPROVAL_MODE, JSON.stringify('acceptEdits'));
const { result } = renderHook(() => useCodeApprovalMode(withoutMode), { wrapper: Provider });
expect(result.current.selected).toBe('acceptEdits');
});

test('a stored conversation mode still wins over the remembered pick', () => {
localStorage.setItem(LocalStorageKeys.LAST_CODE_APPROVAL_MODE, JSON.stringify('acceptEdits'));
const { result } = renderHook(
() => useCodeApprovalMode({ ...conversation, codeApprovalMode: 'ask' } as TConversation),
{ wrapper: Provider },
);
expect(result.current.selected).toBe('ask');
});

test('falls back to ask when policy no longer allows the remembered pick', () => {
localStorage.setItem(LocalStorageKeys.LAST_CODE_APPROVAL_MODE, JSON.stringify('fullAccess'));
const { result } = renderHook(() => useCodeApprovalMode(withoutMode), { wrapper: Provider });
expect(result.current.modes).not.toContain('fullAccess');
expect(result.current.selected).toBe('ask');
});

test('ignores a corrupted remembered value', () => {
localStorage.setItem(LocalStorageKeys.LAST_CODE_APPROVAL_MODE, JSON.stringify('nonsense'));
const { result } = renderHook(() => useCodeApprovalMode(withoutMode), { wrapper: Provider });
expect(result.current.selected).toBe('ask');
});
});
33 changes: 33 additions & 0 deletions client/src/hooks/Agents/codeApprovalPreference.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { useAtom } from 'jotai';
import { CODE_APPROVAL_MODES, LocalStorageKeys } from 'librechat-data-provider';
import type { CodeApprovalMode } from 'librechat-data-provider';
import { createStorageAtom } from '~/store/jotai-utils';

const preference = createStorageAtom<string | null>(LocalStorageKeys.LAST_CODE_APPROVAL_MODE, null);

function validMode(saved: string | null): CodeApprovalMode | undefined {
return (CODE_APPROVAL_MODES as readonly string[]).includes(saved ?? '')
? (saved as CodeApprovalMode)
: undefined;
}

/**
* The mode the reader last picked in this browser, so a new chat opens the way
* they left the last one instead of falling back to `ask` every time. A
* disposable hint and never authorization: the caller re-checks it against the
* modes current policy allows before showing or submitting it, and logout
* clears it with the rest of this browser's conversation state.
*/
export function useCodeApprovalModePreference() {
const [saved, setSaved] = useAtom(preference);
return {
get: () => validMode(saved),
remember: (mode: CodeApprovalMode) => {
try {
setSaved(mode);
} catch {
// Disabled or full browser storage must not prevent an explicit pick.
}
},
};
}
Loading
Loading