-
Notifications
You must be signed in to change notification settings - Fork 2
refactor(agents): configuration-driven agent definitions with YAML #550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,262 @@ | ||
| /** | ||
| * Context pipeline step implementations and pre-execute hooks. | ||
| * | ||
| * Each step function takes a FetchContextParams and returns ContextInjection[]. | ||
| * These are the building blocks composed by the YAML contextPipeline arrays. | ||
| */ | ||
|
|
||
| import { execFileSync } from 'node:child_process'; | ||
|
|
||
| import type { ContextInjection, LogWriter } from '../../backends/types.js'; | ||
| 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'; | ||
| import { githubClient } from '../../github/client.js'; | ||
| import type { AgentInput } from '../../types/index.js'; | ||
| import { parseRepoFullName } from '../../utils/repo.js'; | ||
| import { resolveSquintDbPath } from '../../utils/squintDb.js'; | ||
| import { | ||
| formatPRComments, | ||
| formatPRDetails, | ||
| formatPRDiff, | ||
| formatPRIssueComments, | ||
| formatPRReviews, | ||
| readPRFileContents, | ||
| } from '../shared/prFormatting.js'; | ||
| import type { ContextFile } from '../utils/setup.js'; | ||
|
|
||
| // ============================================================================ | ||
| // Shared interfaces | ||
| // ============================================================================ | ||
|
|
||
| export interface FetchContextParams { | ||
| input: AgentInput; | ||
| repoDir: string; | ||
| contextFiles: ContextFile[]; | ||
| logWriter: LogWriter; | ||
| } | ||
|
|
||
| export interface PreExecuteParams { | ||
| input: AgentInput; | ||
| logWriter: LogWriter; | ||
| } | ||
|
|
||
| // ============================================================================ | ||
| // Atomic context step functions | ||
| // ============================================================================ | ||
|
|
||
| export function fetchDirectoryListingStep(params: FetchContextParams): ContextInjection[] { | ||
| const listDirGadget = new ListDirectory(); | ||
| const gadgetParams = { | ||
| comment: 'Pre-fetching codebase structure for context', | ||
| directoryPath: params.repoDir, | ||
| maxDepth: 3, | ||
| includeGitIgnored: false, | ||
| }; | ||
|
|
||
| const result = listDirGadget.execute(gadgetParams); | ||
| return [ | ||
| { | ||
| toolName: 'ListDirectory', | ||
| params: gadgetParams, | ||
| result, | ||
| description: 'Pre-fetched codebase structure', | ||
| }, | ||
| ]; | ||
| } | ||
|
|
||
| export function fetchContextFilesStep(params: FetchContextParams): ContextInjection[] { | ||
| return params.contextFiles.map((file) => ({ | ||
| toolName: 'ReadFile', | ||
| params: { comment: `Pre-fetching ${file.path} for project context`, filePath: file.path }, | ||
| result: file.content, | ||
| description: `Pre-fetched ${file.path}`, | ||
| })); | ||
| } | ||
|
|
||
| export function fetchSquintStep(params: FetchContextParams): ContextInjection[] { | ||
| const squintDb = resolveSquintDbPath(params.repoDir); | ||
| if (!squintDb) return []; | ||
|
|
||
| try { | ||
| const output = execFileSync('squint', ['overview', '-d', squintDb], { | ||
| encoding: 'utf-8', | ||
| timeout: 30_000, | ||
| }); | ||
| if (!output?.trim()) return []; | ||
|
|
||
| return [ | ||
| { | ||
| toolName: 'SquintOverview', | ||
| params: { | ||
| comment: 'Pre-fetching Squint codebase overview for context', | ||
| database: squintDb, | ||
| }, | ||
| result: output, | ||
| description: 'Pre-fetched Squint codebase overview', | ||
| }, | ||
| ]; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| export async function fetchWorkItemStep(params: FetchContextParams): Promise<ContextInjection[]> { | ||
| if (!params.input.cardId) return []; | ||
| try { | ||
| const cardData = await readWorkItem(params.input.cardId, true); | ||
| return [ | ||
| { | ||
| toolName: 'ReadWorkItem', | ||
| params: { workItemId: params.input.cardId, includeComments: true }, | ||
| result: cardData, | ||
| description: 'Pre-fetched work item data', | ||
| }, | ||
| ]; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| export async function fetchPRContextStep(params: FetchContextParams): Promise<ContextInjection[]> { | ||
| const { repoFullName, prNumber } = params.input; | ||
| if (!repoFullName || !prNumber) { | ||
| throw new Error('fetchPRContextStep requires repoFullName and prNumber in input'); | ||
| } | ||
| const injections: ContextInjection[] = []; | ||
| const { owner, repo } = parseRepoFullName(repoFullName); | ||
|
|
||
| params.logWriter('INFO', 'Fetching PR details, diff, and check status', { | ||
| owner, | ||
| repo, | ||
| prNumber, | ||
| }); | ||
|
|
||
| const prDetails = await githubClient.getPR(owner, repo, prNumber); | ||
| const prDiff = await githubClient.getPRDiff(owner, repo, prNumber); | ||
| const checkStatus = await githubClient.getCheckSuiteStatus(owner, repo, prDetails.headSha); | ||
|
|
||
| const prDetailsFormatted = formatPRDetails(prDetails); | ||
| const diffFormatted = formatPRDiff(prDiff); | ||
| const checkStatusFormatted = formatCheckStatus(prNumber, checkStatus); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRDetails', | ||
| params: { comment: 'Pre-fetching PR details for review context', owner, repo, prNumber }, | ||
| result: prDetailsFormatted, | ||
| description: 'Pre-fetched PR details', | ||
| }); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRDiff', | ||
| params: { comment: 'Pre-fetching PR diff for code review', owner, repo, prNumber }, | ||
| result: diffFormatted, | ||
| description: 'Pre-fetched PR diff', | ||
| }); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRChecks', | ||
| params: { comment: 'Pre-fetching CI check status for review', owner, repo, prNumber }, | ||
| result: checkStatusFormatted, | ||
| description: 'Pre-fetched CI check status', | ||
| }); | ||
|
|
||
| // Read full contents of changed files | ||
| params.logWriter('INFO', 'Reading PR file contents', { fileCount: prDiff.length }); | ||
| const fileContents = await readPRFileContents(params.repoDir, prDiff); | ||
| params.logWriter('INFO', 'File contents loaded', { | ||
| included: fileContents.included.length, | ||
| skipped: fileContents.skipped.length, | ||
| }); | ||
|
|
||
| for (const file of fileContents.included) { | ||
| injections.push({ | ||
| toolName: 'ReadFile', | ||
| params: { comment: `Pre-fetching ${file.path} for review`, filePath: file.path }, | ||
| result: `path=${file.path}\n\n${file.content}`, | ||
| description: `Pre-fetched ${file.path}`, | ||
| }); | ||
| } | ||
|
|
||
| return injections; | ||
| } | ||
|
|
||
| export async function fetchPRConversationStep( | ||
| params: FetchContextParams, | ||
| ): Promise<ContextInjection[]> { | ||
| const { repoFullName, prNumber } = params.input; | ||
| if (!repoFullName || !prNumber) { | ||
| throw new Error('fetchPRConversationStep requires repoFullName and prNumber in input'); | ||
| } | ||
| const injections: ContextInjection[] = []; | ||
| const { owner, repo } = parseRepoFullName(repoFullName); | ||
|
|
||
| params.logWriter('INFO', 'Fetching PR conversation context', { owner, repo, prNumber }); | ||
|
|
||
| const [reviewComments, reviews, issueComments] = await Promise.all([ | ||
| githubClient.getPRReviewComments(owner, repo, prNumber), | ||
| githubClient.getPRReviews(owner, repo, prNumber), | ||
| githubClient.getPRIssueComments(owner, repo, prNumber), | ||
| ]); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRComments', | ||
| params: { | ||
| comment: 'Pre-fetching PR review comments for conversation context', | ||
| owner, | ||
| repo, | ||
| prNumber, | ||
| }, | ||
| result: formatPRComments(reviewComments), | ||
| description: 'Pre-fetched PR review comments', | ||
| }); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRComments', | ||
| params: { | ||
| comment: 'Pre-fetching PR reviews for conversation context', | ||
| owner, | ||
| repo, | ||
| prNumber, | ||
| }, | ||
| result: formatPRReviews(reviews), | ||
| description: 'Pre-fetched PR reviews', | ||
| }); | ||
|
|
||
| injections.push({ | ||
| toolName: 'GetPRComments', | ||
| params: { | ||
| comment: 'Pre-fetching PR issue comments for conversation context', | ||
| owner, | ||
| repo, | ||
| prNumber, | ||
| }, | ||
| result: formatPRIssueComments(issueComments), | ||
| description: 'Pre-fetched PR issue comments', | ||
| }); | ||
|
|
||
| return injections; | ||
| } | ||
|
|
||
| // ============================================================================ | ||
| // 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); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| identity: | ||
| emoji: "\U0001F41B" | ||
| label: Debug Update | ||
| roleHint: Analyzes session logs to identify what went wrong | ||
| initialMessage: "**\U0001F41B Analyzing session logs** — Reviewing what happened and identifying issues..." | ||
|
|
||
| capabilities: | ||
| canEditFiles: true | ||
| canCreatePR: true | ||
| canUpdateChecklists: true | ||
| isReadOnly: false | ||
|
|
||
| tools: | ||
| sets: [all] | ||
| sdkTools: all | ||
|
|
||
| strategies: | ||
| contextPipeline: [directoryListing, contextFiles, squint, workItem] | ||
| taskPromptBuilder: workItem | ||
| gadgetBuilder: workItem | ||
|
|
||
| backend: | ||
| enableStopHooks: true | ||
| needsGitHubToken: false | ||
|
|
||
| compaction: default | ||
|
|
||
| hint: Analyze the current issue fully before moving to the next. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Latent circular dependency: this import creates a cycle ( → → → → ). Currently safe because these are declarations (hoisted in ESM), but will break if any are refactored to arrow functions. Consider importing lazily inside to break the cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the circular import. The functions are currently hoisted
export functiondeclarations, so it works today. I've noted this as a latent fragility — if any are refactored toconstarrow functions in the future, we'd need to break the cycle or use lazy imports. For now, I'm leaving it as-is since it's not causing any runtime issues.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the latent circular dependency. This is currently safe because all functions in contextSteps.ts use
export function(hoisted), but you're right that refactoring to arrow functions would break it. The cycle is: contextSteps.ts → agentMessages.ts → definitions/index.ts → strategies.ts → contextSteps.ts. Breaking it would require having contextSteps.ts import INITIAL_MESSAGES lazily (inside postInitialPRCommentHook) or not importing through the barrel index.ts. For now, I've noted this in code comments for future refactors.