Skip to content

Commit e4f9f8d

Browse files
authored
fix: missing files in skillsMiddleware schema (#160)
* fix missing files in skillsMiddleware * Update tests * Add integration tests + fixes * Restore skills.test.ts from main * Update tests to unit from integration * Update unit tests * fix test * changeset
1 parent c674c61 commit e4f9f8d

File tree

3 files changed

+288
-1
lines changed

3 files changed

+288
-1
lines changed
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"deepagents": patch
3+
---
4+
5+
fix(skills): properly restore skills from StateBackend checkpoint
6+
7+
- Add `files` channel to `SkillsStateSchema` for StateBackend integration
8+
- Fix skills restoration check to require non-empty array instead of just non-null
9+
- Export `FileDataSchema` from fs middleware for reuse

libs/deepagents/src/middleware/skills.test.ts

Lines changed: 265 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ import type {
99
FileDownloadResponse,
1010
FileInfo,
1111
} from "../backends/protocol.js";
12+
import { createFileData } from "../backends/utils.js";
13+
import { createDeepAgent } from "../agent.js";
14+
import { FakeListChatModel } from "@langchain/core/utils/testing";
15+
import {
16+
HumanMessage,
17+
SystemMessage,
18+
type BaseMessage,
19+
} from "@langchain/core/messages";
20+
import { MemorySaver } from "@langchain/langgraph";
1221

1322
// Mock backend that returns specified files and directory listings
1423
function createMockBackend(config: {
@@ -672,3 +681,259 @@ describe("skillsMetadataReducer", () => {
672681
});
673682
});
674683
});
684+
685+
/**
686+
* StateBackend integration tests.
687+
*
688+
* These tests verify that skills are properly loaded from state.files and
689+
* injected into the system prompt when using createDeepAgent with StateBackend.
690+
*/
691+
describe("StateBackend integration with createDeepAgent", () => {
692+
const VALID_SKILL_MD = `---
693+
name: test-skill
694+
description: A test skill for StateBackend integration
695+
---
696+
697+
# Test Skill
698+
699+
Instructions for the test skill.
700+
`;
701+
702+
const ANOTHER_SKILL_MD = `---
703+
name: another-skill
704+
description: Another test skill
705+
---
706+
707+
# Another Skill
708+
`;
709+
710+
/**
711+
* Helper to extract system prompt content from model invoke spy.
712+
* The system message can have content as string or array of content blocks.
713+
*/
714+
function getSystemPromptFromSpy(
715+
invokeSpy: ReturnType<typeof vi.spyOn>,
716+
): string {
717+
const lastCall = invokeSpy.mock.calls[invokeSpy.mock.calls.length - 1];
718+
const messages = lastCall?.[0] as BaseMessage[] | undefined;
719+
if (!messages) return "";
720+
const systemMessage = messages.find(SystemMessage.isInstance);
721+
if (!systemMessage) return "";
722+
723+
return systemMessage.text;
724+
}
725+
726+
it("should load skills from state.files and inject into system prompt", async () => {
727+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
728+
const model = new FakeListChatModel({ responses: ["Done"] });
729+
730+
const checkpointer = new MemorySaver();
731+
const agent = createDeepAgent({
732+
model: model as any,
733+
skills: ["/skills/"],
734+
checkpointer,
735+
});
736+
737+
await agent.invoke(
738+
{
739+
messages: [new HumanMessage("What skills are available?")],
740+
files: {
741+
"/skills/test-skill/SKILL.md": createFileData(VALID_SKILL_MD),
742+
},
743+
} as any,
744+
{ configurable: { thread_id: `test-${Date.now()}` }, recursionLimit: 50 },
745+
);
746+
747+
expect(invokeSpy).toHaveBeenCalled();
748+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
749+
750+
// Verify skill was injected into system prompt
751+
expect(systemPrompt).toContain("test-skill");
752+
expect(systemPrompt).toContain("A test skill for StateBackend integration");
753+
expect(systemPrompt).toContain("/skills/test-skill/SKILL.md");
754+
invokeSpy.mockRestore();
755+
});
756+
757+
it("should load multiple skills from state.files", async () => {
758+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
759+
const model = new FakeListChatModel({ responses: ["Done"] });
760+
761+
const checkpointer = new MemorySaver();
762+
const agent = createDeepAgent({
763+
model: model as any,
764+
skills: ["/skills/"],
765+
checkpointer,
766+
});
767+
768+
await agent.invoke(
769+
{
770+
messages: [new HumanMessage("List all skills")],
771+
files: {
772+
"/skills/test-skill/SKILL.md": createFileData(VALID_SKILL_MD),
773+
"/skills/another-skill/SKILL.md": createFileData(ANOTHER_SKILL_MD),
774+
},
775+
} as any,
776+
{
777+
configurable: { thread_id: `test-multi-${Date.now()}` },
778+
recursionLimit: 50,
779+
},
780+
);
781+
782+
expect(invokeSpy).toHaveBeenCalled();
783+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
784+
785+
// Verify both skills were injected
786+
expect(systemPrompt).toContain("test-skill");
787+
expect(systemPrompt).toContain("another-skill");
788+
expect(systemPrompt).toContain("A test skill for StateBackend integration");
789+
expect(systemPrompt).toContain("Another test skill");
790+
invokeSpy.mockRestore();
791+
});
792+
793+
it("should show no skills message when state.files is empty", async () => {
794+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
795+
const model = new FakeListChatModel({ responses: ["Done"] });
796+
797+
const checkpointer = new MemorySaver();
798+
const agent = createDeepAgent({
799+
model: model as any,
800+
skills: ["/skills/"],
801+
checkpointer,
802+
});
803+
804+
await agent.invoke(
805+
{
806+
messages: [new HumanMessage("Hello")],
807+
files: {},
808+
} as any,
809+
{
810+
configurable: { thread_id: `test-empty-${Date.now()}` },
811+
recursionLimit: 50,
812+
},
813+
);
814+
815+
expect(invokeSpy).toHaveBeenCalled();
816+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
817+
818+
// Verify "no skills" message appears
819+
expect(systemPrompt).toContain("No skills available yet");
820+
expect(systemPrompt).toContain("/skills/");
821+
invokeSpy.mockRestore();
822+
});
823+
824+
it("should load skills from multiple sources via StateBackend", async () => {
825+
const userSkillMd = `---
826+
name: user-skill
827+
description: User-level skill for personal workflows
828+
---
829+
# User Skill`;
830+
831+
const projectSkillMd = `---
832+
name: project-skill
833+
description: Project-level skill for team collaboration
834+
---
835+
# Project Skill`;
836+
837+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
838+
const model = new FakeListChatModel({ responses: ["Done"] });
839+
840+
const checkpointer = new MemorySaver();
841+
const agent = createDeepAgent({
842+
model: model as any,
843+
skills: ["/skills/user/", "/skills/project/"],
844+
checkpointer,
845+
});
846+
847+
await agent.invoke(
848+
{
849+
messages: [new HumanMessage("List skills")],
850+
files: {
851+
"/skills/user/user-skill/SKILL.md": createFileData(userSkillMd),
852+
"/skills/project/project-skill/SKILL.md":
853+
createFileData(projectSkillMd),
854+
},
855+
} as any,
856+
{
857+
configurable: { thread_id: `test-sources-${Date.now()}` },
858+
recursionLimit: 50,
859+
},
860+
);
861+
862+
expect(invokeSpy).toHaveBeenCalled();
863+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
864+
865+
// Verify both sources' skills are present
866+
expect(systemPrompt).toContain("user-skill");
867+
expect(systemPrompt).toContain("project-skill");
868+
expect(systemPrompt).toContain("User-level skill");
869+
expect(systemPrompt).toContain("Project-level skill");
870+
invokeSpy.mockRestore();
871+
});
872+
873+
it("should include skill paths for progressive disclosure", async () => {
874+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
875+
const model = new FakeListChatModel({ responses: ["Done"] });
876+
877+
const checkpointer = new MemorySaver();
878+
const agent = createDeepAgent({
879+
model: model as any,
880+
skills: ["/skills/"],
881+
checkpointer,
882+
});
883+
884+
await agent.invoke(
885+
{
886+
messages: [new HumanMessage("What skills?")],
887+
files: {
888+
"/skills/test-skill/SKILL.md": createFileData(VALID_SKILL_MD),
889+
},
890+
} as any,
891+
{
892+
configurable: { thread_id: `test-paths-${Date.now()}` },
893+
recursionLimit: 50,
894+
},
895+
);
896+
897+
expect(invokeSpy).toHaveBeenCalled();
898+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
899+
900+
// Verify the full path is included for progressive disclosure
901+
expect(systemPrompt).toContain("/skills/test-skill/SKILL.md");
902+
// Verify progressive disclosure instructions are present
903+
expect(systemPrompt).toContain("Progressive Disclosure");
904+
invokeSpy.mockRestore();
905+
});
906+
907+
it("should handle empty skills directory gracefully", async () => {
908+
const invokeSpy = vi.spyOn(FakeListChatModel.prototype, "invoke");
909+
const model = new FakeListChatModel({ responses: ["Done"] });
910+
911+
const checkpointer = new MemorySaver();
912+
const agent = createDeepAgent({
913+
model: model as any,
914+
skills: ["/skills/empty/"],
915+
checkpointer,
916+
});
917+
918+
// Should not throw even when no skills exist (empty files)
919+
await expect(
920+
agent.invoke(
921+
{
922+
messages: [new HumanMessage("Hello")],
923+
files: {},
924+
} as any,
925+
{
926+
configurable: { thread_id: `test-empty-graceful-${Date.now()}` },
927+
recursionLimit: 50,
928+
},
929+
),
930+
).resolves.toBeDefined();
931+
932+
expect(invokeSpy).toHaveBeenCalled();
933+
const systemPrompt = getSystemPromptFromSpy(invokeSpy);
934+
935+
// Should still have a system prompt with the "no skills" message
936+
expect(systemPrompt).toContain("No skills available yet");
937+
invokeSpy.mockRestore();
938+
});
939+
});

libs/deepagents/src/middleware/skills.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import { StateSchema, ReducedValue } from "@langchain/langgraph";
5454
import type { BackendProtocol, BackendFactory } from "../backends/protocol.js";
5555
import type { StateBackend } from "../backends/state.js";
5656
import type { BaseStore } from "@langchain/langgraph-checkpoint";
57+
import { fileDataReducer, FileDataSchema } from "./fs.js";
5758

5859
// Security: Maximum size for SKILL.md files to prevent DoS attacks (10MB)
5960
export const MAX_SKILL_FILE_SIZE = 10 * 1024 * 1024;
@@ -170,6 +171,13 @@ const SkillsStateSchema = new StateSchema({
170171
reducer: skillsMetadataReducer,
171172
},
172173
),
174+
files: new ReducedValue(
175+
z.record(z.string(), FileDataSchema).default(() => ({})),
176+
{
177+
inputSchema: z.record(z.string(), FileDataSchema.nullable()).optional(),
178+
reducer: fileDataReducer,
179+
},
180+
),
173181
});
174182

175183
/**
@@ -513,7 +521,12 @@ export function createSkillsMiddleware(options: SkillsMiddlewareOptions) {
513521
if (loadedSkills.length > 0) {
514522
return undefined;
515523
}
516-
if ("skillsMetadata" in state && state.skillsMetadata != null) {
524+
// Check if skills were restored from checkpoint (non-empty array in state)
525+
if (
526+
"skillsMetadata" in state &&
527+
Array.isArray(state.skillsMetadata) &&
528+
state.skillsMetadata.length > 0
529+
) {
517530
// Restore from state (e.g., after checkpoint restore)
518531
loadedSkills = state.skillsMetadata as SkillMetadata[];
519532
return undefined;

0 commit comments

Comments
 (0)