From 419f8b4845ed576085e08794f5cc083b2f64e480 Mon Sep 17 00:00:00 2001 From: Austin Liu <193228693+Dingding-leo@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:00:18 +0930 Subject: [PATCH] Train 1D: merge via --admin on .113 validation Squash merge from local merge-train (Hard Rule owner-approved). Tip 029cdf4215cf465f0e1716ac9f84a84692b1e881 validated on 192.168.0.113: 26631/26653 pass. --- skills/cli-backup-sync/SKILL.md | 1 + src/lib/agentSkills/cliRegistryParser.ts | 103 +++++++++--------- .../agentSkills-cliRegistryParser.test.ts | 90 ++++++++++----- 3 files changed, 119 insertions(+), 75 deletions(-) diff --git a/skills/cli-backup-sync/SKILL.md b/skills/cli-backup-sync/SKILL.md index e1dccfed63..9dff9daba2 100644 --- a/skills/cli-backup-sync/SKILL.md +++ b/skills/cli-backup-sync/SKILL.md @@ -2,6 +2,7 @@ name: cli-backup-sync description: Backup and restore OmniRoute data from the CLI. Trigger incremental snapshots, sync to cloud storage, manage backup schedules, and restore from archive files. --- + ## Overview diff --git a/src/lib/agentSkills/cliRegistryParser.ts b/src/lib/agentSkills/cliRegistryParser.ts index 88c61fb5b8..d2a49148b3 100644 --- a/src/lib/agentSkills/cliRegistryParser.ts +++ b/src/lib/agentSkills/cliRegistryParser.ts @@ -42,55 +42,55 @@ export interface ParsedCliRegistry { * Files that don't map to a known family are ignored. */ const FILE_FAMILY_MAP: Record = { - "serve": "cli-serve", - "dashboard": "cli-serve", - "stop": "cli-serve", - "restart": "cli-serve", - "health": "cli-health", - "status": "cli-health", - "doctor": "cli-health", - "providers": "cli-providers", + serve: "cli-serve", + dashboard: "cli-serve", + stop: "cli-serve", + restart: "cli-serve", + health: "cli-health", + status: "cli-health", + doctor: "cli-health", + providers: "cli-providers", "provider-cmd": "cli-providers", "test-provider": "cli-providers", - "keys": "cli-keys", - "oauth": "cli-keys", - "models": "cli-models", - "chat": "cli-chat", - "stream": "cli-chat", - "repl": "cli-chat", - "combo": "cli-routing", - "routing": "cli-routing", - "resilience": "cli-resilience", - "quota": "cli-resilience", - "compression": "cli-compression", + keys: "cli-keys", + oauth: "cli-keys", + models: "cli-models", + chat: "cli-chat", + stream: "cli-chat", + repl: "cli-chat", + combo: "cli-routing", + routing: "cli-routing", + resilience: "cli-resilience", + quota: "cli-resilience", + compression: "cli-compression", "context-eng": "cli-contexts", - "contexts": "cli-contexts", - "sessions": "cli-contexts", - "cost": "cli-cost-usage", - "usage": "cli-cost-usage", - "pricing": "cli-cost-usage", - "mcp": "cli-mcp", - "a2a": "cli-a2a", - "tunnel": "cli-tunnel", - "backup": "cli-backup-sync", - "sync": "cli-backup-sync", - "cloud": "cli-backup-sync", - "audit": "cli-policy-audit", - "policy": "cli-policy-audit", - "logs": "cli-policy-audit", - "telemetry": "cli-policy-audit", - "batches": "cli-batches", - "files": "cli-batches", - "eval": "cli-eval", - "simulate": "cli-eval", - "skills": "cli-plugins-skills", - "plugin": "cli-plugins-skills", - "memory": "cli-plugins-skills", - "setup": "cli-setup", - "config": "cli-setup", - "env": "cli-setup", - "update": "cli-setup", - "autostart": "cli-setup", + contexts: "cli-contexts", + sessions: "cli-contexts", + cost: "cli-cost-usage", + usage: "cli-cost-usage", + pricing: "cli-cost-usage", + mcp: "cli-mcp", + a2a: "cli-a2a", + tunnel: "cli-tunnel", + backup: "cli-backup-sync", + sync: "cli-backup-sync", + cloud: "cli-backup-sync", + audit: "cli-policy-audit", + policy: "cli-policy-audit", + logs: "cli-policy-audit", + telemetry: "cli-policy-audit", + batches: "cli-batches", + files: "cli-batches", + eval: "cli-eval", + simulate: "cli-eval", + skills: "cli-plugins-skills", + plugin: "cli-plugins-skills", + memory: "cli-plugins-skills", + setup: "cli-setup", + config: "cli-setup", + env: "cli-setup", + update: "cli-setup", + autostart: "cli-setup", }; // ── Regex patterns ─────────────────────────────────────────────────────────── @@ -139,16 +139,21 @@ function extractCommandsFromContent(content: string, topLevelName: string): RawC // Slice between this command call and the next to scope description/options const slice = content.slice(cmdIndex, nextIndex); + // If the slice itself contains a nested subcommand definition (e.g., `const auto = backup.command("auto")`), + // restrict slice to end before the child `.command()` call so child options aren't attributed to parent. + const subCmdMatch = /[\s\S]+?(?=\b[a-zA-Z0-9_$]+\.command\()/g.exec(slice); + const effectiveSlice = subCmdMatch ? subCmdMatch[0] : slice; + // Extract description (first match in slice) DESCRIPTION_RE.lastIndex = 0; - const descMatch = DESCRIPTION_RE.exec(slice); + const descMatch = DESCRIPTION_RE.exec(effectiveSlice); const description = descMatch ? descMatch[1] : ""; // Extract flags in slice const flags: string[] = []; OPTION_RE.lastIndex = 0; let optMatch: RegExpExecArray | null; - while ((optMatch = OPTION_RE.exec(slice)) !== null) { + while ((optMatch = OPTION_RE.exec(effectiveSlice)) !== null) { flags.push(optMatch[1]); } @@ -187,7 +192,7 @@ export function parseCliRegistry(): ParsedCliRegistry { } catch (err) { throw new Error( `cliRegistryParser: could not read ${commandsDir}. ` + - `Run from project root. Underlying error: ${err instanceof Error ? err.message : String(err)}`, + `Run from project root. Underlying error: ${err instanceof Error ? err.message : String(err)}` ); } diff --git a/tests/unit/agentSkills-cliRegistryParser.test.ts b/tests/unit/agentSkills-cliRegistryParser.test.ts index 086181494f..0e18080e70 100644 --- a/tests/unit/agentSkills-cliRegistryParser.test.ts +++ b/tests/unit/agentSkills-cliRegistryParser.test.ts @@ -5,9 +5,8 @@ import path from "node:path"; import os from "node:os"; // Dynamic import to pick up ESM module -const { parseCliRegistry, getCommandsForFamily } = await import( - "../../src/lib/agentSkills/cliRegistryParser.ts" -); +const { parseCliRegistry, getCommandsForFamily } = + await import("../../src/lib/agentSkills/cliRegistryParser.ts"); // ─── Fixture helpers ────────────────────────────────────────────────────────── @@ -15,9 +14,7 @@ const { parseCliRegistry, getCommandsForFamily } = await import( * Creates a temporary directory mirroring bin/cli/commands/, * writes fixture .mjs files, changes CWD, returns cleanup fn. */ -function withFixtureCli( - files: Record, -): { cleanup: () => void } { +function withFixtureCli(files: Record): { cleanup: () => void } { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-cli-test-")); const commandsDir = path.join(tmpDir, "bin", "cli", "commands"); fs.mkdirSync(commandsDir, { recursive: true }); @@ -159,7 +156,7 @@ test("parseCliRegistry() recognises providers family with ≥5 subcommands", () assert.ok(providerCmds, "Expected 'cli-providers' family to exist"); assert.ok( providerCmds!.length >= 5, - `Expected ≥5 provider commands, got ${providerCmds!.length}: ${providerCmds!.map((c) => c.name).join(", ")}`, + `Expected ≥5 provider commands, got ${providerCmds!.length}: ${providerCmds!.map((c) => c.name).join(", ")}` ); } finally { cleanup(); @@ -174,10 +171,7 @@ test("parseCliRegistry() recognises health family commands", () => { const { families } = parseCliRegistry(); const healthCmds = families.get("cli-health"); assert.ok(healthCmds, "Expected 'cli-health' family to exist"); - assert.ok( - healthCmds!.length >= 2, - `Expected ≥2 health commands, got ${healthCmds!.length}`, - ); + assert.ok(healthCmds!.length >= 2, `Expected ≥2 health commands, got ${healthCmds!.length}`); } finally { cleanup(); } @@ -195,7 +189,7 @@ test("parseCliRegistry() extracts description for each command", () => { assert.ok(providers, "Expected providers command"); assert.ok( providers!.description.length > 0, - `Expected non-empty description for providers, got: "${providers!.description}"`, + `Expected non-empty description for providers, got: "${providers!.description}"` ); } finally { cleanup(); @@ -213,7 +207,7 @@ test("parseCliRegistry() marks subcommands with isSubcommand=true (after first)" const subCmds = providerCmds.filter((c) => c.isSubcommand); assert.ok( subCmds.length >= 4, - `Expected ≥4 subcommands (list, available, test, etc.), got ${subCmds.length}`, + `Expected ≥4 subcommands (list, available, test, etc.), got ${subCmds.length}` ); } finally { cleanup(); @@ -232,10 +226,7 @@ test("parseCliRegistry() extracts flags from .option() calls", () => { if (rotate) { // If rotate exists and has flags, verify format for (const flag of rotate.flags) { - assert.ok( - typeof flag === "string" && flag.length > 0, - `Invalid flag: "${flag}"`, - ); + assert.ok(typeof flag === "string" && flag.length > 0, `Invalid flag: "${flag}"`); } } } finally { @@ -243,6 +234,58 @@ test("parseCliRegistry() extracts flags from .option() calls", () => { } }); +test("parseCliRegistry() does not mis-attribute child options to parent command when subcommand variable is created inline", () => { + const fixture = ` +export function registerBackup(program) { + const backup = program.command("backup").description("Manage backups"); + + backup + .command("create") + .description("Create a backup") + .option("--name ", "Name") + .option("--cloud", "Cloud backup"); + + const auto = backup.command("auto").description("Auto backup"); + + auto + .command("enable") + .description("Enable auto backup") + .option("--cron ", "Cron schedule"); + + backup + .command("status") + .description("Show status"); +} +`; + const { cleanup } = withFixtureCli({ + "backup.mjs": fixture, + }); + try { + const { commands } = parseCliRegistry(); + const backupCmd = commands.get("backup"); + assert.ok(backupCmd, "backup command should exist"); + assert.deepEqual( + backupCmd.flags, + [], + "parent backup command should not collect flags from nested subcommands" + ); + + const statusCmd = commands.get("backup status"); + assert.ok(statusCmd, "backup status command should exist"); + assert.deepEqual( + statusCmd.flags, + [], + "backup status should not collect flags from other subcommands" + ); + + const createCmd = commands.get("backup create"); + assert.ok(createCmd, "backup create command should exist"); + assert.deepEqual(createCmd.flags, ["--name ", "--cloud"]); + } finally { + cleanup(); + } +}); + test("parseCliRegistry() skips unrecognised .mjs files", () => { const { cleanup } = withFixtureCli({ "unknown-custom.mjs": `export function register(p) {}`, @@ -251,9 +294,7 @@ test("parseCliRegistry() skips unrecognised .mjs files", () => { try { const { families } = parseCliRegistry(); // No family should be mapped from unknown-custom - const hasUnknown = [...families.keys()].some((k) => - String(k).includes("unknown-custom"), - ); + const hasUnknown = [...families.keys()].some((k) => String(k).includes("unknown-custom")); assert.equal(hasUnknown, false, "unknown-custom.mjs should not create a family"); // providers.mjs should still be parsed assert.ok(families.has("cli-providers"), "Expected cli-providers family from providers.mjs"); @@ -270,7 +311,7 @@ test("parseCliRegistry() throws if commands directory is missing", () => { assert.throws( () => parseCliRegistry(), /cliRegistryParser: could not read/, - "Expected error when commands dir is missing", + "Expected error when commands dir is missing" ); } finally { process.chdir(originalCwd); @@ -288,16 +329,13 @@ test("parseCliRegistry() with real providers.mjs: providers family has ≥5 comm assert.ok(providerCmds, "Expected cli-providers family from real providers.mjs"); assert.ok( providerCmds!.length >= 5, - `Expected ≥5 real provider commands, got ${providerCmds!.length}: ${providerCmds!.map((c) => c.name).join(", ")}`, + `Expected ≥5 real provider commands, got ${providerCmds!.length}: ${providerCmds!.map((c) => c.name).join(", ")}` ); }); test("getCommandsForFamily('cli-providers') with real files: returns ≥5 strings", () => { const commands = getCommandsForFamily("cli-providers"); - assert.ok( - commands.length >= 5, - `Expected ≥5 cli-providers commands, got ${commands.length}`, - ); + assert.ok(commands.length >= 5, `Expected ≥5 cli-providers commands, got ${commands.length}`); for (const cmd of commands) { assert.ok(typeof cmd === "string" && cmd.length > 0, `Invalid command name: "${cmd}"`); }