mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-31 02:14:58 +00:00
fix(flow): namespace projected skills, unbounded queued activations, steer-merge guard
Review round 33: - Projected flow skills now live under their own catalog namespace (flow:<id>), so a flow can never shadow an ordinary skill with the same id; the TUI and ACP slash builders pass the name through instead of prefixing, which also fixes queued-prompt recall text losing the /flow: prefix. - The pending-activation map no longer caps at eight entries: a deep queue of admitted flow prompts keeps every activation, since a silent eviction would deliver a blueprint whose run never starts (leftover entries from aborted prompts are small and bounded by user action). - A steered prompt that merged several flow activations starts only the first and clears the rest, so a later reconciliation cannot surprise-start the second flow against the same merged prompt. - The live flag-flip status publisher runs only on the main agent, so a worker's null frame can no longer race the session's flow indicator.
This commit is contained in:
parent
fa130d376e
commit
d5ee53d61a
9 changed files with 71 additions and 44 deletions
|
|
@ -34,11 +34,9 @@ export function buildSkillSlashCommands(skills: readonly SkillSummary[]): SkillS
|
|||
const sortedSkills = [...skills].toSorted(compareSkillSlashCommands);
|
||||
const commands = sortedSkills.filter(isUserActivatableSkill).map((skill) => {
|
||||
const commandName =
|
||||
skill.type === 'flow'
|
||||
? `flow:${skill.name}`
|
||||
: skill.source === 'builtin' || skill.isSubSkill === true
|
||||
? skill.name
|
||||
: `skill:${skill.name}`;
|
||||
skill.type === 'flow' || skill.source === 'builtin' || skill.isSubSkill === true
|
||||
? skill.name
|
||||
: `skill:${skill.name}`;
|
||||
commandMap.set(commandName, skill.name);
|
||||
return {
|
||||
name: commandName,
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ describe('skill slash commands', () => {
|
|||
path: '/skills/parent/nested-review/SKILL.md',
|
||||
}),
|
||||
skill('agent-only', 'agent'),
|
||||
skill('commit', 'flow'),
|
||||
skill('flow:commit', 'flow'),
|
||||
]);
|
||||
|
||||
expect(built.commands.map((command) => command.name)).toEqual([
|
||||
|
|
@ -46,7 +46,7 @@ describe('skill slash commands', () => {
|
|||
expect(built.commands[0]).toMatchObject({
|
||||
name: 'flow:commit',
|
||||
aliases: [],
|
||||
description: 'commit skill',
|
||||
description: 'flow:commit skill',
|
||||
});
|
||||
expect(built.commands[1]).toMatchObject({
|
||||
name: 'skill:nested-review',
|
||||
|
|
@ -54,7 +54,7 @@ describe('skill slash commands', () => {
|
|||
description: 'Nested review skill',
|
||||
});
|
||||
expect([...built.commandMap.entries()]).toEqual([
|
||||
['flow:commit', 'commit'],
|
||||
['flow:commit', 'flow:commit'],
|
||||
['skill:nested-review', 'nested-review'],
|
||||
['skill:review', 'review'],
|
||||
]);
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ export interface SkillSlashCommands {
|
|||
/**
|
||||
* Project the session skill summaries into slash commands. Mirrors the TUI's
|
||||
* `buildSkillSlashCommands` (apps/kimi-code/src/tui/commands/skills.ts):
|
||||
* user-activatable skills only: flow-typed skills get the `flow:` prefix,
|
||||
* user-activatable skills only: flow-typed skills already carry their `flow:` catalog prefix,
|
||||
* builtin-source skills and sub-skills the bare name, everything else the
|
||||
* `skill:` prefix, builtin-source group first.
|
||||
* One ACP-specific deviation: a skill whose command name collides with an ACP
|
||||
|
|
@ -88,11 +88,9 @@ export function buildAcpSkillSlashCommands(
|
|||
for (const skill of sorted) {
|
||||
if (!isUserActivatableSkillType(skill.type)) continue;
|
||||
const commandName =
|
||||
skill.type === 'flow'
|
||||
? `flow:${skill.name}`
|
||||
: skill.source === 'builtin' || skill.isSubSkill === true
|
||||
? skill.name
|
||||
: `skill:${skill.name}`;
|
||||
skill.type === 'flow' || skill.source === 'builtin' || skill.isSubSkill === true
|
||||
? skill.name
|
||||
: `skill:${skill.name}`;
|
||||
if (reservedNames.has(commandName)) continue;
|
||||
commandMap.set(commandName, skill.name);
|
||||
commands.push({ name: commandName, description: skill.description });
|
||||
|
|
|
|||
|
|
@ -55,7 +55,7 @@ describe('buildAcpSkillSlashCommands', () => {
|
|||
it('filters out skills the user cannot activate', () => {
|
||||
const { commands } = buildAcpSkillSlashCommands([
|
||||
skill('reference-only', { type: 'reference' }),
|
||||
skill('flow-one', { type: 'flow' }),
|
||||
skill('flow:flow-one', { type: 'flow' }),
|
||||
skill('inline-one'),
|
||||
]);
|
||||
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ import {
|
|||
type FlowStageDefinition,
|
||||
} from './flow';
|
||||
import { FlowGateReview } from './flowGateReview';
|
||||
import { flowDefinitionPath } from './flowsSkillSource';
|
||||
import { FLOW_SKILL_NAME_PREFIX, flowDefinitionPath } from './flowsSkillSource';
|
||||
import { ISessionWorkspaceContext } from '#/session/workspaceContext/workspaceContext';
|
||||
|
||||
import { FlowRunEnded, FlowRunStarted, FlowVerdict, flowGatesKey, flowKey } from './flowOps';
|
||||
|
|
@ -141,6 +141,7 @@ export class AgentFlowService extends Disposable implements IAgentFlowService {
|
|||
const flagNow = this.flags.enabled(FLOW_FLAG_ID);
|
||||
if (flagNow === flagWas) return;
|
||||
flagWas = flagNow;
|
||||
if (this.scopeContext.agentId !== 'main') return;
|
||||
void this.dispatcher.dispatch(
|
||||
new AgentStatusUpdated({ flowRun: flagNow ? this.summary() : null }),
|
||||
);
|
||||
|
|
@ -163,11 +164,13 @@ export class AgentFlowService extends Disposable implements IAgentFlowService {
|
|||
|
||||
private prepareActivationStart(
|
||||
activationId: string,
|
||||
flowId: string | undefined,
|
||||
skillName: string | undefined,
|
||||
task: string | undefined,
|
||||
skillPath: string | undefined,
|
||||
): void {
|
||||
if (flowId === undefined || flowId.length === 0 || this.run().active) return;
|
||||
if (skillName === undefined || !skillName.startsWith(FLOW_SKILL_NAME_PREFIX)) return;
|
||||
const flowId = skillName.slice(FLOW_SKILL_NAME_PREFIX.length);
|
||||
if (flowId.length === 0 || this.run().active) return;
|
||||
if (
|
||||
skillPath === undefined ||
|
||||
resolve(skillPath) !== resolve(flowDefinitionPath(this.workspaceCtx.workDir, flowId))
|
||||
|
|
@ -177,11 +180,6 @@ export class AgentFlowService extends Disposable implements IAgentFlowService {
|
|||
const parsed = FlowDefinitionSchema.safeParse(this.activationData.take(activationId));
|
||||
if (!parsed.success || parsed.data.id !== flowId) return;
|
||||
this.pendingActivations.set(activationId, { definition: parsed.data, task: task?.trim() ?? '' });
|
||||
while (this.pendingActivations.size > 8) {
|
||||
const oldest = this.pendingActivations.keys().next().value;
|
||||
if (oldest === undefined) break;
|
||||
this.pendingActivations.delete(oldest);
|
||||
}
|
||||
}
|
||||
|
||||
reconcilePendingActivation(): void {
|
||||
|
|
@ -192,19 +190,17 @@ export class AgentFlowService extends Disposable implements IAgentFlowService {
|
|||
.findLast((message) => message.role === 'user' && message.origin?.kind !== 'injection');
|
||||
const origin = last?.origin;
|
||||
if (origin === undefined) return;
|
||||
const matchedId =
|
||||
const promptActivationIds =
|
||||
origin.kind === 'skill_activation'
|
||||
? this.pendingActivations.has(origin.activationId)
|
||||
? origin.activationId
|
||||
: undefined
|
||||
? [origin.activationId]
|
||||
: origin.kind === 'user'
|
||||
? (origin.skillActivations ?? []).find((entry) =>
|
||||
this.pendingActivations.has(entry.activationId),
|
||||
)?.activationId
|
||||
: undefined;
|
||||
if (matchedId === undefined) return;
|
||||
const pending = this.pendingActivations.get(matchedId);
|
||||
this.pendingActivations.delete(matchedId);
|
||||
? (origin.skillActivations ?? []).map((entry) => entry.activationId)
|
||||
: [];
|
||||
const matchedIds = promptActivationIds.filter((id) => this.pendingActivations.has(id));
|
||||
const firstMatch = matchedIds[0];
|
||||
if (firstMatch === undefined) return;
|
||||
const pending = this.pendingActivations.get(firstMatch);
|
||||
for (const id of matchedIds) this.pendingActivations.delete(id);
|
||||
if (pending === undefined) return;
|
||||
if (this.run().active) return;
|
||||
let task = pending.task;
|
||||
|
|
|
|||
|
|
@ -21,6 +21,13 @@ import { FLOW_SUPERVISOR_CONTRACT } from './skill/skill';
|
|||
|
||||
export const FLOWS_SKILL_SOURCE_ID = 'flows';
|
||||
|
||||
/**
|
||||
* Catalog-name prefix of every projected flow skill (`flow:<id>`), keeping
|
||||
* flow commands in their own namespace so a flow can never shadow an
|
||||
* ordinary skill with the same id.
|
||||
*/
|
||||
export const FLOW_SKILL_NAME_PREFIX = 'flow:';
|
||||
|
||||
function joinWorkspacePath(root: string, relative: string): string {
|
||||
return `${root.replace(/[\\/]+$/, '')}/${relative}`;
|
||||
}
|
||||
|
|
@ -183,13 +190,13 @@ function toFlowSkill(definition: FlowDefinition, path: string, dir: string): Ski
|
|||
'", task: <the task>). Likewise, if no current-stage reminder appears in your context, the automatic start failed — recover by calling FlowStart yourself.',
|
||||
].join('\n');
|
||||
return {
|
||||
name: id,
|
||||
name: `${FLOW_SKILL_NAME_PREFIX}${id}`,
|
||||
description,
|
||||
path,
|
||||
dir,
|
||||
content,
|
||||
metadata: {
|
||||
name: id,
|
||||
name: `${FLOW_SKILL_NAME_PREFIX}${id}`,
|
||||
description,
|
||||
type: 'flow',
|
||||
disableModelInvocation: true,
|
||||
|
|
|
|||
|
|
@ -399,7 +399,7 @@ describe('AgentFlowService', () => {
|
|||
const origin = {
|
||||
kind: 'skill_activation',
|
||||
activationId,
|
||||
skillName,
|
||||
skillName: `flow:${skillName}`,
|
||||
trigger: 'user-slash',
|
||||
skillType: 'flow',
|
||||
skillPath,
|
||||
|
|
@ -435,7 +435,7 @@ describe('AgentFlowService', () => {
|
|||
origin: {
|
||||
kind: 'skill_activation',
|
||||
activationId: 'act-1',
|
||||
skillName: 'issue-fix',
|
||||
skillName: 'flow:issue-fix',
|
||||
trigger: 'user-slash',
|
||||
skillType: 'flow',
|
||||
skillPath: '/ws/.kimi-code/flows/issue-fix.md',
|
||||
|
|
@ -497,11 +497,39 @@ describe('AgentFlowService', () => {
|
|||
expect(service.run().active).toBe(false);
|
||||
});
|
||||
|
||||
it('starts only the first flow of a steered multi-activation prompt and clears the rest', async () => {
|
||||
await activateFlowSkill({ activationId: 'act-s1', appendPrompt: false, reconcile: false });
|
||||
await activateFlowSkill({
|
||||
activationId: 'act-s2',
|
||||
skillName: 'other-flow',
|
||||
appendPrompt: false,
|
||||
reconcile: false,
|
||||
});
|
||||
contextMessages.push({
|
||||
role: 'user',
|
||||
content: [{ type: 'text', text: 'merged steer prompt' }],
|
||||
toolCalls: [],
|
||||
origin: {
|
||||
kind: 'user',
|
||||
skillActivations: [
|
||||
{ activationId: 'act-s1', skillName: 'flow:issue-fix' },
|
||||
{ activationId: 'act-s2', skillName: 'flow:other-flow' },
|
||||
],
|
||||
},
|
||||
} as unknown as ContextMessage);
|
||||
service.reconcilePendingActivation();
|
||||
expect(service.run().flowId).toBe('issue-fix');
|
||||
|
||||
service.abort('stop');
|
||||
service.reconcilePendingActivation();
|
||||
expect(service.run().active).toBe(false);
|
||||
});
|
||||
|
||||
it('preserves each queued flow activation until its own prompt lands', async () => {
|
||||
await activateFlowSkill({ activationId: 'act-q1', appendPrompt: false, reconcile: false });
|
||||
await activateFlowSkill({
|
||||
activationId: 'act-q2',
|
||||
skillName: 'other-flow',
|
||||
skillName: 'flow:other-flow',
|
||||
appendPrompt: false,
|
||||
reconcile: false,
|
||||
});
|
||||
|
|
@ -512,7 +540,7 @@ describe('AgentFlowService', () => {
|
|||
origin: {
|
||||
kind: 'skill_activation',
|
||||
activationId: 'act-q1',
|
||||
skillName: 'issue-fix',
|
||||
skillName: 'flow:issue-fix',
|
||||
trigger: 'user-slash',
|
||||
skillType: 'flow',
|
||||
skillPath: '/ws/.kimi-code/flows/issue-fix.md',
|
||||
|
|
@ -529,7 +557,7 @@ describe('AgentFlowService', () => {
|
|||
origin: {
|
||||
kind: 'skill_activation',
|
||||
activationId: 'act-q2',
|
||||
skillName: 'other-flow',
|
||||
skillName: 'flow:other-flow',
|
||||
trigger: 'user-slash',
|
||||
skillType: 'flow',
|
||||
skillPath: '/ws/.kimi-code/flows/other-flow.md',
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ describe('FlowsSkillSource', () => {
|
|||
const contribution = await source.load();
|
||||
expect(contribution.skills).toHaveLength(1);
|
||||
const skill = contribution.skills[0]!;
|
||||
expect(skill.name).toBe('issue-fix');
|
||||
expect(skill.name).toBe('flow:issue-fix');
|
||||
expect(skill.source).toBe('project');
|
||||
expect(skill.metadata.type).toBe('flow');
|
||||
expect(skill.metadata.disableModelInvocation).toBe(true);
|
||||
|
|
|
|||
|
|
@ -58,7 +58,7 @@ describe('flows skill source e2e', () => {
|
|||
headers: authHeaders(server!),
|
||||
} as never);
|
||||
const body = (await res.json()) as { data: { skills: { name: string; type?: string }[] } };
|
||||
const flow = body.data.skills.find((s) => s.name === 'issue-fix');
|
||||
const flow = body.data.skills.find((s) => s.name === 'flow:issue-fix');
|
||||
expect(flow).toBeDefined();
|
||||
expect(flow?.type).toBe('flow');
|
||||
});
|
||||
|
|
@ -76,7 +76,7 @@ describe('flows skill source e2e', () => {
|
|||
{ headers: authHeaders(server!) } as never,
|
||||
);
|
||||
const body = (await res.json()) as { data: { skills: { name: string; type?: string }[] } };
|
||||
const flow = body.data.skills.find((s) => s.name === 'issue-fix');
|
||||
const flow = body.data.skills.find((s) => s.name === 'flow:issue-fix');
|
||||
expect(flow).toBeDefined();
|
||||
expect(flow?.type).toBe('flow');
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue