SR-4 dispatch cutover — replace skillForEvent / skillForAgent with renderPrompt; drop FS fallback + delta lookup + appendix gating #872

Closed
opened 2026-05-05 10:29:10 +00:00 by claude-desktop · 0 comments
Collaborator

User story

As a platform engineer, I want every dispatch site to call the new renderPrompt pipeline instead of the legacy skillForEvent / skillForAgent chain, so that the resolver, render, and abort behaviour are uniform and the legacy ladder is no longer reachable at runtime.

Acceptance criteria

Replacements

  • apps/server/src/domain/workflow/event-handlers.ts — every skillForEvent(...) call becomes renderPrompt(...). skillForAgent(agent.type, base) is dropped — pass the base name straight in.
  • apps/server/src/domain/flows/agent-nodes.ts — same, for the agent.render_skill node body.
  • apps/server/src/background/janitor.ts — same, for fix-ci and rebase paths.
  • apps/server/src/background/tail-pr-rebase-watchdog.ts — same.
  • apps/server/src/domain/workflow/slash-commands.ts — same, for breakdown and any other slash-command paths.
  • All call sites that previously called interpolate + applyAppendix + maybeApplyCavemanAppendix + applyArtifactStyleAppendix collapse to a single renderPrompt(...) call.
  • When renderPrompt returns null (skill missing), the call site invokes abortDispatchMissingSkill(...) and returns. No silent skip, no 500.

Code deletions

  • Delete skillForEvent, STATELESS_SKILLS, shouldApplyCaveman, maybeApplyCavemanAppendix, applyArtifactStyleAppendix from skill-loader.ts. Keep loadSkill (now used only by SR-6's library endpoint and SR-3's migration drift-check) and interpolate.
  • Delete resolveSkill + SkillRow + the skill_overrides_json parsing path from apps/server/src/infrastructure/database/agent-type-config.ts. Keep the rest of the resolver intact (other artifacts unaffected).
  • Existing call sites passing a <base>-delta filename are simplified — there is no delta variant; everyone passes the base name.

Tests

  • Update existing dispatch tests (event-handlers.test.ts, agent-nodes.test.ts, janitor.test.ts, tail-pr-rebase-watchdog.test.ts, slash-commands.test.ts) to use seeded agent_skill rows in their fixtures instead of FS skills/*.md.
  • One regression test per call site: missing skill triggers abortDispatchMissingSkill (assert comment + label strip + task_history row).

Out of scope

  • Webhook routing rewrites (covered in SR-5).
  • API surface (covered in SR-6 / SR-7).
  • UI (covered in SR-8 / SR-9 / SR-10).
  • Dropping the legacy skill table / skill_overrides_json column (covered in SR-11).

References

  • Spec: specs/skills-rework.md §Dispatch path, §Code deletions.
  • Depends on SR-1, SR-2, SR-3.
  • Note: this is the most invasive PR. Run just qa + full test suite. Coordinate timing — no other branch should be touching the dispatch path while this lands.
## User story As a platform engineer, I want every dispatch site to call the new `renderPrompt` pipeline instead of the legacy `skillForEvent` / `skillForAgent` chain, so that the resolver, render, and abort behaviour are uniform and the legacy ladder is no longer reachable at runtime. ## Acceptance criteria ### Replacements - [ ] `apps/server/src/domain/workflow/event-handlers.ts` — every `skillForEvent(...)` call becomes `renderPrompt(...)`. `skillForAgent(agent.type, base)` is dropped — pass the base name straight in. - [ ] `apps/server/src/domain/flows/agent-nodes.ts` — same, for the `agent.render_skill` node body. - [ ] `apps/server/src/background/janitor.ts` — same, for `fix-ci` and rebase paths. - [ ] `apps/server/src/background/tail-pr-rebase-watchdog.ts` — same. - [ ] `apps/server/src/domain/workflow/slash-commands.ts` — same, for `breakdown` and any other slash-command paths. - [ ] All call sites that previously called `interpolate` + `applyAppendix` + `maybeApplyCavemanAppendix` + `applyArtifactStyleAppendix` collapse to a single `renderPrompt(...)` call. - [ ] When `renderPrompt` returns `null` (skill missing), the call site invokes `abortDispatchMissingSkill(...)` and returns. No silent skip, no 500. ### Code deletions - [ ] Delete `skillForEvent`, `STATELESS_SKILLS`, `shouldApplyCaveman`, `maybeApplyCavemanAppendix`, `applyArtifactStyleAppendix` from `skill-loader.ts`. Keep `loadSkill` (now used only by SR-6's library endpoint and SR-3's migration drift-check) and `interpolate`. - [ ] Delete `resolveSkill` + `SkillRow` + the `skill_overrides_json` parsing path from `apps/server/src/infrastructure/database/agent-type-config.ts`. Keep the rest of the resolver intact (other artifacts unaffected). - [ ] Existing call sites passing a `<base>-delta` filename are simplified — there is no delta variant; everyone passes the base name. ### Tests - [ ] Update existing dispatch tests (`event-handlers.test.ts`, `agent-nodes.test.ts`, `janitor.test.ts`, `tail-pr-rebase-watchdog.test.ts`, `slash-commands.test.ts`) to use seeded `agent_skill` rows in their fixtures instead of FS `skills/*.md`. - [ ] One regression test per call site: missing skill triggers `abortDispatchMissingSkill` (assert comment + label strip + task_history row). ## Out of scope - Webhook routing rewrites (covered in SR-5). - API surface (covered in SR-6 / SR-7). - UI (covered in SR-8 / SR-9 / SR-10). - Dropping the legacy `skill` table / `skill_overrides_json` column (covered in SR-11). ## References - Spec: `specs/skills-rework.md` §Dispatch path, §Code deletions. - Depends on **SR-1**, **SR-2**, **SR-3**. - Note: this is the most invasive PR. Run `just qa` + full test suite. Coordinate timing — no other branch should be touching the dispatch path while this lands.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
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#872
No description provided.