Skip to content
Draft
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
12 changes: 4 additions & 8 deletions internal/apply/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@ import (
// hookDeployBase is the repo- or home-relative directory under which agentfiles
// deploys directory-form hook contents. For user-level applies this becomes
// $HOME/<hookDeployBaseUser>/<name>; for repo-level it becomes
// <repoDir>/<hookDeployBaseRepo>/<name>.
//
// hookDeployBaseRepo must NOT live under ".agentfiles/" — that path is the repo
// manifest *file* (".agentfiles"), so ".agentfiles/hooks" collides with it and
// every repo-level apply fails with "open .agentfiles/hooks: not a directory".
// It sits beside the manifest instead, mirroring ".agentfiles.lock".
// <repoDir>/<hookDeployBaseRepo>/<name>. The values live in the hooks package,
// which also uses them to reclaim marker-stripped managed entries during merge.
const (
hookDeployBaseUser = ".local/share/agentfiles/hooks"
hookDeployBaseRepo = ".agentfiles-hooks"
hookDeployBaseUser = hooks.DeployBaseUser
hookDeployBaseRepo = hooks.DeployBaseRepo
)

// isUserLayout reports whether a layout name targets user-level paths
Expand Down
73 changes: 73 additions & 0 deletions internal/hooks/hooks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,79 @@ func TestMergeIntoSettings_CreatesFile(t *testing.T) {
}
}

func TestMergeIntoSettings_ReclaimsStrippedMarkers(t *testing.T) {
dir := t.TempDir()
settingsPath := filepath.Join(dir, "settings.json")

// Simulate a settings file rewritten by another tool (e.g. Claude Code),
// which drops the _agentfiles marker from managed entries: two stripped
// copies of a managed hook sit next to a genuine user hook.
managedCmd := `$HOME/` + DeployBaseUser + `/sync-skills/scripts/sync.sh`
stripped := `{"matcher": "startup", "hooks": [{"type": "command", "command": "` + managedCmd + `"}]}`
userEntry := `{"hooks": [{"type": "command", "command": "$HOME/bin/mine.sh"}]}`
initial := `{"hooks": {"SessionStart": [` + stripped + `, ` + stripped + `, ` + userEntry + `]}}`
if err := os.WriteFile(settingsPath, []byte(initial), 0o644); err != nil {
t.Fatal(err)
}

managed := map[string]*HookFile{
"sync-skills": {
Event: "SessionStart",
Matcher: "startup",
Hooks: []json.RawMessage{json.RawMessage(`{"type": "command", "command": "` + managedCmd + `"}`)},
},
}
if err := MergeIntoSettings(settingsPath, managed, FormatNested); err != nil {
t.Fatalf("MergeIntoSettings: %v", err)
}

data, _ := os.ReadFile(settingsPath)
var topLevel map[string]json.RawMessage
json.Unmarshal(data, &topLevel)
var hooks map[string][]json.RawMessage
json.Unmarshal(topLevel["hooks"], &hooks)

// The stripped duplicates must be reclaimed: one marked managed entry
// plus the untouched user hook.
if len(hooks["SessionStart"]) != 2 {
t.Fatalf("SessionStart hooks = %d, want 2", len(hooks["SessionStart"]))
}
var markers, userKept int
for _, e := range hooks["SessionStart"] {
if extractAgentfilesMarker(e) == "sync-skills" {
markers++
} else if !hasAgentfilesMarker(e) {
userKept++
}
}
if markers != 1 {
t.Errorf("marked sync-skills entries = %d, want 1", markers)
}
if userKept != 1 {
t.Errorf("user entries kept = %d, want 1", userKept)
}
}

func TestIsStrippedManagedEntry(t *testing.T) {
cases := []struct {
name string
entry string
want bool
}{
{"nested managed", `{"hooks": [{"command": "$HOME/` + DeployBaseUser + `/x/run.sh"}]}`, true},
{"nested repo-level managed", `{"hooks": [{"command": "` + DeployBaseRepo + `/x/run.sh"}]}`, true},
{"flat managed", `{"command": "$HOME/` + DeployBaseUser + `/x/run.sh"}`, true},
{"user hook", `{"hooks": [{"command": "$HOME/bin/mine.sh"}]}`, false},
{"mixed commands", `{"hooks": [{"command": "$HOME/` + DeployBaseUser + `/x/run.sh"}, {"command": "$HOME/bin/mine.sh"}]}`, false},
{"no commands", `{"matcher": "startup"}`, false},
}
for _, tc := range cases {
if got := isStrippedManagedEntry(json.RawMessage(tc.entry)); got != tc.want {
t.Errorf("%s: got %v, want %v", tc.name, got, tc.want)
}
}
}

func TestRemoveManaged(t *testing.T) {
dir := t.TempDir()
settingsPath := filepath.Join(dir, "settings.json")
Expand Down
56 changes: 54 additions & 2 deletions internal/hooks/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,23 @@ import (
"os"
"path/filepath"
"sort"
"strings"
)

// DeployBaseUser and DeployBaseRepo are the home- and repo-relative directories
// under which agentfiles deploys directory-form hook contents (used by
// internal/apply). They double as an ownership signal during merging: an entry
// whose commands all point into one of these directories is agentfiles-managed
// even when its _agentfiles marker is missing — other writers of the settings
// file (notably Claude Code) re-serialize hook entries and drop unknown fields.
//
// DeployBaseRepo must NOT live under ".agentfiles/" — that path is the repo
// manifest *file* (".agentfiles"), so ".agentfiles/hooks" collides with it and
// every repo-level apply fails with "open .agentfiles/hooks: not a directory".
// It sits beside the manifest instead, mirroring ".agentfiles.lock".
const (
DeployBaseUser = ".local/share/agentfiles/hooks"
DeployBaseRepo = ".agentfiles-hooks"
)

// MergeIntoSettings merges managed hooks into a settings/hooks JSON file.
Expand Down Expand Up @@ -40,11 +57,12 @@ func MergeIntoSettings(settingsPath string, managed map[string]*HookFile, format
}
}

// Remove all previously managed entries (those with _agentfiles field).
// Remove all previously managed entries: those with an _agentfiles field,
// plus marker-stripped survivors whose commands identify them as ours.
for event, entries := range existingHooks {
var kept []json.RawMessage
for _, entry := range entries {
if !hasAgentfilesMarker(entry) {
if !hasAgentfilesMarker(entry) && !isStrippedManagedEntry(entry) {
kept = append(kept, entry)
}
}
Expand Down Expand Up @@ -210,6 +228,40 @@ func buildNestedEntry(name string, hf *HookFile) (json.RawMessage, error) {
return data, nil
}

// isStrippedManagedEntry reports whether an unmarked entry is agentfiles-managed
// anyway: it has at least one command, and every command points into an
// agentfiles hook deploy dir. Covers both the nested shape
// ({matcher, hooks: [{command}]}) and the flat Cursor shape ({command, ...}).
// File-form hooks with commands outside the deploy dirs are not reclaimable
// this way; they still rely on the _agentfiles marker.
func isStrippedManagedEntry(entry json.RawMessage) bool {
var obj struct {
Command string `json:"command"`
Hooks []struct {
Command string `json:"command"`
} `json:"hooks"`
}
if err := json.Unmarshal(entry, &obj); err != nil {
return false
}
cmds := make([]string, 0, len(obj.Hooks)+1)
if obj.Command != "" {
cmds = append(cmds, obj.Command)
}
for _, h := range obj.Hooks {
cmds = append(cmds, h.Command)
}
if len(cmds) == 0 {
return false
}
for _, cmd := range cmds {
if !strings.Contains(cmd, DeployBaseUser+"/") && !strings.Contains(cmd, DeployBaseRepo+"/") {
return false
}
}
return true
}

// hasAgentfilesMarker returns true if the JSON entry contains an "_agentfiles" field.
func hasAgentfilesMarker(entry json.RawMessage) bool {
return extractAgentfilesMarker(entry) != ""
Expand Down