Conversation
…riven lifecycle hooks
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
LGTM — Clean, well-scoped refactoring that replaces hardcoded agentType === 'implementation' branching in PMLifecycleManager and ProgressMonitor with profile-driven lifecycle hooks. The design is sound: hooks are defined in YAML, resolved through AgentProfile, and consumed at the right layers. Backward compatibility is preserved via defaults ({} for hooks, false for syncChecklist).
Minor Observations
Should Consider (non-blocking)
src/pm/lifecycle.ts:77-78, 91-93— Thehooks.moveOnPrepare as keyof typeof this.pmConfig.statusescast silently resolves toundefinedif someone puts an invalid status key (e.g.,moveOnPrepare: typo) in their YAML definition. This is safe (becausesafeMoveguards againstundefined), but it could be confusing to debug. Consider either (a) validating the string against known status keys at resolution time, or (b) logging a warning when the lookup yieldsundefineddespite the hook being set. Not blocking because definition authors are the only ones writing these values, and the schema could later constrainmoveOnPrepare/moveOnSuccessto an enum.
The remaining agentType === 'implementation' references in ack-comments.ts and handleSuccessOnlyForAgentType: 'implementation' in integration.ts are correctly outside this PR's scope — they control GitHub-specific behavior (comment cleanup and execution gating), not PM lifecycle hooks.
🕵️ claude-code · claude-opus-4-6 · run details
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
AgentDefinitionschema with optionalhooks.lifecycleblock (moveOnPrepare,moveOnSuccess,linkPR,syncChecklist)lifecycleHooksfromAgentProfileresolved from the definitionagentType === 'implementation'checks inPMLifecycleManagerwith profile-driven hooksagentType === 'implementation'check inProgressMonitor.tick()withsyncChecklistconfig flagimplementation.yaml; all other agents default to no-opTrello Card
https://trello.com/c/69c198a5184519ee8c4d2fa4
Changes
Schema (
src/agents/definitions/schema.ts)Added
LifecycleHooksSchemawith optional fieldsmoveOnPrepare,moveOnSuccess,linkPR,syncChecklist. AddedlifecycletoIntegrationHooksSchema.AgentProfile (
src/agents/definitions/profiles.ts)Added
lifecycleHooks: LifecycleHooksfield toAgentProfileinterface. Resolved fromdef.hooks?.lifecycle ?? {}— empty object (no-op) when not defined.PMLifecycleManager (
src/pm/lifecycle.ts)prepareForAgent(workItemId, hooks: LifecycleHooks)— useshooks.moveOnPreparestatus key to optionally move the work itemhandleSuccess(workItemId, hooks: LifecycleHooks, prUrl?, progressCommentId?)— useshooks.moveOnSuccessandhooks.linkPRinstead of checking agentTypeProgressMonitor (
src/backends/progressMonitor.ts,progressLifecycle.ts,adapter.ts)Added optional
syncChecklistfield toProgressMonitorConfig. The tick function usesthis.config.syncChecklistinstead ofagentType === 'implementation'.buildProgressMonitorConfigaccepts asyncChecklistparameter.adapter.tspassesprofile.lifecycleHooks.syncChecklistto the config builder.Execution Pipeline (
src/triggers/shared/agent-execution.ts)Loads lifecycle hooks from the agent profile via
getAgentProfile(agentType)(best-effort, defaults to{}on failure). Passes hooks toprepareForAgentandhandleSuccess.implementation.yaml
Added lifecycle block:
Test plan
npm test)npm run lint)npm run typecheck)syncChecklistflag behavior🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details