Files
multica/server/internal/daemon/execenv/sidecar_manifest_test.go
Bohan Jiang 76c58a4ee8 MUL-3617: remove Gemini CLI runtime (#4503)
* fix: remove gemini cli runtime

Co-authored-by: multica-agent <github@multica.ai>

* fix: skip unsupported custom runtime profiles

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-24 15:15:42 +08:00

1124 lines
41 KiB
Go

package execenv
import (
"errors"
"io/fs"
"os"
"path/filepath"
"reflect"
"sort"
"strings"
"testing"
)
// walkRelative returns every relative path inside root (files and directories),
// sorted, with a `dir/` suffix on directories so dir-vs-file mismatches show up
// in the diff. The root itself is reported as "." so a fully-empty directory
// still surfaces a non-empty fingerprint and an empty-vs-missing comparison
// fails loudly instead of looking identical.
func walkRelative(t *testing.T, root string) []string {
t.Helper()
var entries []string
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
rel, relErr := filepath.Rel(root, path)
if relErr != nil {
return relErr
}
if rel == "." {
entries = append(entries, ".")
return nil
}
if d.IsDir() {
entries = append(entries, rel+string(os.PathSeparator))
return nil
}
entries = append(entries, rel)
return nil
})
if err != nil {
t.Fatalf("walk %s: %v", root, err)
}
sort.Strings(entries)
return entries
}
// snapshotWorkdir captures both the directory listing and the content of every
// regular file inside root, so a round-trip assertion can compare "exactly the
// same bytes everywhere" — not just "no orphan files survived". An empty map
// represents an empty workdir.
type workdirSnapshot struct {
entries []string
files map[string]string
}
func snapshot(t *testing.T, root string) workdirSnapshot {
t.Helper()
snap := workdirSnapshot{files: map[string]string{}}
snap.entries = walkRelative(t, root)
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if d.IsDir() {
return nil
}
rel, relErr := filepath.Rel(root, path)
if relErr != nil {
return relErr
}
data, readErr := os.ReadFile(path)
if readErr != nil {
return readErr
}
snap.files[rel] = string(data)
return nil
})
if err != nil {
t.Fatalf("walk-for-content %s: %v", root, err)
}
return snap
}
func assertSnapshotEqual(t *testing.T, label string, want, got workdirSnapshot) {
t.Helper()
if !reflect.DeepEqual(want.entries, got.entries) {
t.Errorf("[%s] directory listing differs\n want: %v\n got: %v", label, want.entries, got.entries)
}
if !reflect.DeepEqual(want.files, got.files) {
// Find a small diff first so the failure is actionable.
for k, wv := range want.files {
gv, ok := got.files[k]
if !ok {
t.Errorf("[%s] missing file %s after round-trip", label, k)
continue
}
if wv != gv {
t.Errorf("[%s] file %s differs\n want: %q\n got: %q", label, k, wv, gv)
}
}
for k := range got.files {
if _, ok := want.files[k]; !ok {
t.Errorf("[%s] orphan file %s after round-trip", label, k)
}
}
}
}
// runPrepareLikeCycle replays the daemon's local_directory path against the
// supplied workDir and envRoot: writes context files (with manifest tracking),
// injects the runtime brief, then runs the matching cleanups. Tests use this
// to assert byte-exact reversibility without booting the full Prepare/Reuse
// pipeline (which would need a WorkspacesRoot, GC plumbing, etc.).
func runPrepareLikeCycle(t *testing.T, workDir, envRoot, provider string, ctx TaskContextForEnv) {
t.Helper()
manifest := &sidecarManifest{}
if err := writeContextFiles(workDir, provider, ctx, manifest); err != nil {
t.Fatalf("writeContextFiles(%s): %v", provider, err)
}
if err := writeSidecarManifest(envRoot, manifest); err != nil {
t.Fatalf("writeSidecarManifest(%s): %v", provider, err)
}
if _, err := InjectRuntimeConfig(workDir, provider, ctx); err != nil {
t.Fatalf("InjectRuntimeConfig(%s): %v", provider, err)
}
// Mirror daemon.go ordering: runtime config first, sidecars second. The
// order is incidental — neither cleanup touches the other's paths — but
// pinning the same order in tests catches an accidental coupling.
if err := CleanupRuntimeConfig(workDir, provider); err != nil {
t.Fatalf("CleanupRuntimeConfig(%s): %v", provider, err)
}
if err := CleanupSidecars(envRoot); err != nil {
t.Fatalf("CleanupSidecars(%s): %v", provider, err)
}
}
// allFileBasedProviders lists every provider whose `writeContextFiles` /
// `InjectRuntimeConfig` writes into the user's workDir. Codex is included
// because it still writes AGENTS.md + .agent_context/ into the workdir (its
// skills live in codex-home, but that's not in workdir and is out of scope
// for this manifest). Adding a new provider that writes into workDir must
// also add it here so the round-trip invariant is enforced for it on day
// one — review the test diff before merging.
var allFileBasedProviders = []string{
"claude",
"codex",
"copilot",
"opencode",
"openclaw",
"hermes",
"pi",
"cursor",
"kimi",
"kiro",
"antigravity",
}
// TestPrepareThenCleanupSidecarsRoundTripEmptyWorkdir is the headline
// invariant the issue (MUL-2784) calls out: a user repo that contained
// nothing related to Multica before a task ran must contain nothing
// related to Multica after the task finishes — no .agent_context/,
// no .claude/skills/, no .multica/, no stub directories. The test
// runs the full Prepare → Inject → Cleanup cycle for every file-based
// provider against a fresh empty workdir and asserts the directory is
// byte-exactly empty again.
func TestPrepareThenCleanupSidecarsRoundTripEmptyWorkdir(t *testing.T) {
t.Parallel()
for _, provider := range allFileBasedProviders {
provider := provider
t.Run(provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{
Name: "Issue Review",
Description: "Review GH issues",
Content: "Steps to review",
Files: []SkillFileContextForEnv{
{Path: "templates/checklist.md", Content: "- [ ] check"},
},
},
{
Name: "PR Review",
Content: "Review PR diffs",
},
},
ProjectID: "proj-1",
ProjectTitle: "Demo",
}
runPrepareLikeCycle(t, workDir, envRoot, provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, provider, before, after)
})
}
}
// TestPrepareThenCleanupSidecarsPreservesUserSkillSibling pins the
// non-destructive contract from the issue: if the user already keeps a
// hand-authored skill under the same parent directory we use, Cleanup
// must leave it bit-for-bit intact. The user-skill payload is laid down
// BEFORE Prepare runs and snapshotted; after Cleanup the user's skill
// must still exist and the Multica-written sibling must be gone.
func TestPrepareThenCleanupSidecarsPreservesUserSkillSibling(t *testing.T) {
t.Parallel()
// One representative case per provider that writes into a
// provider-native skill directory. Hermes doesn't have a
// native discovery path; they fall back to .agent_context/skills/,
// which is also covered (a user-created sibling under there should
// also survive). Codex is intentionally excluded — its workspace
// skills don't live in workdir, so the "user skill sibling"
// scenario doesn't apply.
cases := []struct {
provider string
userSkillRel string // path under workDir
userSkillFile string // path under userSkillRel
}{
{"claude", filepath.Join(".claude", "skills", "my-own"), "SKILL.md"},
{"copilot", filepath.Join(".github", "skills", "my-own"), "SKILL.md"},
{"opencode", filepath.Join(".opencode", "skills", "my-own"), "SKILL.md"},
{"openclaw", filepath.Join("skills", "my-own"), "SKILL.md"},
{"pi", filepath.Join(".pi", "skills", "my-own"), "SKILL.md"},
{"cursor", filepath.Join(".cursor", "skills", "my-own"), "SKILL.md"},
{"kimi", filepath.Join(".kimi", "skills", "my-own"), "SKILL.md"},
{"kiro", filepath.Join(".kiro", "skills", "my-own"), "SKILL.md"},
{"antigravity", filepath.Join(".agents", "skills", "my-own"), "SKILL.md"},
{"hermes", filepath.Join(".agent_context", "skills", "my-own"), "SKILL.md"},
}
for _, tc := range cases {
tc := tc
t.Run(tc.provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
userDir := filepath.Join(workDir, tc.userSkillRel)
if err := os.MkdirAll(userDir, 0o755); err != nil {
t.Fatalf("seed user skill dir: %v", err)
}
userBody := "---\nname: my-own\n---\n\nUser-authored.\n"
if err := os.WriteFile(filepath.Join(userDir, tc.userSkillFile), []byte(userBody), 0o644); err != nil {
t.Fatalf("seed user skill file: %v", err)
}
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{Name: "Issue Review", Content: "ours"},
},
}
runPrepareLikeCycle(t, workDir, envRoot, tc.provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, tc.provider, before, after)
// Defensive: independently re-read the user skill to make
// sure no clever cleanup heuristic stripped its content.
got, err := os.ReadFile(filepath.Join(userDir, tc.userSkillFile))
if err != nil {
t.Fatalf("user skill went missing after round-trip: %v", err)
}
if string(got) != userBody {
t.Errorf("user skill content changed\n want: %q\n got: %q", userBody, string(got))
}
})
}
}
// TestPrepareThenCleanupSidecarsPreservesUnrelatedUserFiles covers the
// case where the user keeps a non-skill file under a parent we end up
// using — for example `.claude/config.json` next to where we drop
// `.claude/skills/`. Cleanup must rmdir only the directories it created;
// pre-existing siblings (and their parents) must survive.
func TestPrepareThenCleanupSidecarsPreservesUnrelatedUserFiles(t *testing.T) {
t.Parallel()
cases := []struct {
provider string
userFile string // path under workDir
}{
{"claude", filepath.Join(".claude", "settings.json")},
{"copilot", filepath.Join(".github", "CODEOWNERS")},
{"opencode", filepath.Join(".opencode", "config.json")},
{"pi", filepath.Join(".pi", "config.toml")},
{"cursor", filepath.Join(".cursor", "settings.json")},
{"kimi", filepath.Join(".kimi", "config.json")},
{"kiro", filepath.Join(".kiro", "config.json")},
{"antigravity", filepath.Join(".agents", "config.json")},
}
for _, tc := range cases {
tc := tc
t.Run(tc.provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
userPath := filepath.Join(workDir, tc.userFile)
if err := os.MkdirAll(filepath.Dir(userPath), 0o755); err != nil {
t.Fatalf("seed user dir: %v", err)
}
userBody := "user content " + tc.provider
if err := os.WriteFile(userPath, []byte(userBody), 0o644); err != nil {
t.Fatalf("seed user file: %v", err)
}
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{Name: "Issue Review", Content: "ours"},
},
}
runPrepareLikeCycle(t, workDir, envRoot, tc.provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, tc.provider, before, after)
})
}
}
// TestPrepareThenCleanupSidecarsRepeatedCycles guards against the
// failure mode the issue describes most explicitly — every run
// accumulates one more directory layer than the last. Running the cycle
// twice in a row must leave the workdir in the same state as running it
// once (which is the seed state), with the manifest correctly
// regenerated each cycle.
func TestPrepareThenCleanupSidecarsRepeatedCycles(t *testing.T) {
t.Parallel()
for _, provider := range allFileBasedProviders {
provider := provider
t.Run(provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{Name: "Issue Review", Content: "ours"},
},
}
for i := 0; i < 3; i++ {
runPrepareLikeCycle(t, workDir, envRoot, provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, provider, before, after)
}
})
}
}
// TestPrepareThenCleanupSidecarsWithProjectResources extends the
// round-trip to the .multica/project/resources.json branch — a separate
// sidecar write that creates its own intermediate directory tree.
func TestPrepareThenCleanupSidecarsWithProjectResources(t *testing.T) {
t.Parallel()
for _, provider := range allFileBasedProviders {
provider := provider
t.Run(provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
ProjectID: "proj-1",
ProjectTitle: "Demo project",
ProjectResources: []ProjectResourceForEnv{
{
ID: "res-1",
ResourceType: "github_repo",
ResourceRef: []byte(`{"url":"https://github.com/example/repo"}`),
},
},
}
runPrepareLikeCycle(t, workDir, envRoot, provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, provider, before, after)
})
}
}
// TestCleanupSidecarsNoOpWhenManifestMissing pins backward compatibility
// with envRoots that predate the manifest mechanism (older daemons, GC'd
// scratch dirs, fresh tempdirs). Cleanup must be a silent no-op rather
// than an error when there's nothing to clean.
func TestCleanupSidecarsNoOpWhenManifestMissing(t *testing.T) {
t.Parallel()
envRoot := t.TempDir()
if err := CleanupSidecars(envRoot); err != nil {
t.Errorf("CleanupSidecars on empty envRoot returned error: %v", err)
}
if err := CleanupSidecars(""); err != nil {
t.Errorf("CleanupSidecars with empty envRoot returned error: %v", err)
}
}
// TestCleanupSidecarsLeavesUserContentInTrackedDirIntact is the
// directed unit test for the "non-empty rmdir is silently skipped"
// branch. We build a manifest by hand that claims ownership of a
// directory the user later populated; Cleanup must rmdir-skip and
// leave the user's payload in place without surfacing an error.
func TestCleanupSidecarsLeavesUserContentInTrackedDirIntact(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
// Imagine Prepare wrote .multica/sidecar.txt and created
// .multica/ + .multica/project/, then exited. Between Prepare
// and Cleanup the user dropped their own file under .multica/.
managedDir := filepath.Join(workDir, ".multica")
managedProject := filepath.Join(managedDir, "project")
managedFile := filepath.Join(managedProject, "resources.json")
if err := os.MkdirAll(managedProject, 0o755); err != nil {
t.Fatalf("seed dirs: %v", err)
}
if err := os.WriteFile(managedFile, []byte("{}"), 0o644); err != nil {
t.Fatalf("seed managed file: %v", err)
}
userFile := filepath.Join(managedDir, "user-notes.txt")
if err := os.WriteFile(userFile, []byte("hello"), 0o644); err != nil {
t.Fatalf("seed user file: %v", err)
}
manifest := &sidecarManifest{
Files: []string{managedFile},
Dirs: []string{managedDir, managedProject},
}
if err := writeSidecarManifest(envRoot, manifest); err != nil {
t.Fatalf("write manifest: %v", err)
}
if err := CleanupSidecars(envRoot); err != nil {
t.Errorf("CleanupSidecars: %v", err)
}
if _, err := os.Stat(managedFile); !os.IsNotExist(err) {
t.Errorf("managed file %s should be gone, stat err=%v", managedFile, err)
}
if _, err := os.Stat(managedProject); !os.IsNotExist(err) {
t.Errorf("inner managed dir %s should be empty and removed, stat err=%v", managedProject, err)
}
// .multica still holds user-notes.txt, so rmdir must have been
// skipped silently — the directory must survive.
got, err := os.ReadFile(userFile)
if err != nil {
t.Fatalf("user file went missing: %v", err)
}
if string(got) != "hello" {
t.Errorf("user file content changed: %q", string(got))
}
}
// TestCleanupSidecarsDoesNotRemovePreExistingDirs is the directed unit
// test for the "skip recording pre-existing ancestors" branch in
// recordMkdirAll. A directory the user owned before Prepare must NOT
// appear in the manifest, and therefore must NOT be eligible for rmdir
// during Cleanup — even if Cleanup runs after the user removed the
// last file from inside it.
func TestCleanupSidecarsDoesNotRemovePreExistingDirs(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
userDir := filepath.Join(workDir, ".claude")
if err := os.MkdirAll(userDir, 0o755); err != nil {
t.Fatalf("seed user dir: %v", err)
}
manifest := &sidecarManifest{}
target := filepath.Join(userDir, "skills", "ours")
if err := recordMkdirAll(target, 0o755, manifest); err != nil {
t.Fatalf("recordMkdirAll: %v", err)
}
for _, d := range manifest.Dirs {
if d == userDir {
t.Fatalf("manifest must not record pre-existing user dir %s\nfull dirs: %v", userDir, manifest.Dirs)
}
}
if err := writeSidecarManifest(envRoot, manifest); err != nil {
t.Fatalf("write manifest: %v", err)
}
if err := CleanupSidecars(envRoot); err != nil {
t.Fatalf("CleanupSidecars: %v", err)
}
if _, err := os.Stat(userDir); err != nil {
t.Errorf("pre-existing user dir %s removed by cleanup: %v", userDir, err)
}
}
// TestRecordWriteFileRefusesToOverwritePreExistingFile pins the
// invariant the PR #3444 review identified as must-fix: a pre-existing
// path is user-owned (or stale state we can't safely distinguish from
// user-owned), and recordWriteFile MUST refuse to mutate it. The
// function returns errPathPreExists, the file's bytes survive
// untouched, and nothing is added to the manifest.
//
// The previous behaviour — overwrite at write time, then skip the
// manifest record so Cleanup wouldn't undo the damage — destroyed
// user data and called it preservation.
func TestRecordWriteFileRefusesToOverwritePreExistingFile(t *testing.T) {
t.Parallel()
dir := t.TempDir()
target := filepath.Join(dir, "user.md")
if err := os.WriteFile(target, []byte("user bytes"), 0o644); err != nil {
t.Fatalf("seed: %v", err)
}
m := &sidecarManifest{}
err := recordWriteFile(target, []byte("ours"), 0o644, m)
if !errors.Is(err, errPathPreExists) {
t.Fatalf("recordWriteFile must return errPathPreExists for a pre-existing target, got: %v", err)
}
for _, f := range m.Files {
if f == target {
t.Errorf("manifest must not record pre-existing user file %s", target)
}
}
// User bytes must survive the refused write — the whole point of
// the new behaviour is that pre-existing paths are NEVER mutated.
got, err := os.ReadFile(target)
if err != nil {
t.Fatalf("read back: %v", err)
}
if string(got) != "user bytes" {
t.Errorf("user bytes must survive refused write\n want: %q\n got: %q", "user bytes", string(got))
}
}
// TestRecordWriteFileRefusesToOverwriteSymlinkOrDir is the directed
// edge-case companion: any kind of pre-existing entry at the target
// path — symlink or directory, not just a regular file — counts as
// user-owned. We use Lstat to detect symlinks (so a symlink to a
// regular file doesn't accidentally pass the existence check via
// Stat-follows-symlinks semantics) and bail out the same way.
func TestRecordWriteFileRefusesToOverwriteSymlinkOrDir(t *testing.T) {
t.Parallel()
dir := t.TempDir()
// Pre-existing symlink (dangling — points nowhere). Should still
// count as "occupied" so we don't accidentally create a file the
// symlink would then expose at an unexpected target.
symlinkPath := filepath.Join(dir, "symlink.md")
if err := os.Symlink(filepath.Join(dir, "does-not-exist"), symlinkPath); err != nil {
t.Skipf("symlink not supported on this platform: %v", err)
}
if err := recordWriteFile(symlinkPath, []byte("ours"), 0o644, &sidecarManifest{}); !errors.Is(err, errPathPreExists) {
t.Errorf("recordWriteFile on dangling symlink should refuse, got: %v", err)
}
// Pre-existing directory at the file path — should also refuse.
dirPath := filepath.Join(dir, "subdir")
if err := os.Mkdir(dirPath, 0o755); err != nil {
t.Fatalf("seed dir: %v", err)
}
if err := recordWriteFile(dirPath, []byte("ours"), 0o644, &sidecarManifest{}); !errors.Is(err, errPathPreExists) {
t.Errorf("recordWriteFile on pre-existing directory should refuse, got: %v", err)
}
}
// TestSidecarManifestRoundTripJSON pins the on-disk encoding so a
// future field rename or json-tag change doesn't silently break Cleanup
// for envRoots that carry an in-flight manifest at the moment of an
// upgrade.
func TestSidecarManifestRoundTripJSON(t *testing.T) {
t.Parallel()
envRoot := t.TempDir()
original := &sidecarManifest{
Files: []string{"/x/.agent_context/issue_context.md"},
Dirs: []string{"/x/.agent_context"},
}
if err := writeSidecarManifest(envRoot, original); err != nil {
t.Fatalf("write: %v", err)
}
raw, err := os.ReadFile(filepath.Join(envRoot, sidecarManifestFile))
if err != nil {
t.Fatalf("read: %v", err)
}
for _, want := range []string{"files", "dirs", "issue_context.md", ".agent_context"} {
if !strings.Contains(string(raw), want) {
t.Errorf("manifest JSON missing %q\n got: %s", want, string(raw))
}
}
}
// sameSlugSkillProviderCases lists every file-based provider that
// writes per-skill subdirectories under workDir, paired with the
// natural skill path (relative to workDir) that a user-installed
// skill named "issue-review" would occupy. The same-slug collision
// matrix below replays the user-skill-already-present scenario per
// provider and asserts byte-exact round-trip — including that the
// user's SKILL.md bytes survive the task untouched and our
// collision-free sibling (which lives under .../issue-review-multica/)
// is fully cleaned up.
//
// Codex skills live under codex-home (not workdir), so the per-skill
// collision branch doesn't apply to it. Hermes falls back to
// .agent_context/skills/ same as the default; Hermes goes there too.
var sameSlugSkillProviderCases = []struct {
provider string
skillDir string // relative path under workDir for the colliding slug
}{
{"claude", filepath.Join(".claude", "skills", "issue-review")},
{"copilot", filepath.Join(".github", "skills", "issue-review")},
{"opencode", filepath.Join(".opencode", "skills", "issue-review")},
{"openclaw", filepath.Join("skills", "issue-review")},
{"pi", filepath.Join(".pi", "skills", "issue-review")},
{"cursor", filepath.Join(".cursor", "skills", "issue-review")},
{"kimi", filepath.Join(".kimi", "skills", "issue-review")},
{"kiro", filepath.Join(".kiro", "skills", "issue-review")},
{"antigravity", filepath.Join(".agents", "skills", "issue-review")},
{"hermes", filepath.Join(".agent_context", "skills", "issue-review")},
}
// TestPrepareThenCleanupSidecarsSameSlugCollisionPerProvider is the
// must-fix byte-exact matrix the PR #3444 review required: per
// provider, seed a user skill at the exact slug Multica would use
// (`.claude/skills/issue-review/SKILL.md` etc.), run the full
// Prepare → Inject → Cleanup cycle with a Multica skill of the same
// name, and assert the workdir snapshot is byte-identical to the
// seed. The user's SKILL.md must not be touched, and the Multica
// sibling (which lives at `<slug>-multica`) must be fully removed by
// CleanupSidecars.
func TestPrepareThenCleanupSidecarsSameSlugCollisionPerProvider(t *testing.T) {
t.Parallel()
for _, tc := range sameSlugSkillProviderCases {
tc := tc
t.Run(tc.provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
userSkillDir := filepath.Join(workDir, tc.skillDir)
if err := os.MkdirAll(userSkillDir, 0o755); err != nil {
t.Fatalf("seed user skill dir: %v", err)
}
userBody := "---\nname: issue-review\ndescription: user-authored\n---\n\nUser owns this slug.\n"
userSkillFile := filepath.Join(userSkillDir, "SKILL.md")
if err := os.WriteFile(userSkillFile, []byte(userBody), 0o644); err != nil {
t.Fatalf("seed user SKILL.md: %v", err)
}
// A second user-authored file under the same skill dir
// — exercises the case where the user has more than
// just SKILL.md in their skill (templates, scripts).
userExtra := filepath.Join(userSkillDir, "notes.md")
if err := os.WriteFile(userExtra, []byte("private notes"), 0o644); err != nil {
t.Fatalf("seed user extra file: %v", err)
}
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{
Name: "Issue Review",
Description: "Multica's version",
Content: "---\nname: issue-review\n---\n\nMultica skill content.\n",
Files: []SkillFileContextForEnv{
{Path: "templates/checklist.md", Content: "- [ ] check"},
},
},
},
}
runPrepareLikeCycle(t, workDir, envRoot, tc.provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, tc.provider, before, after)
// Defensive double-check: read the user's files
// directly to make sure their content survived.
gotBody, err := os.ReadFile(userSkillFile)
if err != nil {
t.Fatalf("user SKILL.md went missing: %v", err)
}
if string(gotBody) != userBody {
t.Errorf("user SKILL.md mutated\n want: %q\n got: %q", userBody, string(gotBody))
}
gotExtra, err := os.ReadFile(userExtra)
if err != nil {
t.Fatalf("user extra file went missing: %v", err)
}
if string(gotExtra) != "private notes" {
t.Errorf("user extra file mutated\n want: %q\n got: %q", "private notes", string(gotExtra))
}
})
}
}
// TestPrepareThenCleanupSidecarsIssueContextCollisionPerProvider is
// the matching byte-exact matrix for `.agent_context/issue_context.md`
// — a Multica-only namespace file. If the user already has a file at
// that path, the writer must refuse to overwrite it (the runtime
// brief carries the same facts anyway) and CleanupSidecars must
// leave the user's file alone. This covers EVERY file-based provider
// (including Codex, whose skills don't live in workdir but whose
// .agent_context/ write goes through the same path).
func TestPrepareThenCleanupSidecarsIssueContextCollisionPerProvider(t *testing.T) {
t.Parallel()
for _, provider := range allFileBasedProviders {
provider := provider
t.Run(provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
if err := os.MkdirAll(filepath.Join(workDir, ".agent_context"), 0o755); err != nil {
t.Fatalf("seed dir: %v", err)
}
userBody := "# user-authored issue_context.md\n\nDo not touch.\n"
userPath := filepath.Join(workDir, ".agent_context", "issue_context.md")
if err := os.WriteFile(userPath, []byte(userBody), 0o644); err != nil {
t.Fatalf("seed user file: %v", err)
}
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
}
runPrepareLikeCycle(t, workDir, envRoot, provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, provider, before, after)
got, err := os.ReadFile(userPath)
if err != nil {
t.Fatalf("user issue_context.md went missing: %v", err)
}
if string(got) != userBody {
t.Errorf("user issue_context.md mutated\n want: %q\n got: %q", userBody, string(got))
}
})
}
}
// TestPrepareThenCleanupSidecarsProjectResourcesCollisionPerProvider
// is the matching byte-exact matrix for `.multica/project/
// resources.json` — the other Multica-only namespace file. Same
// invariant: pre-existing user content survives the round-trip
// untouched even when the task ships project resources of its own.
func TestPrepareThenCleanupSidecarsProjectResourcesCollisionPerProvider(t *testing.T) {
t.Parallel()
for _, provider := range allFileBasedProviders {
provider := provider
t.Run(provider, func(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
if err := os.MkdirAll(filepath.Join(workDir, ".multica", "project"), 0o755); err != nil {
t.Fatalf("seed dir: %v", err)
}
userBody := `{"user":"owns this file"}`
userPath := filepath.Join(workDir, ".multica", "project", "resources.json")
if err := os.WriteFile(userPath, []byte(userBody), 0o644); err != nil {
t.Fatalf("seed user file: %v", err)
}
before := snapshot(t, workDir)
ctx := TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
ProjectID: "proj-1",
ProjectTitle: "Demo",
ProjectResources: []ProjectResourceForEnv{
{
ID: "res-1",
ResourceType: "github_repo",
ResourceRef: []byte(`{"url":"https://github.com/example/repo"}`),
},
},
}
runPrepareLikeCycle(t, workDir, envRoot, provider, ctx)
after := snapshot(t, workDir)
assertSnapshotEqual(t, provider, before, after)
got, err := os.ReadFile(userPath)
if err != nil {
t.Fatalf("user resources.json went missing: %v", err)
}
if string(got) != userBody {
t.Errorf("user resources.json mutated\n want: %q\n got: %q", userBody, string(got))
}
})
}
}
// TestAllocateCollisionFreeSkillDir pins the slug-suffix policy:
// first try the natural slug, then `-multica`, then `-multica-2`,
// `-multica-3`, … The PR-review concern is "Multica skill must still
// be discoverable" — this test demonstrates that we pick a sibling
// path under the same skillsParent rather than dropping the skill or
// nesting it under the user's directory.
func TestAllocateCollisionFreeSkillDir(t *testing.T) {
t.Parallel()
parent := t.TempDir()
// 1) No collision → use the base slug as-is.
slug, dir, err := allocateCollisionFreeSkillDir(parent, "issue-review")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if slug != "issue-review" {
t.Errorf("first allocation should use base slug; got %q", slug)
}
if dir != filepath.Join(parent, "issue-review") {
t.Errorf("first allocation path = %q, want under parent", dir)
}
// 2) Pre-existing user dir at the base slug → bump to `-multica`.
if err := os.MkdirAll(filepath.Join(parent, "issue-review"), 0o755); err != nil {
t.Fatalf("seed user dir: %v", err)
}
slug, dir, err = allocateCollisionFreeSkillDir(parent, "issue-review")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if slug != "issue-review-multica" {
t.Errorf("second allocation should bump to `-multica`; got %q", slug)
}
if dir != filepath.Join(parent, "issue-review-multica") {
t.Errorf("second allocation path = %q, want under parent", dir)
}
// 3) Pre-existing collision at the bumped slug too → bump again.
if err := os.MkdirAll(filepath.Join(parent, "issue-review-multica"), 0o755); err != nil {
t.Fatalf("seed bumped dir: %v", err)
}
slug, dir, err = allocateCollisionFreeSkillDir(parent, "issue-review")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if slug != "issue-review-multica-2" {
t.Errorf("third allocation should be `-multica-2`; got %q", slug)
}
if dir != filepath.Join(parent, "issue-review-multica-2") {
t.Errorf("third allocation path = %q, want under parent", dir)
}
}
// TestPrepareThenCleanupSidecarsMultiSkillCollisionFreeAllocation is
// the end-to-end coverage for the collision-free sibling: a user has
// `.claude/skills/issue-review/SKILL.md`, the task ships an
// `Issue Review` skill, the Multica sibling must land at a different
// slug (so the agent still sees it), AND Cleanup must remove the
// Multica sibling entirely without touching the user's.
func TestPrepareThenCleanupSidecarsMultiSkillCollisionFreeAllocation(t *testing.T) {
t.Parallel()
workDir := t.TempDir()
envRoot := t.TempDir()
userDir := filepath.Join(workDir, ".claude", "skills", "issue-review")
if err := os.MkdirAll(userDir, 0o755); err != nil {
t.Fatalf("seed: %v", err)
}
userBody := "user-authored skill\n"
userFile := filepath.Join(userDir, "SKILL.md")
if err := os.WriteFile(userFile, []byte(userBody), 0o644); err != nil {
t.Fatalf("seed file: %v", err)
}
// Run only the inject side first — verify the Multica skill
// landed at a NEW path under the same parent, AND the user's
// path is untouched.
manifest := &sidecarManifest{}
if err := writeContextFiles(workDir, "claude", TaskContextForEnv{
IssueID: "11111111-2222-3333-4444-555555555555",
AgentSkills: []SkillContextForEnv{
{Name: "Issue Review", Content: "Multica's version\n"},
},
}, manifest); err != nil {
t.Fatalf("writeContextFiles: %v", err)
}
multicaDir := filepath.Join(workDir, ".claude", "skills", "issue-review-multica")
if _, err := os.Stat(filepath.Join(multicaDir, "SKILL.md")); err != nil {
t.Errorf("Multica sibling skill should exist at %s: %v", multicaDir, err)
}
got, err := os.ReadFile(userFile)
if err != nil {
t.Fatalf("user SKILL.md went missing during inject: %v", err)
}
if string(got) != userBody {
t.Errorf("user SKILL.md mutated during inject\n want: %q\n got: %q", userBody, string(got))
}
// Now persist manifest + run cleanup. After cleanup the
// Multica sibling is gone; user's path survives.
if err := writeSidecarManifest(envRoot, manifest); err != nil {
t.Fatalf("write manifest: %v", err)
}
if err := CleanupSidecars(envRoot); err != nil {
t.Fatalf("CleanupSidecars: %v", err)
}
if _, err := os.Stat(multicaDir); !os.IsNotExist(err) {
t.Errorf("Multica sibling should be removed by Cleanup; stat err=%v", err)
}
got, err = os.ReadFile(userFile)
if err != nil {
t.Fatalf("user SKILL.md went missing after cleanup: %v", err)
}
if string(got) != userBody {
t.Errorf("user SKILL.md mutated after cleanup\n want: %q\n got: %q", userBody, string(got))
}
}
// TestCleanupSidecarsSwallowsMissingAndNonEmptyDirs pins the two
// branches that CleanupSidecars is expected to silently skip without
// surfacing an error:
//
// - the recorded directory no longer exists (race-safe ENOENT)
// - the recorded directory has been populated with user content
// (ENOTEMPTY — preserving user content is the whole point)
func TestCleanupSidecarsSwallowsMissingAndNonEmptyDirs(t *testing.T) {
t.Parallel()
// Case 1: recorded dir is missing → ENOENT must be swallowed.
envRoot1 := t.TempDir()
missing := filepath.Join(t.TempDir(), "never-existed")
if err := writeSidecarManifest(envRoot1, &sidecarManifest{Dirs: []string{missing}}); err != nil {
t.Fatalf("write missing-dir manifest: %v", err)
}
if err := CleanupSidecars(envRoot1); err != nil {
t.Errorf("CleanupSidecars(missing dir) should swallow ENOENT silently, got: %v", err)
}
// Case 2: recorded dir has user content → ENOTEMPTY must be
// swallowed and the user content must survive.
envRoot2 := t.TempDir()
workDir2 := t.TempDir()
recordedDir := filepath.Join(workDir2, "recorded")
if err := os.MkdirAll(recordedDir, 0o755); err != nil {
t.Fatalf("seed recorded dir: %v", err)
}
userFile := filepath.Join(recordedDir, "user.txt")
if err := os.WriteFile(userFile, []byte("user content"), 0o644); err != nil {
t.Fatalf("seed user file: %v", err)
}
if err := writeSidecarManifest(envRoot2, &sidecarManifest{Dirs: []string{recordedDir}}); err != nil {
t.Fatalf("write non-empty-dir manifest: %v", err)
}
if err := CleanupSidecars(envRoot2); err != nil {
t.Errorf("CleanupSidecars(non-empty dir) should swallow ENOTEMPTY silently, got: %v", err)
}
got, err := os.ReadFile(userFile)
if err != nil {
t.Fatalf("user content went missing: %v", err)
}
if string(got) != "user content" {
t.Errorf("user content mutated: %q", string(got))
}
}
// TestCleanupSidecarsSurfacesEACCESOnEmptyRecordedDir is the directed
// integration test for the should-fix branch: rmdir fails (here with
// EACCES because the parent is read-only), the directory is verifiably
// empty, and CleanupSidecars must surface the original rmdir error
// instead of silently skipping it.
//
// Skipped when running as root because chmod is bypassed for uid 0 —
// the EACCES we want to trigger never materialises. CI's daemon runner
// is unprivileged, so the branch is exercised in CI.
func TestCleanupSidecarsSurfacesEACCESOnEmptyRecordedDir(t *testing.T) {
t.Parallel()
if os.Geteuid() == 0 {
t.Skip("chmod is bypassed for uid 0; cannot synthesize EACCES on rmdir")
}
workDir := t.TempDir()
envRoot := t.TempDir()
parent := filepath.Join(workDir, "parent")
recorded := filepath.Join(parent, "empty-dir")
if err := os.MkdirAll(recorded, 0o755); err != nil {
t.Fatalf("seed: %v", err)
}
if err := writeSidecarManifest(envRoot, &sidecarManifest{Dirs: []string{recorded}}); err != nil {
t.Fatalf("write manifest: %v", err)
}
// Strip write from parent so rmdir(recorded) fails EACCES.
// rmdir requires write on the PARENT (it modifies parent's
// directory entries); readdir(recorded) only requires read on
// recorded itself, which we leave intact. That isolates this
// test to the "empty + rmdir refused" branch.
if err := os.Chmod(parent, 0o555); err != nil {
t.Fatalf("chmod parent: %v", err)
}
// Restore parent permissions for t.TempDir() teardown.
t.Cleanup(func() { _ = os.Chmod(parent, 0o755) })
err := CleanupSidecars(envRoot)
if err == nil {
t.Fatal("CleanupSidecars should surface the EACCES rmdir error, got nil")
}
if !strings.Contains(err.Error(), "empty-dir") {
t.Errorf("expected surfaced error to reference recorded path, got: %v", err)
}
if !strings.Contains(err.Error(), "rmdir") {
t.Errorf("expected surfaced error to come from rmdir branch, got: %v", err)
}
}
// TestCleanupSidecarsSurfacesEACCESWhenReadDirFailsToo is the matching
// test for the must-fix branch: rmdir fails AND the post-rmdir ReadDir
// also fails. CleanupSidecars must preserve the ORIGINAL rmdir error
// rather than swallowing it because dirHasEntries couldn't read the
// directory. This is the exact failure mode PR #3444 review surfaced
// — the v1 helper returned (true) on ReadDir error, which made
// CleanupSidecars treat the locked dir as "non-empty" and silently
// drop the rmdir EACCES.
//
// Skipped when running as root for the same reason as above.
func TestCleanupSidecarsSurfacesEACCESWhenReadDirFailsToo(t *testing.T) {
t.Parallel()
if os.Geteuid() == 0 {
t.Skip("chmod is bypassed for uid 0; cannot synthesize EACCES on rmdir + readdir")
}
workDir := t.TempDir()
envRoot := t.TempDir()
parent := filepath.Join(workDir, "parent")
recorded := filepath.Join(parent, "locked-dir")
if err := os.MkdirAll(recorded, 0o755); err != nil {
t.Fatalf("seed: %v", err)
}
if err := writeSidecarManifest(envRoot, &sidecarManifest{Dirs: []string{recorded}}); err != nil {
t.Fatalf("write manifest: %v", err)
}
// Strip both: read on recorded so ReadDir(recorded) fails EACCES,
// write on parent so rmdir(recorded) also fails EACCES. The
// helper must report ok=false; CleanupSidecars must surface the
// rmdir error anyway.
if err := os.Chmod(recorded, 0o000); err != nil {
t.Fatalf("chmod recorded: %v", err)
}
if err := os.Chmod(parent, 0o555); err != nil {
_ = os.Chmod(recorded, 0o755)
t.Fatalf("chmod parent: %v", err)
}
t.Cleanup(func() {
_ = os.Chmod(parent, 0o755)
_ = os.Chmod(recorded, 0o755)
})
err := CleanupSidecars(envRoot)
if err == nil {
t.Fatal("CleanupSidecars should surface the rmdir error even when ReadDir also fails, got nil")
}
if !strings.Contains(err.Error(), "locked-dir") {
t.Errorf("expected surfaced error to reference recorded path, got: %v", err)
}
if !strings.Contains(err.Error(), "rmdir") {
t.Errorf("expected surfaced error to be the ORIGINAL rmdir error, not the ReadDir failure, got: %v", err)
}
}
// TestDirHasEntries is the directed unit test for the helper Cleanup
// uses to tell ENOTEMPTY (silently skip) apart from genuine rmdir
// errors (surface). The helper returns (hasEntries, ok); CleanupSidecars
// surfaces the original rmdir error whenever ok=false so a ReadDir
// failure on a chmod'd / not-a-directory / faulted path doesn't get
// laundered into a phantom "non-empty directory" skip.
func TestDirHasEntries(t *testing.T) {
t.Parallel()
root := t.TempDir()
empty := filepath.Join(root, "empty")
if err := os.Mkdir(empty, 0o755); err != nil {
t.Fatalf("seed empty: %v", err)
}
if has, ok := dirHasEntries(empty); !ok || has {
t.Errorf("dirHasEntries(empty dir) = (%v, %v), want (false, true)", has, ok)
}
full := filepath.Join(root, "full")
if err := os.Mkdir(full, 0o755); err != nil {
t.Fatalf("seed full: %v", err)
}
if err := os.WriteFile(filepath.Join(full, "file"), []byte(""), 0o644); err != nil {
t.Fatalf("seed full content: %v", err)
}
if has, ok := dirHasEntries(full); !ok || !has {
t.Errorf("dirHasEntries(non-empty dir) = (%v, %v), want (true, true)", has, ok)
}
missing := filepath.Join(root, "missing")
if has, ok := dirHasEntries(missing); !ok || has {
t.Errorf("dirHasEntries(missing dir) = (%v, %v), want (false, true) — ENOENT collapses to empty so the rmdir-race resolves cleanly", has, ok)
}
// ReadDir failure path: pass a regular file. ReadDir returns
// ENOTDIR, which is NOT ENOENT, so the helper must report
// (false, false) — "couldn't tell" — and let CleanupSidecars
// surface the underlying rmdir error rather than silently
// skipping. This is the must-fix branch PR #3444 review caught:
// the v1 helper returned (true) here, which made every ReadDir
// failure look like ENOTEMPTY.
regular := filepath.Join(root, "regular.txt")
if err := os.WriteFile(regular, []byte("not a dir"), 0o644); err != nil {
t.Fatalf("seed regular: %v", err)
}
if has, ok := dirHasEntries(regular); ok || has {
t.Errorf("dirHasEntries(regular file) = (%v, %v), want (false, false) — ENOTDIR must NOT be laundered as ENOTEMPTY", has, ok)
}
}