From 405ea2fcc8caadb47bf5f3be7b31f715c87fb502 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 7 Aug 2026 23:13:41 -0400 Subject: [PATCH] fix: preserve skill attachment semantics --- packages/client/src/effect/api/api.ts | 1 + .../client/src/effect/generated/client.ts | 1 + .../client/src/promise/generated/client.ts | 1 + .../client/src/promise/generated/types.ts | 59 ++++++++++++++ packages/core/src/plugin/promise.ts | 1 + packages/core/src/session.ts | 80 ++++++++++++++----- packages/core/src/session/pending.ts | 23 ++++-- packages/core/src/skill.ts | 4 +- packages/core/test/session-skill.test.ts | 42 +++++++--- packages/protocol/src/groups/session.ts | 1 + packages/server/src/handlers/session.ts | 7 ++ .../tui/src/component/prompt/autocomplete.tsx | 2 + packages/tui/src/component/prompt/index.tsx | 2 + packages/tui/src/mini/stream-v2.transport.ts | 7 +- .../tui/test/mini/stream-v2.transport.test.ts | 8 +- 15 files changed, 195 insertions(+), 44 deletions(-) diff --git a/packages/client/src/effect/api/api.ts b/packages/client/src/effect/api/api.ts index 0c02f37320..e0867bedd9 100644 --- a/packages/client/src/effect/api/api.ts +++ b/packages/client/src/effect/api/api.ts @@ -197,6 +197,7 @@ export type Endpoint5_13Input = { readonly model?: Model.Ref | undefined readonly files?: ReadonlyArray | undefined readonly agents?: ReadonlyArray | undefined + readonly skills?: ReadonlyArray | undefined readonly delivery?: "steer" | "queue" | undefined readonly resume?: boolean | undefined } diff --git a/packages/client/src/effect/generated/client.ts b/packages/client/src/effect/generated/client.ts index 1b5a900f5b..4441e14f6f 100644 --- a/packages/client/src/effect/generated/client.ts +++ b/packages/client/src/effect/generated/client.ts @@ -435,6 +435,7 @@ const Endpoint5_13 = (raw: RawClient["server.session"]) => (input: Endpoint5_13I model: input["model"], files: input["files"], agents: input["agents"], + skills: input["skills"], delivery: input["delivery"], resume: input["resume"], }, diff --git a/packages/client/src/promise/generated/client.ts b/packages/client/src/promise/generated/client.ts index 29f831bbc7..dda31d908a 100644 --- a/packages/client/src/promise/generated/client.ts +++ b/packages/client/src/promise/generated/client.ts @@ -639,6 +639,7 @@ export function make(options: ClientOptions) { model: input["model"], files: input["files"], agents: input["agents"], + skills: input["skills"], delivery: input["delivery"], resume: input["resume"], }, diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index 2cda4b2579..090811bc07 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -3520,6 +3520,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["id"] @@ -3539,6 +3543,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["command"] @@ -3558,6 +3566,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["arguments"] @@ -3577,6 +3589,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["agent"] @@ -3596,6 +3612,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["model"] @@ -3615,6 +3635,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["files"] @@ -3634,9 +3658,36 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["agents"] + readonly skills?: { + readonly id?: string | null + readonly command: string + readonly arguments?: string | null + readonly agent?: string | null + readonly model?: { readonly id: string; readonly providerID: string; readonly variant?: string } | null + readonly files?: ReadonlyArray<{ + readonly uri: string + readonly name?: string + readonly description?: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> + readonly agents?: ReadonlyArray<{ + readonly name: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> + readonly delivery?: "steer" | "queue" | null + readonly resume?: boolean | null + }["skills"] readonly delivery?: { readonly id?: string | null readonly command: string @@ -3653,6 +3704,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["delivery"] @@ -3672,6 +3727,10 @@ export type SessionCommandInput = { readonly name: string readonly mention?: { readonly start: number; readonly end: number; readonly text: string } }> + readonly skills?: ReadonlyArray<{ + readonly id: string + readonly mention?: { readonly start: number; readonly end: number; readonly text: string } + }> readonly delivery?: "steer" | "queue" | null readonly resume?: boolean | null }["resume"] diff --git a/packages/core/src/plugin/promise.ts b/packages/core/src/plugin/promise.ts index bbe1bb3218..a2d7f7b68d 100644 --- a/packages/core/src/plugin/promise.ts +++ b/packages/core/src/plugin/promise.ts @@ -312,6 +312,7 @@ export function fromPromise(plugin: Plugin) { id: input.id == null ? undefined : SessionMessage.ID.make(input.id), agent: input.agent == null ? undefined : Agent.ID.make(input.agent), model: input.model == null ? undefined : model(input.model), + skills: input.skills?.map((skill) => ({ ...skill, id: Skill.ID.make(skill.id) })), arguments: input.arguments ?? undefined, delivery: input.delivery ?? undefined, resume: input.resume ?? undefined, diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index d65b7f9e0f..240d215c88 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -144,6 +144,13 @@ type PendingInputRef = { readonly sessionID: SessionSchema.ID; readonly inputID: export class SkillNotFoundError extends Schema.TaggedErrorClass()("Session.SkillNotFoundError", { skill: Skill.ID, }) {} +export class SkillAttachmentError extends Schema.TaggedErrorClass()( + "Session.SkillAttachmentError", + { + skill: Skill.ID, + message: Schema.String, + }, +) {} export class DestinationNotFoundError extends Schema.TaggedErrorClass()( "Session.DestinationNotFoundError", @@ -222,7 +229,10 @@ export interface Interface { metadata?: Record delivery?: SessionPending.Delivery resume?: boolean - }) => Effect.Effect + }) => Effect.Effect< + SessionPending.User, + NotFoundError | PromptConflictError | AttachmentError | SkillNotFoundError | SkillAttachmentError + > /** Generates text from current Session context without admitting input or mutating history. */ readonly generate: (input: { sessionID: SessionSchema.ID @@ -237,6 +247,7 @@ export interface Interface { model?: Model.Ref files?: PromptInput.Prompt["files"] agents?: PromptInput.Prompt["agents"] + skills?: PromptInput.Prompt["skills"] delivery?: SessionPending.Delivery resume?: boolean }) => Effect.Effect< @@ -245,6 +256,7 @@ export interface Interface { | PromptConflictError | AttachmentError | SkillNotFoundError + | SkillAttachmentError | Command.NotFoundError | Command.EvaluationError > @@ -572,16 +584,32 @@ const layer = Layer.effect( // image attachment actually needs the resizer. const image = Image.Service.pipe(Effect.provide(locations.get(session.location))) const skills = Skill.Service.pipe(Effect.provide(locations.get(session.location))) + const messageID = input.id ?? SessionMessage.ID.create() + const delivery = input.delivery ?? "steer" + const previous = + input.id && input.skills?.length + ? yield* SessionPending.existing(db, { + id: messageID, + sessionID: input.sessionID, + delivery, + }).pipe( + Effect.catchDefect((defect) => + defect instanceof SessionPending.LifecycleConflict + ? new PromptConflictError({ sessionID: input.sessionID, messageID }) + : Effect.die(defect), + ), + ) + : undefined const prompt = yield* resolvePrompt( { text: input.text, files: input.files, agents: input.agents, skills: input.skills }, image, skills, + previous?.type === "user" ? previous.data.skills : undefined, ).pipe(Effect.provideService(FSUtil.Service, fs)) - const messageID = input.id ?? SessionMessage.ID.create() const admittedInput = SessionPending.Message.make({ type: "user", data: { ...prompt, metadata: input.metadata }, - delivery: input.delivery ?? "steer", + delivery, }) const admitted = yield* SessionPending.admit(db, bus, { id: messageID, @@ -641,6 +669,7 @@ const layer = Layer.effect( text: evaluated.text, files: input.files, agents: input.agents, + skills: input.skills, delivery: input.delivery, resume: input.resume, }) @@ -904,30 +933,39 @@ const resolvePrompt = Effect.fn("Session.resolvePrompt")(function* ( input: PromptInput.Prompt, image: Effect.Effect, skills: Effect.Effect, + previous: Prompt["skills"], ) { const fs = yield* FSUtil.Service const files = input.files ? yield* Effect.forEach(input.files, (file) => materializeAttachment(fs, file, image), { concurrency: 8 }) : undefined const requested = input.skills - const selected = requested?.length - ? yield* Effect.gen(function* () { - const available = yield* (yield* skills).list() - const attachments = Array.from(new Map(requested.map((attachment) => [attachment.id, attachment])).values()) - return yield* Effect.forEach(attachments, (attachment) => { - const skill = available.find((item) => item.id === attachment.id) - if (!skill) return Effect.fail(new SkillNotFoundError({ skill: attachment.id })) - return Skill.modelOutput(fs, skill).pipe( - Effect.map((output) => ({ - id: skill.id, - name: skill.name, - text: output.output, - mention: attachment.mention, - })), - ) - }) - }) - : undefined + const reusable = + requested && + previous && + JSON.stringify(requested) === JSON.stringify(previous.map((skill) => ({ id: skill.id, mention: skill.mention }))) + ? previous + : undefined + const selected = yield* Effect.gen(function* () { + if (!requested?.length) return undefined + if (reusable) return reusable + const available = yield* (yield* skills).list() + return yield* Effect.forEach(requested, (attachment) => + Effect.gen(function* () { + const skill = available.find((item) => item.id === attachment.id) + if (!skill) return yield* new SkillNotFoundError({ skill: attachment.id }) + const output = yield* Skill.modelOutput(fs, skill).pipe( + Effect.mapError((error) => new SkillAttachmentError({ skill: skill.id, message: String(error) })), + ) + return { + id: skill.id, + name: skill.name, + text: output.output, + mention: attachment.mention, + } + }), + ) + }) return Prompt.make({ text: input.text, agents: input.agents, files, skills: selected?.length ? selected : undefined }) }) diff --git a/packages/core/src/session/pending.ts b/packages/core/src/session/pending.ts index d656564b51..00975a0e0d 100644 --- a/packages/core/src/session/pending.ts +++ b/packages/core/src/session/pending.ts @@ -130,6 +130,15 @@ const promotedFromMessage = Effect.fn("SessionPending.promotedFromMessage")(func return yield* Effect.die(new LifecycleConflict({ id })) }) +export const existing = Effect.fn("SessionPending.existing")(function* ( + db: DatabaseService, + input: PendingRef & { readonly delivery: Delivery }, +) { + const pending = yield* find(db, input.id) + if (pending !== undefined) return pending + return yield* promotedFromMessage(db, input.sessionID, input.id, input.delivery) +}) + export const admit = Effect.fn("SessionPending.admit")(function* ( db: DatabaseService, bus: Bus.Interface, @@ -139,13 +148,15 @@ export const admit = Effect.fn("SessionPending.admit")(function* ( readonly input: Message }, ) { - const existing = yield* find(db, request.id) - if (existing !== undefined) { - if (existing.type === "compaction") return yield* Effect.die(new LifecycleConflict({ id: request.id })) - return existing + const stored = yield* existing(db, { + id: request.id, + sessionID: request.sessionID, + delivery: request.input.delivery, + }) + if (stored !== undefined) { + if (stored.type === "compaction") return yield* Effect.die(new LifecycleConflict({ id: request.id })) + return stored } - const promoted = yield* promotedFromMessage(db, request.sessionID, request.id, request.input.delivery) - if (promoted !== undefined) return promoted return yield* bus .publish(SessionEvent.InputAdmitted, { inputID: request.id, diff --git a/packages/core/src/skill.ts b/packages/core/src/skill.ts index 866709e16a..54481bb40c 100644 --- a/packages/core/src/skill.ts +++ b/packages/core/src/skill.ts @@ -63,9 +63,7 @@ export const modelOutput = Effect.fn("Skill.modelOutput")(function* (fs: FSUtil. const directory = path.dirname(skill.location) const files = path.basename(skill.location) === "SKILL.md" - ? (yield* fs - .scan("**/*", { cwd: directory, absolute: true, include: "file", dot: true }) - .pipe(Effect.catch(() => Effect.succeed([] as string[])))) + ? (yield* fs.scan("**/*", { cwd: directory, absolute: true, include: "file", dot: true })) .filter((file) => path.basename(file) !== "SKILL.md") .toSorted() .slice(0, FILE_LIMIT) diff --git a/packages/core/test/session-skill.test.ts b/packages/core/test/session-skill.test.ts index 7b0fc072fd..68e2d7a9d5 100644 --- a/packages/core/test/session-skill.test.ts +++ b/packages/core/test/session-skill.test.ts @@ -23,17 +23,16 @@ const location = Location.Ref.make({ directory: AbsolutePath.make("/project") }) const projects = Layer.mock(Project.Service, { resolve: (directory) => Effect.succeed({ id: Project.ID.global, directory, canonical: directory }), }) +const effectSkill = Skill.Info.make({ + id: Skill.ID.make("effect"), + name: Skill.Name.make("Effect"), + description: "Effect guidance", + location: AbsolutePath.make(path.resolve("/skills/effect.md")), + content: "Use Effect", +}) +let listedSkills: Skill.Info[] = [effectSkill] const skills = Layer.mock(Skill.Service, { - list: () => - Effect.succeed([ - Skill.Info.make({ - id: Skill.ID.make("effect"), - name: Skill.Name.make("Effect"), - description: "Effect guidance", - location: AbsolutePath.make(path.resolve("/skills/effect.md")), - content: "Use Effect", - }), - ]), + list: () => Effect.sync(() => listedSkills), }) const locations = Layer.effect( LocationServiceMap.Service, @@ -56,6 +55,29 @@ const it = testEffect( ) describe("Session.skill", () => { + it.effect("reconciles a promoted skill prompt after the live skill disappears", () => + Effect.gen(function* () { + listedSkills = [effectSkill] + const sessions = yield* Session.Service + const database = yield* Database.Service + const bus = yield* Bus.Service + const session = yield* sessions.create({ location }) + const input = { + id: SessionMessage.ID.make("msg_skill_retry"), + sessionID: session.id, + text: "Apply this guidance", + skills: [{ id: Skill.ID.make("effect") }], + resume: false, + } + + yield* sessions.prompt(input) + yield* SessionPending.promote(database.db, bus, session.id, "steer") + listedSkills = [] + + expect(yield* sessions.prompt(input)).toMatchObject({ id: input.id, data: { text: input.text } }) + }).pipe(Effect.ensuring(Effect.sync(() => (listedSkills = [effectSkill])))), + ) + it.effect("attaches a resolved skill snapshot to a normal prompt", () => Effect.gen(function* () { const sessions = yield* Session.Service diff --git a/packages/protocol/src/groups/session.ts b/packages/protocol/src/groups/session.ts index 2380a25f9f..f2e87c1863 100644 --- a/packages/protocol/src/groups/session.ts +++ b/packages/protocol/src/groups/session.ts @@ -347,6 +347,7 @@ export const makeSessionGroup = (sessionLo model: Model.Ref.pipe(Schema.optional), files: PromptInput.Prompt.fields.files, agents: PromptInput.Prompt.fields.agents, + skills: PromptInput.Prompt.fields.skills, delivery: SessionPending.Delivery.pipe(Schema.optional), resume: Schema.Boolean.pipe(Schema.optional), }), diff --git a/packages/server/src/handlers/session.ts b/packages/server/src/handlers/session.ts index f68b364b25..955c4e5bcd 100644 --- a/packages/server/src/handlers/session.ts +++ b/packages/server/src/handlers/session.ts @@ -341,6 +341,9 @@ export const SessionHandler = HttpApiBuilder.group(Api, "server.session", (handl Effect.catchTag("Session.SkillNotFoundError", (error) => Effect.fail(new InvalidRequestError({ message: `Skill not found: ${error.skill}`, field: "skills" })), ), + Effect.catchTag("Session.SkillAttachmentError", (error) => + Effect.fail(new InvalidRequestError({ message: error.message, field: "skills" })), + ), ), } }), @@ -359,6 +362,7 @@ export const SessionHandler = HttpApiBuilder.group(Api, "server.session", (handl model: ctx.payload.model, files: ctx.payload.files, agents: ctx.payload.agents, + skills: ctx.payload.skills, delivery: ctx.payload.delivery, resume: ctx.payload.resume, }) @@ -401,6 +405,9 @@ export const SessionHandler = HttpApiBuilder.group(Api, "server.session", (handl Effect.catchTag("Session.SkillNotFoundError", (error) => Effect.fail(new InvalidRequestError({ message: `Skill not found: ${error.skill}`, field: "skills" })), ), + Effect.catchTag("Session.SkillAttachmentError", (error) => + Effect.fail(new InvalidRequestError({ message: error.message, field: "skills" })), + ), ), } }), diff --git a/packages/tui/src/component/prompt/autocomplete.tsx b/packages/tui/src/component/prompt/autocomplete.tsx index 6ee3e010bc..024f354968 100644 --- a/packages/tui/src/component/prompt/autocomplete.tsx +++ b/packages/tui/src/component/prompt/autocomplete.tsx @@ -54,6 +54,7 @@ export function Autocomplete(props: { fileStyleId: number agentStyleId: number skillStyleId: number + hasSkill: (id: string) => boolean promptPartTypeId: () => number }) { const editor = useEditorContext() @@ -146,6 +147,7 @@ export function Autocomplete(props: { | { type: "agent"; value: NonNullable[number] } | { type: "skill"; value: NonNullable[number] }, ) { + if (part.type === "skill" && props.hasSkill(part.value.id)) return const input = props.input() const currentCursorOffset = input.cursorOffset diff --git a/packages/tui/src/component/prompt/index.tsx b/packages/tui/src/component/prompt/index.tsx index 67787e39dd..247b20fa58 100644 --- a/packages/tui/src/component/prompt/index.tsx +++ b/packages/tui/src/component/prompt/index.tsx @@ -1117,6 +1117,7 @@ export function Prompt(props: PromptProps) { model, files: store.prompt.files, agents: store.prompt.agents, + skills: store.prompt.skills?.length ? store.prompt.skills : undefined, delivery, }) .catch((error) => { @@ -1724,6 +1725,7 @@ export function Prompt(props: PromptProps) { fileStyleId={fileStyleId} agentStyleId={agentStyleId} skillStyleId={skillStyleId} + hasSkill={(id) => store.prompt.skills?.some((skill) => skill.id === id) ?? false} promptPartTypeId={() => promptPartTypeId} /> diff --git a/packages/tui/src/mini/stream-v2.transport.ts b/packages/tui/src/mini/stream-v2.transport.ts index 0caa637a39..637b5b45f6 100644 --- a/packages/tui/src/mini/stream-v2.transport.ts +++ b/packages/tui/src/mini/stream-v2.transport.ts @@ -373,12 +373,12 @@ const catalogEvents = new Set([ // briefly so the output commit renders inside it. const SHELL_OUTPUT_GRACE_MS = 1500 -function skillCommit(messageID: string, name: string): StreamCommit { +function skillCommit(messageID: string, name: string, skillID = messageID): StreamCommit { return { kind: "system", source: "system", messageID, - partID: `skill:${messageID}`, + partID: `skill:${skillID}`, text: `→ Skill "${name}"`, phase: "start", } @@ -653,7 +653,7 @@ export async function createSessionTransport(input: StreamInput): Promise skillCommit(message.id + ":" + skill.id, skill.name)), + ...(message.skills ?? []).map((skill) => skillCommit(message.id, skill.name, skill.id)), { kind: "user", source: "system", text: message.text, phase: "start", messageID: message.id }, ]) return @@ -1666,6 +1666,7 @@ export async function createSessionTransport(input: StreamInput): Promise { variant: undefined, prompt: { messageID: "msg_cmd", - text: "/deploy prod", + text: "/deploy prod /api-design", parts: [ { type: "file", @@ -2757,6 +2757,11 @@ describe("V2 mini transport", () => { filename: "mentioned.txt", source: { type: "file", text: { start: 8, end: 12, value: "prod" } }, }, + { + type: "skill", + id: "api-design", + source: { start: 13, end: 24, value: "/api-design" }, + }, ], command: { name: "deploy", arguments: "prod" }, }, @@ -2779,6 +2784,7 @@ describe("V2 mini transport", () => { mention: { start: 8, end: 12, text: "prod" }, }, ], + skills: [{ id: "api-design", mention: { start: 13, end: 24, text: "/api-design" } }], delivery: "steer", }) // Selection rides the command payload; no separate client-side switch.