🛃 fix: Screen persistent agent files under both current policies

Persistent agent context files are read inside primeResources, after the caller
has screened this turn's other files, so neither of the caller's checks reached
them.

Content policy was one of those checks. Current and deferred files are inspected
before their bytes can be sent anywhere; a persistent file was not, so a policy
that started refusing it after it was attached did not stop it being uploaded to
the Code API or RAG on the next tool call. It is now inspected the same way and
dropped rather than failing the turn, matching the deferred candidates, since it
was not attached by this request.

The endpoint total-size allowance was the other. Filtering the persistent set
from zero let a current attachment and a persistent file that each fit alone
exceed the limit once merged, which is the same defect the earlier two-set fix
addressed and the reason a per-request budget is the right unit. All three sets
now draw on one allowance. An earlier reply argued these files sit outside the
request's budget; that was wrong, because they are merged into the same delivery
and provisioning sets the limit exists to bound.
This commit is contained in:
Danny Avila 2026-08-31 19:09:12 -04:00
parent a6ddebb0af
commit b43acf91a4
4 changed files with 139 additions and 14 deletions

View file

@ -111,6 +111,20 @@ jest.mock('../resources', () => ({
}),
}));
jest.mock('../../middleware/modelBoundContent', () => {
const actual = jest.requireActual('../../middleware/modelBoundContent');
/* Real by default; a single test overrides it to stand in for a policy that started
* refusing a file after it was attached. */
return {
...actual,
assertModelBoundContent: jest.fn((...args: unknown[]) =>
(actual as { assertModelBoundContent: (...a: unknown[]) => void }).assertModelBoundContent(
...args,
),
),
};
});
import { initializeAgent } from '../initialize';
import { isFatalAgentInitializationError } from '../errors';
@ -2955,6 +2969,88 @@ describe('initializeAgent — code-generated file thread filter (regression)', (
expect(getUserCodeFiles).not.toHaveBeenCalled();
});
it('screens persistent agent files under the remaining size allowance and content policy', async () => {
/* These are read inside primeResources, so the caller never sees them. Both checks it
* applied to this turn's other files have to reach them through the callback. */
const { filterFilesByEndpointRuntimeConfig } = jest.requireMock('~/files') as {
filterFilesByEndpointRuntimeConfig: jest.Mock;
};
const { primeResources } = jest.requireMock('../resources') as { primeResources: jest.Mock };
const { agent, req, res, loadTools, db } = setupExecuteCodeAgent();
await initializeAgent(
{
req,
res,
agent,
loadTools,
endpointOption: { endpoint: EModelEndpoint.agents },
allowedProviders: new Set([Providers.OPENAI]),
isInitialAgent: true,
codeEnvAvailable: true,
},
db,
);
const screen = primeResources.mock.calls[0][0].screenPersistentFiles as (
files: unknown[],
) => unknown[];
expect(typeof screen).toBe('function');
filterFilesByEndpointRuntimeConfig.mockClear();
const persistent = [{ file_id: 'persistent-1', filename: 'notes.csv', bytes: 10 }];
filterFilesByEndpointRuntimeConfig.mockReturnValueOnce(persistent);
expect(screen(persistent)).toEqual(persistent);
expect(filterFilesByEndpointRuntimeConfig).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({ consumedBytes: expect.any(Number) }),
);
});
it('drops a persistent agent file the content policy now refuses', async () => {
const { filterFilesByEndpointRuntimeConfig } = jest.requireMock('~/files') as {
filterFilesByEndpointRuntimeConfig: jest.Mock;
};
const { primeResources } = jest.requireMock('../resources') as { primeResources: jest.Mock };
const { assertModelBoundContent } = jest.requireMock('../../middleware/modelBoundContent') as {
assertModelBoundContent: jest.Mock;
};
const { agent, req, res, loadTools, db } = setupExecuteCodeAgent();
await initializeAgent(
{
req,
res,
agent,
loadTools,
endpointOption: { endpoint: EModelEndpoint.agents },
allowedProviders: new Set([Providers.OPENAI]),
isInitialAgent: true,
codeEnvAvailable: true,
},
db,
);
const screen = primeResources.mock.calls[0][0].screenPersistentFiles as (
files: unknown[],
) => unknown[];
const persistent = [
{
file_id: 'blocked-1',
filename: 'secrets.bin',
bytes: 10,
type: 'application/octet-stream',
},
];
filterFilesByEndpointRuntimeConfig.mockReturnValueOnce(persistent);
assertModelBoundContent.mockImplementationOnce(() => {
throw new Error('content policy');
});
expect(screen(persistent)).toEqual([]);
});
it('finds deferred files from the conversation when no anchor is supplied', async () => {
/* The Responses API always continues via `previous_response_id` and passes a null
* parentMessageId, and chat completions may omit it. Without an anchor there is no

View file

@ -1323,12 +1323,41 @@ export async function initializeAgent(
loadCodeApiKey: db.loadCodeApiKey,
provisionCandidates: deferredProvisionFiles as unknown as TFile[],
legacyFileUploadUX,
filterByEndpointPolicy: (files) =>
filterFilesByEndpointRuntimeConfig(appConfig, {
screenPersistentFiles: (files) => {
/* Persistent agent files are read inside primeResources, so they miss both checks
* the caller already applied to this turn's other files. They face the same
* endpoint policy under the remainder of the one total-size allowance the current
* and deferred sets have already drawn on, and the same content policy, which can
* have changed since the file was attached. */
const committedBytes =
(currentFiles ?? []).reduce((sum, file) => sum + (file.bytes ?? 0), 0) +
deferredProvisionFiles.reduce((sum, file) => sum + (file.bytes ?? 0), 0);
const withinPolicy = filterFilesByEndpointRuntimeConfig(appConfig, {
files: files as unknown as IMongoFile[],
endpoint: agent.endpoint ?? '',
endpointType: endpointFileType,
}) as unknown as TFile[],
consumedBytes: committedBytes,
}) as unknown as TFile[];
/* Dropped rather than fatal, matching the deferred candidates: these were not
* attached by this request, so refusing the conversation over a historical record
* would be harsher than leaving it out. */
return withinPolicy.filter((file) => {
try {
assertModelBoundContent({
filters: appConfig?.filters,
files: [file] as unknown as IMongoFile[],
});
return true;
} catch (error) {
logger.warn(
`[initializeAgent] Skipping persistent agent file "${file.filename}" (${file.file_id}): content policy`,
error,
);
return false;
}
});
},
});
/**

View file

@ -97,7 +97,7 @@ describe('primeResources', () => {
});
});
describe('when the endpoint policy rejects a persistent context file', () => {
describe('when policy screening rejects a persistent context file', () => {
it('keeps it out of provisioning and out of attachments', async () => {
/* These files are read inside primeResources, so the caller never sees them to
* filter. A provider or policy change since they were attached must still stop
@ -128,7 +128,7 @@ describe('primeResources', () => {
tool_resources: { [EToolResources.context]: { file_ids: ['stale-context-file'] } },
agentId: 'agent_test',
enabledToolResources: new Set([EToolResources.execute_code, EToolResources.file_search]),
filterByEndpointPolicy: () => [],
screenPersistentFiles: () => [],
});
expect(result.provisionState).toBeUndefined();
@ -162,7 +162,7 @@ describe('primeResources', () => {
tool_resources: { [EToolResources.context]: { file_ids: ['live-context-file'] } },
agentId: 'agent_test',
enabledToolResources: new Set([EToolResources.execute_code, EToolResources.file_search]),
filterByEndpointPolicy: (files) => files,
screenPersistentFiles: (files) => files,
});
expect(result.provisionState?.codeEnvFiles.map((f) => f.file_id)).toEqual([

View file

@ -480,7 +480,7 @@ export const primeResources = async ({
loadCodeApiKey,
provisionCandidates,
legacyFileUploadUX,
filterByEndpointPolicy,
screenPersistentFiles,
}: {
req?: ServerRequest;
principal?: Pick<IUser, 'id' | 'role'>;
@ -503,11 +503,11 @@ export const primeResources = async ({
provisionCandidates?: Array<TFile>;
/** True when this endpoint still shows the explicit upload-destination chooser. */
legacyFileUploadUX?: boolean;
/** Applies the current endpoint's file policy. Persistent agent files are read here
* rather than by the caller, so the caller has no chance to filter them itself and
* a provider or policy change since they were attached would otherwise let their
* bytes reach the Code API or RAG. */
filterByEndpointPolicy?: (files: Array<TFile>) => Array<TFile>;
/** Applies the caller's endpoint and content policies. Persistent agent files are read
* here rather than by the caller, so the caller has no chance to screen them itself
* and a configuration or policy change since they were attached would otherwise let
* their bytes reach the model, the Code API or RAG. */
screenPersistentFiles?: (files: Array<TFile>) => Array<TFile>;
}): Promise<{
attachments: Array<TFile | undefined> | undefined;
requestAttachments: Array<TFile | undefined> | undefined;
@ -617,8 +617,8 @@ export const primeResources = async ({
});
}
if (filterByEndpointPolicy) {
persistedResourceFiles = filterByEndpointPolicy(persistedResourceFiles);
if (screenPersistentFiles) {
persistedResourceFiles = screenPersistentFiles(persistedResourceFiles);
}
}