fix(cli): remove side-effect from agent:end state updater (#133) (#147)
Some checks failed
ci/woodpecker/push/ci Pipeline failed

Co-authored-by: Jason Woltje <jason@diversecanvas.com>
Co-committed-by: Jason Woltje <jason@diversecanvas.com>
This commit was merged in pull request #147.
This commit is contained in:
2026-03-15 19:09:13 +00:00
committed by jason.woltje
parent c4850fe6c1
commit 76abf11eba
4 changed files with 293 additions and 31 deletions

View File

@@ -1,6 +1,7 @@
import { Inject, Injectable, Logger, type OnModuleDestroy } from '@nestjs/common'; import { Inject, Injectable, Logger, type OnModuleDestroy } from '@nestjs/common';
import { import {
createAgentSession, createAgentSession,
DefaultResourceLoader,
SessionManager, SessionManager,
type AgentSession as PiAgentSession, type AgentSession as PiAgentSession,
type AgentSessionEvent, type AgentSessionEvent,
@@ -27,6 +28,27 @@ import type { SessionInfoDto } from './session.dto.js';
export interface AgentSessionOptions { export interface AgentSessionOptions {
provider?: string; provider?: string;
modelId?: string; modelId?: string;
/**
* Sandbox working directory for the session.
* File, git, and shell tools will be restricted to this directory.
* Falls back to AGENT_FILE_SANDBOX_DIR env var or process.cwd().
*/
sandboxDir?: string;
/**
* Platform-level system prompt for this session.
* Merged with skill prompt additions (platform prompt first, then skills).
* Falls back to AGENT_SYSTEM_PROMPT env var when omitted.
*/
systemPrompt?: string;
/**
* Explicit allowlist of tool names available in this session.
* When set, only listed tools are registered with the agent.
* When omitted for non-admin users, falls back to AGENT_USER_TOOLS env var.
* Admins (isAdmin=true) always receive the full tool set unless explicitly restricted.
*/
allowedTools?: string[];
/** Whether the requesting user has admin privileges. Controls default tool access. */
isAdmin?: boolean;
} }
export interface AgentSession { export interface AgentSession {
@@ -41,6 +63,10 @@ export interface AgentSession {
channels: Set<string>; channels: Set<string>;
/** System prompt additions injected from enabled prompt-type skills. */ /** System prompt additions injected from enabled prompt-type skills. */
skillPromptAdditions: string[]; skillPromptAdditions: string[];
/** Resolved sandbox directory for this session. */
sandboxDir: string;
/** Tool names available in this session, or null when all tools are available. */
allowedTools: string[] | null;
} }
@Injectable() @Injectable()
@@ -49,8 +75,6 @@ export class AgentService implements OnModuleDestroy {
private readonly sessions = new Map<string, AgentSession>(); private readonly sessions = new Map<string, AgentSession>();
private readonly creating = new Map<string, Promise<AgentSession>>(); private readonly creating = new Map<string, Promise<AgentSession>>();
private readonly customTools: ToolDefinition[];
constructor( constructor(
@Inject(ProviderService) private readonly providerService: ProviderService, @Inject(ProviderService) private readonly providerService: ProviderService,
@Inject(BRAIN) private readonly brain: Brain, @Inject(BRAIN) private readonly brain: Brain,
@@ -59,21 +83,49 @@ export class AgentService implements OnModuleDestroy {
@Inject(CoordService) private readonly coordService: CoordService, @Inject(CoordService) private readonly coordService: CoordService,
@Inject(McpClientService) private readonly mcpClientService: McpClientService, @Inject(McpClientService) private readonly mcpClientService: McpClientService,
@Inject(SkillLoaderService) private readonly skillLoaderService: SkillLoaderService, @Inject(SkillLoaderService) private readonly skillLoaderService: SkillLoaderService,
) { ) {}
const fileBaseDir = process.env['AGENT_FILE_SANDBOX_DIR'] ?? process.cwd();
const gitDefaultCwd = process.env['AGENT_GIT_CWD'] ?? process.cwd();
const shellDefaultCwd = process.env['AGENT_SHELL_CWD'] ?? process.cwd();
this.customTools = [ /**
...createBrainTools(brain), * Build the full set of custom tools scoped to the given sandbox directory.
...createCoordTools(coordService), * Brain/coord/memory/web tools are stateless with respect to cwd; file/git/shell
...createMemoryTools(memory, embeddingService.available ? embeddingService : null), * tools receive the resolved sandboxDir so they operate within the sandbox.
...createFileTools(fileBaseDir), */
...createGitTools(gitDefaultCwd), private buildToolsForSandbox(sandboxDir: string): ToolDefinition[] {
...createShellTools(shellDefaultCwd), return [
...createBrainTools(this.brain),
...createCoordTools(this.coordService),
...createMemoryTools(
this.memory,
this.embeddingService.available ? this.embeddingService : null,
),
...createFileTools(sandboxDir),
...createGitTools(sandboxDir),
...createShellTools(sandboxDir),
...createWebTools(), ...createWebTools(),
]; ];
this.logger.log(`Registered ${this.customTools.length} custom tools`); }
/**
* Resolve the tool allowlist for a session.
* - Admin users: all tools unless an explicit allowedTools list is passed.
* - Regular users: use allowedTools if provided, otherwise parse AGENT_USER_TOOLS env var.
* Returns null when all tools should be available.
*/
private resolveAllowedTools(isAdmin: boolean, allowedTools?: string[]): string[] | null {
if (allowedTools !== undefined) {
return allowedTools.length === 0 ? [] : allowedTools;
}
if (isAdmin) {
return null; // admins get everything
}
const envTools = process.env['AGENT_USER_TOOLS'];
if (!envTools) {
return null; // no restriction configured
}
return envTools
.split(',')
.map((t) => t.trim())
.filter((t) => t.length > 0);
} }
async createSession(sessionId: string, options?: AgentSessionOptions): Promise<AgentSession> { async createSession(sessionId: string, options?: AgentSessionOptions): Promise<AgentSession> {
@@ -98,8 +150,15 @@ export class AgentService implements OnModuleDestroy {
const providerName = model?.provider ?? 'default'; const providerName = model?.provider ?? 'default';
const modelId = model?.id ?? 'default'; const modelId = model?.id ?? 'default';
// Resolve sandbox directory: option > env var > process.cwd()
const sandboxDir =
options?.sandboxDir ?? process.env['AGENT_FILE_SANDBOX_DIR'] ?? process.cwd();
// Resolve allowed tool set
const allowedTools = this.resolveAllowedTools(options?.isAdmin ?? false, options?.allowedTools);
this.logger.log( this.logger.log(
`Creating agent session: ${sessionId} (provider=${providerName}, model=${modelId})`, `Creating agent session: ${sessionId} (provider=${providerName}, model=${modelId}, sandbox=${sandboxDir}, tools=${allowedTools === null ? 'all' : allowedTools.join(',') || 'none'})`,
); );
// Load skill tools from the catalog // Load skill tools from the catalog
@@ -114,21 +173,53 @@ export class AgentService implements OnModuleDestroy {
); );
} }
// Combine static tools with dynamically discovered MCP client tools // Build per-session tools scoped to the sandbox directory
const sandboxTools = this.buildToolsForSandbox(sandboxDir);
// Combine static tools with dynamically discovered MCP client tools and skill tools
const mcpTools = this.mcpClientService.getToolDefinitions(); const mcpTools = this.mcpClientService.getToolDefinitions();
const allCustomTools = [...this.customTools, ...skillMetaTools, ...mcpTools]; let allCustomTools = [...sandboxTools, ...skillMetaTools, ...mcpTools];
if (mcpTools.length > 0) { if (mcpTools.length > 0) {
this.logger.log(`Attaching ${mcpTools.length} MCP client tool(s) to session ${sessionId}`); this.logger.log(`Attaching ${mcpTools.length} MCP client tool(s) to session ${sessionId}`);
} }
// Filter tools by allowlist when a restriction is in effect
if (allowedTools !== null) {
const allowedSet = new Set(allowedTools);
const before = allCustomTools.length;
allCustomTools = allCustomTools.filter((t) => allowedSet.has(t.name));
this.logger.log(
`Tool restriction applied: ${allCustomTools.length}/${before} tools allowed for session ${sessionId}`,
);
}
// Build system prompt: platform prompt + skill additions appended
const platformPrompt = options?.systemPrompt ?? process.env['AGENT_SYSTEM_PROMPT'] ?? undefined;
const appendSystemPrompt =
promptAdditions.length > 0 ? promptAdditions.join('\n\n') : undefined;
// Construct a resource loader that injects the configured system prompt
const resourceLoader = new DefaultResourceLoader({
cwd: sandboxDir,
noExtensions: true,
noSkills: true,
noPromptTemplates: true,
noThemes: true,
systemPrompt: platformPrompt,
appendSystemPrompt: appendSystemPrompt,
});
await resourceLoader.reload();
let piSession: PiAgentSession; let piSession: PiAgentSession;
try { try {
const result = await createAgentSession({ const result = await createAgentSession({
sessionManager: SessionManager.inMemory(), sessionManager: SessionManager.inMemory(),
modelRegistry: this.providerService.getRegistry(), modelRegistry: this.providerService.getRegistry(),
model: model ?? undefined, model: model ?? undefined,
cwd: sandboxDir,
tools: [], tools: [],
customTools: allCustomTools, customTools: allCustomTools,
resourceLoader,
}); });
piSession = result.session; piSession = result.session;
} catch (err) { } catch (err) {
@@ -162,6 +253,8 @@ export class AgentService implements OnModuleDestroy {
promptCount: 0, promptCount: 0,
channels: new Set(), channels: new Set(),
skillPromptAdditions: promptAdditions, skillPromptAdditions: promptAdditions,
sandboxDir,
allowedTools,
}; };
this.sessions.set(sessionId, session); this.sessions.set(sessionId, session);

View File

@@ -2,12 +2,29 @@ import { Type } from '@sinclair/typebox';
import type { ToolDefinition } from '@mariozechner/pi-coding-agent'; import type { ToolDefinition } from '@mariozechner/pi-coding-agent';
import { exec } from 'node:child_process'; import { exec } from 'node:child_process';
import { promisify } from 'node:util'; import { promisify } from 'node:util';
import { resolve, relative } from 'node:path';
const execAsync = promisify(exec); const execAsync = promisify(exec);
const GIT_TIMEOUT_MS = 15_000; const GIT_TIMEOUT_MS = 15_000;
const MAX_OUTPUT_BYTES = 100 * 1024; // 100 KB const MAX_OUTPUT_BYTES = 100 * 1024; // 100 KB
/**
* Clamp a user-supplied cwd to within the sandbox directory.
* If the resolved path escapes the sandbox (via ../ or absolute path outside),
* falls back to the sandbox directory itself.
*/
function clampCwd(sandboxDir: string, requestedCwd?: string): string {
if (!requestedCwd) return sandboxDir;
const resolved = resolve(sandboxDir, requestedCwd);
const rel = relative(sandboxDir, resolved);
if (rel.startsWith('..') || rel.startsWith('/')) {
// Escape attempt — fall back to sandbox root
return sandboxDir;
}
return resolved;
}
async function runGit( async function runGit(
args: string[], args: string[],
cwd?: string, cwd?: string,
@@ -41,17 +58,24 @@ async function runGit(
} }
} }
export function createGitTools(defaultCwd?: string): ToolDefinition[] { export function createGitTools(sandboxDir?: string): ToolDefinition[] {
const defaultCwd = sandboxDir ?? process.cwd();
const gitStatus: ToolDefinition = { const gitStatus: ToolDefinition = {
name: 'git_status', name: 'git_status',
label: 'Git Status', label: 'Git Status',
description: 'Show the working tree status (staged, unstaged, untracked files).', description: 'Show the working tree status (staged, unstaged, untracked files).',
parameters: Type.Object({ parameters: Type.Object({
cwd: Type.Optional(Type.String({ description: 'Repository working directory.' })), cwd: Type.Optional(
Type.String({
description: 'Repository working directory (relative to sandbox or absolute within it).',
}),
),
}), }),
async execute(_toolCallId, params) { async execute(_toolCallId, params) {
const { cwd } = params as { cwd?: string }; const { cwd } = params as { cwd?: string };
const result = await runGit(['status', '--short', '--branch'], cwd ?? defaultCwd); const safeCwd = clampCwd(defaultCwd, cwd);
const result = await runGit(['status', '--short', '--branch'], safeCwd);
const text = result.error const text = result.error
? `Error: ${result.error}\n${result.stderr}` ? `Error: ${result.error}\n${result.stderr}`
: result.stdout || '(no output)'; : result.stdout || '(no output)';
@@ -71,7 +95,11 @@ export function createGitTools(defaultCwd?: string): ToolDefinition[] {
oneline: Type.Optional( oneline: Type.Optional(
Type.Boolean({ description: 'Compact one-line format (default true)' }), Type.Boolean({ description: 'Compact one-line format (default true)' }),
), ),
cwd: Type.Optional(Type.String({ description: 'Repository working directory.' })), cwd: Type.Optional(
Type.String({
description: 'Repository working directory (relative to sandbox or absolute within it).',
}),
),
}), }),
async execute(_toolCallId, params) { async execute(_toolCallId, params) {
const { limit, oneline, cwd } = params as { const { limit, oneline, cwd } = params as {
@@ -79,9 +107,10 @@ export function createGitTools(defaultCwd?: string): ToolDefinition[] {
oneline?: boolean; oneline?: boolean;
cwd?: string; cwd?: string;
}; };
const safeCwd = clampCwd(defaultCwd, cwd);
const args = ['log', `--max-count=${limit ?? 20}`]; const args = ['log', `--max-count=${limit ?? 20}`];
if (oneline !== false) args.push('--oneline'); if (oneline !== false) args.push('--oneline');
const result = await runGit(args, cwd ?? defaultCwd); const result = await runGit(args, safeCwd);
const text = result.error const text = result.error
? `Error: ${result.error}\n${result.stderr}` ? `Error: ${result.error}\n${result.stderr}`
: result.stdout || '(no commits)'; : result.stdout || '(no commits)';
@@ -106,7 +135,11 @@ export function createGitTools(defaultCwd?: string): ToolDefinition[] {
path: Type.Optional( path: Type.Optional(
Type.String({ description: 'Limit diff to a specific file or directory' }), Type.String({ description: 'Limit diff to a specific file or directory' }),
), ),
cwd: Type.Optional(Type.String({ description: 'Repository working directory.' })), cwd: Type.Optional(
Type.String({
description: 'Repository working directory (relative to sandbox or absolute within it).',
}),
),
}), }),
async execute(_toolCallId, params) { async execute(_toolCallId, params) {
const { staged, ref, path, cwd } = params as { const { staged, ref, path, cwd } = params as {
@@ -115,12 +148,13 @@ export function createGitTools(defaultCwd?: string): ToolDefinition[] {
path?: string; path?: string;
cwd?: string; cwd?: string;
}; };
const safeCwd = clampCwd(defaultCwd, cwd);
const args = ['diff']; const args = ['diff'];
if (staged) args.push('--cached'); if (staged) args.push('--cached');
if (ref) args.push(ref); if (ref) args.push(ref);
args.push('--'); args.push('--');
if (path) args.push(path); if (path) args.push(path);
const result = await runGit(args, cwd ?? defaultCwd); const result = await runGit(args, safeCwd);
const text = result.error const text = result.error
? `Error: ${result.error}\n${result.stderr}` ? `Error: ${result.error}\n${result.stderr}`
: result.stdout || '(no diff)'; : result.stdout || '(no diff)';

View File

@@ -1,17 +1,30 @@
import { import {
BadRequestException, BadRequestException,
Body,
Controller, Controller,
Delete,
Get, Get,
HttpCode,
HttpStatus,
Inject, Inject,
NotFoundException, NotFoundException,
Param, Param,
Patch,
Post,
Query, Query,
UseGuards, UseGuards,
} from '@nestjs/common'; } from '@nestjs/common';
import fs from 'node:fs'; import fs from 'node:fs';
import path from 'node:path'; import path from 'node:path';
import { AuthGuard } from '../auth/auth.guard.js'; import { AuthGuard } from '../auth/auth.guard.js';
import { CurrentUser } from '../auth/current-user.decorator.js';
import { CoordService } from './coord.service.js'; import { CoordService } from './coord.service.js';
import type {
CreateDbMissionDto,
UpdateDbMissionDto,
CreateMissionTaskDto,
UpdateMissionTaskDto,
} from './coord.dto.js';
/** Walk up from cwd to find the monorepo root (has pnpm-workspace.yaml). */ /** Walk up from cwd to find the monorepo root (has pnpm-workspace.yaml). */
function findMonorepoRoot(start: string): string { function findMonorepoRoot(start: string): string {
@@ -49,6 +62,8 @@ function resolveAndValidatePath(raw: string | undefined): string {
export class CoordController { export class CoordController {
constructor(@Inject(CoordService) private readonly coordService: CoordService) {} constructor(@Inject(CoordService) private readonly coordService: CoordService) {}
// ── File-based coord endpoints (legacy) ──
@Get('status') @Get('status')
async missionStatus(@Query('projectPath') projectPath?: string) { async missionStatus(@Query('projectPath') projectPath?: string) {
const resolvedPath = resolveAndValidatePath(projectPath); const resolvedPath = resolveAndValidatePath(projectPath);
@@ -70,4 +85,121 @@ export class CoordController {
if (!detail) throw new NotFoundException(`Task ${taskId} not found in coord mission`); if (!detail) throw new NotFoundException(`Task ${taskId} not found in coord mission`);
return detail; return detail;
} }
// ── DB-backed mission endpoints ──
@Get('missions')
async listDbMissions(@CurrentUser() user: { id: string }) {
return this.coordService.getMissionsByUser(user.id);
}
@Get('missions/:id')
async getDbMission(@Param('id') id: string, @CurrentUser() user: { id: string }) {
const mission = await this.coordService.getMissionByIdAndUser(id, user.id);
if (!mission) throw new NotFoundException('Mission not found');
return mission;
}
@Post('missions')
async createDbMission(@Body() dto: CreateDbMissionDto, @CurrentUser() user: { id: string }) {
return this.coordService.createDbMission({
name: dto.name,
description: dto.description,
projectId: dto.projectId,
userId: user.id,
phase: dto.phase,
milestones: dto.milestones,
config: dto.config,
status: dto.status,
});
}
@Patch('missions/:id')
async updateDbMission(
@Param('id') id: string,
@Body() dto: UpdateDbMissionDto,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.updateDbMission(id, user.id, dto);
if (!mission) throw new NotFoundException('Mission not found');
return mission;
}
@Delete('missions/:id')
@HttpCode(HttpStatus.NO_CONTENT)
async deleteDbMission(@Param('id') id: string, @CurrentUser() user: { id: string }) {
const deleted = await this.coordService.deleteDbMission(id, user.id);
if (!deleted) throw new NotFoundException('Mission not found');
}
// ── DB-backed mission task endpoints ──
@Get('missions/:missionId/mission-tasks')
async listMissionTasks(
@Param('missionId') missionId: string,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.getMissionByIdAndUser(missionId, user.id);
if (!mission) throw new NotFoundException('Mission not found');
return this.coordService.getMissionTasksByMissionAndUser(missionId, user.id);
}
@Get('missions/:missionId/mission-tasks/:taskId')
async getMissionTask(
@Param('missionId') missionId: string,
@Param('taskId') taskId: string,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.getMissionByIdAndUser(missionId, user.id);
if (!mission) throw new NotFoundException('Mission not found');
const task = await this.coordService.getMissionTaskByIdAndUser(taskId, user.id);
if (!task) throw new NotFoundException('Mission task not found');
return task;
}
@Post('missions/:missionId/mission-tasks')
async createMissionTask(
@Param('missionId') missionId: string,
@Body() dto: CreateMissionTaskDto,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.getMissionByIdAndUser(missionId, user.id);
if (!mission) throw new NotFoundException('Mission not found');
return this.coordService.createMissionTask({
missionId,
taskId: dto.taskId,
userId: user.id,
status: dto.status,
description: dto.description,
notes: dto.notes,
pr: dto.pr,
});
}
@Patch('missions/:missionId/mission-tasks/:taskId')
async updateMissionTask(
@Param('missionId') missionId: string,
@Param('taskId') taskId: string,
@Body() dto: UpdateMissionTaskDto,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.getMissionByIdAndUser(missionId, user.id);
if (!mission) throw new NotFoundException('Mission not found');
const updated = await this.coordService.updateMissionTask(taskId, user.id, dto);
if (!updated) throw new NotFoundException('Mission task not found');
return updated;
}
@Delete('missions/:missionId/mission-tasks/:taskId')
@HttpCode(HttpStatus.NO_CONTENT)
async deleteMissionTask(
@Param('missionId') missionId: string,
@Param('taskId') taskId: string,
@CurrentUser() user: { id: string },
) {
const mission = await this.coordService.getMissionByIdAndUser(missionId, user.id);
if (!mission) throw new NotFoundException('Mission not found');
const deleted = await this.coordService.deleteMissionTask(taskId, user.id);
if (!deleted) throw new NotFoundException('Mission task not found');
}
} }

View File

@@ -52,6 +52,7 @@ export function TuiApp({
const [availableModels, setAvailableModels] = useState<ModelInfo[]>([]); const [availableModels, setAvailableModels] = useState<ModelInfo[]>([]);
const socketRef = useRef<Socket | null>(null); const socketRef = useRef<Socket | null>(null);
const currentStreamTextRef = useRef('');
// Fetch available models on mount // Fetch available models on mount
useEffect(() => { useEffect(() => {
@@ -102,20 +103,22 @@ export function TuiApp({
socket.on('agent:start', () => { socket.on('agent:start', () => {
setIsStreaming(true); setIsStreaming(true);
currentStreamTextRef.current = '';
setCurrentStreamText(''); setCurrentStreamText('');
}); });
socket.on('agent:text', (data: { text: string }) => { socket.on('agent:text', (data: { text: string }) => {
setCurrentStreamText((prev) => prev + data.text); currentStreamTextRef.current += data.text;
setCurrentStreamText(currentStreamTextRef.current);
}); });
socket.on('agent:end', () => { socket.on('agent:end', () => {
setCurrentStreamText((prev) => { const finalText = currentStreamTextRef.current;
if (prev) { currentStreamTextRef.current = '';
setMessages((msgs) => [...msgs, { role: 'assistant', content: prev }]); setCurrentStreamText('');
} if (finalText) {
return ''; setMessages((msgs) => [...msgs, { role: 'assistant', content: finalText }]);
}); }
setIsStreaming(false); setIsStreaming(false);
}); });