refactor(agents): configuration-driven agent definitions with YAML

[zbigniewsobiecki]

Summary

Phase 2 of the configuration-driven agents architecture. Extracts hardcoded agent profiles into declarative YAML definitions with typed schemas, registry-based strategy resolution, and Eta task prompt templates. Includes code review fixes for correctness, safety, and cleanup.

Architecture

• YAML agent definitions (src/agents/definitions/*.yaml) — declarative config for all 9 agent types covering identity, capabilities, tools, strategies, backend behavior, compaction, hints, and trailing messages
• Zod-validated schema (schema.ts) — compile-time + runtime validation of YAML structure
• Registry pattern (strategies.ts) — maps YAML string references to implementations (tool sets, context pipeline steps, gadget builders, pre-execute hooks)
• Context pipeline (contextSteps.ts) — composable step functions wired by YAML contextPipeline arrays
• Eta task templates (task-templates/*.eta) — replaces TS functions with data-driven templates

Review Fixes (from Phase 2 code review)

• Fix DB task prompt override context — resolveModelConfig now receives agentInput so DB task prompt overrides can use <%= it.commentText %>, <%= it.commentAuthor %>, etc.
• Remove identity-mapping TASK_PROMPT_TEMPLATE_REGISTRY — Zod schema validates allowed values directly; no need for a registry where every key equals its value
• Add guard clauses to PR context steps — replaces unsafe as casts in fetchPRContextStep, fetchPRConversationStep, postInitialPRCommentHook with descriptive error messages
• Fix duplicate section header in strategies.ts
• Fix template whitespace in prCommentResponse.eta using Eta -%> trimming tags

Stats

• 47 files changed — +2,078 / -905 lines (net reduction from replacing hardcoded logic with YAML + templates)
• 3,259 tests pass (8 new tests for review fixes, plus 542 new tests for Phase 2 definitions/schema)

Test plan

• npm run typecheck — no errors
• npm run lint — no warnings
• npm test — 189 test files, 3,259 tests pass
• Pre-commit hooks (lint + typecheck) pass
• Pre-push hooks (full test suite) pass
• CI passes on GitHub

🤖 Generated with Claude Code

[nhopeatall]

Review Feedback Addressed

Changes Made

• [BLOCKING] Added task-templates to build and Docker images — created build:copy-task-templates script in package.json and added COPY directives to Dockerfile.worker and Dockerfile.dashboard. The templates are now copied to dist/agents/prompts/task-templates/ during build.
• Circular import — Acknowledged the latent fragility in the import chain. Currently safe (hoisted function declarations), but documented for future awareness.

Verification

• All tests passing (3,259 tests)
• Build succeeds with templates copied to dist/
• Lint and type checks passing
• Changes pushed to remote

The blocking issue has been resolved — task prompt templates are now included in the Docker build and will be available at runtime.

[nhopeatall]

Summary

Well-structured refactoring that successfully extracts hardcoded agent profiles into YAML definitions with Zod validation and registry-based strategy resolution. The architecture is clean — schema validates YAML structure, registries map string references to implementations, and the pipeline pattern composes context steps declaratively. One blocking issue: task prompt templates are missing from the Docker build, which will cause runtime failures in production.

Architecture & Design

The YAML → Zod → Registry pattern is a solid fit for this use case. The test coverage is thorough — 362 lines of loader tests and 180 lines of schema tests verify the entire YAML ↔ registry contract. The behavioral parity with the old hardcoded profiles is verified by the existing tests (with appropriate assertion updates).

One latent fragility worth noting: there is a circular import chain (contextSteps.ts → agentMessages.ts → definitions/index.ts → strategies.ts → contextSteps.ts). This currently works because the functions in contextSteps.ts use export function declarations which are hoisted in ESM. However, if any of these are refactored to const arrow functions in the future, the circular dependency would cause undefined references at module init time. Consider having contextSteps.ts import INITIAL_MESSAGES lazily or breaking the cycle by not importing through the barrel index.ts.

Code Issues

Blocking

• Missing task templates in Docker build — The new src/agents/prompts/task-templates/*.eta files (5 templates: ci.eta, commentResponse.eta, prCommentResponse.eta, review.eta, workItem.eta) are not copied to dist/ during the build and are not referenced in any Dockerfile. The package.json build:copy-yaml script only copies .yaml files. The existing Dockerfile line that copies system prompt templates (COPY src/agents/prompts/templates ./dist/agents/prompts/templates) doesn't cover task-templates/. At runtime, renderTaskPrompt() calls loadTaskTemplate() which uses readFileSync with __dirname-relative paths — this will throw ENOENT in the Docker container. Fix: Add task-templates to the build copy and Dockerfiles.

Should Fix

None — the implementation is correct and well-tested beyond the deployment gap.

Questions

• Is CI's integration-tests failure related to this PR, or a pre-existing issue?

[nhopeatall]

[BLOCKING] The build:copy-yaml script only copies YAML files to dist/, but the new src/agents/prompts/task-templates/*.eta files also need to be copied. Without this, renderTaskPrompt() will fail with ENOENT in production (compiled JS looks for templates relative to dist/agents/prompts/task-templates/).

Suggested fix — add a second copy step:

"build:copy-templates": "cp -r src/agents/prompts/task-templates dist/agents/prompts/task-templates",
"build": "tsc && npm run build:copy-yaml && npm run build:copy-templates"

[aaight]

Fixed! Added build:copy-task-templates script to package.json and added COPY directives for src/agents/prompts/task-templates to both Dockerfile.worker and Dockerfile.dashboard. The templates are now copied to dist/agents/prompts/task-templates/ during build, ensuring renderTaskPrompt() can read them at runtime.

[aaight]

Fixed in commit 7cfe927! Added build:copy-task-templates script to package.json and updated both Dockerfile.worker and Dockerfile.dashboard to copy the task-templates/ directory. The .eta files are now deployed to dist/agents/prompts/task-templates/ in production containers.

[nhopeatall]

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.

[aaight]

Good catch on the circular import. The functions are currently hoisted export function declarations, so it works today. I've noted this as a latent fragility — if any are refactored to const arrow 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.

[aaight]

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.

[aaight]

🤔 Looking into the missing task prompt templates now

---

Progress: [██░░░░░░░░] 19% (iteration 13/70)

🔧 Review Response Update (1 min)

I've identified the blocking issue from the code review: the task prompt templates are not being included in the Docker builds. I've started addressing this by modifying package.json, Dockerfile.worker, and Dockerfile.dashboard to ensure these templates are copied. I'm currently verifying these changes and testing the build process.

_{Last updated: iteration 13 · respond-to-review}

[nhopeatall]

Review already submitted — see inline comments. Blocking issue: task prompt templates missing from Docker build.

[aaight]

🤔 On it — checking the Docker build for those task prompt templates

---

Progress: [████████░░] 81% (iteration 57/70)

🔧 Review Response Update (4 min)

I've successfully verified that the Dockerfile changes I was about to implement were already pushed by another agent (commit 7cfe927), effectively resolving the issue. I've also confirmed that all three required changes (including package.json updates) are in place. I'm currently replying to the review comments on GitHub, confirming the fixes.

_{Last updated: iteration 57 · respond-to-review}

[aaight]

Review Feedback Addressed

Changes Made

Blocking Issue Fixed (commit 7cfe927):

• ✅ Added build:copy-task-templates script to package.json
• ✅ Updated Dockerfile.worker to copy task-templates/ directory
• ✅ Updated Dockerfile.dashboard to copy task-templates/ directory
• ✅ All 5 .eta task prompt templates now deployed to dist/agents/prompts/task-templates/ in production

The renderTaskPrompt() ENOENT runtime failure is now resolved — templates are present in the Docker images.

Circular Import Warning (non-blocking):
Acknowledged the latent circular dependency in contextSteps.ts. Currently safe because all exports use hoisted export function declarations, but noted for future refactors to avoid arrow functions without breaking the cycle first.

Verification

• ✅ TypeScript compilation passes
• ✅ Lint passes (zero warnings)
• ✅ All 3,259 unit tests pass

[nhopeatall]

🤖 Just a sec, looking into that agent config refactor

---

Progress: [█████████░] 91% (iteration 64/70)

🔍 Code Review Update (4 min)

I've completed a thorough review of the pull request, verifying consistency across various files including src/agents/definitions/index.ts, src/db/migrations/0016_add_task_prompt_column.sql, and src/db/repositories/configMapper.ts. I also confirmed the makeMockProfile function in tests/unit/backends/adapter.test.ts handles the requiresPR property correctly and that all task templates are present. I've submitted my approval for the PR and am now updating the initial PR comment.

_{Last updated: iteration 64 · review}

[nhopeatall]

Summary

Well-executed configuration-driven refactor. The YAML-based agent definitions with Zod validation, registry-based strategy resolution, and Eta task templates cleanly replace 541 lines of hardcoded profiles with declarative config. No behavioral regressions detected — all 9 agent types preserve their exact capabilities, tool sets, context pipelines, and backend flags.

Architecture & Design

The architecture is sound:

• Separation of concerns is good: YAML for declarative config, Zod schema for validation, registries for runtime resolution, Eta templates for task prompts. Each layer has a clear responsibility.
• Registry pattern is appropriate: The TOOL_SET_REGISTRY, CONTEXT_STEP_REGISTRY, GADGET_BUILDER_REGISTRY, and PRE_EXECUTE_REGISTRY provide a clean extension point. Adding a new agent type now requires only a YAML file — no TypeScript changes needed.
• Zod schema acts as a contract: The enum constraints in the schema (tool sets, pipeline steps, builders, hooks) prevent YAML typos from causing runtime failures. This is a good "make illegal states unrepresentable" approach.
• Tests are thorough: 542 new tests for the definitions layer, including roundtrip validation (YAML → profile → capabilities match), registry reference resolution, and template file existence checks.

Minor observations (not blocking)

1. Circular import: contextSteps.ts → agentMessages.ts → definitions/index.ts → strategies.ts → contextSteps.ts. This is safe in practice (all cross-module references are inside functions, not at module evaluation time), but worth being aware of if future changes add top-level side effects to any of these modules.

2. Eager YAML loading at import time: agentMessages.ts reads all YAML definitions during module initialization. This means a malformed YAML file will crash the process at startup rather than at first use. This is arguably better (fail-fast), but different from the old static object approach. The try/catch wrapping is appropriate.

3. Profile objects are not cached: getAgentProfile() creates a new profile on each call (though loadAgentDefinition() caches the YAML parse). The old code used static singletons. The per-call allocation is negligible for this use case, but if getAgentProfile is ever called in a hot path, a profile-level cache could be added.

Code Issues

None blocking. The implementation correctly mirrors the old behavior across all 9 agent types. Specifically verified:

• respond-to-review keeps sdkTools: all with isReadOnly: true (matching old behavior)
• implementation is the only agent with requiresPR: true
• Docker builds include YAML files via the npm run build → build:copy-yaml pipeline, which writes to dist/ before the COPY --from=builder /app/dist step
• The prCommentResponse.eta template correctly handles empty commentPath via Eta truthiness check
• DB migration is idempotent (IF NOT EXISTS)
• taskPrompts flows correctly through schema → configMapper → modelResolution → adapter

LGTM — clean refactor with no behavioral changes and strong validation guardrails.