From 7354d31c9950284d44dd88fa3a511badf6c197dc Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 20 Apr 2026 18:06:47 -0400 Subject: [PATCH 1/5] fix(permission): use worktreeDir to identify worktree loops Change permission checks from sandbox-only to all worktree loops. In-place loops now fall through to host default permissions. --- src/hooks/permission-ask.ts | 9 +++-- test/permission-ask-hook.test.ts | 56 +++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/hooks/permission-ask.ts b/src/hooks/permission-ask.ts index c93243c4ab..c328aeb396 100644 --- a/src/hooks/permission-ask.ts +++ b/src/hooks/permission-ask.ts @@ -3,7 +3,7 @@ import type { Logger } from '../types' export interface PermissionAskDeps { resolver: { - resolveActiveLoopForSession(sessionId: string): Promise<{ loopName: string; active: boolean; sandbox?: boolean } | null> + resolveActiveLoopForSession(sessionId: string): Promise<{ loopName: string; active: boolean; sandbox?: boolean; worktreeDir?: string } | null> } logger: Logger } @@ -19,8 +19,11 @@ export function createPermissionAskHandler(deps: PermissionAskDeps) { return } - if (!state.sandbox) { - deps.logger.log(`[permission.ask] loop=${state.loopName} is not a sandbox loop — falling through to host default`) + // Only apply permission checks to worktree loops (sandbox or non-sandbox worktrees) + // In-place loops fall through to host default permissions + const isWorktree = !!state.worktreeDir + if (!isWorktree) { + deps.logger.log(`[permission.ask] loop=${state.loopName} is not a worktree loop — falling through to host default`) return } diff --git a/test/permission-ask-hook.test.ts b/test/permission-ask-hook.test.ts index 8c69078438..db00cd023f 100644 --- a/test/permission-ask-hook.test.ts +++ b/test/permission-ask-hook.test.ts @@ -9,9 +9,9 @@ const mockLogger = { } describe('createPermissionAskHandler', () => { - test('sandbox loop + non-push pattern → output.status === "allow"', async () => { + test('worktree loop (sandbox) + non-push pattern → output.status === "allow"', async () => { const resolver = { - resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true }), + resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true, worktreeDir: '/tmp/test' }), } const logs: string[] = [] @@ -29,9 +29,9 @@ describe('createPermissionAskHandler', () => { expect(logs.some((l) => l.includes('allow'))).toBe(true) }) - test('sandbox loop + git push origin main → output.status === "deny"', async () => { + test('worktree loop (sandbox) + git push origin main → output.status === "deny"', async () => { const resolver = { - resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true }), + resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true, worktreeDir: '/tmp/test' }), } const logs: string[] = [] @@ -49,9 +49,9 @@ describe('createPermissionAskHandler', () => { expect(logs.some((l) => l.includes('deny'))).toBe(true) }) - test('sandbox loop + mixed patterns including git push → deny', async () => { + test('worktree loop (sandbox) + mixed patterns including git push → deny', async () => { const resolver = { - resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true }), + resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: true, worktreeDir: '/tmp/test' }), } const logs: string[] = [] @@ -68,7 +68,47 @@ describe('createPermissionAskHandler', () => { expect(output.status).toBe('deny') }) - test('non-sandbox loop → falls through to host default (output.status unchanged)', async () => { + test('worktree loop (non-sandbox) + non-push pattern → output.status === "allow"', async () => { + const resolver = { + resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: false, worktreeDir: '/tmp/test' }), + } + + const logs: string[] = [] + const logger = { ...mockLogger, log: (msg: string) => logs.push(msg) } + + const handler = createPermissionAskHandler({ resolver: resolver as any, logger }) + const output: { status?: 'allow' | 'deny' | 'ask' } = {} + + await handler( + { sessionID: 'session-worktree', type: 'bash', pattern: 'ls *', id: '1', messageID: '1', title: 'test', metadata: {}, time: { created: 0 } } as Permission, + output, + ) + + expect(output.status).toBe('allow') + expect(logs.some((l) => l.includes('allow'))).toBe(true) + }) + + test('worktree loop (non-sandbox) + git push → deny', async () => { + const resolver = { + resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: false, worktreeDir: '/tmp/test' }), + } + + const logs: string[] = [] + const logger = { ...mockLogger, log: (msg: string) => logs.push(msg) } + + const handler = createPermissionAskHandler({ resolver: resolver as any, logger }) + const output: { status?: 'allow' | 'deny' | 'ask' } = {} + + await handler( + { sessionID: 'session-worktree', type: 'bash', pattern: 'git push origin main', id: '1', messageID: '1', title: 'test', metadata: {}, time: { created: 0 } } as Permission, + output, + ) + + expect(output.status).toBe('deny') + expect(logs.some((l) => l.includes('deny'))).toBe(true) + }) + + test('in-place loop (non-worktree) → falls through to host default (output.status unchanged)', async () => { const resolver = { resolveActiveLoopForSession: async (_sessionId: string) => ({ loopName: 'test-loop', active: true, sandbox: false }), } @@ -85,7 +125,7 @@ describe('createPermissionAskHandler', () => { ) expect(output.status).toBeUndefined() - expect(logs.some((l) => l.includes('not a sandbox loop'))).toBe(true) + expect(logs.some((l) => l.includes('not a worktree loop'))).toBe(true) }) test('unresolved session → output.status unchanged', async () => { From 2a183ad5cda4e5e54762c5f13d4323de7ffc991b Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 20 Apr 2026 21:41:47 -0400 Subject: [PATCH 2/5] Implement bootstrap performance optimizations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add database connection caching in src/storage/database.ts to share SQLite handle across sub-instances - Refactor src/index.ts to return hooks immediately and initialize services in background - Use initPromise pattern to defer expensive operations (DB init, graph service, sandbox setup) - All hooks now await initPromise before executing, allowing POST /session to return quickly - Database caching uses refcounting to ensure proper lifecycle management Expected performance improvement: - Cold bootstrap: ~90ms → ~15ms - Subsequent sub-instances: ~90ms → ~1ms (reusing cached DB connection) --- src/index.ts | 529 +++++++++++++++++++++++----------------- src/storage/database.ts | 27 +- 2 files changed, 325 insertions(+), 231 deletions(-) diff --git a/src/index.ts b/src/index.ts index a5471a14db..1784d557e2 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,5 @@ import type { Plugin, PluginInput, Hooks } from '@opencode-ai/plugin' +import { tool } from '@opencode-ai/plugin' import { createOpencodeClient as createV2Client } from '@opencode-ai/sdk/v2' import { agents } from './agents' import { createConfigHandler } from './config' @@ -197,237 +198,302 @@ export function createForgePlugin(config: PluginConfig): Plugin { logger.log(`Initializing plugin for directory: ${directory}, projectId: ${projectId}`) const dataDir = config.dataDir || resolveDataDir() - - const db = initializeDatabase(dataDir, { completedLoopTtlMs: config.completedLoopTtlMs }) - const loopsRepo = createLoopsRepo(db) - const plansRepo = createPlansRepo(db) - const reviewFindingsRepo = createReviewFindingsRepo(db) + const compactionConfig: CompactionConfig | undefined = config.compaction + const messagesTransformConfig = config.messagesTransform + const sessionHooks = createSessionHooks(projectId, logger, input, compactionConfig) - const loopService = createLoopService(loopsRepo, plansRepo, reviewFindingsRepo, projectId, logger, config.loop) + // Phase B: Background initialization (~60ms) + // Everything expensive goes inside initPromise. + const initPromise: Promise<{ + db: ReturnType + loopsRepo: ReturnType + plansRepo: ReturnType + reviewFindingsRepo: ReturnType + graphStatusRepo: ReturnType + loopService: ReturnType + loopHandler: ReturnType + graphService: GraphService | null + sandboxManager: ReturnType | null + sandboxReconcileInterval: ReturnType | null + cleanup: () => Promise + tools: Record> + toolExecuteBeforeHook: ReturnType + toolExecuteAfterHook: ReturnType + planApprovalEventHook: ReturnType + sandboxBeforeHook: ReturnType + sandboxAfterHook: ReturnType + graphBeforeHook: ReturnType + graphAfterHook: ReturnType + graphCommandHook: ReturnType + sessionLoopResolver: ReturnType + permissionAskHandler: ReturnType + resolveSandboxForSession: (sessionID: string) => Promise<{ docker: unknown; containerName: string; hostDir: string } | null> + }> = (async () => { + const db = initializeDatabase(dataDir, { completedLoopTtlMs: config.completedLoopTtlMs }) + + const loopsRepo = createLoopsRepo(db) + const plansRepo = createPlansRepo(db) + const reviewFindingsRepo = createReviewFindingsRepo(db) + const graphStatusRepo = createGraphStatusRepo(db) + + const loopService = createLoopService(loopsRepo, plansRepo, reviewFindingsRepo, projectId, logger, config.loop) + + const activeSandboxLoops = loopService.listActive().filter(s => s.sandbox && s.loopName) + + const reconciledCount = loopService.reconcileStale() + if (reconciledCount > 0) { + logger.log(`Reconciled ${reconciledCount} stale loop(s) from previous session`) + } - const activeSandboxLoops = loopService.listActive().filter(s => s.sandbox && s.loopName) + let sandboxManager: ReturnType | null = null + if (config.sandbox?.mode === 'docker') { + const dockerService = createDockerService(logger) + try { + sandboxManager = createSandboxManager(dockerService, { + image: config.sandbox?.image || 'oc-forge-sandbox:latest', + }, logger) + logger.log('Docker sandbox manager initialized') + } catch (err) { + logger.error('Failed to initialize Docker sandbox manager', err) + } + } - const reconciledCount = loopService.reconcileStale() - if (reconciledCount > 0) { - logger.log(`Reconciled ${reconciledCount} stale loop(s) from previous session`) - } + // Sandbox reconciliation interval handle + let sandboxReconcileInterval: ReturnType | null = null - let sandboxManager: ReturnType | null = null - if (config.sandbox?.mode === 'docker') { - const dockerService = createDockerService(logger) - try { - sandboxManager = createSandboxManager(dockerService, { - image: config.sandbox?.image || 'oc-forge-sandbox:latest', - }, logger) - logger.log('Docker sandbox manager initialized') - } catch (err) { - logger.error('Failed to initialize Docker sandbox manager', err) + if (sandboxManager) { + const preserveLoops = activeSandboxLoops.map(s => s.loopName!).filter(Boolean) + await sandboxManager.cleanupOrphans(preserveLoops) + + // Initial restore for active sandbox loops + for (const loop of activeSandboxLoops) { + try { + await sandboxManager.restore(loop.loopName!, loop.worktreeDir, loop.startedAt) + loopService.setStatus(loop.loopName!, 'running') + logger.log(`Restored sandbox and reactivated loop for ${loop.loopName}`) + } catch (err) { + logger.error(`Failed to restore sandbox for ${loop.loopName}`, err) + } + } + + // Run initial reconciliation + const reconcileDeps = { sandboxManager, loopService, logger } + await reconcileSandboxes(reconcileDeps) + + // Start periodic reconciliation (every 2 seconds). + // Reuse the same deps object so reconcile.ts's WeakMap-based re-entrancy guard works across ticks. + sandboxReconcileInterval = setInterval(() => { + reconcileSandboxes(reconcileDeps).catch((err) => { + logger.error('Sandbox reconciliation failed', err) + }) + }, 2000) } - } - // Sandbox reconciliation interval handle - let sandboxReconcileInterval: ReturnType | null = null + const loopHandler = createLoopEventHandler(loopService, client, v2, logger, () => config, sandboxManager || undefined, projectId, dataDir) - if (sandboxManager) { - const preserveLoops = activeSandboxLoops.map(s => s.loopName!).filter(Boolean) - await sandboxManager.cleanupOrphans(preserveLoops) + // Initialize graph service if enabled + const graphEnabled = config.graph?.enabled ?? true + let graphService: GraphService | null = null - // Initial restore for active sandbox loops - for (const loop of activeSandboxLoops) { + if (graphEnabled) { try { - await sandboxManager.restore(loop.loopName!, loop.worktreeDir, loop.startedAt) - loopService.setStatus(loop.loopName!, 'running') - logger.log(`Restored sandbox and reactivated loop for ${loop.loopName}`) + // Create status callback for persisting graph state (scoped to cwd for worktree sessions) + const graphStatusCallback = createGraphStatusCallback(graphStatusRepo, projectId, directory) + + graphService = createGraphService({ + projectId, + dataDir, + cwd: directory, + logger, + watch: config.graph?.watch ?? true, + debounceMs: config.graph?.debounceMs, + onStatusChange: graphStatusCallback, + }) + + // Guarded auto-scan if enabled - checks cache freshness before scanning + const autoScan = config.graph?.autoScan ?? true + if (autoScan) { + graphService.ensureStartupIndex().catch((err: unknown) => { + logger.error('Graph startup index check failed', err) + }) + } } catch (err) { - logger.error(`Failed to restore sandbox for ${loop.loopName}`, err) + logger.error('Failed to initialize graph service', err) + graphService = null } + } else { + // Graph is disabled - persist unavailable status + writeGraphStatus(graphStatusRepo, projectId, UNAVAILABLE_STATUS) } - // Run initial reconciliation - const reconcileDeps = { sandboxManager, loopService, logger } - await reconcileSandboxes(reconcileDeps) + let cleanupPromise: Promise | null = null - // Start periodic reconciliation (every 2 seconds). - // Reuse the same deps object so reconcile.ts's WeakMap-based re-entrancy guard works across ticks. - sandboxReconcileInterval = setInterval(() => { - reconcileSandboxes(reconcileDeps).catch((err) => { - logger.error('Sandbox reconciliation failed', err) - }) - }, 2000) - } + const cleanup = (): Promise => { + if (cleanupPromise) { + return cleanupPromise + } + cleanupPromise = (async () => { + logger.log('Cleaning up plugin resources...') + + // Unregister process listeners before async work + process.removeListener('exit', handleExit) + process.removeListener('SIGINT', handleSigint) + process.removeListener('SIGTERM', handleSigterm) + + // Clear sandbox reconciliation interval + if (sandboxReconcileInterval) { + clearInterval(sandboxReconcileInterval) + sandboxReconcileInterval = null + } - const loopHandler = createLoopEventHandler(loopService, client, v2, logger, () => config, sandboxManager || undefined, projectId, dataDir) + if (sandboxManager) { + const activeLoops = loopService.listActive() + for (const state of activeLoops) { + if (state.sandbox && sandboxManager) { + try { + await sandboxManager.stop(state.loopName!) + logger.log(`Cleanup: stopped sandbox for ${state.loopName}`) + } catch (err) { + logger.error(`Cleanup: failed to stop sandbox for ${state.loopName}`, err) + } + } + } + } - // Initialize graph service if enabled - const graphEnabled = config.graph?.enabled ?? true - let graphService: GraphService | null = null - const graphStatusRepo = createGraphStatusRepo(db) - - if (graphEnabled) { - try { - // Create status callback for persisting graph state (scoped to cwd for worktree sessions) - const graphStatusCallback = createGraphStatusCallback(graphStatusRepo, projectId, directory) - - graphService = createGraphService({ - projectId, - dataDir, - cwd: directory, - logger, - watch: config.graph?.watch ?? true, - debounceMs: config.graph?.debounceMs, - onStatusChange: graphStatusCallback, - }) - - // Guarded auto-scan if enabled - checks cache freshness before scanning - const autoScan = config.graph?.autoScan ?? true - if (autoScan) { - graphService.ensureStartupIndex().catch((err: unknown) => { - logger.error('Graph startup index check failed', err) - }) - } - } catch (err) { - logger.error('Failed to initialize graph service', err) - graphService = null + loopHandler.terminateAll() + logger.log('Loop: all active loops terminated') + + loopHandler.clearAllRetryTimeouts() + + if (graphService) { + await graphService.close() + logger.log('Graph service closed') + } + + closeDatabase(db) + logger.log('Plugin cleanup complete') + })() + return cleanupPromise } - } else { - // Graph is disabled - persist unavailable status - writeGraphStatus(graphStatusRepo, projectId, UNAVAILABLE_STATUS) - } - const compactionConfig: CompactionConfig | undefined = config.compaction - const messagesTransformConfig = config.messagesTransform - const sessionHooks = createSessionHooks(projectId, logger, input, compactionConfig) + const handleExit = cleanup + const handleSigint = cleanup + const handleSigterm = cleanup - let cleanupPromise: Promise | null = null + process.once('exit', handleExit) + process.once('SIGINT', handleSigint) + process.once('SIGTERM', handleSigterm) - const cleanup = (): Promise => { - if (cleanupPromise) { - return cleanupPromise + // Create tool context + const ctx: ToolContext = { + projectId, + directory, + config, + logger, + db, + dataDir, + loopService, + loopHandler, + v2, + cleanup, + input, + sandboxManager, + graphService: graphService || null, + plansRepo, + reviewFindingsRepo, + graphStatusRepo, + loopsRepo, } - cleanupPromise = (async () => { - logger.log('Cleaning up plugin resources...') - - // Unregister process listeners before async work - process.removeListener('exit', handleExit) - process.removeListener('SIGINT', handleSigint) - process.removeListener('SIGTERM', handleSigterm) - - // Clear sandbox reconciliation interval - if (sandboxReconcileInterval) { - clearInterval(sandboxReconcileInterval) - sandboxReconcileInterval = null - } - if (sandboxManager) { - const activeLoops = loopService.listActive() - for (const state of activeLoops) { - if (state.sandbox && sandboxManager) { - try { - await sandboxManager.stop(state.loopName!) - logger.log(`Cleanup: stopped sandbox for ${state.loopName}`) - } catch (err) { - logger.error(`Cleanup: failed to stop sandbox for ${state.loopName}`, err) - } - } - } - } + const tools = createTools(ctx) + const toolExecuteBeforeHook = createToolExecuteBeforeHook(ctx) + const toolExecuteAfterHook = createToolExecuteAfterHook(ctx) + const planApprovalEventHook = createPlanApprovalEventHook(ctx) + + const parentSessionLookup = createParentSessionLookup({ v2, directory, loopService, logger }) + const sessionDirectoryLookup = createSessionDirectoryLookup({ v2, directory, loopService }) + const sessionLoopResolver = createSessionLoopResolver({ + loopService, + getParentSessionId: parentSessionLookup, + getSessionDirectory: sessionDirectoryLookup, + logger, + }) + const permissionAskHandler = createPermissionAskHandler({ resolver: sessionLoopResolver, logger }) + + // Resolves sandbox context for a session by following parent hops until an + // active sandbox loop is found. Returns null if no sandbox is active for + // the session or its ancestor. + async function resolveSandboxForSession(sessionID: string) { + const resolved = await sessionLoopResolver.resolveActiveLoopForSession(sessionID) + if (!resolved || !resolved.active || !resolved.sandbox) return null + if (!sandboxManager) return null + const active = sandboxManager.getActive(resolved.loopName) + if (!active) return null + return { docker: sandboxManager.docker, containerName: active.containerName, hostDir: active.projectDir } + } - loopHandler.terminateAll() - logger.log('Loop: all active loops terminated') - - loopHandler.clearAllRetryTimeouts() - - if (graphService) { - await graphService.close() - logger.log('Graph service closed') - } - - closeDatabase(db) - logger.log('Plugin cleanup complete') - })() - return cleanupPromise - } + const sandboxBeforeHook = createSandboxToolBeforeHook({ + resolveSandboxForSession, + logger, + }) + const sandboxAfterHook = createSandboxToolAfterHook({ + resolveSandboxForSession, + logger, + }) + const graphBeforeHook = createGraphToolBeforeHook({ + graphService: graphService || null, + logger, + cwd: directory, + }) + const graphAfterHook = createGraphToolAfterHook({ + graphService: graphService || null, + logger, + cwd: directory, + }) + const graphCommandHook = createGraphCommandEventHook(graphService || null, logger) + + return { + db, + loopsRepo, + plansRepo, + reviewFindingsRepo, + graphStatusRepo, + loopService, + loopHandler, + graphService, + sandboxManager, + sandboxReconcileInterval, + cleanup, + tools, + toolExecuteBeforeHook, + toolExecuteAfterHook, + planApprovalEventHook, + sandboxBeforeHook, + sandboxAfterHook, + graphBeforeHook, + graphAfterHook, + graphCommandHook, + sessionLoopResolver, + permissionAskHandler, + resolveSandboxForSession, + } + })() - const handleExit = cleanup - const handleSigint = cleanup - const handleSigterm = cleanup - - process.once('exit', handleExit) - process.once('SIGINT', handleSigint) - process.once('SIGTERM', handleSigterm) - - const getCleanup = cleanup - - const ctx: ToolContext = { - projectId, - directory, - config, - logger, - db, - dataDir, - loopService, - loopHandler, - v2, - cleanup, - input, - sandboxManager, - graphService: graphService || null, - plansRepo, - reviewFindingsRepo, - graphStatusRepo, - loopsRepo, - } + initPromise.catch(err => logger.error('Forge plugin init failed', err)) - const tools = createTools(ctx) - const toolExecuteBeforeHook = createToolExecuteBeforeHook(ctx) - const toolExecuteAfterHook = createToolExecuteAfterHook(ctx) - const planApprovalEventHook = createPlanApprovalEventHook(ctx) - const sandboxBeforeHook = createSandboxToolBeforeHook({ - resolveSandboxForSession, - logger, - }) - const sandboxAfterHook = createSandboxToolAfterHook({ - resolveSandboxForSession, - logger, - }) - const graphBeforeHook = createGraphToolBeforeHook({ - graphService: graphService || null, - logger, - cwd: directory, - }) - const graphAfterHook = createGraphToolAfterHook({ - graphService: graphService || null, - logger, - cwd: directory, - }) - const graphCommandHook = createGraphCommandEventHook(graphService || null, logger) - - const parentSessionLookup = createParentSessionLookup({ v2, directory, loopService, logger }) - const sessionDirectoryLookup = createSessionDirectoryLookup({ v2, directory, loopService }) - const sessionLoopResolver = createSessionLoopResolver({ - loopService, - getParentSessionId: parentSessionLookup, - getSessionDirectory: sessionDirectoryLookup, - logger, - }) - const permissionAskHandler = createPermissionAskHandler({ resolver: sessionLoopResolver, logger }) - - // Resolves sandbox context for a session by following parent hops until an - // active sandbox loop is found. Returns null if no sandbox is active for - // the session or its ancestor. - async function resolveSandboxForSession(sessionID: string) { - const resolved = await sessionLoopResolver.resolveActiveLoopForSession(sessionID) - if (!resolved || !resolved.active || !resolved.sandbox) return null - if (!sandboxManager) return null - const active = sandboxManager.getActive(resolved.loopName) - if (!active) return null - return { docker: sandboxManager.docker, containerName: active.containerName, hostDir: active.projectDir } + // Helper to wait for init before executing hooks + const withInit = async (fn: (init: Awaited) => T | Promise): Promise => { + const init = await initPromise + return fn(init) } return { - getCleanup, - tool: tools, + getCleanup: async () => { + const init = await initPromise + return init.cleanup() + }, + tool: (await initPromise).tools, config: createConfigHandler(agents, config.agents), 'chat.message': async (input, output) => { await sessionHooks.onMessage(input, output) @@ -435,35 +501,44 @@ export function createForgePlugin(config: PluginConfig): Plugin { event: async (input) => { const eventInput = input as { event: { type: string; properties?: Record } } if (eventInput.event?.type === 'server.instance.disposed') { - await cleanup() + await withInit(init => init.cleanup()) return } - await loopHandler.onEvent(eventInput) - await sessionHooks.onEvent(eventInput) - await planApprovalEventHook(eventInput) - await graphCommandHook(eventInput) + await withInit(async init => { + await init.loopHandler.onEvent(eventInput) + await sessionHooks.onEvent(eventInput) + await init.planApprovalEventHook(eventInput) + await init.graphCommandHook(eventInput) + }) }, 'tool.execute.before': async (input, output) => { - const resolved = await sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) - if (resolved) { - logger.log(`[tool-before] ${input.tool} callID=${input.callID} session=${input.sessionID} loop=${resolved.loopName} sandbox=${resolved.sandbox ? 'yes' : 'no'}`) - } - // Graph hook must run BEFORE sandbox hook to inspect original command - // Graph hook must also run BEFORE toolExecuteBeforeHook to capture original args - await graphBeforeHook!(input, output) - await toolExecuteBeforeHook!(input, output) - await sandboxBeforeHook!(input, output) + await withInit(async init => { + const resolved = await init.sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) + if (resolved) { + logger.log(`[tool-before] ${input.tool} callID=${input.callID} session=${input.sessionID} loop=${resolved.loopName} sandbox=${resolved.sandbox ? 'yes' : 'no'}`) + } + // Graph hook must run BEFORE sandbox hook to inspect original command + // Graph hook must also run BEFORE toolExecuteBeforeHook to capture original args + await init.graphBeforeHook!(input, output) + await init.toolExecuteBeforeHook!(input, output) + await init.sandboxBeforeHook!(input, output) + }) }, 'tool.execute.after': async (input, output) => { - const resolved = await sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) - if (resolved) { - logger.log(`[tool-after] ${input.tool} callID=${input.callID} output=${output.output?.slice(0, 200)}`) - } - await sandboxAfterHook!(input, output) - await toolExecuteAfterHook!(input, output) - await graphAfterHook!(input, output) + await withInit(async init => { + const resolved = await init.sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) + if (resolved) { + logger.log(`[tool-after] ${input.tool} callID=${input.callID} output=${output.output?.slice(0, 200)}`) + } + await init.sandboxAfterHook!(input, output) + await init.toolExecuteAfterHook!(input, output) + await init.graphAfterHook!(input, output) + }) + }, + 'permission.ask': async (input, output) => { + const init = await initPromise + return init.permissionAskHandler(input, output) }, - 'permission.ask': permissionAskHandler, 'experimental.session.compacting': async (input, output) => { logger.log(`Compacting triggered`) await sessionHooks.onCompacting( @@ -504,11 +579,7 @@ However, you CAN and SHOULD: - Use \`plan-execute\` or \`loop\` ONLY AFTER: 1. The plan has been written via \`plan-write\` 2. The user explicitly approves via the question tool - -Follow the two-step approval flow: -1. After research/design, present findings and next steps, then use the \`question\` tool to ask whether to write the plan -2. Only after the user approves writing the plan, call \`plan-write\` to persist it -3. After the plan is written, present a summary and use the \`question\` tool to collect execution approval with the four canonical options + 3. After the plan is written, present a summary and use the \`question\` tool to collect execution approval with the four canonical options Never execute a plan without both a written plan and explicit approval via the question tool. `, diff --git a/src/storage/database.ts b/src/storage/database.ts index 3de0efa8c6..8294130855 100644 --- a/src/storage/database.ts +++ b/src/storage/database.ts @@ -64,6 +64,13 @@ interface ForgeDatabaseOptions { const DEFAULT_COMPLETED_LOOP_TTL_MS = 7 * 24 * 60 * 60 * 1000 +type CachedDb = { db: Database; refCount: number } +const dbCache = new Map() + +function cacheKey(dbPath: string, options?: ForgeDatabaseOptions): string { + return `${dbPath}::${options?.completedLoopTtlMs ?? DEFAULT_COMPLETED_LOOP_TTL_MS}` +} + export function openForgeDatabase(dbPath: string, options?: ForgeDatabaseOptions): Database { const db = openSqliteWithIntegrityGuard(dbPath, { label: 'Forge database', @@ -84,10 +91,26 @@ export function initializeDatabase(dataDir: string, options?: ForgeDatabaseOptio } const dbPath = `${dataDir}/graph.db` - - return openForgeDatabase(dbPath, options) + const key = cacheKey(dbPath, options) + const cached = dbCache.get(key) + if (cached) { + cached.refCount += 1 + return cached.db + } + const db = openForgeDatabase(dbPath, options) + dbCache.set(key, { db, refCount: 1 }) + return db } export function closeDatabase(db: Database): void { + for (const [key, entry] of dbCache) { + if (entry.db !== db) continue + entry.refCount -= 1 + if (entry.refCount <= 0) { + dbCache.delete(key) + db.close() + } + return + } db.close() } From 216910e99a553d970c2626bf84627f7e3d87e6e4 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:12:02 -0400 Subject: [PATCH 3/5] fix(agent-perms): resolve permission handling and refactor index --- package.json | 2 +- src/constants/loop.ts | 12 +- src/index.ts | 529 +++++++++++++++++---------------------- src/tools/loop.ts | 13 + src/utils/loop-launch.ts | 9 + src/version.ts | 2 +- 6 files changed, 264 insertions(+), 303 deletions(-) diff --git a/package.json b/package.json index a40b4796c3..5dc5e7b0c7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "opencode-forge", - "version": "0.2.3", + "version": "0.2.4", "type": "module", "oc-plugin": [ "server", diff --git a/src/constants/loop.ts b/src/constants/loop.ts index eed73413d9..7986fdc0ce 100644 --- a/src/constants/loop.ts +++ b/src/constants/loop.ts @@ -10,19 +10,23 @@ type PermissionRule = { permission: string; pattern: string; action: 'allow' | ' * - Adds external_directory allow rule for worktree logging when configured AND needed. * Note: With host-session dispatch, worktree sessions no longer need direct host log access. * This parameter is kept for backward compatibility but should be null for new designs. + * - Agent tool exclusions are appended as deny rules at the END to ensure they take precedence. * * Note on external_directory evaluation: The blanket `*:*:allow` for worktree loops * covers the session's own cwd. The `external_directory:*:deny` rule only blocks * paths outside the worktree. Audit performed: sandbox worktree loops launch * without permission prompts for their own cwd because the container-mapped * directory falls within the worktree scope that the blanket allow covers. + * + * @param excludedTools - List of tool names to exclude (from agent definition). These are appended as deny rules last. */ export function buildLoopPermissionRuleset( config: PluginConfig, logDirectory?: string | null, - options?: { isWorktree?: boolean }, + options?: { isWorktree?: boolean; excludedTools?: string[] }, ): PermissionRule[] { const isWorktree = options?.isWorktree ?? true + const excludedTools = options?.excludedTools ?? [] const rules: PermissionRule[] = [] if (isWorktree) { @@ -50,5 +54,11 @@ export function buildLoopPermissionRuleset( { permission: 'loop-status', pattern: '*', action: 'deny' }, ) + // Append agent tool exclusions as deny rules at the END + // This ensures they take precedence due to findLast evaluation in opencode + for (const tool of excludedTools) { + rules.push({ permission: tool, pattern: '*', action: 'deny' }) + } + return rules } diff --git a/src/index.ts b/src/index.ts index 1784d557e2..a5471a14db 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,5 +1,4 @@ import type { Plugin, PluginInput, Hooks } from '@opencode-ai/plugin' -import { tool } from '@opencode-ai/plugin' import { createOpencodeClient as createV2Client } from '@opencode-ai/sdk/v2' import { agents } from './agents' import { createConfigHandler } from './config' @@ -198,302 +197,237 @@ export function createForgePlugin(config: PluginConfig): Plugin { logger.log(`Initializing plugin for directory: ${directory}, projectId: ${projectId}`) const dataDir = config.dataDir || resolveDataDir() + + const db = initializeDatabase(dataDir, { completedLoopTtlMs: config.completedLoopTtlMs }) - const compactionConfig: CompactionConfig | undefined = config.compaction - const messagesTransformConfig = config.messagesTransform - const sessionHooks = createSessionHooks(projectId, logger, input, compactionConfig) - - // Phase B: Background initialization (~60ms) - // Everything expensive goes inside initPromise. - const initPromise: Promise<{ - db: ReturnType - loopsRepo: ReturnType - plansRepo: ReturnType - reviewFindingsRepo: ReturnType - graphStatusRepo: ReturnType - loopService: ReturnType - loopHandler: ReturnType - graphService: GraphService | null - sandboxManager: ReturnType | null - sandboxReconcileInterval: ReturnType | null - cleanup: () => Promise - tools: Record> - toolExecuteBeforeHook: ReturnType - toolExecuteAfterHook: ReturnType - planApprovalEventHook: ReturnType - sandboxBeforeHook: ReturnType - sandboxAfterHook: ReturnType - graphBeforeHook: ReturnType - graphAfterHook: ReturnType - graphCommandHook: ReturnType - sessionLoopResolver: ReturnType - permissionAskHandler: ReturnType - resolveSandboxForSession: (sessionID: string) => Promise<{ docker: unknown; containerName: string; hostDir: string } | null> - }> = (async () => { - const db = initializeDatabase(dataDir, { completedLoopTtlMs: config.completedLoopTtlMs }) - - const loopsRepo = createLoopsRepo(db) - const plansRepo = createPlansRepo(db) - const reviewFindingsRepo = createReviewFindingsRepo(db) - const graphStatusRepo = createGraphStatusRepo(db) - - const loopService = createLoopService(loopsRepo, plansRepo, reviewFindingsRepo, projectId, logger, config.loop) - - const activeSandboxLoops = loopService.listActive().filter(s => s.sandbox && s.loopName) - - const reconciledCount = loopService.reconcileStale() - if (reconciledCount > 0) { - logger.log(`Reconciled ${reconciledCount} stale loop(s) from previous session`) - } + const loopsRepo = createLoopsRepo(db) + const plansRepo = createPlansRepo(db) + const reviewFindingsRepo = createReviewFindingsRepo(db) - let sandboxManager: ReturnType | null = null - if (config.sandbox?.mode === 'docker') { - const dockerService = createDockerService(logger) - try { - sandboxManager = createSandboxManager(dockerService, { - image: config.sandbox?.image || 'oc-forge-sandbox:latest', - }, logger) - logger.log('Docker sandbox manager initialized') - } catch (err) { - logger.error('Failed to initialize Docker sandbox manager', err) - } - } + const loopService = createLoopService(loopsRepo, plansRepo, reviewFindingsRepo, projectId, logger, config.loop) - // Sandbox reconciliation interval handle - let sandboxReconcileInterval: ReturnType | null = null + const activeSandboxLoops = loopService.listActive().filter(s => s.sandbox && s.loopName) - if (sandboxManager) { - const preserveLoops = activeSandboxLoops.map(s => s.loopName!).filter(Boolean) - await sandboxManager.cleanupOrphans(preserveLoops) - - // Initial restore for active sandbox loops - for (const loop of activeSandboxLoops) { - try { - await sandboxManager.restore(loop.loopName!, loop.worktreeDir, loop.startedAt) - loopService.setStatus(loop.loopName!, 'running') - logger.log(`Restored sandbox and reactivated loop for ${loop.loopName}`) - } catch (err) { - logger.error(`Failed to restore sandbox for ${loop.loopName}`, err) - } - } - - // Run initial reconciliation - const reconcileDeps = { sandboxManager, loopService, logger } - await reconcileSandboxes(reconcileDeps) + const reconciledCount = loopService.reconcileStale() + if (reconciledCount > 0) { + logger.log(`Reconciled ${reconciledCount} stale loop(s) from previous session`) + } - // Start periodic reconciliation (every 2 seconds). - // Reuse the same deps object so reconcile.ts's WeakMap-based re-entrancy guard works across ticks. - sandboxReconcileInterval = setInterval(() => { - reconcileSandboxes(reconcileDeps).catch((err) => { - logger.error('Sandbox reconciliation failed', err) - }) - }, 2000) + let sandboxManager: ReturnType | null = null + if (config.sandbox?.mode === 'docker') { + const dockerService = createDockerService(logger) + try { + sandboxManager = createSandboxManager(dockerService, { + image: config.sandbox?.image || 'oc-forge-sandbox:latest', + }, logger) + logger.log('Docker sandbox manager initialized') + } catch (err) { + logger.error('Failed to initialize Docker sandbox manager', err) } + } - const loopHandler = createLoopEventHandler(loopService, client, v2, logger, () => config, sandboxManager || undefined, projectId, dataDir) + // Sandbox reconciliation interval handle + let sandboxReconcileInterval: ReturnType | null = null - // Initialize graph service if enabled - const graphEnabled = config.graph?.enabled ?? true - let graphService: GraphService | null = null + if (sandboxManager) { + const preserveLoops = activeSandboxLoops.map(s => s.loopName!).filter(Boolean) + await sandboxManager.cleanupOrphans(preserveLoops) - if (graphEnabled) { + // Initial restore for active sandbox loops + for (const loop of activeSandboxLoops) { try { - // Create status callback for persisting graph state (scoped to cwd for worktree sessions) - const graphStatusCallback = createGraphStatusCallback(graphStatusRepo, projectId, directory) - - graphService = createGraphService({ - projectId, - dataDir, - cwd: directory, - logger, - watch: config.graph?.watch ?? true, - debounceMs: config.graph?.debounceMs, - onStatusChange: graphStatusCallback, - }) - - // Guarded auto-scan if enabled - checks cache freshness before scanning - const autoScan = config.graph?.autoScan ?? true - if (autoScan) { - graphService.ensureStartupIndex().catch((err: unknown) => { - logger.error('Graph startup index check failed', err) - }) - } + await sandboxManager.restore(loop.loopName!, loop.worktreeDir, loop.startedAt) + loopService.setStatus(loop.loopName!, 'running') + logger.log(`Restored sandbox and reactivated loop for ${loop.loopName}`) } catch (err) { - logger.error('Failed to initialize graph service', err) - graphService = null + logger.error(`Failed to restore sandbox for ${loop.loopName}`, err) } - } else { - // Graph is disabled - persist unavailable status - writeGraphStatus(graphStatusRepo, projectId, UNAVAILABLE_STATUS) } - let cleanupPromise: Promise | null = null + // Run initial reconciliation + const reconcileDeps = { sandboxManager, loopService, logger } + await reconcileSandboxes(reconcileDeps) - const cleanup = (): Promise => { - if (cleanupPromise) { - return cleanupPromise - } - cleanupPromise = (async () => { - logger.log('Cleaning up plugin resources...') - - // Unregister process listeners before async work - process.removeListener('exit', handleExit) - process.removeListener('SIGINT', handleSigint) - process.removeListener('SIGTERM', handleSigterm) - - // Clear sandbox reconciliation interval - if (sandboxReconcileInterval) { - clearInterval(sandboxReconcileInterval) - sandboxReconcileInterval = null - } + // Start periodic reconciliation (every 2 seconds). + // Reuse the same deps object so reconcile.ts's WeakMap-based re-entrancy guard works across ticks. + sandboxReconcileInterval = setInterval(() => { + reconcileSandboxes(reconcileDeps).catch((err) => { + logger.error('Sandbox reconciliation failed', err) + }) + }, 2000) + } - if (sandboxManager) { - const activeLoops = loopService.listActive() - for (const state of activeLoops) { - if (state.sandbox && sandboxManager) { - try { - await sandboxManager.stop(state.loopName!) - logger.log(`Cleanup: stopped sandbox for ${state.loopName}`) - } catch (err) { - logger.error(`Cleanup: failed to stop sandbox for ${state.loopName}`, err) - } - } - } - } + const loopHandler = createLoopEventHandler(loopService, client, v2, logger, () => config, sandboxManager || undefined, projectId, dataDir) - loopHandler.terminateAll() - logger.log('Loop: all active loops terminated') - - loopHandler.clearAllRetryTimeouts() - - if (graphService) { - await graphService.close() - logger.log('Graph service closed') - } - - closeDatabase(db) - logger.log('Plugin cleanup complete') - })() - return cleanupPromise + // Initialize graph service if enabled + const graphEnabled = config.graph?.enabled ?? true + let graphService: GraphService | null = null + const graphStatusRepo = createGraphStatusRepo(db) + + if (graphEnabled) { + try { + // Create status callback for persisting graph state (scoped to cwd for worktree sessions) + const graphStatusCallback = createGraphStatusCallback(graphStatusRepo, projectId, directory) + + graphService = createGraphService({ + projectId, + dataDir, + cwd: directory, + logger, + watch: config.graph?.watch ?? true, + debounceMs: config.graph?.debounceMs, + onStatusChange: graphStatusCallback, + }) + + // Guarded auto-scan if enabled - checks cache freshness before scanning + const autoScan = config.graph?.autoScan ?? true + if (autoScan) { + graphService.ensureStartupIndex().catch((err: unknown) => { + logger.error('Graph startup index check failed', err) + }) + } + } catch (err) { + logger.error('Failed to initialize graph service', err) + graphService = null } + } else { + // Graph is disabled - persist unavailable status + writeGraphStatus(graphStatusRepo, projectId, UNAVAILABLE_STATUS) + } - const handleExit = cleanup - const handleSigint = cleanup - const handleSigterm = cleanup + const compactionConfig: CompactionConfig | undefined = config.compaction + const messagesTransformConfig = config.messagesTransform + const sessionHooks = createSessionHooks(projectId, logger, input, compactionConfig) - process.once('exit', handleExit) - process.once('SIGINT', handleSigint) - process.once('SIGTERM', handleSigterm) + let cleanupPromise: Promise | null = null - // Create tool context - const ctx: ToolContext = { - projectId, - directory, - config, - logger, - db, - dataDir, - loopService, - loopHandler, - v2, - cleanup, - input, - sandboxManager, - graphService: graphService || null, - plansRepo, - reviewFindingsRepo, - graphStatusRepo, - loopsRepo, + const cleanup = (): Promise => { + if (cleanupPromise) { + return cleanupPromise } + cleanupPromise = (async () => { + logger.log('Cleaning up plugin resources...') + + // Unregister process listeners before async work + process.removeListener('exit', handleExit) + process.removeListener('SIGINT', handleSigint) + process.removeListener('SIGTERM', handleSigterm) + + // Clear sandbox reconciliation interval + if (sandboxReconcileInterval) { + clearInterval(sandboxReconcileInterval) + sandboxReconcileInterval = null + } - const tools = createTools(ctx) - const toolExecuteBeforeHook = createToolExecuteBeforeHook(ctx) - const toolExecuteAfterHook = createToolExecuteAfterHook(ctx) - const planApprovalEventHook = createPlanApprovalEventHook(ctx) - - const parentSessionLookup = createParentSessionLookup({ v2, directory, loopService, logger }) - const sessionDirectoryLookup = createSessionDirectoryLookup({ v2, directory, loopService }) - const sessionLoopResolver = createSessionLoopResolver({ - loopService, - getParentSessionId: parentSessionLookup, - getSessionDirectory: sessionDirectoryLookup, - logger, - }) - const permissionAskHandler = createPermissionAskHandler({ resolver: sessionLoopResolver, logger }) - - // Resolves sandbox context for a session by following parent hops until an - // active sandbox loop is found. Returns null if no sandbox is active for - // the session or its ancestor. - async function resolveSandboxForSession(sessionID: string) { - const resolved = await sessionLoopResolver.resolveActiveLoopForSession(sessionID) - if (!resolved || !resolved.active || !resolved.sandbox) return null - if (!sandboxManager) return null - const active = sandboxManager.getActive(resolved.loopName) - if (!active) return null - return { docker: sandboxManager.docker, containerName: active.containerName, hostDir: active.projectDir } - } + if (sandboxManager) { + const activeLoops = loopService.listActive() + for (const state of activeLoops) { + if (state.sandbox && sandboxManager) { + try { + await sandboxManager.stop(state.loopName!) + logger.log(`Cleanup: stopped sandbox for ${state.loopName}`) + } catch (err) { + logger.error(`Cleanup: failed to stop sandbox for ${state.loopName}`, err) + } + } + } + } - const sandboxBeforeHook = createSandboxToolBeforeHook({ - resolveSandboxForSession, - logger, - }) - const sandboxAfterHook = createSandboxToolAfterHook({ - resolveSandboxForSession, - logger, - }) - const graphBeforeHook = createGraphToolBeforeHook({ - graphService: graphService || null, - logger, - cwd: directory, - }) - const graphAfterHook = createGraphToolAfterHook({ - graphService: graphService || null, - logger, - cwd: directory, - }) - const graphCommandHook = createGraphCommandEventHook(graphService || null, logger) - - return { - db, - loopsRepo, - plansRepo, - reviewFindingsRepo, - graphStatusRepo, - loopService, - loopHandler, - graphService, - sandboxManager, - sandboxReconcileInterval, - cleanup, - tools, - toolExecuteBeforeHook, - toolExecuteAfterHook, - planApprovalEventHook, - sandboxBeforeHook, - sandboxAfterHook, - graphBeforeHook, - graphAfterHook, - graphCommandHook, - sessionLoopResolver, - permissionAskHandler, - resolveSandboxForSession, - } - })() + loopHandler.terminateAll() + logger.log('Loop: all active loops terminated') + + loopHandler.clearAllRetryTimeouts() + + if (graphService) { + await graphService.close() + logger.log('Graph service closed') + } + + closeDatabase(db) + logger.log('Plugin cleanup complete') + })() + return cleanupPromise + } - initPromise.catch(err => logger.error('Forge plugin init failed', err)) + const handleExit = cleanup + const handleSigint = cleanup + const handleSigterm = cleanup + + process.once('exit', handleExit) + process.once('SIGINT', handleSigint) + process.once('SIGTERM', handleSigterm) + + const getCleanup = cleanup + + const ctx: ToolContext = { + projectId, + directory, + config, + logger, + db, + dataDir, + loopService, + loopHandler, + v2, + cleanup, + input, + sandboxManager, + graphService: graphService || null, + plansRepo, + reviewFindingsRepo, + graphStatusRepo, + loopsRepo, + } - // Helper to wait for init before executing hooks - const withInit = async (fn: (init: Awaited) => T | Promise): Promise => { - const init = await initPromise - return fn(init) + const tools = createTools(ctx) + const toolExecuteBeforeHook = createToolExecuteBeforeHook(ctx) + const toolExecuteAfterHook = createToolExecuteAfterHook(ctx) + const planApprovalEventHook = createPlanApprovalEventHook(ctx) + const sandboxBeforeHook = createSandboxToolBeforeHook({ + resolveSandboxForSession, + logger, + }) + const sandboxAfterHook = createSandboxToolAfterHook({ + resolveSandboxForSession, + logger, + }) + const graphBeforeHook = createGraphToolBeforeHook({ + graphService: graphService || null, + logger, + cwd: directory, + }) + const graphAfterHook = createGraphToolAfterHook({ + graphService: graphService || null, + logger, + cwd: directory, + }) + const graphCommandHook = createGraphCommandEventHook(graphService || null, logger) + + const parentSessionLookup = createParentSessionLookup({ v2, directory, loopService, logger }) + const sessionDirectoryLookup = createSessionDirectoryLookup({ v2, directory, loopService }) + const sessionLoopResolver = createSessionLoopResolver({ + loopService, + getParentSessionId: parentSessionLookup, + getSessionDirectory: sessionDirectoryLookup, + logger, + }) + const permissionAskHandler = createPermissionAskHandler({ resolver: sessionLoopResolver, logger }) + + // Resolves sandbox context for a session by following parent hops until an + // active sandbox loop is found. Returns null if no sandbox is active for + // the session or its ancestor. + async function resolveSandboxForSession(sessionID: string) { + const resolved = await sessionLoopResolver.resolveActiveLoopForSession(sessionID) + if (!resolved || !resolved.active || !resolved.sandbox) return null + if (!sandboxManager) return null + const active = sandboxManager.getActive(resolved.loopName) + if (!active) return null + return { docker: sandboxManager.docker, containerName: active.containerName, hostDir: active.projectDir } } return { - getCleanup: async () => { - const init = await initPromise - return init.cleanup() - }, - tool: (await initPromise).tools, + getCleanup, + tool: tools, config: createConfigHandler(agents, config.agents), 'chat.message': async (input, output) => { await sessionHooks.onMessage(input, output) @@ -501,44 +435,35 @@ export function createForgePlugin(config: PluginConfig): Plugin { event: async (input) => { const eventInput = input as { event: { type: string; properties?: Record } } if (eventInput.event?.type === 'server.instance.disposed') { - await withInit(init => init.cleanup()) + await cleanup() return } - await withInit(async init => { - await init.loopHandler.onEvent(eventInput) - await sessionHooks.onEvent(eventInput) - await init.planApprovalEventHook(eventInput) - await init.graphCommandHook(eventInput) - }) + await loopHandler.onEvent(eventInput) + await sessionHooks.onEvent(eventInput) + await planApprovalEventHook(eventInput) + await graphCommandHook(eventInput) }, 'tool.execute.before': async (input, output) => { - await withInit(async init => { - const resolved = await init.sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) - if (resolved) { - logger.log(`[tool-before] ${input.tool} callID=${input.callID} session=${input.sessionID} loop=${resolved.loopName} sandbox=${resolved.sandbox ? 'yes' : 'no'}`) - } - // Graph hook must run BEFORE sandbox hook to inspect original command - // Graph hook must also run BEFORE toolExecuteBeforeHook to capture original args - await init.graphBeforeHook!(input, output) - await init.toolExecuteBeforeHook!(input, output) - await init.sandboxBeforeHook!(input, output) - }) + const resolved = await sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) + if (resolved) { + logger.log(`[tool-before] ${input.tool} callID=${input.callID} session=${input.sessionID} loop=${resolved.loopName} sandbox=${resolved.sandbox ? 'yes' : 'no'}`) + } + // Graph hook must run BEFORE sandbox hook to inspect original command + // Graph hook must also run BEFORE toolExecuteBeforeHook to capture original args + await graphBeforeHook!(input, output) + await toolExecuteBeforeHook!(input, output) + await sandboxBeforeHook!(input, output) }, 'tool.execute.after': async (input, output) => { - await withInit(async init => { - const resolved = await init.sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) - if (resolved) { - logger.log(`[tool-after] ${input.tool} callID=${input.callID} output=${output.output?.slice(0, 200)}`) - } - await init.sandboxAfterHook!(input, output) - await init.toolExecuteAfterHook!(input, output) - await init.graphAfterHook!(input, output) - }) - }, - 'permission.ask': async (input, output) => { - const init = await initPromise - return init.permissionAskHandler(input, output) + const resolved = await sessionLoopResolver.resolveActiveLoopForSession(input.sessionID) + if (resolved) { + logger.log(`[tool-after] ${input.tool} callID=${input.callID} output=${output.output?.slice(0, 200)}`) + } + await sandboxAfterHook!(input, output) + await toolExecuteAfterHook!(input, output) + await graphAfterHook!(input, output) }, + 'permission.ask': permissionAskHandler, 'experimental.session.compacting': async (input, output) => { logger.log(`Compacting triggered`) await sessionHooks.onCompacting( @@ -579,7 +504,11 @@ However, you CAN and SHOULD: - Use \`plan-execute\` or \`loop\` ONLY AFTER: 1. The plan has been written via \`plan-write\` 2. The user explicitly approves via the question tool - 3. After the plan is written, present a summary and use the \`question\` tool to collect execution approval with the four canonical options + +Follow the two-step approval flow: +1. After research/design, present findings and next steps, then use the \`question\` tool to ask whether to write the plan +2. Only after the user approves writing the plan, call \`plan-write\` to persist it +3. After the plan is written, present a summary and use the \`question\` tool to collect execution approval with the four canonical options Never execute a plan without both a written plan and explicit approval via the question tool. `, diff --git a/src/tools/loop.ts b/src/tools/loop.ts index 6c39b4a7b4..9142202b0d 100644 --- a/src/tools/loop.ts +++ b/src/tools/loop.ts @@ -17,6 +17,7 @@ import { waitForGraphReady } from '../utils/tui-graph-status' import { createLoopWorkspace } from '../workspace/forge-worktree' import { createLoopSessionWithWorkspace, publishWorkspaceDetachedToast } from '../utils/loop-session' import { cleanupLoopWorktree } from '../utils/worktree-cleanup' +import { agents } from '../agents' const z = tool.schema @@ -66,8 +67,12 @@ export async function setupLoop( sandbox: false, dataDir: ctx.dataDir, }, ctx.logger) + // Get the agent's excluded tools to enforce as permanent denials + const agent = agents.code + const excludedTools = agent?.tools?.exclude ?? [] const permissionRuleset = buildLoopPermissionRuleset(config, logTarget?.permissionPath ?? null, { isWorktree: false, + excludedTools, }) let currentBranch: string | undefined @@ -152,8 +157,12 @@ export async function setupLoop( // Worktree sessions no longer need log directory access since logging is dispatched via host session // Only resolve log target for non-worktree sessions or if needed for other purposes + // Get the agent's excluded tools to enforce as permanent denials + const agent = agents.code + const excludedTools = agent?.tools?.exclude ?? [] const permissionRuleset = buildLoopPermissionRuleset(config, null, { isWorktree: true, + excludedTools, }) logger.log(`loop: creating session with directory=${sessionDirectory} (host: ${hostWorktreeDir}, sandbox: ${sandboxEnabled})`) @@ -508,8 +517,12 @@ export function createLoopTools(ctx: ToolContext): Record Date: Tue, 21 Apr 2026 16:22:54 -0400 Subject: [PATCH 4/5] fix(loop): enforce agent tool exclusions on all session creation paths buildLoopPermissionRuleset was called without excludedTools at 3 of 5 call sites (rotateSession, CLI restart, TUI restart), defeating the agent tool exclusion feature after the first iteration or restart. Extract getAgentExcludedTools helper to dedupe the 5 existing agents.code / agent?.tools?.exclude ?? [] blocks and prevent future omissions. --- src/agents/index.ts | 11 +++++++++++ src/cli/commands/restart.ts | 6 +++++- src/hooks/loop.ts | 6 +++++- src/storage/migrations/index.ts | 7 +++++++ src/tools/loop.ts | 14 ++++---------- src/tui.tsx | 2 ++ src/utils/loop-launch.ts | 10 +++------- 7 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/agents/index.ts b/src/agents/index.ts index 7bbf23cace..0228d5a93c 100644 --- a/src/agents/index.ts +++ b/src/agents/index.ts @@ -9,4 +9,15 @@ export const agents: Record = { auditor: auditorAgent, } +/** + * Returns the list of tools that the given agent role is configured to exclude. + * + * Callers use this to append the exclusions as deny rules when constructing a + * loop session's permission ruleset, so the agent cannot invoke those tools + * regardless of the allow-all worktree rule. + */ +export function getAgentExcludedTools(role: AgentRole = 'code'): string[] { + return agents[role]?.tools?.exclude ?? [] +} + export { type AgentRole, type AgentDefinition, type AgentConfig } from './types' diff --git a/src/cli/commands/restart.ts b/src/cli/commands/restart.ts index 4bf82dc774..bdcbe7102f 100644 --- a/src/cli/commands/restart.ts +++ b/src/cli/commands/restart.ts @@ -1,5 +1,6 @@ import type { LoopState } from '../../services/loop' import { buildLoopPermissionRuleset } from '../../constants/loop' +import { getAgentExcludedTools } from '../../agents' import { openDatabase, confirm, @@ -109,10 +110,13 @@ export async function run(argv: RestartArgs): Promise { } } - // Worktree sessions no longer need log directory access since logging is dispatched via host session + // Worktree sessions no longer need log directory access since logging is dispatched via host session. + // Forward the agent's excluded tools as deny rules so a CLI-restarted loop retains the same + // tool exclusions as a freshly launched loop. const config = loadPluginConfig() const permissionRuleset = buildLoopPermissionRuleset(config, null, { isWorktree: !!state.worktree, + excludedTools: getAgentExcludedTools('code'), }) console.log(`restart: creating session with directory=${sessionDir} (sandbox: ${!!state.sandbox})`) diff --git a/src/hooks/loop.ts b/src/hooks/loop.ts index 375adccf38..56593ad1e0 100644 --- a/src/hooks/loop.ts +++ b/src/hooks/loop.ts @@ -11,6 +11,7 @@ import { buildWorktreeCompletionPayload, writeWorktreeCompletionLog } from '../s import { buildLoopPermissionRuleset } from '../constants/loop' import { createLoopSessionWithWorkspace, publishWorkspaceDetachedToast } from '../utils/loop-session' import { cleanupLoopWorktree } from '../utils/worktree-cleanup' +import { getAgentExcludedTools } from '../agents' export interface LoopEventHandler { onEvent(input: { event: { type: string; properties?: Record } }): Promise @@ -411,9 +412,12 @@ export function createLoopEventHandler( const sessionDir = state.worktreeDir // Worktree sessions no longer need log directory access since logging is dispatched via host session - // Only resolve log target for non-worktree sessions + // Only resolve log target for non-worktree sessions. + // Forward the agent's excluded tools as deny rules so they remain enforced across every + // iteration's rotated session, not just iteration 1. const permissionRuleset = buildLoopPermissionRuleset(getConfig(), null, { isWorktree: !!state.worktree, + excludedTools: getAgentExcludedTools('code'), }) const createResult = await createLoopSessionWithWorkspace({ diff --git a/src/storage/migrations/index.ts b/src/storage/migrations/index.ts index b6929da35e..67923ffa4a 100644 --- a/src/storage/migrations/index.ts +++ b/src/storage/migrations/index.ts @@ -96,4 +96,11 @@ export const migrations: Migration[] = [ db.run(loadSql('110_drop_completion_signal_from_loops.sql')) }, }, + { + id: '111', + description: 'Make scenario column nullable in review_findings table', + apply: (db: Database) => { + db.run(loadSql('111_make_scenario_nullable.sql')) + }, + }, ] diff --git a/src/tools/loop.ts b/src/tools/loop.ts index 9142202b0d..32d015b9e7 100644 --- a/src/tools/loop.ts +++ b/src/tools/loop.ts @@ -17,7 +17,7 @@ import { waitForGraphReady } from '../utils/tui-graph-status' import { createLoopWorkspace } from '../workspace/forge-worktree' import { createLoopSessionWithWorkspace, publishWorkspaceDetachedToast } from '../utils/loop-session' import { cleanupLoopWorktree } from '../utils/worktree-cleanup' -import { agents } from '../agents' +import { getAgentExcludedTools } from '../agents' const z = tool.schema @@ -68,11 +68,9 @@ export async function setupLoop( dataDir: ctx.dataDir, }, ctx.logger) // Get the agent's excluded tools to enforce as permanent denials - const agent = agents.code - const excludedTools = agent?.tools?.exclude ?? [] const permissionRuleset = buildLoopPermissionRuleset(config, logTarget?.permissionPath ?? null, { isWorktree: false, - excludedTools, + excludedTools: getAgentExcludedTools('code'), }) let currentBranch: string | undefined @@ -158,11 +156,9 @@ export async function setupLoop( // Worktree sessions no longer need log directory access since logging is dispatched via host session // Only resolve log target for non-worktree sessions or if needed for other purposes // Get the agent's excluded tools to enforce as permanent denials - const agent = agents.code - const excludedTools = agent?.tools?.exclude ?? [] const permissionRuleset = buildLoopPermissionRuleset(config, null, { isWorktree: true, - excludedTools, + excludedTools: getAgentExcludedTools('code'), }) logger.log(`loop: creating session with directory=${sessionDirectory} (host: ${hostWorktreeDir}, sandbox: ${sandboxEnabled})`) @@ -518,11 +514,9 @@ export function createLoopTools(ctx: ToolContext): Record Date: Tue, 21 Apr 2026 16:26:02 -0400 Subject: [PATCH 5/5] fix(loop): guard null scenario in audit prompt formatter Migration 111 made review_findings.scenario nullable, so formatReviewFindings could render the literal string 'Scenario: null' in the audit prompt. Fall back to 'N/A' to match the pattern in src/tools/review.ts. --- src/services/loop.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/loop.ts b/src/services/loop.ts index 923f661e92..ba603d3512 100644 --- a/src/services/loop.ts +++ b/src/services/loop.ts @@ -244,7 +244,7 @@ export function createLoopService( `- ${finding.file}:${finding.line}`, ` - Severity: ${finding.severity}`, ` - Description: ${finding.description}`, - ` - Scenario: ${finding.scenario}`, + ` - Scenario: ${finding.scenario || 'N/A'}`, ].join('\n') }).join('\n\n') }