Skip to content
Open
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
2 changes: 2 additions & 0 deletions dev/src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ export function createProgram(): Command {
host: options['host'],
port: parseInt(options['port'], 10),
serveDebugUI: true,
installProcessHandlers: true,
allowOrigins: options['allow_origins'],
sessionService: getSessionServiceFromOptions(options),
artifactService: getArtifactServiceFromOptions(options),
Expand Down Expand Up @@ -285,6 +286,7 @@ export function createProgram(): Command {
host: options['host'],
port: parseInt(options['port'], 10),
serveDebugUI: false,
installProcessHandlers: true,
allowOrigins: options['allow_origins'],
sessionService: getSessionServiceFromOptions(options),
artifactService: getArtifactServiceFromOptions(options),
Expand Down
1 change: 1 addition & 0 deletions dev/src/cli/deploy/cli_deploy_agent_engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export async function deployToAgentEngine(options: DeployToAgentEngineOptions) {
options.agentPath,
options.agentFileLoadOptions,
);
agentLoader.installProcessHandlers();

const isFileProvided = await isFile(options.agentPath);
const agentDir = isFileProvided
Expand Down
1 change: 1 addition & 0 deletions dev/src/cli/deploy/cli_deploy_cloud_run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export async function deployToCloudRun(options: DeployToCloudRunOptions) {
options.agentPath,
options.agentFileLoadOptions,
);
agentLoader.installProcessHandlers();

const isFileProvided = await isFile(options.agentPath);
const agentDir = isFileProvided
Expand Down
11 changes: 11 additions & 0 deletions dev/src/server/adk_api_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ interface ServerOptions {
memoryService?: BaseMemoryService;
artifactService?: BaseArtifactService;
agentLoader?: AgentLoader;
/**
* Installs process exit and signal handlers on the agent loader. Only a CLI
* entrypoint that owns the process should enable this: the handlers call
* `process.exit()`. Defaults to false so an embedded server never mutates
* the host process.
*/
installProcessHandlers?: boolean;
agentFileLoadOptions?: AgentFileOptions;
serveDebugUI?: boolean;
allowOrigins?: string;
Expand Down Expand Up @@ -146,6 +153,10 @@ export class AdkApiServer {
this.a2aAuthToken =
options.a2aAuthToken || process.env[A2A_AUTH_TOKEN_ENV_VAR] || undefined;
this.app = express();

if (options.installProcessHandlers) {
this.agentLoader.installProcessHandlers();
}
}

private async setupTelemetry(): Promise<void> {
Expand Down
56 changes: 36 additions & 20 deletions dev/src/utils/agent_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ const FILE_MODULE_TYPE_EXTENSION_MAP = {
[FileModuleType.ESM]: '.mjs',
};

/**
* Termination signals wired up by {@link AgentLoader.installProcessHandlers}.
*/
const TERMINATION_SIGNALS = ['SIGINT', 'SIGUSR1', 'SIGUSR2'] as const;

/**
* Metadata for a file.
*/
Expand Down Expand Up @@ -361,34 +366,44 @@ export class AgentLoader {
private agentsAlreadyPreloaded = false;
private readonly preloadedAgents: Record<string, AgentFile> = {};
private watcher?: fs.FSWatcher;
private removeProcessHandlers?: () => void;

constructor(
private readonly agentsDirPath: string = process.cwd(),
private readonly options = DEFAULT_AGENT_FILE_OPTIONS,
private readonly watchForChanges = false,
) {
// Do cleanups on exit
const exitHandler = async ({
exit,
cleanup,
}: {
exit?: boolean;
cleanup?: boolean;
}) => {
if (cleanup) {
await this.disposeAll();
}
) {}

/**
* Wires process exit and termination signals to this loader's cleanup.
*
* This mutates global process state and terminates the process on a
* termination signal, so it is only appropriate for a CLI entrypoint that
* owns the process. Library and test consumers must instead call
* {@link disposeAll} when they are done. Calling this twice is a no-op, and
* {@link disposeAll} removes the listeners again.
*/
installProcessHandlers(): void {
if (this.removeProcessHandlers) {
return;
}

// An `exit` listener cannot await, so this cleanup is best-effort.
const onExit = () => void this.disposeAll();
const onSignal = () => process.exit();

if (exit) {
process.exit();
process.on('exit', onExit);
for (const signal of TERMINATION_SIGNALS) {
process.on(signal, onSignal);
}

this.removeProcessHandlers = () => {
process.removeListener('exit', onExit);
for (const signal of TERMINATION_SIGNALS) {
process.removeListener(signal, onSignal);
}
this.removeProcessHandlers = undefined;
};

process.on('exit', () => exitHandler({cleanup: true}));
process.on('SIGINT', () => exitHandler({exit: true}));
process.on('SIGUSR1', () => exitHandler({exit: true}));
process.on('SIGUSR2', () => exitHandler({exit: true}));
process.on('uncaughtException', () => exitHandler({exit: true}));
}

/**
Expand Down Expand Up @@ -471,6 +486,7 @@ export class AgentLoader {
}

async disposeAll(): Promise<void> {
this.removeProcessHandlers?.();
this.watcher?.close();
this.watcher = undefined;
await Promise.all(
Expand Down
9 changes: 9 additions & 0 deletions dev/test/cli/cli_deploy_agent_engine_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ vi.mock('../../src/utils/agent_loader.js', () => ({
getFilePath: vi.fn().mockReturnValue('path/to/agent1.ts'),
}),
disposeAll: vi.fn().mockResolvedValue(undefined),
installProcessHandlers: vi.fn(),
})),
}));

Expand Down Expand Up @@ -196,6 +197,7 @@ describe('deployToAgentEngine', () => {
getFilePath: vi.fn().mockReturnValue('path/to/agent1.ts'),
}),
disposeAll: vi.fn().mockResolvedValue(undefined),
installProcessHandlers: vi.fn(),
}));

execMock.mockImplementation((cmd: string, callback: Callback) => {
Expand Down Expand Up @@ -300,6 +302,13 @@ describe('deployToAgentEngine', () => {
expect(exists).toBe(false);
});

it('installs process handlers on the agent loader', async () => {
await deployToAgentEngine(defaultOptions);

const agentLoader = vi.mocked(AgentLoader).mock.results[0].value;
expect(agentLoader.installProcessHandlers).toHaveBeenCalledOnce();
});

it('should deploy successfully with all optional parameters', async () => {
const optionsWithAll: DeployToAgentEngineOptions = {
...defaultOptions,
Expand Down
9 changes: 9 additions & 0 deletions dev/test/cli/cli_deploy_cloud_run_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ vi.mock('../../src/utils/agent_loader.js', () => ({
getFilePath: vi.fn().mockReturnValue('path/to/agent1.ts'),
}),
disposeAll: vi.fn().mockResolvedValue(undefined),
installProcessHandlers: vi.fn(),
})),
}));

Expand Down Expand Up @@ -151,6 +152,7 @@ describe('deployToCloudRun', () => {
getFilePath: vi.fn().mockReturnValue('path/to/agent1.ts'),
}),
disposeAll: vi.fn().mockResolvedValue(undefined),
installProcessHandlers: vi.fn(),
}));

execMock.mockImplementation((cmd: string, callback: Callback) => {
Expand Down Expand Up @@ -198,6 +200,13 @@ describe('deployToCloudRun', () => {
});
});

it('installs process handlers on the agent loader', async () => {
await deployToCloudRun(defaultOptions);

const agentLoader = vi.mocked(AgentLoader).mock.results[0].value;
expect(agentLoader.installProcessHandlers).toHaveBeenCalledOnce();
});

it('should resolve default project and region from gcloud if not provided', async () => {
const optionsWithoutProjectRegion = {
...defaultOptions,
Expand Down
14 changes: 14 additions & 0 deletions dev/test/cli/cli_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ describe('CLI Entrypoint', () => {
const args = (AdkApiServer as unknown as Mock).mock.calls[0][0];
expect(args.a2aAuthToken).toBe('tok');
});

it('should opt the server into process handlers', async () => {
await parse(['web']);

const args = vi.mocked(AdkApiServer).mock.calls[0][0];
expect(args.installProcessHandlers).toBe(true);
});
});

describe('command: api_server', () => {
Expand Down Expand Up @@ -181,6 +188,13 @@ describe('CLI Entrypoint', () => {
const args = (AdkApiServer as unknown as Mock).mock.calls[0][0];
expect(args.a2aAuthToken).toBe('tok');
});

it('should opt the server into process handlers', async () => {
await parse(['api_server']);

const args = vi.mocked(AdkApiServer).mock.calls[0][0];
expect(args.installProcessHandlers).toBe(true);
});
});

describe('command: create', () => {
Expand Down
28 changes: 28 additions & 0 deletions dev/test/server/adk_api_server_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,4 +1255,32 @@ describe('AdkWebServer', () => {
}
});
});

describe('process handlers', () => {
it('does not install agent loader process handlers by default', () => {
const loader = new AgentLoader(process.cwd());
const installProcessHandlers = vi.spyOn(loader, 'installProcessHandlers');
const signalListeners = process.listenerCount('SIGINT');

new AdkApiServer({agentLoader: loader});

expect(installProcessHandlers).not.toHaveBeenCalled();
expect(process.listenerCount('SIGINT')).toBe(signalListeners);
});

it('installs agent loader process handlers when opted in', async () => {
const loader = new AgentLoader(process.cwd());
const installProcessHandlers = vi.spyOn(loader, 'installProcessHandlers');
const signalListeners = process.listenerCount('SIGINT');

try {
new AdkApiServer({agentLoader: loader, installProcessHandlers: true});

expect(installProcessHandlers).toHaveBeenCalledOnce();
expect(process.listenerCount('SIGINT')).toBe(signalListeners + 1);
} finally {
await loader.disposeAll();
}
});
});
});
Loading
Loading