Skip to content

chore(check): enforce formatting and clippy gates #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
samzong merged 1 commit into from
Jun 30, 2026
Merged

chore(check): enforce formatting and clippy gates #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test-rust: ## Run Rust SDK tests
fmt: fmt-ts fmt-go fmt-rust ## Format all SDK code

fmt-ts: ## Format TypeScript code
cd $(TS_DIR) && pnpm exec prettier --write src test ../examples/ts/cli.ts ../scripts/prepare-release.mjs
cd $(TS_DIR) && pnpm exec prettier --write src test ../examples/ts/cli.ts ../scripts/check.mjs ../scripts/prepare-release.mjs

fmt-go: ## Format Go code
gofmt -w $(GO_FILES)
Expand Down
28 changes: 7 additions & 21 deletions rust/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,16 +680,7 @@ pub fn run_bundled_skill_install_with_io<R: BufRead, W: Write>(
input: &mut R,
output: &mut W,
) -> io::Result<InstallWorkflowReport> {
let (scope, scope_error) = resolve_workflow_scope(
input,
output,
options.install.scope,
options.scope_set,
options.prompt_scope,
options.default_scope,
options.yes,
options.stdin_tty,
)?;
let (scope, scope_error) = resolve_workflow_scope(input, output, options)?;
if let Some(selection) = scope_error {
render_selection_errors(output, &selection)?;
let empty = install_report(vec![]);
Expand Down Expand Up @@ -1034,21 +1025,16 @@ fn has_install_writes(report: &Value) -> bool {
fn resolve_workflow_scope<R: BufRead, W: Write>(
input: &mut R,
output: &mut W,
requested: Scope,
scope_set: bool,
prompt_scope: bool,
default_scope: Option<Scope>,
yes: bool,
stdin_tty: bool,
options: &InstallWorkflowOptions,
) -> io::Result<(Option<Scope>, Option<InstallSelection>)> {
let default_scope = default_scope.unwrap_or(Scope::User);
if scope_set || !prompt_scope {
return Ok((Some(requested), None));
let default_scope = options.default_scope.unwrap_or(Scope::User);
if options.scope_set || !options.prompt_scope {
return Ok((Some(options.install.scope), None));
}
if yes {
if options.yes {
return Ok((Some(default_scope), None));
}
if !stdin_tty {
if !options.stdin_tty {
return Ok((
None,
Some(error_selection(
Expand Down
117 changes: 94 additions & 23 deletions scripts/check.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@ function validateHosts(spec) {
const idPattern = /^[a-z0-9]+(-[a-z0-9]+)*$/;
const projectPattern = /^(?!\/)(?!~)(?!.*(^|\/)\.\.(\/|$))[^\0]+$/;
const homePattern = /^~\/[^\0]+$/;
const statuses = new Set(["verified", "documented", "community", "experimental"]);
const statuses = new Set([
"verified",
"documented",
"community",
"experimental",
]);
const ids = new Set();
const aliases = new Set();

Expand All @@ -36,29 +41,44 @@ function validateHosts(spec) {
ids.add(host.id);
assert(host.displayName, `missing displayName: ${host.id}`);

assert(Array.isArray(host.projectSkillsDirs), `projectSkillsDirs must be an array: ${host.id}`);
assert(Array.isArray(host.userSkillsDirs), `userSkillsDirs must be an array: ${host.id}`);
assert(
Array.isArray(host.projectSkillsDirs),
`projectSkillsDirs must be an array: ${host.id}`,
);
assert(
Array.isArray(host.userSkillsDirs),
`userSkillsDirs must be an array: ${host.id}`,
);
assert(
host.projectSkillsDirs.length + host.userSkillsDirs.length > 0,
`host needs at least one install path: ${host.id}`
`host needs at least one install path: ${host.id}`,
);

for (const path of host.projectSkillsDirs) {
assert(projectPattern.test(path), `bad project path for ${host.id}: ${path}`);
assert(
projectPattern.test(path),
`bad project path for ${host.id}: ${path}`,
);
}
for (const path of host.userSkillsDirs) {
assert(homePattern.test(path), `bad user path for ${host.id}: ${path}`);
}

assert(Array.isArray(host.detect) && host.detect.length > 0, `missing detect paths: ${host.id}`);
assert(
Array.isArray(host.detect) && host.detect.length > 0,
`missing detect paths: ${host.id}`,
);
for (const path of host.detect) {
assert(
homePattern.test(path) || projectPattern.test(path),
`bad detect path for ${host.id}: ${path}`
`bad detect path for ${host.id}: ${path}`,
);
}

assert(statuses.has(host.status), `bad status for ${host.id}: ${host.status}`);
assert(
statuses.has(host.status),
`bad status for ${host.id}: ${host.status}`,
);

for (const alias of host.aliases || []) {
assert(idPattern.test(alias), `bad alias for ${host.id}: ${alias}`);
Expand All @@ -81,12 +101,18 @@ function validateCases(cases, hosts) {
caseIds.add(testCase.id);
}

const allHostsCase = cases.cases.find((testCase) => testCase.id === "all-supported-hosts-load");
const allHostsCase = cases.cases.find(
(testCase) => testCase.id === "all-supported-hosts-load",
);
assert(allHostsCase, "missing all-supported-hosts-load case");
assert(allHostsCase.expected.count === hosts.length, "all-supported-hosts-load count drifted");
assert(
JSON.stringify(allHostsCase.expected.hostIds) === JSON.stringify(hosts.map((host) => host.id)),
"all-supported-hosts-load hostIds drifted"
allHostsCase.expected.count === hosts.length,
"all-supported-hosts-load count drifted",
);
assert(
JSON.stringify(allHostsCase.expected.hostIds) ===
JSON.stringify(hosts.map((host) => host.id)),
"all-supported-hosts-load hostIds drifted",
);

for (const id of [
Expand Down Expand Up @@ -133,17 +159,26 @@ function validateCases(cases, hosts) {
"workflow-yes-batch-installs-detected",
"workflow-many-detected-yes-installs",
"workflow-non-tty-no-agent-error",
"workflow-zero-detected-yes-error"
"workflow-zero-detected-yes-error",
]) {
assert(caseIds.has(id), `missing golden case: ${id}`);
}
}

function validateFixtures() {
const skill = readFileSync(new URL("testdata/skills/basic/SKILL.md", root), "utf8");
assert(/^---\n[\s\S]*?\n---\n/.test(skill), "basic SKILL.md missing frontmatter");
const skill = readFileSync(
new URL("testdata/skills/basic/SKILL.md", root),
"utf8",
);
assert(
/^---\n[\s\S]*?\n---\n/.test(skill),
"basic SKILL.md missing frontmatter",
);
assert(/^name: basic$/m.test(skill), "basic skill name mismatch");
assert(/^description: .{1,1024}$/m.test(skill), "basic skill description missing");
assert(
/^description: .{1,1024}$/m.test(skill),
"basic skill description missing",
);
readJson("testdata/skills/basic/assets/template.json");
}

Expand All @@ -168,43 +203,79 @@ function detectedEnv(prefix) {
return {
CARGO_HOME: process.env.CARGO_HOME ?? `${process.env.HOME}/.cargo`,
HOME: home,
RUSTUP_HOME: process.env.RUSTUP_HOME ?? `${process.env.HOME}/.rustup`
RUSTUP_HOME: process.env.RUSTUP_HOME ?? `${process.env.HOME}/.rustup`,
};
}

for (const [name, command, args, cwd, env] of [
["generated-hosts", "node", ["scripts/sync-hosts.mjs", "--check"], rootPath],
[
"typescript-format",
"pnpm",
[
"--dir",
"ts",
"exec",
"prettier",
"--check",
"src",
"test",
"../examples/ts/cli.ts",
"../scripts/check.mjs",
"../scripts/prepare-release.mjs",
],
rootPath,
],
["typescript", "pnpm", ["--dir", "ts", "test"], rootPath],
["go", "go", ["test", "./..."], new URL("../go/", import.meta.url)],
["go-cobra", "go", ["test", "./..."], new URL("../go-cobra/", import.meta.url)],
[
"go-cobra",
"go",
["test", "./..."],
new URL("../go-cobra/", import.meta.url),
],
["rust", "cargo", ["test"], new URL("../rust/", import.meta.url)],
[
"rust-clippy",
"cargo",
[
"clippy",
"--manifest-path",
"rust/Cargo.toml",
"--all-targets",
"--",
"-D",
"warnings",
],
rootPath,
],
[
"example-ts",
"pnpm",
["--dir", "examples/ts", "install-skill"],
rootPath,
detectedEnv("kitup-example-ts-")
detectedEnv("kitup-example-ts-"),
],
[
"example-go",
"go",
["run", "."],
new URL("../examples/go/", import.meta.url),
detectedEnv("kitup-example-go-")
detectedEnv("kitup-example-go-"),
],
[
"example-rust",
"cargo",
["run", "--quiet"],
new URL("../examples/rust/", import.meta.url),
detectedEnv("kitup-example-rust-")
]
detectedEnv("kitup-example-rust-"),
],
]) {
console.log(`\n==> ${name}`);
const result = spawnSync(command, args, {
cwd,
env: { ...process.env, ...env },
stdio: "inherit"
stdio: "inherit",
});
if (result.error) throw result.error;
if (result.status !== 0) process.exit(result.status ?? 1);
Expand Down
54 changes: 39 additions & 15 deletions ts/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,7 @@ export interface InstallWorkflowReport {
}

export type InstallWorkflowExitCode =
| "ok"
| "canceled"
| "selection-error"
| "conflict"
| "error";
"ok" | "canceled" | "selection-error" | "conflict" | "error";
Comment thread
samzong marked this conversation as resolved.

export interface InstallWorkflowExit {
ok: boolean;
Expand Down Expand Up @@ -246,13 +242,15 @@ export function filesBundle(files: SkillFile[]): SkillBundle {
return { kind: "files", files };
}

export function parseInstallFlags(flags: InstallFlagValues): ParsedInstallFlags {
export function parseInstallFlags(
flags: InstallFlagValues,
): ParsedInstallFlags {
const errors: InstallFlagError[] = [];
const scope = parseScopeFlag(flags.scope, errors);
const agents = agentSelectorFromFlags(flags.agents ?? [], errors);
return {
scope,
scopeSet: flags.scopeSet ?? (flags.scope !== undefined),
scopeSet: flags.scopeSet ?? flags.scope !== undefined,
agents,
yes: Boolean(flags.yes),
dryRun: Boolean(flags.dryRun),
Expand Down Expand Up @@ -322,11 +320,17 @@ export function installWorkflowError(
workflow: InstallWorkflowReport,
): Error | undefined {
const exit = classifyInstallWorkflowExit(workflow);
return exit.ok || exit.code === "canceled" ? undefined : new Error(exit.message);
return exit.ok || exit.code === "canceled"
? undefined
: new Error(exit.message);
}

export function installFlagError(errors: InstallFlagError[]): Error | undefined {
return errors.length === 0 ? undefined : new Error(installUxText.invalidFlags);
export function installFlagError(
errors: InstallFlagError[],
): Error | undefined {
return errors.length === 0
? undefined
: new Error(installUxText.invalidFlags);
}

export async function loadHostSpec(hostsFile?: string): Promise<HostSpec> {
Expand Down Expand Up @@ -482,7 +486,7 @@ export async function runBundledSkillInstall(
reader,
output,
options.scope,
options.scopeSet ?? (options.scope !== undefined),
options.scopeSet ?? options.scope !== undefined,
Boolean(options.promptScope),
options.defaultScope,
Boolean(options.yes),
Expand Down Expand Up @@ -564,11 +568,25 @@ export async function runBundledSkillInstall(
renderInstallSummary(output, plan);

if (options.dryRun) {
return { selection, scope, plan, report: plan, canceled: false, dryRun: true };
return {
selection,
scope,
plan,
report: plan,
canceled: false,
dryRun: true,
};
}

if (!hasInstallWrites(plan)) {
return { selection, scope, plan, report: plan, canceled: false, dryRun: false };
return {
selection,
scope,
plan,
report: plan,
canceled: false,
dryRun: false,
};
}

if (selection.needsConfirmation) {
Expand Down Expand Up @@ -1097,13 +1115,19 @@ async function promptScopeSelection(
writeLine(output, " 1. user");
writeLine(output, " 2. project");
output.write(`${installUxText.scopePrompt} [${defaultScope}]: `);
const selected = parseScopeSelection((await reader.readLine()) ?? "", defaultScope);
const selected = parseScopeSelection(
(await reader.readLine()) ?? "",
defaultScope,
);
if (selected) return selected;
writeLine(output, installUxText.invalidScopeSelection);
}
}

function parseScopeSelection(line: string, defaultScope: Scope): Scope | undefined {
function parseScopeSelection(
line: string,
defaultScope: Scope,
): Scope | undefined {
switch (line.trim().toLowerCase()) {
case "":
return defaultScope;
Expand Down
Loading