Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/cli/commands/generate-skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ export const handleGenerateSkills = async (args: string[]): Promise<void> => {
// Get full schemas for each tool (already filtered by access control)
const allowedToolNames = new Set(tools.map((tool) => tool.name));
const schemas: SchemaOutput[] = discovery.schemas.filter((schema) => allowedToolNames.has(schema.tool));
const cachedSchemas = discovery.cachedSchemas.filter((schema) => allowedToolNames.has(schema.name));

// Group tools by prefix
const groups = detectPrefixGroups(schemas, serviceName);
Expand All @@ -222,8 +223,9 @@ export const handleGenerateSkills = async (args: string[]): Promise<void> => {
const descriptions = tools.map((t) => t.description);
const triggerKeywords = extractTriggerKeywords(serviceName, descriptions);

// Compute schema hash from tool names + descriptions for drift detection
const schemaHash = await computeSchemaHash(tools);
// Compute schema hash from the same full-description surface that `skills list`
// compares against. The rendered tool table may use truncated descriptions.
const schemaHash = await computeSchemaHash(cachedSchemas);

// H3: Only update generatedAt when the schema hash actually changes
const outputDir = resolveOutputDir(serviceName, outputFlag);
Expand Down
15 changes: 11 additions & 4 deletions src/cli/commands/skills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
import { loadConfig } from "../../config/index.ts";
import { listCachedServices, readCacheRaw } from "../../cache/index.ts";
import { extractPolicy, filterTools } from "../../access/filter.ts";
import { resolveOutputDir } from "../../generation/file-manager.ts";
import { computeSchemaHash } from "../../generation/skill-hash.ts";
import { validateIdentifier } from "../../validation/pipelines.ts";
Expand Down Expand Up @@ -103,10 +104,13 @@ async function handleSkillsList(args: string[]): Promise<void> {

if (!exists) {
const cached = cachedServices.includes(name) ? await readCacheRaw(name) : null;
const visibleCachedTools = cached
? filterTools(cached.tools, extractPolicy(config.services[name]!))
: null;
statuses.push({
service: name,
status: "missing",
cachedToolCount: cached?.tools.length,
cachedToolCount: visibleCachedTools?.length,
});
continue;
}
Expand All @@ -118,11 +122,14 @@ async function handleSkillsList(args: string[]): Promise<void> {
const existingHash = hashMatch?.[1];

const cached = cachedServices.includes(name) ? await readCacheRaw(name) : null;
const visibleCachedTools = cached
? filterTools(cached.tools, extractPolicy(config.services[name]!))
: null;
let isStale = false;
let toolCount: number | undefined;

if (cached) {
const cacheHash = await computeSchemaHash(cached.tools);
if (visibleCachedTools) {
const cacheHash = await computeSchemaHash(visibleCachedTools);
isStale = existingHash !== cacheHash;
}

Expand All @@ -134,7 +141,7 @@ async function handleSkillsList(args: string[]): Promise<void> {
service: name,
status: isStale ? "stale" : "generated",
toolCount,
cachedToolCount: cached?.tools.length,
cachedToolCount: visibleCachedTools?.length,
schemaHash: existingHash,
path: skillDir,
});
Expand Down
100 changes: 99 additions & 1 deletion src/generation/file-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,101 @@ export function resolveOutputDir(
/** Tolerant regex for AUTO-GENERATED markers */
const MARKER_START_RE = /<!--\s*AUTO-GENERATED:START\s*-->/;
const MARKER_END_RE = /<!--\s*AUTO-GENERATED:END\s*-->/;
const FRONTMATTER_RE = /^---\n[\s\S]*?\n---\n?/;

interface FrontmatterBlock {
key: string;
lines: string[];
}

function splitFrontmatterBlocks(frontmatter: string): FrontmatterBlock[] {
const body = frontmatter
.replace(/^---\n/, "")
.replace(/\n---\n?$/, "");
const blocks: FrontmatterBlock[] = [];

for (const line of body.split("\n")) {
const keyMatch = line.match(/^([A-Za-z0-9_-]+):/);
if (keyMatch) {
blocks.push({ key: keyMatch[1]!, lines: [line] });
} else if (blocks.length > 0) {
blocks[blocks.length - 1]!.lines.push(line);
}
}

return blocks;
}

function triggerValues(block: FrontmatterBlock | undefined): string[] {
if (!block) return [];

const firstLine = block.lines[0] ?? "";
const inlineArrayMatch = firstLine.match(/^[A-Za-z0-9_-]+:\s*\[(.*)\]\s*$/);
if (inlineArrayMatch) {
return inlineArrayMatch[1]!
.split(",")
.map((value) => value.trim().replace(/^['"]|['"]$/g, ""))
.filter(Boolean);
}

const inlineScalarMatch = firstLine.match(/^[A-Za-z0-9_-]+:\s*(\S.*)$/);
if (inlineScalarMatch) {
return [inlineScalarMatch[1]!.trim().replace(/^['"]|['"]$/g, "")].filter(Boolean);
}

return block.lines
.slice(1)
.map((line) => line.match(/^\s*-\s+(.+)\s*$/)?.[1])
.filter((value): value is string => Boolean(value));
}

function mergeFrontmatter(existingFrontmatter: string, generatedFrontmatter: string): string {
const existingBlocks = splitFrontmatterBlocks(existingFrontmatter);
const generatedBlocks = splitFrontmatterBlocks(generatedFrontmatter);
const generatedKeys = new Set(generatedBlocks.map((block) => block.key));
const existingByKey = new Map(existingBlocks.map((block) => [block.key, block]));
const merged: string[] = ["---"];

for (const generatedBlock of generatedBlocks) {
if (generatedBlock.key === "triggers") {
const values = [
...triggerValues(generatedBlock),
...triggerValues(existingByKey.get("triggers")),
];
const deduped = [...new Set(values)];
merged.push("triggers:");
for (const value of deduped) {
merged.push(` - ${value}`);
}
} else {
merged.push(...generatedBlock.lines);
}
}

for (const existingBlock of existingBlocks) {
if (!generatedKeys.has(existingBlock.key)) {
merged.push(...existingBlock.lines);
}
}

merged.push("---");
return merged.join("\n") + "\n";
}

function mergeGeneratedFrontmatter(existing: string, generated: string): string {
const generatedFrontmatter = generated.match(FRONTMATTER_RE)?.[0];
if (!generatedFrontmatter) {
return existing;
}

if (FRONTMATTER_RE.test(existing)) {
return existing.replace(FRONTMATTER_RE, (existingFrontmatter) =>
mergeFrontmatter(existingFrontmatter, generatedFrontmatter)
);
}

return generatedFrontmatter + existing;
}

/**
* Merge generated content into existing file content.
Expand Down Expand Up @@ -63,7 +158,10 @@ export function mergeContent(existing: string, generated: string): string {
}

// Replace between markers, preserving user content before and after
const beforeMarker = existing.slice(0, startMatch.index!);
const beforeMarker = mergeGeneratedFrontmatter(
existing.slice(0, startMatch.index!),
generated,
);
const afterMarker = existing.slice(endMatch.index! + endMatch[0]!.length);

return beforeMarker + autoContent + afterMarker;
Expand Down
51 changes: 51 additions & 0 deletions tests/cli/skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,57 @@ describe("skills list", () => {
}
});

test("uses access-filtered cache surface for stale detection", async () => {
const tools = [makeTool("allowed_tool", "Allowed"), makeTool("blocked_tool", "Blocked")];
await writeCache("test-svc", tools);

const { computeSchemaHash } = await import("../../src/generation/skill-hash.ts");
const expectedHash = await computeSchemaHash([tools[0]!]);

const skillContent = [
"---",
"name: test-svc",
"tool_count: 1",
"generated_at: 2025-01-01T00:00:00.000Z",
`schema_hash: ${expectedHash}`,
"triggers:",
" - test-svc",
"---",
"",
"# test-svc",
].join("\n");
await writeSkillFile("test-svc", skillContent);

const origConfig = process.env.MCP2CLI_CONFIG;
const configPath = join(testDir, "services.json");
await Bun.write(configPath, JSON.stringify({
services: {
"test-svc": {
backend: "stdio",
command: "echo",
args: [],
allowTools: ["allowed_*"],
},
},
}));
process.env.MCP2CLI_CONFIG = configPath;

try {
const { stdout } = await captureSkills(["list"]);
expect(stdout).toContain("test-svc");
expect(stdout).toContain("ok");
expect(stdout).toContain("1 tools");
expect(stdout).not.toContain("stale");
expect(stdout).not.toContain("cache has 2");
} finally {
if (origConfig !== undefined) {
process.env.MCP2CLI_CONFIG = origConfig;
} else {
delete process.env.MCP2CLI_CONFIG;
}
}
});

// L11: Test --json output
test("outputs JSON when --json flag is passed via args", async () => {
const origConfig = process.env.MCP2CLI_CONFIG;
Expand Down
36 changes: 36 additions & 0 deletions tests/generation/file-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,42 @@ describe("mergeContent", () => {
expect(result).toContain("Before");
expect(result).toContain("After");
});

test("preserves inline trigger array values when merging frontmatter", () => {
const existing = [
"---",
"name: old-skill",
"triggers: [custom-trigger, \"quoted trigger\"]",
"x-owner: human",
"---",
"# Custom header",
"<!-- AUTO-GENERATED:START -->",
"old",
"<!-- AUTO-GENERATED:END -->",
].join("\n");

const generated = [
"---",
"name: generated-skill",
"triggers:",
" - generated-trigger",
"schema_hash: abc123",
"---",
"<!-- AUTO-GENERATED:START -->",
"new",
"<!-- AUTO-GENERATED:END -->",
].join("\n");

const result = mergeContent(existing, generated);
expect(result).toContain("name: generated-skill");
expect(result).toContain("schema_hash: abc123");
expect(result).toContain(" - generated-trigger");
expect(result).toContain(" - custom-trigger");
expect(result).toContain(" - quoted trigger");
expect(result).toContain("x-owner: human");
expect(result).toContain("new");
expect(result).not.toContain("old");
});
});

// -- planFileWrites --
Expand Down
44 changes: 44 additions & 0 deletions tests/integration/generate-skills.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,50 @@ describe("generate-skills integration", () => {
expect(afterContent).toContain("AUTO-GENERATED:END");
}, 30_000);

test("conflict merge refreshes generated frontmatter metadata", async () => {
const { env, outputDir } = await setupTestEnv();

runCli(
["generate-skills", "mock-server", `--output=${outputDir}`],
env,
);

const skillPath = join(outputDir, "SKILL.md");
const original = await Bun.file(skillPath).text();
const oldFrontmatter = [
"---",
"name: mock-server",
"description: MCP tools for mock-server",
"triggers:",
" - mock-server",
" - custom-trigger",
"x-owner: hand-edited",
"---",
"",
].join("\n");
const withoutMetadata = original.replace(/^---\n[\s\S]*?\n---\n\n/, oldFrontmatter);
await Bun.write(
skillPath,
withoutMetadata + "\n## My Custom Notes\n\nThis should be preserved.\n",
);

const result = runCli(
["generate-skills", "mock-server", `--output=${outputDir}`, "--conflict=merge"],
env,
);

expect(result.exitCode).toBe(0);

const afterContent = await Bun.file(skillPath).text();
expect(afterContent).toContain("tool_count: 3");
expect(afterContent).toMatch(/^generated_at:\s*\S+/m);
expect(afterContent).toMatch(/^schema_hash:\s*[0-9a-f]{16}/m);
expect(afterContent).toContain(" - custom-trigger");
expect(afterContent).toContain("x-owner: hand-edited");
expect(afterContent).toContain("My Custom Notes");
expect(afterContent).toContain("This should be preserved.");
}, 30_000);

test("missing service arg returns structured validation error", async () => {
const { env } = await setupTestEnv();

Expand Down