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
33 changes: 31 additions & 2 deletions shared/glean/mcp/src/skill-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,33 @@ function parseFrontmatter(content: string): Record<string, string> {
return result;
}

/**
* Keep approval preferences out of the local skill cache. The remote
* get_tool_approval lookup is the source of current preferences, so a stale or
* hand-edited skill file must not retain a second approval setting. Other tool
* metadata remains cached: inputSchema for argument shaping, and downstream
* annotations so read-only tools skip the approval lookup and HITL entirely.
*/
function sanitizeSkillFile(filePath: string, text: string): string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we okay doing this? would this require a network call always now?
Read tools are always allow anyway right, can we use that information to optimize network call count from the client?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed that we should optimize for read tools. Fixed, now we retain read-only annotations and use them to skip approval checks entirely. Approval preferences are still excluded from the cache, so write tools use current remote settings.

if (!/^tools[\\/]\S+\.json$/.test(filePath)) return text;

try {
const parsed = JSON.parse(text) as unknown;
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
return text;
}
const { requires_approval: _ignored, ...metadata } = parsed as Record<
string,
unknown
>;
return JSON.stringify(metadata);
} catch {
// Leave malformed/non-object tool files alone; run_tool will not use them as
// approval state, and preserving the original content keeps diagnostics intact.
return text;
}
}

type LogFn = (label: string, detail?: Record<string, unknown>) => void;

/**
Expand Down Expand Up @@ -99,8 +126,10 @@ export async function writeSkillsToDisk(
continue;
}
await fs.mkdir(path.dirname(fullPath), { recursive: true });
const text =
typeof content === "string" ? content : JSON.stringify(content);
const text = sanitizeSkillFile(
filePath,
typeof content === "string" ? content : JSON.stringify(content),
);
await fs.writeFile(fullPath, text, "utf-8");
writtenFiles.push(fullPath);
}
Expand Down
Loading
Loading