From 4d8128ae4826cbfcfc9255aec6b5d01f8bcd2f6e Mon Sep 17 00:00:00 2001 From: Kowser Date: Tue, 4 Aug 2026 19:40:47 -0700 Subject: [PATCH 1/5] chore: delete skill command layer - rm cmd/skill.go, skill_register.go, skill_load.go, skill_run.go, skill_payload.go, skill_materialize.go - rm matching _test.go files --- cmd/skill.go | 287 ------------------- cmd/skill_load.go | 95 ------- cmd/skill_materialize.go | 150 ---------- cmd/skill_payload.go | 546 ------------------------------------- cmd/skill_payload_test.go | 234 ---------------- cmd/skill_register.go | 391 -------------------------- cmd/skill_register_test.go | 123 --------- cmd/skill_run.go | 304 --------------------- cmd/skill_run_test.go | 247 ----------------- 9 files changed, 2377 deletions(-) delete mode 100644 cmd/skill.go delete mode 100644 cmd/skill_load.go delete mode 100644 cmd/skill_materialize.go delete mode 100644 cmd/skill_payload.go delete mode 100644 cmd/skill_payload_test.go delete mode 100644 cmd/skill_register.go delete mode 100644 cmd/skill_register_test.go delete mode 100644 cmd/skill_run.go delete mode 100644 cmd/skill_run_test.go diff --git a/cmd/skill.go b/cmd/skill.go deleted file mode 100644 index b05fc5c..0000000 --- a/cmd/skill.go +++ /dev/null @@ -1,287 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "archive/zip" - "bytes" - "encoding/json" - "fmt" - "io" - "os" - "path/filepath" - "strconv" - "strings" - "text/tabwriter" - - "github.com/spf13/cobra" - - "github.com/conductor-oss/conductor-cli/internal" - "github.com/conductor-oss/conductor-cli/internal/skill" -) - -const skillVersionDisplayLen = 12 - -var skillCmd = &cobra.Command{ - Use: "skill", - Short: "Manage skills", - GroupID: "conductor", -} - -// ---- skill list ---- - -var skillAllVersions bool - -var skillListCmd = &cobra.Command{ - Use: "list", - Short: "List registered skills", - Args: cobra.NoArgs, - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - format, err := GetOutputFormat(cmd) - if err != nil { - return err - } - skills, err := internal.GetSkillService().List(cmd.Context(), skillAllVersions) - if err != nil { - return err - } - return renderSkillList(skills, format) - }, -} - -func renderSkillList(skills []skill.Summary, format OutputFormat) error { - switch format { - case OutputFormatJSON: - data, err := json.MarshalIndent(skills, "", " ") - if err != nil { - return err - } - fmt.Println(string(data)) - case OutputFormatCSV: - w := NewCSVWriter() - w.WriteHeader("NAME", "VERSION", "FILES", "AGENTS", "SCRIPTS", "RESOURCES", "DESCRIPTION") - for _, s := range skills { - w.WriteRow(s.Name, shortVersion(s.Version), strconv.Itoa(s.FileCount), - strconv.Itoa(s.SubAgentCount), strconv.Itoa(s.ScriptCount), strconv.Itoa(s.ResourceCount), s.Description) - } - w.Flush() - default: - if len(skills) == 0 { - fmt.Println("No skills registered.") - return nil - } - w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0) - fmt.Fprintln(w, "NAME\tVERSION\tFILES\tAGENTS\tSCRIPTS\tRESOURCES\tDESCRIPTION") - for _, s := range skills { - fmt.Fprintf(w, "%s\t%s\t%d\t%d\t%d\t%d\t%s\n", - s.Name, shortVersion(s.Version), s.FileCount, s.SubAgentCount, s.ScriptCount, s.ResourceCount, s.Description) - } - w.Flush() - } - return nil -} - -// ---- skill get ---- - -var skillGetVersion string - -var skillGetCmd = &cobra.Command{ - Use: "get [version]", - Short: "Get a registered skill", - Args: cobra.RangeArgs(1, 2), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - detail, err := internal.GetSkillService().Get(cmd.Context(), args[0], versionArg(args, skillGetVersion)) - if err != nil { - return err - } - data, err := json.MarshalIndent(detail, "", " ") - if err != nil { - return err - } - fmt.Println(string(data)) - return nil - }, -} - -// ---- skill pull ---- - -var skillPullVersion string - -var skillPullCmd = &cobra.Command{ - Use: "pull [destination]", - Short: "Download and extract a skill package", - Args: cobra.RangeArgs(1, 2), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - name := args[0] - dest := name - if len(args) > 1 { - dest = args[1] - } - svc := internal.GetSkillService() - detail, err := svc.Get(cmd.Context(), name, skillPullVersion) - if err != nil { - return err - } - data, err := svc.DownloadPackage(cmd.Context(), name, detail.Version) - if err != nil { - return err - } - if err := extractSkillPackage(data, dest); err != nil { - return err - } - fmt.Printf("Skill %s@%s pulled to %s.\n", detail.Name, detail.Version, dest) - return nil - }, -} - -// ---- skill delete ---- - -var skillDeleteVersion string - -var skillDeleteCmd = &cobra.Command{ - Use: "delete [version]", - Short: "Delete a registered skill version", - Args: cobra.RangeArgs(1, 2), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - name := args[0] - version := versionArg(args, skillDeleteVersion) - label := name - if version != "" { - label = name + "@" + version - } - if !confirmDeletion("skill", label) { - fmt.Println("Aborted.") - return nil - } - if err := internal.GetSkillService().Delete(cmd.Context(), name, version); err != nil { - return err - } - fmt.Printf("Deleted skill %s.\n", label) - return nil - }, -} - -// ---- helpers (file I/O lives in the cmd layer) ---- - -// versionArg resolves the skill version from the optional positional arg or the -// --version flag (positional wins, matching the AgentSpan CLI). -func versionArg(args []string, flagVersion string) string { - if len(args) > 1 { - return args[1] - } - return flagVersion -} - -func shortVersion(version string) string { - if len(version) > skillVersionDisplayLen { - return version[:skillVersionDisplayLen] - } - return version -} - -// extractSkillPackage unzips a skill package into dest. Destination must be empty or -// absent; entries are written through safePackagePath to prevent zip-slip. -func extractSkillPackage(data []byte, dest string) error { - targetRoot, err := filepath.Abs(expandUserPath(dest)) - if err != nil { - return fmt.Errorf("resolve destination: %w", err) - } - if entries, err := os.ReadDir(targetRoot); err == nil && len(entries) > 0 { - return fmt.Errorf("destination %q already exists and is not empty", targetRoot) - } - if err := os.MkdirAll(targetRoot, 0o755); err != nil { - return fmt.Errorf("create destination: %w", err) - } - - reader, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) - if err != nil { - return fmt.Errorf("open skill package: %w", err) - } - for _, file := range reader.File { - if file.FileInfo().IsDir() { - continue - } - target, err := safePackagePath(targetRoot, file.Name) - if err != nil { - return err - } - if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { - return fmt.Errorf("create directory for %s: %w", file.Name, err) - } - if err := writeZipEntry(file, target); err != nil { - return err - } - } - return nil -} - -func writeZipEntry(file *zip.File, target string) error { - rc, err := file.Open() - if err != nil { - return fmt.Errorf("open package entry %s: %w", file.Name, err) - } - defer rc.Close() - out, err := os.OpenFile(target, os.O_CREATE|os.O_EXCL|os.O_WRONLY, file.Mode().Perm()) - if err != nil { - return fmt.Errorf("create %s: %w", target, err) - } - if _, err := io.Copy(out, rc); err != nil { - out.Close() - return fmt.Errorf("extract %s: %w", file.Name, err) - } - return out.Close() -} - -// safePackagePath joins relPath onto root, rejecting paths that escape root (zip-slip). -func safePackagePath(root, relPath string) (string, error) { - cleanRel := filepath.Clean(filepath.FromSlash(relPath)) - if filepath.IsAbs(cleanRel) || cleanRel == ".." || strings.HasPrefix(cleanRel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("package path %q is outside the skill directory", relPath) - } - target := filepath.Join(root, cleanRel) - if rel, err := filepath.Rel(root, target); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("package path %q is outside the skill directory", relPath) - } - return target, nil -} - -func expandUserPath(path string) string { - if path == "~" || strings.HasPrefix(path, "~/") { - if home, err := os.UserHomeDir(); err == nil { - return filepath.Join(home, strings.TrimPrefix(path, "~")) - } - } - return path -} - -func init() { - skillListCmd.Flags().BoolVar(&skillAllVersions, "all-versions", false, "List all versions instead of only the latest") - AddOutputFlags(skillListCmd) - - skillGetCmd.Flags().StringVar(&skillGetVersion, "version", "", "Skill version or checksum prefix") - skillPullCmd.Flags().StringVar(&skillPullVersion, "version", "", "Skill version or checksum prefix") - skillDeleteCmd.Flags().StringVar(&skillDeleteVersion, "version", "", "Skill version or checksum prefix") - - skillCmd.AddCommand( - skillListCmd, - skillGetCmd, - skillPullCmd, - skillDeleteCmd, - ) - rootCmd.AddCommand(skillCmd) -} diff --git a/cmd/skill_load.go b/cmd/skill_load.go deleted file mode 100644 index c307b3a..0000000 --- a/cmd/skill_load.go +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "encoding/json" - "fmt" - - "github.com/spf13/cobra" - - "github.com/conductor-oss/conductor-cli/internal" - "github.com/conductor-oss/conductor-cli/internal/agent" -) - -var ( - skillLoadModel string - skillLoadAgentModels []string - skillLoadSearchPaths []string -) - -var skillLoadCmd = &cobra.Command{ - Use: "load ", - Short: "Package a local skill and deploy it as an agent", - Long: `Read a local skill directory, package its contents into a skill agent -definition, and deploy it on the server for later execution via -'conductor agent run --name '. Unlike 'skill run', load only publishes the -agent; it does not start an execution.`, - Args: cobra.ExactArgs(1), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - if skillLoadModel == "" { - return fmt.Errorf("--model is required for skill load") - } - agentModels, err := parseAgentModelFlags(skillLoadAgentModels) - if err != nil { - return err - } - - cfg, local, err := BuildSkillPayload(args[0], PayloadOptions{ - Model: skillLoadModel, - AgentModels: agentModels, - SearchPaths: skillLoadSearchPaths, - }) - if err != nil { - return err - } - - raw, err := json.Marshal(cfg) - if err != nil { - return fmt.Errorf("encode skill config: %w", err) - } - - result, err := internal.GetAgentService().Deploy(cmd.Context(), frameworkSkill, raw) - if err != nil { - return err - } - return renderDeployResult(local.SkillName, result) - }, -} - -// renderDeployResult prints a concise summary of a deployed skill agent, including -// the tool task types the caller must serve (via 'skill run'/'skill serve') for the -// agent to run end to end. -func renderDeployResult(skillName string, result agent.DeployResult) error { - name := result.AgentName - if name == "" { - name = skillName - } - fmt.Printf("Skill %s deployed as agent %s.\n", skillName, name) - if len(result.RequiredWorkers) > 0 { - fmt.Println("Required workers:") - for _, w := range result.RequiredWorkers { - fmt.Printf(" - %s\n", w) - } - } - return nil -} - -func init() { - skillLoadCmd.Flags().StringVar(&skillLoadModel, "model", "", "Orchestrator and default model (required)") - skillLoadCmd.Flags().StringArrayVar(&skillLoadAgentModels, "agent-model", nil, "Sub-agent model override (name=model, repeatable)") - skillLoadCmd.Flags().StringArrayVar(&skillLoadSearchPaths, "search-path", nil, "Cross-skill search directory (repeatable)") - skillCmd.AddCommand(skillLoadCmd) -} diff --git a/cmd/skill_materialize.go b/cmd/skill_materialize.go deleted file mode 100644 index bff9db7..0000000 --- a/cmd/skill_materialize.go +++ /dev/null @@ -1,150 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "context" - "crypto/sha256" - "encoding/hex" - "fmt" - "os" - "path/filepath" - "regexp" - "strings" - - "github.com/conductor-oss/conductor-cli/internal/skill" - "github.com/conductor-oss/conductor-cli/internal/updater" -) - -// skillCacheDirName is the sub-directory of the Conductor CLI config home under -// which downloaded skill packages are cached (config-driven, never ~/.agentspan). -const skillCacheDirName = "skills" - -var cacheSegmentUnsafe = regexp.MustCompile(`[^A-Za-z0-9._-]+`) - -// materializeSkill resolves a skill argument to a local directory. A local skill -// directory is used as-is; otherwise the argument is treated as a registered skill -// name, downloaded (and cached), and its cache directory returned. detail is nil -// for a local directory. -func materializeSkill(ctx context.Context, svc skill.Service, skillArg, version string) (dir string, detail *skill.Detail, err error) { - if isSkillDirectory(skillArg) { - return skillArg, nil, nil - } - d, err := svc.Get(ctx, skillArg, version) - if err != nil { - return "", nil, fmt.Errorf("skill %q is not a local skill directory and was not found on the server: %w", skillArg, err) - } - dir, err = ensureCachedSkillPackage(ctx, svc, d) - if err != nil { - return "", nil, err - } - return dir, &d, nil -} - -// ensureCachedSkillPackage returns the local files directory for a registered -// skill, downloading and extracting the package when the cache is absent or stale. -// The download is verified against the server checksum and installed atomically. -func ensureCachedSkillPackage(ctx context.Context, svc skill.Service, detail skill.Detail) (string, error) { - cacheDir, filesDir, checksumPath, err := skillCachePaths(detail) - if err != nil { - return "", err - } - if isCachedSkillCurrent(filesDir, checksumPath, detail.Checksum) { - return filesDir, nil - } - - data, err := svc.DownloadPackage(ctx, detail.Name, detail.Version) - if err != nil { - return "", err - } - if checksum := strings.TrimSpace(detail.Checksum); checksum != "" { - if actual := skillPackageChecksum(data); !strings.EqualFold(actual, checksum) { - return "", fmt.Errorf("downloaded skill package checksum mismatch for %s@%s: expected %s, got %s", - detail.Name, detail.Version, checksum, actual) - } - } - - parentDir := filepath.Dir(cacheDir) - if err := os.MkdirAll(parentDir, 0o700); err != nil { - return "", fmt.Errorf("create skill cache parent: %w", err) - } - tmpDir, err := os.MkdirTemp(parentDir, "."+filepath.Base(cacheDir)+"-*") - if err != nil { - return "", fmt.Errorf("create temp skill cache: %w", err) - } - cleanupTmp := true - defer func() { - if cleanupTmp { - _ = os.RemoveAll(tmpDir) - } - }() - - if err := extractSkillPackage(data, filepath.Join(tmpDir, "files")); err != nil { - return "", err - } - if err := os.WriteFile(filepath.Join(tmpDir, "checksum"), []byte(detail.Checksum), 0o600); err != nil { - return "", fmt.Errorf("write skill cache checksum: %w", err) - } - if err := os.RemoveAll(cacheDir); err != nil { - return "", fmt.Errorf("clear stale skill cache: %w", err) - } - if err := os.Rename(tmpDir, cacheDir); err != nil { - return "", fmt.Errorf("install skill cache: %w", err) - } - cleanupTmp = false - return filesDir, nil -} - -// skillCachePaths derives the cache directory for a skill under the Conductor CLI -// config home (config-driven, not ~/.agentspan). -func skillCachePaths(detail skill.Detail) (cacheDir, filesDir, checksumPath string, err error) { - configDir, err := updater.GetConfigDir() - if err != nil { - return "", "", "", fmt.Errorf("resolve config directory: %w", err) - } - cacheDir = filepath.Join(configDir, skillCacheDirName, safeCacheSegment(detail.Name), safeCacheSegment(detail.Version)) - return cacheDir, filepath.Join(cacheDir, "files"), filepath.Join(cacheDir, "checksum"), nil -} - -// safeCacheSegment makes a name/version safe as a single path segment, disambiguating -// with a content hash suffix when characters had to be replaced. -func safeCacheSegment(value string) string { - cleaned := strings.Trim(cacheSegmentUnsafe.ReplaceAllString(value, "_"), "._-") - if cleaned == "" { - cleaned = "unnamed" - } - if cleaned != value { - sum := sha256.Sum256([]byte(value)) - cleaned = cleaned + "-" + hex.EncodeToString(sum[:])[:8] - } - return cleaned -} - -func skillPackageChecksum(data []byte) string { - sum := sha256.Sum256(data) - return hex.EncodeToString(sum[:]) -} - -// isCachedSkillCurrent reports whether a cached skill's files exist and match the -// expected checksum. -func isCachedSkillCurrent(filesDir, checksumPath, checksum string) bool { - if !isSkillDirectory(filesDir) { - return false - } - if checksum == "" { - return true - } - data, err := os.ReadFile(checksumPath) - return err == nil && strings.TrimSpace(string(data)) == checksum -} diff --git a/cmd/skill_payload.go b/cmd/skill_payload.go deleted file mode 100644 index 16c6bfb..0000000 --- a/cmd/skill_payload.go +++ /dev/null @@ -1,546 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "encoding/json" - "fmt" - "os" - "path/filepath" - "regexp" - "sort" - "strings" - - "github.com/conductor-oss/conductor-cli/internal/skillworker" -) - -// frameworkSkill is the framework marker for skill-backed agent definitions; it is -// the envelope selector sent to /api/agent/deploy and /api/agent/start. -const frameworkSkill = "skill" - -// skillSectionSplitBytes is the SKILL.md body size (~50 KB) above which the builder -// pre-splits the instructions into per-heading sections that the read_skill_file -// worker can serve on demand instead of shipping the whole body inline. -const skillSectionSplitBytes = 50 << 10 - -const ( - scriptsDirName = "scripts" // optional executables directory inside a skill - agentFileSuffix = "-agent.md" // sub-agent instruction files: "{name}-agent.md" - defaultScriptLanguage = skillworker.LangBash // language for scripts with an unknown extension -) - -// scriptLanguageByExt maps a script file extension to its execution language. The -// language vocabulary is owned by skillworker (which executes scripts), so the two -// sides cannot drift. Extensions absent here fall back to defaultScriptLanguage. -var scriptLanguageByExt = map[string]string{ - ".py": skillworker.LangPython, - ".sh": skillworker.LangBash, - ".bat": skillworker.LangBatch, - ".cmd": skillworker.LangBatch, - ".js": skillworker.LangNode, - ".mjs": skillworker.LangNode, - ".ts": skillworker.LangNode, - ".rb": skillworker.LangRuby, - ".go": skillworker.LangGo, -} - -// skillResourceDirs are the sub-directories whose files are exposed as skill -// resources (in addition to eligible root-level files). -var skillResourceDirs = []string{"references", "examples", "assets"} - -// agentsSkillsSubpath is the conventional per-project / per-user skill library -// location (".agents/skills") searched when resolving cross-skill references. -var agentsSkillsSubpath = filepath.Join(".agents", "skills") - -// crossSkillRefPattern matches prose like "invoke the foo skill" / "use bar skill" -// in a SKILL.md body, capturing the referenced skill name. -var crossSkillRefPattern = regexp.MustCompile(`(?i)(?:invoke|use|call)\s+(?:the\s+)?([a-z][a-z0-9-]*)\s+skill`) - -// sectionHeadingPrefix marks a level-2 markdown heading; the body is split into -// sections at each line that begins with it. -const sectionHeadingPrefix = "## " - -// SkillConfig is the server-bound skill definition. Every field is serialized to -// the deploy/start rawConfig; there are no local-only fields (paths and pre-split -// sections live in LocalContext instead), so no strip step is needed before the -// wire — the type is the contract. -type SkillConfig struct { - Model string `json:"model,omitempty"` - AgentModels map[string]string `json:"agentModels,omitempty"` - SkillMd string `json:"skillMd"` - AgentFiles map[string]string `json:"agentFiles,omitempty"` // agentName → body - Scripts map[string]ScriptInfo `json:"scripts,omitempty"` // toolName → {filename,language} - ResourceFiles []string `json:"resourceFiles,omitempty"` // skill-relative paths - CrossSkillRefs map[string]SkillConfig `json:"crossSkillRefs,omitempty"` // refName → nested config - DefaultParams map[string]ParamValue `json:"defaultParams,omitempty"` - Params map[string]ParamValue `json:"params,omitempty"` - // Workspace is set only by the run/serve worker runtime; load never populates it. - // The wire type is owned by skillworker (which serves the workspace tools). - Workspace *skillworker.WorkspaceWire `json:"workspace,omitempty"` -} - -// ScriptInfo describes one discovered script tool. -type ScriptInfo struct { - Filename string `json:"filename"` - Language string `json:"language"` -} - -// ParamValue is one skill-parameter value. Skill params are scalars: a --param -// override parses to a bool ("true"/"false") or a string, and a SKILL.md -// frontmatter default carries through its parsed YAML scalar. ParamValue keeps the -// value typed at the wire boundary (SkillConfig holds no bare interface{} map) and -// both marshals to, and stringifies as, its underlying scalar. -type ParamValue struct { - v any -} - -func rawParamValue(v any) ParamValue { return ParamValue{v: v} } - -// MarshalJSON emits the underlying scalar. -func (p ParamValue) MarshalJSON() ([]byte, error) { return json.Marshal(p.v) } - -// format renders the value for the [Skill Parameters] block injected into SKILL.md. -func (p ParamValue) format() string { return fmt.Sprint(p.v) } - -// LocalContext is the never-serialized side of a built skill. It stays on the CLI -// side and feeds the run/serve workers: the skill name is the worker task-type -// prefix, SkillDir roots local file/script tools, Sections lets the read_skill_file -// worker serve "skill_section:{slug}" requests for large bodies, and CrossSkills -// carries the same context for each resolved cross-skill so their script/file -// workers can be started too. -type LocalContext struct { - SkillName string - SkillDir string - Sections map[string]string // slug → SKILL.md section body - CrossSkills map[string]LocalContext // refName → nested local context -} - -// PayloadOptions carries the caller-resolved (in cmd) inputs the builder needs. -type PayloadOptions struct { - Model string - AgentModels map[string]string - SearchPaths []string - ParamOverrides map[string]ParamValue -} - -// BuildSkillPayload reads a local skill directory and produces its typed wire -// config plus the local-only context. All filesystem access stays here in the cmd -// layer; the returned SkillConfig is pure data ready to marshal for deploy/start. -func BuildSkillPayload(dir string, opts PayloadOptions) (SkillConfig, LocalContext, error) { - return buildSkillPayloadInternal(dir, opts, map[string]bool{}) -} - -func buildSkillPayloadInternal(dir string, opts PayloadOptions, seen map[string]bool) (SkillConfig, LocalContext, error) { - absPath, err := filepath.Abs(expandUserPath(dir)) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("resolve path: %w", err) - } - absPath, err = filepath.EvalSymlinks(absPath) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("resolve path: %w", err) - } - - skillMdContent, err := os.ReadFile(filepath.Join(absPath, skillMarkdownFile)) - if err != nil { - if os.IsNotExist(err) { - return SkillConfig{}, LocalContext{}, fmt.Errorf("directory %q is not a valid skill: %s not found", absPath, skillMarkdownFile) - } - return SkillConfig{}, LocalContext{}, fmt.Errorf("read %s: %w", skillMarkdownFile, err) - } - - frontmatter, err := parseFrontmatter(string(skillMdContent)) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("parse %s frontmatter: %w", skillMarkdownFile, err) - } - skillName, _ := frontmatter["name"].(string) - if skillName == "" { - return SkillConfig{}, LocalContext{}, fmt.Errorf("%s missing required 'name' field in frontmatter", skillMarkdownFile) - } - - agentFiles, err := discoverAgentFiles(absPath) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("discover agent files: %w", err) - } - scripts, err := discoverScripts(absPath) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("discover scripts: %w", err) - } - resourceFiles, err := collectResourceFiles(absPath) - if err != nil { - return SkillConfig{}, LocalContext{}, fmt.Errorf("collect resource files: %w", err) - } - crossRefs, crossLocals, err := resolveCrossSkills(string(skillMdContent), absPath, opts, seen) - if err != nil { - return SkillConfig{}, LocalContext{}, err - } - - // Params: frontmatter defaults overlaid with caller overrides, then rendered - // into SKILL.md so the server-side orchestrator sees them. - defaultParams := extractDefaultParams(frontmatter) - mergedParams := mergeParams(defaultParams, opts.ParamOverrides) - skillMd := string(skillMdContent) - if len(mergedParams) > 0 { - skillMd = skillMd + "\n\n" + formatParamMap(mergedParams) + "\n" - } - - sections := splitSkillSections(extractBody(skillMd)) - - cfg := SkillConfig{ - Model: opts.Model, - AgentModels: opts.AgentModels, - SkillMd: skillMd, - AgentFiles: agentFiles, - Scripts: scripts, - ResourceFiles: resourceFiles, - CrossSkillRefs: crossRefs, - DefaultParams: defaultParams, - Params: mergedParams, - } - local := LocalContext{SkillName: skillName, SkillDir: absPath, Sections: sections, CrossSkills: crossLocals} - return cfg, local, nil -} - -// discoverAgentFiles globs "*-agent.md" and maps agent name → file body. -func discoverAgentFiles(skillDir string) (map[string]string, error) { - matches, err := filepath.Glob(filepath.Join(skillDir, "*"+agentFileSuffix)) - if err != nil { - return nil, fmt.Errorf("glob agent files: %w", err) - } - if len(matches) == 0 { - return nil, nil - } - result := make(map[string]string, len(matches)) - for _, match := range matches { - base := filepath.Base(match) - content, err := os.ReadFile(match) - if err != nil { - return nil, fmt.Errorf("read %s: %w", base, err) - } - result[strings.TrimSuffix(base, agentFileSuffix)] = string(content) - } - return result, nil -} - -// discoverScripts lists files in scripts/ and maps tool name → script info. The -// directory is optional. -func discoverScripts(skillDir string) (map[string]ScriptInfo, error) { - entries, err := os.ReadDir(filepath.Join(skillDir, scriptsDirName)) - if err != nil { - if os.IsNotExist(err) { - return nil, nil - } - return nil, fmt.Errorf("read scripts directory: %w", err) - } - result := make(map[string]ScriptInfo) - for _, entry := range entries { - if entry.IsDir() { - continue - } - filename := entry.Name() - toolName := strings.TrimSuffix(filename, filepath.Ext(filename)) - if toolName == "" { - toolName = filename - } - result[toolName] = ScriptInfo{Filename: filename, Language: detectScriptLanguage(filename)} - } - if len(result) == 0 { - return nil, nil - } - return result, nil -} - -// detectScriptLanguage maps a filename to its language via scriptLanguageByExt, -// defaulting to defaultScriptLanguage. -func detectScriptLanguage(filename string) string { - if lang, ok := scriptLanguageByExt[strings.ToLower(filepath.Ext(filename))]; ok { - return lang - } - return defaultScriptLanguage -} - -// collectResourceFiles lists resource-dir files plus eligible root files (excluding -// SKILL.md and *-agent.md) as skill-relative slash paths, honoring the ignore file. -func collectResourceFiles(skillDir string) ([]string, error) { - ignore, err := loadSkillPackageIgnore(skillDir) - if err != nil { - return nil, err - } - var result []string - for _, subdir := range skillResourceDirs { - dir := filepath.Join(skillDir, subdir) - if _, statErr := os.Stat(dir); os.IsNotExist(statErr) { - continue - } - walkErr := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return err - } - if info.IsDir() { - return nil - } - rel, relErr := filepath.Rel(skillDir, path) - if relErr != nil { - return relErr - } - rel = filepath.ToSlash(rel) - if shouldExcludeSkillPackagePath(rel, false, ignore) { - return nil - } - result = append(result, rel) - return nil - }) - if walkErr != nil { - return nil, fmt.Errorf("walk %s: %w", subdir, walkErr) - } - } - - entries, err := os.ReadDir(skillDir) - if err != nil { - return nil, fmt.Errorf("read skill directory: %w", err) - } - for _, entry := range entries { - if entry.IsDir() { - continue - } - name := entry.Name() - if shouldExcludeSkillPackagePath(name, false, ignore) || name == skillMarkdownFile || strings.HasSuffix(name, agentFileSuffix) { - continue - } - result = append(result, name) - } - return result, nil -} - -// resolveCrossSkills packages referenced sibling/project/user skills recursively, -// guarding against cycles. It returns the typed wire configs (no local fields leak -// into them — what removes the original's strip step) paired with their local -// contexts (so run/serve can start each cross-skill's workers). -func resolveCrossSkills(skillMd, skillDir string, opts PayloadOptions, seen map[string]bool) (map[string]SkillConfig, map[string]LocalContext, error) { - refNames := referencedSkillNames(skillMd) - if len(refNames) == 0 { - return nil, nil, nil - } - - searchDirs := []string{filepath.Dir(skillDir), filepath.Join(".", agentsSkillsSubpath)} - if home, err := os.UserHomeDir(); err == nil { - searchDirs = append(searchDirs, filepath.Join(home, agentsSkillsSubpath)) - } - searchDirs = append(searchDirs, opts.SearchPaths...) - - refs := make(map[string]SkillConfig) - locals := make(map[string]LocalContext) - for _, refName := range refNames { - refDir, ok := findSkillDir(refName, searchDirs) - if !ok { - continue - } - refAbs, err := filepath.Abs(refDir) - if err != nil { - return nil, nil, err - } - refAbs, err = filepath.EvalSymlinks(refAbs) - if err != nil { - return nil, nil, err - } - if refAbs == skillDir { - continue - } - if seen[refAbs] { - return nil, nil, fmt.Errorf("circular skill reference detected: %s", refName) - } - nextSeen := make(map[string]bool, len(seen)+1) - for k, v := range seen { - nextSeen[k] = v - } - nextSeen[skillDir] = true - - refCfg, refLocal, err := buildSkillPayloadInternal(refAbs, opts, nextSeen) - if err != nil { - return nil, nil, fmt.Errorf("resolve cross-skill %q: %w", refName, err) - } - refs[refName] = refCfg - locals[refName] = refLocal - } - if len(refs) == 0 { - return nil, nil, nil - } - return refs, locals, nil -} - -// referencedSkillNames returns the de-duplicated, sorted skill names referenced in -// the SKILL.md body prose. -func referencedSkillNames(skillMd string) []string { - matches := crossSkillRefPattern.FindAllStringSubmatch(extractBody(skillMd), -1) - seen := make(map[string]bool) - var names []string - for _, m := range matches { - name := strings.ToLower(m[1]) - if seen[name] { - continue - } - seen[name] = true - names = append(names, name) - } - sort.Strings(names) - return names -} - -// findSkillDir returns the first search dir that contains "{name}/SKILL.md". -func findSkillDir(name string, dirs []string) (string, bool) { - for _, dir := range dirs { - if dir == "" { - continue - } - candidate := filepath.Join(expandUserPath(dir), name) - if _, err := os.Stat(filepath.Join(candidate, skillMarkdownFile)); err == nil { - return candidate, true - } - } - return "", false -} - -// extractBody returns the markdown body after the YAML frontmatter (mirrors the -// register command's parseFrontmatter, which returns the parsed head). -func extractBody(content string) string { - content = strings.TrimSpace(content) - if !strings.HasPrefix(content, "---") { - return content - } - rest := strings.TrimPrefix(content[3:], "\n") - end := strings.Index(rest, "\n---") - if end < 0 { - return content - } - return strings.TrimPrefix(rest[end+4:], "\n") -} - -// extractDefaultParams reads frontmatter "params" defaults. A param may be given as -// a scalar or as a "{default: ...}" object; both collapse to a ParamValue. -func extractDefaultParams(frontmatter map[string]any) map[string]ParamValue { - params, ok := frontmatter["params"].(map[string]any) - if !ok { - return nil - } - defaults := make(map[string]ParamValue, len(params)) - for name, raw := range params { - if def, ok := raw.(map[string]any); ok { - if value, exists := def["default"]; exists { - defaults[name] = rawParamValue(value) - continue - } - } - defaults[name] = rawParamValue(raw) - } - if len(defaults) == 0 { - return nil - } - return defaults -} - -// mergeParams overlays overrides onto defaults (overrides win). -func mergeParams(defaults, overrides map[string]ParamValue) map[string]ParamValue { - if len(defaults) == 0 && len(overrides) == 0 { - return nil - } - merged := make(map[string]ParamValue, len(defaults)+len(overrides)) - for k, v := range defaults { - merged[k] = v - } - for k, v := range overrides { - merged[k] = v - } - return merged -} - -// formatParamMap renders a deterministic "[Skill Parameters]" block. -func formatParamMap(params map[string]ParamValue) string { - if len(params) == 0 { - return "" - } - keys := make([]string, 0, len(params)) - for k := range params { - keys = append(keys, k) - } - sort.Strings(keys) - var sb strings.Builder - sb.WriteString("[Skill Parameters]\n") - for i, k := range keys { - if i > 0 { - sb.WriteString("\n") - } - sb.WriteString(k) - sb.WriteString(": ") - sb.WriteString(params[k].format()) - } - return sb.String() -} - -// splitSkillSections breaks a large body into slug → "## section" bodies so the -// read_skill_file worker can serve them on demand. It scans line by line, starting -// a new section at each heading line (Go's RE2 has no look-ahead, so a split regex -// is not an option). Small bodies, and bodies with no headings, return nil. -func splitSkillSections(body string) map[string]string { - if len(body) <= skillSectionSplitBytes { - return nil - } - sections := make(map[string]string) - var block []string - flush := func() { - if len(block) == 0 { - return - } - section := strings.TrimSpace(strings.Join(block, "\n")) - block = block[:0] - if !strings.HasPrefix(section, sectionHeadingPrefix) { - return // discard the pre-heading preamble - } - firstLine := strings.SplitN(section, "\n", 2)[0] - slug := slugifyHeading(strings.TrimSpace(strings.TrimPrefix(firstLine, sectionHeadingPrefix))) - if slug != "" { - sections[slug] = section - } - } - for _, line := range strings.Split(body, "\n") { - if strings.HasPrefix(line, sectionHeadingPrefix) { - flush() - } - block = append(block, line) - } - flush() - if len(sections) == 0 { - return nil - } - return sections -} - -// slugifyHeading lowercases text and collapses runs of spaces/dashes into single -// dashes, keeping only [a-z0-9-]. -func slugifyHeading(text string) string { - text = strings.ToLower(text) - var sb strings.Builder - lastDash := false - for _, r := range text { - switch { - case r >= 'a' && r <= 'z', r >= '0' && r <= '9': - sb.WriteRune(r) - lastDash = false - case r == ' ' || r == '\t' || r == '-': - if !lastDash && sb.Len() > 0 { - sb.WriteByte('-') - lastDash = true - } - } - } - return strings.Trim(sb.String(), "-") -} diff --git a/cmd/skill_payload_test.go b/cmd/skill_payload_test.go deleted file mode 100644 index 2933751..0000000 --- a/cmd/skill_payload_test.go +++ /dev/null @@ -1,234 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "encoding/json" - "os" - "path/filepath" - "strings" - "testing" -) - -// writeSkillFile writes content to dir/rel, creating parent directories. -func writeSkillFile(t *testing.T, dir, rel, content string) { - t.Helper() - full := filepath.Join(dir, filepath.FromSlash(rel)) - if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { - t.Fatalf("mkdir for %s: %v", rel, err) - } - if err := os.WriteFile(full, []byte(content), 0o644); err != nil { - t.Fatalf("write %s: %v", rel, err) - } -} - -func TestBuildSkillPayloadTypedFields(t *testing.T) { - dir := t.TempDir() - writeSkillFile(t, dir, "SKILL.md", "---\nname: demo\nparams:\n tone:\n default: friendly\n verbose: false\n---\nBody text.\n") - writeSkillFile(t, dir, "planner-agent.md", "You plan.") - writeSkillFile(t, dir, "scripts/greet.py", "print('hi')") - writeSkillFile(t, dir, "scripts/build.sh", "echo build") - writeSkillFile(t, dir, "references/guide.md", "guide") - writeSkillFile(t, dir, "notes.txt", "root resource") - - cfg, local, err := BuildSkillPayload(dir, PayloadOptions{ - Model: "gpt-x", - AgentModels: map[string]string{"planner": "gpt-mini"}, - }) - if err != nil { - t.Fatalf("BuildSkillPayload: %v", err) - } - - if cfg.Model != "gpt-x" { - t.Errorf("Model = %q, want gpt-x", cfg.Model) - } - if cfg.AgentModels["planner"] != "gpt-mini" { - t.Errorf("AgentModels = %v", cfg.AgentModels) - } - if body := cfg.AgentFiles["planner"]; body != "You plan." { - t.Errorf("AgentFiles[planner] = %q", body) - } - if got := cfg.Scripts["greet"]; got.Filename != "greet.py" || got.Language != "python" { - t.Errorf("Scripts[greet] = %+v", got) - } - if got := cfg.Scripts["build"]; got.Filename != "build.sh" || got.Language != "bash" { - t.Errorf("Scripts[build] = %+v", got) - } - if !containsString(cfg.ResourceFiles, "references/guide.md") || !containsString(cfg.ResourceFiles, "notes.txt") { - t.Errorf("ResourceFiles = %v", cfg.ResourceFiles) - } - // SKILL.md and *-agent.md must never be listed as resources. - if containsString(cfg.ResourceFiles, "SKILL.md") || containsString(cfg.ResourceFiles, "planner-agent.md") { - t.Errorf("ResourceFiles leaked non-resource files: %v", cfg.ResourceFiles) - } - // Frontmatter defaults: {default: friendly} collapses to the value; scalar carries through. - if cfg.DefaultParams["tone"].format() != "friendly" || cfg.DefaultParams["verbose"].format() != "false" { - t.Errorf("DefaultParams = tone=%q verbose=%q", cfg.DefaultParams["tone"].format(), cfg.DefaultParams["verbose"].format()) - } - // Params get injected into skillMd for server visibility. - if !strings.Contains(cfg.SkillMd, "[Skill Parameters]") || !strings.Contains(cfg.SkillMd, "tone: friendly") { - t.Errorf("skillMd missing injected params:\n%s", cfg.SkillMd) - } - if local.SkillName != "demo" { - t.Errorf("LocalContext.SkillName = %q, want demo", local.SkillName) - } - if !filepath.IsAbs(local.SkillDir) { - t.Errorf("LocalContext.SkillDir not absolute: %q", local.SkillDir) - } -} - -// TestBuildSkillPayloadParamOverride verifies overrides win over frontmatter defaults. -func TestBuildSkillPayloadParamOverride(t *testing.T) { - dir := t.TempDir() - writeSkillFile(t, dir, "SKILL.md", "---\nname: demo\nparams:\n tone:\n default: friendly\n---\nBody.\n") - - cfg, _, err := BuildSkillPayload(dir, PayloadOptions{ - Model: "m", - ParamOverrides: map[string]ParamValue{"tone": rawParamValue("terse")}, - }) - if err != nil { - t.Fatalf("BuildSkillPayload: %v", err) - } - if cfg.Params["tone"].format() != "terse" { - t.Errorf("merged param tone = %q, want terse", cfg.Params["tone"].format()) - } - if !strings.Contains(cfg.SkillMd, "tone: terse") { - t.Errorf("skillMd should carry the overridden param:\n%s", cfg.SkillMd) - } -} - -// TestBuildSkillPayloadWireLocalSplit asserts the wire type carries no local paths -// and the local context carries no server fields — the invariant that replaces the -// original's _skill-prefixed keys and stripLocalSkillFields step. -func TestBuildSkillPayloadWireLocalSplit(t *testing.T) { - dir := t.TempDir() - writeSkillFile(t, dir, "SKILL.md", "---\nname: demo\n---\nBody.\n") - - cfg, local, err := BuildSkillPayload(dir, PayloadOptions{Model: "m"}) - if err != nil { - t.Fatalf("BuildSkillPayload: %v", err) - } - raw, err := json.Marshal(cfg) - if err != nil { - t.Fatalf("marshal: %v", err) - } - wire := string(raw) - for _, banned := range []string{"_skill", "skillPath", "skillDir", local.SkillDir} { - if strings.Contains(wire, banned) { - t.Errorf("wire config leaked %q: %s", banned, wire) - } - } - // No local-context field name should appear as a JSON key either. - var probe map[string]json.RawMessage - if err := json.Unmarshal(raw, &probe); err != nil { - t.Fatalf("unmarshal: %v", err) - } - for _, k := range []string{"sections", "skillDir", "skillName"} { - if _, ok := probe[k]; ok { - t.Errorf("wire config unexpectedly has key %q", k) - } - } -} - -// TestBuildSkillPayloadCrossSkillRecursion checks a referenced sibling skill is -// packaged recursively as a nested typed config. -func TestBuildSkillPayloadCrossSkillRecursion(t *testing.T) { - root := t.TempDir() - main := filepath.Join(root, "main") - writeSkillFile(t, main, "SKILL.md", "---\nname: main\n---\nPlease invoke the helper skill to assist.\n") - helper := filepath.Join(root, "helper") - writeSkillFile(t, helper, "SKILL.md", "---\nname: helper\n---\nHelper body.\n") - writeSkillFile(t, helper, "scripts/aid.sh", "echo aid") - - cfg, _, err := BuildSkillPayload(main, PayloadOptions{Model: "m"}) - if err != nil { - t.Fatalf("BuildSkillPayload: %v", err) - } - ref, ok := cfg.CrossSkillRefs["helper"] - if !ok { - t.Fatalf("expected cross-skill ref 'helper', got %v", keysOfStringMap(cfg.CrossSkillRefs)) - } - if _, ok := ref.Scripts["aid"]; !ok { - t.Errorf("nested helper config missing its script: %+v", ref.Scripts) - } -} - -// TestBuildSkillPayloadCycleGuard ensures a circular reference is rejected, not -// recursed forever. -func TestBuildSkillPayloadCycleGuard(t *testing.T) { - root := t.TempDir() - a := filepath.Join(root, "alpha") - writeSkillFile(t, a, "SKILL.md", "---\nname: alpha\n---\nUse the beta skill.\n") - b := filepath.Join(root, "beta") - writeSkillFile(t, b, "SKILL.md", "---\nname: beta\n---\nUse the alpha skill.\n") - - _, _, err := BuildSkillPayload(a, PayloadOptions{Model: "m"}) - if err == nil { - t.Fatal("expected a circular skill reference error") - } - if !strings.Contains(err.Error(), "circular") { - t.Errorf("error = %v, want a circular-reference error", err) - } -} - -// TestBuildSkillPayloadRequiresName rejects a SKILL.md without a name. -func TestBuildSkillPayloadRequiresName(t *testing.T) { - dir := t.TempDir() - writeSkillFile(t, dir, "SKILL.md", "---\ndescription: no name here\n---\nBody.\n") - if _, _, err := BuildSkillPayload(dir, PayloadOptions{Model: "m"}); err == nil { - t.Fatal("expected an error for a SKILL.md missing 'name'") - } -} - -// TestSplitSkillSectionsLargeBody exercises the section split above the threshold. -func TestSplitSkillSectionsLargeBody(t *testing.T) { - filler := strings.Repeat("x", skillSectionSplitBytes) - body := "## First Heading\n" + filler + "\n## Second Heading\nmore\n" - sections := splitSkillSections(body) - if _, ok := sections["first-heading"]; !ok { - t.Errorf("missing first-heading section: %v", keysOfStringMap2(sections)) - } - if _, ok := sections["second-heading"]; !ok { - t.Errorf("missing second-heading section: %v", keysOfStringMap2(sections)) - } - // Small bodies do not split. - if splitSkillSections("## Tiny\nshort") != nil { - t.Error("small body should not be split") - } -} - -func containsString(xs []string, want string) bool { - for _, x := range xs { - if x == want { - return true - } - } - return false -} - -func keysOfStringMap(m map[string]SkillConfig) []string { - ks := make([]string, 0, len(m)) - for k := range m { - ks = append(ks, k) - } - return ks -} - -func keysOfStringMap2(m map[string]string) []string { - ks := make([]string, 0, len(m)) - for k := range m { - ks = append(ks, k) - } - return ks -} diff --git a/cmd/skill_register.go b/cmd/skill_register.go deleted file mode 100644 index 4e56a30..0000000 --- a/cmd/skill_register.go +++ /dev/null @@ -1,391 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "archive/zip" - "bytes" - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" - "os" - pathpkg "path" - "path/filepath" - "sort" - "strings" - "time" - - "github.com/spf13/cobra" - "gopkg.in/yaml.v3" - - "github.com/conductor-oss/conductor-cli/internal" -) - -// skillPackageFile is the per-file manifest entry sent to the server. -type skillPackageFile struct { - Path string `json:"path"` - Size int64 `json:"size"` - SHA256 string `json:"sha256"` - ContentType string `json:"contentType"` -} - -type skillPackageSource struct { - FullPath string - RelPath string - Mode os.FileMode -} - -// skillManifest is the JSON manifest uploaded alongside the package zip. It is a -// typed struct so the wire shape is explicit and no map crosses a layer boundary. -type skillManifest struct { - Name string `json:"name"` - Version string `json:"version,omitempty"` - Description string `json:"description,omitempty"` - Metadata any `json:"metadata,omitempty"` - Model string `json:"model,omitempty"` - AgentModels map[string]string `json:"agentModels,omitempty"` - Files []skillPackageFile `json:"files"` -} - -var ( - skillRegisterVersion string - skillRegisterModel string - skillRegisterAgentModels []string -) - -var skillRegisterCmd = &cobra.Command{ - Use: "register ", - Short: "Package and register a local skill with the server", - Args: cobra.ExactArgs(1), - SilenceUsage: true, - RunE: func(cmd *cobra.Command, args []string) error { - path := args[0] - if !isSkillDirectory(path) { - return fmt.Errorf("%q is not a skill directory (SKILL.md not found)", path) - } - pkg, files, err := buildSkillPackage(path) - if err != nil { - return err - } - manifest, err := buildSkillManifest(path, skillRegisterVersion, skillRegisterModel, skillRegisterAgentModels, files) - if err != nil { - return err - } - detail, err := internal.GetSkillService().Register(cmd.Context(), manifest, pkg) - if err != nil { - return err - } - fmt.Printf("Skill %s registered as version %s.\n", detail.Name, detail.Version) - return nil - }, -} - -// buildSkillManifest reads SKILL.md frontmatter and assembles the upload manifest. -func buildSkillManifest(skillDir, version, model string, agentModelFlags []string, files []skillPackageFile) (json.RawMessage, error) { - data, err := os.ReadFile(filepath.Join(expandUserPath(skillDir), skillMarkdownFile)) - if err != nil { - return nil, fmt.Errorf("read %s: %w", skillMarkdownFile, err) - } - frontmatter, err := parseFrontmatter(string(data)) - if err != nil { - return nil, err - } - name, _ := frontmatter["name"].(string) - if name == "" { - return nil, fmt.Errorf("%s is missing the required 'name' field in its frontmatter", skillMarkdownFile) - } - description, _ := frontmatter["description"].(string) - agentModels, err := parseAgentModelFlags(agentModelFlags) - if err != nil { - return nil, err - } - manifest := skillManifest{ - Name: name, - Version: version, - Description: description, - Metadata: frontmatter["metadata"], - Model: model, - AgentModels: agentModels, - Files: files, - } - return json.Marshal(manifest) -} - -const skillMarkdownFile = "SKILL.md" - -// isSkillDirectory reports whether path is a directory containing a SKILL.md. -func isSkillDirectory(path string) bool { - expanded := expandUserPath(path) - info, err := os.Stat(expanded) - if err != nil || !info.IsDir() { - return false - } - _, err = os.Stat(filepath.Join(expanded, skillMarkdownFile)) - return err == nil -} - -// buildSkillPackage zips a skill directory deterministically and returns the bytes -// plus the per-file manifest. Secret files and ignored paths are excluded. -func buildSkillPackage(skillPath string) ([]byte, []skillPackageFile, error) { - absPath, err := filepath.Abs(expandUserPath(skillPath)) - if err != nil { - return nil, nil, fmt.Errorf("resolve path: %w", err) - } - if absPath, err = filepath.EvalSymlinks(absPath); err != nil { - return nil, nil, fmt.Errorf("resolve path: %w", err) - } - ignore, err := loadSkillPackageIgnore(absPath) - if err != nil { - return nil, nil, err - } - - var sources []skillPackageSource - walkErr := filepath.WalkDir(absPath, func(path string, entry os.DirEntry, err error) error { - if err != nil { - return err - } - rel, relErr := filepath.Rel(absPath, path) - if relErr != nil { - return relErr - } - rel = filepath.ToSlash(rel) - if rel == "." { - return nil - } - if entry.IsDir() { - if shouldExcludeSkillPackagePath(rel, true, ignore) || isDefaultGeneratedSkillDir(entry.Name()) { - return filepath.SkipDir - } - return nil - } - info, infoErr := entry.Info() - if infoErr != nil { - return infoErr - } - if !info.Mode().IsRegular() || shouldExcludeSkillPackagePath(rel, false, ignore) { - return nil - } - sources = append(sources, skillPackageSource{FullPath: path, RelPath: rel, Mode: info.Mode()}) - return nil - }) - if walkErr != nil { - return nil, nil, fmt.Errorf("walk skill directory: %w", walkErr) - } - sort.Slice(sources, func(i, j int) bool { return sources[i].RelPath < sources[j].RelPath }) - - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - files := make([]skillPackageFile, 0, len(sources)) - for _, src := range sources { - data, readErr := os.ReadFile(src.FullPath) - if readErr != nil { - _ = zw.Close() - return nil, nil, fmt.Errorf("read %s: %w", src.RelPath, readErr) - } - header := &zip.FileHeader{Name: src.RelPath, Method: zip.Deflate, Modified: time.Unix(0, 0).UTC()} - header.SetMode(src.Mode.Perm()) - w, hErr := zw.CreateHeader(header) - if hErr != nil { - _ = zw.Close() - return nil, nil, fmt.Errorf("create zip entry %s: %w", src.RelPath, hErr) - } - if _, wErr := w.Write(data); wErr != nil { - _ = zw.Close() - return nil, nil, fmt.Errorf("write zip entry %s: %w", src.RelPath, wErr) - } - sum := sha256.Sum256(data) - files = append(files, skillPackageFile{ - Path: src.RelPath, - Size: int64(len(data)), - SHA256: hex.EncodeToString(sum[:]), - ContentType: guessContentType(src.RelPath), - }) - } - if err := zw.Close(); err != nil { - return nil, nil, fmt.Errorf("close skill package: %w", err) - } - return buf.Bytes(), files, nil -} - -// --- frontmatter --- - -// parseFrontmatter extracts the YAML frontmatter (delimited by ---) from SKILL.md. -func parseFrontmatter(content string) (map[string]any, error) { - content = strings.TrimSpace(content) - if !strings.HasPrefix(content, "---") { - return nil, fmt.Errorf("%s does not start with YAML frontmatter (---)", skillMarkdownFile) - } - rest := strings.TrimPrefix(content[3:], "\n") - end := strings.Index(rest, "\n---") - if end < 0 { - return nil, fmt.Errorf("%s frontmatter is not closed (missing second ---)", skillMarkdownFile) - } - var result map[string]any - if err := yaml.Unmarshal([]byte(rest[:end]), &result); err != nil { - return nil, fmt.Errorf("invalid YAML in frontmatter: %w", err) - } - if result == nil { - result = map[string]any{} - } - return result, nil -} - -// parseAgentModelFlags parses repeated name=model overrides. -func parseAgentModelFlags(flags []string) (map[string]string, error) { - if len(flags) == 0 { - return nil, nil - } - result := make(map[string]string, len(flags)) - for _, f := range flags { - parts := strings.SplitN(f, "=", 2) - if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return nil, fmt.Errorf("invalid --agent-model value %q: expected name=model", f) - } - result[parts[0]] = parts[1] - } - return result, nil -} - -// --- ignore matching --- - -type skillPackageIgnoreMatcher struct { - patterns []string -} - -const skillIgnoreFile = ".agentspanignore" - -func loadSkillPackageIgnore(skillRoot string) (skillPackageIgnoreMatcher, error) { - matcher := skillPackageIgnoreMatcher{} - data, err := os.ReadFile(filepath.Join(skillRoot, skillIgnoreFile)) - if err != nil { - if os.IsNotExist(err) { - return matcher, nil - } - return matcher, fmt.Errorf("read %s: %w", skillIgnoreFile, err) - } - for _, line := range strings.Split(string(data), "\n") { - if line = strings.TrimSpace(line); line != "" && !strings.HasPrefix(line, "#") { - matcher.patterns = append(matcher.patterns, filepath.ToSlash(line)) - } - } - return matcher, nil -} - -func shouldExcludeSkillPackagePath(relPath string, isDir bool, matcher skillPackageIgnoreMatcher) bool { - relPath = filepath.ToSlash(strings.TrimPrefix(relPath, "./")) - if relPath == "" || relPath == "." { - return false - } - base := pathpkg.Base(relPath) - if base == skillIgnoreFile || isDefaultSecretSkillFile(base) { - return true - } - if isDir && isDefaultGeneratedSkillDir(base) { - return true - } - return matcher.matches(relPath, isDir) -} - -func (m skillPackageIgnoreMatcher) matches(relPath string, isDir bool) bool { - for _, pattern := range m.patterns { - if skillIgnorePatternMatches(pattern, relPath, isDir) { - return true - } - } - return false -} - -func skillIgnorePatternMatches(pattern, relPath string, isDir bool) bool { - pattern = filepath.ToSlash(strings.TrimSpace(pattern)) - if pattern == "" || strings.HasPrefix(pattern, "!") { - return false - } - dirOnly := strings.HasSuffix(pattern, "/") - pattern = strings.TrimSuffix(pattern, "/") - if dirOnly && !isDir { - return relPath == pattern || strings.HasPrefix(relPath, pattern+"/") - } - if strings.Contains(pattern, "/") { - if ok, err := pathpkg.Match(pattern, relPath); err == nil && ok { - return true - } - return relPath == pattern || strings.HasPrefix(relPath, pattern+"/") - } - for _, part := range strings.Split(relPath, "/") { - if ok, err := pathpkg.Match(pattern, part); err == nil && ok { - return true - } - } - return false -} - -func isDefaultGeneratedSkillDir(name string) bool { - switch name { - case ".git", "__pycache__", "node_modules", ".venv", "venv", ".tox", "dist", "build", "target", ".gradle", ".pytest_cache", ".mypy_cache": - return true - default: - return false - } -} - -func isDefaultSecretSkillFile(name string) bool { - lower := strings.ToLower(name) - if lower == ".ds_store" || lower == ".env" || strings.HasPrefix(lower, ".env.") { - return true - } - switch lower { - case "id_rsa", "id_dsa", "id_ecdsa", "id_ed25519", "known_hosts": - return true - } - for _, suffix := range []string{".pem", ".key", ".p12", ".pfx", ".jks", ".keystore", ".crt", ".cer"} { - if strings.HasSuffix(lower, suffix) { - return true - } - } - return false -} - -func guessContentType(path string) string { - lower := strings.ToLower(path) - switch { - case strings.HasSuffix(lower, ".md"), strings.HasSuffix(lower, ".txt"): - return "text/plain" - case strings.HasSuffix(lower, ".json"): - return "application/json" - case strings.HasSuffix(lower, ".yaml"), strings.HasSuffix(lower, ".yml"): - return "application/yaml" - case strings.HasSuffix(lower, ".html"), strings.HasSuffix(lower, ".htm"): - return "text/html" - case strings.HasSuffix(lower, ".css"): - return "text/css" - case strings.HasSuffix(lower, ".js"), strings.HasSuffix(lower, ".mjs"): - return "text/javascript" - case strings.HasSuffix(lower, ".png"): - return "image/png" - case strings.HasSuffix(lower, ".jpg"), strings.HasSuffix(lower, ".jpeg"): - return "image/jpeg" - case strings.HasSuffix(lower, ".svg"): - return "image/svg+xml" - default: - return "application/octet-stream" - } -} - -func init() { - skillRegisterCmd.Flags().StringVar(&skillRegisterVersion, "version", "", "Optional version label (defaults to a content hash on the server)") - skillRegisterCmd.Flags().StringVar(&skillRegisterModel, "model", "", "Orchestrator and default model") - skillRegisterCmd.Flags().StringArrayVar(&skillRegisterAgentModels, "agent-model", nil, "Sub-agent model override (name=model, repeatable)") - skillCmd.AddCommand(skillRegisterCmd) -} diff --git a/cmd/skill_register_test.go b/cmd/skill_register_test.go deleted file mode 100644 index 7b57e9b..0000000 --- a/cmd/skill_register_test.go +++ /dev/null @@ -1,123 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "archive/zip" - "bytes" - "encoding/json" - "os" - "path/filepath" - "testing" -) - -func writeFile(t *testing.T, dir, rel, content string) { - t.Helper() - full := filepath.Join(dir, rel) - if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(full, []byte(content), 0o644); err != nil { - t.Fatal(err) - } -} - -func newSkillFixture(t *testing.T) string { - t.Helper() - dir := t.TempDir() - writeFile(t, dir, "SKILL.md", "---\nname: summarize\ndescription: Summarize text\n---\nBody text\n") - writeFile(t, dir, "writer-agent.md", "agent body") - writeFile(t, dir, "scripts/run.py", "print('hi')") - writeFile(t, dir, "references/doc.md", "reference") - writeFile(t, dir, ".agentspanignore", "*.tmp\n") - writeFile(t, dir, "scratch.tmp", "ignored") - writeFile(t, dir, ".env", "SECRET=x") - writeFile(t, dir, "node_modules/dep/index.js", "junk") - return dir -} - -func zipNames(t *testing.T, data []byte) map[string]bool { - t.Helper() - r, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) - if err != nil { - t.Fatalf("open zip: %v", err) - } - names := map[string]bool{} - for _, f := range r.File { - names[f.Name] = true - } - return names -} - -func TestBuildSkillPackageIncludesAndExcludes(t *testing.T) { - dir := newSkillFixture(t) - - pkg, files, err := buildSkillPackage(dir) - if err != nil { - t.Fatalf("buildSkillPackage: %v", err) - } - names := zipNames(t, pkg) - - for _, want := range []string{"SKILL.md", "writer-agent.md", "scripts/run.py", "references/doc.md"} { - if !names[want] { - t.Errorf("expected %q in package, got %v", want, names) - } - } - for _, unwanted := range []string{"scratch.tmp", ".env", ".agentspanignore", "node_modules/dep/index.js"} { - if names[unwanted] { - t.Errorf("did not expect %q in package", unwanted) - } - } - - // Manifest entries carry checksum + content type and match the included files. - if len(files) != len(names) { - t.Errorf("manifest has %d files, package has %d", len(files), len(names)) - } - for _, f := range files { - if f.SHA256 == "" || f.ContentType == "" { - t.Errorf("file entry missing checksum/content-type: %+v", f) - } - } -} - -func TestBuildSkillManifestReadsFrontmatter(t *testing.T) { - dir := newSkillFixture(t) - _, files, err := buildSkillPackage(dir) - if err != nil { - t.Fatalf("buildSkillPackage: %v", err) - } - - raw, err := buildSkillManifest(dir, "v1", "openai/gpt-4o", []string{"writer=anthropic/claude"}, files) - if err != nil { - t.Fatalf("buildSkillManifest: %v", err) - } - var m skillManifest - if err := json.Unmarshal(raw, &m); err != nil { - t.Fatalf("unmarshal manifest: %v", err) - } - if m.Name != "summarize" || m.Description != "Summarize text" { - t.Errorf("manifest = %+v", m) - } - if m.Version != "v1" || m.Model != "openai/gpt-4o" || m.AgentModels["writer"] != "anthropic/claude" { - t.Errorf("manifest flags not applied: %+v", m) - } -} - -func TestBuildSkillManifestRequiresName(t *testing.T) { - dir := t.TempDir() - writeFile(t, dir, "SKILL.md", "---\ndescription: no name here\n---\nbody") - if _, err := buildSkillManifest(dir, "", "", nil, nil); err == nil { - t.Fatal("expected an error when SKILL.md has no name") - } -} diff --git a/cmd/skill_run.go b/cmd/skill_run.go deleted file mode 100644 index 9c457d9..0000000 --- a/cmd/skill_run.go +++ /dev/null @@ -1,304 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "context" - "encoding/json" - "fmt" - "os" - "os/signal" - "path/filepath" - "strings" - "syscall" - "time" - - "github.com/spf13/cobra" - - "github.com/conductor-oss/conductor-cli/internal" - "github.com/conductor-oss/conductor-cli/internal/agent" - "github.com/conductor-oss/conductor-cli/internal/skillworker" -) - -// Skill run/serve flag defaults. -const ( - defaultScriptTimeoutSeconds = 300 - defaultScriptOutputLimit = 10 << 20 // 10 MiB - defaultWorkspaceDir = "." - defaultWorkspaceFileLimit = 1 << 20 // 1 MiB -) - -var ( - // run only - skillRunModel string - skillRunAgentModels []string - skillRunParams []string - // run + serve - skillRunVersion string - skillSearchPaths []string - skillScriptTimeout int - skillScriptOutputLimit int - skillWorkspaceDir string - skillNoWorkspace bool - skillFileSystems []string - skillWorkspaceFileLimit int -) - -var skillRunCmd = &cobra.Command{ - Use: "run ", - Short: "Run a local or registered skill and stream its output", - Long: `Run a local skill directory or a server-registered skill by name. The CLI -starts local tool workers (read_skill_file, scripts, workspace tools), starts the -skill agent on the server, and streams its execution. Workers run for the duration -of the execution.`, - Args: cobra.MinimumNArgs(2), - SilenceUsage: true, - RunE: runSkillRun, -} - -var skillServeCmd = &cobra.Command{ - Use: "serve ", - Short: "Start local tool workers for a skill without running it", - Long: `Start the local tool workers for a skill (read_skill_file, scripts, workspace -tools) and block until interrupted. Use this to serve a skill's tools while it is -run elsewhere (e.g. from the UI).`, - Args: cobra.ExactArgs(1), - SilenceUsage: true, - RunE: runSkillServe, -} - -func runSkillRun(cmd *cobra.Command, args []string) error { - if skillRunModel == "" { - return fmt.Errorf("--model is required for skill run") - } - prompt := strings.Join(args[1:], " ") - - agentModels, err := parseAgentModelFlags(skillRunAgentModels) - if err != nil { - return err - } - params, err := parseParamOverrides(skillRunParams) - if err != nil { - return err - } - ws, err := resolveSkillWorkspaceConfig() - if err != nil { - return err - } - - dir, _, err := materializeSkill(cmd.Context(), internal.GetSkillService(), args[0], skillRunVersion) - if err != nil { - return err - } - cfg, local, err := BuildSkillPayload(dir, PayloadOptions{ - Model: skillRunModel, - AgentModels: agentModels, - SearchPaths: skillSearchPaths, - ParamOverrides: params, - }) - if err != nil { - return err - } - cfg.Workspace = ws.WireConfig() - - raw, err := json.Marshal(cfg) - if err != nil { - return fmt.Errorf("encode skill config: %w", err) - } - - // One signal-aware context governs both the workers and the stream; cancelling - // it (Ctrl-C) stops everything. Workers are also cancelled when the execution - // ends normally. - ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) - defer stop() - workerCtx, cancelWorkers := context.WithCancel(ctx) - defer cancelWorkers() - startSkillWorkers(workerCtx, buildSkillWorkerRegistry(cfg, local, ws, scriptOptions(), skillWorkspaceFileLimit)) - - svc := internal.GetAgentService() - exec, err := svc.Run(ctx, agent.RunRequest{Framework: frameworkSkill, Definition: raw, Prompt: prompt}) - if err != nil { - return err - } - fmt.Printf("Skill: %s (Execution: %s)\n\n", exec.AgentName, exec.ID) - - streamErr := svc.StreamExecution(ctx, exec.ID, "", terminalSink{}) - cancelWorkers() - return streamErr -} - -func runSkillServe(cmd *cobra.Command, args []string) error { - ws, err := resolveSkillWorkspaceConfig() - if err != nil { - return err - } - dir, _, err := materializeSkill(cmd.Context(), internal.GetSkillService(), args[0], skillRunVersion) - if err != nil { - return err - } - cfg, local, err := BuildSkillPayload(dir, PayloadOptions{SearchPaths: skillSearchPaths}) - if err != nil { - return err - } - - ctx, stop := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM) - defer stop() - startSkillWorkers(ctx, buildSkillWorkerRegistry(cfg, local, ws, scriptOptions(), skillWorkspaceFileLimit)) - - fmt.Printf("Serving workers for skill %s. Press Ctrl-C to stop.\n", local.SkillName) - <-ctx.Done() - return nil -} - -// scriptOptions builds the script-execution bounds from the shared flags. -func scriptOptions() skillworker.ScriptOptions { - return skillworker.ScriptOptions{ - Timeout: time.Duration(skillScriptTimeout) * time.Second, - OutputLimit: skillScriptOutputLimit, - } -} - -// startSkillWorkers launches one polling worker goroutine per registered task type. -// They run until ctx is cancelled. -func startSkillWorkers(ctx context.Context, registry map[string]skillworker.ToolHandler) { - taskClient := internal.GetTaskClient() - for taskType, handler := range registry { - w := skillworker.NewWorker(skillworker.NewConductorRunner(taskClient)) - go w.Run(ctx, taskType, handler) - } -} - -// buildSkillWorkerRegistry maps every "{skillName}__{tool}" task type the skill (and -// its resolved cross-skills) needs to the handler that serves it locally. -func buildSkillWorkerRegistry(cfg SkillConfig, local LocalContext, ws skillworker.WorkspaceConfig, opts skillworker.ScriptOptions, fileLimit int) map[string]skillworker.ToolHandler { - reg := map[string]skillworker.ToolHandler{} - addSkillWorkerHandlers(reg, cfg, local, ws, opts, fileLimit) - return reg -} - -func addSkillWorkerHandlers(reg map[string]skillworker.ToolHandler, cfg SkillConfig, local LocalContext, ws skillworker.WorkspaceConfig, opts skillworker.ScriptOptions, fileLimit int) { - name := local.SkillName - if name == "" { - return - } - reg[skillworker.TaskType(name, skillworker.ToolReadSkillFile)] = skillworker.NewReadSkillFileHandler(local.SkillDir, cfg.ResourceFiles, local.Sections) - for tool, info := range cfg.Scripts { - scriptPath := filepath.Join(local.SkillDir, scriptsDirName, info.Filename) - reg[skillworker.TaskType(name, tool)] = skillworker.NewScriptHandler(scriptPath, info.Language, ws, opts) - } - if ws.Enabled { - reg[skillworker.TaskType(name, skillworker.ToolListWorkspace)] = skillworker.NewListWorkspaceFilesHandler(ws) - reg[skillworker.TaskType(name, skillworker.ToolReadWorkspaceFile)] = skillworker.NewReadWorkspaceFileHandler(ws, fileLimit) - reg[skillworker.TaskType(name, skillworker.ToolSearchWorkspace)] = skillworker.NewSearchWorkspaceHandler(ws, fileLimit) - reg[skillworker.TaskType(name, skillworker.ToolGitStatus)] = skillworker.NewGitStatusHandler(ws, fileLimit) - reg[skillworker.TaskType(name, skillworker.ToolGitDiff)] = skillworker.NewGitDiffHandler(ws, fileLimit) - } - for refName, refCfg := range cfg.CrossSkillRefs { - if refLocal, ok := local.CrossSkills[refName]; ok { - addSkillWorkerHandlers(reg, refCfg, refLocal, ws, opts, fileLimit) - } - } -} - -// resolveSkillWorkspaceConfig builds the workspace configuration from the shared -// flags. Flag parsing stays in the cmd layer; skillworker validates each root. -func resolveSkillWorkspaceConfig() (skillworker.WorkspaceConfig, error) { - cfg := skillworker.WorkspaceConfig{} - seen := map[string]bool{} - addRoot := func(name, pathValue, kind string) error { - root, err := skillworker.NewWorkspaceRoot(name, expandUserPath(pathValue), kind) - if err != nil { - return err - } - if seen[root.Name] { - return fmt.Errorf("duplicate filesystem root %q", root.Name) - } - seen[root.Name] = true - cfg.Roots = append(cfg.Roots, root) - return nil - } - - if !skillNoWorkspace { - workspacePath := skillWorkspaceDir - if strings.TrimSpace(workspacePath) == "" { - workspacePath = defaultWorkspaceDir - } - if err := addRoot(skillworker.KindWorkspace, workspacePath, skillworker.KindWorkspace); err != nil { - return cfg, err - } - } - for _, spec := range skillFileSystems { - name, pathValue, ok := strings.Cut(spec, "=") - if !ok || strings.TrimSpace(name) == "" || strings.TrimSpace(pathValue) == "" { - return cfg, fmt.Errorf("invalid --filesystem value %q: expected name=path", spec) - } - if err := addRoot(strings.TrimSpace(name), strings.TrimSpace(pathValue), skillworker.KindFilesystem); err != nil { - return cfg, err - } - } - cfg.Enabled = len(cfg.Roots) > 0 - return cfg, nil -} - -// parseParamOverrides parses repeated --param key=value flags into typed values. -func parseParamOverrides(flags []string) (map[string]ParamValue, error) { - if len(flags) == 0 { - return nil, nil - } - result := make(map[string]ParamValue, len(flags)) - for _, f := range flags { - name, value, ok := strings.Cut(f, "=") - if !ok || name == "" { - return nil, fmt.Errorf("invalid --param value %q: expected key=value", f) - } - result[name] = parseParamValue(value) - } - return result, nil -} - -// parseParamValue coerces "true"/"false" to a bool, keeping everything else as a -// string (faithful to the original). -func parseParamValue(value string) ParamValue { - switch strings.ToLower(value) { - case "true": - return rawParamValue(true) - case "false": - return rawParamValue(false) - default: - return rawParamValue(value) - } -} - -func init() { - skillRunCmd.Flags().StringVar(&skillRunModel, "model", "", "Orchestrator and default model (required)") - skillRunCmd.Flags().StringArrayVar(&skillRunAgentModels, "agent-model", nil, "Sub-agent model override (name=model, repeatable)") - skillRunCmd.Flags().StringArrayVar(&skillRunParams, "param", nil, "Skill parameter override (key=value, repeatable)") - addSkillWorkerFlags(skillRunCmd) - - addSkillWorkerFlags(skillServeCmd) - - skillCmd.AddCommand(skillRunCmd, skillServeCmd) -} - -// addSkillWorkerFlags binds the flags shared by run and serve. -func addSkillWorkerFlags(cmd *cobra.Command) { - cmd.Flags().StringArrayVar(&skillSearchPaths, "search-path", nil, "Cross-skill search directory (repeatable)") - cmd.Flags().StringVar(&skillRunVersion, "version", "", "Registered skill version or checksum prefix") - cmd.Flags().IntVar(&skillScriptTimeout, "script-timeout", defaultScriptTimeoutSeconds, "Skill script timeout in seconds") - cmd.Flags().IntVar(&skillScriptOutputLimit, "script-output-limit", defaultScriptOutputLimit, "Maximum bytes captured from skill script output") - cmd.Flags().StringVar(&skillWorkspaceDir, "workspace", defaultWorkspaceDir, "Workspace directory exposed to workspace tools") - cmd.Flags().BoolVar(&skillNoWorkspace, "no-workspace", false, "Disable exposing the current workspace") - cmd.Flags().StringArrayVar(&skillFileSystems, "filesystem", nil, "Additional read-only filesystem root name=path (repeatable)") - cmd.Flags().IntVar(&skillWorkspaceFileLimit, "workspace-file-limit", defaultWorkspaceFileLimit, "Maximum bytes returned by workspace file tools") -} diff --git a/cmd/skill_run_test.go b/cmd/skill_run_test.go deleted file mode 100644 index 3d5496a..0000000 --- a/cmd/skill_run_test.go +++ /dev/null @@ -1,247 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package cmd - -import ( - "archive/zip" - "bytes" - "context" - "crypto/sha256" - "encoding/hex" - "encoding/json" - "os" - "path/filepath" - "testing" - - "github.com/conductor-oss/conductor-cli/internal/skill" - "github.com/conductor-oss/conductor-cli/internal/skillworker" -) - -func TestResolveSkillWorkspaceConfig(t *testing.T) { - wsDir := t.TempDir() - extraDir := t.TempDir() - - skillNoWorkspace = false - skillWorkspaceDir = wsDir - skillFileSystems = []string{"docs=" + extraDir} - t.Cleanup(func() { skillFileSystems = nil }) - - cfg, err := resolveSkillWorkspaceConfig() - if err != nil { - t.Fatalf("resolve: %v", err) - } - if !cfg.Enabled || len(cfg.Roots) != 2 { - t.Fatalf("cfg = %+v", cfg) - } - if _, ok := cfg.Root("workspace"); !ok { - t.Error("missing default workspace root") - } - if _, ok := cfg.Root("docs"); !ok { - t.Error("missing named filesystem root") - } - - // --no-workspace with no filesystems disables the workspace. - skillNoWorkspace = true - skillFileSystems = nil - cfg, err = resolveSkillWorkspaceConfig() - if err != nil { - t.Fatalf("resolve: %v", err) - } - if cfg.Enabled { - t.Errorf("expected disabled workspace, got %+v", cfg) - } - skillNoWorkspace = false - - // Invalid --filesystem spec is rejected. - skillFileSystems = []string{"bad-spec"} - if _, err := resolveSkillWorkspaceConfig(); err == nil { - t.Error("expected an error for a malformed --filesystem value") - } - skillFileSystems = nil -} - -func TestParseParamOverrides(t *testing.T) { - out, err := parseParamOverrides([]string{"tone=terse", "verbose=true", "keep=false"}) - if err != nil { - t.Fatalf("parse: %v", err) - } - if out["tone"].format() != "terse" { - t.Errorf("tone = %q", out["tone"].format()) - } - if out["verbose"].format() != "true" { - t.Errorf("verbose = %q", out["verbose"].format()) - } - // bool values marshal as JSON bools, not strings. - b, _ := json.Marshal(out["verbose"]) - if string(b) != "true" { - t.Errorf("verbose JSON = %s", b) - } - if _, err := parseParamOverrides([]string{"noequals"}); err == nil { - t.Error("expected an error for a param without '='") - } -} - -func TestBuildSkillWorkerRegistry(t *testing.T) { - cfg := SkillConfig{ - ResourceFiles: []string{"notes.txt"}, - Scripts: map[string]ScriptInfo{"greet": {Filename: "greet.sh", Language: skillworker.LangBash}}, - CrossSkillRefs: map[string]SkillConfig{ - "helper": {Scripts: map[string]ScriptInfo{"aid": {Filename: "aid.sh", Language: skillworker.LangBash}}}, - }, - } - local := LocalContext{ - SkillName: "main", - SkillDir: t.TempDir(), - Sections: map[string]string{"intro": "## Intro"}, - CrossSkills: map[string]LocalContext{"helper": {SkillName: "helper", SkillDir: t.TempDir()}}, - } - ws := skillworker.WorkspaceConfig{Enabled: true, Roots: []skillworker.WorkspaceRoot{{Name: "workspace", Path: t.TempDir(), Kind: skillworker.KindWorkspace}}} - - reg := buildSkillWorkerRegistry(cfg, local, ws, skillworker.ScriptOptions{}, 1<<20) - - for _, taskType := range []string{ - "main__read_skill_file", "main__greet", - "main__list_workspace_files", "main__read_workspace_file", - "main__search_workspace", "main__git_status", "main__git_diff", - "helper__read_skill_file", "helper__aid", - } { - if _, ok := reg[taskType]; !ok { - t.Errorf("registry missing task type %q (have %v)", taskType, registryKeys(reg)) - } - } -} - -func TestBuildSkillWorkerRegistryNoWorkspace(t *testing.T) { - cfg := SkillConfig{Scripts: map[string]ScriptInfo{"greet": {Filename: "greet.sh", Language: skillworker.LangBash}}} - local := LocalContext{SkillName: "main", SkillDir: t.TempDir()} - reg := buildSkillWorkerRegistry(cfg, local, skillworker.WorkspaceConfig{}, skillworker.ScriptOptions{}, 1<<20) - - if _, ok := reg["main__list_workspace_files"]; ok { - t.Error("workspace tools should not be registered when the workspace is disabled") - } - if _, ok := reg["main__read_skill_file"]; !ok { - t.Error("read_skill_file should always be registered") - } -} - -// ---- materialize ---- - -type fakeSkillService struct { - detail skill.Detail - pkg []byte - downloads int -} - -func (f *fakeSkillService) List(context.Context, bool) ([]skill.Summary, error) { return nil, nil } -func (f *fakeSkillService) Get(context.Context, string, string) (skill.Detail, error) { - return f.detail, nil -} -func (f *fakeSkillService) DownloadPackage(context.Context, string, string) ([]byte, error) { - f.downloads++ - return f.pkg, nil -} -func (f *fakeSkillService) Register(context.Context, json.RawMessage, []byte) (skill.Detail, error) { - return skill.Detail{}, nil -} -func (f *fakeSkillService) Delete(context.Context, string, string) error { return nil } - -func TestMaterializeLocalDirectory(t *testing.T) { - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: x\n---\n"), 0o644); err != nil { - t.Fatal(err) - } - got, detail, err := materializeSkill(context.Background(), &fakeSkillService{}, dir, "") - if err != nil { - t.Fatalf("materialize local: %v", err) - } - if got != dir || detail != nil { - t.Errorf("local dir should be used as-is: got=%q detail=%v", got, detail) - } -} - -func TestMaterializeRegisteredSkillCaches(t *testing.T) { - t.Setenv("HOME", t.TempDir()) // isolate the config-home cache - - pkg := skillZip(t, map[string]string{"SKILL.md": "---\nname: reg\n---\nbody"}) - sum := sha256.Sum256(pkg) - svc := &fakeSkillService{ - detail: skill.Detail{Name: "reg", Version: "1", Checksum: hex.EncodeToString(sum[:])}, - pkg: pkg, - } - - dir, detail, err := materializeSkill(context.Background(), svc, "reg", "") - if err != nil { - t.Fatalf("materialize registered: %v", err) - } - if detail == nil || detail.Name != "reg" { - t.Fatalf("detail = %+v", detail) - } - if _, err := os.Stat(filepath.Join(dir, "SKILL.md")); err != nil { - t.Errorf("cached skill missing SKILL.md: %v", err) - } - if svc.downloads != 1 { - t.Errorf("expected 1 download, got %d", svc.downloads) - } - - // Second call hits the cache — no additional download. - dir2, _, err := materializeSkill(context.Background(), svc, "reg", "") - if err != nil { - t.Fatalf("materialize (cache hit): %v", err) - } - if dir2 != dir { - t.Errorf("cache-hit dir = %q, want %q", dir2, dir) - } - if svc.downloads != 1 { - t.Errorf("cache hit re-downloaded: downloads = %d", svc.downloads) - } -} - -func TestMaterializeChecksumMismatch(t *testing.T) { - t.Setenv("HOME", t.TempDir()) - svc := &fakeSkillService{ - detail: skill.Detail{Name: "reg", Version: "1", Checksum: "deadbeef"}, - pkg: skillZip(t, map[string]string{"SKILL.md": "---\nname: reg\n---\n"}), - } - if _, _, err := materializeSkill(context.Background(), svc, "reg", ""); err == nil { - t.Fatal("expected a checksum mismatch error") - } -} - -// skillZip builds an in-memory skill package zip from path→content. -func skillZip(t *testing.T, files map[string]string) []byte { - t.Helper() - var buf bytes.Buffer - zw := zip.NewWriter(&buf) - for name, content := range files { - w, err := zw.Create(name) - if err != nil { - t.Fatal(err) - } - if _, err := w.Write([]byte(content)); err != nil { - t.Fatal(err) - } - } - if err := zw.Close(); err != nil { - t.Fatal(err) - } - return buf.Bytes() -} - -func registryKeys(reg map[string]skillworker.ToolHandler) []string { - keys := make([]string, 0, len(reg)) - for k := range reg { - keys = append(keys, k) - } - return keys -} From 395e3ab60f701ac0305fa0ef9632488d82bbd7d7 Mon Sep 17 00:00:00 2001 From: Kowser Date: Tue, 4 Aug 2026 19:41:00 -0700 Subject: [PATCH 2/5] chore: delete internal/skillworker --- internal/skillworker/handlers.go | 772 -------------------------- internal/skillworker/handlers_test.go | 288 ---------- internal/skillworker/runner.go | 105 ---- internal/skillworker/runner_test.go | 69 --- internal/skillworker/worker.go | 124 ----- internal/skillworker/worker_test.go | 154 ----- internal/skillworker/workspace.go | 219 -------- 7 files changed, 1731 deletions(-) delete mode 100644 internal/skillworker/handlers.go delete mode 100644 internal/skillworker/handlers_test.go delete mode 100644 internal/skillworker/runner.go delete mode 100644 internal/skillworker/runner_test.go delete mode 100644 internal/skillworker/worker.go delete mode 100644 internal/skillworker/worker_test.go delete mode 100644 internal/skillworker/workspace.go diff --git a/internal/skillworker/handlers.go b/internal/skillworker/handlers.go deleted file mode 100644 index 12c9b88..0000000 --- a/internal/skillworker/handlers.go +++ /dev/null @@ -1,772 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "fmt" - "os" - "os/exec" - pathpkg "path" - "path/filepath" - "regexp" - "runtime" - "sort" - "strconv" - "strings" - "time" -) - -// Tool names (the "{tool}" half of a "{skillName}__{tool}" task type). Shared with -// the cmd registry that maps them to these handlers. -const ( - ToolReadSkillFile = "read_skill_file" - ToolListWorkspace = "list_workspace_files" - ToolReadWorkspaceFile = "read_workspace_file" - ToolSearchWorkspace = "search_workspace" - ToolGitStatus = "git_status" - ToolGitDiff = "git_diff" -) - -// Script languages — the vocabulary produced by the payload builder's extension -// table and consumed by executeScript, kept in one place to avoid drift. -const ( - LangPython = "python" - LangNode = "node" - LangRuby = "ruby" - LangGo = "go" - LangBatch = "batch" - LangBash = "bash" -) - -// Script execution defaults and the script-facing environment variables. The -// AGENTSPAN_* names are a stable skill-script contract (a script reads them to find -// its skill dir and workspace roots), so they are kept verbatim as named constants. -const ( - defaultScriptTimeout = 300 * time.Second - defaultScriptOutputLimit = 10 << 20 // 10 MiB - - envSkillDir = "AGENTSPAN_SKILL_DIR" - envWorkspaceDir = "AGENTSPAN_WORKSPACE_DIR" - envFilesystemRootPrefix = "AGENTSPAN_FILESYSTEM_ROOT_" -) - -// Workspace tool limits and the git command timeout. -const ( - listDefaultLimit = 500 - listMaxLimit = 5000 - readMaxLimit = 5 << 20 // 5 MiB - searchDefaultLimit = 100 - searchMaxLimit = 1000 - searchLineMax = 500 - gitCommandTimeout = 30 * time.Second -) - -// skillSectionPrefix marks a read_skill_file request for a pre-split SKILL.md -// section ("skill_section:{slug}") rather than a resource file. -const skillSectionPrefix = "skill_section:" - -// toolFunc adapts a function to ToolHandler. -type toolFunc func(ctx context.Context, input json.RawMessage) (json.RawMessage, error) - -func (f toolFunc) Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) { - return f(ctx, input) -} - -// ---- read_skill_file ---- - -type readSkillFileInput struct { - Path string `json:"path"` -} - -// NewReadSkillFileHandler serves skill resource files and pre-split SKILL.md -// sections. Only paths in resourceFiles (and "skill_section:{slug}" for each known -// section) are allowed; expected failures (unknown/unreadable path) are returned as -// an "ERROR: ..." result string so the agent can see them, matching the original. -func NewReadSkillFileHandler(skillDir string, resourceFiles []string, sections map[string]string) ToolHandler { - allowed := make(map[string]bool, len(resourceFiles)+len(sections)) - for _, f := range resourceFiles { - allowed[f] = true - } - for name := range sections { - allowed[skillSectionPrefix+name] = true - } - return toolFunc(func(_ context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in readSkillFileInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - if in.Path == "" { - return nil, fmt.Errorf("missing 'path' parameter") - } - path := normalizeSkillResourcePath(in.Path) - if !allowed[path] { - return jsonString(fmt.Sprintf("ERROR: '%s' not found. Available: %v", path, sortedKeys(allowed))) - } - if strings.HasPrefix(path, skillSectionPrefix) { - name := strings.TrimPrefix(path, skillSectionPrefix) - if section, ok := sections[name]; ok { - return jsonString(section) - } - return jsonString(fmt.Sprintf("ERROR: section '%s' not found", name)) - } - fullPath, err := safeSkillPath(skillDir, path) - if err != nil { - return jsonString(fmt.Sprintf("ERROR: %v", err)) - } - data, err := os.ReadFile(fullPath) - if err != nil { - return jsonString(fmt.Sprintf("ERROR: failed to read '%s': %v", path, err)) - } - return jsonString(string(data)) - }) -} - -func normalizeSkillResourcePath(path string) string { - if strings.HasPrefix(path, skillSectionPrefix) { - return path - } - return pathpkg.Clean(strings.ReplaceAll(path, "\\", "/")) -} - -// safeSkillPath joins relPath onto absSkillDir, rejecting escapes (skill-dir -// boundary, symlink-resolved). -func safeSkillPath(absSkillDir, relPath string) (string, error) { - cleanRel := filepath.Clean(relPath) - if filepath.IsAbs(cleanRel) || cleanRel == ".." || strings.HasPrefix(cleanRel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("'%s' is outside the skill directory", relPath) - } - target := filepath.Join(absSkillDir, cleanRel) - resolvedTarget, err := filepath.EvalSymlinks(target) - if err != nil { - return target, nil // path may not exist yet; join is already contained - } - resolvedDir, err := filepath.EvalSymlinks(absSkillDir) - if err != nil { - resolvedDir = absSkillDir - } - if rel, err := filepath.Rel(resolvedDir, resolvedTarget); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("'%s' is outside the skill directory", relPath) - } - return resolvedTarget, nil -} - -// ---- scripts ---- - -type scriptInput struct { - Command string `json:"command"` -} - -// ScriptOptions bounds a script execution (zero values fall back to the defaults). -type ScriptOptions struct { - Timeout time.Duration - OutputLimit int -} - -// NewScriptHandler runs one skill script. Script failure is returned as an error -// whose message includes the captured output, so the failed task's reason carries -// the diagnostic output. -func NewScriptHandler(scriptPath, language string, ws WorkspaceConfig, opts ScriptOptions) ToolHandler { - return toolFunc(func(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in scriptInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - out, err := executeScript(ctx, scriptPath, language, in.Command, ws, opts) - if err != nil { - return nil, err - } - return jsonString(out) - }) -} - -func executeScript(ctx context.Context, scriptPath, language, command string, ws WorkspaceConfig, opts ScriptOptions) (string, error) { - timeout := opts.Timeout - if timeout <= 0 { - timeout = defaultScriptTimeout - } - outputLimit := opts.OutputLimit - if outputLimit <= 0 { - outputLimit = defaultScriptOutputLimit - } - - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() - - cmd := buildScriptCmd(ctx, language, scriptPath, command) - if skillRoot := skillRootFromScriptPath(scriptPath); skillRoot != "" { - cmd.Dir = skillRoot - cmd.Env = scriptEnv(skillRoot, ws) - } - - output := &limitedOutputBuffer{limit: outputLimit} - cmd.Stdout = output - cmd.Stderr = output - err := cmd.Run() - out := output.String() - if err != nil { - if ctx.Err() == context.DeadlineExceeded { - return out, fmt.Errorf("script timed out after %s\n%s", timeout, out) - } - return out, fmt.Errorf("script failed: %w\n%s", err, out) - } - return out, nil -} - -func buildScriptCmd(ctx context.Context, language, scriptPath, command string) *exec.Cmd { - args := splitCommandArgs(command) - switch language { - case LangPython: - py := "python3" - if _, err := exec.LookPath("python3"); err != nil { - py = "python" // Windows ships python, not python3 - } - return exec.CommandContext(ctx, py, append([]string{scriptPath}, args...)...) - case LangNode: - return exec.CommandContext(ctx, "node", append([]string{scriptPath}, args...)...) - case LangRuby: - return exec.CommandContext(ctx, "ruby", append([]string{scriptPath}, args...)...) - case LangGo: - return exec.CommandContext(ctx, "go", append([]string{"run", scriptPath}, args...)...) - case LangBatch: - return exec.CommandContext(ctx, "cmd", append([]string{"/c", scriptPath}, args...)...) - default: // LangBash / shell - if runtime.GOOS != "windows" { - return exec.CommandContext(ctx, "bash", append([]string{scriptPath}, args...)...) - } - if _, err := exec.LookPath("bash"); err == nil { - return exec.CommandContext(ctx, "bash", append([]string{scriptPath}, args...)...) - } - return exec.CommandContext(ctx, "cmd", append([]string{"/c", scriptPath}, args...)...) - } -} - -// skillRootFromScriptPath returns the skill directory for a "…/scripts/foo" path, -// or "" when the script is not under a scripts/ directory. -func skillRootFromScriptPath(scriptPath string) string { - scriptsDir := filepath.Dir(scriptPath) - if filepath.Base(scriptsDir) != scriptsDirName { - return "" - } - return filepath.Dir(scriptsDir) -} - -// scriptsDirName mirrors the payload builder's scripts directory name. -const scriptsDirName = "scripts" - -func scriptEnv(skillRoot string, ws WorkspaceConfig) []string { - env := append(os.Environ(), envSkillDir+"="+skillRoot) - if wsRoot, ok := ws.Root(workspaceRootName); ok { - env = append(env, envWorkspaceDir+"="+wsRoot.Path) - } - for _, root := range ws.Roots { - env = append(env, envFilesystemRootPrefix+filesystemEnvName(root.Name)+"="+root.Path) - } - return env -} - -var filesystemEnvReplacer = regexp.MustCompile(`[^A-Za-z0-9]+`) - -func filesystemEnvName(name string) string { - return strings.ToUpper(strings.Trim(filesystemEnvReplacer.ReplaceAllString(name, "_"), "_")) -} - -// splitCommandArgs splits a command string into arguments, honoring single/double -// quotes and backslash escapes. -func splitCommandArgs(command string) []string { - var args []string - var current strings.Builder - inSingle, inDouble, escaped := false, false, false - for _, r := range command { - switch { - case escaped: - current.WriteRune(r) - escaped = false - case r == '\\' && !inSingle: - escaped = true - case r == '\'' && !inDouble: - inSingle = !inSingle - case r == '"' && !inSingle: - inDouble = !inDouble - case (r == ' ' || r == '\t' || r == '\n') && !inSingle && !inDouble: - if current.Len() > 0 { - args = append(args, current.String()) - current.Reset() - } - default: - current.WriteRune(r) - } - } - if current.Len() > 0 { - args = append(args, current.String()) - } - return args -} - -type limitedOutputBuffer struct { - buf strings.Builder - limit int - truncated bool -} - -func (b *limitedOutputBuffer) Write(p []byte) (int, error) { - if b.limit <= 0 { - return len(p), nil - } - remaining := b.limit - b.buf.Len() - if remaining <= 0 { - b.truncated = true - return len(p), nil - } - if len(p) > remaining { - b.truncated = true - _, _ = b.buf.Write(p[:remaining]) - return len(p), nil - } - _, _ = b.buf.Write(p) - return len(p), nil -} - -func (b *limitedOutputBuffer) String() string { - out := b.buf.String() - if b.truncated { - out += fmt.Sprintf("\n[output truncated after %d bytes]", b.limit) - } - return out -} - -// ---- workspace tools ---- - -type listInput struct { - Root string `json:"root"` - Path string `json:"path"` - Glob string `json:"glob"` - Limit flexInt `json:"limit"` -} - -type listResult struct { - Root string `json:"root"` - Path string `json:"path"` - Files []string `json:"files"` - Truncated bool `json:"truncated"` -} - -// NewListWorkspaceFilesHandler lists files under a workspace root, filtered by glob. -func NewListWorkspaceFilesHandler(ws WorkspaceConfig) ToolHandler { - return toolFunc(func(_ context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in listInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - root, err := workspaceRootFromInput(ws, in.Root) - if err != nil { - return nil, err - } - res, err := listWorkspaceFiles(root, in.Path, in.Glob, applyLimit(int(in.Limit), listDefaultLimit, listMaxLimit)) - if err != nil { - return nil, err - } - return json.Marshal(res) - }) -} - -func listWorkspaceFiles(root WorkspaceRoot, pathValue, pattern string, limit int) (listResult, error) { - rootPath := resolvedWorkspaceRootPath(root.Path) - startPath, err := safeWorkspacePath(root.Path, defaultString(pathValue, ".")) - if err != nil { - return listResult{}, err - } - info, err := os.Stat(startPath) - if err != nil { - return listResult{}, err - } - if !info.IsDir() { - return listResult{}, fmt.Errorf("path is not a directory: %s", pathValue) - } - - files := []string{} - truncated := false - err = filepath.WalkDir(startPath, func(current string, entry os.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr - } - if current == startPath { - return nil - } - if entry.IsDir() && shouldSkipWorkspaceDir(entry.Name()) { - return filepath.SkipDir - } - if entry.IsDir() { - return nil - } - rel, err := filepath.Rel(rootPath, current) - if err != nil { - return err - } - rel = filepath.ToSlash(rel) - if !matchesWorkspacePattern(pattern, rel) { - return nil - } - files = append(files, rel) - if limit > 0 && len(files) >= limit { - truncated = true - return filepath.SkipAll - } - return nil - }) - if err != nil { - return listResult{}, err - } - return listResult{Root: root.Name, Path: defaultString(pathValue, "."), Files: files, Truncated: truncated}, nil -} - -type readInput struct { - Root string `json:"root"` - Path string `json:"path"` - Limit flexInt `json:"limit"` -} - -type readResult struct { - Root string `json:"root"` - Path string `json:"path"` - Content string `json:"content"` - Truncated bool `json:"truncated"` -} - -// NewReadWorkspaceFileHandler reads one workspace file (bounded by fileLimit). -func NewReadWorkspaceFileHandler(ws WorkspaceConfig, fileLimit int) ToolHandler { - return toolFunc(func(_ context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in readInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - root, err := workspaceRootFromInput(ws, in.Root) - if err != nil { - return nil, err - } - if strings.TrimSpace(in.Path) == "" { - return nil, fmt.Errorf("missing 'path' parameter") - } - res, err := readWorkspaceFile(root, in.Path, applyLimit(int(in.Limit), fileLimit, readMaxLimit)) - if err != nil { - return nil, err - } - return json.Marshal(res) - }) -} - -func readWorkspaceFile(root WorkspaceRoot, pathValue string, limit int) (readResult, error) { - rootPath := resolvedWorkspaceRootPath(root.Path) - fullPath, err := safeWorkspacePath(root.Path, pathValue) - if err != nil { - return readResult{}, err - } - info, err := os.Stat(fullPath) - if err != nil { - return readResult{}, err - } - if info.IsDir() { - return readResult{}, fmt.Errorf("path is a directory: %s", pathValue) - } - content, truncated, err := readLimitedTextFile(fullPath, limit) - if err != nil { - return readResult{}, err - } - rel, _ := filepath.Rel(rootPath, fullPath) - return readResult{Root: root.Name, Path: filepath.ToSlash(rel), Content: content, Truncated: truncated}, nil -} - -type searchInput struct { - Root string `json:"root"` - Path string `json:"path"` - Glob string `json:"glob"` - Query string `json:"query"` - IgnoreCase *flexBool `json:"ignoreCase"` - Limit flexInt `json:"limit"` -} - -type searchMatch struct { - Path string `json:"path"` - Line int `json:"line"` - Text string `json:"text"` -} - -type searchResult struct { - Root string `json:"root"` - Query string `json:"query"` - Matches []searchMatch `json:"matches"` - Truncated bool `json:"truncated"` -} - -// NewSearchWorkspaceHandler does a substring search across a workspace root. -func NewSearchWorkspaceHandler(ws WorkspaceConfig, fileLimit int) ToolHandler { - return toolFunc(func(_ context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in searchInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - root, err := workspaceRootFromInput(ws, in.Root) - if err != nil { - return nil, err - } - if in.Query == "" { - return nil, fmt.Errorf("missing 'query' parameter") - } - res, err := searchWorkspace(root, in.Path, in.Glob, in.Query, - flexBoolValue(in.IgnoreCase, true), applyLimit(int(in.Limit), searchDefaultLimit, searchMaxLimit), fileLimit) - if err != nil { - return nil, err - } - return json.Marshal(res) - }) -} - -func searchWorkspace(root WorkspaceRoot, pathValue, pattern, query string, ignoreCase bool, limit, fileLimit int) (searchResult, error) { - rootPath := resolvedWorkspaceRootPath(root.Path) - startPath, err := safeWorkspacePath(root.Path, defaultString(pathValue, ".")) - if err != nil { - return searchResult{}, err - } - info, err := os.Stat(startPath) - if err != nil { - return searchResult{}, err - } - if !info.IsDir() { - return searchResult{}, fmt.Errorf("path is not a directory: %s", pathValue) - } - - needle := query - if ignoreCase { - needle = strings.ToLower(needle) - } - matches := []searchMatch{} - truncated := false - err = filepath.WalkDir(startPath, func(current string, entry os.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr - } - if current == startPath { - return nil - } - if entry.IsDir() && shouldSkipWorkspaceDir(entry.Name()) { - return filepath.SkipDir - } - if entry.IsDir() { - return nil - } - rel, err := filepath.Rel(rootPath, current) - if err != nil { - return err - } - rel = filepath.ToSlash(rel) - if !matchesWorkspacePattern(pattern, rel) { - return nil - } - content, _, err := readLimitedTextFile(current, fileLimit) - if err != nil || strings.Contains(content, "\x00") { - return nil - } - for i, line := range strings.Split(content, "\n") { - haystack := line - if ignoreCase { - haystack = strings.ToLower(haystack) - } - if strings.Contains(haystack, needle) { - matches = append(matches, searchMatch{Path: rel, Line: i + 1, Text: trimLongLine(line, searchLineMax)}) - if limit > 0 && len(matches) >= limit { - truncated = true - return filepath.SkipAll - } - } - } - return nil - }) - if err != nil { - return searchResult{}, err - } - return searchResult{Root: root.Name, Query: query, Matches: matches, Truncated: truncated}, nil -} - -type gitStatusInput struct { - Root string `json:"root"` -} - -type gitDiffInput struct { - Root string `json:"root"` - Base string `json:"base"` - Path string `json:"path"` - Staged flexBool `json:"staged"` -} - -type gitResult struct { - Root string `json:"root"` - Output string `json:"output"` -} - -// NewGitStatusHandler runs "git status --short" on a workspace root. -func NewGitStatusHandler(ws WorkspaceConfig, fileLimit int) ToolHandler { - return toolFunc(func(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in gitStatusInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - root, err := workspaceRootFromInput(ws, in.Root) - if err != nil { - return nil, err - } - res, err := runGitWorkspaceCommand(ctx, root, []string{"status", "--short"}, fileLimit) - if err != nil { - return nil, err - } - return json.Marshal(res) - }) -} - -// NewGitDiffHandler runs "git diff" (optionally staged / against a base / for a -// path) on a workspace root. -func NewGitDiffHandler(ws WorkspaceConfig, fileLimit int) ToolHandler { - return toolFunc(func(ctx context.Context, raw json.RawMessage) (json.RawMessage, error) { - var in gitDiffInput - if err := json.Unmarshal(raw, &in); err != nil { - return nil, fmt.Errorf("decode input: %w", err) - } - root, err := workspaceRootFromInput(ws, in.Root) - if err != nil { - return nil, err - } - args := []string{"diff", "--no-ext-diff", "--color=never"} - if bool(in.Staged) { - args = append(args, "--cached") - } - if base := strings.TrimSpace(in.Base); base != "" { - args = append(args, base) - } - if pathValue := strings.TrimSpace(in.Path); pathValue != "" { - fullPath, err := safeWorkspacePath(root.Path, pathValue) - if err != nil { - return nil, err - } - rel, err := filepath.Rel(resolvedWorkspaceRootPath(root.Path), fullPath) - if err != nil { - return nil, err - } - args = append(args, "--", filepath.ToSlash(rel)) - } - res, err := runGitWorkspaceCommand(ctx, root, args, fileLimit) - if err != nil { - return nil, err - } - return json.Marshal(res) - }) -} - -func runGitWorkspaceCommand(ctx context.Context, root WorkspaceRoot, args []string, limit int) (gitResult, error) { - ctx, cancel := context.WithTimeout(ctx, gitCommandTimeout) - defer cancel() - - cmd := exec.CommandContext(ctx, "git", append([]string{"-C", root.Path}, args...)...) - output := &limitedOutputBuffer{limit: limit} - cmd.Stdout = output - cmd.Stderr = output - err := cmd.Run() - out := output.String() - if ctx.Err() == context.DeadlineExceeded { - return gitResult{}, fmt.Errorf("git command timed out after %s\n%s", gitCommandTimeout, out) - } - if err != nil { - return gitResult{}, fmt.Errorf("git command failed: %w\n%s", err, out) - } - return gitResult{Root: root.Name, Output: out}, nil -} - -func workspaceRootFromInput(ws WorkspaceConfig, rootName string) (WorkspaceRoot, error) { - if root, ok := ws.Root(rootName); ok { - return root, nil - } - names := make([]string, 0, len(ws.Roots)) - for _, r := range ws.Roots { - names = append(names, r.Name) - } - return WorkspaceRoot{}, fmt.Errorf("unknown filesystem root %q; available: %s", rootName, strings.Join(names, ", ")) -} - -// ---- shared helpers ---- - -// jsonString marshals s as a JSON string (the result form for read_skill_file and -// script handlers). -func jsonString(s string) (json.RawMessage, error) { - b, err := json.Marshal(s) - return b, err -} - -// applyLimit clamps a caller-supplied limit: non-positive falls back to def, and a -// positive max caps the value (mirrors the original intInput semantics). -func applyLimit(value, def, max int) int { - if value <= 0 { - value = def - } - if max > 0 && value > max { - return max - } - return value -} - -func sortedKeys(m map[string]bool) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - return keys -} - -// flexInt decodes a JSON number or numeric string; anything else (including null) -// decodes to 0, which callers treat as "unset" and replace with a default. -type flexInt int - -func (n *flexInt) UnmarshalJSON(b []byte) error { - s := strings.Trim(strings.TrimSpace(string(b)), `"`) - if s == "" || s == "null" { - *n = 0 - return nil - } - if v, err := strconv.Atoi(s); err == nil { - *n = flexInt(v) - return nil - } - if f, err := strconv.ParseFloat(s, 64); err == nil { - *n = flexInt(int(f)) - return nil - } - *n = 0 - return nil -} - -// flexBool decodes a JSON bool or a "true"/"false" string; anything else is false. -type flexBool bool - -func (b *flexBool) UnmarshalJSON(data []byte) error { - s := strings.Trim(strings.TrimSpace(string(data)), `"`) - *b = flexBool(s == "true") - return nil -} - -// flexBoolValue returns p's value, or def when p is nil (absent). -func flexBoolValue(p *flexBool, def bool) bool { - if p == nil { - return def - } - return bool(*p) -} diff --git a/internal/skillworker/handlers_test.go b/internal/skillworker/handlers_test.go deleted file mode 100644 index 15a1e2c..0000000 --- a/internal/skillworker/handlers_test.go +++ /dev/null @@ -1,288 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "os" - "path/filepath" - "runtime" - "strings" - "testing" - "time" -) - -func mustWrite(t *testing.T, dir, rel, content string) { - t.Helper() - full := filepath.Join(dir, filepath.FromSlash(rel)) - if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(full, []byte(content), 0o644); err != nil { - t.Fatal(err) - } -} - -// handle is a test helper that invokes a handler and returns the raw output. -func handle(t *testing.T, h ToolHandler, input string) (json.RawMessage, error) { - t.Helper() - return h.Handle(context.Background(), json.RawMessage(input)) -} - -// ---- read_skill_file ---- - -func TestReadSkillFileServesResourceAndSection(t *testing.T) { - dir := t.TempDir() - mustWrite(t, dir, "references/guide.md", "the guide") - h := NewReadSkillFileHandler(dir, []string{"references/guide.md"}, map[string]string{"intro": "## Intro\nbody"}) - - out, err := handle(t, h, `{"path":"references/guide.md"}`) - if err != nil { - t.Fatalf("read resource: %v", err) - } - if unquote(t, out) != "the guide" { - t.Errorf("resource body = %s", out) - } - - out, err = handle(t, h, `{"path":"skill_section:intro"}`) - if err != nil { - t.Fatalf("read section: %v", err) - } - if unquote(t, out) != "## Intro\nbody" { - t.Errorf("section body = %s", out) - } -} - -func TestReadSkillFileRejectsUnknownAndTraversal(t *testing.T) { - dir := t.TempDir() - mustWrite(t, dir, "SKILL.md", "secret") - h := NewReadSkillFileHandler(dir, []string{"notes.txt"}, nil) - - // Unknown path → soft ERROR result (task completes), not a hard failure. - out, err := handle(t, h, `{"path":"../../etc/passwd"}`) - if err != nil { - t.Fatalf("unexpected hard error: %v", err) - } - if !strings.HasPrefix(unquote(t, out), "ERROR:") { - t.Errorf("expected ERROR result, got %s", out) - } - - // Missing path → hard error (task fails). - if _, err := handle(t, h, `{}`); err == nil { - t.Error("expected hard error for missing path") - } -} - -// TestSafeSkillPathBlocksEscape directly exercises the skill-dir boundary. -func TestSafeSkillPathBlocksEscape(t *testing.T) { - dir := t.TempDir() - if _, err := safeSkillPath(dir, "../outside"); err == nil { - t.Error("expected traversal to be rejected") - } - mustWrite(t, dir, "ok.txt", "x") - if _, err := safeSkillPath(dir, "ok.txt"); err != nil { - t.Errorf("legitimate path rejected: %v", err) - } -} - -// ---- workspace: path safety ---- - -func TestSafeWorkspacePathTraversalAndSymlink(t *testing.T) { - root := t.TempDir() - mustWrite(t, root, "inside.txt", "x") - if _, err := safeWorkspacePath(root, "inside.txt"); err != nil { - t.Errorf("inside path rejected: %v", err) - } - if _, err := safeWorkspacePath(root, "../escape"); err == nil { - t.Error("expected ../escape to be rejected") - } - if _, err := safeWorkspacePath(root, "/etc/passwd"); err == nil { - t.Error("expected absolute path to be rejected") - } - - // A symlink pointing outside the root must be rejected. - outside := t.TempDir() - mustWrite(t, outside, "secret.txt", "top secret") - link := filepath.Join(root, "link") - if err := os.Symlink(filepath.Join(outside, "secret.txt"), link); err != nil { - t.Skipf("symlink unsupported: %v", err) - } - if _, err := safeWorkspacePath(root, "link"); err == nil { - t.Error("expected symlink escaping the root to be rejected") - } -} - -// ---- workspace: list / read / search ---- - -func TestWorkspaceListReadSearch(t *testing.T) { - root := t.TempDir() - mustWrite(t, root, "a.txt", "hello world\nsecond line") - mustWrite(t, root, "sub/b.md", "another file with hello") - mustWrite(t, root, "node_modules/skip.js", "should be skipped") - ws := WorkspaceConfig{Enabled: true, Roots: []WorkspaceRoot{{Name: "workspace", Path: root, Kind: KindWorkspace}}} - - // list - out, err := handle(t, NewListWorkspaceFilesHandler(ws), `{}`) - if err != nil { - t.Fatalf("list: %v", err) - } - var lr listResult - mustUnmarshal(t, out, &lr) - if !containsStr(lr.Files, "a.txt") || !containsStr(lr.Files, "sub/b.md") { - t.Errorf("list missing files: %v", lr.Files) - } - if containsStr(lr.Files, "node_modules/skip.js") { - t.Errorf("list should skip node_modules: %v", lr.Files) - } - - // read - out, err = handle(t, NewReadWorkspaceFileHandler(ws, 1<<20), `{"path":"a.txt"}`) - if err != nil { - t.Fatalf("read: %v", err) - } - var rr readResult - mustUnmarshal(t, out, &rr) - if !strings.Contains(rr.Content, "hello world") { - t.Errorf("read content = %q", rr.Content) - } - - // search (case-insensitive default) finds matches in both files - out, err = handle(t, NewSearchWorkspaceHandler(ws, 1<<20), `{"query":"HELLO"}`) - if err != nil { - t.Fatalf("search: %v", err) - } - var sr searchResult - mustUnmarshal(t, out, &sr) - if len(sr.Matches) < 2 { - t.Errorf("expected >=2 matches, got %d: %+v", len(sr.Matches), sr.Matches) - } -} - -func TestWorkspaceUnknownRoot(t *testing.T) { - ws := WorkspaceConfig{Enabled: true, Roots: []WorkspaceRoot{{Name: "workspace", Path: t.TempDir(), Kind: KindWorkspace}}} - if _, err := handle(t, NewReadWorkspaceFileHandler(ws, 1<<20), `{"root":"nope","path":"x"}`); err == nil { - t.Error("expected unknown-root error") - } -} - -// ---- scripts ---- - -func TestScriptHandlerRunsAndLimitsOutput(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("shell script test is POSIX-only") - } - dir := t.TempDir() - mustWrite(t, dir, "SKILL.md", "---\nname: s\n---\nbody") - mustWrite(t, dir, "scripts/echo.sh", "#!/bin/bash\necho hello-from-script") - scriptPath := filepath.Join(dir, "scripts", "echo.sh") - - out, err := handle(t, NewScriptHandler(scriptPath, LangBash, WorkspaceConfig{}, ScriptOptions{}), `{"command":""}`) - if err != nil { - t.Fatalf("script: %v", err) - } - if !strings.Contains(unquote(t, out), "hello-from-script") { - t.Errorf("script output = %s", out) - } - - // Output limit truncates. - limited := NewScriptHandler(scriptPath, LangBash, WorkspaceConfig{}, ScriptOptions{OutputLimit: 4}) - out, err = handle(t, limited, `{"command":""}`) - if err != nil { - t.Fatalf("script (limited): %v", err) - } - if !strings.Contains(unquote(t, out), "output truncated") { - t.Errorf("expected truncation marker, got %s", out) - } -} - -func TestScriptHandlerTimeoutFails(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("shell script test is POSIX-only") - } - dir := t.TempDir() - mustWrite(t, dir, "scripts/sleep.sh", "#!/bin/bash\nsleep 5") - scriptPath := filepath.Join(dir, "scripts", "sleep.sh") - - _, err := handle(t, NewScriptHandler(scriptPath, LangBash, WorkspaceConfig{}, ScriptOptions{Timeout: 100 * time.Millisecond}), `{"command":""}`) - if err == nil { - t.Fatal("expected a timeout error") - } - if !strings.Contains(err.Error(), "timed out") { - t.Errorf("error = %v, want a timeout", err) - } -} - -// ---- helpers ---- - -func TestFlexIntAndBoolDecode(t *testing.T) { - var li listInput - if err := json.Unmarshal([]byte(`{"limit":"250"}`), &li); err != nil { - t.Fatal(err) - } - if int(li.Limit) != 250 { - t.Errorf("flexInt string = %d", li.Limit) - } - if err := json.Unmarshal([]byte(`{"limit":42}`), &li); err != nil { - t.Fatal(err) - } - if int(li.Limit) != 42 { - t.Errorf("flexInt number = %d", li.Limit) - } - - var gd gitDiffInput - if err := json.Unmarshal([]byte(`{"staged":"true"}`), &gd); err != nil { - t.Fatal(err) - } - if !bool(gd.Staged) { - t.Error("flexBool string 'true' should decode true") - } -} - -func TestApplyLimit(t *testing.T) { - if got := applyLimit(0, 500, 5000); got != 500 { - t.Errorf("default = %d", got) - } - if got := applyLimit(10000, 500, 5000); got != 5000 { - t.Errorf("cap = %d", got) - } - if got := applyLimit(300, 500, 5000); got != 300 { - t.Errorf("passthrough = %d", got) - } -} - -func unquote(t *testing.T, raw json.RawMessage) string { - t.Helper() - var s string - if err := json.Unmarshal(raw, &s); err != nil { - t.Fatalf("output not a JSON string: %s", raw) - } - return s -} - -func mustUnmarshal(t *testing.T, raw json.RawMessage, v any) { - t.Helper() - if err := json.Unmarshal(raw, v); err != nil { - t.Fatalf("unmarshal %s: %v", raw, err) - } -} - -func containsStr(xs []string, want string) bool { - for _, x := range xs { - if x == want { - return true - } - } - return false -} diff --git a/internal/skillworker/runner.go b/internal/skillworker/runner.go deleted file mode 100644 index 6ae99b8..0000000 --- a/internal/skillworker/runner.go +++ /dev/null @@ -1,105 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "fmt" - - "github.com/antihax/optional" - "github.com/conductor-sdk/conductor-go/sdk/client" - "github.com/conductor-sdk/conductor-go/sdk/model" -) - -// Poll tuning: one task per poll, with a short server-side long-poll wait. The -// worker loop adds its own backoff on top (see pollBackoff). -const ( - pollBatchSize = 1 // tasks requested per poll - pollTimeoutMs = 100 // server-side long-poll wait, milliseconds -) - -// conductorRunner adapts Conductor's TaskResourceApiService to TaskRunner. It is the -// ONLY place model.* and *client.TaskResourceApiService appear in this package — -// the worker loop and the handlers see only skillworker.Task and json.RawMessage. -type conductorRunner struct { - client *client.TaskResourceApiService -} - -// NewConductorRunner returns a TaskRunner backed by the Conductor task client -// (supplied by the cmd layer via internal.GetTaskClient()). -func NewConductorRunner(taskClient *client.TaskResourceApiService) TaskRunner { - return &conductorRunner{client: taskClient} -} - -func (r *conductorRunner) Poll(ctx context.Context, taskType string) (Task, bool, error) { - opts := &client.TaskResourceApiBatchPollOpts{ - Workerid: optional.NewString(workerID), - Count: optional.NewInt32(pollBatchSize), - Timeout: optional.NewInt32(pollTimeoutMs), - } - tasks, _, err := r.client.BatchPoll(ctx, taskType, opts) - if err != nil { - return Task{}, false, err - } - if len(tasks) == 0 { - return Task{}, false, nil - } - return taskFromModel(tasks[0]) -} - -func (r *conductorRunner) Complete(ctx context.Context, t Task, output json.RawMessage) error { - return r.update(ctx, t, model.CompletedTask, wrapResult(output), "") -} - -func (r *conductorRunner) Fail(ctx context.Context, t Task, reason string) error { - return r.update(ctx, t, model.FailedTask, nil, reason) -} - -func (r *conductorRunner) update(ctx context.Context, t Task, status model.TaskResultStatus, output map[string]interface{}, reason string) error { - result := &model.TaskResult{ - TaskId: t.ID, - WorkflowInstanceId: t.WorkflowID, - WorkerId: workerID, - Status: status, - OutputData: output, - } - if reason != "" { - result.ReasonForIncompletion = reason - } - _, _, err := r.client.UpdateTask(ctx, result) - return err -} - -// taskFromModel marshals the SDK task's input map to bytes at the seam, so the -// map[string]interface{} never crosses into the worker loop or the handlers. -func taskFromModel(t model.Task) (Task, bool, error) { - input, err := json.Marshal(t.InputData) - if err != nil { - return Task{}, false, fmt.Errorf("marshal task input: %w", err) - } - return Task{ID: t.TaskId, WorkflowID: t.WorkflowInstanceId, Input: input}, true, nil -} - -// wrapResult wraps a handler's raw output under the "result" key the skill agent -// expects. Decoding to a generic value happens only here, at the seam. -func wrapResult(output json.RawMessage) map[string]interface{} { - var v interface{} - if len(output) > 0 { - if err := json.Unmarshal(output, &v); err != nil { - v = string(output) - } - } - return map[string]interface{}{outputKeyResult: v} -} diff --git a/internal/skillworker/runner_test.go b/internal/skillworker/runner_test.go deleted file mode 100644 index dfeecc8..0000000 --- a/internal/skillworker/runner_test.go +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "encoding/json" - "testing" - - "github.com/conductor-sdk/conductor-go/sdk/model" -) - -// TestTaskFromModelMarshalsInput verifies the bridge maps the SDK task's InputData -// map to json.RawMessage at the seam (no map crosses into the loop/handlers). -func TestTaskFromModelMarshalsInput(t *testing.T) { - mt := model.Task{ - TaskId: "t1", - WorkflowInstanceId: "w1", - InputData: map[string]interface{}{"path": "notes.md"}, - } - task, ok, err := taskFromModel(mt) - if err != nil || !ok { - t.Fatalf("taskFromModel: ok=%v err=%v", ok, err) - } - if task.ID != "t1" || task.WorkflowID != "w1" { - t.Errorf("task ids = %+v", task) - } - var decoded map[string]string - if err := json.Unmarshal(task.Input, &decoded); err != nil { - t.Fatalf("input not valid JSON: %v", err) - } - if decoded["path"] != "notes.md" { - t.Errorf("input = %s", task.Input) - } -} - -// TestWrapResultWrapsUnderResultKey checks the output wrapping the skill agent -// expects, for both an object and a bare-string handler output. -func TestWrapResultWrapsUnderResultKey(t *testing.T) { - obj := wrapResult(json.RawMessage(`{"files":["a","b"]}`)) - inner, ok := obj[outputKeyResult].(map[string]interface{}) - if !ok { - t.Fatalf("result not an object: %#v", obj) - } - if _, ok := inner["files"]; !ok { - t.Errorf("wrapped object lost its fields: %#v", inner) - } - - str := wrapResult(json.RawMessage(`"hello"`)) - if str[outputKeyResult] != "hello" { - t.Errorf("string result = %#v", str[outputKeyResult]) - } - - // Empty output still produces the result key (nil value). - empty := wrapResult(nil) - if _, ok := empty[outputKeyResult]; !ok { - t.Errorf("empty output missing result key: %#v", empty) - } -} diff --git a/internal/skillworker/worker.go b/internal/skillworker/worker.go deleted file mode 100644 index f5da002..0000000 --- a/internal/skillworker/worker.go +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -// Package skillworker is the local tool-worker runtime for skill run/serve. When a -// skill agent runs on the server, it dispatches tool tasks (read_skill_file, each -// script, workspace tools) back to the CLI; this package polls for those tasks, -// runs them locally, and returns the result. It is layered: the poll→handle→update -// loop and its two interfaces live here, the Conductor SDK is confined to the -// runner bridge, and the concrete tool logic lives in the handlers (later stage). -package skillworker - -import ( - "context" - "encoding/json" - "time" -) - -// Worker protocol constants — fixed by the skill agent/server contract, so they are -// named constants, never inline literals. -const ( - taskTypeSep = "__" // task type is "{skillName}__{tool}" - outputKeyResult = "result" // handler output is wrapped as {result: } - workerID = "conductor-cli" // identifies this worker in task results -) - -// pollBackoff is the idle wait between polls that return no task or an error. The -// production runner also long-polls the server, so this is a hot-loop backstop. -const pollBackoff = 100 * time.Millisecond - -// TaskType builds the "{skillName}__{tool}" task type dispatched for a skill tool. -func TaskType(skillName, tool string) string { - return skillName + taskTypeSep + tool -} - -// ToolHandler handles one dispatched tool task. IO is json.RawMessage, so no map -// crosses this boundary; the concrete tool logic lives in the handlers. -type ToolHandler interface { - Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) -} - -// Task is one polled tool task, decoupled from the SDK's model.Task. -type Task struct { - ID string - WorkflowID string - Input json.RawMessage -} - -// TaskRunner is the poll/complete/fail seam. The production impl (runner.go) wraps -// Conductor's TaskResourceApiService; tests inject a fake. It keeps model.Task and -// *client.TaskResourceApiService out of the worker loop and the handlers. -type TaskRunner interface { - // Poll returns the next task for taskType. ok=false means no task was available - // (poll again); a non-nil err is a real polling failure. - Poll(ctx context.Context, taskType string) (task Task, ok bool, err error) - Complete(ctx context.Context, t Task, output json.RawMessage) error - Fail(ctx context.Context, t Task, reason string) error -} - -// Worker runs the poll→handle→update loop for a single task type over a TaskRunner. -type Worker struct { - runner TaskRunner -} - -// NewWorker returns a Worker backed by the given TaskRunner. -func NewWorker(runner TaskRunner) *Worker { - return &Worker{runner: runner} -} - -// Run polls taskType and dispatches each task to h until ctx is cancelled. Transient -// poll failures back off and retry rather than stop the loop; a handler error fails -// only that task. Run returns when ctx is done. -func (w *Worker) Run(ctx context.Context, taskType string, h ToolHandler) { - for { - select { - case <-ctx.Done(): - return - default: - } - - task, ok, err := w.runner.Poll(ctx, taskType) - if err != nil { - if !sleep(ctx, pollBackoff) { - return - } - continue - } - if !ok { - if !sleep(ctx, pollBackoff) { - return - } - continue - } - - output, handleErr := h.Handle(ctx, task.Input) - if handleErr != nil { - _ = w.runner.Fail(ctx, task, handleErr.Error()) - continue - } - _ = w.runner.Complete(ctx, task, output) - } -} - -// sleep waits d or until ctx is cancelled; it returns false if ctx was cancelled, -// which keeps the poll loop responsive to Ctrl-C during idle waits. -func sleep(ctx context.Context, d time.Duration) bool { - t := time.NewTimer(d) - defer t.Stop() - select { - case <-ctx.Done(): - return false - case <-t.C: - return true - } -} diff --git a/internal/skillworker/worker_test.go b/internal/skillworker/worker_test.go deleted file mode 100644 index 6b52d8a..0000000 --- a/internal/skillworker/worker_test.go +++ /dev/null @@ -1,154 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "context" - "encoding/json" - "errors" - "sync" - "testing" - "time" -) - -// handlerFunc adapts a function to ToolHandler. -type handlerFunc func(ctx context.Context, input json.RawMessage) (json.RawMessage, error) - -func (f handlerFunc) Handle(ctx context.Context, input json.RawMessage) (json.RawMessage, error) { - return f(ctx, input) -} - -// fakeRunner is a TaskRunner that returns queued tasks then reports empty polls. It -// records terminal updates and can cancel the loop once the queue is drained. -type fakeRunner struct { - mu sync.Mutex - queue []Task - pollCount int - completed []completedCall - failed []failedCall - stopAfterOne context.CancelFunc // cancel the loop after the first terminal update -} - -type completedCall struct { - task Task - output json.RawMessage -} - -type failedCall struct { - task Task - reason string -} - -func (r *fakeRunner) Poll(ctx context.Context, taskType string) (Task, bool, error) { - r.mu.Lock() - defer r.mu.Unlock() - r.pollCount++ - if len(r.queue) == 0 { - return Task{}, false, nil - } - t := r.queue[0] - r.queue = r.queue[1:] - return t, true, nil -} - -func (r *fakeRunner) Complete(ctx context.Context, t Task, output json.RawMessage) error { - r.mu.Lock() - r.completed = append(r.completed, completedCall{task: t, output: output}) - r.mu.Unlock() - if r.stopAfterOne != nil { - r.stopAfterOne() - } - return nil -} - -func (r *fakeRunner) Fail(ctx context.Context, t Task, reason string) error { - r.mu.Lock() - r.failed = append(r.failed, failedCall{task: t, reason: reason}) - r.mu.Unlock() - if r.stopAfterOne != nil { - r.stopAfterOne() - } - return nil -} - -func TestWorkerRunCompletesTask(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{ - queue: []Task{{ID: "t1", WorkflowID: "w1", Input: json.RawMessage(`{"path":"a"}`)}}, - stopAfterOne: cancel, - } - var gotInput json.RawMessage - h := handlerFunc(func(_ context.Context, in json.RawMessage) (json.RawMessage, error) { - gotInput = in - return json.RawMessage(`"file body"`), nil - }) - - NewWorker(fr).Run(ctx, "demo__read_skill_file", h) - - if string(gotInput) != `{"path":"a"}` { - t.Errorf("handler input = %s", gotInput) - } - if len(fr.completed) != 1 || len(fr.failed) != 0 { - t.Fatalf("completed=%d failed=%d", len(fr.completed), len(fr.failed)) - } - got := fr.completed[0] - if got.task.ID != "t1" || string(got.output) != `"file body"` { - t.Errorf("completed call = %+v", got) - } -} - -func TestWorkerRunFailsTaskOnHandlerError(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{ - queue: []Task{{ID: "t2", WorkflowID: "w2"}}, - stopAfterOne: cancel, - } - h := handlerFunc(func(context.Context, json.RawMessage) (json.RawMessage, error) { - return nil, errors.New("boom") - }) - - NewWorker(fr).Run(ctx, "demo__script", h) - - if len(fr.failed) != 1 || len(fr.completed) != 0 { - t.Fatalf("completed=%d failed=%d", len(fr.completed), len(fr.failed)) - } - if fr.failed[0].reason != "boom" || fr.failed[0].task.ID != "t2" { - t.Errorf("fail call = %+v", fr.failed[0]) - } -} - -func TestWorkerRunStopsOnContextCancel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - fr := &fakeRunner{} // always reports empty polls - done := make(chan struct{}) - go func() { - NewWorker(fr).Run(ctx, "demo__x", handlerFunc(func(context.Context, json.RawMessage) (json.RawMessage, error) { - return nil, nil - })) - close(done) - }() - - cancel() - select { - case <-done: - case <-time.After(2 * time.Second): - t.Fatal("Run did not stop after context cancel") - } -} - -func TestTaskType(t *testing.T) { - if got := TaskType("demo", "read_skill_file"); got != "demo__read_skill_file" { - t.Errorf("TaskType = %q", got) - } -} diff --git a/internal/skillworker/workspace.go b/internal/skillworker/workspace.go deleted file mode 100644 index b6d9957..0000000 --- a/internal/skillworker/workspace.go +++ /dev/null @@ -1,219 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skillworker - -import ( - "fmt" - "io" - "os" - pathpkg "path" - "path/filepath" - "regexp" - "strings" -) - -// Workspace root kinds and the reserved default root name. -const ( - KindWorkspace = "workspace" // the primary, editable working directory - KindFilesystem = "filesystem" // an additional named read-only root - workspaceRootName = "workspace" // Root("") and AGENTSPAN_WORKSPACE_DIR resolve to this -) - -// defaultTextReadLimit bounds a single text read when no limit is supplied. -const defaultTextReadLimit = 1 << 20 // 1 MiB - -// workspaceRootNamePattern constrains a root name to a filesystem/env-safe set. -var workspaceRootNamePattern = regexp.MustCompile(`^[A-Za-z0-9._-]+$`) - -// WorkspaceConfig is the set of local roots the workspace tools expose. It is -// resolved from flags in the cmd layer and passed to the handlers; the server only -// ever sees the wire form (WireConfig) and tool outputs, never these paths. -type WorkspaceConfig struct { - Enabled bool - Roots []WorkspaceRoot -} - -// WorkspaceRoot is one named, absolute, symlink-resolved local root. -type WorkspaceRoot struct { - Name string - Path string - Kind string -} - -// WorkspaceWire is the workspace section of a skill config as sent to the server: -// the enable flag and the root names/kinds only — never their local paths. -type WorkspaceWire struct { - Enabled bool `json:"enabled"` - Roots []WorkspaceRootWire `json:"roots,omitempty"` -} - -// WorkspaceRootWire is one named workspace root on the wire (no path). -type WorkspaceRootWire struct { - Name string `json:"name"` - Kind string `json:"kind,omitempty"` -} - -// NewWorkspaceRoot validates and resolves one root. The name must be -// filesystem/env-safe and the path must be an existing directory; the returned -// path is absolute and symlink-resolved. File I/O is expected here — this is the -// local CLI runtime layer. -func NewWorkspaceRoot(name, pathValue, kind string) (WorkspaceRoot, error) { - if !workspaceRootNamePattern.MatchString(name) { - return WorkspaceRoot{}, fmt.Errorf("invalid filesystem root name %q: use letters, numbers, dot, underscore, or dash", name) - } - absPath, err := filepath.Abs(pathValue) - if err != nil { - return WorkspaceRoot{}, fmt.Errorf("resolve filesystem root %q: %w", name, err) - } - info, err := os.Stat(absPath) - if err != nil { - return WorkspaceRoot{}, fmt.Errorf("filesystem root %q does not exist: %w", name, err) - } - if !info.IsDir() { - return WorkspaceRoot{}, fmt.Errorf("filesystem root %q is not a directory: %s", name, absPath) - } - if resolved, err := filepath.EvalSymlinks(absPath); err == nil { - absPath = resolved - } - return WorkspaceRoot{Name: name, Path: absPath, Kind: kind}, nil -} - -// Root returns the named root, or the first root for an empty name. -func (c WorkspaceConfig) Root(name string) (WorkspaceRoot, bool) { - if name == "" && len(c.Roots) > 0 { - return c.Roots[0], true - } - for _, root := range c.Roots { - if root.Name == name { - return root, true - } - } - return WorkspaceRoot{}, false -} - -// WireConfig produces the server-bound workspace section (no local paths), or nil -// when the workspace is disabled. -func (c WorkspaceConfig) WireConfig() *WorkspaceWire { - if !c.Enabled { - return nil - } - roots := make([]WorkspaceRootWire, 0, len(c.Roots)) - for _, r := range c.Roots { - roots = append(roots, WorkspaceRootWire{Name: r.Name, Kind: r.Kind}) - } - return &WorkspaceWire{Enabled: true, Roots: roots} -} - -// safeWorkspacePath joins relPath onto absRoot and rejects any path that escapes -// the root (after resolving symlinks) — the workspace security boundary. -func safeWorkspacePath(absRoot, relPath string) (string, error) { - cleanRel := filepath.Clean(filepath.FromSlash(defaultString(relPath, "."))) - if filepath.IsAbs(cleanRel) || cleanRel == ".." || strings.HasPrefix(cleanRel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("'%s' is outside the filesystem root", relPath) - } - target := filepath.Join(absRoot, cleanRel) - resolvedRoot, err := filepath.EvalSymlinks(absRoot) - if err != nil { - resolvedRoot = absRoot - } - resolvedTarget, err := filepath.EvalSymlinks(target) - if err != nil { - resolvedTarget = target - } - if rel, err := filepath.Rel(resolvedRoot, resolvedTarget); err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) { - return "", fmt.Errorf("'%s' is outside the filesystem root", relPath) - } - return resolvedTarget, nil -} - -// resolvedWorkspaceRootPath returns absRoot with symlinks resolved (used to compute -// relative paths consistently with safeWorkspacePath's resolved targets). -func resolvedWorkspaceRootPath(absRoot string) string { - if resolved, err := filepath.EvalSymlinks(absRoot); err == nil { - return resolved - } - return absRoot -} - -// readLimitedTextFile reads up to limit bytes and reports whether it truncated. -func readLimitedTextFile(pathValue string, limit int) (string, bool, error) { - if limit <= 0 { - limit = defaultTextReadLimit - } - file, err := os.Open(pathValue) - if err != nil { - return "", false, err - } - defer file.Close() - data, err := io.ReadAll(io.LimitReader(file, int64(limit)+1)) - if err != nil { - return "", false, err - } - truncated := len(data) > limit - if truncated { - data = data[:limit] - } - return string(data), truncated, nil -} - -// shouldSkipWorkspaceDir reports directories the workspace walk never descends into. -func shouldSkipWorkspaceDir(name string) bool { - switch name { - case ".git", "node_modules", "__pycache__", ".venv", "venv", ".tox", "dist", "build", "target", ".gradle", ".idea", ".mypy_cache", ".pytest_cache": - return true - default: - return false - } -} - -// matchesWorkspacePattern reports whether relPath matches a glob (supporting **), -// or true when the pattern is empty. -func matchesWorkspacePattern(pattern, relPath string) bool { - pattern = filepath.ToSlash(strings.TrimSpace(pattern)) - if pattern == "" { - return true - } - relPath = filepath.ToSlash(relPath) - if ok, err := pathpkg.Match(pattern, relPath); err == nil && ok { - return true - } - if strings.Contains(pattern, "**") { - re := regexp.QuoteMeta(pattern) - re = strings.ReplaceAll(re, `\*\*`, `.*`) - re = strings.ReplaceAll(re, `\*`, `[^/]*`) - re = strings.ReplaceAll(re, `\?`, `[^/]`) - ok, err := regexp.MatchString("^"+re+"$", relPath) - return err == nil && ok - } - if strings.HasPrefix(pattern, "*") || strings.HasSuffix(pattern, "*") { - return strings.Contains(relPath, strings.Trim(pattern, "*")) - } - return relPath == pattern -} - -// trimLongLine caps a line length for search-match display. -func trimLongLine(value string, limit int) string { - if limit <= 0 || len(value) <= limit { - return value - } - return value[:limit] + "...[truncated]" -} - -// defaultString returns fallback when value is blank. -func defaultString(value, fallback string) string { - if strings.TrimSpace(value) == "" { - return fallback - } - return value -} From 78cac97a95f18489ea47e5d67754689c70ba7105 Mon Sep 17 00:00:00 2001 From: Kowser Date: Tue, 4 Aug 2026 19:41:38 -0700 Subject: [PATCH 3/5] chore: delete internal/skill, drop GetSkillService - rm internal/skill package (types.go, client.go, service.go, client_test.go) - settings.go: drop skill import + GetSkillService, reword agent-only comments --- internal/settings.go | 14 +--- internal/skill/client.go | 152 ---------------------------------- internal/skill/client_test.go | 139 ------------------------------- internal/skill/service.go | 58 ------------- internal/skill/types.go | 60 -------------- 5 files changed, 4 insertions(+), 419 deletions(-) delete mode 100644 internal/skill/client.go delete mode 100644 internal/skill/client_test.go delete mode 100644 internal/skill/service.go delete mode 100644 internal/skill/types.go diff --git a/internal/settings.go b/internal/settings.go index 92f76a5..ffd3289 100644 --- a/internal/settings.go +++ b/internal/settings.go @@ -18,7 +18,6 @@ import ( "github.com/conductor-sdk/conductor-go/sdk/client" "github.com/conductor-oss/conductor-cli/internal/agent" - "github.com/conductor-oss/conductor-cli/internal/skill" "github.com/conductor-oss/conductor-cli/internal/transport" ) @@ -26,9 +25,9 @@ var ( workflowClient *client.WorkflowResourceApiService = nil apiClient *client.APIClient - // agentTransport is the shared HTTP transport for the agent and skill clients, + // agentTransport is the shared HTTP transport for the agent client, // configured once at startup (cmd/root.go) from the same server URL and auth as - // apiClient. The agent/skill endpoints are not part of the conductor-go SDK. + // apiClient. The agent endpoints are not part of the conductor-go SDK. agentTransport transport.Config ) @@ -69,13 +68,13 @@ func SetAPIClient(client *client.APIClient) { apiClient = client } -// SetTransport stores the shared transport used by the agent and skill clients. +// SetTransport stores the shared transport used by the agent client. // Called once at startup after the server URL and authentication are resolved. func SetTransport(cfg transport.Config) { agentTransport = cfg } -// Transport returns the shared agent/skill transport configured at startup. +// Transport returns the shared agent transport configured at startup. func Transport() transport.Config { return agentTransport } @@ -84,8 +83,3 @@ func Transport() transport.Config { func GetAgentService() agent.Service { return agent.NewService(agent.NewClient(agentTransport)) } - -// GetSkillService returns the skill use-case service over the shared transport. -func GetSkillService() skill.Service { - return skill.NewService(skill.NewClient(agentTransport)) -} diff --git a/internal/skill/client.go b/internal/skill/client.go deleted file mode 100644 index e232051..0000000 --- a/internal/skill/client.go +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skill - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "mime/multipart" - "net/http" - "net/url" - - "github.com/conductor-oss/conductor-cli/internal/transport" -) - -// Endpoint paths and fixed tokens — named constants, never inline literals. -// pathSkills is relative to the transport BaseURL, which already carries the -// "/api" prefix (see cmd/root.go); do not re-add it here. -const ( - pathSkills = "/skills" - registerSegment = "/register" - versionSegment = "/versions/" - packageSegment = "/package" - queryAllVersions = "allVersions" - valueTrue = "true" - versionLatest = "latest" - - fieldManifest = "manifest" - fieldPackage = "package" - packageFileName = "skill.zip" - headerContentTyp = "Content-Type" -) - -// Client is the transport boundary for skill endpoints. It returns domain types or -// raw package bytes — never *http.Response or transport types. -type Client interface { - List(ctx context.Context, allVersions bool) ([]Summary, error) - Get(ctx context.Context, name, version string) (Detail, error) - DownloadPackage(ctx context.Context, name, version string) ([]byte, error) - Register(ctx context.Context, manifest json.RawMessage, pkg []byte) (Detail, error) - Delete(ctx context.Context, name, version string) error -} - -// NewClient returns a Client backed by the shared transport. -func NewClient(t transport.Config) Client { - return &restClient{t: t} -} - -type restClient struct { - t transport.Config -} - -func (c *restClient) List(ctx context.Context, allVersions bool) ([]Summary, error) { - path := pathSkills - if allVersions { - q := url.Values{} - q.Set(queryAllVersions, valueTrue) - path += "?" + q.Encode() - } - var out []Summary - if err := c.t.DoJSON(ctx, http.MethodGet, path, nil, &out); err != nil { - return nil, err - } - return out, nil -} - -func (c *restClient) Get(ctx context.Context, name, version string) (Detail, error) { - path := pathSkills + "/" + url.PathEscape(name) - if version != "" { - path += versionSegment + url.PathEscape(version) - } - var out Detail - if err := c.t.DoJSON(ctx, http.MethodGet, path, nil, &out); err != nil { - return Detail{}, err - } - return out, nil -} - -func (c *restClient) DownloadPackage(ctx context.Context, name, version string) ([]byte, error) { - path := pathSkills + "/" + url.PathEscape(name) + versionSegment + url.PathEscape(resolveVersion(version)) + packageSegment - resp, err := c.t.Do(ctx, http.MethodGet, path, nil, nil) - if err != nil { - return nil, err - } - defer resp.Body.Close() - return io.ReadAll(resp.Body) -} - -// Register uploads a skill manifest (JSON) and its package (zip) as multipart form -// data. The manifest is built by the cmd layer from the local skill directory; the -// client only frames the request — it never touches the filesystem. -func (c *restClient) Register(ctx context.Context, manifest json.RawMessage, pkg []byte) (Detail, error) { - var body bytes.Buffer - w := multipart.NewWriter(&body) - mf, err := w.CreateFormField(fieldManifest) - if err != nil { - return Detail{}, err - } - if _, err := mf.Write(manifest); err != nil { - return Detail{}, err - } - pf, err := w.CreateFormFile(fieldPackage, packageFileName) - if err != nil { - return Detail{}, err - } - if _, err := pf.Write(pkg); err != nil { - return Detail{}, err - } - if err := w.Close(); err != nil { - return Detail{}, fmt.Errorf("close multipart body: %w", err) - } - - header := http.Header{} - header.Set(headerContentTyp, w.FormDataContentType()) - resp, err := c.t.Do(ctx, http.MethodPost, pathSkills+registerSegment, &body, header) - if err != nil { - return Detail{}, err - } - defer resp.Body.Close() - var out Detail - if err := json.NewDecoder(resp.Body).Decode(&out); err != nil { - return Detail{}, fmt.Errorf("decode response: %w", err) - } - return out, nil -} - -func (c *restClient) Delete(ctx context.Context, name, version string) error { - path := pathSkills + "/" + url.PathEscape(name) + versionSegment + url.PathEscape(resolveVersion(version)) - return c.t.DoJSON(ctx, http.MethodDelete, path, nil, nil) -} - -// resolveVersion defaults an empty version to "latest", matching the server's -// version-pinned package and delete endpoints. -func resolveVersion(version string) string { - if version == "" { - return versionLatest - } - return version -} diff --git a/internal/skill/client_test.go b/internal/skill/client_test.go deleted file mode 100644 index 8faec4a..0000000 --- a/internal/skill/client_test.go +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skill - -import ( - "context" - "io" - "net/http" - "net/http/httptest" - "testing" - - "github.com/conductor-oss/conductor-cli/internal/transport" -) - -func newTestClient(t *testing.T, h http.HandlerFunc) Client { - t.Helper() - srv := httptest.NewServer(h) - t.Cleanup(srv.Close) - return NewClient(transport.Config{BaseURL: srv.URL}) -} - -func TestListSkills(t *testing.T) { - c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != pathSkills { - t.Errorf("path = %q, want %q", r.URL.Path, pathSkills) - } - if r.URL.Query().Get(queryAllVersions) != valueTrue { - t.Errorf("expected allVersions=true, got %q", r.URL.RawQuery) - } - _, _ = w.Write([]byte(`[{"name":"summarize","version":"abc123","fileCount":3}]`)) - }) - out, err := c.List(context.Background(), true) - if err != nil { - t.Fatalf("List: %v", err) - } - if len(out) != 1 || out[0].Name != "summarize" || out[0].FileCount != 3 { - t.Errorf("got %+v", out) - } -} - -func TestGetSkillWithVersionPath(t *testing.T) { - c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { - want := pathSkills + "/summarize" + versionSegment + "v1" - if r.URL.Path != want { - t.Errorf("path = %q, want %q", r.URL.Path, want) - } - _, _ = w.Write([]byte(`{"name":"summarize","version":"v1"}`)) - }) - d, err := c.Get(context.Background(), "summarize", "v1") - if err != nil { - t.Fatalf("Get: %v", err) - } - if d.Version != "v1" { - t.Errorf("version = %q, want v1", d.Version) - } -} - -func TestDownloadPackageDefaultsToLatest(t *testing.T) { - c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { - want := pathSkills + "/summarize" + versionSegment + versionLatest + packageSegment - if r.URL.Path != want { - t.Errorf("path = %q, want %q", r.URL.Path, want) - } - _, _ = w.Write([]byte("ZIPBYTES")) - }) - data, err := c.DownloadPackage(context.Background(), "summarize", "") - if err != nil { - t.Fatalf("DownloadPackage: %v", err) - } - if string(data) != "ZIPBYTES" { - t.Errorf("got %q", data) - } -} - -func TestRegisterSendsMultipart(t *testing.T) { - var gotManifest string - var gotPackage []byte - c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != pathSkills+registerSegment { - t.Errorf("path = %q, want %q", r.URL.Path, pathSkills+registerSegment) - } - if err := r.ParseMultipartForm(1 << 20); err != nil { - t.Fatalf("parse multipart: %v", err) - } - gotManifest = r.FormValue(fieldManifest) - file, _, err := r.FormFile(fieldPackage) - if err != nil { - t.Fatalf("form file: %v", err) - } - defer file.Close() - gotPackage, _ = io.ReadAll(file) - _, _ = w.Write([]byte(`{"name":"summarize","version":"v1"}`)) - }) - - detail, err := c.Register(context.Background(), []byte(`{"name":"summarize"}`), []byte("ZIPDATA")) - if err != nil { - t.Fatalf("Register: %v", err) - } - if detail.Version != "v1" { - t.Errorf("version = %q, want v1", detail.Version) - } - if gotManifest != `{"name":"summarize"}` { - t.Errorf("manifest = %q", gotManifest) - } - if string(gotPackage) != "ZIPDATA" { - t.Errorf("package = %q", gotPackage) - } -} - -func TestDeleteSkill(t *testing.T) { - called := false - c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { - called = true - if r.Method != http.MethodDelete { - t.Errorf("method = %q, want DELETE", r.Method) - } - want := pathSkills + "/summarize" + versionSegment + "v2" - if r.URL.Path != want { - t.Errorf("path = %q, want %q", r.URL.Path, want) - } - }) - if err := c.Delete(context.Background(), "summarize", "v2"); err != nil { - t.Fatalf("Delete: %v", err) - } - if !called { - t.Error("delete request was not made") - } -} diff --git a/internal/skill/service.go b/internal/skill/service.go deleted file mode 100644 index 84bb8e3..0000000 --- a/internal/skill/service.go +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -package skill - -import ( - "context" - "encoding/json" -) - -// Service is the skill use-case layer. It depends only on Client and is free of -// presentation and transport concerns. -type Service interface { - List(ctx context.Context, allVersions bool) ([]Summary, error) - Get(ctx context.Context, name, version string) (Detail, error) - DownloadPackage(ctx context.Context, name, version string) ([]byte, error) - Register(ctx context.Context, manifest json.RawMessage, pkg []byte) (Detail, error) - Delete(ctx context.Context, name, version string) error -} - -// NewService returns a Service backed by the given Client. -func NewService(c Client) Service { - return &service{client: c} -} - -type service struct { - client Client -} - -func (s *service) List(ctx context.Context, allVersions bool) ([]Summary, error) { - return s.client.List(ctx, allVersions) -} - -func (s *service) Get(ctx context.Context, name, version string) (Detail, error) { - return s.client.Get(ctx, name, version) -} - -func (s *service) DownloadPackage(ctx context.Context, name, version string) ([]byte, error) { - return s.client.DownloadPackage(ctx, name, version) -} - -func (s *service) Register(ctx context.Context, manifest json.RawMessage, pkg []byte) (Detail, error) { - return s.client.Register(ctx, manifest, pkg) -} - -func (s *service) Delete(ctx context.Context, name, version string) error { - return s.client.Delete(ctx, name, version) -} diff --git a/internal/skill/types.go b/internal/skill/types.go deleted file mode 100644 index f4a7a26..0000000 --- a/internal/skill/types.go +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Copyright 2026 Conductor Authors. - *

- * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - *

- * http://www.apache.org/licenses/LICENSE-2.0 - *

- * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. - */ - -// Package skill is the CLI-owned client and service for the Conductor skill -// endpoints (/api/skills/*). It mirrors the layering of the agent package. This -// stage covers the server-backed management operations (list/get/pull/delete); -// register/run/serve, which need the local skill packaging-and-execution engine, -// build on the same client in a later step. -package skill - -import "encoding/json" - -// Summary is the list view of a registered skill. -type Summary struct { - Name string `json:"name"` - Version string `json:"version"` - Description string `json:"description"` - Status string `json:"status"` - PackageSize int64 `json:"packageSize"` - FileCount int `json:"fileCount"` - ScriptCount int `json:"scriptCount"` - SubAgentCount int `json:"subAgentCount"` - ResourceCount int `json:"resourceCount"` -} - -// FileEntry describes one file inside a skill package. -type FileEntry struct { - Path string `json:"path"` - Size int64 `json:"size"` - SHA256 string `json:"sha256"` - ContentType string `json:"contentType"` -} - -// Detail is the full server-side skill package record. RawConfig and Metadata stay -// as raw JSON so their free-form shapes do not leak as untyped maps across a boundary. -type Detail struct { - Name string `json:"name"` - Version string `json:"version"` - Description string `json:"description"` - Checksum string `json:"checksum"` - Status string `json:"status"` - OwnerID string `json:"ownerId"` - CreatedAt *int64 `json:"createdAt"` - UpdatedAt *int64 `json:"updatedAt"` - PackageSize int64 `json:"packageSize"` - FileCount int `json:"fileCount"` - Files []FileEntry `json:"files"` - Metadata json.RawMessage `json:"metadata,omitempty"` - RawConfig json.RawMessage `json:"rawConfig,omitempty"` -} From dd0907b2fb74fbad9fda385c062f472153260f7f Mon Sep 17 00:00:00 2001 From: Kowser Date: Tue, 4 Aug 2026 19:42:37 -0700 Subject: [PATCH 4/5] chore: reword agent/skill comments after skill removal - root.go, transport.go, internal/transport: drop "and skill" from comments - internal/agent/service.go: note frameworkSkill kept for backward compat --- cmd/root.go | 2 +- cmd/transport.go | 2 +- internal/agent/service.go | 2 ++ internal/transport/transport.go | 10 +++++----- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index ee2b064..a178804 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -224,7 +224,7 @@ var rootCmd = &cobra.Command{ internal.SetAPIClient(apiClient) - // Share the same server URL and auth with the agent/skill transport, whose + // Share the same server URL and auth with the agent transport, whose // endpoints are not part of the conductor-go SDK. agentTokens is nil when no // credentials are configured (anonymous access). internal.SetTransport(transport.Config{ diff --git a/cmd/transport.go b/cmd/transport.go index f581374..989abb6 100644 --- a/cmd/transport.go +++ b/cmd/transport.go @@ -30,7 +30,7 @@ type sdkTokenManager interface { } // tokenProvider adapts a conductor-go token manager to transport.TokenProvider so -// that agent and skill traffic reuses exactly the same JWT — including refresh and +// that agent traffic reuses exactly the same JWT — including refresh and // caching — as the conductor-go SDK client. type tokenProvider struct { manager sdkTokenManager diff --git a/internal/agent/service.go b/internal/agent/service.go index 8649a6b..ad9e5b5 100644 --- a/internal/agent/service.go +++ b/internal/agent/service.go @@ -21,6 +21,8 @@ import ( ) // frameworkSkill is the framework marker inferred for skill-backed agent definitions. +// Kept after the `skill` command's removal for backward compatibility: conductor-cli +// can no longer create these, but `agent run` must keep recognizing ones already deployed. const frameworkSkill = "skill" // EventSink receives streamed events. It decouples streaming from rendering: the cmd diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 63a162d..abe8c11 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -11,7 +11,7 @@ * specific language governing permissions and limitations under the License. */ -// Package transport is the shared HTTP transport for the agent and skill clients. +// Package transport is the shared HTTP transport for the agent client. // Those endpoints are not part of the conductor-go SDK, so the CLI owns a small, // interface-bounded transport that reuses Conductor's resolved server URL and JWT // (see cmd/root.go) — one backend, one auth path. No file paths, ports, or URLs are @@ -38,7 +38,7 @@ type TokenProvider interface { Token(ctx context.Context) (string, error) } -// Config is the shared transport for agent and skill traffic. BaseURL and Tokens are +// Config is the shared transport for agent traffic. BaseURL and Tokens are // resolved once at startup from the same source as the conductor-go client, so these // calls reuse Conductor's server URL and authentication — no second config, no second // auth path. @@ -134,9 +134,9 @@ func (c Config) applyAuth(ctx context.Context, req *http.Request) error { } // APIError is a non-2xx response from the Conductor server. It mirrors the server's -// JSON error shape ({status, message, error}) so agent and skill errors read -// consistently with the rest of the CLI (cf. cmd.parseAPIError, which does the same -// for SDK-surfaced errors). +// JSON error shape ({status, message, error}) so agent errors read consistently with +// the rest of the CLI (cf. cmd.parseAPIError, which does the same for SDK-surfaced +// errors). type APIError struct { Status int Method string From 73ad6264a9c8e89670f6374f6603630c7fcc417c Mon Sep 17 00:00:00 2001 From: Kowser Date: Tue, 4 Aug 2026 19:44:04 -0700 Subject: [PATCH 5/5] docs: remove skill command from CLAUDE.md - drop conductor skill from intro + Conductor Management group - delete Skill Commands section, skill list bullet --- CLAUDE.md | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3f29931..4189c45 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,8 +7,7 @@ Conductor CLI (`conductor`) is a command-line tool for managing Netflix Conductor workflows, executions, tasks, webhooks, and schedules. It connects to Conductor server instances for workflow orchestration. It also runs a local Conductor server for development (`conductor server start`), runs task workers -(`conductor worker`), and manages AI agents and skills (`conductor agent`, `conductor skill`, -`conductor deploy`). +(`conductor worker`), and manages AI agents (`conductor agent`, `conductor deploy`). ## Installation @@ -88,7 +87,7 @@ by hand. ## Command Reference Commands are organized into three help groups: -- **Conductor Management** — `workflow`, `task`, `schedule`, `webhook`, `secret`, `api-gateway`, `agent`, `skill`, `worker` +- **Conductor Management** — `workflow`, `task`, `schedule`, `webhook`, `secret`, `api-gateway`, `agent`, `worker` - **CLI Configuration** — `config`, `whoami`, `update`, `completion` - **Development** — `server`, `code`, `deploy`, `doctor` @@ -397,36 +396,6 @@ Columns: NAME, VERSION, TYPE, DESCRIPTION **Table Output (agent execution):** Columns: ID, AGENT, STATUS, START_TIME, DURATION -### Skill Commands - -Package local skill directories (a directory containing `SKILL.md`) and run them as agents. - -| Command | Description | Required Args | Optional Flags | Example | -|---------|-------------|---------------|----------------|---------| -| `skill register ` | Package and register a local skill | skill directory | `--version`, `--model`, `--agent-model` | `conductor skill register ./my-skill` | -| `skill load ` | Package a local skill and deploy it as an agent | skill directory | `--model` (required), `--agent-model`, `--search-path` | `conductor skill load ./my-skill --model claude-opus-5` | -| `skill run ` | Run a local or registered skill and stream output | path or name, prompt | `--model` (required), `--agent-model`, `--param`, `--version`, `--search-path`, `--workspace`, `--no-workspace`, `--filesystem`, `--script-timeout`, `--script-output-limit`, `--workspace-file-limit` | `conductor skill run ./my-skill "summarize the logs" --model claude-opus-5` | -| `skill serve ` | Start local tool workers without running the skill | path or name | same as `skill run` (minus `--param`) | `conductor skill serve ./my-skill` | -| `skill list` | List registered skills | None | `--all-versions`, `--json`, `--csv` | `conductor skill list` | -| `skill get [version]` | Get a registered skill | skill name | `--version` | `conductor skill get my-skill` | -| `skill pull [destination]` | Download and extract a skill package | skill name | `--version` | `conductor skill pull my-skill ./out` | -| `skill delete [version]` | Delete a registered skill version | skill name | `--version` | `conductor skill delete my-skill` | - -**Flags:** -- `--model` - Orchestrator and default model (required for `load`, `run`, and `serve`) -- `--agent-model` - Sub-agent model override in `name=model` form (repeatable) -- `--param` - Skill parameter override in `key=value` form (repeatable) -- `--version` - Skill version or checksum prefix -- `--search-path` - Cross-skill search directory (repeatable) -- `--workspace` - Workspace directory exposed to workspace tools -- `--no-workspace` - Do not expose the current workspace -- `--filesystem` - Additional read-only filesystem root as `name=path` (repeatable) -- `--all-versions` - List all versions instead of only the latest -- `--script-timeout` - Skill script timeout in seconds -- `--script-output-limit` / `--workspace-file-limit` - Maximum bytes captured from script output / returned by workspace file tools - -**`load` vs `run`:** `load` only publishes the agent (run it later with `agent run --name `); `run` starts local tool workers, launches the agent, and streams the execution. `serve` starts only the workers so the skill can be driven from elsewhere (e.g. the UI). - ### Worker Commands Run task workers that poll Conductor and execute work locally. @@ -510,7 +479,6 @@ See [WORKER_JS.md](./WORKER_JS.md) and [WORKER_STDIO.md](./WORKER_STDIO.md) for - `webhook list` - Table with NAME, WEBHOOK ID, WORKFLOWS, URL (or `--json`) - `secret list` - Table with KEY, or KEY and TAGS with `--with-tags` (or `--json`) - `agent list` - Table with NAME, VERSION, TYPE, DESCRIPTION (or `--json`/`--csv`) -- `skill list` - Table of registered skills (or `--json`/`--csv`) **Important:** To parse output reliably, redirect stderr to `/dev/null` to suppress update notifications and warnings: ```bash