mirror of
https://github.com/multica-ai/multica.git
synced 2026-06-17 11:48:42 +02:00
44feb3d06d
A skill_file row whose path is the skill's own SKILL.md (persisted by older builds or direct create/update API calls) collides with the primary content the daemon writes itself, failing task prep with errPathPreExists on every non-codex local runtime (#3489). #3526 guarded this with strings.EqualFold(path, "SKILL.md") at the daemon write site and the three API ingress points, but the stored path is not canonicalized: "./SKILL.md" or "sub/../SKILL.md" slip past the exact-match guard while filepath.Join still resolves them onto the same SKILL.md, so prep can still break. Extract one canonical helper, skill.IsReservedContentPath, that cleans the path before the case-insensitive compare, and use it at all four sites (execenv writeSkillFiles, skill create, update, single-file upsert). Add a daemon-side regression test for writeSkillFiles ignoring a bundled SKILL.md (exact + "./" spellings) — the load-bearing fix previously had only API-layer coverage — plus a unit test for the helper. Existing poisoned rows are intentionally left in place (skipped at prep) per the decision on MUL-2928. MUL-2928 Follow-up to #3526; supersedes #3560. Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>
66 lines
2.4 KiB
Go
66 lines
2.4 KiB
Go
package execenv
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
// TestWriteSkillFilesIgnoresBundledSkillMd is the daemon-side regression guard
|
|
// for #3489 / MUL-2928. A skill whose Files include the skill's own SKILL.md
|
|
// (stored as a supporting file by older builds or direct create/update API
|
|
// calls) used to fail task prep with errPathPreExists: writeSkillFiles writes
|
|
// the primary content to dir/SKILL.md first, then the supporting-files loop
|
|
// tried to write the duplicate over it. The non-canonical "./SKILL.md"
|
|
// spelling resolves onto the same path and must be skipped too. Every
|
|
// non-codex provider hit this; codex is unaffected because it writes with a
|
|
// nil manifest (plain os.WriteFile, no refuse-to-overwrite).
|
|
func TestWriteSkillFilesIgnoresBundledSkillMd(t *testing.T) {
|
|
t.Parallel()
|
|
skillsDir := filepath.Join(t.TempDir(), ".claude", "skills")
|
|
|
|
skills := []SkillContextForEnv{
|
|
{
|
|
Name: "Issue Review",
|
|
Content: "Primary skill body.",
|
|
Files: []SkillFileContextForEnv{
|
|
{Path: "README.md", Content: "readme"},
|
|
{Path: "SKILL.md", Content: "duplicate primary, must be skipped"},
|
|
{Path: "./SKILL.md", Content: "non-canonical duplicate, must be skipped"},
|
|
{Path: "helper.go", Content: "package main"},
|
|
},
|
|
},
|
|
}
|
|
|
|
// A non-nil manifest is the production Prepare path: recordWriteFile
|
|
// enforces refuse-to-overwrite, so a duplicate SKILL.md would error here
|
|
// if it were not skipped.
|
|
manifest := &sidecarManifest{}
|
|
if err := writeSkillFiles(skillsDir, skills, manifest); err != nil {
|
|
t.Fatalf("writeSkillFiles errored on a bundled SKILL.md: %v", err)
|
|
}
|
|
|
|
skillDir := filepath.Join(skillsDir, "issue-review")
|
|
|
|
// SKILL.md must hold the primary content, never a bundled duplicate.
|
|
got, err := os.ReadFile(filepath.Join(skillDir, "SKILL.md"))
|
|
if err != nil {
|
|
t.Fatalf("read SKILL.md: %v", err)
|
|
}
|
|
if !strings.Contains(string(got), "Primary skill body.") {
|
|
t.Errorf("SKILL.md = %q, want primary content", string(got))
|
|
}
|
|
if strings.Contains(string(got), "must be skipped") {
|
|
t.Error("SKILL.md was overwritten by a bundled duplicate")
|
|
}
|
|
|
|
// Unique supporting files are still written — this also proves the loop
|
|
// continued past the skipped duplicates instead of aborting on them.
|
|
for _, name := range []string{"README.md", "helper.go"} {
|
|
if _, err := os.Stat(filepath.Join(skillDir, name)); err != nil {
|
|
t.Errorf("expected supporting file %s to be written: %v", name, err)
|
|
}
|
|
}
|
|
}
|