Skip to content

Commit bf7b670

Browse files
committed
chore: fix code review
1 parent b0482cb commit bf7b670

File tree

3 files changed

+36
-14
lines changed

3 files changed

+36
-14
lines changed

.changeset/olive-tomatoes-juggle.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,5 @@ Enable workspace skills prompt injection by default when an agent has a workspac
66

77
- Agents now auto-compose a workspace skills prompt hook by default.
88
- Added `workspaceSkillsPrompt` to `AgentOptions` to customize (`WorkspaceSkillsPromptOptions`), force (`true`), or disable (`false`) prompt injection.
9-
- To avoid duplicate injections for existing setups, default auto-injection is skipped when a custom `hooks.onPrepareMessages` is already provided.
9+
- When a custom `hooks.onPrepareMessages` is provided, it now composes with the default workspace skills prompt hook unless `workspaceSkillsPrompt` is explicitly set to `false`.
1010
- Updated workspace skills docs and the `examples/with-workspace` sample to document and use the new behavior.

packages/core/src/agent/agent.spec.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,32 @@ Use pandas and summarize findings.`.split("\n"),
215215
expect(systemTexts.some((text) => text.includes("Data Analyst (/skills/data)"))).toBe(true);
216216
});
217217

218-
it("skips auto-injection when custom onPrepareMessages exists", async () => {
218+
it("composes auto-injection with custom onPrepareMessages by default", async () => {
219+
const workspace = createWorkspaceWithSkill();
220+
const onPrepareMessages = vi.fn(({ messages }) => ({
221+
messages: (messages || []).filter((message) => message.role !== "system"),
222+
}));
223+
224+
const agent = new Agent({
225+
name: "TestAgent",
226+
instructions: "Use skills when relevant.",
227+
model: mockModel as any,
228+
workspace,
229+
hooks: { onPrepareMessages },
230+
});
231+
232+
vi.mocked(ai.generateText).mockResolvedValue(createMockGenerateTextResponse() as any);
233+
234+
await agent.generateText("Analyze my data");
235+
236+
const callArgs = vi.mocked(ai.generateText).mock.calls[0]?.[0];
237+
const systemTexts = getSystemTexts(callArgs?.messages);
238+
239+
expect(onPrepareMessages).toHaveBeenCalledTimes(1);
240+
expect(systemTexts.some((text) => text.includes("<workspace_skills>"))).toBe(true);
241+
});
242+
243+
it("disables auto-injection when workspaceSkillsPrompt is false", async () => {
219244
const workspace = createWorkspaceWithSkill();
220245
const onPrepareMessages = vi.fn(({ messages }) => ({ messages }));
221246

@@ -225,6 +250,7 @@ Use pandas and summarize findings.`.split("\n"),
225250
model: mockModel as any,
226251
workspace,
227252
hooks: { onPrepareMessages },
253+
workspaceSkillsPrompt: false,
228254
});
229255

230256
vi.mocked(ai.generateText).mockResolvedValue(createMockGenerateTextResponse() as any);
@@ -248,6 +274,7 @@ Use pandas and summarize findings.`.split("\n"),
248274
model: mockModel as any,
249275
workspace,
250276
hooks: { onPrepareMessages },
277+
workspaceToolkits: { skills: false },
251278
workspaceSkillsPrompt: true,
252279
});
253280

packages/core/src/agent/agent.ts

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -379,27 +379,26 @@ const resolveWorkspaceSkillsPromptHook = (
379379
workspace: Workspace | undefined,
380380
options: AgentOptions,
381381
): AgentHooks["onPrepareMessages"] | undefined => {
382+
const existingHook = options.hooks?.onPrepareMessages;
382383
if (!workspace?.skills) {
383-
return undefined;
384+
return existingHook;
384385
}
385386

386387
const promptConfig = options.workspaceSkillsPrompt;
387388
if (promptConfig === false) {
388-
return undefined;
389+
return existingHook;
389390
}
390391

391392
const hasExplicitPromptConfig = promptConfig !== undefined;
392-
if (!hasExplicitPromptConfig && options.hooks?.onPrepareMessages) {
393-
return undefined;
394-
}
395393
if (!hasExplicitPromptConfig && !isWorkspaceSkillsToolkitEnabled(options.workspaceToolkits)) {
396-
return undefined;
394+
return existingHook;
397395
}
398396

399397
const promptOptions =
400398
typeof promptConfig === "object" && promptConfig !== null ? promptConfig : {};
401399

402-
return workspace.createSkillsPromptHook(promptOptions).onPrepareMessages;
400+
const skillsPromptHook = workspace.createSkillsPromptHook(promptOptions).onPrepareMessages;
401+
return composePrepareMessagesHooks([existingHook, skillsPromptHook]);
403402
};
404403

405404
const searchToolsParameters = z.object({
@@ -822,11 +821,7 @@ export class Agent {
822821
const globalWorkspace = AgentRegistry.getInstance().getGlobalWorkspace();
823822
const workspaceOption = options.workspace === undefined ? globalWorkspace : options.workspace;
824823
this.workspace = resolveWorkspace(workspaceOption);
825-
const workspaceSkillsPromptHook = resolveWorkspaceSkillsPromptHook(this.workspace, options);
826-
const onPrepareMessages = composePrepareMessagesHooks([
827-
workspaceSkillsPromptHook,
828-
options.hooks?.onPrepareMessages,
829-
]);
824+
const onPrepareMessages = resolveWorkspaceSkillsPromptHook(this.workspace, options);
830825
this.hooks = onPrepareMessages
831826
? { ...(options.hooks || {}), onPrepareMessages }
832827
: options.hooks || {};

0 commit comments

Comments
 (0)