From 8051f987f1546c2d23317aef181d0c7b87b2976f Mon Sep 17 00:00:00 2001 From: Dutch Dim Date: Wed, 15 Jul 2026 16:53:22 +0200 Subject: [PATCH 1/2] fix: inline dependabot auto-merge; test pi + docker-sandbox executors - Inline the Dependabot auto-merge job: the reusable workflow in the private djimit/.github repo cannot be called from this public repo (same root cause as the CodeQL inline). Auto-merges patch/minor only. - Add pi-executor tests: buildPiArgs env/option contract, mapPiEvent lifecycle + tool mapping, JSON parsing, token-usage capture, heuristic fallback classification. - Add docker-sandbox-executor tests: buildDockerArgs isolation flags, env filtering, bind mounts, wrapper delegation and event bracketing. Closes the last two executor coverage gaps (8/10 -> 10/10). Co-Authored-By: Claude Fable 5 --- .github/workflows/dependabot-auto-merge.yml | 19 +- .../__tests__/docker-sandbox-executor.test.ts | 167 +++++++++++++++++ .../server/src/__tests__/pi-executor.test.ts | 174 ++++++++++++++++++ 3 files changed, 358 insertions(+), 2 deletions(-) create mode 100644 packages/server/src/__tests__/docker-sandbox-executor.test.ts create mode 100644 packages/server/src/__tests__/pi-executor.test.ts diff --git a/.github/workflows/dependabot-auto-merge.yml b/.github/workflows/dependabot-auto-merge.yml index 393b797a..e6096837 100644 --- a/.github/workflows/dependabot-auto-merge.yml +++ b/.github/workflows/dependabot-auto-merge.yml @@ -5,7 +5,22 @@ permissions: contents: write pull-requests: write +# Inlined rather than calling djimit/.github's reusable workflow: that repo is +# private with Actions access_level "none", so a public repo like this one +# cannot call its reusable workflow (the call fails at startup). jobs: auto-merge: - uses: djimit/.github/.github/workflows/dependabot-auto-merge.yml@main - secrets: inherit + if: github.actor == 'dependabot[bot]' + runs-on: ubuntu-latest + steps: + - name: Fetch Dependabot metadata + id: metadata + uses: dependabot/fetch-metadata@v2 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Enable auto-merge for patch/minor updates + if: steps.metadata.outputs.update-type != 'version-update:semver-major' + run: gh pr merge --auto --squash "$PR_URL" + env: + PR_URL: ${{ github.event.pull_request.html_url }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/packages/server/src/__tests__/docker-sandbox-executor.test.ts b/packages/server/src/__tests__/docker-sandbox-executor.test.ts new file mode 100644 index 00000000..d1bab0f2 --- /dev/null +++ b/packages/server/src/__tests__/docker-sandbox-executor.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, it } from 'vitest'; +import type { Task, ExecutionEventCreateInput } from '@djimitflo/shared'; +import { ExecutionEventType, LogLevel } from '@djimitflo/shared'; +import type { TaskExecutor, ExecutionSession, ExecutorKind } from '../execution/types'; +import { + DockerSandboxExecutor, + DEFAULT_SANDBOX_CONFIG, + type DockerSandboxConfig, +} from '../execution/executors/docker-sandbox-executor'; + +function makeTask(overrides: Partial = {}): Task { + return { + id: 'test-task-id', + title: 'Test task', + description: 'echo hello world', + status: 'pending' as any, + priority: 'medium' as any, + risk_level: 'low' as any, + execution_mode: 'local' as any, + agent_id: null, + parent_task_id: null, + repository_id: null, + instruction_profile_id: null, + started_at: null, + completed_at: null, + failed_at: null, + execution_time_ms: null, + token_usage: null, + tags: [], + metadata: {}, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + ...overrides, + }; +} + +const testConfig: DockerSandboxConfig = { + image: 'test-image:latest', + cpuLimit: '2.0', + memoryLimit: '256m', + networkMode: 'none', + timeoutMs: 60000, + readOnlyRoot: true, + bindMounts: [], +}; + +/** Fast inner executor stub — MockExecutor sleeps between events, this one doesn't. */ +class StubExecutor implements TaskExecutor { + readonly kind: ExecutorKind = 'mock'; + lastEnvironment: Record | undefined; + + canExecute(task: Task): boolean { + return task.risk_level !== ('critical' as any); + } + + async start(task: Task, options?: { environment?: Record }): Promise { + this.lastEnvironment = options?.environment; + const events = (async function* (): AsyncIterable { + yield { + task_id: task.id, + event_type: ExecutionEventType.LOG, + message: 'inner event', + level: LogLevel.INFO, + metadata: {}, + }; + })(); + return { + id: 'inner-session', + taskId: task.id, + executorKind: this.kind, + status: 'running', + startedAt: new Date(), + events, + result: Promise.resolve({ status: 'completed', message: 'inner done' }), + }; + } +} + +describe('DockerSandboxExecutor.buildDockerArgs', () => { + it('applies resource limits, network isolation, and read-only root with tmpfs', () => { + const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', testConfig, 'node', ['script.js']); + + expect(args.slice(0, 4)).toEqual(['run', '--rm', '--name', 'sandbox-1']); + expect(args[args.indexOf('--cpus') + 1]).toBe('2.0'); + expect(args[args.indexOf('--memory') + 1]).toBe('256m'); + expect(args[args.indexOf('--network') + 1]).toBe('none'); + expect(args).toContain('--read-only'); + expect(args[args.indexOf('--tmpfs') + 1]).toBe('/tmp:size=64m'); + // image, then command and its args, close the invocation + expect(args.slice(-3)).toEqual(['test-image:latest', 'node', 'script.js']); + }); + + it('omits --read-only when readOnlyRoot is disabled', () => { + const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', { ...testConfig, readOnlyRoot: false }, 'sh', []); + expect(args).not.toContain('--read-only'); + expect(args).not.toContain('--tmpfs'); + }); + + it('mounts the working directory rw at /workspace and applies extra bind mounts', () => { + const config = { + ...testConfig, + bindMounts: [{ host: '/host/cache', container: '/cache', mode: 'ro' as const }], + }; + const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', config, 'sh', [], '/host/project'); + + expect(args).toContain('/host/project:/workspace:rw'); + expect(args[args.indexOf('-w') + 1]).toBe('/workspace'); + expect(args).toContain('/host/cache:/cache:ro'); + }); + + it('forwards env vars but never the __-prefixed internal markers', () => { + const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', testConfig, 'sh', [], undefined, { + MY_VAR: 'value', + __DOCKER_SANDBOX_ENABLED: 'true', + }); + + expect(args).toContain('MY_VAR=value'); + expect(args.join(' ')).not.toContain('__DOCKER_SANDBOX_ENABLED'); + }); +}); + +describe('DockerSandboxExecutor wrapper', () => { + // ponytail: dockerPath 'true' — cleanupContainer spawns a no-op instead of the docker CLI + const makeSandbox = (inner: TaskExecutor) => new DockerSandboxExecutor(inner, testConfig, 'true'); + + it('mirrors the inner executor kind and delegates canExecute', () => { + const inner = new StubExecutor(); + const sandbox = makeSandbox(inner); + + expect(sandbox.kind).toBe('mock'); + expect(sandbox.canExecute(makeTask())).toBe(true); + expect(sandbox.canExecute(makeTask({ risk_level: 'critical' as any }))).toBe(false); + }); + + it('marks the inner environment as sandboxed and brackets events with sandbox lifecycle', async () => { + const inner = new StubExecutor(); + const sandbox = makeSandbox(inner); + const task = makeTask(); + + const session = await sandbox.start(task); + expect(inner.lastEnvironment?.__DOCKER_SANDBOX_ENABLED).toBe('true'); + + const events: ExecutionEventCreateInput[] = []; + for await (const event of session.events) events.push(event); + + expect(events[0].message).toContain('Docker sandbox started'); + expect(events[0].metadata).toMatchObject({ sandbox: 'docker', image: 'test-image:latest', network: 'none' }); + expect(events[1].message).toBe('inner event'); + expect(events[events.length - 1].message).toContain('Docker sandbox cleaned up'); + }); + + it('passes the inner result through', async () => { + const sandbox = makeSandbox(new StubExecutor()); + const session = await sandbox.start(makeTask()); + const result = await session.result; + + expect(result.status).toBe('completed'); + expect(result.message).toBe('inner done'); + }); +}); + +describe('DEFAULT_SANDBOX_CONFIG', () => { + it('is default-deny on network and read-only on root', () => { + expect(DEFAULT_SANDBOX_CONFIG.networkMode).toBe('none'); + expect(DEFAULT_SANDBOX_CONFIG.readOnlyRoot).toBe(true); + }); +}); diff --git a/packages/server/src/__tests__/pi-executor.test.ts b/packages/server/src/__tests__/pi-executor.test.ts new file mode 100644 index 00000000..6922767c --- /dev/null +++ b/packages/server/src/__tests__/pi-executor.test.ts @@ -0,0 +1,174 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import type { Task } from '@djimitflo/shared'; +import { ExecutionEventType, LogLevel } from '@djimitflo/shared'; +import { PiExecutor } from '../execution/executors/pi-executor'; +import { buildPiArgs, mapPiEvent } from '../execution/executors/pi-shared'; + +function makeTask(overrides: Partial = {}): Task { + return { + id: 'test-task-id', + title: 'Test task', + description: 'echo hello world', + status: 'pending' as any, + priority: 'medium' as any, + risk_level: 'low' as any, + execution_mode: 'local' as any, + agent_id: null, + parent_task_id: null, + repository_id: null, + instruction_profile_id: null, + started_at: null, + completed_at: null, + failed_at: null, + execution_time_ms: null, + token_usage: null, + tags: [], + metadata: {}, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + ...overrides, + }; +} + +const PI_ENV_KEYS = [ + 'PI_NO_APPROVE', 'PI_NO_CONTEXT_FILES', 'PI_NO_EXTENSIONS', 'PI_NO_SKILLS', + 'PI_OFFLINE', 'PI_PROVIDER', 'PI_MODEL', 'PI_THINKING', 'PI_TOOLS', 'PI_EXCLUDE_TOOLS', +]; + +describe('buildPiArgs', () => { + const previousEnv = { ...process.env }; + + beforeEach(() => { + for (const key of PI_ENV_KEYS) delete process.env[key]; + }); + + afterEach(() => { + for (const key of Object.keys(process.env)) { + if (!(key in previousEnv)) delete process.env[key]; + } + Object.assign(process.env, previousEnv); + }); + + it('builds the headless json invocation with deterministic flags on by default', () => { + const args = buildPiArgs(makeTask({ description: 'test prompt' })); + + expect(args.slice(0, 4)).toEqual(['--mode', 'json', '-p', '--no-session']); + // default-on hygiene flags (PI_NO_APPROVE / PI_NO_EXTENSIONS / PI_NO_SKILLS default to '1') + expect(args).toContain('--no-approve'); + expect(args).toContain('--no-extensions'); + expect(args).toContain('--no-skills'); + // default-off flags + expect(args).not.toContain('--no-context-files'); + expect(args).not.toContain('--offline'); + // prompt is the trailing positional + expect(args[args.length - 1]).toBe('test prompt'); + }); + + it('allows disabling the default-on flags via env', () => { + process.env.PI_NO_APPROVE = '0'; + process.env.PI_NO_EXTENSIONS = '0'; + process.env.PI_NO_SKILLS = '0'; + const args = buildPiArgs(makeTask()); + + expect(args).not.toContain('--no-approve'); + expect(args).not.toContain('--no-extensions'); + expect(args).not.toContain('--no-skills'); + }); + + it('prefers options.model over PI_MODEL env', () => { + process.env.PI_MODEL = 'env-model'; + const optArgs = buildPiArgs(makeTask(), { model: 'option-model' }); + expect(optArgs[optArgs.indexOf('--model') + 1]).toBe('option-model'); + + const envArgs = buildPiArgs(makeTask()); + expect(envArgs[envArgs.indexOf('--model') + 1]).toBe('env-model'); + + delete process.env.PI_MODEL; + expect(buildPiArgs(makeTask())).not.toContain('--model'); + }); + + it('passes provider, thinking, tools allowlist, and offline flag from env', () => { + process.env.PI_PROVIDER = 'ollama'; + process.env.PI_THINKING = 'high'; + process.env.PI_TOOLS = 'read,ls'; + process.env.PI_EXCLUDE_TOOLS = 'bash'; + process.env.PI_OFFLINE = '1'; + const args = buildPiArgs(makeTask()); + + expect(args[args.indexOf('--provider') + 1]).toBe('ollama'); + expect(args[args.indexOf('--thinking') + 1]).toBe('high'); + expect(args[args.indexOf('--tools') + 1]).toBe('read,ls'); + expect(args[args.indexOf('--exclude-tools') + 1]).toBe('bash'); + expect(args).toContain('--offline'); + }); +}); + +describe('mapPiEvent', () => { + const metrics = () => ({ tokenUsage: 0, toolCalls: 0, approvalsRequested: 0 }); + + it('maps the lifecycle events to execution events', () => { + const start = mapPiEvent('t1', { type: 'agent_start' }, metrics()); + expect(start?.event_type).toBe(ExecutionEventType.TASK_STARTED); + + const end = mapPiEvent('t1', { type: 'agent_end' }, metrics()); + expect(end?.event_type).toBe(ExecutionEventType.TASK_COMPLETED); + + const session = mapPiEvent('t1', { type: 'session', id: 's1', version: 3, cwd: '/w' }, metrics()); + expect(session?.event_type).toBe(ExecutionEventType.LOG); + expect(session?.metadata).toMatchObject({ pi_session_id: 's1', cwd: '/w' }); + }); + + it('maps tool execution events and flags tool errors', () => { + const call = mapPiEvent('t1', { type: 'tool_execution_start', toolName: 'ls', toolCallId: 'c1' }, metrics()); + expect(call?.event_type).toBe(ExecutionEventType.TOOL_CALL); + expect(call?.metadata).toMatchObject({ tool_name: 'ls' }); + + const ok = mapPiEvent('t1', { type: 'tool_execution_end', toolName: 'ls', isError: false }, metrics()); + expect(ok?.event_type).toBe(ExecutionEventType.TOOL_RESULT); + expect(ok?.level).toBe(LogLevel.INFO); + + const failed = mapPiEvent('t1', { type: 'tool_execution_end', toolName: 'bash', isError: true }, metrics()); + expect(failed?.level).toBe(LogLevel.ERROR); + }); + + it('returns null for events that should not reach the audit trail', () => { + expect(mapPiEvent('t1', { type: 'message_update' }, metrics())).toBeNull(); + expect(mapPiEvent('t1', { type: 'turn_start' }, metrics())).toBeNull(); + }); +}); + +describe('PiExecutor', () => { + it('parses only valid typed JSON lines', () => { + const executor = new PiExecutor('/usr/bin/pi'); + expect((executor as any).parseJsonEvent('{"type":"agent_start"}')).toMatchObject({ type: 'agent_start' }); + expect((executor as any).parseJsonEvent('{"no_type":true}')).toBeNull(); + expect((executor as any).parseJsonEvent('not json at all')).toBeNull(); + }); + + it('captures token usage from assistant message_end and maps toolResult messages', () => { + const executor = new PiExecutor('/usr/bin/pi'); + const metrics = { tokenUsage: 0, toolCalls: 0, approvalsRequested: 0 }; + + const logEvent = (executor as any).mapJsonEventToExecutionEvent('t1', { + type: 'message_end', + message: { role: 'assistant', content: [{ type: 'text', text: 'done' }], usage: { totalTokens: 123 } }, + }, metrics); + expect(metrics.tokenUsage).toBe(123); + expect(logEvent?.event_type).toBe(ExecutionEventType.LOG); + expect(logEvent?.message).toBe('done'); + + const toolResult = (executor as any).mapJsonEventToExecutionEvent('t1', { + type: 'message_end', + message: { role: 'toolResult', toolName: 'read', content: [{ type: 'text', text: 'file contents' }] }, + }, metrics); + expect(toolResult?.event_type).toBe(ExecutionEventType.TOOL_RESULT); + expect(toolResult?.tool_output).toBe('file contents'); + }); + + it('classifies stderr and error-like lines in heuristic fallback', () => { + const executor = new PiExecutor('/usr/bin/pi'); + expect((executor as any).parseHeuristicLine('anything', 'stderr').event_type).toBe(ExecutionEventType.ERROR); + expect((executor as any).parseHeuristicLine('operation failed', 'stdout').event_type).toBe(ExecutionEventType.ERROR); + expect((executor as any).parseHeuristicLine('all good', 'stdout').event_type).toBe(ExecutionEventType.LOG); + }); +}); From 7937c90548ca8d71c4f3134df17e42fa4aa9ddde Mon Sep 17 00:00:00 2001 From: Dutch Dim Date: Wed, 15 Jul 2026 17:00:50 +0200 Subject: [PATCH 2/2] =?UTF-8?q?test:=20apply=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20guard=20indexOf=20flag=20lookups,=20use=20ExecutorO?= =?UTF-8?q?ptions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- .../server/src/__tests__/docker-sandbox-executor.test.ts | 9 +++++++-- packages/server/src/__tests__/pi-executor.test.ts | 6 ++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/server/src/__tests__/docker-sandbox-executor.test.ts b/packages/server/src/__tests__/docker-sandbox-executor.test.ts index d1bab0f2..584d7ce0 100644 --- a/packages/server/src/__tests__/docker-sandbox-executor.test.ts +++ b/packages/server/src/__tests__/docker-sandbox-executor.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vitest'; import type { Task, ExecutionEventCreateInput } from '@djimitflo/shared'; import { ExecutionEventType, LogLevel } from '@djimitflo/shared'; -import type { TaskExecutor, ExecutionSession, ExecutorKind } from '../execution/types'; +import type { TaskExecutor, ExecutionSession, ExecutorKind, ExecutorOptions } from '../execution/types'; import { DockerSandboxExecutor, DEFAULT_SANDBOX_CONFIG, @@ -53,7 +53,7 @@ class StubExecutor implements TaskExecutor { return task.risk_level !== ('critical' as any); } - async start(task: Task, options?: { environment?: Record }): Promise { + async start(task: Task, options?: ExecutorOptions): Promise { this.lastEnvironment = options?.environment; const events = (async function* (): AsyncIterable { yield { @@ -81,10 +81,14 @@ describe('DockerSandboxExecutor.buildDockerArgs', () => { const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', testConfig, 'node', ['script.js']); expect(args.slice(0, 4)).toEqual(['run', '--rm', '--name', 'sandbox-1']); + expect(args).toContain('--cpus'); expect(args[args.indexOf('--cpus') + 1]).toBe('2.0'); + expect(args).toContain('--memory'); expect(args[args.indexOf('--memory') + 1]).toBe('256m'); + expect(args).toContain('--network'); expect(args[args.indexOf('--network') + 1]).toBe('none'); expect(args).toContain('--read-only'); + expect(args).toContain('--tmpfs'); expect(args[args.indexOf('--tmpfs') + 1]).toBe('/tmp:size=64m'); // image, then command and its args, close the invocation expect(args.slice(-3)).toEqual(['test-image:latest', 'node', 'script.js']); @@ -104,6 +108,7 @@ describe('DockerSandboxExecutor.buildDockerArgs', () => { const args = DockerSandboxExecutor.buildDockerArgs('sandbox-1', config, 'sh', [], '/host/project'); expect(args).toContain('/host/project:/workspace:rw'); + expect(args).toContain('-w'); expect(args[args.indexOf('-w') + 1]).toBe('/workspace'); expect(args).toContain('/host/cache:/cache:ro'); }); diff --git a/packages/server/src/__tests__/pi-executor.test.ts b/packages/server/src/__tests__/pi-executor.test.ts index 6922767c..6eda0f1c 100644 --- a/packages/server/src/__tests__/pi-executor.test.ts +++ b/packages/server/src/__tests__/pi-executor.test.ts @@ -78,9 +78,11 @@ describe('buildPiArgs', () => { it('prefers options.model over PI_MODEL env', () => { process.env.PI_MODEL = 'env-model'; const optArgs = buildPiArgs(makeTask(), { model: 'option-model' }); + expect(optArgs).toContain('--model'); expect(optArgs[optArgs.indexOf('--model') + 1]).toBe('option-model'); const envArgs = buildPiArgs(makeTask()); + expect(envArgs).toContain('--model'); expect(envArgs[envArgs.indexOf('--model') + 1]).toBe('env-model'); delete process.env.PI_MODEL; @@ -95,9 +97,13 @@ describe('buildPiArgs', () => { process.env.PI_OFFLINE = '1'; const args = buildPiArgs(makeTask()); + expect(args).toContain('--provider'); expect(args[args.indexOf('--provider') + 1]).toBe('ollama'); + expect(args).toContain('--thinking'); expect(args[args.indexOf('--thinking') + 1]).toBe('high'); + expect(args).toContain('--tools'); expect(args[args.indexOf('--tools') + 1]).toBe('read,ls'); + expect(args).toContain('--exclude-tools'); expect(args[args.indexOf('--exclude-tools') + 1]).toBe('bash'); expect(args).toContain('--offline'); });