refactor(agents): remove preExecute hook - move postInitialPRComment to ack infrastructure

src/agents/definitions/contextSteps.ts

@@ -7,7 +7,6 @@
 
 import { execFileSync } from 'node:child_process';
 
-import { INITIAL_MESSAGES } from '../../config/agentMessages.js';
 import { ListDirectory } from '../../gadgets/ListDirectory.js';
 import { formatCheckStatus } from '../../gadgets/github/core/getPRChecks.js';
 import { readWorkItem } from '../../gadgets/pm/core/readWorkItem.js';
@@ -37,11 +36,6 @@ export interface FetchContextParams {
 	logWriter: LogWriter;
 }
 
-export interface PreExecuteParams {
-	input: AgentInput;
-	logWriter: LogWriter;
-}
-
 // ============================================================================
 // Atomic context step functions
 // ============================================================================
@@ -267,25 +261,3 @@ export function fetchEmailsFromInputStep(params: FetchContextParams): ContextInj
 		},
 	];
 }
-
-// ============================================================================
-// Pre-execute hooks
-// ============================================================================
-
-export async function postInitialPRCommentHook(
-	agentType: string,
-	{ input, logWriter }: PreExecuteParams,
-): Promise<void> {
-	// Skip if ack comment already posted by router or webhook handler
-	if (input.ackCommentId) return;
-
-	const { repoFullName, prNumber } = input;
-	if (!repoFullName || !prNumber) {
-		throw new Error('postInitialPRCommentHook requires repoFullName and prNumber in input');
-	}
-	const { owner, repo } = parseRepoFullName(repoFullName);
-
-	const message = (input.ackMessage as string | undefined) ?? INITIAL_MESSAGES[agentType];
-	logWriter('INFO', `Posting initial ${agentType} comment`, { owner, repo, prNumber });
-	await githubClient.createPRComment(owner, repo, prNumber, message);
-}

src/agents/definitions/index.ts

@@ -9,8 +9,8 @@ export {
 	resolveKnownAgentTypes,
 	invalidateDefinitionCache,
 } from './loader.js';
-export { CONTEXT_STEP_REGISTRY, PRE_EXECUTE_REGISTRY } from './strategies.js';
-export type { FetchContextParams, PreExecuteParams } from './contextSteps.js';
+export { CONTEXT_STEP_REGISTRY } from './strategies.js';
+export type { FetchContextParams } from './contextSteps.js';
 export type { AgentProfile } from './profiles.js';
 export { getAgentProfile, getAgentCapabilities } from './profiles.js';
 export { getToolManifests } from './toolManifests.js';

src/agents/definitions/profiles.ts

@@ -19,7 +19,7 @@ import {
 	validateTemplate,
 } from '../prompts/index.js';
 import { buildGadgetsForAgent } from '../shared/gadgets.js';
-import type { FetchContextParams, PreExecuteParams } from './contextSteps.js';
+import type { FetchContextParams } from './contextSteps.js';
 import { resolveAgentDefinition } from './loader.js';
 import type {
 	AgentCapabilities,
@@ -28,7 +28,7 @@ import type {
 	ScmHooks,
 	SupportedTrigger,
 } from './schema.js';
-import { CONTEXT_STEP_REGISTRY, PRE_EXECUTE_REGISTRY } from './strategies.js';
+import { CONTEXT_STEP_REGISTRY } from './strategies.js';
 
 // Re-export for backward compatibility
 export type { AgentCapabilities } from './schema.js';
@@ -58,8 +58,6 @@ export interface AgentProfile {
 	fetchContext(params: FetchContextParams): Promise<ContextInjection[]>;
 	/** Build the task prompt for this agent type */
 	buildTaskPrompt(input: AgentInput): string;
-	/** Optional pre-execute hook (e.g., post initial PR comment) */
-	preExecute?(params: PreExecuteParams): Promise<void>;
 	/** Agent capabilities (required + optional) */
 	capabilities: AgentCapabilities;
 	/**
@@ -221,12 +219,6 @@ function buildProfileFromDefinition(def: AgentDefinition, agentType: string): Ag
 		},
 	};
 
-	if (def.backend.preExecute) {
-		const preExecFn = resolveRegistry(PRE_EXECUTE_REGISTRY, def.backend.preExecute, 'preExecute');
-		// Pass agentType so the hook can look up initial messages
-		profile.preExecute = (params) => preExecFn(agentType, params);
-	}
-
 	return profile;
 }
 

src/agents/definitions/respond-to-ci.yaml

@@ -47,7 +47,6 @@ backend:
       enableStopHooks: true
       blockGitPush: false
       requiresPushedChanges: true
-  preExecute: postInitialPRComment
 
 
 hint: Fix CI failures with minimal, focused changes. Batch related file edits together.

src/agents/definitions/review.yaml

@@ -64,7 +64,6 @@ backend:
     scm:
       enableStopHooks: false
       requiresReview: true
-  preExecute: postInitialPRComment
 
 
 hint: Focus on the current aspect of review before moving to the next. Read related files together.

src/agents/definitions/schema.ts

@@ -250,7 +250,6 @@ const BackendSchema = z.object({
 	requiresPR: z.boolean().optional(),
 	/** Category-scoped hook configuration */
 	hooks: HooksSchema.optional(),
-	preExecute: z.enum(['postInitialPRComment']).optional(),
 	postConfigure: z.enum(['sequentialGadgetExecution']).optional(),
 });
 

src/agents/definitions/strategies.ts

@@ -1,7 +1,7 @@
 /**
  * Strategy Registries
  *
- * Contains registries for context pipeline steps and pre-execute hooks.
+ * Contains registries for context pipeline steps.
  *
  * Note: Tool set and gadget builder registries have been removed.
  * Tools and gadgets are now derived from capabilities via the capability registry.
@@ -11,15 +11,13 @@
 import type { ContextInjection } from '../contracts/index.js';
 import {
 	type FetchContextParams,
-	type PreExecuteParams,
 	fetchContextFilesStep,
 	fetchDirectoryListingStep,
 	fetchEmailsFromInputStep,
 	fetchPRContextStep,
 	fetchPRConversationStep,
 	fetchSquintStep,
 	fetchWorkItemStep,
-	postInitialPRCommentHook,
 } from './contextSteps.js';
 
 // ============================================================================
@@ -38,14 +36,3 @@ export const CONTEXT_STEP_REGISTRY: Record<
 	prConversation: fetchPRConversationStep,
 	prefetchedEmails: fetchEmailsFromInputStep,
 };
-
-// ============================================================================
-// Pre-Execute Hook Registry
-// ============================================================================
-
-export const PRE_EXECUTE_REGISTRY: Record<
-	string,
-	(agentType: string, params: PreExecuteParams) => Promise<void>
-> = {
-	postInitialPRComment: postInitialPRCommentHook,
-};

src/backends/adapter.ts

@@ -119,11 +119,6 @@ async function buildBackendInput(
 		projectSecrets.GITHUB_TOKEN = gitHubToken;
 	}
 
-	// Pre-execute hook (e.g., post initial PR comment for review)
-	if (profile.preExecute) {
-		await profile.preExecute({ input, logWriter });
-	}
-
 	return {
 		agentType,
 		project,

tests/unit/agents/definitions/loader.test.ts

@@ -190,16 +190,6 @@ describe('YAML agent definitions loader', () => {
 			]);
 		});
 
-		it('review has preExecute hook', () => {
-			const def = loadAgentDefinition('review');
-			expect(def.backend.preExecute).toBe('postInitialPRComment');
-		});
-
-		it('respond-to-ci has preExecute hook', () => {
-			const def = loadAgentDefinition('respond-to-ci');
-			expect(def.backend.preExecute).toBe('postInitialPRComment');
-		});
-
 		it('planning has read-only capabilities (no fs:write)', () => {
 			const def = loadAgentDefinition('planning');
 			expect(def.capabilities.required).toContain('fs:read');
@@ -275,28 +265,25 @@ describe('YAML agent definitions loader', () => {
 			expect(caps.isReadOnly).toBe(false);
 			expect(def.backend.hooks?.scm?.enableStopHooks).toBe(true);
 			expect(def.backend.needsGitHubToken).toBe(true);
-			expect(def.backend.preExecute).toBeUndefined();
 			expect(def.backend.postConfigure).toBe('sequentialGadgetExecution');
 		});
 
-		it('review agent is read-only with preExecute hook', async () => {
+		it('review agent is read-only', async () => {
 			const def = loadAgentDefinition('review');
 			const caps = await getAgentCapabilities('review');
 
 			expect(caps.canEditFiles).toBe(false);
 			expect(caps.isReadOnly).toBe(true);
 			expect(def.backend.hooks?.scm?.enableStopHooks).toBe(false);
 			expect(def.backend.needsGitHubToken).toBe(true);
-			expect(def.backend.preExecute).toBe('postInitialPRComment');
 		});
 
-		it('respond-to-ci agent has preExecute and needsGitHubToken', async () => {
+		it('respond-to-ci agent has needsGitHubToken', async () => {
 			const def = loadAgentDefinition('respond-to-ci');
 			const caps = await getAgentCapabilities('respond-to-ci');
 
 			expect(caps.canEditFiles).toBe(true);
 			expect(def.backend.needsGitHubToken).toBe(true);
-			expect(def.backend.preExecute).toBe('postInitialPRComment');
 		});
 
 		it('capabilities from getAgentCapabilities are derived correctly for all agents', async () => {

tests/unit/agents/definitions/schema.test.ts

@@ -350,7 +350,6 @@ describe('AgentDefinitionSchema', () => {
 			backend: {
 				...validDefinition.backend,
 				blockGitPush: false,
-				preExecute: 'postInitialPRComment',
 				postConfigure: 'sequentialGadgetExecution',
 			},
 			trailingMessage: {
@@ -389,24 +388,6 @@ describe('AgentDefinitionSchema', () => {
 		}
 	});
 
-	it('rejects invalid preExecute hook names', () => {
-		const bad = {
-			...validDefinition,
-			backend: { ...validDefinition.backend, preExecute: 'typoInHookName' },
-		};
-		const result = AgentDefinitionSchema.safeParse(bad);
-		expect(result.success).toBe(false);
-	});
-
-	it('accepts valid preExecute hook name', () => {
-		const good = {
-			...validDefinition,
-			backend: { ...validDefinition.backend, preExecute: 'postInitialPRComment' },
-		};
-		const result = AgentDefinitionSchema.safeParse(good);
-		expect(result.success).toBe(true);
-	});
-
 	it('rejects invalid postConfigure hook names', () => {
 		const bad = {
 			...validDefinition,

tests/unit/backends/agent-profiles.test.ts

@@ -247,133 +247,6 @@ describe('getAgentProfile', () => {
 			expect(prompt).toContain('fix/ci-errors');
 			expect(prompt).toContain('CI checks have failed');
 		});
-
-		it('has a preExecute hook', () => {
-			expect(profile.preExecute).toBeDefined();
-		});
-
-		it('preExecute skips posting when ackCommentId exists', async () => {
-			const logWriter = vi.fn();
-			await profile.preExecute?.({
-				input: {
-					prNumber: 42,
-					prBranch: 'fix/ci',
-					repoFullName: 'acme/widgets',
-					ackCommentId: 12345,
-				},
-				logWriter,
-			});
-
-			expect(mockGithub.createPRComment).not.toHaveBeenCalled();
-		});
-
-		it('preExecute posts with ackMessage when no ackCommentId', async () => {
-			const logWriter = vi.fn();
-			mockGithub.createPRComment.mockResolvedValue(undefined as never);
-
-			await profile.preExecute?.({
-				input: {
-					prNumber: 42,
-					prBranch: 'fix/ci',
-					repoFullName: 'acme/widgets',
-					ackMessage: 'On it — checking the CI failures...',
-				},
-				logWriter,
-			});
-
-			expect(mockGithub.createPRComment).toHaveBeenCalledWith(
-				'acme',
-				'widgets',
-				42,
-				'On it — checking the CI failures...',
-			);
-		});
-
-		it('preExecute falls back to INITIAL_MESSAGES when no ackCommentId or ackMessage', async () => {
-			const logWriter = vi.fn();
-			mockGithub.createPRComment.mockResolvedValue(undefined as never);
-
-			await profile.preExecute?.({
-				input: {
-					prNumber: 42,
-					prBranch: 'fix/ci',
-					repoFullName: 'acme/widgets',
-				},
-				logWriter,
-			});
-
-			// Expects either the agent-specific message or a fallback message
-			expect(mockGithub.createPRComment).toHaveBeenCalledWith(
-				'acme',
-				'widgets',
-				42,
-				expect.stringMatching(/Fixing CI|Working on it/),
-			);
-		});
-	});
-
-	describe('review profile preExecute', () => {
-		let profile: AgentProfile;
-
-		beforeEach(async () => {
-			profile = await getAgentProfile('review');
-		});
-
-		it('skips posting when ackCommentId exists', async () => {
-			const logWriter = vi.fn();
-			await profile.preExecute?.({
-				input: {
-					prNumber: 10,
-					repoFullName: 'org/repo',
-					ackCommentId: 999,
-				},
-				logWriter,
-			});
-
-			expect(mockGithub.createPRComment).not.toHaveBeenCalled();
-		});
-
-		it('posts with ackMessage when no ackCommentId', async () => {
-			const logWriter = vi.fn();
-			mockGithub.createPRComment.mockResolvedValue(undefined as never);
-
-			await profile.preExecute?.({
-				input: {
-					prNumber: 10,
-					repoFullName: 'org/repo',
-					ackMessage: 'Looking into the PR changes now...',
-				},
-				logWriter,
-			});
-
-			expect(mockGithub.createPRComment).toHaveBeenCalledWith(
-				'org',
-				'repo',
-				10,
-				'Looking into the PR changes now...',
-			);
-		});
-
-		it('falls back to INITIAL_MESSAGES when no ackCommentId or ackMessage', async () => {
-			const logWriter = vi.fn();
-			mockGithub.createPRComment.mockResolvedValue(undefined as never);
-
-			await profile.preExecute?.({
-				input: {
-					prNumber: 10,
-					repoFullName: 'org/repo',
-				},
-				logWriter,
-			});
-
-			// Expects either the agent-specific message or a fallback message
-			expect(mockGithub.createPRComment).toHaveBeenCalledWith(
-				'org',
-				'repo',
-				10,
-				expect.stringMatching(/Reviewing code|Working on it/),
-			);
-		});
 	});
 
 	describe('respond-to-pr-comment profile', () => {
@@ -431,10 +304,6 @@ describe('getAgentProfile', () => {
 			expect(profile.needsGitHubToken).toBe(true);
 		});
 
-		it('does NOT have a preExecute hook', () => {
-			expect(profile.preExecute).toBeUndefined();
-		});
-
 		it('buildTaskPrompt includes comment body, PR number, and branch', () => {
 			const prompt = profile.buildTaskPrompt({
 				prNumber: 99,