From 8c95e6111c2102513b29d55c0eca76f4926be6f7 Mon Sep 17 00:00:00 2001 From: evan-claw Date: Thu, 19 Mar 2026 21:26:40 +0000 Subject: [PATCH] fix(dev-cli): handle non-compose infra services in logs command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The logs command assumed all 'infra' services are Docker Compose services, but 'migrations' is an infra service that runs 'pnpm drizzle migrate' — not a docker compose service. Running 'pnpm kilo logs migrations' would execute 'docker compose logs -f migrations' which fails immediately since no such compose service exists. Now the logs command checks whether an infra service's devCommand starts with 'docker compose' before attempting to tail compose logs. Non-compose infra services get a helpful warning message instead of a silent failure. Adds test/logs.test.ts with 4 tests validating these invariants. --- dev/cli/src/commands/logs.ts | 6 ++++- dev/cli/test/logs.test.ts | 45 ++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 dev/cli/test/logs.test.ts diff --git a/dev/cli/src/commands/logs.ts b/dev/cli/src/commands/logs.ts index 69e18bc639..57009728dc 100644 --- a/dev/cli/src/commands/logs.ts +++ b/dev/cli/src/commands/logs.ts @@ -18,12 +18,16 @@ export async function logs(args: string[], root: string) { return; } - if (svc.type === 'infra') { + if (svc.type === 'infra' && svc.devCommand?.startsWith('docker compose')) { const proc = Bun.spawn( ['docker', 'compose', '-f', 'dev/docker-compose.yml', 'logs', '-f', svc.name], { stdout: 'inherit', stderr: 'inherit', cwd: root } ); await proc.exited; + } else if (svc.type === 'infra') { + ui.warn( + `"${svc.name}" is not a Docker Compose service (runs: ${svc.devCommand ?? 'n/a'}).\n It does not produce persistent logs that can be tailed.` + ); } else { ui.warn( `Log tailing for running dev servers is not yet supported.\n Start the service with 'pnpm kilo dev up ${name}' to see its output.` diff --git a/dev/cli/test/logs.test.ts b/dev/cli/test/logs.test.ts new file mode 100644 index 0000000000..13a894fc94 --- /dev/null +++ b/dev/cli/test/logs.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from 'bun:test'; +import { services } from '../src/services/registry'; + +/** + * The `logs` command tails `docker compose logs -f ` for infra services. + * Only infra services whose devCommand starts with "docker compose" actually + * exist in docker-compose.yml. Non-compose infra services (e.g. migrations) + * must NOT be passed to `docker compose logs` — they'd fail silently or error. + * + * These tests validate the invariants that the logs command relies on. + */ +describe('logs command invariants', () => { + const infraServices = services.filter(s => s.type === 'infra'); + const composeInfra = infraServices.filter(s => s.devCommand?.startsWith('docker compose')); + const nonComposeInfra = infraServices.filter(s => !s.devCommand?.startsWith('docker compose')); + + test('migrations is an infra service with a non-compose devCommand', () => { + const migrations = services.find(s => s.name === 'migrations'); + expect(migrations).toBeDefined(); + expect(migrations!.type).toBe('infra'); + expect(migrations!.devCommand).toBe('pnpm drizzle migrate'); + expect(migrations!.devCommand!.startsWith('docker compose')).toBe(false); + }); + + test('postgres and redis are infra services with docker compose devCommands', () => { + for (const name of ['postgres', 'redis']) { + const svc = services.find(s => s.name === name); + expect(svc).toBeDefined(); + expect(svc!.type).toBe('infra'); + expect(svc!.devCommand!.startsWith('docker compose')).toBe(true); + } + }); + + test('at least one infra service is non-compose (guard against regression)', () => { + expect(nonComposeInfra.length).toBeGreaterThanOrEqual(1); + }); + + test('compose infra services have names matching docker-compose service names', () => { + // The logs command uses svc.name as the compose service name. + // Compose infra devCommands should contain `-d ` referencing the same service. + for (const svc of composeInfra) { + expect(svc.devCommand).toContain(svc.name); + } + }); +});