refactor(agents): unify dual-backend gadget builders and prompt functions#429
Conversation
|
🔍 Reviewing PR... |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — clean deduplication refactoring that extracts shared gadget builders and prompt functions into src/agents/shared/, eliminating ~220 lines of duplicated code between the llmist and Claude Code backends. All changes are structurally sound.
Notes
Prompt text change (intentional): The llmist backend's buildPrompt() previously included richer text ("...title, description, checklists, attachments, comments...Review it and proceed with your task.") while the Claude Code backend used a shorter version. The PR unifies to the shorter version. This is a minor behavioral change for the llmist path but is consistent with the stated goal of making both backends produce identical outputs.
Gadget set expansion (intentional, correct): createPRAgentGadgets() (now buildPRAgentGadgets()) gains FileMultiEdit, VerifyChanges, and GetPRChecks that agent-profiles.ts already had. The three llmist agent files (respond-to-ci, respond-to-review, respond-to-pr-comment) that call the deprecated alias will now get these additional gadgets — this is the intended parity fix per the PR description.
process.chdir() removal: The fix is correct. ListDirectory.execute() calls path.resolve(process.cwd(), directoryPath), and since repoDir is absolute, path.resolve uses it directly regardless of cwd. The race condition is eliminated.
Backwards compatibility: createPRAgentGadgets is kept as a thin deprecated alias, and the three existing callers in src/agents/ continue to work unchanged.
Tests: 36 new tests cover all shared builders with proper mocking. CI passes (4/4 checks green).
nhopeatall
left a comment
There was a problem hiding this comment.
Review completed - see previous review submission.
Summary
Refactors the dual-backend agent architecture to eliminate code duplication between the llmist backend (
agents/base.ts) and the Claude Code backend (backends/agent-profiles.ts).Card: https://trello.com/c/aLAQMmke/49-find-top-candidate-for-refactoring-and-plan-clean-refactoring-of-it-look-for-god-classes-modules-functions-files-and-code-duplic
What was done
Gadget unification (Step 2): Extracted
buildWorkItemGadgets(),buildReviewGadgets(), andbuildPRAgentGadgets()intosrc/agents/shared/gadgets.tsas the single source of truth. Both backends now call these shared factories instead of maintaining separate identical builders. The old comment "Mirrors the logic in agents/base.ts getBaseAgentGadgets()" is gone because the code is now actually shared. Net: ~130 lines of duplicated gadget builders removed.Prompt builder consolidation (Step 1): Created
src/agents/shared/taskPrompts.tswith canonical prompt builder functions (buildWorkItemPrompt,buildCommentResponsePrompt,buildReviewPrompt,buildCIResponsePrompt,buildPRCommentResponsePrompt,buildCheckFailurePrompt,buildDebugPrompt). Bothagents/base.tsandbackends/agent-profiles.tsnow import from this module instead of each defining their own versions. Net: ~90 lines of duplicated prompt builders removed.process.chdir()side effect removal (Step 4): FixedfetchDirectoryListing()inagent-profiles.tswhich was usingprocess.chdir(repoDir)as a workaround. Now passes the absoluterepoDirpath directly toListDirectory, eliminating the race condition.Backwards compatibility:
createPRAgentGadgets()is kept as a deprecated alias forbuildPRAgentGadgets()to avoid breaking existing callers in the llmist agent files.Key decisions
agent-profiles.tsthin wrapper functions (buildWorkItemTaskPrompt, etc.) that delegate to shared builders — avoids changing theAgentProfileinterface which would require updating all 10+ profile objects and their tests.src/agents/shared/as the location for shared code (notsrc/backends/) to avoid circular imports sincebackends/already imports fromagents/shared/.buildPRAgentGadgetsis now complete (includesFileMultiEdit,VerifyChanges,GetPRChecksthat were missing from the oldcreatePRAgentGadgets), making both backends produce identical gadget sets.Testing
tests/unit/agents/shared/taskPrompts.test.ts— 20 tests for all prompt builder functionstests/unit/agents/shared/gadgets.test.ts— 16 tests verifying gadget factory behavior per capability flags🤖 Generated with Claude Code