diff --git a/src/agents/definitions/contextSteps.ts b/src/agents/definitions/contextSteps.ts index 34613321..b4415efd 100644 --- a/src/agents/definitions/contextSteps.ts +++ b/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 { - // 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); -} diff --git a/src/agents/definitions/index.ts b/src/agents/definitions/index.ts index d9586ddd..8eed5026 100644 --- a/src/agents/definitions/index.ts +++ b/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'; diff --git a/src/agents/definitions/profiles.ts b/src/agents/definitions/profiles.ts index 550c1b7d..63a78982 100644 --- a/src/agents/definitions/profiles.ts +++ b/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; /** 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; /** 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; } diff --git a/src/agents/definitions/respond-to-ci.yaml b/src/agents/definitions/respond-to-ci.yaml index aa95bdaa..5efcd863 100644 --- a/src/agents/definitions/respond-to-ci.yaml +++ b/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. diff --git a/src/agents/definitions/review.yaml b/src/agents/definitions/review.yaml index 518ba469..26522efc 100644 --- a/src/agents/definitions/review.yaml +++ b/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. diff --git a/src/agents/definitions/schema.ts b/src/agents/definitions/schema.ts index b57fe0f5..abdcaf5d 100644 --- a/src/agents/definitions/schema.ts +++ b/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(), }); diff --git a/src/agents/definitions/strategies.ts b/src/agents/definitions/strategies.ts index 852fc66b..bd11b44e 100644 --- a/src/agents/definitions/strategies.ts +++ b/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,7 +11,6 @@ import type { ContextInjection } from '../contracts/index.js'; import { type FetchContextParams, - type PreExecuteParams, fetchContextFilesStep, fetchDirectoryListingStep, fetchEmailsFromInputStep, @@ -19,7 +18,6 @@ import { 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 -> = { - postInitialPRComment: postInitialPRCommentHook, -}; diff --git a/src/backends/adapter.ts b/src/backends/adapter.ts index 155075d9..a970040d 100644 --- a/src/backends/adapter.ts +++ b/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, diff --git a/tests/unit/agents/definitions/loader.test.ts b/tests/unit/agents/definitions/loader.test.ts index c764118f..88653f21 100644 --- a/tests/unit/agents/definitions/loader.test.ts +++ b/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,11 +265,10 @@ 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'); @@ -287,16 +276,14 @@ describe('YAML agent definitions loader', () => { 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 () => { diff --git a/tests/unit/agents/definitions/schema.test.ts b/tests/unit/agents/definitions/schema.test.ts index f1be384c..72f9df59 100644 --- a/tests/unit/agents/definitions/schema.test.ts +++ b/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, diff --git a/tests/unit/backends/agent-profiles.test.ts b/tests/unit/backends/agent-profiles.test.ts index 79a7d69c..ca6c2862 100644 --- a/tests/unit/backends/agent-profiles.test.ts +++ b/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, diff --git a/web/src/components/settings/agent-definition-editor.tsx b/web/src/components/settings/agent-definition-editor.tsx index f72e3b38..aa8de8eb 100644 --- a/web/src/components/settings/agent-definition-editor.tsx +++ b/web/src/components/settings/agent-definition-editor.tsx @@ -450,25 +450,7 @@ function BackendSection({ /> -
-
-
- - -
- -
+