mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-13 13:22:35 -07:00
Exclude extension context from skill extraction agent (#26879)
This commit is contained in:
@@ -4199,6 +4199,49 @@ describe('LocalAgentExecutor', () => {
|
||||
expect(memoryPart).toBeDefined();
|
||||
expect(memoryPart?.text).toContain(mockMemory);
|
||||
});
|
||||
|
||||
it('should omit extension context from session memory when disabled by the agent', async () => {
|
||||
const definition = createTestDefinition();
|
||||
definition.includeExtensionContext = false;
|
||||
const executor = await LocalAgentExecutor.create(
|
||||
definition,
|
||||
mockConfig,
|
||||
onActivity,
|
||||
);
|
||||
|
||||
const getSessionMemorySpy = vi
|
||||
.spyOn(mockConfig, 'getSessionMemory')
|
||||
.mockImplementation(
|
||||
(options?: { includeExtensionContext?: boolean }) =>
|
||||
options?.includeExtensionContext === false
|
||||
? '<loaded_context>\n<project_context>\nProject memory rule\n</project_context>\n</loaded_context>'
|
||||
: '<loaded_context>\n<extension_context>\nExtension memory rule\n</extension_context>\n<project_context>\nProject memory rule\n</project_context>\n</loaded_context>',
|
||||
);
|
||||
vi.spyOn(mockConfig, 'isJitContextEnabled').mockReturnValue(true);
|
||||
|
||||
mockModelResponse([
|
||||
{
|
||||
name: COMPLETE_TASK_TOOL_NAME,
|
||||
args: { finalResult: 'done' },
|
||||
id: 'call1',
|
||||
},
|
||||
]);
|
||||
|
||||
await executor.run({ goal: 'test' }, signal);
|
||||
|
||||
expect(getSessionMemorySpy).toHaveBeenCalledWith({
|
||||
includeExtensionContext: false,
|
||||
});
|
||||
const { message } = getMockMessageParams(0);
|
||||
const parts = message as Part[];
|
||||
const memoryPart = parts.find((p) =>
|
||||
p.text?.includes('<loaded_context>'),
|
||||
);
|
||||
|
||||
expect(memoryPart?.text).toContain('Project memory rule');
|
||||
expect(memoryPart?.text).not.toContain('<extension_context>');
|
||||
expect(memoryPart?.text).not.toContain('Extension memory rule');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -640,10 +640,19 @@ export class LocalAgentExecutor<TOutput extends z.ZodTypeAny> {
|
||||
);
|
||||
const formattedInitialHints = formatUserHintsForModel(initialHints);
|
||||
|
||||
// Inject loaded memory files (JIT + extension/project memory)
|
||||
const environmentMemory = this.context.config.isJitContextEnabled?.()
|
||||
? this.context.config.getSessionMemory()
|
||||
: this.context.config.getEnvironmentMemory();
|
||||
// Inject loaded memory files. Some background agents opt out of
|
||||
// extension memory while still retaining project session context.
|
||||
let environmentMemory: string;
|
||||
if (this.context.config.isJitContextEnabled?.()) {
|
||||
environmentMemory =
|
||||
this.definition.includeExtensionContext === false
|
||||
? this.context.config.getSessionMemory({
|
||||
includeExtensionContext: false,
|
||||
})
|
||||
: this.context.config.getSessionMemory();
|
||||
} else {
|
||||
environmentMemory = this.context.config.getEnvironmentMemory();
|
||||
}
|
||||
|
||||
const initialParts: Part[] = [];
|
||||
if (environmentMemory) {
|
||||
|
||||
@@ -37,6 +37,7 @@ describe('SkillExtractionAgent', () => {
|
||||
expect(agent.modelConfig.model).toBe(PREVIEW_GEMINI_FLASH_MODEL);
|
||||
expect(agent.memoryInboxAccess).toBe(true);
|
||||
expect(agent.autoMemoryExtractionWriteAccess).toBe(true);
|
||||
expect(agent.includeExtensionContext).toBe(false);
|
||||
expect(agent.toolConfig?.tools).toEqual(
|
||||
expect.arrayContaining([
|
||||
READ_FILE_TOOL_NAME,
|
||||
|
||||
@@ -415,6 +415,7 @@ export const SkillExtractionAgent = (
|
||||
},
|
||||
memoryInboxAccess: true,
|
||||
autoMemoryExtractionWriteAccess: true,
|
||||
includeExtensionContext: false,
|
||||
toolConfig: {
|
||||
tools: [
|
||||
ACTIVATE_SKILL_TOOL_NAME,
|
||||
|
||||
@@ -244,6 +244,12 @@ export interface LocalAgentDefinition<
|
||||
*/
|
||||
autoMemoryExtractionWriteAccess?: boolean;
|
||||
|
||||
/**
|
||||
* Controls whether extension memory is injected into this agent's initial
|
||||
* session context when JIT context is enabled. Defaults to true.
|
||||
*/
|
||||
includeExtensionContext?: boolean;
|
||||
|
||||
/**
|
||||
* Optional inline MCP servers for this agent.
|
||||
*/
|
||||
|
||||
@@ -3525,6 +3525,16 @@ describe('Config JIT Initialization', () => {
|
||||
expect(sessionMemory).toContain('</project_context>');
|
||||
expect(sessionMemory).toContain('</loaded_context>');
|
||||
|
||||
const sessionMemoryWithoutExtension = config.getSessionMemory({
|
||||
includeExtensionContext: false,
|
||||
});
|
||||
expect(sessionMemoryWithoutExtension).toContain('<loaded_context>');
|
||||
expect(sessionMemoryWithoutExtension).not.toContain('<extension_context>');
|
||||
expect(sessionMemoryWithoutExtension).not.toContain('Extension Memory');
|
||||
expect(sessionMemoryWithoutExtension).toContain('<project_context>');
|
||||
expect(sessionMemoryWithoutExtension).toContain('Environment Memory');
|
||||
expect(sessionMemoryWithoutExtension).toContain('</loaded_context>');
|
||||
|
||||
// Verify state update (delegated to MemoryContextManager)
|
||||
expect(config.getGeminiMdFileCount()).toBe(1);
|
||||
expect(config.getGeminiMdFilePaths()).toEqual(['/path/to/GEMINI.md']);
|
||||
@@ -3746,6 +3756,8 @@ describe('Config JIT Initialization', () => {
|
||||
expect(config.isPathAllowed(privateExtractionPatch)).toBe(true);
|
||||
expect(config.validatePathAccess(privateExtractionPatch)).toBeNull();
|
||||
expect(config.isPathAllowed(globalExtractionPatch)).toBe(true);
|
||||
// Writes (the default checkType for isPathAllowed) remain restricted
|
||||
// to the canonical extraction.patch filenames.
|
||||
expect(
|
||||
config.isPathAllowed(path.join(inboxRoot, 'private', 'other.patch')),
|
||||
).toBe(false);
|
||||
@@ -3754,9 +3766,49 @@ describe('Config JIT Initialization', () => {
|
||||
path.join(inboxRoot, 'private', 'nested', 'extraction.patch'),
|
||||
),
|
||||
).toBe(false);
|
||||
|
||||
// Reads are broadened to the .inbox/{private,global}/ subtree so the
|
||||
// extractor can list and inspect prior patches before consolidating.
|
||||
const privateOtherPatch = path.join(
|
||||
inboxRoot,
|
||||
'private',
|
||||
'other.patch',
|
||||
);
|
||||
const globalLeftover = path.join(inboxRoot, 'global', 'topic-a.patch');
|
||||
const nestedReadPath = path.join(
|
||||
inboxRoot,
|
||||
'private',
|
||||
'nested',
|
||||
'extraction.patch',
|
||||
);
|
||||
expect(config.validatePathAccess(privateOtherPatch, 'read')).toBeNull();
|
||||
expect(config.validatePathAccess(globalLeftover, 'read')).toBeNull();
|
||||
expect(config.validatePathAccess(nestedReadPath, 'read')).toBeNull();
|
||||
expect(config.validatePathAccess(inboxRoot, 'read')).toBeNull();
|
||||
expect(
|
||||
config.validatePathAccess(path.join(inboxRoot, 'private'), 'read'),
|
||||
).toBeNull();
|
||||
expect(
|
||||
config.validatePathAccess(path.join(inboxRoot, 'global'), 'read'),
|
||||
).toBeNull();
|
||||
|
||||
// Writes to the same broadened paths are still rejected.
|
||||
expect(config.validatePathAccess(privateOtherPatch)).toContain(
|
||||
'Path not in workspace',
|
||||
);
|
||||
expect(config.validatePathAccess(nestedReadPath)).toContain(
|
||||
'Path not in workspace',
|
||||
);
|
||||
});
|
||||
|
||||
expect(config.isPathAllowed(privateExtractionPatch)).toBe(false);
|
||||
// Outside the scope, reads of inbox files are denied again.
|
||||
expect(
|
||||
config.validatePathAccess(
|
||||
path.join(inboxRoot, 'private', 'other.patch'),
|
||||
'read',
|
||||
),
|
||||
).toContain('Path not in workspace');
|
||||
});
|
||||
|
||||
it('should restrict scoped auto-memory extraction writes to generated artifacts', () => {
|
||||
|
||||
@@ -2511,12 +2511,15 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
* user message when JIT is enabled. Returns empty string when JIT is
|
||||
* disabled (Tier 2 memory is already in the system instruction).
|
||||
*/
|
||||
getSessionMemory(): string {
|
||||
getSessionMemory(options?: { includeExtensionContext?: boolean }): string {
|
||||
if (!this.experimentalJitContext || !this.memoryContextManager) {
|
||||
return '';
|
||||
}
|
||||
const sections: string[] = [];
|
||||
const extension = this.memoryContextManager.getExtensionMemory();
|
||||
const includeExtensionContext = options?.includeExtensionContext ?? true;
|
||||
const extension = includeExtensionContext
|
||||
? this.memoryContextManager.getExtensionMemory()
|
||||
: '';
|
||||
const project = this.memoryContextManager.getEnvironmentMemory();
|
||||
if (extension?.trim()) {
|
||||
sections.push(
|
||||
@@ -3088,12 +3091,49 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
absolutePath: string,
|
||||
resolvedPath: string,
|
||||
inboxRoot: string,
|
||||
checkType: 'read' | 'write' = 'write',
|
||||
): boolean {
|
||||
if (!hasScopedMemoryInboxAccess()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const normalizedPath = path.resolve(absolutePath);
|
||||
const resolvedMemoryRoot = resolveToRealPath(
|
||||
this.storage.getProjectMemoryTempDir(),
|
||||
);
|
||||
|
||||
// Reads: allow the inbox root and the per-kind subtrees so the extraction
|
||||
// agent can list/inspect prior patches (including non-canonical filenames
|
||||
// left over from older runs) before deciding how to rewrite the canonical
|
||||
// extraction.patch. Writes still flow through the strict canonical-path
|
||||
// check below so the inbox cannot be backdoored with arbitrary files.
|
||||
if (checkType === 'read') {
|
||||
const resolvedInboxRoot = resolveToRealPath(inboxRoot);
|
||||
const normalizedInboxRoot = path.resolve(inboxRoot);
|
||||
if (
|
||||
resolvedPath === resolvedInboxRoot ||
|
||||
normalizedPath === normalizedInboxRoot
|
||||
) {
|
||||
return isSubpath(resolvedMemoryRoot, resolvedPath);
|
||||
}
|
||||
|
||||
for (const kind of ['private', 'global'] as const) {
|
||||
const kindRoot = path.join(inboxRoot, kind);
|
||||
const resolvedKindRoot = resolveToRealPath(kindRoot);
|
||||
const normalizedKindRoot = path.resolve(kindRoot);
|
||||
if (
|
||||
resolvedPath === resolvedKindRoot ||
|
||||
normalizedPath === normalizedKindRoot ||
|
||||
isSubpath(resolvedKindRoot, resolvedPath) ||
|
||||
isSubpath(normalizedKindRoot, normalizedPath)
|
||||
) {
|
||||
return isSubpath(resolvedMemoryRoot, resolvedPath);
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
const isCanonicalPatchPath = (['private', 'global'] as const).some(
|
||||
(kind) =>
|
||||
normalizedPath === path.resolve(inboxRoot, kind, 'extraction.patch'),
|
||||
@@ -3102,9 +3142,6 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
return false;
|
||||
}
|
||||
|
||||
const resolvedMemoryRoot = resolveToRealPath(
|
||||
this.storage.getProjectMemoryTempDir(),
|
||||
);
|
||||
return isSubpath(resolvedMemoryRoot, resolvedPath);
|
||||
}
|
||||
|
||||
@@ -3148,7 +3185,9 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
* the auto-memory extraction agent and the `/memory inbox` review flow. The
|
||||
* main agent is denied access to it even though it falls inside the project
|
||||
* temp dir; the extraction agent receives a narrow execution-scoped exception
|
||||
* for `.inbox/{private,global}/extraction.patch`.
|
||||
* for *writes* to `.inbox/{private,global}/extraction.patch`. Scoped *read*
|
||||
* access to the wider `.inbox/{private,global}/` subtree is granted in
|
||||
* `validatePathAccess` so the extractor can enumerate prior patches.
|
||||
*
|
||||
* @param absolutePath The absolute path to check.
|
||||
* @returns true if the path is allowed, false otherwise.
|
||||
@@ -3243,6 +3282,28 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
if (this.getWorkspaceContext().isPathReadable(absolutePath)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// The memory inbox is carved out of the standard temp-dir allowlist by
|
||||
// `isPathAllowed`. The extraction agent is granted a scoped read
|
||||
// exception so it can enumerate prior patches (including non-canonical
|
||||
// filenames) before consolidating them into the canonical
|
||||
// extraction.patch. Writes remain restricted to canonical paths.
|
||||
if (hasScopedMemoryInboxAccess()) {
|
||||
const inboxRoot = path.join(
|
||||
this.storage.getProjectMemoryTempDir(),
|
||||
'.inbox',
|
||||
);
|
||||
if (
|
||||
this.isScopedMemoryInboxPatchPathAllowed(
|
||||
absolutePath,
|
||||
resolveToRealPath(absolutePath),
|
||||
inboxRoot,
|
||||
'read',
|
||||
)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Then check standard allowed paths (Workspace + Temp)
|
||||
|
||||
Reference in New Issue
Block a user