feat(skills): resolveSkillBody + renderPrompt + abortDispatchMissingSkill (SR-2) #884

Merged

charles merged 2 commits from dev/870 into main 2026-05-05 18:01:05 +00:00

Conversation 0 Commits 2 Files changed 7 +1363 -2

charles commented 2026-05-05 17:47:30 +00:00

Owner

Copy link

Implements the three building blocks of the SR-2 dispatch pipeline. No call-site wiring yet — that follows in SR-4.

Closes #870

What's in this PR

infrastructure/database/agent-skill-store.ts

• resolveAgentSkillBody(agentType, instanceId, name) — single Drizzle query, ORDER BY scope ASC LIMIT 1 (instance wins because 'instance' < 'type' in ASCII; the spec's DESC was an editorial error). No FS fallback, no delta lookup.
• getAgentTypeAppendixFlags(agentTypeName) — reads apply_caveman / apply_artifact_style from the first non-builtin agent_type row; column defaults (caveman=off, artifact-style=on) when no row exists.

infrastructure/database/task-store.ts

• Added 'aborted_no_skill' to TERMINAL_STATUSES.

domain/analytics/skill-loader.ts

• resolveSkillBody(agentType, instanceId, name): Promise<string | null> — thin async wrapper over the infra resolver; domain layer owns the export name, infra layer owns the SQL.
• renderPrompt(opts: RenderPromptOpts): Promise<string | null> — full pipeline: resolve → interpolate → applyAppendix → caveman (if globalCavemanMode || apply_caveman) → artifact-style (if apply_artifact_style). Missing appendix rows emit [dispatch] appendix_missing and continue; missing primary skill returns null.
• abortDispatchMissingSkill(forge, repo, issueOrPr, agentType, skillName, opts) — logs [dispatch] skill_missing, posts forge comment, strips dispatch label (safe no-op when absent), writes task_history row with status='aborted_no_skill'. All forge I/O is behind the _abortImpl seam for test injection.

Test plan

• resolveSkillBody.test.ts — 7 tests: instance > type > null, boundary isolation, instanceId=null never returns instance row
• renderPrompt.test.ts — 11 tests: null on missing skill, interpolation, prompt_appendix, both bools on/off, globalCavemanMode override, graceful appendix skip with log
• abort-missing-skill.test.ts — 5 tests: happy path (comment + label + task_history), structured log fields, label no-op variants, comment failure resilience
• bun x turbo run test — 3197 pass, 0 fail
• just qa — typecheck + biome clean on touched files (pre-existing warnings in unrelated files unchanged)

Implements the three building blocks of the SR-2 dispatch pipeline. No call-site wiring yet — that follows in SR-4. Closes #870 ## What's in this PR ### `infrastructure/database/agent-skill-store.ts` - **`resolveAgentSkillBody(agentType, instanceId, name)`** — single Drizzle query, `ORDER BY scope ASC LIMIT 1` (instance wins because `'instance' < 'type'` in ASCII; the spec's `DESC` was an editorial error). No FS fallback, no delta lookup. - **`getAgentTypeAppendixFlags(agentTypeName)`** — reads `apply_caveman` / `apply_artifact_style` from the first non-builtin `agent_type` row; column defaults (`caveman=off`, `artifact-style=on`) when no row exists. ### `infrastructure/database/task-store.ts` - Added `'aborted_no_skill'` to `TERMINAL_STATUSES`. ### `domain/analytics/skill-loader.ts` - **`resolveSkillBody(agentType, instanceId, name): Promise<string | null>`** — thin async wrapper over the infra resolver; domain layer owns the export name, infra layer owns the SQL. - **`renderPrompt(opts: RenderPromptOpts): Promise<string | null>`** — full pipeline: resolve → interpolate → applyAppendix → caveman (if `globalCavemanMode || apply_caveman`) → artifact-style (if `apply_artifact_style`). Missing appendix rows emit `[dispatch] appendix_missing` and continue; missing primary skill returns `null`. - **`abortDispatchMissingSkill(forge, repo, issueOrPr, agentType, skillName, opts)`** — logs `[dispatch] skill_missing`, posts forge comment, strips dispatch label (safe no-op when absent), writes `task_history` row with `status='aborted_no_skill'`. All forge I/O is behind the `_abortImpl` seam for test injection. ## Test plan - `resolveSkillBody.test.ts` — 7 tests: instance > type > null, boundary isolation, instanceId=null never returns instance row - `renderPrompt.test.ts` — 11 tests: null on missing skill, interpolation, prompt_appendix, both bools on/off, globalCavemanMode override, graceful appendix skip with log - `abort-missing-skill.test.ts` — 5 tests: happy path (comment + label + task_history), structured log fields, label no-op variants, comment failure resilience - `bun x turbo run test` — 3197 pass, 0 fail - `just qa` — typecheck + biome clean on touched files (pre-existing warnings in unrelated files unchanged)

charles added 1 commit 2026-05-05 17:47:30 +00:00

feat(skills): resolveSkillBody + renderPrompt + abortDispatchMissingSkill (SR-2) ... ae6731136c

Closes #870

charles referenced this pull request from a commit 2026-05-05 17:58:20 +00:00

fix(sr-2): address review findings from PR #884

charles added 1 commit 2026-05-05 17:58:20 +00:00

fix(sr-2): address review findings from PR #884 ... 2ac0e36eb5

- agent-skill-store: replace ne(scope,'builtin') with eq(scope,'global') in
  getAgentTypeAppendixFlags — the negative filter was non-deterministic when
  multiple non-builtin rows existed for the same agent type name
- agent-skill-store: document ORDER BY scope ASC ASCII byte-order assumption
  with an explicit warning about future scope discriminator additions
- skill-loader: remove ne import (now unused after filter fix)
- skill-loader: document resolveSkillBody as intentionally async
- skill-loader: document forge param as log-only in abortDispatchMissingSkill
- skill-loader: use '(none)' for user/agent fields in persistTask call instead
  of agentType, which violates the PersistTaskInput field contract
- abort-missing-skill.test.ts: centralise _abortImpl restore in afterEach so
  a failing assertion can never leave a stub live for subsequent tests; wrap
  per-test warnSpy in try/finally for the same reason

charles merged commit 67f949cc8d into main 2026-05-05 18:01:05 +00:00

charles deleted branch dev/870 2026-05-05 18:01:05 +00:00

charles referenced this pull request from a commit 2026-05-05 18:01:06 +00:00

feat(skills): resolveSkillBody + renderPrompt + abortDispatchMissingSkill (SR-2) (#884)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!884

No description provided.