Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,22 @@ func gentleAIUpgradeVersionFromTUI(finalModel tea.Model) (string, bool) {
}

func runSkillRegistry(args []string, stdout io.Writer) error {
if len(args) == 0 || args[0] != "refresh" {
return fmt.Errorf("usage: gentle-ai skill-registry refresh [--cwd <dir>] [--force] [--quiet] [--no-gitignore]")
if len(args) == 0 || (args[0] != "refresh" && args[0] != "load") {
return fmt.Errorf("usage: gentle-ai skill-registry <refresh|load> [--cwd <dir>] [--force] [--quiet] [--no-gitignore] [--load <path>]")
}

cmd := args[0]
cwd := ""
force := false
quiet := false
ensureGitignore := true
loadPath := ""

if cmd == "load" && len(args) > 1 && !strings.HasPrefix(args[1], "-") {
loadPath = args[1]
args = append(args[:1], args[2:]...)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

for i := 1; i < len(args); i++ {
switch args[i] {
case "--force", "-f":
Expand All @@ -280,10 +288,20 @@ func runSkillRegistry(args []string, stdout io.Writer) error {
}
cwd = args[i+1]
i++
case "--load":
if i+1 >= len(args) {
return fmt.Errorf("--load requires a value")
}
loadPath = args[i+1]
i++
default:
return fmt.Errorf("unknown skill-registry argument %q", args[i])
}
}
if cmd == "load" && loadPath == "" {
return fmt.Errorf("skill-registry load requires a source file path")
}

if cwd == "" {
var err error
cwd, err = os.Getwd()
Expand All @@ -300,13 +318,23 @@ func runSkillRegistry(args []string, stdout io.Writer) error {
return err
}
}
result, err := skillregistry.Regenerate(cwd, home, force)

var result skillregistry.Result
if cmd == "load" || loadPath != "" {
result, err = skillregistry.LoadRegistry(loadPath, cwd, force)
} else {
result, err = skillregistry.Regenerate(cwd, home, force)
}
if err != nil {
return err
}
if !quiet {
if result.Regenerated {
_, _ = fmt.Fprintf(stdout, "Skill registry refreshed (%d skills): %s\n", result.SkillCount, result.Registry)
if cmd == "load" || loadPath != "" {
_, _ = fmt.Fprintf(stdout, "Skill registry loaded (%d skills): %s\n", result.SkillCount, result.Registry)
} else {
_, _ = fmt.Fprintf(stdout, "Skill registry refreshed (%d skills): %s\n", result.SkillCount, result.Registry)
}
} else {
_, _ = fmt.Fprintf(stdout, "Skill registry up to date (%s): %s\n", result.Reason, result.Registry)
}
Expand Down
2 changes: 2 additions & 0 deletions internal/app/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ COMMANDS
sync Sync agent configs and skills to current version
skill-registry refresh
Refresh .atl/skill-registry.md with cache-hit fast path
skill-registry load <path>
Load pre-curated .atl/skill-registry.md into current project
sdd-status [change]
Print native SDD phase status for orchestrators
sdd-continue [change]
Expand Down
63 changes: 63 additions & 0 deletions internal/skillregistry/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,69 @@ func Regenerate(cwd, home string, force bool) (Result, error) {
return Result{Regenerated: true, SkillCount: len(entries), Reason: reason, Registry: registryPath, Cache: cachePath}, nil
}

func LoadRegistry(loadPath, cwd string, force bool) (Result, error) {
cwd = filepath.Clean(cwd)
targetRegistryPath := filepath.Join(cwd, RegistryRelPath)
cachePath := filepath.Join(cwd, CacheRelPath)

if loadPath == "" {
loadPath = targetRegistryPath
} else if !filepath.IsAbs(loadPath) {
loadPath = filepath.Join(cwd, loadPath)
}
loadPath = filepath.Clean(loadPath)

data, err := os.ReadFile(loadPath)
if err != nil {
return Result{}, fmt.Errorf("read target skill registry %q: %w", loadPath, err)
}

content := string(data)
if !strings.Contains(content, "## Skills") || !strings.Contains(content, "| Skill |") {
return Result{}, fmt.Errorf("invalid skill registry format in %q: missing required headers or table", loadPath)
}
Comment on lines +181 to +184

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Require all registry markers before accepting the load.

Line 182 only rejects files when all markers are missing. A file with # Skill Registry but no ## Skills section or | Skill | table header still passes, gets copied into .atl/skill-registry.md, and is cached as a successful load.

Suggested fix
-	if !strings.Contains(content, "## Skills") && !strings.Contains(content, "| Skill |") && !strings.Contains(content, "# Skill Registry") {
+	if !strings.Contains(content, "# Skill Registry") ||
+		!strings.Contains(content, "## Skills") ||
+		!strings.Contains(content, "| Skill |") {
 		return Result{}, fmt.Errorf("invalid skill registry format in %q: missing required headers or table", loadPath)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
content := string(data)
if !strings.Contains(content, "## Skills") && !strings.Contains(content, "| Skill |") && !strings.Contains(content, "# Skill Registry") {
return Result{}, fmt.Errorf("invalid skill registry format in %q: missing required headers or table", loadPath)
}
content := string(data)
if !strings.Contains(content, "# Skill Registry") ||
!strings.Contains(content, "## Skills") ||
!strings.Contains(content, "| Skill |") {
return Result{}, fmt.Errorf("invalid skill registry format in %q: missing required headers or table", loadPath)
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/skillregistry/registry.go` around lines 181 - 184, The validation in
registry.go’s load path is too permissive because it accepts a file when any one
of the markers is present. Update the check around the content scan so the skill
registry is only accepted when all required markers are present, and return the
existing invalid-format error otherwise. Keep the fix localized to the registry
loading logic that populates `.atl/skill-registry.md`, using the current
header/table marker strings in the condition.


if !force && loadPath == targetRegistryPath && fileExists(targetRegistryPath) && fileExists(cachePath) {
cached := readCachedFingerprint(cachePath)
if strings.HasPrefix(cached, "loaded:") {
return Result{Regenerated: false, Reason: "cache-hit", Registry: targetRegistryPath, Cache: cachePath}, nil
}
}

if err := os.MkdirAll(filepath.Join(cwd, ".atl"), 0o755); err != nil {
return Result{}, fmt.Errorf("create .atl directory: %w", err)
}

if loadPath != targetRegistryPath {
if _, err := filemerge.WriteFileAtomic(targetRegistryPath, data, 0o644); err != nil {
return Result{}, fmt.Errorf("write registry: %w", err)
}
}

skillCount := 0
for _, line := range strings.Split(content, "\n") {
if strings.HasPrefix(line, "| `") {
skillCount++
}
}

fp := "loaded:" + loadPath
cacheBytes, err := json.MarshalIndent(cacheFile{Fingerprint: fp}, "", " ")
if err != nil {
return Result{}, fmt.Errorf("marshal cache fingerprint: %w", err)
}
cacheBytes = append(cacheBytes, '\n')
if _, err := filemerge.WriteFileAtomic(cachePath, cacheBytes, 0o644); err != nil {
return Result{}, fmt.Errorf("write cache fingerprint: %w", err)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

reason := "loaded"
if force {
reason = "forced-load"
}
return Result{Regenerated: true, SkillCount: skillCount, Reason: reason, Registry: targetRegistryPath, Cache: cachePath}, nil
}

func EnsureATLIgnored(cwd string) error {
gitignorePath := filepath.Join(cwd, ".gitignore")
existingBytes, err := os.ReadFile(gitignorePath)
Expand Down
61 changes: 61 additions & 0 deletions internal/skillregistry/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,3 +434,64 @@ func containsPath(paths []string, want string) bool {
}
return false
}

func TestLoadRegistryValidatesAndLoadsRegistry(t *testing.T) {
sourceDir := t.TempDir()
targetDir := t.TempDir()

sourceRegistry := filepath.Join(sourceDir, ".atl", "skill-registry.md")
if err := os.MkdirAll(filepath.Dir(sourceRegistry), 0o755); err != nil {
t.Fatal(err)
}

content := `# Skill Registry — Test
## Contract
Contract description.
## Skills
| Skill | Trigger / description | Scope | Path |
| --- | --- | --- | --- |
| ` + "`custom-skill`" + ` | Custom trigger | project | ` + "`skills/custom/SKILL.md`" + ` |
`
if err := os.WriteFile(sourceRegistry, []byte(content), 0o644); err != nil {
t.Fatal(err)
}

// Test invalid format error (completely invalid)
invalidPath := filepath.Join(sourceDir, "invalid.md")
if err := os.WriteFile(invalidPath, []byte("invalid content"), 0o644); err != nil {
t.Fatal(err)
}
if _, err := LoadRegistry(invalidPath, targetDir, false); err == nil || !strings.Contains(err.Error(), "invalid skill registry format") {
t.Fatalf("expected invalid skill registry format error, got %v", err)
}

// Test partial invalid format error (missing table header)
partialPath := filepath.Join(sourceDir, "partial.md")
if err := os.WriteFile(partialPath, []byte("## Skills\nno table here"), 0o644); err != nil {
t.Fatal(err)
}
if _, err := LoadRegistry(partialPath, targetDir, false); err == nil || !strings.Contains(err.Error(), "invalid skill registry format") {
t.Fatalf("expected invalid skill registry format error, got %v", err)
}

// Test successful load
res, err := LoadRegistry(sourceRegistry, targetDir, false)
if err != nil {
t.Fatalf("LoadRegistry error = %v", err)
}

if !res.Regenerated || res.SkillCount != 1 || res.Reason != "loaded" {
t.Fatalf("unexpected LoadRegistry result: %#v", res)
}

got := readFile(t, filepath.Join(targetDir, RegistryRelPath))
if got != content {
t.Fatalf("loaded content mismatch: got %q, want %q", got, content)
}

cacheContent := readFile(t, filepath.Join(targetDir, CacheRelPath))
if !strings.Contains(cacheContent, "loaded:") {
t.Fatalf("cache fingerprint mismatch: got %q, expected 'loaded:' prefix", cacheContent)
}
}

Loading