-
Notifications
You must be signed in to change notification settings - Fork 41
merge the stop hook into an existing settings.json #500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
9bc7357
93f46e3
9a3274f
d668650
adbf765
357e493
fc47142
190b2f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ import ( | |
| "bytes" | ||
| "encoding/json" | ||
| "fmt" | ||
| "slices" | ||
| "sort" | ||
|
|
||
| udiff "github.com/aymanbagabas/go-udiff" | ||
|
|
@@ -25,6 +26,11 @@ const CommitIfFilter = "Bash(git commit*)" | |
| // to the current format without leaving a stale duplicate group behind. | ||
| const legacyCommitMatcher = "Bash(git commit*)" | ||
|
|
||
| // StopCommand is the Stop hook command that chunk manages. Merge identifies | ||
| // chunk's own Stop entry by this exact string, so it must stay in sync with the | ||
| // command written by Build and BuildCodex. | ||
| const StopCommand = "chunk validate" | ||
|
|
||
| // MergeResult holds the computed merge without performing any I/O. | ||
| type MergeResult struct { | ||
| Original []byte // existing settings.json content (re-marshaled for normalized formatting) | ||
|
|
@@ -73,6 +79,11 @@ func Merge(existing, generated []byte) (*MergeResult, error) { | |
| // Merge hooks.PreToolUse — replace the chunk-managed hook group by matcher. | ||
| mergeHooks(merged, generatedMap) | ||
|
|
||
| // Merge hooks.Stop — replace the chunk-managed group by command. Without | ||
| // this a repo that already had a settings.json keeps its commit hooks but | ||
| // never gets the Stop hook, so validation stops running at session end. | ||
| mergeStopHooks(merged, generatedMap) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i don't think you need a new function to merge the stop hooks, merge hooks just wasn't handling the case probably. i'd extend merge hooks to also include the stop hook merging as well.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from claude: done in adbf765 - kept the difference in how they merge, since it is not incidental: chunk owns the whole PreToolUse group (matched by matcher) but only a single Stop entry (matched by command), because a user may have their own entries in that same group and replacing the group would delete them. That is now documented on the one function instead of split across two, and the duplicated "create hooks if absent" block collapses into a no behavior change — the existing tests pin both halves independently: six exercise generated settings carrying only PreToolUse, six only Stop, three both. |
||
|
|
||
| mergedBytes, err := json.MarshalIndent(merged, "", " ") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("marshal merged settings: %w", err) | ||
|
|
@@ -174,28 +185,28 @@ func mergeHooks(merged, generated map[string]interface{}) { | |
| } | ||
|
|
||
| // Replace existing group with same matcher (or legacy matcher), or append. | ||
| replaced := false | ||
| for i, g := range mergedPreToolUse { | ||
| group, isMap := g.(map[string]interface{}) | ||
| if !isMap { | ||
| continue | ||
| } | ||
| matcher, _ := group["matcher"].(string) | ||
| if matcher == CommitMatcher || matcher == legacyCommitMatcher { | ||
| mergedPreToolUse[i] = chunkGroup | ||
| replaced = true | ||
| break | ||
| } | ||
| } | ||
| if !replaced { | ||
| mergedPreToolUse = append(mergedPreToolUse, chunkGroup) | ||
| } | ||
| mergedHooks["PreToolUse"] = replaceOrAppend(mergedPreToolUse, isChunkCommitGroup, chunkGroup) | ||
| } | ||
|
|
||
| mergedHooks["PreToolUse"] = mergedPreToolUse | ||
| // isChunkCommitGroup reports whether a PreToolUse group is the chunk-managed | ||
| // commit group, accepting the legacy matcher so older settings migrate in place. | ||
| func isChunkCommitGroup(g interface{}) bool { | ||
| group, ok := g.(map[string]interface{}) | ||
| if !ok { | ||
| return false | ||
| } | ||
| matcher, _ := group["matcher"].(string) | ||
| return matcher == CommitMatcher || matcher == legacyCommitMatcher | ||
| } | ||
|
|
||
| // mergeStopHooks replaces the chunk-managed Stop hook group (identified by the | ||
| // "chunk validate" command) within Stop, preserving all other Stop groups. | ||
| // mergeStopHooks installs chunk's Stop hook entry into hooks.Stop, preserving | ||
| // every entry chunk does not own. | ||
| // | ||
| // Chunk owns a single entry (identified by StopCommand), not a whole group. A | ||
| // user may have added their own entries to that same group, so the entry is | ||
| // replaced in place and its siblings are left alone — replacing the enclosing | ||
| // group would silently delete them. Only when no chunk entry exists anywhere is | ||
| // chunk's own group appended. | ||
| func mergeStopHooks(merged, generated map[string]interface{}) { | ||
| genHooks, ok := generated["hooks"].(map[string]interface{}) | ||
| if !ok { | ||
|
|
@@ -206,15 +217,19 @@ func mergeStopHooks(merged, generated map[string]interface{}) { | |
| return | ||
| } | ||
|
|
||
| // Find the chunk-managed group in generated Stop hooks. | ||
| var chunkGroup interface{} | ||
| // Find chunk's group in the generated Stop hooks, and the entry within it. | ||
| var chunkGroup, chunkEntry interface{} | ||
| for _, g := range genStop { | ||
| if isChunkStopGroup(g) { | ||
| chunkGroup = g | ||
| _, entries, isGroup := stopGroupEntries(g) | ||
| if !isGroup { | ||
| continue | ||
| } | ||
| if i := slices.IndexFunc(entries, isChunkStopEntry); i >= 0 { | ||
| chunkGroup, chunkEntry = g, entries[i] | ||
| break | ||
| } | ||
| } | ||
| if chunkGroup == nil { | ||
| if chunkEntry == nil { | ||
| return | ||
| } | ||
|
|
||
|
|
@@ -230,43 +245,53 @@ func mergeStopHooks(merged, generated map[string]interface{}) { | |
| mergedStop = []interface{}{} | ||
| } | ||
|
|
||
| // Replace existing chunk-managed group, or append. | ||
| replaced := false | ||
| for i, g := range mergedStop { | ||
| if isChunkStopGroup(g) { | ||
| mergedStop[i] = chunkGroup | ||
| replaced = true | ||
| break | ||
| // Update chunk's entry wherever it already lives, keeping the user's own | ||
| // entries in that group intact. | ||
| for _, g := range mergedStop { | ||
| group, entries, isGroup := stopGroupEntries(g) | ||
| if !isGroup || !slices.ContainsFunc(entries, isChunkStopEntry) { | ||
| continue | ||
| } | ||
| } | ||
| if !replaced { | ||
| mergedStop = append(mergedStop, chunkGroup) | ||
| group["hooks"] = replaceOrAppend(entries, isChunkStopEntry, chunkEntry) | ||
| mergedHooks["Stop"] = mergedStop | ||
| return | ||
| } | ||
|
|
||
| mergedHooks["Stop"] = mergedStop | ||
| mergedHooks["Stop"] = append(mergedStop, chunkGroup) | ||
| } | ||
|
|
||
| // isChunkStopGroup reports whether a Stop hook group is chunk-managed, | ||
| // identified by containing a hook with command "chunk validate". | ||
| func isChunkStopGroup(g interface{}) bool { | ||
| // stopGroupEntries returns a Stop hook group's map and its list of hook entries. | ||
| func stopGroupEntries(g interface{}) (map[string]interface{}, []interface{}, bool) { | ||
| group, ok := g.(map[string]interface{}) | ||
| if !ok { | ||
| return false | ||
| return nil, nil, false | ||
| } | ||
| hooks, ok := group["hooks"].([]interface{}) | ||
| entries, ok := group["hooks"].([]interface{}) | ||
| if !ok { | ||
| return nil, nil, false | ||
| } | ||
| return group, entries, true | ||
| } | ||
|
|
||
| // isChunkStopEntry reports whether a Stop hook entry is the one chunk manages, | ||
| // identified by its command. | ||
| func isChunkStopEntry(h interface{}) bool { | ||
| entry, ok := h.(map[string]interface{}) | ||
| if !ok { | ||
| return false | ||
| } | ||
| for _, h := range hooks { | ||
| entry, ok := h.(map[string]interface{}) | ||
| if !ok { | ||
| continue | ||
| } | ||
| if cmd, _ := entry["command"].(string); cmd == "chunk validate" { | ||
| return true | ||
| } | ||
| cmd, _ := entry["command"].(string) | ||
| return cmd == StopCommand | ||
| } | ||
|
|
||
| // replaceOrAppend replaces the first element of s that match reports true for | ||
| // with v, appending v when nothing matches. Returns the possibly-grown slice. | ||
| func replaceOrAppend[T any](s []T, match func(T) bool, v T) []T { | ||
| if i := slices.IndexFunc(s, match); i >= 0 { | ||
| s[i] = v | ||
| return s | ||
| } | ||
| return false | ||
| return append(s, v) | ||
| } | ||
|
|
||
| // MergeCodex computes the merged .codex/hooks.json from existing and generated bytes. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.