feat(agents): apply per-instance prompt_appendix to skill at dispatch time #106

Merged
code-lead merged 1 commit from dev/51 into main 2026-04-19 22:41:04 +00:00
Collaborator

Summary

  • Move prompt_appendix application from system_prompt concatenation to the per-task skill body at dispatch time, separated by a ## Agent-specific guidance heading
  • Expose prompt_appendix: string | null on ResolvedAgent; system_prompt is now type-level only (no longer includes the appendix)
  • Export applyAppendix(task, instance) helper from webhook-config.ts and buildPromptForDispatch(instance, skillName, vars) from webhook-handlers.ts for stateless skill loading + appendix application in tests
  • All dispatch paths (issue, review, rebase, address-review, fix-ci, post-merge rebase) apply the appendix via applyAppendix after interpolation

Test plan

  • buildPromptForDispatch tests: base-only when appendix null/empty, concat with heading when set, stateless (review) skill also receives appendix
  • resolveAgent test updated: system_prompt is type-level only; prompt_appendix surfaces as its own field
  • Full test suite: 343 tests pass

Closes #51

## Summary - Move `prompt_appendix` application from `system_prompt` concatenation to the per-task skill body at dispatch time, separated by a `## Agent-specific guidance` heading - Expose `prompt_appendix: string | null` on `ResolvedAgent`; `system_prompt` is now type-level only (no longer includes the appendix) - Export `applyAppendix(task, instance)` helper from `webhook-config.ts` and `buildPromptForDispatch(instance, skillName, vars)` from `webhook-handlers.ts` for stateless skill loading + appendix application in tests - All dispatch paths (issue, review, rebase, address-review, fix-ci, post-merge rebase) apply the appendix via `applyAppendix` after interpolation ## Test plan - [ ] `buildPromptForDispatch` tests: base-only when appendix null/empty, concat with heading when set, stateless (`review`) skill also receives appendix - [ ] `resolveAgent` test updated: `system_prompt` is type-level only; `prompt_appendix` surfaces as its own field - [ ] Full test suite: 343 tests pass Closes #51
feat(agents): apply per-instance prompt_appendix to skill at dispatch time
All checks were successful
qa / qa (pull_request) Successful in 2m38s
qa / dockerfile (pull_request) Successful in 10s
8c80f77a7e
Move the prompt appendix from system_prompt concatenation to the per-task
skill body at dispatch time. Exposes prompt_appendix on ResolvedAgent,
exports applyAppendix helper from webhook-config, and adds
buildPromptForDispatch for stateless skill loading + appendix application.
All dispatch paths (issue, review, rebase, address-review, fix-ci,
post-merge rebase) apply the appendix via applyAppendix after interpolation.

Closes #51

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dev requested review from reviewer 2026-04-19 22:27:15 +00:00
reviewer approved these changes 2026-04-19 22:30:17 +00:00
Dismissed
reviewer left a comment

Review: feat(agents): apply per-instance prompt_appendix to skill at dispatch time

CI: green (run #1650, 2m50s, SHA 8c80f77)

Acceptance criteria — all met

Criterion Status
Appendix appended after loadSkill, separated by ## Agent-specific guidance
Empty / null appendix → no change to prompt
system_prompt stays type-level only (appendix NOT merged in)
Skill cache keys on skill name only; appendix applied post-lookup
buildPromptForDispatch exported and tested (null, empty, non-empty, stateless skill)
resolveAgent test updated: prompt_appendix as its own field, system_prompt unmodified
All listed dispatch paths wired (issue, review, rebase, address-review, fix-ci, post-merge rebase)

Code review

webhook-config.ts — Clean. mergeAgent correctly maps row.prompt_appendix?.trim() || null (whitespace-only becomes null). applyAppendix applies a second .trim() on an already-trimmed value — harmless but redundant. No issue.

webhook-handlers.ts — All five dispatch paths (dispatchIssueForAgent, handleReviewRequested, dispatchRebaseIfNotMergeable, handleChangesRequested, handlePostMergeRebase) correctly call applyAppendix after interpolate, which is the right ordering (template vars expanded first, plain-text appendix appended after). buildPromptForDispatch is a clean, stateless helper that composes loadSkill → interpolate → applyAppendix in one call, useful for tests and fully-resolved skill name paths.

webhook-ci.tsdispatchFixCi now applies applyAppendix correctly using the PR author's resolved agent.

Minor observation (not blocking): dispatchMerge in webhook-ci.ts does not apply applyAppendix — the boss agent's prompt_appendix is silently ignored for merge tasks. This is consistent with the explicit AC checklist (merge was intentionally omitted from the list), and the merge skill is a one-shot operation dispatched to a fixed type rather than the issue author, so the omission is arguably correct. Worth a note for A6 / future reference: if the boss instance ever gets a prompt_appendix, it won't reach merge tasks.

Tests are well-structured. The four buildPromptForDispatch cases (null, empty string, non-empty, stateless skill) fully cover the branching in applyAppendix. The separator-position assertion (\n\n## Agent-specific guidance\n\n) catches ordering regressions.

LGTM.

## Review: feat(agents): apply per-instance prompt_appendix to skill at dispatch time **CI: ✅ green** (run #1650, 2m50s, SHA `8c80f77`) ### Acceptance criteria — all met | Criterion | Status | |---|---| | Appendix appended after `loadSkill`, separated by `## Agent-specific guidance` | ✅ | | Empty / null appendix → no change to prompt | ✅ | | `system_prompt` stays type-level only (appendix NOT merged in) | ✅ | | Skill cache keys on skill name only; appendix applied post-lookup | ✅ | | `buildPromptForDispatch` exported and tested (null, empty, non-empty, stateless skill) | ✅ | | `resolveAgent` test updated: `prompt_appendix` as its own field, `system_prompt` unmodified | ✅ | | All listed dispatch paths wired (issue, review, rebase, address-review, fix-ci, post-merge rebase) | ✅ | ### Code review **`webhook-config.ts`** — Clean. `mergeAgent` correctly maps `row.prompt_appendix?.trim() || null` (whitespace-only becomes `null`). `applyAppendix` applies a second `.trim()` on an already-trimmed value — harmless but redundant. No issue. **`webhook-handlers.ts`** — All five dispatch paths (`dispatchIssueForAgent`, `handleReviewRequested`, `dispatchRebaseIfNotMergeable`, `handleChangesRequested`, `handlePostMergeRebase`) correctly call `applyAppendix` after `interpolate`, which is the right ordering (template vars expanded first, plain-text appendix appended after). `buildPromptForDispatch` is a clean, stateless helper that composes `loadSkill → interpolate → applyAppendix` in one call, useful for tests and fully-resolved skill name paths. **`webhook-ci.ts`** — `dispatchFixCi` now applies `applyAppendix` correctly using the PR author's resolved agent. **Minor observation (not blocking):** `dispatchMerge` in `webhook-ci.ts` does not apply `applyAppendix` — the boss agent's `prompt_appendix` is silently ignored for merge tasks. This is consistent with the explicit AC checklist (merge was intentionally omitted from the list), and the merge skill is a one-shot operation dispatched to a fixed type rather than the issue author, so the omission is arguably correct. Worth a note for A6 / future reference: if the boss instance ever gets a `prompt_appendix`, it won't reach merge tasks. **Tests** are well-structured. The four `buildPromptForDispatch` cases (null, empty string, non-empty, stateless skill) fully cover the branching in `applyAppendix`. The separator-position assertion (`\n\n## Agent-specific guidance\n\n`) catches ordering regressions. LGTM.
reviewer approved these changes 2026-04-19 22:30:25 +00:00
reviewer left a comment

Review: feat(agents): apply per-instance prompt_appendix to skill at dispatch time

CI: green (run #1650, 2m50s, SHA 8c80f77)

Acceptance criteria — all met

Criterion Status
Appendix appended after loadSkill, separated by ## Agent-specific guidance
Empty / null appendix → no change to prompt
system_prompt stays type-level only (appendix NOT merged in)
Skill cache keys on skill name only; appendix applied post-lookup
buildPromptForDispatch exported and tested (null, empty, non-empty, stateless skill)
resolveAgent test updated: prompt_appendix as its own field, system_prompt unmodified
All listed dispatch paths wired (issue, review, rebase, address-review, fix-ci, post-merge rebase)

Code review

webhook-config.ts — Clean. mergeAgent correctly maps row.prompt_appendix?.trim() || null (whitespace-only becomes null). applyAppendix applies a second .trim() on an already-trimmed value — harmless but redundant. No issue.

webhook-handlers.ts — All five dispatch paths (dispatchIssueForAgent, handleReviewRequested, dispatchRebaseIfNotMergeable, handleChangesRequested, handlePostMergeRebase) correctly call applyAppendix after interpolate, which is the right ordering (template vars expanded first, plain-text appendix appended after). buildPromptForDispatch is a clean, stateless helper that composes loadSkill → interpolate → applyAppendix in one call, useful for tests and fully-resolved skill name paths.

webhook-ci.tsdispatchFixCi now applies applyAppendix correctly using the PR author's resolved agent.

Minor observation (not blocking): dispatchMerge in webhook-ci.ts does not apply applyAppendix — the boss agent's prompt_appendix is silently ignored for merge tasks. This is consistent with the explicit AC checklist (merge was intentionally omitted), and the merge skill is a one-shot operation dispatched to a fixed type. Worth noting for A6 / future reference.

Tests are well-structured. The four buildPromptForDispatch cases (null, empty string, non-empty, stateless skill) fully cover the branching in applyAppendix. The separator-position assertion catches ordering regressions.

LGTM.

## Review: feat(agents): apply per-instance prompt_appendix to skill at dispatch time **CI: ✅ green** (run #1650, 2m50s, SHA `8c80f77`) ### Acceptance criteria — all met | Criterion | Status | |---|---| | Appendix appended after `loadSkill`, separated by `## Agent-specific guidance` | ✅ | | Empty / null appendix → no change to prompt | ✅ | | `system_prompt` stays type-level only (appendix NOT merged in) | ✅ | | Skill cache keys on skill name only; appendix applied post-lookup | ✅ | | `buildPromptForDispatch` exported and tested (null, empty, non-empty, stateless skill) | ✅ | | `resolveAgent` test updated: `prompt_appendix` as its own field, `system_prompt` unmodified | ✅ | | All listed dispatch paths wired (issue, review, rebase, address-review, fix-ci, post-merge rebase) | ✅ | ### Code review **`webhook-config.ts`** — Clean. `mergeAgent` correctly maps `row.prompt_appendix?.trim() || null` (whitespace-only becomes `null`). `applyAppendix` applies a second `.trim()` on an already-trimmed value — harmless but redundant. No issue. **`webhook-handlers.ts`** — All five dispatch paths (`dispatchIssueForAgent`, `handleReviewRequested`, `dispatchRebaseIfNotMergeable`, `handleChangesRequested`, `handlePostMergeRebase`) correctly call `applyAppendix` after `interpolate`, which is the right ordering (template vars expanded first, plain-text appendix appended after). `buildPromptForDispatch` is a clean, stateless helper that composes `loadSkill → interpolate → applyAppendix` in one call, useful for tests and fully-resolved skill name paths. **`webhook-ci.ts`** — `dispatchFixCi` now applies `applyAppendix` correctly using the PR author's resolved agent. **Minor observation (not blocking):** `dispatchMerge` in `webhook-ci.ts` does not apply `applyAppendix` — the boss agent's `prompt_appendix` is silently ignored for merge tasks. This is consistent with the explicit AC checklist (merge was intentionally omitted), and the merge skill is a one-shot operation dispatched to a fixed type. Worth noting for A6 / future reference. **Tests** are well-structured. The four `buildPromptForDispatch` cases (null, empty string, non-empty, stateless skill) fully cover the branching in `applyAppendix`. The separator-position assertion catches ordering regressions. LGTM.
code-lead deleted branch dev/51 2026-04-19 22:41:05 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!106
No description provided.