Skip to content
Merged
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
19 changes: 13 additions & 6 deletions internal/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1380,11 +1380,18 @@ func (a *App) generateAgentsMD() {
}

// Hooks (SPEC-004 Part C)
// Hooks are indexed regardless of extension: .json files are
// settings-fragments the sync merges into .claude/settings.json;
// everything else (shell scripts, helpers) is a companion file
// that fragments reference by path. Filtering to .json here
// silently erased script-style hooks from the index on every
// regeneration — files that exist on disk must never disappear
// from AGENTS.md.
hooksBucketDir := filepath.Join(agentsDir, "hooks")
if entries, err := os.ReadDir(hooksBucketDir); err == nil {
var hookFiles []string
for _, e := range entries {
if !e.IsDir() && filepath.Ext(e.Name()) == ".json" {
if !e.IsDir() && !strings.HasPrefix(e.Name(), ".") {
hookFiles = append(hookFiles, e.Name())
}
}
Expand All @@ -1393,11 +1400,11 @@ func (a *App) generateAgentsMD() {
b.WriteString("## Hooks\n\n")
for _, name := range hookFiles {
link := ".agents/hooks/" + name
srcPath := filepath.Join(hooksBucketDir, name)
// Hook fragments don't have markdown frontmatter for
// description — just link the file.
b.WriteString(fmt.Sprintf("- [%s](%s)\n", name, link))
_ = srcPath
if filepath.Ext(name) == ".json" {
b.WriteString(fmt.Sprintf("- [%s](%s) — merged into `.claude/settings.json`\n", name, link))
} else {
b.WriteString(fmt.Sprintf("- [%s](%s) — companion file (not merged; reference it from a fragment)\n", name, link))
}
}
b.WriteString("\n")
}
Expand Down
38 changes: 35 additions & 3 deletions internal/agent/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"sort"
"strings"
)

// hooks.go implements SPEC-004 Part C: merging .agents/hooks/*.json
Expand Down Expand Up @@ -89,6 +90,18 @@ func (a *App) MergeHooks(hooksDir, claudeSettingsPath, statePath string) (int, e
if err != nil {
return 0, err
}
// Legacy/script-style hooks directories hold executable scripts
// (.sh etc.), not JSON fragments. Those files are still indexed
// and synced (the bucket dir symlink covers them) but nothing
// here will ever merge them into settings.json — say so once per
// sync instead of silently doing nothing, so a user migrating
// from a script convention isn't left wondering why their hooks
// never fire.
if len(frags) == 0 {
if n := countNonFragmentHookFiles(hooksDir); n > 0 {
a.Warn(fmt.Sprintf("hooks/: %d non-JSON file(s) (scripts?) are indexed and synced but NOT merged into settings.json — reference them from a <name>.json fragment ({\"event\":…,\"hooks\":[{\"type\":\"command\",\"command\":\".agents/hooks/<script>\"}]}) for Claude to run them", n))
}
}
if len(frags) == 0 && !fileExists(statePath) && !fileExists(claudeSettingsPath) {
return 0, nil // nothing to merge and nothing to clean
}
Expand Down Expand Up @@ -295,8 +308,8 @@ func readHooksState(path string) (*hooksStateInternal, error) {
// hooksStateInternal is the on-disk shape, extended with a creation
// flag that the public hooksState doesn't expose.
type hooksStateInternal struct {
Entries map[string]hooksStateEntry `json:"entries"`
SyncAgentsCreated bool `json:"syncAgentsCreated,omitempty"`
Entries map[string]hooksStateEntry `json:"entries"`
SyncAgentsCreated bool `json:"syncAgentsCreated,omitempty"`
}

// findHookOwner returns the fragment name that owns a given hook
Expand Down Expand Up @@ -483,4 +496,23 @@ func (a *App) CleanHooks(claudeSettingsPath, statePath string) (int, error) {
return 0, err
}
return removed, nil
}
}

// countNonFragmentHookFiles reports how many plain files in the hooks
// bucket are not JSON fragments (companion scripts, READMEs, legacy
// .sh hooks). Used only for the "these never merge" warning.
func countNonFragmentHookFiles(hooksDir string) int {
entries, err := os.ReadDir(hooksDir)
if err != nil {
return 0
}
n := 0
for _, e := range entries {
name := e.Name()
if e.IsDir() || strings.HasPrefix(name, ".") || filepath.Ext(name) == ".json" {
continue
}
n++
}
return n
}
76 changes: 73 additions & 3 deletions internal/agent/index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,9 @@ func TestListMDFilesRecursive_Nested(t *testing.T) {
t.Errorf("expected 3 files, got %d: %v", len(names), names)
}
want := map[string]bool{
"plan": true,
"effort-a/rollout": true,
"effort-b/testing": true,
"plan": true,
"effort-a/rollout": true,
"effort-b/testing": true,
}
for _, n := range names {
if !want[n] {
Expand Down Expand Up @@ -515,3 +515,73 @@ func TestCmdIndex_NoFix(t *testing.T) {
t.Fatal(err)
}
}

// TestIndex_PreservesScriptHooks reproduces the acme regression:
// .agents/hooks/ holding shell scripts (the pre-SPEC-004 convention)
// lost its ## Hooks section on regeneration because the index
// filtered to .json fragments. Files on disk must never disappear
// from the index.
func TestIndex_PreservesScriptHooks(t *testing.T) {
a, _, _ := newLocalIndexTestApp(t)
hooksDir := filepath.Join(a.ProjectRoot, ".agents", "hooks")
if err := os.MkdirAll(hooksDir, 0o755); err != nil {
t.Fatal(err)
}
os.WriteFile(filepath.Join(hooksDir, "backfill-session-id.sh"), []byte("#!/bin/sh\n"), 0o755)
os.WriteFile(filepath.Join(hooksDir, "guard.json"), []byte(`{"event":"PreToolUse","hooks":[{"type":"command","command":".agents/hooks/backfill-session-id.sh"}]}`), 0o644)

if err := a.CmdIndex(); err != nil {
t.Fatal(err)
}
idx := readFile(t, filepath.Join(a.ProjectRoot, "AGENTS.md"))
if !strings.Contains(idx, "## Hooks") {
t.Fatalf("Hooks section missing:\n%s", idx)
}
if !strings.Contains(idx, "backfill-session-id.sh") {
t.Fatalf("script hook dropped from index:\n%s", idx)
}
if !strings.Contains(idx, "companion file") {
t.Fatalf("script hook missing not-merged annotation:\n%s", idx)
}
if !strings.Contains(idx, "guard.json") || !strings.Contains(idx, "merged into") {
t.Fatalf("json fragment annotation missing:\n%s", idx)
}

// Regeneration is stable: scripts survive a second index.
if err := a.CmdIndex(); err != nil {
t.Fatal(err)
}
idx2 := readFile(t, filepath.Join(a.ProjectRoot, "AGENTS.md"))
if idx2 != idx {
t.Fatal("second index changed the Hooks section")
}
}

// TestMergeHooks_WarnsOnScriptOnlyDir: a hooks dir with only scripts
// never merges anything — the sync must say so rather than silently
// doing nothing.
func TestMergeHooks_WarnsOnScriptOnlyDir(t *testing.T) {
a, _, _ := newLocalIndexTestApp(t)
hooksDir := filepath.Join(a.ProjectRoot, ".agents", "hooks")
os.MkdirAll(hooksDir, 0o755)
os.WriteFile(filepath.Join(hooksDir, "sync-permissions.sh"), []byte("#!/bin/sh\n"), 0o755)

var out bytes.Buffer
a.Stdout = &out
n, err := a.MergeHooks(hooksDir,
filepath.Join(a.ProjectRoot, ".claude", "settings.json"),
filepath.Join(a.ProjectRoot, ".agents", ".sync", "claude-hooks-state.json"))
if err != nil {
t.Fatal(err)
}
if n != 0 {
t.Fatalf("merged %d fragments from a script-only dir", n)
}
if !strings.Contains(out.String(), "NOT merged") {
t.Fatalf("no warning for script-only hooks dir; output:\n%s", out.String())
}
// And no settings.json conjured out of nothing.
if _, err := os.Stat(filepath.Join(a.ProjectRoot, ".claude", "settings.json")); !os.IsNotExist(err) {
t.Fatal("script-only dir created a settings.json")
}
}
Loading