Skills commands to wrap npm's skills

[christhompsongoogle]

No description provided.

[wiz-9635d3485b]

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-
Secrets	-
IaC Misconfigurations	-
SAST Findings	10
Software Management Findings	-
Total	10

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

[gemini-code-assist]

Code Review

This pull request introduces a new 'skills' command set to the CLI, including functionality to add, update, and delete skills. The implementation includes a manifest system for tracking local file hashes and transformations, as well as a registry for synchronizing with remote updates. Feedback focuses on improving type safety by removing 'any' casts, enhancing the robustness of CLI argument parsing for multiple flags, addressing potential regex injection vulnerabilities during template replacement, and optimizing I/O operations during bulk updates.

[gemini-code-assist]

There are two issues here:

1. Casting to any violates the repository style guide regarding type safety.
2. Using the raw path as the skillName is problematic when path is a local file path (e.g., ./myskill.ts). This leads to the manifest using the path as a key and installSkill generating filenames like .agent/skills/./myskill.ts.ts. Consider deriving a clean name from the path or requiring an alias for local files.

References

1. Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. ^(link)

[gemini-code-assist]

Using new RegExp with an unescaped variable (key) can lead to regex injection or unexpected behavior if the key contains special characters (e.g., ., *, or {). Since this is a simple template replacement, using split().join() is safer and more performant.

Suggested change

	content = content.replace(new RegExp(`{{${key}}}`, "g"), String(value));
	content = content.split(`{{${key}}}`).join(String(value));

[gemini-code-assist]

The current logic only handles a single --set flag if it is passed as a string. In many CLI configurations, providing multiple --set flags results in an array. This implementation would skip processing entirely if options.set is an array, preventing users from applying multiple transformations.

Suggested change

	if (typeof options.set === "string") {
	const [key, value] = options.set.split("=");
	if (key && value) {
	set[key] = value;
	}
	}
	const set: Record<string, string> = {};
	const setArgs = Array.isArray(options.set) ? options.set : (typeof options.set === "string" ? [options.set] : []);
	for (const s of setArgs) {
	const [key, value] = s.split("=");
	if (key && value) {
	set[key] = value;
	}
	}

[gemini-code-assist]

Calling installSkill inside a loop is inefficient because installSkill performs individual manifest loads and saves (I/O) for every item. For bulk updates, it is better to update the manifest in memory and perform a single save operation at the end of the process.

[gemini-code-assist]

Avoid using any as an escape hatch for transformation values. Define a more specific type (e.g., string | number | boolean) to adhere to the style guide and ensure type safety.

References

1. Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. ^(link)