Skip to content

Commit a158ffb

Browse files
authored
feat: make container dev mode language-agnostic (#500)
Remove Python/uvicorn-specific code from container dev mode. The container now simply builds the user's Dockerfile and runs it without hot reloading. Changes: - Remove dev layer that installed uvicorn; build image directly - Remove entrypoint/user overrides; use Dockerfile's CMD/ENTRYPOINT - Remove source volume mount (no hot reload) - Stream build output in real-time instead of blocking - Use async spawn for container stop so UI can render "Stopping..." - Add sessionId header and Accept: text/event-stream for invoke requests - Support {"text": "..."} SSE format from bedrock-agentcore runtime - Show "Stopping container..." instead of generic "Stopping server..."
1 parent 5f0695d commit a158ffb

4 files changed

Lines changed: 190 additions & 190 deletions

File tree

src/cli/operations/dev/__tests__/container-dev-server.test.ts

Lines changed: 96 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,24 @@ function createMockChildProcess() {
4040
return proc;
4141
}
4242

43+
/** Create a mock child process that auto-closes with the given exit code (for build). */
44+
function createMockBuildProcess(exitCode = 0, stdoutData?: string) {
45+
const proc = createMockChildProcess();
46+
// Emit 'close' after the listener is attached to guarantee correct ordering
47+
const origOn = proc.on.bind(proc);
48+
proc.on = function (event: string, fn: (...args: any[]) => void) {
49+
origOn(event, fn);
50+
if (event === 'close') {
51+
process.nextTick(() => {
52+
if (stdoutData) proc.stdout.emit('data', Buffer.from(stdoutData));
53+
proc.emit('close', exitCode);
54+
});
55+
}
56+
return proc;
57+
};
58+
return proc;
59+
}
60+
4361
function mockSuccessfulPrepare() {
4462
// Runtime detected
4563
mockDetectContainerRuntime.mockResolvedValue({
@@ -48,11 +66,12 @@ function mockSuccessfulPrepare() {
4866
});
4967
// Dockerfile exists (first call), ~/.aws exists (second call in getSpawnConfig)
5068
mockExistsSync.mockReturnValue(true);
51-
// rm, base build, dev build all succeed
69+
// rm succeeds (spawnSync)
5270
mockSpawnSync.mockReturnValue({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') });
53-
// spawn for the actual server
71+
// spawn: first call = build (auto-closes with 0), second call = server run
72+
const mockBuild = createMockBuildProcess(0);
5473
const mockChild = createMockChildProcess();
55-
mockSpawn.mockReturnValue(mockChild);
74+
mockSpawn.mockReturnValueOnce(mockBuild).mockReturnValueOnce(mockChild);
5675
return mockChild;
5776
}
5877

@@ -141,16 +160,16 @@ describe('ContainerDevServer', () => {
141160
expect(rmCall![1]).toEqual(['rm', '-f', 'agentcore-dev-testagent']);
142161
});
143162

144-
it('returns null when base image build fails', async () => {
163+
it('returns null when image build fails', async () => {
145164
mockDetectContainerRuntime.mockResolvedValue({
146165
runtime: { runtime: 'docker', binary: 'docker', version: 'Docker 24.0' },
147166
notReadyRuntimes: [],
148167
});
149168
mockExistsSync.mockReturnValue(true);
150-
// rm succeeds, base build fails
151-
mockSpawnSync
152-
.mockReturnValueOnce({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }) // rm
153-
.mockReturnValueOnce({ status: 1, stdout: Buffer.from(''), stderr: Buffer.from('build error') }); // base build
169+
// rm succeeds (spawnSync)
170+
mockSpawnSync.mockReturnValue({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') });
171+
// build fails (spawn, auto-closes with exit code 1)
172+
mockSpawn.mockReturnValue(createMockBuildProcess(1));
154173

155174
const server = new ContainerDevServer(defaultConfig, defaultOptions);
156175
const result = await server.start();
@@ -159,26 +178,7 @@ describe('ContainerDevServer', () => {
159178
expect(mockCallbacks.onLog).toHaveBeenCalledWith('error', expect.stringContaining('Container build failed'));
160179
});
161180

162-
it('returns null when dev layer build fails', async () => {
163-
mockDetectContainerRuntime.mockResolvedValue({
164-
runtime: { runtime: 'docker', binary: 'docker', version: 'Docker 24.0' },
165-
notReadyRuntimes: [],
166-
});
167-
mockExistsSync.mockReturnValue(true);
168-
// rm succeeds, base build succeeds, dev build fails
169-
mockSpawnSync
170-
.mockReturnValueOnce({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }) // rm
171-
.mockReturnValueOnce({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }) // base build
172-
.mockReturnValueOnce({ status: 1, stdout: Buffer.from(''), stderr: Buffer.from('dev error') }); // dev build
173-
174-
const server = new ContainerDevServer(defaultConfig, defaultOptions);
175-
const result = await server.start();
176-
177-
expect(result).toBeNull();
178-
expect(mockCallbacks.onLog).toHaveBeenCalledWith('error', expect.stringContaining('Dev layer build failed'));
179-
});
180-
181-
it('succeeds when both builds pass and logs success message', async () => {
181+
it('succeeds when build passes and logs success message', async () => {
182182
mockSuccessfulPrepare();
183183

184184
const server = new ContainerDevServer(defaultConfig, defaultOptions);
@@ -188,67 +188,49 @@ describe('ContainerDevServer', () => {
188188
expect(mockCallbacks.onLog).toHaveBeenCalledWith('system', 'Container image built successfully.');
189189
});
190190

191-
it('dev layer prefers uv when available, falls back to pip', async () => {
192-
mockSuccessfulPrepare();
193-
194-
const server = new ContainerDevServer(defaultConfig, defaultOptions);
195-
await server.start();
196-
197-
// The dev build is the 3rd spawnSync call (rm, base build, dev build)
198-
const devBuildCall = mockSpawnSync.mock.calls[2]!;
199-
expect(devBuildCall).toBeDefined();
200-
const input = devBuildCall[2]?.input as string;
201-
// uv path tried first with --system flag
202-
expect(input).toContain('uv pip install --system -q uvicorn');
203-
expect(input).toContain('uv pip install --system /app');
204-
// pip fallback
205-
expect(input).toContain('pip install -q uvicorn');
206-
expect(input).toContain('pip install -q /app');
207-
// No requirements.txt fallback — pip install /app reads pyproject.toml
208-
expect(input).not.toContain('requirements.txt');
209-
});
210-
211-
it('dev layer FROM references the base image name', async () => {
191+
it('logs system-level start message and triggers TUI readiness after container is spawned', async () => {
212192
mockSuccessfulPrepare();
213193

214194
const server = new ContainerDevServer(defaultConfig, defaultOptions);
215195
await server.start();
216196

217-
const devBuildCall = mockSpawnSync.mock.calls[2]!;
218-
const input = devBuildCall[2]?.input as string;
219-
expect(input).toContain('FROM agentcore-dev-testagent-base');
197+
expect(mockCallbacks.onLog).toHaveBeenCalledWith(
198+
'system',
199+
'Container agentcore-dev-testagent started on port 9000.'
200+
);
201+
// Emits readiness trigger for TUI detection
202+
expect(mockCallbacks.onLog).toHaveBeenCalledWith('info', 'Application startup complete');
220203
});
221204

222-
it('dev layer does not set USER (runs as root for dev)', async () => {
205+
it('builds image directly without a dev layer', async () => {
223206
mockSuccessfulPrepare();
224207

225208
const server = new ContainerDevServer(defaultConfig, defaultOptions);
226209
await server.start();
227210

228-
const devBuildCall = mockSpawnSync.mock.calls[2]!;
229-
const input = devBuildCall[2]?.input as string;
230-
// Should have USER root but not USER bedrock_agentcore
231-
expect(input).toContain('USER root');
232-
expect(input).not.toContain('USER bedrock_agentcore');
211+
// spawnSync only called once for rm (build uses async spawn)
212+
expect(mockSpawnSync).toHaveBeenCalledTimes(1);
213+
// First spawn call is the build
214+
const buildCall = mockSpawn.mock.calls[0]!;
215+
const buildArgs = buildCall[1] as string[];
216+
// Image is built directly as agentcore-dev-testagent (no -base suffix)
217+
expect(buildArgs).toContain('-t');
218+
const tagIdx = buildArgs.indexOf('-t');
219+
expect(buildArgs[tagIdx + 1]).toBe('agentcore-dev-testagent');
233220
});
234221

235-
it('logs non-empty build output lines at system level', async () => {
222+
it('streams build output lines at system level in real-time', async () => {
236223
mockDetectContainerRuntime.mockResolvedValue({
237224
runtime: { runtime: 'docker', binary: 'docker', version: 'Docker 24.0' },
238225
notReadyRuntimes: [],
239226
});
240227
mockExistsSync.mockReturnValue(true);
241-
mockSpawnSync
242-
.mockReturnValueOnce({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }) // rm
243-
.mockReturnValueOnce({
244-
status: 0,
245-
stdout: Buffer.from('Step 1/3: FROM python\nStep 2/3: COPY . .\n'),
246-
stderr: Buffer.from(''),
247-
}) // base build
248-
.mockReturnValueOnce({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }); // dev build
228+
mockSpawnSync.mockReturnValue({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') }); // rm
249229

250-
const mockChild = createMockChildProcess();
251-
mockSpawn.mockReturnValue(mockChild);
230+
// Build process that emits stdout lines then closes
231+
const buildProc = createMockBuildProcess(0, 'Step 1/3: FROM python\nStep 2/3: COPY . .\n');
232+
const serverProc = createMockChildProcess();
233+
mockSpawn.mockReturnValueOnce(buildProc).mockReturnValueOnce(serverProc);
252234

253235
const server = new ContainerDevServer(defaultConfig, defaultOptions);
254236
await server.start();
@@ -258,9 +240,9 @@ describe('ContainerDevServer', () => {
258240
});
259241
});
260242

261-
/** Extract the args array from the first mockSpawn call. */
243+
/** Extract the args array from the server run spawn call (second spawn — first is the build). */
262244
function getSpawnArgs(): string[] {
263-
return mockSpawn.mock.calls[0]![1] as string[];
245+
return mockSpawn.mock.calls[1]![1] as string[];
264246
}
265247

266248
describe('getSpawnConfig() — verified via spawn args', () => {
@@ -288,39 +270,36 @@ describe('ContainerDevServer', () => {
288270
expect(spawnArgs[nameIdx + 1]).toBe('agentcore-dev-testagent');
289271
});
290272

291-
it('overrides entrypoint to python', async () => {
273+
it('does not override entrypoint — uses Dockerfile CMD/ENTRYPOINT', async () => {
292274
mockSuccessfulPrepare();
293275

294276
const server = new ContainerDevServer(defaultConfig, defaultOptions);
295277
await server.start();
296278

297279
const spawnArgs = getSpawnArgs();
298-
const entrypointIdx = spawnArgs.indexOf('--entrypoint');
299-
expect(entrypointIdx).toBeGreaterThan(-1);
300-
expect(spawnArgs[entrypointIdx + 1]).toBe('python');
280+
expect(spawnArgs).not.toContain('--entrypoint');
301281
});
302282

303-
it('runs as root to ensure system site-packages are accessible', async () => {
283+
it('does not override user', async () => {
304284
mockSuccessfulPrepare();
305285

306286
const server = new ContainerDevServer(defaultConfig, defaultOptions);
307287
await server.start();
308288

309289
const spawnArgs = getSpawnArgs();
310-
const userIdx = spawnArgs.indexOf('--user');
311-
expect(userIdx).toBeGreaterThan(-1);
312-
expect(spawnArgs[userIdx + 1]).toBe('root');
290+
expect(spawnArgs).not.toContain('--user');
313291
});
314292

315-
it('mounts source directory as /app volume', async () => {
293+
it('does not mount source directory as volume', async () => {
316294
mockSuccessfulPrepare();
317295

318296
const server = new ContainerDevServer(defaultConfig, defaultOptions);
319297
await server.start();
320298

321299
const spawnArgs = getSpawnArgs();
322-
expect(spawnArgs).toContain('-v');
323-
expect(spawnArgs).toContain('/project/app:/app');
300+
// No source:/app volume mount (only ~/.aws mount should be present)
301+
const volumeArgs = spawnArgs.filter((arg: string) => arg.includes(':/app'));
302+
expect(volumeArgs).toHaveLength(0);
324303
});
325304

326305
it('maps host port to container internal port', async () => {
@@ -355,6 +334,16 @@ describe('ContainerDevServer', () => {
355334
expect(spawnArgs).toContain(`PORT=${CONTAINER_INTERNAL_PORT}`);
356335
});
357336

337+
it('disables OpenTelemetry SDK to avoid missing-collector errors', async () => {
338+
mockSuccessfulPrepare();
339+
340+
const server = new ContainerDevServer(defaultConfig, defaultOptions);
341+
await server.start();
342+
343+
const spawnArgs = getSpawnArgs();
344+
expect(spawnArgs).toContain('OTEL_SDK_DISABLED=true');
345+
});
346+
358347
it('forwards AWS env vars when present in process.env', async () => {
359348
process.env.AWS_ACCESS_KEY_ID = 'AKIAIOSFODNN7EXAMPLE';
360349
process.env.AWS_SECRET_ACCESS_KEY = 'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY';
@@ -391,21 +380,32 @@ describe('ContainerDevServer', () => {
391380
await server.start();
392381

393382
const spawnArgs = getSpawnArgs();
394-
const awsArgs = spawnArgs.filter((arg: string) => arg.startsWith('AWS_'));
383+
// Filter only forwarded AWS cred env vars, not AWS_CONFIG_FILE/CREDENTIALS_FILE
384+
const awsArgs = spawnArgs.filter((arg: string) => arg.startsWith('AWS_') && !arg.includes('_FILE='));
395385
expect(awsArgs).toHaveLength(0);
396386
});
397387

398-
it('mounts ~/.aws to /root/.aws when exists', async () => {
388+
it('mounts ~/.aws to /aws-config for any container user', async () => {
399389
mockSuccessfulPrepare();
400390
// existsSync returns true for all calls (Dockerfile and ~/.aws)
401391

402392
const server = new ContainerDevServer(defaultConfig, defaultOptions);
403393
await server.start();
404394

405395
const spawnArgs = getSpawnArgs();
406-
expect(spawnArgs).toContain('/home/testuser/.aws:/root/.aws:ro');
396+
expect(spawnArgs).toContain('/home/testuser/.aws:/aws-config:ro');
407397
});
408398

399+
it('sets AWS_CONFIG_FILE and AWS_SHARED_CREDENTIALS_FILE when ~/.aws exists', async () => {
400+
mockSuccessfulPrepare();
401+
402+
const server = new ContainerDevServer(defaultConfig, defaultOptions);
403+
await server.start();
404+
405+
const spawnArgs = getSpawnArgs();
406+
expect(spawnArgs).toContain('AWS_CONFIG_FILE=/aws-config/config');
407+
expect(spawnArgs).toContain('AWS_SHARED_CREDENTIALS_FILE=/aws-config/credentials');
408+
});
409409
it('skips ~/.aws mount when directory does not exist', async () => {
410410
mockDetectContainerRuntime.mockResolvedValue({
411411
runtime: { runtime: 'docker', binary: 'docker', version: 'Docker 24.0' },
@@ -417,8 +417,9 @@ describe('ContainerDevServer', () => {
417417
return true; // Dockerfile exists
418418
});
419419
mockSpawnSync.mockReturnValue({ status: 0, stdout: Buffer.from(''), stderr: Buffer.from('') });
420+
const mockBuild = createMockBuildProcess(0);
420421
const mockChild = createMockChildProcess();
421-
mockSpawn.mockReturnValue(mockChild);
422+
mockSpawn.mockReturnValueOnce(mockBuild).mockReturnValueOnce(mockChild);
422423

423424
const server = new ContainerDevServer(defaultConfig, defaultOptions);
424425
await server.start();
@@ -428,28 +429,16 @@ describe('ContainerDevServer', () => {
428429
expect(awsMountArg).toBeUndefined();
429430
});
430431

431-
it('uses uvicorn with --reload and --reload-dir /app', async () => {
432-
mockSuccessfulPrepare();
433-
434-
const server = new ContainerDevServer(defaultConfig, defaultOptions);
435-
await server.start();
436-
437-
const spawnArgs = getSpawnArgs();
438-
expect(spawnArgs).toContain('-m');
439-
expect(spawnArgs).toContain('uvicorn');
440-
expect(spawnArgs).toContain('--reload');
441-
expect(spawnArgs).toContain('--reload-dir');
442-
expect(spawnArgs).toContain('/app');
443-
});
444-
445-
it('converts entrypoint via convertEntrypointToModule (main.py -> main:app)', async () => {
432+
it('does not include uvicorn or reload args', async () => {
446433
mockSuccessfulPrepare();
447434

448435
const server = new ContainerDevServer(defaultConfig, defaultOptions);
449436
await server.start();
450437

451438
const spawnArgs = getSpawnArgs();
452-
expect(spawnArgs).toContain('main:app');
439+
expect(spawnArgs).not.toContain('uvicorn');
440+
expect(spawnArgs).not.toContain('--reload');
441+
expect(spawnArgs).not.toContain('-m');
453442
});
454443
});
455444

@@ -461,20 +450,22 @@ describe('ContainerDevServer', () => {
461450
const child = await server.start();
462451

463452
// Clear mocks to isolate the kill call
464-
mockSpawnSync.mockClear();
453+
mockSpawn.mockClear();
465454

466455
server.kill();
467456

468-
expect(mockSpawnSync).toHaveBeenCalledWith('docker', ['stop', 'agentcore-dev-testagent'], { stdio: 'ignore' });
457+
// Container stop is async (spawn not spawnSync) so UI can render "Stopping..." message
458+
expect(mockSpawn).toHaveBeenCalledWith('docker', ['stop', 'agentcore-dev-testagent'], { stdio: 'ignore' });
469459
expect(child!.kill).toHaveBeenCalledWith('SIGTERM'); // eslint-disable-line @typescript-eslint/unbound-method
470460
});
471461

472462
it('does not call container stop when runtimeBinary is empty (prepare not called)', () => {
473463
const server = new ContainerDevServer(defaultConfig, defaultOptions);
464+
mockSpawn.mockClear();
474465

475466
server.kill();
476467

477-
expect(mockSpawnSync).not.toHaveBeenCalled();
468+
expect(mockSpawn).not.toHaveBeenCalled();
478469
});
479470
});
480471
});

0 commit comments

Comments
 (0)